Skip to content

refactor test directories - #2326

Merged
MagnusS merged 19 commits into
includeos:mainfrom
mazunki:ref-dirs
Oct 29, 2025
Merged

refactor test directories#2326
MagnusS merged 19 commits into
includeos:mainfrom
mazunki:ref-dirs

Conversation

@mazunki

@mazunkimazunki commented Oct 20, 2025

Copy link
Copy Markdown
Contributor

This PR reorganizes the ./test/ subdirectory and reunites a few files which are specific to tests only into this subdirectory. ./test.sh is now under here too.

There is now a clear separation between what constitutes as a unit test, and what constitutes as an integration test at the top level. Before, this was done under each target. This will make it easier to later refactor ./test.sh.

This comes with quite many atomic commits, which might seem scary at first sight; but doing it this way makes it easier to understand each change. I hope.

I am pretty confident we're running all the same tests, but I would appreciate someone double checking it. All 8 (previously 7) test sets are passing on my end.

I've noticed several tests are never actually being run: util/{tar,tar_gz}/*, posix/*, plugin/*, mod/*, hw/*, fs/*, unless I'm missing something. This remains the same as it was.

I also cannot sea any use of /userspace nor /test/userspace within any of the scripts. I haven't touched these, but I have to say it's a bit confusing: what does it do, and why does it exist? Is it legacy code?


In a future PR I would like to move unittests.nix into here too, but (while still possible) would be better done after refactoring some of the other *.nix files to reduce maintenance burden.

@mazunki

Copy link
Copy Markdown
ContributorAuthor

Useful tip: tree -d test/ is useful to navigate tests. find -empty is also worth looking at:

$ find -empty
./.git/objects/info
./.git/refs/tags
./api/net/http/parse.hpp
./src/plugins/system_log.cpp
./test/tests-integration/hw/serial/README.md
./test/tests-integration/performance/.keep
./test/tests-integration/fs/memdisk/sector0.disk
./test/tests-unit/performance/.keep
./test/tests-unit/stl/.keep

Also there's several references to conan:

