-
Notifications
You must be signed in to change notification settings - Fork 14
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
Strict interfaces test #748
Conversation
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.
Generally great work but a comment on maintainability.
We cannot hardcode the channels.
.github/workflows/nightly-test.yaml
Outdated
os: ["ubuntu:20.04"] | ||
arch: ["amd64", "arm64"] | ||
release: ["1.31/edge", "1.30/edge"] | ||
fail-fast: false # TODO: remove once arm64 works |
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 should work now, right @addyess ?
.github/workflows/nightly-test.yaml
Outdated
matrix: | ||
os: ["ubuntu:20.04"] | ||
arch: ["amd64", "arm64"] | ||
release: ["1.31/edge", "1.30/edge"] |
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 will not be maintainable.
Please do something like this:
https://github.com/canonical/k8s-snap/blob/main/.github/workflows/nightly-test.yaml#L47
so that the channels are dynamically obtained:
https://github.com/canonical/k8s-snap/blob/main/tests/integration/tests/test_version_upgrades.py#L23
Bonus points if you refactor this logic into a reusable function.
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.
Baking this into Python, then we can integrate this nicely in the integration test like your version upgrades.
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, minor nit
Strict interfaces test
This test verifies that the snap store has auto-connected all necessary interfaces for our strict snap in a nightly test on our strict/edge snaps.