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

NO-ISSUE: add e2e-techpreview-shared, remove layering test target #4183

Merged
merged 1 commit into from Feb 14, 2024

Conversation

djoshy
Copy link
Contributor

@djoshy djoshy commented Feb 13, 2024

- What I did
This PR adds a new test package, e2e-techpreview-shared. This test package should be used for e2e tests that need to be tested under the default featureset and the techpreview featureset. This would mean that your test
has two paths, one when the feature gate is enabled and one when the feature gate is disabled. Tests added here will run under the test-e2e and test-e2e-techpreview Makefile targets, which is used by ci/prow/e2e-gcp-op and ci/prow/e2e-gcp-op-techpreview test suites respectively.

This PR also removes the old makefile target for e2e-gcp-op-layering which has now been repurposed into e2e-gcp-op-techpreview.

This should merge after openshift/release#48618 merges.

- How to verify it

  • tests, including the new e2e-gcp-op-techpreview should succeed
  • both e2e-gcp-op and e2e-gcp-op-techpreview should run TestNoop.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Feb 13, 2024
@openshift-ci-robot
Copy link
Contributor

@djoshy: This pull request explicitly references no jira issue.

In response to this:

- What I did
This removes the old makefile target for e2e-gcp-op-layering which has now been repurposed into e2e-gcp-op-techpreview. This should merge after openshift/release#48618 merges.

- How to verify it
e2e-gcp-op-techpreview should succeed

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 openshift-eng/jira-lifecycle-plugin repository.

@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 Feb 13, 2024
Copy link
Contributor

openshift-ci bot commented Feb 13, 2024

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

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 13, 2024
@cheesesashimi
Copy link
Member

Let's use this PR to lay the foundation for sharing tests between the test/e2e and test/e2e-techpreview packages. I don't think that we need to use something as complicated as the e2e-shared pattern that I originally envisioned. Instead, we can rely on the fact that go test will let you specify multiple test targets on the CLI. We'll add another test package under test/ and modify the Makefile targets to run that as well. For this example, we'll call this new test package e2e-featuregated, which isn't the best name so feel free to change it if you think of something better.

Here's how we can add it:

  1. $ mkdir -p test/e2e-featuregated
  2. Add a file at test/e2e-featuregated/noop_test.go containing something like the following:
package e2e_featuregated_test

import "testing"

func TestNoop(t *testing.T) {
	t.Logf("In TestNoop")
}
  1. Adjust the test-e2e and test-e2e-techpreview Makefile targets thusly:
test-e2e: install-go-junit-report
     set -o pipefail; go test -tags=$(GOTAGS) -failfast -timeout 150m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e/ ./test/e2e-featuregated/ | ./hack/test-with-junit.sh $(@)

test-e2e-techpreview: install-go-junit-report
     set -o pipefail; go test -tags=$(GOTAGS) -failfast -timeout 150m -v$${WHAT:+ -run="$$WHAT"} ./test/e2e-techpreview/ ./test/e2e-featuregated/ | ./hack/test-with-junit.sh $(@)

When you push this up, look for TestNoop to show up in test output to verify that it is actually running. Now, any tests added to ./test/e2e-featuregated will execute in both the test-e2e and test-techpreview test suites 😄

We may also want to put a README.md in the test/e2e-featuregated package as well to explain that.

@djoshy djoshy changed the title NO-ISSUE: remove gcp-op-layering Makefile test target NO-ISSUE: add e2e-techpreview-shared, remove layering test target Feb 13, 2024
@djoshy
Copy link
Contributor Author

djoshy commented Feb 13, 2024

/test e2e-gcp-op

@djoshy djoshy marked this pull request as ready for review February 13, 2024 18:31
@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 Feb 13, 2024
@djoshy
Copy link
Contributor Author

djoshy commented Feb 13, 2024

Taking this out of draft as openshift/release#48618 merged. I've gone with Zack's idea and added a short Readme as well (:

@cheesesashimi
Copy link
Member

/lgtm
/approve

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 13, 2024
Copy link
Contributor

openshift-ci bot commented Feb 13, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cheesesashimi, djoshy

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [cheesesashimi,djoshy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Contributor

openshift-ci bot commented Feb 14, 2024

@djoshy: The following test 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/okd-scos-e2e-aws-ovn 24fd263 link false /test okd-scos-e2e-aws-ovn

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-bot openshift-merge-bot bot merged commit 53a9e00 into openshift:master Feb 14, 2024
15 of 16 checks passed
@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-machine-config-operator-container-v4.16.0-202402140927.p0.g53a9e00.assembly.stream.el8 for distgit ose-machine-config-operator.
All builds following this will include this PR.

@djoshy djoshy deleted the remove-test-target branch February 16, 2024 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants