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

Add e2e variant that modifies common environment variables #688

Merged
merged 1 commit into from
Jun 8, 2022

Conversation

komish
Copy link
Contributor

@komish komish commented Jun 7, 2022

Part of #646

The pipeline is using environment variables to configure preflight. No flags.

To a certain extent, we have a degree of testing ensuring that the environment variables (that make it to viper) end up in our runtime configs. What they do, however, could probably use more coverage.

Ideal case would be to run the pipeline's tekton task, but that requires an entire cluster + tekton installed. For the moment, I've added a variant of our existing e2e script that modifies things using the environment. Those that can be tested somewhat reliably have been added to this script.

I'm considering either running this in lieu of our current e2e (which requires changing some of our e2e tests in openshift/release), or running it as a second task. As of right now, it does not run in our e2e pipeline.

I was able to get this to pass using the default operator listed in the script.

Signed-off-by: Jose R. Gonzalez jose@flutes.dev

@openshift-ci openshift-ci bot requested review from acornett21 and bcrochet June 7, 2022 20:58
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 7, 2022
@coveralls
Copy link

coveralls commented Jun 7, 2022

Coverage Status

coverage: 59.354% (-0.8%) from 60.199%
when pulling 19fec13 on komish:custom-env-e2e
into 1d6894d on redhat-openshift-ecosystem:main.

@bcrochet
Copy link
Contributor

bcrochet commented Jun 8, 2022

/retest

@acornett21
Copy link
Contributor

I'm considering either running this in lieu of our current e2e (which requires changing some of our e2e tests in openshift/release), or running it as a second task. As of right now, it does not run in our e2e pipeline.

Is there any downside to running both? Would they run in parallel? Or sequentially? To me it seems having both provides more value, unless it takes a ton more time and/or there is overlap between the two.

@komish
Copy link
Contributor Author

komish commented Jun 8, 2022

I think they would run serially. I'm not sure there's a downside to running both, personally, other than time, but I'm not overly offended by the e2e duration at the moment. I'm keeping the options open for now.

@komish
Copy link
Contributor Author

komish commented Jun 8, 2022

At the very least, having it in repo means I could just as easily run it locally to ensure we're testing some degree of the pipeline's invocation.

Signed-off-by: Jose R. Gonzalez <jose@flutes.dev>
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 8, 2022
@openshift-ci
Copy link

openshift-ci bot commented Jun 8, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: acornett21, jomkz, komish

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 [acornett21,jomkz,komish]

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

@komish komish merged commit 81161f9 into redhat-openshift-ecosystem:main Jun 8, 2022
@komish komish deleted the custom-env-e2e branch June 8, 2022 17:55
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. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants