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

DEV: Add schema checking to api doc testing #11721

Merged
merged 3 commits into from Jan 21, 2021
Merged

Conversation

oblakeerickson
Copy link
Member

This commit improves upon rswag which lacks schema checking. rswag
really only checks that the https status matches, but this change adds
in the json-schema_builder gem which also has schema validation.

Now we can define schemas for each of our requests/responses in the
spec/requests/api/schemas directory which will make our documentation
specs a lot cleaner.

If we update a serializer by either adding or removing an attribute the
tests will now fail (this is a good thing!). Also if you change the type
of an attribute say from an array to a string the tests will now fail.
This will help significantly with keeping the docs in sync with actual
code changes! Now if you change how an endpoint will respond you will
have to update the docs too in order for the tests to pass. :D

This PR is inspired by:

https://www.tealhq.com/post/how-teal-keeps-their-api-tests-and-documentation-in-sync

This commit improves upon rswag which lacks schema checking. rswag
really only checks that the https status matches, but this change adds
in the json-schema_builder gem which also has schema validation.

Now we can define schemas for each of our requests/responses in the
`spec/requests/api/schemas` directory which will make our documentation
specs a lot cleaner.

If we update a serializer by either adding or removing an attribute the
tests will now fail (this is a good thing!). Also if you change the type
of an attribute say from an array to a string the tests will now fail.
This will help significantly with keeping the docs in sync with actual
code changes! Now if you change how an endpoint will respond you will
have to update the docs too in order for the tests to pass. :D

This PR is inspired by:

 https://www.tealhq.com/post/how-teal-keeps-their-api-tests-and-documentation-in-sync
@CvX
Copy link
Contributor

CvX commented Jan 14, 2021

json-schema_builder hasn't been updated in 3 years and json-schema gem it relies on also isn't actively maintained anymore.

We could choose a more recent alternative (e.g. json_schemer) to avoid having to fork and maintain those deps by ourselves down the line.

@oblakeerickson
Copy link
Member Author

Yea I saw that they were pretty out of date. I didn't know about json_schemer though!!! I'll update the PR to use that, looks like it should work. Thanks!

Swapped out the outdated json-schema_builder gem with the json_schemer
gem.
@oblakeerickson
Copy link
Member Author

Okay PR has been updated to use the newer json_schemer gem.

It was kind of nice to use the ruby-like schema builder, but that can still be a pain to hand write as well so the prefered method of generating schema is to just take the json output of the api response and use a converter to json_schema and then to just save that .json file. Probably going forward would be to read a json_schema json file into ruby instead of writing a ruby hash for the json_schema.

@oblakeerickson
Copy link
Member Author

Actually I got ahead of myself with json_schemer. Its not doing strict validation like json-schema_builder was:

require 'json_schemer'

schema = {
  'type' => 'object',
  'properties' => {
    'abc' => {
      'type' => 'integer',
      'minimum' => 11
    }
  }
}
schemer = JSONSchemer.schema(schema)

puts schemer.valid?({}) # true? should be false
puts schemer.valid?({ 'xyz' => 10 }) # true? should be false

In order to have "strict" validation we need to add
`additionalProperties: false` to the schema, and we need to specify
which attributes are required.

Updated the debugging test output to print out the error details if
there are any.
@oblakeerickson
Copy link
Member Author

I pushed another update. We can add similar strict validation by using additionalProperties: false and by specifying which values are required.

Copy link
Contributor

@martin-brennan martin-brennan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one minor comment, you are the expert in this area and from what I can see based on the context this all looks in order :)

it "matches the documented request schema" do |example|
schemer = JSONSchemer.schema(expected_request_schema.schemer)
valid = schemer.valid?(params)
unless valid # for debugging
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to leave this unless valid part in? If you did maybe making a helper method would be useful e.g. expect_schema_valid(schemer, params) that does the expectation and also the puts debugging on failure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea its handy to have when trying to figure out why a spec failed, but yes I'll work on a helper method for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants