From 74341169332a546765d5afa2db9df9fa4fb52763 Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Thu, 21 Jan 2021 16:28:08 -0700 Subject: [PATCH] DEV: Add schema checking to api doc testing (#11721) * DEV: Add schema checking to api doc testing 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 * Swap out json schema validator gem Swapped out the outdated json-schema_builder gem with the json_schemer gem. * Add validation fields to schema 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. --- Gemfile | 1 + Gemfile.lock | 10 +++ .../requests/api/schemas/tag_group_schemas.rb | 77 +++++++++++++++++++ spec/requests/api/shared/shared_examples.rb | 41 ++++++++++ spec/requests/api/tags_spec.rb | 46 +++-------- spec/swagger_helper.rb | 7 ++ 6 files changed, 147 insertions(+), 35 deletions(-) create mode 100644 spec/requests/api/schemas/tag_group_schemas.rb create mode 100644 spec/requests/api/shared/shared_examples.rb diff --git a/Gemfile b/Gemfile index 3ea59bc92e5..80bf241db0d 100644 --- a/Gemfile +++ b/Gemfile @@ -167,6 +167,7 @@ group :test, :development do gem 'parallel_tests' gem 'rswag-specs' + gem 'json_schemer' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index e671a04f952..56f0b0c4e6d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -114,6 +114,8 @@ GEM in_threads (~> 1.3) progress (~> 3.0, >= 3.0.1) docile (1.3.5) + ecma-re-validator (0.3.0) + regexp_parser (~> 2.0) email_reply_trimmer (0.1.13) ember-data-source (3.0.2) ember-source (>= 2, < 3.0) @@ -141,6 +143,7 @@ GEM globalid (0.4.2) activesupport (>= 4.2.0) guess_html_encoding (0.0.11) + hana (1.3.7) hashdiff (1.0.1) hashie (4.1.0) highline (2.0.3) @@ -159,6 +162,11 @@ GEM json (2.5.1) json-schema (2.8.1) addressable (>= 2.4) + json_schemer (0.2.17) + ecma-re-validator (~> 0.3) + hana (~> 1.3) + regexp_parser (~> 2.0) + uri_template (~> 0.7) jwt (2.2.2) kgio (2.11.3) libv8 (8.4.255.0) @@ -432,6 +440,7 @@ GEM kgio (~> 2.6) raindrops (~> 0.7) uniform_notifier (1.13.2) + uri_template (0.7.0) webmock (3.11.1) addressable (>= 2.3.6) crack (>= 0.3.2) @@ -494,6 +503,7 @@ DEPENDENCIES htmlentities http_accept_language json + json_schemer listen lograge logstash-event diff --git a/spec/requests/api/schemas/tag_group_schemas.rb b/spec/requests/api/schemas/tag_group_schemas.rb new file mode 100644 index 00000000000..86fd718de61 --- /dev/null +++ b/spec/requests/api/schemas/tag_group_schemas.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +module SpecSchemas + + class TagGroupCreateRequest + def schemer + schema = { + 'type' => 'object', + 'additionalProperties' => false, + 'properties' => { + 'name' => { + 'type' => 'string', + } + }, + 'required' => ['name'] + } + end + end + + class TagGroupResponse + def schemer + schema = { + 'type' => 'object', + 'additionalProperties' => false, + 'properties' => { + 'tag_group' => { + 'type' => 'object', + 'properties' => { + 'id' => { + 'type' => 'integer', + }, + 'name' => { + 'type' => 'string', + }, + 'tag_names' => { + 'type' => 'array', + 'items' => { + 'type' => 'string' + } + }, + 'parent_tag_name' => { + 'type' => 'array', + 'items' => { + 'type' => 'string' + } + }, + 'one_per_topic' => { + 'type' => 'boolean', + }, + 'permissions' => { + 'type' => 'object', + 'properties' => { + 'everyone' => { + 'type' => 'integer', + 'example' => 1 + } + } + } + }, + 'required' => [ + 'id', + 'name', + 'tag_names', + 'parent_tag_name', + 'one_per_topic', + 'permissions' + ] + } + }, + 'required' => [ + 'tag_group' + ] + } + end + end + +end diff --git a/spec/requests/api/shared/shared_examples.rb b/spec/requests/api/shared/shared_examples.rb new file mode 100644 index 00000000000..a4361e27583 --- /dev/null +++ b/spec/requests/api/shared/shared_examples.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +RSpec.shared_examples "a JSON endpoint" do |expected_response_status| + before do |example| + submit_request(example.metadata) + end + + describe "response status" do + it "returns expected response status" do + expect(response.status).to eq(expected_response_status) + end + end + + describe "request body" do + it "matches the documented request schema" do |example| + schemer = JSONSchemer.schema(expected_request_schema.schemer) + valid = schemer.valid?(params) + unless valid # for debugging + puts params + puts schemer.validate(params).to_a[0]["details"] + end + expect(valid).to eq(true) + end + end + + describe "response body" do + let(:json_response) { JSON.parse(response.body) } + + it "matches the documented response schema" do |example| + schemer = JSONSchemer.schema( + expected_response_schema.schemer, + ) + valid = schemer.valid?(json_response) + unless valid # for debugging + puts json_response + puts schemer.validate(json_response).to_a[0]["details"] + end + expect(valid).to eq(true) + end + end +end diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index b5c53cf4fb7..e599af201ae 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -60,46 +60,22 @@ describe 'tags' do post 'Creates a tag group' do tags 'Tags' consumes 'application/json' - parameter name: :post_body, in: :body, schema: { - type: :object, - properties: { - name: { type: :string } - }, - required: [ 'name' ] - } + expected_request_schema = SpecSchemas::TagGroupCreateRequest.new + + parameter name: :params, in: :body, schema: expected_request_schema.schemer produces 'application/json' response '200', 'tag group created' do - schema type: :object, properties: { - tag_group: { - type: :object, - properties: { - id: { type: :integer }, - name: { type: :string }, - tag_names: { - type: :array, - items: { - }, - }, - parent_tag_name: { - type: :array, - items: { - }, - }, - one_per_topic: { type: :boolean }, - permissions: { - type: :object, - properties: { - everyone: { type: :integer }, - } - }, - } - }, - } + expected_response_schema = SpecSchemas::TagGroupResponse.new - let(:post_body) { { name: 'todo' } } + let(:params) { { 'name' => 'todo' } } - run_test! + schema(expected_response_schema.schemer) + + it_behaves_like "a JSON endpoint", 200 do + let(:expected_response_schema) { expected_response_schema } + let(:expected_request_schema) { expected_request_schema } + end end end diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index 12d9f547183..d40966a9610 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -1,6 +1,13 @@ # frozen_string_literal: true require 'rails_helper' +require 'json_schemer' + +# Require schema files +Dir["./spec/requests/api/schemas/*.rb"].each { |file| require file } + +# Require shared spec examples +Dir["./spec/requests/api/shared/*.rb"].each { |file| require file } RSpec.configure do |config| # Specify a root folder where Swagger JSON files are generated