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

Add unit tests for "validate" struct tags #613

Merged
merged 5 commits into from
Sep 17, 2024
Merged

Add unit tests for "validate" struct tags #613

merged 5 commits into from
Sep 17, 2024

Conversation

mbarnes
Copy link
Collaborator

@mbarnes mbarnes commented Sep 16, 2024

What this PR does

Adds unit tests for the various "validate" struct tags in use, which leverage the package validator module. I also added some tweaks and corrections to existing "validate" tags based on this testing.

Another benefit to this test framework is it allows us to easily try out new validators, or combinations of validators, without having to stand up a full test environment.

Jira: none, was inspired by a Slack thread
Link to demo recording:

Special notes for your reviewer

Matthew Barnes added 4 commits September 16, 2024 13:21
This results in more detail in the validation errors in case
entire subsections of the resource specification are missing.

For example, instead of just reporting a missing "version"

  "Missing required field 'properties.spec.version'"

call out the required fields within "version"

  "Missing required field 'properties.spec.version.id'"
  "Missing required field 'properties.spec.version.channelGroup'"
Read-only fields don't require user input validation.
Node pool validation tags are lagging behind the API spec.
Specifically for OutboundType which is always "loadBalancer".

  ... (must be loadBalancer)

instead of

  ... (must be one of: loadBalancer)
This is important to have anyway, but it also provides a way to
try new validation tags without requiring a fully deployed test
environment.
@mjlshen mjlshen enabled auto-merge (rebase) September 17, 2024 12:47
@mjlshen mjlshen disabled auto-merge September 17, 2024 12:47
@mjlshen mjlshen enabled auto-merge (squash) September 17, 2024 12:47
Copy link
Contributor

@mjlshen mjlshen left a comment

Choose a reason for hiding this comment

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

This is great - thanks for working on it!

@mjlshen mjlshen merged commit 930de94 into main Sep 17, 2024
22 checks passed
@mjlshen mjlshen deleted the validation-tests branch September 17, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants