CI: run tests that need root as root #2878
Conversation
seems to work |
contrib/cirrus/runner.sh
Outdated
| # that matches all these names. | ||
| # With that we don't have to run everything twice and can just run the ones that | ||
| # actually need to be root. | ||
| test_filter=$(sed -n '$!N;/\n[[:space:]]ensureTestCanCreateImages(.*$/P;D' storage/*_test.go | |
There was a problem hiding this comment.
The sed command looks correct but I worry it will be hard to maintain.
git grep --show-function does something similar natively (looking for a preceding line that doesn’t start with whitespace), maybe
git grep -h --show-function ensureTestCanCreateImages | sed -n 's/func \(Test[[:alnum:]]*\)(.*/^\1\$/p'
That would also avoid the need for precise positioning of ensureTestCanCreateImages.
There was a problem hiding this comment.
git grep --show-function
huh, didn't know that is a thing. That certainly looks much nicer.
contrib/cirrus/runner.sh
Outdated
| test_filter=$(sed -n '$!N;/\n[[:space:]]ensureTestCanCreateImages(.*$/P;D' storage/*_test.go | | ||
| sed 's/func \([a-zA-Z0-9_]*\).*/^\1\$/' | | ||
| paste -sd "|" -) | ||
| go test -v -run "$test_filter" ./storage/ |
There was a problem hiding this comment.
This does not handle BUILDTAGS, we run the test twice with different flags tags (admittedly that difference doesn’t matter for these tests) —and I’ll need to add more BUILDFLAGS in an unrelated PR.
Doing this via a Make rule would keep all of that logic centralized.
There was a problem hiding this comment.
Ah yeah I didn't pay to much attention for this as the storage dir didn't need it.
I can move the entire logic it to the Makefile if you like, but then what should I call the target test-root-only?
There was a problem hiding this comment.
Yeah, naming that is hard, and it seems unlikely to have external users. (And if we did add test-root-only, should we overengineer the logic to not be just ensureTestCanCreateImages, which is not exactly “root only”? Let’s just not deal with that, I think.)
Maybe make test BUILDTAGS="$BUILDTAGS" BUILDFLAGS=$the_new_filter here, and use BUILDFLAGS += in the Makefile. That would be sufficient to centralize the Go option logic, without committing to the root/non-root test split.
There was a problem hiding this comment.
flag provided but not defined: -run
I’m afraid that using BUILDFLAGS for this doesn’t work — we’d need something like specialized TESTFLAGS. (Also, #2873 (comment) , for -v)
869cfc7 to
58e39c6
Compare
For CI it would be useful if we can pass extra arguments to go test such as -v or extra filters via -run. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This is a quick and dirty fix but should get the job done and ensures the tests are actually run by CI. The idea is simple to not waste resources we just look for the test names using ensureTestCanCreateImages() to skip as rootless. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
For CI logs it is extremely useful to get the full output to see what tests were run and skipped so one can verifiy CI is working as expected and runs all out tests. Signed-off-by: Paul Holzinger <pholzing@redhat.com>
|
Ok now it should work. Makefile and git gave me lot of troubles. Makefile var really want to interpret the |
For reference, https://www.gnu.org/software/make/manual/make.html#Overriding : I didn’t know that |
This is a quick and dirty fix but should get the job done and ensures the tests are actually run by CI. The idea is simple to not waste resources we just look for the test names using
ensureTestCanCreateImages() to skip as rootless.