-
Notifications
You must be signed in to change notification settings - Fork 66
🌱 OPRUN-3956: Add experimental e2e tests #2064
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
Conversation
This adds `manifests/experimental-e2e.yaml`, and uses it to run the experimental-e2e. The experimental-e2e is run via `make test-experimental-e2e` and a CI job is added to run it. A "no-op" experimental-e2e test has been added. This is part of the feature-gated API functionality. Signed-off-by: Todd Short <[email protected]>
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
/approved cancel |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2064 +/- ##
==========================================
+ Coverage 73.82% 73.89% +0.06%
==========================================
Files 81 81
Lines 7365 7365
==========================================
+ Hits 5437 5442 +5
+ Misses 1588 1585 -3
+ Partials 340 338 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@@ -0,0 +1,1984 @@ | |||
apiVersion: v1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quick question — I noticed that before we didn’t commit the install.yaml file.
Are we planning to commit both files now? Just curious what’s changed and why we’re doing that now, if so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What install.yaml
file are you referring to?
This does not change the existing release mechanism; those files are still created via the quickstart
Makefile target.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But we generated it when we run target, why do we need to commit?
Can we not add those to gitignore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you're talking about manifest/experimental-e2e.yaml
?
We are now checking in all the manifests for standard, experimental and e2e (look in the manifest
directory). This is done on purpose (it's mentioned in the RFC, and is a continuation of #2025). It allows people (and reviewers) to easily identify the effects of any changes to the kustomize config, which can usually be hard to figure out.
Having these files committed made it very easy to do all the earlier kustomizae changes (#2063, #2061, #2060, #2059, #2057, #2056, #2055 and #2025), because I could easily see that there were no changes to the resulting manifests.
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, tmshort 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:
Approvers can indicate their approval by writing |
Signed-off-by: Todd Short <[email protected]>
Signed-off-by: Todd Short <[email protected]>
@@ -1,6 +1,6 @@ | |||
codecov: | |||
notify: | |||
after_n_builds: 2 | |||
after_n_builds: 3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change related ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as the experimental-e2e pushes code coverage. There are three builds that do code-coverage:
- e2e
- unit
- experimental-e2e
This is supposed to make codecov wait until it gets three build pushes (the above).
See: https://app.codecov.io/gh/operator-framework/operator-controller/pull/2064/flags
go-version-file: go.mod | ||
|
||
- name: Run e2e tests | ||
run: ARTIFACT_PATH=/tmp/artifacts make test-experimental-e2e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For curiosity, what was the issue that was not finding the path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CollectTestArtifacts()
function was not being called; one of the things it does is create the /tmp/artifacts
directory.
|
||
func TestNoop(t *testing.T) { | ||
t.Log("Running experimental-e2e tests") | ||
defer utils.CollectTestArtifacts(t, artifactName, c, cfg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@camilamacedo86 as a side effect, this creates the /tmp/artifacts
directory.
/lgtm Great work 🎉 Thank you for all explanations |
6465e63
into
operator-framework:main
Fix #2016
This adds
manifests/experimental-e2e.yaml
, and uses it to run the experimental-e2e.The experimental-e2e is run via
make test-experimental-e2e
and a CI job is added to run it.A "no-op" experimental-e2e test has been added.
This is part of the feature-gated API functionality.
Description
Reviewer Checklist