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

Test/schema coverage #297

Merged
merged 15 commits into from
Dec 23, 2020
Merged

Test/schema coverage #297

merged 15 commits into from
Dec 23, 2020

Conversation

datbth
Copy link
Contributor

@datbth datbth commented Dec 16, 2020

Track schema coverage in tests (see #296)

Other changes

@ota42y ota42y self-requested a review December 19, 2020 03:36
Copy link
Member

@ota42y ota42y left a comment

Choose a reason for hiding this comment

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

It's very good feature! 👍 👍 👍 👍 👍 👍 👍 👍 👍 👍
But I add few comments, please check it.

README.md Outdated
@@ -403,6 +403,38 @@ end

The default assertion option in 2.* was `validate_success_only=true`, but this becomes `validate_success_only=false` in 3.*. For the smoothest possible upgrade, you should set it to `false` in your test suite before upgrading to 3.*.

**Test schema coverage**
NOTE: Currently committee only supports schema coverage for openapi schemas, and only checks coverage on responses.
Copy link
Member

Choose a reason for hiding this comment

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

Now we support tests using 'assert_schema_conform' or 'assert_response_schema_confirm' only.
(i.e. the user who use committee middleware in test )

So please write NOTICE to these user

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added more details to the docs

application/json:
schema:
type: object
'404':
Copy link
Member

Choose a reason for hiding this comment

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

OpenAPI 3 support default. When status code wasn't matched defined, use default` definithion.
https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.3.md#responses-object-example

So please implement and test it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done via 0712eed

@datbth datbth requested a review from ota42y December 21, 2020 04:13
@datbth
Copy link
Contributor Author

datbth commented Dec 22, 2020

Just made this commit 32600b2 to fix committee accepting requests even when they don't have matching prefix (in tests)

Copy link
Member

@ota42y ota42y left a comment

Choose a reason for hiding this comment

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

OK, I think this is very great changes!!!!!! Thanks!!!!!!!

path = request.path
path = path.gsub(@prefix_regexp, '') if prefix_request?(request)
path = path.gsub(@prefix_regexp, '') if @prefix_regexp
Copy link
Member

Choose a reason for hiding this comment

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

nice 👍 👍 👍 👍

2. Use `assert_response_schema_confirm` or `assert_schema_conform`
3. Then use `SchemaCoverage#report` or `SchemaCoverage#report_flatten` to get coverage report

Example:
Copy link
Member

Choose a reason for hiding this comment

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

very nice!!!!!!!!!!!!!!!!! 👍 👍 👍 👍 👍 👍 👍 👍 👍

@ota42y ota42y merged commit 094ec94 into interagent:master Dec 23, 2020
@datbth
Copy link
Contributor Author

datbth commented Dec 24, 2020

hmm I have just realized I put the docs in the changes/migration section, should have put it in the upper big Test Assertion section instead 😿

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

Successfully merging this pull request may close these issues.

2 participants