-
Notifications
You must be signed in to change notification settings - Fork 41
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 the unit tests validating for required rule type fields #4998
Conversation
pkg/profiles/validator_test.go
Outdated
@@ -277,7 +277,7 @@ func makeProfile(opts ...func(*minderv1.Profile)) *minderv1.Profile { | |||
|
|||
func loadRawRuleTypeDef() (json.RawMessage, error) { | |||
// read rule type from disk and set it up as a fixture | |||
f, err := os.Open("../../examples/rules-and-profiles/rule-types/github/branch_protection_allow_force_pushes.yaml") | |||
f, err := os.Open("../../examples/rules-and-profiles/rule-types/github/branch_protection_allow_fork_syncing.yaml") |
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.
doesn't allowing fork sync also have the same issue? the branch is no longer mandatory.
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.
We are not relying on the branch
field anymore, instead we use the allow_fork_syncing
required field, i.e. https://github.com/mindersec/minder-rules-and-profiles/blob/5e9722f3472294e3b2cac6ffa115c36acef7aacd/rule-types/github/branch_protection_allow_fork_syncing.yaml#L41
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.
d'oh! that makes sense and somehow I missed it. Thanks for clarifying!
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.
Which is interesting because now it makes me wonder why this test worked before...
118f8eb
to
f863e6d
Compare
Signed-off-by: Radoslav Dimitrov <[email protected]>
f863e6d
to
8d84552
Compare
Summary
The following PR fixes the unit tests validating for required rule type fields
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: