Correctly handle record_errors scenario #51
Conversation
| create_schema("foo_schema", { | ||
| "type" => "object", | ||
| "required" => ["foo"], | ||
| }) |
There was a problem hiding this comment.
Indent the right brace the same as the first position after the preceding left parenthesis.
|
|
||
| it "fails when the body is missing a required property" do | ||
| create_schema("foo_schema", { | ||
| "type" => "object", |
There was a problem hiding this comment.
Use 2 spaces for indentation in a hash, relative to the first position after the preceding left parenthesis.
| end | ||
|
|
||
| it "fails when the body is missing a required property" do | ||
| create_schema("foo_schema", { |
There was a problem hiding this comment.
Redundant curly braces around a hash parameter.
| JSON::Validator.validate!( | ||
| schema_path.to_s, | ||
| Payload.new(response).to_s, | ||
| options |
There was a problem hiding this comment.
Put a comma after the last parameter of a multiline method call.
| errors = JSON::Validator.fully_validate( | ||
| schema_path.to_s, | ||
| Payload.new(response).to_s, | ||
| options |
There was a problem hiding this comment.
Put a comma after the last parameter of a multiline method call.
| options, | ||
| ) | ||
| # validate! will not raise and will always return true if you configure the validator | ||
| # to record errors, so we must instead inspect fully_validate's errors response |
| Payload.new(response).to_s, | ||
| options, | ||
| ) | ||
| # validate! will not raise and will always return true if you configure the validator |
| create_schema("foo_schema", | ||
| "type" => "object", | ||
| "required" => ["foo"], | ||
| ) |
There was a problem hiding this comment.
Closing method call brace must be on the same line as the last argument when opening brace is on the same line as the first argument.
| it "fails when the body is missing a required property" do | ||
| create_schema("foo_schema", | ||
| "type" => "object", | ||
| "required" => ["foo"],) |
There was a problem hiding this comment.
Avoid comma after the last parameter of a method call, unless each item is on its own line.
|
We need Can this be merged in ASAP? Currently scratching heads at failing test and this would really help debug. |
Closes [#51]. Extract `JsonMatchers::Validator` to encapsulate error reporting behavior. `JSON::Validator` exposes a `#validate!` method that will raise on validation failures, and a `#fully_validate!` method that won't raise, but will return an array of error messages instead. Additionally, this commit introduces the `#with_options` test helper that scopes configuration options to a given test block. [#51]: #51
Closes [#51]. Extract `JsonMatchers::Validator` to encapsulate error reporting behavior. `JSON::Validator` exposes a `#validate!` method that will raise on validation failures, and a `#fully_validate!` method that won't raise, but will return an array of error messages instead. Additionally, this commit introduces the `#with_options` test helper that scopes configuration options to a given test block. [#51]: #51
|
Thank you @BBonifield for this contribution! This will make failure messages much more informative. This PR was merged in 1a5c39e. I've opened #52 as a follow-up. What are the downsides to leaving Do you think we should default it to |
Closes [#51]. By default, `JsonMatchers.configuration.options[:record_errors] = true`. Extract `JsonMatchers::Validator` to encapsulate error reporting behavior. `JSON::Validator` exposes a `#validate!` method that will raise on validation failures, and a `#fully_validate!` method that won't raise, but will return an array of error messages instead. [#51]: #51
Ensure that validation is always configured with `record_errors: true`, based on the following [comments]: > By default, the underlying Validator will raise an error when passed > invalid JSON. Unfortunately, this error tends to be high-level and > [vague](#51). > When configured with `record_errors: true`, the validator will collect > and return an array of error messages instead of raising. > This feels like a [reasonable > default](#51 (comment)), > so this PR makes it so. [comments]: #52 (comment)
Ensure that validation is always configured with `record_errors: true`, based on the following [comments]: > By default, the underlying Validator will raise an error when passed > invalid JSON. Unfortunately, this error tends to be high-level and > [vague](#51). > When configured with `record_errors: true`, the validator will collect > and return an array of error messages instead of raising. > This feels like a [reasonable > default](#51 (comment)), > so this PR makes it so. [comments]: #52 (comment)
When you configure
JsonMatcherstorecord_errors, it will result in all matchers returning as valid. The matcher was written around the idea that.validate!will raise an error if the JSON does not match the schema. However when you haverecord_errorsset totrue, things fall apart.First,
json_schemawill catch all errors and collect them in an array for error reporting. Next,validate!will always returntruesince it's written to assume that it should raise. In order to handle this scenario, I switched the matcher to usefully_validate, which allows for inspection of the errors.The use-case that brought this up for me was the desire to have better error reporting for when you use
allOf. The default error message fromjson_schemais pretty useless because it doesn't give you any of the errors from the sub-schemas.