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

Allow to skip go test #320

Merged
merged 1 commit into from May 25, 2023

Conversation

gkurz
Copy link
Member

@gkurz gkurz commented May 25, 2023

make test has been broken since #300 was merged and fixing it is going to take time (see #310 for details). It is a problem for local development since the test target is a prequisite for docker-build in the makefile.

Add a variable to control whether go test should run when the test target is invoked.

$ make test SKIP_TESTS= # Run go test when unset

$ make test SKIP_TESTS=1 # Don't run go test when set

Default is unset. [edit]

Fixes https://issues.redhat.com/browse/KATA-2167

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 25, 2023
@openshift-ci
Copy link

openshift-ci bot commented May 25, 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
Contributor

@pmores pmores left a comment

Choose a reason for hiding this comment

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

I believe this is a good if not necessary change for the time being. Our approach to make test implementation needs serious rethinking. Until then, the current implementation seems fragile with a lot of false failures and very tedious to maintain while not bringing in much benefit (very few if any bugs were pointed out by make test to my knowledge).

@gkurz gkurz removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 25, 2023
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 25, 2023
`make test` has been broken since openshift#300 was merged and fixing it is going to
take time (see openshift#310 for details). It is a problem for local development since
the `test` target is a prequisite for `docker-build` in the makefile.

Add a variable to control whether `go test` should run when the `test`
target is invoked.

 $ make test SKIP_TESTS=   # Run `go test` when unset

 $ make test SKIP_TESTS=1  # Don't run `go test` when set

Default is unset.

Signed-off-by: Greg Kurz <groug@kaod.org>
@gkurz
Copy link
Member Author

gkurz commented May 25, 2023

Forced push with default is unset so that everyone keeps in mind that make test is still broken.

@gkurz gkurz requested review from snir911 and cpmeadors May 25, 2023 13:29
@gkurz gkurz marked this pull request as ready for review May 25, 2023 13:30
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 25, 2023
@openshift-ci openshift-ci bot requested review from bpradipt and pmores May 25, 2023 13:32
Copy link
Contributor

@cpmeadors cpmeadors left a comment

Choose a reason for hiding this comment

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

Premerge tested per PR description. All cases work. Code looks good.

@gkurz gkurz merged commit 560078d into openshift:peer-pods-tech-preview May 25, 2023
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