Update Version regex to support ranges #359
Conversation
Codecov Report
@@ Coverage Diff @@
## main #359 +/- ##
==========================================
+ Coverage 81.46% 82.11% +0.64%
==========================================
Files 21 21
Lines 928 928
==========================================
+ Hits 756 762 +6
+ Misses 119 116 -3
+ Partials 53 50 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| @@ -69,7 +77,51 @@ var _ = Describe("Operator Spec Validations", func() { | |||
| })) | |||
|
|
|||
| Expect(err).To(HaveOccurred(), "expected error for invalid semver %q", invalidSemver) | |||
| Expect(err.Error()).To(ContainSubstring("spec.version in body should match '^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(-(0|[1-9]\\d*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*)(\\.(0|[1-9]\\d*|[0-9]*[a-zA-Z-][0-9a-zA-Z-]*))*)?(\\+([0-9a-zA-Z-]+(\\.[0-9a-zA-Z-]+)*))?$'")) | |||
| Expect(err.Error()).To(ContainSubstring("spec.version in body should match '^(\\s*(=||!=|>|<|>=|=>|<=|=<|~|~>|\\^)\\s*(v?(0|[1-9]\\d*|[x|X|\\*])(\\.(0|[1-9]\\d*|x|X|\\*]))?(\\.(0|[1-9]\\d*|x|X|\\*))?(-([0-9A-Za-z\\-]+(\\.[0-9A-Za-z\\-]+)*))?(\\+([0-9A-Za-z\\-]+(\\.[0-9A-Za-z\\-]+)*))?)\\s*)((?:\\s+|,\\s*|\\s*\\|\\|\\s*)(=||!=|>|<|>=|=>|<=|=<|~|~>|\\^)\\s*(v?(0|[1-9]\\d*|x|X|\\*])(\\.(0|[1-9]\\d*|x|X|\\*))?(\\.(0|[1-9]\\d*|x|X|\\*]))?(-([0-9A-Za-z\\-]+(\\.[0-9A-Za-z\\-]+)*))?(\\+([0-9A-Za-z\\-]+(\\.[0-9A-Za-z\\-]+)*))?)\\s*)*$'")) | |||
There was a problem hiding this comment.
Do we get anything out of including the entire regexp pattern in the assertion? I wonder if we should just check for substring spec.version in body should match?
There was a problem hiding this comment.
Probably not, there's only one important regex per field; it's the field name that's important.
|
@joelanford made requested changes. |
Fixes operator-framework#345 Add positive and negative test cases. Signed-off-by: Todd Short <tshort@redhat.com>
tmshort
changed the title
|
LGTM However, do we need to keep this PR held open until the backing implementation described in #346 is implemented? I'm guessing if we merge without masterminds in the picture, the result is that masterminds-but-not-blang ranges will result in a reconcile error instead of a validation error? |
Yes, but it's necessary to be able to test ("get to") the reconcile code. |
|
@joelanford since there's a PR up for the MM code changes (updates), do you have any objections to this being merged? |
|
Closing in favor of #374 (which includes this). |
Fixes #345
Add positive and negative test cases.
Description
Reviewer Checklist