query: introduce new ExpectTotalLengthForMatching query check#607
query: introduce new ExpectTotalLengthForMatching query check#607
ExpectTotalLengthForMatching query check#607Conversation
| // produced by multiple list blocks is exactly the given value. | ||
| // | ||
| // This query check can only be used with managed resources that support query. Query is only supported in Terraform v1.14+ | ||
| func ExpectLengthForMultiple(regex *regexp.Regexp, length int) QueryResultCheck { |
There was a problem hiding this comment.
I'm not attached to this name and open to other suggestions, this just seems the most descriptive at first glance to me.
There was a problem hiding this comment.
Yeah, tough naming 🤔
Two things I think would be nice to communicate with the name are that it's checking total length and that it's checking via a form of matching resource addresses via regex. What do you think of ExpectTotalLengthForMatching? ExpectTotalLengthForMultiple? 🤔
There was a problem hiding this comment.
I went with ExpectTotalLengthForMatching
austinvalle
left a comment
There was a problem hiding this comment.
Looking good! I left some thoughts
| // produced by multiple list blocks is exactly the given value. | ||
| // | ||
| // This query check can only be used with managed resources that support query. Query is only supported in Terraform v1.14+ | ||
| func ExpectLengthForMultiple(regex *regexp.Regexp, length int) QueryResultCheck { |
There was a problem hiding this comment.
Yeah, tough naming 🤔
Two things I think would be nice to communicate with the name are that it's checking total length and that it's checking via a form of matching resource addresses via regex. What do you think of ExpectTotalLengthForMatching? ExpectTotalLengthForMultiple? 🤔
88908bf to
273cddb
Compare
|
Failing CI is unrelated to this change |
ExpectLengthForMultiple query checkExpectTotalLengthForMatching query check
| @@ -0,0 +1,5 @@ | |||
| kind: FEATURES | |||
| body: 'querycheck: Add new `ExpectLengthForMultiple` querycheck for validating the number of found resources for multiple list blocks' | |||
There was a problem hiding this comment.
To match the new name 🙂
| body: 'querycheck: Add new `ExpectLengthForMultiple` querycheck for validating the number of found resources for multiple list blocks' | |
| body: 'querycheck: Add new `ExpectTotalLengthForMatching` querycheck for validating the number of found resources for multiple list blocks' |
Related Issue
Related to #597
Description
ExpectTotalLengthForMatchingfor validating the number of found resources of multiple list blocksExpectLengthAtLeastcheckRollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
None