-
Notifications
You must be signed in to change notification settings - Fork 166
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
fix: Add toUpperCase to provider and region fields in cluster resources #1837
Conversation
@@ -611,3 +613,18 @@ func flattenEndpoints(listEndpoints []matlas.Endpoint) []map[string]any { | |||
} | |||
return endpoints | |||
} | |||
|
|||
func StringIsUppercase() schema.SchemaValidateDiagFunc { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks Andrea!
Question: do we really need one more e2e test for this? Why don't we simply unit test this?
My rationale is: the only business logic that we are responsible for is StringIsUppercase
; because of that, unit test should suffice. The added value that an e2e can bring here is that we are effectively confirming ValidateDiagFunc
method is correctly called.. but that should not be our responsibility and we should trust our framework that will do so.
Happy to discuss more about this (I am thinking this from a "invert the test pyramid" perspective)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I will update the PR. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Ticket: https://jira.mongodb.org/browse/CLOUDP-222427
This PR adds a validation to the
provider_name
andregion_name
fields of the cluster resources to return an error if the user did not provide the uppercase string.Type of change:
Required Checklist:
Testing
I added the unit test
TestStringIsUppercase