Skip to content

Commit

Permalink
Reformat validation violation messages
Browse files Browse the repository at this point in the history
... to improve readability.

Usability testing (#707) made clear that violation messages on a single
line are quite difficult to visually parse, making such error messages
less readable.

To support the reformatting, context is gathered at the time a violation
is detected. The message is formatted once all violations have
been collected.

Also includes minor improvements to "named" rule messages to improve
readability.
  • Loading branch information
jtigger committed Sep 12, 2022
1 parent bbb155a commit c035955
Show file tree
Hide file tree
Showing 39 changed files with 316 additions and 138 deletions.
94 changes: 70 additions & 24 deletions pkg/cmd/template/schema_consumer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1536,8 +1536,11 @@ foo: 0
`
valuesYAML := `foo: 1`

expectedErrMsg := `One or more data values were invalid:
- (schema.yaml:3) requires "foo > 2" (by schema.yaml:2)
expectedErrMsg := `Validating final data values:
(document)
from: schema.yaml:3
- must be: foo > 2 (by: schema.yaml:2)
`
assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
})
Expand All @@ -1549,8 +1552,11 @@ foo: 0
`
valuesYAML := `foo: 1`

expectedErrMsg := `One or more data values were invalid:
- .foo (values.yaml:1) requires "foo > 2" (by schema.yaml:3)
expectedErrMsg := `Validating final data values:
foo
from: values.yaml:1
- must be: foo > 2 (by: schema.yaml:3)
`
assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
})
Expand All @@ -1566,9 +1572,15 @@ foo:
- 1
`

expectedErrMsg := `One or more data values were invalid:
- .foo[0] (values.yaml:2) requires "foo > 2" (by schema.yaml:4)
- .foo[1] (values.yaml:3) requires "foo > 2" (by schema.yaml:4)
expectedErrMsg := `Validating final data values:
foo[0]
from: values.yaml:2
- must be: foo > 2 (by: schema.yaml:4)
foo[1]
from: values.yaml:3
- must be: foo > 2 (by: schema.yaml:4)
`
assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
})
Expand All @@ -1585,8 +1597,12 @@ bar: 0
`
valuesYAML := ``

expectedErrMsg := `One or more data values were invalid:
- .bar (schema.yaml:8) requires "not null"; fail: value is null (by schema.yaml:7)
expectedErrMsg := `Validating final data values:
bar
from: schema.yaml:8
- must be: not null (by: schema.yaml:7)
found: value is null
`
assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
})
Expand All @@ -1600,8 +1616,11 @@ foo: 0
`
valuesYAML := `foo: 1`

expectedErrMsg := `One or more data values were invalid:
- .foo (values.yaml:1) requires "foo > 2" (by schema.yaml:4)
expectedErrMsg := `Validating final data values:
foo
from: values.yaml:1
- must be: foo > 2 (by: schema.yaml:4)
`
assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
})
Expand All @@ -1622,8 +1641,11 @@ new: ""
`
valuesYAML := `existing: foo`

expectedErrMsg := `One or more data values were invalid:
- .new (schema.yaml:11) requires "non-empty" (by schema.yaml:10)
expectedErrMsg := `Validating final data values:
new
from: schema.yaml:11
- must be: non-empty (by: schema.yaml:10)
`
assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
})
Expand All @@ -1643,8 +1665,11 @@ existing: ""
existing: foo
`

expectedErrMsg := `One or more data values were invalid:
- .existing (values.yaml:2) requires "a long string" (by schema.yaml:4)
expectedErrMsg := `Validating final data values:
existing
from: values.yaml:2
- must be: a long string (by: schema.yaml:4)
`
assertFailsWithSchemaAndDataValues(t, schemaYAML, valuesYAML, expectedErrMsg)
})
Expand Down Expand Up @@ -1673,10 +1698,19 @@ bar:
`

// none of the rules are from values.yml:
expectedErr := `One or more data values were invalid:
- (schema.yml:3) requires "has 3 map items" (by schema.yml:2)
- .foo (values.yml:5) requires "non-zero" (by schema.yml:4)
- .bar (schema.yml:7) requires "has 2 items" (by schema.yml:6)
expectedErr := `Validating final data values:
(document)
from: schema.yml:3
- must be: has 3 map items (by: schema.yml:2)
foo
from: values.yml:5
- must be: non-zero (by: schema.yml:4)
bar
from: schema.yml:7
- must be: has 2 items (by: schema.yml:6)
`

