Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] Failing story validation does not tell which property is unevaluated #364

Closed
dblock opened this issue Jun 26, 2024 · 3 comments · Fixed by #378
Closed

[FEATURE] Failing story validation does not tell which property is unevaluated #364

dblock opened this issue Jun 26, 2024 · 3 comments · Fixed by #378
Assignees
Labels
enhancement New feature or request

Comments

@dblock
Copy link
Member

dblock commented Jun 26, 2024

Is your feature request related to a problem?

After #362 if a story has an unevaluated property the error doesn't say which property is unevaluated.

In the following example synopsis is invalid.

prologues:
  - synopsis: Create an index named `books` with mappings and settings.
    path: /{index}
    method: PUT
    parameters:
      index: books
    request_body:
      payload: {}

When running tests.

ERROR   cat/indices.yaml (Invalid Story: data/prologues/0 must NOT have unevaluated properties)

What solution would you like?

Similar to the use of ajv_errors in the response body/schema validator.

@dblock dblock added enhancement New feature or request untriaged and removed untriaged labels Jun 26, 2024
@nhtruong
Copy link
Collaborator

nhtruong commented Jun 27, 2024

I'm not a fan of having to inject error message instructions into every schema to get the proper message with ajv_errors. It's too cumbersome and relies on our schema traversing mechanism not missing anything. So far, it's easy to pinpoint where the violation happens with ajv.errorsText except for additionalProperties and unevaludatedProperrties errors. A solution I'm thinking of:

const validate = ajv.compile(schema)
if(!validate(data)) {
 const { prohibited_props,  others } = validate.errors.reduce((result, error) => {
    if (is_prohibited_prop_error(error)) result.prohibited_props.push(item);
    else result.fail.push(item);
    return result;
  }, { prohibited_props: [], others: [] });
  
  return ajv.errorsText(others) + ' ' + prohibited_props.map((e) => prohibited_prop_message(e)).join(' ')
}

@dblock
Copy link
Member Author

dblock commented Jun 27, 2024

I think any solution is fine. We probably do want to consolidate code that invokes ajv into one class for reuse.

@nhtruong nhtruong self-assigned this Jul 3, 2024
@nhtruong
Copy link
Collaborator

nhtruong commented Jul 3, 2024

I'll handle this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants