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.

Sign up for GitHub

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

Add support for Minitest #66

Merged

seanpdoyle merged 2 commits into master from minitest-support

Apr 6, 2018

Collaborator

seanpdoyle commented Mar 2, 2018 •

edited

Loading

Introduce JsonMatchers::Minitest::Assertions module, which provides
assert_response_matches_schema and refute_response_matches_schema
assertion methods.

Configure Hound to allow JSON-like hash declarations inside
test/**/*_test.rb files like spec/**/*_spec.rb files.

Add rake test to compliment rake spec. Rake's default task, which
is run in CI, will run both rake test and rake spec.

When including the Minitest assertions, the schema directory will
default to test/support/api/schemas.

This commit re-implements the RSpec matcher to re-use
JsonMatchers::Assertion instead of declaring JsonMatchers::Rspec.

Update the README.md to explain how to configure and use the Minitest
assertions.

Update CONTRIBUTING.md to explain that pull requests must include
tests that cover both Minitest and RSpec.

houndci-bot reviewed

View reviewed changes

test/json_matchers/minitest/assertions_test.rb Outdated

+ create_schema("collection", {
+ "type": "array",
+ "items": { "$ref": "single.json" },
+ })

houndci-bot Mar 2, 2018

Indent the right brace the same as the first position after the preceding left parenthesis.

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

thoughtbot deleted a comment from houndci-bot

houndci-bot reviewed

View reviewed changes

test/json_matchers/minitest/assertions_test.rb Outdated

+ invalid_response = response_for([{ "foo": 0 }])
+ assert_response_matches_schema valid_response ,"collection"
+ refute_response_matches_schema invalid_response ,"collection"

houndci-bot Mar 2, 2018

Space found before comma.
Space missing after comma.

test/json_matchers/minitest/assertions_test.rb Outdated

+ valid_response = response_for([{ "foo": "is a string" }])
+ invalid_response = response_for([{ "foo": 0 }])
+ assert_response_matches_schema valid_response ,"collection"

houndci-bot Mar 2, 2018

Space found before comma.
Space missing after comma.

test/json_matchers/minitest/assertions_test.rb Outdated

+ "required": ["foo"],
+ "properties": {
+ "foo": { "type": "string" },
+ }

houndci-bot Mar 2, 2018

Put a comma after the last item of a multiline hash.

test/json_matchers/minitest/assertions_test.rb Outdated

+ def test_contains_the_body_in_the_failure_message_when_refuted
+ create_schema("foo", { "type": "array" })
+ assert_raises_formatted_error '[ ]' do

houndci-bot Mar 2, 2018

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

test/json_matchers/minitest/assertions_test.rb Outdated

+ "type": "object",
+ "properties": {
+ "foo": { "type": "string" },
+ }

houndci-bot Mar 2, 2018

Put a comma after the last item of a multiline hash.

test/json_matchers/minitest/assertions_test.rb Outdated

+ end
+ end
+ def test_when_passed_an_Array_validates_the_schema_matches

houndci-bot Mar 2, 2018

Inconsistent indentation detected.
Use snake_case for method names.

test/json_matchers/minitest/assertions_test.rb Outdated

+ create_schema("foo_schema",
+ "type": "object",
+ "required": ["foo"],
+ )

houndci-bot Mar 2, 2018

Align ) with (.
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.

test/json_matchers/minitest/assertions_test.rb Outdated

+ end
+ end
+ def test_does_not_fail_with_an_empty_JSON_body

houndci-bot Mar 2, 2018

Use snake_case for method names.

test/json_matchers/minitest/assertions_test.rb Outdated

+class AssertResponseMatchesSchemaTest < JsonMatchers::TestCase
+ include FileHelpers
+ def test_fails_with_an_invalid_JSON_body

houndci-bot Mar 2, 2018

Use snake_case for method names.

test/support/json_matchers/test_case.rb Outdated

+ JSON::Validator.clear_cache
+ @original_schema_root = JsonMatchers.schema_root
+ JsonMatchers.schema_root = File.join(Dir.pwd, "spec", "fixtures", "schemas")

houndci-bot Mar 2, 2018

Line is too long. [82/80]

mike-burns reviewed

View reviewed changes

mike-burns left a comment

The code reads fine but what's the plan here? Should each new test be done twice?

Can you update CONTRIBUTING.md with the answer to that, actually?

seanpdoyle force-pushed the minitest-support branch from 6015e79 to 04cb412 Compare

March 2, 2018 23:09

Collaborator Author

seanpdoyle commented Mar 9, 2018

The code reads fine but what's the plan here? Should each new test be done twice?

I'm not really sure how to proceed.

There are a few options I'm considering:

Continue development with separate integration tests for rspec andminitest, with each test responsible for setting up it's own fixture data in-line.
Continue development with separate integration tests for rspec andminitest, extracting out duplicated fixtures into FactoryBot declarations. Hopefully, with appropriate traits, the duplication can be minimized.
Continue development with a single set of integration tests for rspec andminitest, coming up with a testing abstracting to swap in the appropriate assertion tool. I don't yet know what this looks like, but a class in {spec,test}/support would do the trick.
Continue development with a single set of integration tests for rspec andminitest, re-using the rspec test blocks and doubling down on the verification phase.
Don't add minitest support, and maybe extract a separate gem.

What do you think?

mike-burns commented Mar 9, 2018

I tried looking at what shoulda-matchers and shoulda-context do -- shoulda-matchers works in both Test::Unit and Rspec (and minitest?). It looks like shoulda-matchers simply does not test this. Can you confirm anything about this @jferris ?

I love option (3) (how does it differ from (4)/I don't understand (4)?).

There's also the point that there's not a ton of churn on this library, so maybe the duplication is OK?

Collaborator Author

seanpdoyle commented Mar 9, 2018 •

edited

Loading

@mike-burns for option 3, I was imagining some custom matcher, separate from the public API. Maybe something like:

create_schema("user", {
  # ...
}

json = { id: 1, name: "The User" }

expect(json).to pass_with_all_tools_against_schema("user")

Which could look something like:

RSpec::Matchers.define :pass_with_all_tools_against_schema do |schema_name|
  match do |response|
    expect(response).to match_response_schema(schema_name) &&
    assert_response_matches_schema(response, schema_name)
  end
  
  failure_message do | response|
    # ...
  end
end

Option 4 would very similar, but not behind an internal matcher. Keeping the assertions inside the test blocks, as separate expressions, without trying to synthesize a nice error message when one or the other fails.

I don't know what would go into having both matchers live side-by-side. I assume mixing minitest modules into RSpec describe blocks would do it, though I haven't tried.

If it's not possible, or not pleasant to work with, an additional option could be to create template files for duplicated stuff, swapping in the appropriate assertion calls. Templating out tests and running them feels a bit clever and over-engineered, and might be a barrier to contributions. 🤷‍♂️

Collaborator Author

seanpdoyle commented Mar 9, 2018 •

edited

Loading

There's also the point that there's not a ton of churn on this library, so maybe the duplication is OK?

I was considering this as well. A lot of the tests build their own schemata, which are mostly arbitrary, and there are several tests for covering passing in a request/controller tests' response, a Hash, and a String.

The suite, in its current state, could be consolidated, and factories could be introduced to hide away the noise. There'd still be concepts duplicated across both suites, but the tests themselves could be more terse.

mike-burns commented Mar 9, 2018

I think you've convinced me of:

The duplicated tests are not currently a problem.
We don't expect them to be a problem.
It'd be fun to refactor the test suite to make it easier to read.
We can tackle the duplication when it becomes a problem.

Do you agree? If so: LGTM!

seanpdoyle force-pushed the minitest-support branch 2 times, most recently from a3165de to 8f83c16 Compare

March 9, 2018 20:01

Collaborator Author

seanpdoyle commented Mar 9, 2018

@mike-burns I've updated the README, NEWS, and CONTRIBUTING documents, could you take a look?

mike-burns reviewed

View reviewed changes

mike-burns left a comment

Nice updates! Found one typo, otherwise LGTM.

README.md Outdated

 ```
-Define your [JSON Schema](http://json-schema.org/example1.html) in the schema directory:
+#### RSpec

mike-burns Mar 9, 2018

Minitest

seanpdoyle force-pushed the minitest-support branch from f0ce86e to de3caa6 Compare

March 16, 2018 14:18

seanpdoyle force-pushed the minitest-support branch from e6542fd to 5cc6320 Compare

April 6, 2018 18:06

seanpdoyle force-pushed the minitest-support branch from 3f0150f to 92a4dc3 Compare

April 6, 2018 19:37

houndci-bot reviewed

View reviewed changes

lib/json_matchers/assertion.rb

+ def invalid_failure_message
+ <<-FAIL
+#{last_error_message}

houndci-bot Apr 6, 2018

Layout/IndentHeredoc: Use 2 spaces for indentation in a heredoc by using <<~ instead of <<-.

lib/json_matchers/assertion.rb

+ def valid_failure_message
+ <<-FAIL
+#{last_error_message}

houndci-bot Apr 6, 2018

Layout/IndentHeredoc: Use 2 spaces for indentation in a heredoc by using <<~ instead of <<-.

seanpdoyle added 2 commits

April 6, 2018 17:22


Add support for Minitest

1bcceaf

Introduce `JsonMatchers::Minitest::Assertions` module, which provides
`assert_response_matches_schema` and `refute_response_matches_schema`
assertion methods.

Configure Hound to allow JSON-like hash declarations inside
`test/**/*_test.rb` files like `spec/**/*_spec.rb` files.

Add `rake test` to compliment `rake spec`. Rake's `default` task, which
is run in CI, will run both `rake test` and `rake spec`.

When including the Minitest assertions, the schema directory will
default to `test/support/api/schemas`.

This commit re-implements the RSpec matcher to re-use
`JsonMatchers::Assertion` instead of declaring `JsonMatchers::Rspec`.


Add Minitest documentation

d70ee97

Update the `README.md` to explain how to configure and use the Minitest
assertions.

Update `CONTRIBUTING.md` to explain that pull requests _must_ include
tests that cover both `Minitest` and `RSpec`.

seanpdoyle force-pushed the minitest-support branch from bcc27d9 to d70ee97 Compare

April 6, 2018 21:29

seanpdoyle merged commit d70ee97 into master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment