Move the resource test under conformance.#2733
Move the resource test under conformance.#2733knative-prow-robot merged 2 commits intoknative:masterfrom
Conversation
|
/lgtm |
| test.CleanupOnInterrupt(func() { TearDown(clients, names, logger) }, logger) | ||
| defer TearDown(clients, names, logger) | ||
|
|
||
| test.CleanupOnInterrupt(func() { tearDown(clients, names) }, logger) |
There was a problem hiding this comment.
Shouldn't this be before CreateRunLatestServiceReady?
| test.CleanupOnInterrupt(func() { tearDown(clients, names) }, logger) | ||
| defer tearDown(clients, names) | ||
|
|
||
| logger.Infof("When the Revision can have traffic routed to it, the Route is marked as Ready.") |
There was a problem hiding this comment.
This is probably unnecessary. The CreateRunLatestServiceReady will have already validated that the service went into "Ready" meaning that Config/Route are Ready. It also validates that the domain is populated.
| @@ -69,43 +73,7 @@ func TestCustomResourcesLimits(t *testing.T) { | |||
| } | |||
| domain := route.Status.Domain | |||
There was a problem hiding this comment.
You should just be able to replace _ on CreateRunLatestServiceReady with objects, and do objects.Route.Status.Domain (or grab it from Service.Status.Domain).
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dgerd, jonjohnsonjr, mattmoor 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 |
|
/retest |
This was where I'd originally wanted this, but realized it had somehow ended up under e2e. This also removes the pod checks, which reach around the Knative API surface.
b984a12 to
ea31c12
Compare
|
/retest |
1 similar comment
|
/retest |
|
/lgtm |
This was where I'd originally wanted this, but realized it had somehow ended up under e2e.
This also removes the pod checks, which reach around the Knative API surface.