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

Check that rpms installed in tmt come from copr #676

Closed
wants to merge 3 commits into from

Conversation

kwk
Copy link
Collaborator

@kwk kwk commented Aug 27, 2024

Add a script to ensure that the llvm package installed comes from the copr repo. A failure in the script aborts the test plan

See #671

- name: Assert correct llvm package is installed
how: shell
order: 99
script: tests/assert_llvm.sh
Copy link
Collaborator Author

@kwk kwk Aug 27, 2024

Choose a reason for hiding this comment

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

Suggested change
script: tests/assert_llvm.sh
script: rpm -q llvm-libs | grep -P "Version[^:]+:.*~pre.*"

This should do the trick.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that your point here is to keep it simple. I do think that the script might be more beneficial in this case, though. The script gives us some advantages here:

  • We can make several checks at the prepare step end, all grouped under the same script. This may include, checking that the copr repo is reachable.
  • We can write more complex logic. For example testing several packages rather that llvm-libs only.
  • We can properly report what failed and why.

For example, upon wrong package installation with the script, this is what we see in the report:

    prepare step failed

    The exception was caused by 1 earlier exceptions

    Cause number 1:

        Command '/var/ARTIFACTS/work-snapshot-gatingejv1c0ks/tests/snapshot-gating/tree/tmt-prepare-wrapper.sh-Assert-correct-llvm-package-is-installed-default-0' returned 1.

        stdout (2 lines)
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        *** Unexpected llvm package installed: llvm-18.1.8-2.fc41.x86_64
        *** Expected llvm: llvm-20.0.0~pre20240825.g3ef64f7ab5b865-1.fc42.x86_64
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

However, using the plain command, we would see this instead, which is much less intuitive:

    prepare step failed

    The exception was caused by 1 earlier exceptions

    Cause number 1:

        Command '/var/tmp/tmt/run-077/tests/snapshot-gating/tree/tmt-prepare-wrapper.sh-Assert-correct-llvm-package-is-installed-default-0' returned 1.

@@ -0,0 +1,34 @@
#!/bin/bash
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think you can remove this file.

@@ -23,6 +23,11 @@ prepare:
script: |
dnf install -y dnf-plugins-core
dnf config-manager --save --setopt="testing-farm-tag-repository.priority=999" || true

- name: Assert correct llvm package is installed
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
- name: Assert correct llvm package is installed
- name: "Check that snapshot (~pre) version of LLVM is installed"

Jesus Checa Hidalgo added 3 commits August 27, 2024 15:57
Add a prepare step that checks the version of installed llvm-libs
to ensure it's a copr package.
Followup fedora-llvm-team#670. Reimplement without using a specific dnf.
Replace the unexpanded $distname variable in the script
Before trying to modify the repo we check that it exists, hence we no
longer need to mask the result of commands. This now works with both
dnf4 and dnf5

Fixes fedora-llvm-team#671
@kwk
Copy link
Collaborator Author

kwk commented Aug 28, 2024

Closed in favor of #680

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.

2 participants