-
Notifications
You must be signed in to change notification settings - Fork 236
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
[e2e] Add test: OpenShfit story behind proxy #3519
Conversation
4bcf99a
to
db2ba55
Compare
/hold |
db2ba55
to
ea4c97c
Compare
d713ec4
to
b8983d9
Compare
/unhold |
b8983d9
to
da98b7f
Compare
da98b7f
to
839848e
Compare
test/e2e/testsuite/testsuite.go
Outdated
@@ -238,7 +238,6 @@ func InitializeScenario(s *godog.ScenarioContext) { | |||
fmt.Println(err) | |||
os.Exit(1) | |||
} | |||
|
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.
@jsliacan I don't think just removing this line is doing what you described in the commit log.
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.
Oh great, I must've mingled the commits when rearranging after last changes. Thanks for noticing.
839848e
to
26037ae
Compare
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is almost a copy&paste from story_openshift.feature. It would be useful to have an indication of this in the commit log/in the file itself (as a reminder both have to be kept in sync). And some explanation why this is duplicated would not hurt (I guess reuse is hard with what we use for testing?) |
/retest |
26037ae
to
4b7af7e
Compare
Indeed @cfergeau I think we should avoid the duplication, already discussed with @jsliacan, so we probably switch a bit the approach:
|
Also, I asked on cucumber/godog about Examples (Outline) for the Additionally, if you have suggestions about how to re-word my issue over there, I'm all ears. Thanks! |
how does this relate to #3684? |
/need-rebase |
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. |
@jsliacan: The following tests failed, say
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. |
I'm closing this for now as the changes in here aren't substantial to keep and we anyways agreed on a different approach than duplicating a feature file. |
Depends on: PR #3417
We start proxy in Before hook of each scenario and remove it in the After hook of each scenario. This way all openshift stories should directly work behind proxy.
Testing
Run e2e tests (this is part of them). Or run only this feature with
@linux && @story_openshift && @proxy
tag.