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

mantle/kola/harness: allow success if all tests are denylisted #3489

Merged
merged 1 commit into from
Jun 7, 2023

Conversation

Adam0Brien
Copy link
Member

@Adam0Brien Adam0Brien commented May 29, 2023

Fix : #3464

This PR introduces a new check in coreos-assembler that notifies users when running only denylisted tests

eg output:

adamobrien@fedora:~/cosa/fcos » cosa kola run podman.workflow                                                                                                                                                                             1 ↵
+cosa:16> podman run --rm -ti '--security-opt=label=disable' --privileged '--uidmap=1000:0:1' '--uidmap=0:1:1000' '--uidmap=1001:1001:64536' '-v=/home/adamobrien/cosa/fcos:/srv/' '--device=/dev/kvm' '--device=/dev/fuse' '--tmpfs=/tmp' '-v=/var/tmp:/var/tmp' '--name=cosa' '-v=/home/adamobrien/cosa/fedora-coreos-config/:/srv/src/config/:ro' '-v=/home/adamobrien/cosa/coreos-assembler/src/:/usr/lib/coreos-assembler/:ro' localhost/coreos-assembler kola run podman.workflow
kola -p qemu-unpriv run podman.workflow --output-dir tmp/kola
⚠️  Skipping kola test pattern "fcos.internet":
  👉 https://github.com/coreos/coreos-assembler/pull/1478
⚠️  Skipping kola test pattern "podman.workflow":
  👉 https://github.com/coreos/coreos-assembler/pull/1478
There are no tests to run because all tests are denylisted. Output in tmp/kola
+cosa:25> rc=0 
+cosa:25> set +x

And an additional feature that ensures your running a test with a valid name


adamobrien@fedora:~/cosa/fcos » cosa kola run test.that.does.not.exist
+cosa:16> podman run --rm -ti '--security-opt=label=disable' --privileged '--uidmap=1000:0:1' '--uidmap=0:1:1000' '--uidmap=1001:1001:64536' '-v=/home/adamobrien/cosa/fcos:/srv/' '--device=/dev/kvm' '--device=/dev/fuse' '--tmpfs=/tmp' '-v=/var/tmp:/var/tmp' '--name=cosa' '-v=/home/adamobrien/cosa/fedora-coreos-config/:/srv/src/config/:ro' '-v=/home/adamobrien/cosa/coreos-assembler/src/:/usr/lib/coreos-assembler/:ro' localhost/coreos-assembler kola run test.that.does.not.exist
kola -p qemu-unpriv run test.that.does.not.exist --output-dir tmp/kola
⚠️  Skipping kola test pattern "fcos.internet":
  👉 https://github.com/coreos/coreos-assembler/pull/1478
⚠️  Skipping kola test pattern "podman.workflow":
  👉 https://github.com/coreos/coreos-assembler/pull/1478
2023-06-06T13:00:38Z kola: The user provided pattern didn't match any tests: test.that.does.not.exist
error: failed to execute cmd-kola: exit status 1
+cosa:25> rc=1 

@openshift-ci
Copy link

openshift-ci bot commented May 29, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@Adam0Brien Adam0Brien force-pushed the kola-denylist-check branch 3 times, most recently from 65d2baa to 494a6cb Compare May 29, 2023 12:32
@Adam0Brien Adam0Brien self-assigned this May 29, 2023
mantle/kola/harness.go Outdated Show resolved Hide resolved
mantle/kola/harness.go Outdated Show resolved Hide resolved
mantle/kola/harness.go Outdated Show resolved Hide resolved
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

This looks good, but let's squash the commits down into a single commit if possible.

@Adam0Brien Adam0Brien marked this pull request as ready for review May 31, 2023 14:47
@Adam0Brien Adam0Brien force-pushed the kola-denylist-check branch 3 times, most recently from 2798c96 to 4a33163 Compare May 31, 2023 16:07
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

@dustymabe dustymabe enabled auto-merge (rebase) May 31, 2023 16:13
@dustymabe
Copy link
Member

Once this merges we should be able to merge coreos/fedora-coreos-config#2446

@dustymabe dustymabe disabled auto-merge May 31, 2023 18:59
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

A few things I found later. Sorry.

Also, since we have to push up a new commit anyway maybe we can add some more context to the commit title/body:

mantle/kola/harness: allow success if all tests are denylisted

This will allow us to denylist certain categories of tests and not
have the pipeline fail. See [1] for a real world example of why this
is useful.

[1] https://github.com/coreos/fedora-coreos-config/commit/e983073e791073ae3e4fb5f4fc28f3d0e1e41ebb

mantle/kola/harness.go Outdated Show resolved Hide resolved
mantle/kola/harness.go Outdated Show resolved Hide resolved
@Adam0Brien Adam0Brien force-pushed the kola-denylist-check branch 2 times, most recently from a87f889 to 7869990 Compare June 1, 2023 15:15
dustymabe
dustymabe previously approved these changes Jun 2, 2023
@dustymabe dustymabe enabled auto-merge (rebase) June 2, 2023 03:38
@dustymabe dustymabe changed the title mantle/kola/harness: Add Denylist Check mantle/kola/harness: allow success if all tests are denylisted Jun 2, 2023
@dustymabe
Copy link
Member

dustymabe commented Jun 2, 2023 via email

@Adam0Brien
Copy link
Member Author

/retest

@Adam0Brien Adam0Brien force-pushed the kola-denylist-check branch 6 times, most recently from 9f9f657 to dba6831 Compare June 6, 2023 12:31
This will allow us to denylist certain categories of tests and not
have the pipeline fail. See [1] for a real world example of why this
is useful.

[1] coreos/fedora-coreos-config@e983073
Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

@dustymabe dustymabe enabled auto-merge (rebase) June 6, 2023 13:28
@dustymabe
Copy link
Member

/retest

1 similar comment
@Adam0Brien
Copy link
Member Author

/retest

@dustymabe dustymabe merged commit 8166991 into coreos:main Jun 7, 2023
2 checks passed
@Adam0Brien Adam0Brien deleted the kola-denylist-check branch June 8, 2023 13:51
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.

kola returns an error when no tests are run
2 participants