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/tests: Make basic kola tests denylist-able #3531

Closed
wants to merge 2 commits into from

Conversation

Adam0Brien
Copy link
Member

Fix: #3418

@openshift-ci
Copy link

openshift-ci bot commented Jul 11, 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

Copy link
Member

@ravanelli ravanelli left a comment

Choose a reason for hiding this comment

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

@Adam0Brien Great job! We will also need to remove the calls from cmd-kola, we don't need it anymore.

@ravanelli
Copy link
Member

I tested the changes and the denylist:

[root@localhost builder]# cosa kola run basic.bios

warning: /sys/fs/selinux appears to be mounted but should not be; enabling workaround
kola run basic.bios --output-dir tmp/kola
⚠️  Skipping kola test pattern "basic.bios":
  👉 https://github.com/coreos/coreos-assembler/pull/1478
⚠️  Skipping kola test pattern "podman.workflow":
  👉 https://github.com/coreos/coreos-assembler/pull/1478
FAIL, output in tmp/kola
Error: harness: no tests to run
[root@localhost builder]# cosa kola run basic.nvme.bios
warning: /sys/fs/selinux appears to be mounted but should not be; enabling workaround
kola run basic.nvme.bios --output-dir tmp/kola
=== RUN   basic.nvme.bios

mantle/kola/tests/coretest/core.go Outdated Show resolved Hide resolved
@@ -112,6 +112,8 @@ type Test struct {
// Conflicts is non-empty iff nonexclusive is true
// Contains the tests that conflict with this particular test
Conflicts []string

Nvme bool
Copy link
Member

Choose a reason for hiding this comment

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

To add the nvme you still need to touch the mantle/kola/harness.go to make use of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Adam0Brien Adam0Brien force-pushed the basic-tests branch 3 times, most recently from 3d55de4 to 78f1e7c Compare July 12, 2023 13:53
@Adam0Brien Adam0Brien marked this pull request as ready for review July 12, 2023 13:53
@Adam0Brien Adam0Brien force-pushed the basic-tests branch 2 times, most recently from 9a7d127 to 84b26a3 Compare July 18, 2023 14:56
@Adam0Brien Adam0Brien force-pushed the basic-tests branch 2 times, most recently from 6384a32 to a9b6c8e Compare July 21, 2023 14:36
@Adam0Brien
Copy link
Member Author

/retest-required

@Adam0Brien Adam0Brien force-pushed the basic-tests branch 3 times, most recently from 5df91d0 to 19a0733 Compare July 26, 2023 09:54
@openshift-ci
Copy link

openshift-ci bot commented Aug 8, 2023

@Adam0Brien: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/rhcos af51c8b link true /test rhcos
ci/prow/images af51c8b link true /test images

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@prestist
Copy link
Contributor

prestist commented Nov 7, 2023

Closing this PR, I have cherry-picked these changes into a new PR

@Adam0Brien Thank you for all of your work on this you got this issue very far and I was happy to learn from what you did.

@prestist prestist closed this Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert basic tests (cosa kola --basic-qemu-scenarios) to a denylist-able one
4 participants