opts := &cmdtpl.Options{}
Expand All @@ -1702,11 +1736,23 @@ foo:
- -1
`

expectedErr := `One or more data values were invalid:
- .foo[0] (values.yml:5) requires "non-zero" (by schema.yml:4)
- .foo[0] (values.yml:5) requires "be odd" (by values.yml:4)
- .foo[1] (values.yml:7) requires "non-zero" (by schema.yml:4)
- .foo[1] (values.yml:7) requires "be even" (by values.yml:6)
expectedErr := `Validating final data values:
foo[0]
from: values.yml:5
- must be: non-zero (by: schema.yml:4)
foo[0]
from: values.yml:5
- must be: be odd (by: values.yml:4)
foo[1]
from: values.yml:7
- must be: non-zero (by: schema.yml:4)
foo[1]
from: values.yml:7
- must be: be even (by: values.yml:6)
`

opts := &cmdtpl.Options{}
Expand Down
18 changes: 13 additions & 5 deletions pkg/validations/filetests/all-rules-are-run.tpltest
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,16 @@ config: [1,1]
+++

ERR:
- .config (stdin:4) requires "custom rule"; fail: fails (by stdin:3)
- .config (stdin:4) requires "length greater or equal to 10"; fail: length of 2 is less than 10 (by stdin:3)
- .config (stdin:4) requires "length less than or equal to 1"; fail: length of 2 is more than 1 (by stdin:3)
- .config (stdin:4) requires "a value greater or equal to [2, 2]"; fail: value is less than [2, 2] (by stdin:3)
- .config (stdin:4) requires "a value less than or equal to [0, 0]"; fail: value is more than [0, 0] (by stdin:3)
config
from: stdin:4
- must be: custom rule (by: stdin:3)
found: fails
- must be: length >= 10 (by: stdin:3)
found: length = 2
- must be: length <= 1 (by: stdin:3)
found: length = 2
- must be: a value >= [2, 2] (by: stdin:3)
found: value < [2, 2]
- must be: a value <= [0, 0] (by: stdin:3)
found: value > [0, 0]

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ foo: 11
+++

ERR:
- .foo (stdin:2) requires "a value less than or equal to 10"; fail: value is more than 10 (by stdin:1)
foo
from: stdin:2
- must be: a value <= 10 (by: stdin:1)
found: value > 10

Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ value:
+++

ERR:
- .value (stdin:2) requires "a value less than or equal to {\"foo\": True}"; dict <= dict not implemented (by stdin:1)
value
from: stdin:2
- must be: a value <= {"foo": True} (by: stdin:1)
found: dict <= dict not implemented

Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@ value:
+++

ERR:
- .value (stdin:2) requires "a value less than or equal to 4"; dict <= int not implemented (by stdin:1)
value
from: stdin:2
- must be: a value <= 4 (by: stdin:1)
found: dict <= int not implemented

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ value: "123456"
+++

ERR:
- .value (stdin:2) requires "length less than or equal to 5"; fail: length of 6 is more than 5 (by stdin:1)
value
from: stdin:2
- must be: length <= 5 (by: stdin:1)
found: length = 6

12 changes: 10 additions & 2 deletions pkg/validations/filetests/max_len=/works-with-int-and-float
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,13 @@ bar: "longer than 10"
+++

ERR:
- .foo (stdin:2) requires "length less than or equal to 10"; fail: length of 14 is more than 10 (by stdin:1)
- .bar (stdin:4) requires "length less than or equal to 10"; fail: length of 14 is more than 10 (by stdin:3)
foo
from: stdin:2
- must be: length <= 10 (by: stdin:1)
found: length = 14

