Skip to content

Add status propagation tests for Service#1798

Merged
knative-prow-robot merged 2 commits into
knative:masterfrom
jonjohnsonjr:jon/1542
Aug 7, 2018
Merged

Add status propagation tests for Service#1798
knative-prow-robot merged 2 commits into
knative:masterfrom
jonjohnsonjr:jon/1542

Conversation

@jonjohnsonjr
Copy link
Copy Markdown
Contributor

@jonjohnsonjr jonjohnsonjr commented Aug 3, 2018

Fixes #1542

This tests:

  1. Route + Config Ready -> Service Ready
  2. Config Fails + Route Ready -> Service Fails
  3. Config Ready + Route Fails -> Service Fails

These are similar to the propagation tests in
apis/serving/v1alpha1/service_types_test.go but from the reconciler's
perspective.

@jonjohnsonjr
Add status propagation tests for Service
102197e 
Fixes knative#1542

This tests:
1. Route + Config Ready -> Service Ready
2. Config Fails + Route Ready -> Service Fails
3. Config Ready + Route Fails -> Service Fails

These are similar to the propagation tests in
apis/serving/v1alpha1/service_types_test.go but from the reconciler's
perspective.
@google-prow-robot google-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 3, 2018
@google-prow-robot
Copy link
Copy Markdown

google-prow-robot commented Aug 3, 2018

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jonjohnsonjr
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: grantr

If they are not already assigned, you can assign the PR to them by writing /assign @grantr in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@jonjohnsonjr
Copy link
Copy Markdown
Contributor Author

jonjohnsonjr commented Aug 3, 2018

/assign @mattmoor

The way I'm setting the status feels gross, but I think it makes sense to wait for #1762 to make that nicer.

@adrcunha
Copy link
Copy Markdown
Contributor

adrcunha commented Aug 3, 2018

/test pull-knative-serving-build-tests
/test pull-knative-serving-unit-tests
/test pull-knative-serving-integration-tests
/test pull-knative-serving-go-coverage

@adrcunha
Copy link
Copy Markdown
Contributor

adrcunha commented Aug 3, 2018

/hold

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 3, 2018
@adrcunha
Copy link
Copy Markdown
Contributor

adrcunha commented Aug 3, 2018

/hold cancel

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 3, 2018
mattmoor
mattmoor reviewed Aug 7, 2018
View reviewed changes
Comment thread pkg/reconciler/v1alpha1/service/service_test.go Outdated
return cfg
}

// TODO(1762): Replace with builders.
Copy link
Copy Markdown
Member

@mattmoor mattmoor Aug 7, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: TODO(#1762)

mattmoor
mattmoor reviewed Aug 7, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@mattmoor mattmoor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

Will LGTM post nit

@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Aug 7, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jonjohnsonjr, mattmoor

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 7, 2018
mattmoor
mattmoor approved these changes Aug 7, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@mattmoor mattmoor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 7, 2018
@knative-prow-robot knative-prow-robot merged commit da4cbe9 into knative:master Aug 7, 2018
@jonjohnsonjr jonjohnsonjr deleted the jon/1542 branch September 10, 2018 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@grantr grantr Awaiting requested review from grantr

@tcnghia tcnghia Awaiting requested review from tcnghia

1 more reviewer

@mattmoor mattmoor mattmoor approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mattmoor mattmoor

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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jonjohnsonjr @google-prow-robot @adrcunha @knative-prow-robot @mattmoor