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

Validate nodeinfo response by schema #21395

Merged
merged 13 commits into from Dec 15, 2022

Conversation

MFTabriz
Copy link
Contributor

@MFTabriz MFTabriz commented Nov 22, 2022

using a new matcher match_json_schema which uses json-schema gem. The schema file is also validated.

@ineffyble ineffyble added the refactoring Improving code quality label Nov 24, 2022
Copy link
Member

@ineffyble ineffyble left a comment

Choose a reason for hiding this comment

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

Can you address the build failures? Looks like you need to regenerate the Gemfile.lock

@MFTabriz MFTabriz force-pushed the validate_nodeinfo_schema branch 2 times, most recently from 49d536e to f31aae7 Compare November 24, 2022 15:25
@MFTabriz
Copy link
Contributor Author

MFTabriz commented Nov 24, 2022

Can you address the build failures? Looks like you need to regenerate the Gemfile.lock

I did a bundle lock --update. Now the tests pass for ruby2.7 but fail for ruby3.0. Did I break something? Can you please check this?

I guess I found it! bundle lock with no argument!

PS: Done!

@MFTabriz MFTabriz marked this pull request as ready for review November 24, 2022 15:42
@MFTabriz MFTabriz changed the title Validate nodeinfo schema Validate nodeinfo response schema Nov 24, 2022
@MFTabriz MFTabriz changed the title Validate nodeinfo response schema Validate nodeinfo response by schema Nov 24, 2022
@MFTabriz MFTabriz force-pushed the validate_nodeinfo_schema branch 2 times, most recently from e4ac3ac to e78d63a Compare November 24, 2022 21:00
@MFTabriz MFTabriz marked this pull request as draft November 24, 2022 21:41
@MFTabriz MFTabriz marked this pull request as ready for review November 24, 2022 21:50
@MFTabriz MFTabriz force-pushed the validate_nodeinfo_schema branch 3 times, most recently from 9371244 to 9e8d9ae Compare November 26, 2022 11:52
@MFTabriz
Copy link
Contributor Author

MFTabriz commented Dec 1, 2022

Hi @ClearlyClaire ! Can you please check this out or ask someone to review it?

@MFTabriz MFTabriz marked this pull request as draft December 1, 2022 10:31
@MFTabriz MFTabriz marked this pull request as ready for review December 1, 2022 10:45
@MFTabriz MFTabriz force-pushed the validate_nodeinfo_schema branch 2 times, most recently from ee97f56 to 37c5232 Compare December 5, 2022 14:51
@MFTabriz MFTabriz requested review from ClearlyClaire and removed request for ineffyble December 5, 2022 15:57
@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2022

This pull request has merge conflicts that must be resolved before it can be merged.

Copy link
Contributor

@ClearlyClaire ClearlyClaire left a comment

Choose a reason for hiding this comment

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

It looks like there's a bunch of unrelated dependency updates (I made in-line comments for some of them but not all). Can you revert those changes?

Gemfile.lock Outdated Show resolved Hide resolved
Gemfile.lock Outdated Show resolved Hide resolved
Gemfile.lock Outdated Show resolved Hide resolved
Gemfile.lock Outdated Show resolved Hide resolved
Gemfile.lock Outdated Show resolved Hide resolved
Gemfile.lock Outdated Show resolved Hide resolved
Gemfile.lock Outdated Show resolved Hide resolved
@MFTabriz
Copy link
Contributor Author

It looks like there's a bunch of unrelated dependency updates (I made in-line comments for some of them but not all). Can you revert those changes?

Thanks for checking it. I think I fixed all of them.

@MFTabriz
Copy link
Contributor Author

MFTabriz commented Dec 13, 2022

@ineffyble you asked for changes when this PR was a draft 3 weeks ago and you did not review it afterwards. I have to ask you again: Can you please check this out?

@ineffyble
Copy link
Member

@MFTabriz Claire is the one you need sign-off from, I've just been trying to help with PR hygiene :)

@MFTabriz
Copy link
Contributor Author

@MFTabriz Claire is the one you need sign-off from, I've just been trying to help with PR hygiene :)

Thank you @ineffyble! With your change request still open, in the list this PR was being shown as “Changes requested”. Now it's marked as "Approved" and the maintainers can easily see that everything has been taken care of.

@Gargron Gargron merged commit 6cdbc34 into mastodon:main Dec 15, 2022
neatchee pushed a commit to neatchee/mastodon that referenced this pull request Dec 16, 2022
* add json-schema to :test in Gemfile

* Create node_info_2.0_schema.json

* test match_response_schema

* Create match_response_schema.rb

* Update nodeinfo_controller_spec.rb

* Rename spec/support/node_info_2.0_schema.json to spec/support/schema/node_info_2.0_schema.json

* Update match_response_schema.rb

* cleanup

* additionally validate the json schema itself

disable throwing errors

test the schema matcher

* rename nodeinfo schema to nodeinfo_2.0

* use Rails.root.join to construct the path

* prettify json

* sync Gemfile.lock
@MFTabriz MFTabriz deleted the validate_nodeinfo_schema branch December 17, 2022 04:09
nametoolong pushed a commit to nametoolong/nuage that referenced this pull request Jan 12, 2023
* add json-schema to :test in Gemfile

* Create node_info_2.0_schema.json

* test match_response_schema

* Create match_response_schema.rb

* Update nodeinfo_controller_spec.rb

* Rename spec/support/node_info_2.0_schema.json to spec/support/schema/node_info_2.0_schema.json

* Update match_response_schema.rb

* cleanup

* additionally validate the json schema itself

disable throwing errors

test the schema matcher

* rename nodeinfo schema to nodeinfo_2.0

* use Rails.root.join to construct the path

* prettify json

* sync Gemfile.lock
darronschall added a commit to darronschall/mastodon that referenced this pull request Jan 18, 2023
This aims to be a complete specification and source of truth for how the endpoint behaves. It specifies the responses in both happy and sad paths, error handling, and authentication and authorization handling.

Use JSON-schema definitions to validate the API responses via `match_json_schema` added in mastodon#21395.

There is some repeated code and abstractions here. The next step is to extract [RSpec shared_examples](https://relishapp.com/rspec/rspec-core/docs/example-groups/shared-examples) to DRY. Another option to consider is moving towards the [`rswag` gem](https://github.com/rswag/rswag) per issue mastodon#20572.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Improving code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants