feat: Ensure example matches regex when creating a term #525
Conversation
| timestamp = `^([\+-]?\d{4}(?!\d{2}\b))((-?)((0[1-9]|1[0-2])(\3([12]\d|0[1-9]|3[01]))?|W([0-4]\d|5[0-2])(-?[1-7])?|(00[1-9]|0[1-9]\d|[12]\d{2}|3([0-5]\d|6[1-6])))([T\s]((([01]\d|2[0-3])((:?)[0-5]\d)?|24\:?00)([\.,]\d+(?!:))?)?(\17[0-5]\d([\.,]\d+)?)?([zZ]|([\+-])([01]\d|2[0-3]):?([0-5]\d)?)?)?)?$` | ||
| date = `^([\+-]?\d{4}(?!\d{2}\b))((-?)((0[1-9]|1[0-2])(\3([12]\d|0[1-9]|3[01]))?|W([0-4]\d|5[0-2])(-?[1-7])?|(00[1-9]|0[1-9]\d|[12]\d{2}|3([0-5]\d|6[1-6])))?)` | ||
| timestamp = `^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(Z|[+-]\d{2}:\d{2})$` | ||
| date = `^\d{4}-\d{2}-\d{2}` |
There was a problem hiding this comment.
It appears that Go’s regexp engine does not support Perl-style negative lookaheads: (?!. When running the test suite against the existing regular expressions using the newly added regex validation, I got the following error:
=== RUN TestMatcher_SugarMatchers
--- FAIL: TestMatcher_SugarMatchers (0.00s)
panic: Term: invalid regex "^([\\+-]?\\d{4}(?!\\d{2}\\b))((-?)((0[1-9]|1[0-2])(\\3([12]\\d|0[1-9]|3[01]))?|W([0-4]\\d|5[0-2])(-?[1-7])?|(00[1-9]|0[1-9]\\d|[12]\\d{2}|3([0-5]\\d|6[1-6])))?)": error parsing regexp: invalid or unsupported Perl syntax: `(?!` [recovered]
panic: Term: invalid regex "^([\\+-]?\\d{4}(?!\\d{2}\\b))((-?)((0[1-9]|1[0-2])(\\3([12]\\d|0[1-9]|3[01]))?|W([0-4]\\d|5[0-2])(-?[1-7])?|(00[1-9]|0[1-9]\\d|[12]\\d{2}|3([0-5]\\d|6[1-6])))?)": error parsing regexp: invalid or unsupported Perl syntax: `(?!`
goroutine 38 [running]:
testing.tRunner.func1.2({0x104eb5980, 0xc00002b410})
/usr/local/go/src/testing/testing.go:1734 +0x2bc
testing.tRunner.func1()
/usr/local/go/src/testing/testing.go:1737 +0x47c
panic({0x104eb5980?, 0xc00002b410?})
/usr/local/go/src/runtime/panic.go:792 +0x124
github.com/pact-foundation/pact-go/v2/matchers.Term({0xc00000fcb6, 0xa}, {0x104e33f99, 0x96})
/Users/nathan/Development/pact-go/matchers/matcher.go:118 +0x2a0
github.com/pact-foundation/pact-go/v2/matchers.Date()
/Users/nathan/Development/pact-go/matchers/matcher.go:161 +0xe4
github.com/pact-foundation/pact-go/v2/matchers.TestMatcher_SugarMatchers(0xc00016d880)
/Users/nathan/Development/pact-go/matchers/matcher_test.go:472 +0x8b4
testing.tRunner(0xc00016d880, 0x104f0f158)
/usr/local/go/src/testing/testing.go:1792 +0x184
created by testing.(*T).Run in goroutine 1
/usr/local/go/src/testing/testing.go:1851 +0x688
FAIL github.com/pact-foundation/pact-go/v2/matchers 0.279s
FAIL
make: *** [test] Error 1
thompsnm
force-pushed
the
verify-term-example-against-regex
branch
from
8189207 to
e49be00
Compare
|
Thanks for the change, looks good from a cursory review, have tagged Matt as well for review and set CI off :) |
|
Thanks. Go doesn't evaluate those regexes - they're evaluated in the core matching engine. This is likely why the issue is only appearing once you also apply the check. I think the change is reasonable, but probably makes more sense to be in the core so all languages get the same benefit (in fact, I thought that was the behaviour of the core already). The regex appears to be a looser version of the current one, so I don't think it will break existing clients that may upgrade to a version that includes this. Please correct me if I'm wrong. |
I noticed that the Term matcher does not verify that the example matches the regular expression. For example, the following was considered valid:
This creates the potential for a consumer to write tests against an example value in the mock provider that does not match what the provider actually returns, causing a breakdown in the assurances the Pact ecosystem is meant to provide. I updated the
Termfunction to verify the regex is valid and that the example matches the regex. In doing so, I discovered that two of the sugar regexes defined in thematcher.gofile were not valid regular expressions in Go. I took a first pass at updating them to make them valid.