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.

Sign up for GitHub

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

Jump to bottom

Clean up managed reconciler tests #67

Merged
merged 2 commits into from
Oct 31, 2019
Merged

Clean up managed reconciler tests #67

merged 2 commits into from
Oct 31, 2019

Conversation

negz
Copy link
Member

@negz negz commented Oct 31, 2019 •
edited
Loading

Description of your changes

No functional updates to the code here, just:

  • Tidying up the tests a bit to ensure they all mock out their various moving parts via options.
  • Adding a "reason" that explains each cases's purpose (and is printed when tests fail). Some of our test case names were getting super long, so I wanted to encode their purpose in a distinct string.

Checklist

I have:

  • Run make reviewable to ensure this PR is ready for review.
  • Ensured this PR contains a neat, self documenting set of commits.
  • Updated any relevant documentation, examples, or release notes.
  • Updated the RBAC permissions in clusterrole.yaml to include any new types.

@negz
Trivial comment updates
6db5ced 
Just wrapping a comment and updating the linter override to reflect that we've
moved beyond "slightly" over our cyclomatic complexity goal. :(

Signed-off-by: Nic Cope <negz@rk0n.org>
@negz
Clean up managed resource reconciler tests
4fbddd0 
No functional updates to the code here, just tidying up the tests a bit to
ensure they all mock out their various moving parts via options.

Signed-off-by: Nic Cope <negz@rk0n.org>
@upbound-bot
Copy link
Collaborator

92% (+1.1%) vs master 91%

@negz negz mentioned this pull request Oct 31, 2019
Closed
4 tasks
hasheddan
hasheddan approved these changes Oct 31, 2019
View reviewed changes
Copy link
Member

@hasheddan hasheddan left a comment

Choose a reason for hiding this comment

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

LGTM! Glad to have these reason fields on tests now!

@negz
Copy link
Member Author

negz commented Oct 31, 2019

Thanks for the quick review @hasheddan!

@negz negz merged commit 702f6cb into crossplane:master Oct 31, 2019
@negz negz deleted the rectests branch October 31, 2019 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hasheddan hasheddan hasheddan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@negz @upbound-bot @hasheddan