-
Couldn't load subscription status.
- Fork 4
feat: Adds an integration test workflow #27
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
Conversation
cf9305d to
f8e2cae
Compare
f78c4cc to
03ea3ed
Compare
03ea3ed to
cfffb2c
Compare
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.
Just a few nitpicks and minor suggestions. Otherwise seems good.
Entirely optional, but it may also be nice to set the default shell per job where possible, it makes expansion of jobs easier since you don't need to think about adding the shell: line.
Allow image caching to succeed despite the test results Thus we can fail on tests due broken code while a good image previously downloaded is retained for faster iterations.
Thus we can run it inside a WSL instance at devel time, not only in CI. Plus a small fix for questing and later: chrony instead of systemd-timesyncd To make the test case more generic I test first if the unit is enabled and then if active. Since the test runs against a fresh installed instance, if the unit is not active is because of the image state, not users disabling or masking units etc.
cfffb2c to
c256ba1
Compare
- She-bang - exit code monotonic increase - disregards chrony.service for now.
Sprinkling whitespaces after long steps or jobs.
c256ba1 to
cd6515c
Compare
|
I rebased over the current tip of the main branch and moved this PR target back to |
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.
Pre-approving, just one nitpick that probably doesn't matter since the step isn't needed for ephemeral runners anyway.
| exec: | | ||
| source test/integration-test.sh | ||
|
|
||
| - name: "Clean up" # Probably not necessary since we're leveraging ephemeral GH's runners. |
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.
If actually intended for cleanup, the cleanup step should have if: always() so that it runs even if previous steps fail. But like the comment mentions, the step isn't necessary anyway since we're using ephemeral runners. It is probably fine as is if intended as an independent test step to make sure that we can unregister.
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.
I'm with you despite the ephemeral nature of those runners. ;)
Looks like we can go quite far with GitHub hosted runners these days (since they made large runners generally available).
I devised an integration test workflow for
wsl-setupthat:wsl -d DISTRO) andTo support development time, part of the assertions are placed in a separate shell script that we can run inside a VM, LXD container or WSL instance at development time (though some can only pass on WSL), assertions which are really coupled to the contents of the package, thus we should pay attention to them when changing the package. Unfortunately I'm not bold enough to put those assertions in an override_build step of debian/rules because I that could result in unnecessary breakages.
My image caching system saves more bandwidth than time. I was pleased to see that CI was taking less than 30s to download a WSL image (building the deb takes much longer, there's probably some caching opportunity in the callee side?). The cache key is the image checksum read from the SHA256SUMS file hosted aside from the image. I had to learn a few tricks to make the cache working cross OSes.
The WSL instance is created from the cached image with
wsl --install --from-file ... --no-launch ...so we can further customise it with otherwsl -d DISTRO -- <COMMANDS>without those counting as if the OOBE script was run, that is, WSL keeps track of that instance as still non-initialized.This whole scheme allows us to:
UDENG-7866