$ grep -rn conan
grep: .git/objects/pack/pack-f1464c7e43d57642af99b907de2ae8a1459fb08b.pack: binary file matches
CHANGELOG.md:13: - 3rd party dependencies are now built and managed in Jenkins. All recipes can be found [here](https://github.com/includeos/conan)
CHANGELOG.md:17: - A repo to install Conan configs for IncludeOS: [conan_config](https://github.com/includeos/conan_config)
cmake/includeos.cmake:54:#TODO improve this to get the platform from conan somehow
cmake/os.cmake:282: #if in conan expect it to be in order ?
deps/uzlib/default.nix:17: # same version as listed in ./conanfile.py
deps/botan/WIP.md:2:The current conanfile builds botan 3.4, which is the latest at the time of writing.
deps/s2n/default.nix:8: # ./conanfile.py lists 0.8, but there are not tags in the repo with version < 0.9.0
lib/LiveUpdate/README.md:3:# Put Liveupdate in conan editable mode
lib/LiveUpdate/README.md:6:1. Remove the version variable from the `conanfile.py`.
lib/LiveUpdate/README.md:8:awk '!/version = conan_tools/' lib/LiveUpdate/conanfile.py > temp && mv temp lib/LiveUpdate/conanfile.py
lib/LiveUpdate/README.md:12:conan editable add lib/LiveUpdate liveupdate/9.9.9@includeos/latest --layout lib/LiveUpdate/layout.txt
lib/LiveUpdate/README.md:16:conan install -if lib/LiveUpdate/build lib/LiveUpdate
lib/LiveUpdate/README.md:20:conan build -sf . -bf lib/LiveUpdate/build lib/LiveUpdate
src/net/CMakeLists.txt:12: #TODO get from conan
src/plugins/CMakeLists.txt:43:#handle build targets for conan editable
test/misc/solo5-hvt/test.sh:18:conan install .. -pr clang-6.0-linux-x86_64
vmbuild/CMakeLists.txt:20:#TODO pull vmbuild conanfile.py inn when not building with conan to get deps
.gitignore:35:build_conan/

I just mention this since it's probably worth cleaning that up at some point.

@alfrebalfreb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conditional approval: I prefer ./test/integration over ./test/tests-integration - besides that this is all good.

There is a case to be made for keeping tests testing the same thing next to one another, since most humans will probably find it easier to understand which tests we have for a certain set of features if they're bundled together. So you're prioritizing reorganizing this to make test.sh maintainance slightly easier - which was not very hard to begin with. Still, I don't object - it's no big deal. Just change the name.

Adding a separate category for memory tests is a big improvement - thanks for that!

Comment threadmisc/debug/service.gdb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the perspective of maintaining test.sh this makes sense. But from the perspective of working on a specific feature, or understanding what coverage we have for it, it's a bit worse. But either way is fine, so this is ok.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intend to make that sort of overview rather simple after refactoring test.sh (again). If we requested to test some module (say net/tcp), it will report that there were no unit tests, integration tests or performance/stres found for that unit. Not an error, but an informative message.

Personally I use find and tree a lot for this form of generic overview, but I do see your point.

Comment threadtest/CMakeLists.txt
Comment threadtest/lest
Comment threadtest/stress/net/CMakeLists.txt
Comment threadtest/test.sh

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh well, ok.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You prefer to clutter the top level directory? I was hoping to move unittests.nix into here eventually too. I can revert it if you think that's better.

Comment threadtest/test.sh Outdated
# to make sure core functionality is not broken by missing locks etc. when waking up more cores.
nix-shell --pure --arg smp true $CCACHE_FLAG --argstr unikernel ${INTEGRATION_TESTS}/net/udp --run ./test.py
nix-shell --pure --arg smp true $CCACHE_FLAG --argstr unikernel ${INTEGRATION_TESTS}/kernel/paging --run ./test.py
nix-shell --pure --arg smp true $CCACHE_FLAG --argstr unikernel ${INTEGRATION_TESTS}/memory/paging --run ./test.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, this is better

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related: I don't have a good argument, but I feel ilke kernel is a very broad term for anything test-related. Everything is a kernel test, is it not? The files under it probably should all be moved to more semantic locations.

Comment threadtest/CMakeLists.txt
${UNIT_TESTS}/util/base64.cpp
${UNIT_TESTS}/util/bitops.cpp
${UNIT_TESTS}/util/buddy_alloc_test.cpp
${UNIT_TESTS}/memory/alloc/buddy_alloc_test.cpp

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, much better

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downside to this change is that it no longer matches the structure of src/. Oh well. (Maybe that should be moved too)

Comment threadtest/test.sh Outdated
Comment threadtest/test.sh
#
exclusions=(
)
run_testsuite "${INTEGRATION_TESTS}/memory" "${exclusions[@]}"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you catch the other missing ones? I feel they should be added too, but probably best on its own PR.

@mazunki

mazunki commented Oct 22, 2025

Copy link
Copy Markdown
ContributorAuthor

Removed the prefix and reverted performance to stress, all 8 tests are passing ok.

@mazunki
mazunkiforce-pushed the ref-dirs branch 2 times, most recently from 9f3a85d to 382b435CompareOctober 27, 2025 15:01
@mazunki

Copy link
Copy Markdown
ContributorAuthor

Rebased to merge with #2316. Tests are still happy.

@MagnusS

Copy link
Copy Markdown
Member

@mazunki looks like #2309 introduced new conflicts...

@mazunki

mazunki commented Oct 28, 2025

Copy link
Copy Markdown
ContributorAuthor

Weird. It rebased cleanly on my end. Let's see if this helps. Running tests now.

@mazunki

Copy link
Copy Markdown
ContributorAuthor

Yep, they're all fine. Maybe Github just wanted confirmation on the path changes?

@MagnusSMagnusS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally lgtm, but README also needs an update to refer to the new test.sh location

@mazunki

mazunki commented Oct 29, 2025

Copy link
Copy Markdown
ContributorAuthor

I have updated the README. One can now also actually use the unikernel.nix as a shell. Tests are still passing on my end.

@MagnusS
MagnusS merged commit ddd0585 into includeos:mainOct 29, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@mazunki@MagnusS@alfreb