-
Notifications
You must be signed in to change notification settings - Fork 33
functests: Add set up method to add schemes #74
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
functests: Add set up method to add schemes #74
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
9622f29 to
fdc9892
Compare
/retest |
|
/test operator-e2e |
|
e2e: $ oc --context build01 delete project ci-op-nhr8xycy/retest |
|
All green :). /assign @jottofar |
|
|
||
| // setUp performs test setup. | ||
| func setUp() { | ||
| if err := routev1.AddToScheme(scheme.Scheme); err != nil { |
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 there a reason you dropped this?
| if err := apis.AddToScheme(scheme.Scheme); err != nil { |
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.
Yeah, we don't import that yet. We can add it with the commit that adds the import
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 you're just adding back stuff that shouldn't have been removed to begin with. I should have caught it and commented in in the Remove Ginkgp commit so not understanding why this commit doen't just add it all back in.
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.
We used to add the route scheme (twice?). And out local APIs, which you link. I dunno why we don't actually need the local-API schemes injected, but tests are green, so we must not need them.
|
/lgtm |
raw/metadata.json: add historical metadata from quay
Pulled out of #65, since this should green up CI and seems orthogonal to the SDK pivot.