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.

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

Refactor ent controller tests #342

Merged
merged 3 commits into from
Sep 30, 2020

Conversation

thisisnotashwin
Copy link
Contributor

Changes proposed in this PR:

  • Refactor enterprise controller tests to reduce duplication. This should also make it straightforward to extend them when new kinds of resources are added that buck the trend of being namespaced or global like the existing config entry CRDs
  • Rename controllers package to controller
  • Rename Validators to Webhooks

How I've tested this PR:

  • Ensure all the existing tests continue to pass

How I expect reviewers to test this PR:

  • Do you like the refactor to the tests? How could this be made DRY-er or more readable?

ishustava and others added 2 commits September 30, 2020 09:02
- This ensures we dont have to necessarily write failing tests every
  time we add a field
@thisisnotashwin thisisnotashwin force-pushed the refactor-ent-controller-tests branch 4 times, most recently from b90cf28 to bf0c6c8 Compare September 30, 2020 13:27
@thisisnotashwin thisisnotashwin requested review from lkysow, a team and kschoche and removed request for a team September 30, 2020 13:28
@thisisnotashwin thisisnotashwin marked this pull request as ready for review September 30, 2020 13:28
Comment on lines 29 to 32
// NOTE: We're not testing each controller type here because that's done in
// the OSS tests and it would result in too many permutations. Instead
// we're only testing with the ServiceDefaults controller which will exercise
// all the namespaces code.
Copy link
Member

Choose a reason for hiding this comment

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

Need to update this comment a bit

Copy link
Contributor

@kschoche kschoche left a comment

Choose a reason for hiding this comment

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

Love this refactor, great stuff :D

@thisisnotashwin thisisnotashwin force-pushed the refactor-ent-controller-tests branch from bf0c6c8 to a0c74d7 Compare September 30, 2020 18:46
@thisisnotashwin thisisnotashwin merged commit 7aaa508 into crd-controller-base Sep 30, 2020
@thisisnotashwin thisisnotashwin deleted the refactor-ent-controller-tests branch September 30, 2020 18:51
ndhanushkodi pushed a commit to ndhanushkodi/consul-k8s that referenced this pull request Jul 9, 2021
Fixed template newline issue in connect-inject deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/crds type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants