-
Notifications
You must be signed in to change notification settings - Fork 562
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
tests: add jammy to spread executions #11274
Conversation
…d7d23..dc6be12e76 dc6be12e76 Merge pull request #3 from snapcore/update-os-query 403242c037 Include jammy as part of the supported systems in os.query 8e2dca12b7 Merge branch 'main' of github.com:snapcore/snapd-testing-tools into main 217fadaf0d Run tests on pull requests 2e905b73ca Merge pull request #2 from snapcore/add-new-tools-doc 2cdc7691f2 Upda docs based on graham comments fe151f4427 Adding a new section to the doc git-subtree-dir: tests/lib/external/snapd-testing-tools git-subtree-split: dc6be12e763a55d86e6f7126f3e77c78b479e880
Fix os.query test, initially was done: https://github.com/snapcore/snapd-testing-tools.git main --squash Also fixed cgroup tests
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.
Looks good, but a few comments about changes that look unrelated.
@@ -1,8 +1,11 @@ | |||
name: Jobs | |||
on: | |||
pull_request: |
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 this change intended for this PR?
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.
@stolowski thanks for the review, I had to import snapd-testing-tools to support jammy and when I do that It includes all the changes in that project since the last import.
When we do the import we import the main branch by doing:
git subtree pull --prefix tests/lib/external/snapd-testing-tools/ https://github.com/snapcore/snapd-testing-tools.git main --squash
push: | ||
branches: | ||
- '**' # matches every branch | ||
- main |
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.
Same.
## Adding new tools | ||
|
||
The tools included in this project are intended to be reused by other projects. | ||
|
||
Read the following considerations before adding new tools: | ||
|
||
- Each tool needs to be accompanied by at least 1 spread test in `tests/<tool-name>/` | ||
- At least 1 spread test needs to be included in the tests directory for each tool | ||
- If the tool is a shell script, it needs to first pass a [ShellCheck](https://github.com/koalaman/shellcheck) assessment | ||
- All tools need to be as generic as possible | ||
- Each tool must also provide a command line interface (CLI), including _help_ output | ||
|
||
|
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.
Same.
Codecov Report
@@ Coverage Diff @@
## master #11274 +/- ##
==========================================
- Coverage 78.37% 78.36% -0.02%
==========================================
Files 927 927
Lines 105610 105700 +90
==========================================
+ Hits 82774 82831 +57
- Misses 17693 17720 +27
- Partials 5143 5149 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
LGTM
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.
Thank you
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
@@ -276,6 +276,7 @@ jobs: | |||
- ubuntu-20.04-64 | |||
- ubuntu-21.04-64 |
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.
Can we drop 21.04 in a followup then?
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.
Yes please
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.
yes
@@ -1,6 +1,6 @@ | |||
summary: measuring basic properties of device cgroup | |||
|
|||
# Disable the test on all systems that boot with cgroup v2 | |||
systems: [ -fedora-33-*, -fedora-34-*, -fedora-35-*, -debian-11-*, -debian-sid-*, -arch-*, -opensuse-tumbleweed-*, -ubuntu-21.10-*] | |||
systems: [ -fedora-33-*, -fedora-34-*, -fedora-35-*, -debian-11-*, -debian-sid-*, -arch-*, -opensuse-tumbleweed-*, -ubuntu-21.10-*, -ubuntu-22.04-*] |
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.
Reminds me we should probably clean up fedora 33 at some point.
The tests fail right now with:
|
@mvo5, thanks for the review, the tests are failing because the image ubuntu-os-cloud-devel/ubuntu-2204-lts is in the second page and as spread doesn't have pagination for google backend, it can't get the image. I tested the image by using a local fork of spread but we need the pagination to get that image from the ubuntu-os-cloud-devel project. |
This change includes new ubuntu Jammy as part of the regular spread executions.