-
Notifications
You must be signed in to change notification settings - Fork 221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test/system: Connect system tests to Meson #1062
base: main
Are you sure you want to change the base?
test/system: Connect system tests to Meson #1062
Conversation
Build failed. ❌ unit-test TIMED_OUT in 10m 48s |
The project already has a number of different tests plugged into the build system. To have greater control over which tests are executed, assign them into different suites. 'linting' for formatting and linting and 'unit' for unit tests and generally self-contained tests testing a small portion of Toolbx. containers#1062
A bit of plumbing in the build files has been done to accommodate the change as much as possible. If we're to use meson.build in the 'test/system' subdir we better also install the data from there. This also adjust the CI playbooks to launch the tests via Meson by specifying the specific test suite ('system'). containers#1062
9ae1251
to
e4d823f
Compare
Build succeeded. ✔️ unit-test SUCCESS in 7m 07s |
The project already has a number of different tests plugged into the build system. To have greater control over which tests are executed, assign them into different suites. 'linting' for formatting and linting and 'unit' for unit tests and generally self-contained tests testing a small portion of Toolbx. Also, in a follow-up commit the system tests will be plugged into Meson and having a way of running only some of the tests will be needed. containers#1062
e4d823f
to
0c42ab0
Compare
A bit of plumbing in the build files has been done to accommodate the change as much as possible. If we're to use meson.build in the 'test/system' subdir we better also install the data from there. This also adjust the CI playbooks to launch the tests via Meson by specifying the specific test suite ('system'). This change also means the test suite needs to be run from the 'test/system' subdirectory for the bats helper libraries to be discovered correctly. containers#1062
Build failed. ❌ unit-test RETRY_LIMIT in 7m 33s |
A bit of plumbing in the build files has been done to accommodate the change as much as possible. If we're to use meson.build in the 'test/system' subdir we better also install the data from there. This also adjust the CI playbooks to launch the tests via Meson by specifying the specific test suite ('system'). This change also means the test suite needs to be run from the 'test/system' subdirectory for the bats helper libraries to be discovered correctly. containers#1062
0c42ab0
to
eb57489
Compare
Build failed. ✔️ unit-test SUCCESS in 8m 21s |
A bit of plumbing in the build files has been done to accommodate the change as much as possible. If we're to use meson.build in the 'test/system' subdir we better also install the data from there. This also adjust the CI playbooks to launch the tests via Meson by specifying the specific test suite ('system'). This change also means the test suite needs to be run from the 'test/system' subdirectory for the bats helper libraries to be discovered correctly. containers#1062
eb57489
to
20e9f6b
Compare
Build failed. ✔️ unit-test SUCCESS in 8m 13s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this @HarryMichal !
@@ -19,13 +19,8 @@ subid_dep = cc.find_library('subid', has_headers: ['shadow/subid.h']) | |||
go = find_program('go') | |||
go_md2man = find_program('go-md2man') | |||
|
|||
bats = find_program('bats', required: false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really necessary to move them? Isn't it a lot more obvious to have all the dependencies listed in the top level meson.build
? Otherwise, distributors will have to go through all the meson.build
files, which most won't do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not necessary. To me it made sense to have purely testing dependencies specified in the test
subdirectory. I can move those back to the root build file.
shellcheck = find_program('shellcheck', required: false) | ||
skopeo = find_program('skopeo', required: false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the case of Podman and Skopeo, they aren't just dependencies for the test suite, but also runtime dependencies for the main toolbox(1)
binary. So moving them under test/system
can be misleading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving the podman
binary is an oversight and I didn't realise at the time of writing that skopeo
is now a runtime dependency. That wasn't the case in the past.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like the output of the system tests changed.
They no longer have a working counter:
fedora-38 | ▶ 1/1 version: Check version using option --version in 52ms OK
fedora-38 | ▶ 1/1 help: Try to run toolbox with no command in 203ms OK
fedora-38 | ▶ 1/1 help: Run command 'help' in 146ms OK
It's always 1/1 and doesn't show how many more tests are left to go.
The reasons for a failure are also no longer shown:
fedora-rawhide | ▶ 1/1 version: Check version using option --version in 49ms OK
fedora-rawhide | ▶ 1/1 help: Try to run toolbox with no command in 212ms OK
fedora-rawhide | ▶ 1/1 help: Run command 'help' in 166ms FAIL
Finally, are the codespell tests failing on Ubuntu?
This seems to be a deliberate design decision from Meson. This issue seems to be related to that. I'll ask around.
That is a known issue (mesonbuild/meson#11185) and is yet to be resolved. We could contribute that feature but who has the time :). Until then (and most likely even after) we can upload
Once we start uploading the log artifacts, we'll know for sure :). |
A bit of plumbing in the build files has been done to accommodate the
change as much as possible. If we're to use meson.build in the
'test/system' subdir we better also install the data from there.
This also adjust the CI playbooks to launch the tests via Meson by
specifying the specific test suite ('system').