Add string constraints (pattern)

[hwchiu]

for issue #50

[coveralls]

Coverage decreased (-0.2%) to 94.099% when pulling 2270e43c4940c1cb0597de5f70b03b8f5c367249 on hwchiu:add-string-constraints into 8912050fec40fd0560512d547472f27b9c85d1d8 on frictionlessdata:master.

[coveralls]

Coverage decreased (-0.2%) to 94.099% when pulling a35d23b647ff1c17c509d1b7552dc69f547dbd7a on hwchiu:add-string-constraints into 8912050fec40fd0560512d547472f27b9c85d1d8 on frictionlessdata:master.

[coveralls]

Coverage increased (+0.05%) to 94.31% when pulling ea0334a115fb1b840f59bf238e7c167ded9b06be on hwchiu:add-string-constraints into 8912050fec40fd0560512d547472f27b9c85d1d8 on frictionlessdata:master.

[coveralls]

Coverage decreased (-0.3%) to 93.911% when pulling f07283b21b9172ab19ee8d18f75cd5959e8e1416 on hwchiu:add-string-constraints into 8912050fec40fd0560512d547472f27b9c85d1d8 on frictionlessdata:master.

[hwchiu]

In order to check the error of regex.Compile, I compile pattern to regex before we call decodeString() in string_test.go. By the way, for the coverage result of field.go, should I also add some testing to test pattern for compiledRegexp ?

[danielfireman]

Agreed.

Em 4 de out de 2017 10:04, "Hung-Wei Chiu" notifications@github.com escreveu:

@hwchiu commented on this pull request.

In schema/string.go https://github.com/frictionlessdata/tableschema-go/pull/52#discussion_r142663415 :

}

if maxLength != 0 && len(v) > maxLength {

• return fmt.Errorf("constraint check error: %s:%v %v > maximum:%v", t, v, maxLength)
• return fmt.Errorf("constraint check error: %v %v > maximum:%v", v, len(v), maxLength)
• }
• if pattern != nil {
• match := pattern.MatchString(v)

I think we still need to check if compiledRegexp is nil, because the compiledRegexp won't be initialized in some cases.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/frictionlessdata/tableschema-go/pull/52#discussion_r142663415, or mute the thread https://github.com/notifications/unsubscribe-auth/AIiWQ9DYV3oGNy7iath0pBsg4tnmDLe7ks5so4J0gaJpZM4PrnkL .

[danielfireman]

Exactly! It is okay to use in our testing.

Em 4 de out de 2017 10:15, "Hung-Wei Chiu" notifications@github.com escreveu:

@hwchiu commented on this pull request.

In schema/string_test.go https://github.com/frictionlessdata/tableschema-go/pull/52#discussion_r142665808 :

@@ -29,9 +32,20 @@ func TestDecodeString_ErrorCheckingConstraints(t *testing.T) { {"InvalidMaxLength_UUID", "6fa459ea-ee8a-3ca4-894e-db77e160355e", stringUUID, Constraints{MaxLength: 1}}, {"InvalidMaxLength_Email", "foo@bar.com", stringEmail, Constraints{MaxLength: 1}}, {"InvalidMaxLength_URI", "http://google.com", stringURI, Constraints{MaxLength: 1}},

• {"InvalidPattern_UUID", "6fa459ea-ee8a-3ca4-894e-db77e160355e", stringUUID, Constraints{Pattern: "^[0-9a-f]{1}-.*"}},
• {"InvalidPattern_Email", "foo@bar.com", stringEmail, Constraints{Pattern: "[0-9].*"}},
• {"InvalidPattern_URI", "http://google.com", stringURI, Constraints{Pattern: "^//.*"}},

ok, The reason why I don't use MustCompile before is it will panic the program when it compile fails rather than issue an error. I think we can control those testing pattern to make sure it always compile success, so, it's fine to use MustCompile to pass complied patten.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/frictionlessdata/tableschema-go/pull/52#discussion_r142665808, or mute the thread https://github.com/notifications/unsubscribe-auth/AIiWQ7qR1slINS7UZMoPweEhUHnzHGwmks5so4TXgaJpZM4PrnkL .

[danielfireman]

But we will in our production code, which is what matters for this snippet.

So the test case should adapt and pass both: Pattern and compiled pattern. In that way you have the testing mimicking production, which is super desired.

Em 4 de out de 2017 10:21 AM, "Hung-Wei Chiu" notifications@github.com escreveu:

@hwchiu commented on this pull request.

In schema/string.go https://github.com/frictionlessdata/tableschema-go/pull/52#discussion_r142667452 :

}

if maxLength != 0 && len(v) > maxLength {

• return fmt.Errorf("constraint check error: %s:%v %v > maximum:%v", t, v, maxLength)
• return fmt.Errorf("constraint check error: %v %v > maximum:%v", v, len(v), maxLength)
• }
• if pattern != nil {
• match := pattern.MatchString(v)
• if false == match {
• return fmt.Errorf("constraint check error: %v don't fit pattern : %v ", v, pattern)

I think we should print out the compiled pattern to show the pattern instead of the Constraints.Pattern, because we won't pass any pattern(string) to Constraints in our testing function (string_test.go).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/frictionlessdata/tableschema-go/pull/52#discussion_r142667452, or mute the thread https://github.com/notifications/unsubscribe-auth/AIiWQx9sQJoA5MpNLI0QoNo8nHHwGv0bks5so4ZugaJpZM4PrnkL .

[coveralls]

Coverage decreased (-0.3%) to 93.924% when pulling ea0f0f63f3fe968cc954238d7d0b25e30b05db38 on hwchiu:add-string-constraints into 8912050fec40fd0560512d547472f27b9c85d1d8 on frictionlessdata:master.

[hwchiu]

ok, I will pass pattern to constraint too

[coveralls]

Coverage decreased (-0.3%) to 93.924% when pulling 73082d035797e5c22c47bee26d3985de8ee5d534 on hwchiu:add-string-constraints into 8912050fec40fd0560512d547472f27b9c85d1d8 on frictionlessdata:master.

[danielfireman]

Super! Thanks for those changes!