bar
from: stdin:4
- must be: length <= 10 (by: stdin:3)
found: length = 14

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ value: 9
+++

ERR:
- .value (stdin:2) requires "a value greater or equal to 10"; fail: value is less than 10 (by stdin:1)
value
from: stdin:2
- must be: a value >= 10 (by: stdin:1)
found: value < 10

Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ value:
+++

ERR:
- .value (stdin:2) requires "a value greater or equal to {\"foo\": True}"; dict >= dict not implemented (by stdin:1)
value
from: stdin:2
- must be: a value >= {"foo": True} (by: stdin:1)
found: dict >= dict not implemented

Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@ value:
+++

ERR:
- .value (stdin:2) requires "a value greater or equal to 4"; dict >= int not implemented (by stdin:1)
value
from: stdin:2
- must be: a value >= 4 (by: stdin:1)
found: dict >= int not implemented

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ value: "1234"
+++

ERR:
- .value (stdin:2) requires "length greater or equal to 5"; fail: length of 4 is less than 5 (by stdin:1)
value
from: stdin:2
- must be: length >= 5 (by: stdin:1)
found: length = 4

Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,13 @@ bar: "shorter than 20"
+++

ERR:
- .foo (stdin:2) requires "length greater or equal to 20"; fail: length of 15 is less than 20 (by stdin:1)
- .bar (stdin:4) requires "length greater or equal to 20"; fail: length of 15 is less than 20 (by stdin:3)
foo
from: stdin:2
- must be: length >= 20 (by: stdin:1)
found: length = 15

bar
from: stdin:4
- must be: length >= 20 (by: stdin:3)
found: length = 15

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ value: null
+++

ERR:
- .value (stdin:2) requires "not null"; fail: value is null (by stdin:1)
value
from: stdin:2
- must be: not null (by: stdin:1)
found: value is null

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ value: null
+++

ERR:
- .value (stdin:2) requires "not null"; fail: value is null (by stdin:1)
value
from: stdin:2
- must be: not null (by: stdin:1)
found: value is null

Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,8 @@ config:
+++

ERR:
- .config (stdin:2) requires "exactly one child not null"; check: multiple values are not null ["foo" "bar"] (by stdin:1)
config
from: stdin:2
- must be: exactly one of all children to be not null (by: stdin:1)
found: ["foo", "bar"] are not null

Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ foo: critical
+++

ERR:
- .foo (stdin:2) requires "one of"; fail: value not in ["debug", "info", "warning", "error", "fatal"] (by stdin:1)
foo
from: stdin:2
- must be: one of ["debug", "info", "warning", "error", "fatal"] (by: stdin:1)
found: not one of allowed values

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#@assert/validate not_null=True
foo: null

+++

ERR:
foo
from: stdin:2
- must be: not null (by: stdin:1)
found: value is null

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
#@assert/validate ("", lambda v: fail("rules were run"))
foo: ""

+++

ERR:
foo
from: stdin:2
- must be: (by: stdin:1)
found: rules were run

6 changes: 5 additions & 1 deletion pkg/validations/filetests/when=/can-use-parent-value.tpltest
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@ counters:
+++

ERR:
- .counters.foo (stdin:4) requires "fail"; fail: fails (by stdin:3)
counters.foo
from: stdin:4
- must be: fail (by: stdin:3)
found: fails

6 changes: 5 additions & 1 deletion pkg/validations/filetests/when=/can-use-root-value.tpltest
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@ counters:
+++

ERR:
- .counters.bar (stdin:6) requires "fail"; fail: fails (by stdin:5)
counters.bar
from: stdin:6
- must be: fail (by: stdin:5)
found: fails

Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ foo: bar
+++

ERR:
Validating .foo: Failure evaluating when=: fail: error from within lambda
Validating foo: Failure evaluating when=: fail: error from within lambda
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ foo: bar
+++

ERR:
Validating .foo: want when= to be bool, got string
Validating foo: want when= to be bool, got string
Loading

0 comments on commit c035955

Please sign in to comment.