-
Notifications
You must be signed in to change notification settings - Fork 165
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
Add option to run CI on all kernel flavors, plus other improvements #417
Conversation
This makes it simple to run all tests for a specific flavor. That's especially useful if submitting a pull request, where CI will test the default flavor, but you want to double check some other flavor first. Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
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.
This is awesome, thank you! One minor comment below.
vmtest/download.py
Outdated
# risk of filling up the limited disk space is Github Actions. Set a limit | ||
# of no more than 5 files which can be downloaded ahead of time. This is a | ||
# magic number with no testing, but should work. | ||
maxsize = 5 if IN_GITHUB_ACTIONS else 0 |
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.
This would be cleaner as an argument passed to download_in_thread()
. Other than that, this is a really clever solution.
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! Would you like vmtest/__main__.py
to pass this parameter based on GITHUB_ACTIONS
environment variable? There are two callers, but of course setup.py
is the only one used in CI. But in case we switch over to vmtest/__main__.py
we may want the logic in both places.
(edit: I'll leave vmtest/__main__.py
alone for now)
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.
Good catch, yeah, just copy the logic in both places. I have a dream of getting off setuptools entirely, and vmtest/__main__.py
will replace this if that ever happens.
The Github Actions disk space can get filled up if we test all kernel flavors. So, delete kernels after testing when running in Github Actions. However, if the download speed is far greater than the speed of tests, then there is also the possibility that the disk space will be filled up by the download thread, before the tests will run and delete the older kernels. So we also need to ensure the download thread can't get too far ahead of the tests, which we implement by setting a max size on the download queue. Signed-off-by: Stephen Brennan <stephen.s.brennan@oracle.com>
I was hoping that adding the label to this PR would do a test run, but maybe since it's not in the main branch yet, it didn't. Oh well, this looks good! |
This branch adds three improvements related to testing different kernel flavors:
python setup.py test -K -f flavor
. This is just a convenience that I wanted.main
branch (whereas we do test all Python versions for every push tomain
). I think that makes sense, because pushes to main can be rather frequent, and testing each flavor triples the test burden (which is of course multiplicative with all the Python versions). The "test-all-kernel-flavors" option is active after each vmtest build, or when the label is applied to a PR, or for a manual run on the Github Actions UI.I tested part 3 locally and verified that when
GITHUB_ACTIONS=true
, mybuild/vmtest
directory never grew beyond ~2.5 GiB for a standard-K
test, and it was emptied of kernels by the end of the run. I didn't want to do the-K -F
locally, but I'd assume the result is about the same. For a local-K
test withGITHUB_ACTIONS=false
, thebuild/vmtest
directory reached 7.5 GiB quickly and never decreased.Test runs in Github Actions: