-
Notifications
You must be signed in to change notification settings - Fork 162
Add example tests for artifact providers #4449
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
base: main
Are you sure you want to change the base?
Conversation
dc828a0 to
ca4c960
Compare
ca4c960 to
a5284c8
Compare
LecrisUT
left a comment
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 the goal for this PR to split the /tests/prepare/artifact into individual provider tests? If so, why not add it to it instead? For example you can add a for loop in the test.sh to loop over each provider if that is the intent.
One concern with the current approach is that the koji list-tagged logic is quite non-trivial and would prefer to be defined in a single place.
| rpm -q make | ||
|
|
||
| # Install docker-ce-cli from the configured repository | ||
| dnf install -y docker-ce-cli |
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.
We should not be installing from the script since this is uncontrolled by us.
d3d3e13 to
92bad06
Compare
92bad06 to
6e7c129
Compare
tcornell-bus
left a comment
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.
Minor questions, but LGTM overall!
tcornell-bus
left a comment
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, but in the tests on fedora-rawhide there is an issue with finding make on koji I guess?
Add simple, example-based integration tests for artifact providers: - koji.build: Fetches packages from Koji using build IDs - repository-url: Configures external package repositories - multi: Demonstrates using multiple providers together These tests serve as usage examples and verify basic functionality. Each test: - Shows clear usage of the --provide argument - Runs a complete tmt workflow - Verifies packages are installed correctly Once the design is approved, additional providers (koji.task, koji.nvr, file, copr.build, brew.*) can be added following the same pattern. Related to #4420
b0c23dc to
d3a60c8
Compare
| prepare: | ||
| - how: artifact | ||
| provide: | ||
| - koji.build:2761810 |
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.
Hardcoded koji build ID 2761810 may not be compatible with the dynamically determined Fedora 43 image used in the test. The test script in test.sh dynamically fetches the build ID for Fedora 43 (line 23), but the plan file test (lines 40-46) uses this hardcoded value. If build 2761810 is not valid for Fedora 43, the plan file test will fail.
Fix: Either use a build ID known to be valid for Fedora 43, or template this value dynamically during test execution:
provide:
- koji.build:${MAKE_BUILD_ID}
- repository-file:https://download.docker.com/linux/fedora/docker-ce.repo| - koji.build:2761810 | |
| - koji.build:${MAKE_BUILD_ID} |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
Add simple, example-based integration tests for artifact providers:
Once the design is approved, additional providers (koji.task, koji.nvr, file, copr.build, brew.*) can be added following the same pattern.
Related to #4420