Skip to content
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

test: some updates on test #552

Merged
merged 1 commit into from
May 28, 2024
Merged

test: some updates on test #552

merged 1 commit into from
May 28, 2024

Conversation

henrywang
Copy link
Contributor

@henrywang henrywang commented May 20, 2024

  1. add bootc swtich test. Switch to local folder /mnt in this case
  2. add bootc install to-disk test
  3. add case installing SELinux-enabled targets from SELinux-disabled hosts for Ensure no-selinux case is at least e2e tested periodically #419
  4. cover more distros, like rhel 9.5, fedora 40 and fedora 41(rawhide)
  5. upgrade testing-farm-as-github-action to v2

@cgwalters
Copy link
Collaborator

(Huh, weird that you hit cargo fmt on docgen.rs, but not elsewhere. Anyways the fix is in #547 )

Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot going on here 😄
I skimmed this, looks sane to me.

I do want to think about down the line how to align this with #543 too; I have some not-conflicting, but partially overlapping changes queued in #548 too

@henrywang henrywang force-pushed the update branch 2 times, most recently from 6b96dab to 79852fb Compare May 21, 2024 00:52
@henrywang henrywang marked this pull request as ready for review May 22, 2024 23:42
@henrywang henrywang force-pushed the update branch 2 times, most recently from b07f634 to fe9de8b Compare May 23, 2024 14:56
@cgwalters
Copy link
Collaborator

We're running a lot of tests per PR today, I'm not sure always need to all these tests per PR. Maybe we can move some of them to post-merge testing? For example testing both c9s and rhel9.4 makes sense, but I'm not sure about every PR. Similarly both x86_64 and aarch64.

I'd like to be able to opt-in (via a comment or label on a PR) to full testing, but my instinct here is to ensure we have baseline sanity testing to start...and what do you think about ensuring that if say we choose to say only test aarch64 on a PR for c9s (skipping x86_64) we are diligently watching post-merge testing and can quickly identify any breakage?

@henrywang
Copy link
Contributor Author

I totally agree with you. Our daily test covered much more. We can catch failure with daily test. What about the plan like this:

  • rhel9.4: x86_64
  • rhel9.5: x86_64, aarch64
  • cs9: aarch64
  • fedora 40: x86_64
  • fedora 41: aarch64

@cgwalters
Copy link
Collaborator

Let's drop fedora 41 there: I don't want to gate PRs on fedora rawhide stability. Or to future proof it the matrix would just have fedora $stable. We can also pretty confidently drop 9.4 I think for PR testing, the chance that we pass on 9.5 but break on 9.4 is currently very close to zero.

1. add bootc swtich test
2. add bootc install to-disk test
3. cover more distros, like rhel 9.5, fedora 40 and 41(rawhide)

Signed-off-by: Xiaofeng Wang <henrywangxf@me.com>
@lmilbaum
Copy link
Contributor

This is a huge PR. Is it possible to break it into smaller PRs such that it is less risky and easier to review?

@henrywang
Copy link
Contributor Author

henrywang commented May 26, 2024

This is a huge PR. Is it possible to break it into smaller PRs such that it is less risky and easier to review?

It's not easy to split this PR now. :-) I run this PR with testing farm. That can avoid test issue imported by this PR.

This PR is ready to merge now.

@cgwalters cgwalters merged commit 4e74262 into containers:main May 28, 2024
10 of 15 checks passed
@henrywang henrywang deleted the update branch May 29, 2024 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants