-
Notifications
You must be signed in to change notification settings - Fork 156
test: Add Packit and gating test #1642
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
cde5644 to
61205f8
Compare
cgwalters
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.
Thanks for picking this up!
| source /etc/os-release | ||
| case "$ID" in | ||
| "centos") | ||
| BASE="quay.io/centos-bootc/centos-bootc:stream${VERSION_ID}" |
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 keep adding new copies of this...I'd like to consolidate that. here's an random idea. How about we add a field BOOTC_IMAGE into os-release?
But in the intermediate term, I think we could probably have a lib.sh or something that's shared with build.sh too that we use here?
A different idea: Move all the tmt stuff into tests/ and have this "image mapping helper" live in 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.
Thinking... Create a file with content like BASE_CENOTS_10=quay.io/centos-bootc/centos-bootc:stream10 and source it?
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.
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 like the JSON a lot better than doing it in shell for sure
4fe2142 to
b246f67
Compare
cfea8f4 to
138dc6b
Compare
tmt/plans/packit.fmf
Outdated
| discover+: | ||
| how: fmf | ||
| test: | ||
| - /tmt/tests/test-00-system-reinstall-bootc |
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't we do this step not as a test but as a "provision phase" step?
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 trying to run this in tmt prepare https://github.com/bootc-dev/bootc/pull/1642/files#diff-bbbfb45ca8f2b20a0985a7efd1f2d17bda0102fb40dcee7f43b6db2ef32a4433R16, and it'll not be a tmt test any more. And it's possible to share same tmt plan (only package mode VM, Packit and gating in this case, run this provision script, github and local run is running on image mode VM) to avoid duplicate plan issue.
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! That works!
tmt/plans/packit.fmf
Outdated
| how: tmt | ||
| exit-first: true | ||
|
|
||
| /test-01-readonly: |
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 had an existing problem where "add a new test" requires a lot of ceremony (write the code as a file, write the fmf file, then add it to the plan) - it'd be really great to fix this (can't we use the tmt discover stuff).
But this takes that problem and makes it worse as now all the tests are listed twice more in this file.
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 as above comment.
8e6d931 to
62cd486
Compare
ae575bc to
ff40dc9
Compare
|
CS9 failure: issue #1659. Can we skip CS9 packit test until issue fixed? |
Use 'system-reinstall-bootc' to re-install TF runner from package mode to image mode Signed-off-by: Xiaofeng Wang <[email protected]>
|
Thanks! So now we're running all the tests in two different ways, via a pregenerated disk image and via s-r-b. I'm not sure we need to do that on every PR, but we can continue to fine tune this. Offhand for example I'd probably say that we could do the full matrix as an opt-in via label again or pre-release? Anyways not a blocker for now so let's keep moving forward!
Sure, though they aren't gating right now, and hopefully the fix will be pretty easy, and then we can prove the PR to fix it works. |
Use 'system-reinstall-bootc' to re-install TF runner from package mode to image mode