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

fmf: Uninstall cockpit-packagekit for basic and network plans #19556

Merged
merged 2 commits into from
Nov 2, 2023

Conversation

martinpitt
Copy link
Member

@martinpitt martinpitt commented Nov 2, 2023

These plans are not meant to have cockpit-packagekit installed. That isn't in the test dependency list, but dnf installs "missing" weak dependencies on upgrades by default, which is undesired: The packagekit page gets preloaded and thus influences the test outcome, particularly when it is an old version (as happens during downstream gating).

So remove the package again if it got installed for plans other than "optional" (which is the one that actually tests packagekit).

Fixes https://gitlab.com/redhat/centos-stream/rpms/cockpit/-/merge_requests/68


See the link, this breaks the downstream release. I ran the tests against this branch to confirm that it fixes it.

This is essentially the same approach as in e.g. cockpit-project/cockpit-machines@430b819 or cockpit-project/cockpit-podman@99efdb6 , which work around the same dnf misfeature.

@martinpitt martinpitt added release-blocker Targetted for next release no-test For doc/workflow changes, or experiments which don't need a full CI run, labels Nov 2, 2023
@martinpitt martinpitt marked this pull request as draft November 2, 2023 13:59
@martinpitt
Copy link
Member Author

martinpitt commented Nov 2, 2023

It makes downstream gating tests green, but one regression in testCPUSecurityMitigationsDetect which is easy to fix.

Update: On closer look it actually is a bit tricky: TestSystemInfo uses PackageCase which assumes packagekit. At the moment, testCPUSecurityMitigationsDetect is the only test that runs from that class, but in principle we may want more. It would be wrong to move it to the "optional" plan, as the code+page live in the Overview (aka "basic"). I added a commit which splits the test class.

@martinpitt martinpitt removed the no-test For doc/workflow changes, or experiments which don't need a full CI run, label Nov 2, 2023
These are the only tests that need the `PackageCase` API. This split
makes it possible to run the other TestSystemInfo tests on a machine
which does not have cockpit-packagekit/PackageKit installed.

Rename the pixel references accordingly.
These plans are not meant to have cockpit-packagekit installed. That
isn't in the test dependency list, but dnf installs "missing" weak
dependencies on upgrades by default, which is undesired: The packagekit
page gets preloaded and thus influences the test outcome, particularly
when it is an old version (as happens during downstream gating).

So remove the package again if it got installed for plans other than
"optional" (which is the one that actually tests packagekit).

Fixes https://gitlab.com/redhat/centos-stream/rpms/cockpit/-/merge_requests/68
@martinpitt martinpitt marked this pull request as ready for review November 2, 2023 16:01
@martinpitt
Copy link
Member Author

Interesting, this c9s TF failure is the same that I got in https://gitlab.com/redhat/centos-stream/rpms/cockpit/-/merge_requests/67 . So that's a flake, not a failure.

@martinpitt martinpitt merged commit c8720ed into main Nov 2, 2023
34 checks passed
@martinpitt martinpitt deleted the fmf-no-weak branch November 2, 2023 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker Targetted for next release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants