Add missing request specs for /api/v1/tags endpoint
#23146
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds a [JSON-Schema](https://json-schema.org) specification for [Tag](https://docs.joinmastodon.org/entities/Tag/) and the nested history object that I've called `TrendHistory`.
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.
renchap
added
testing
|
This pull request has merge conflicts that must be resolved before it can be merged. |
|
Closing in favor of #25439 which added similar coverage. If you want to rebase this and re-open with whatever additional improvements are in here, go for it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For discussion.
This relates to both #20572 and #22378 (comment). I was kicking the tires on swapping out JSON serializers to improve API responsiveness, and was somewhat surprised to discover that there were no API request specs. The API is tested in some controller specs, but those specs are not exhaustive. So, I decided to pick a small endpoint and completely cover the behavior of it.
Given the number of connected clients and the API contract, I believe it's important to have request specs as a source of truth: it proves behavior, documents how the API responds in various circumstances, and prevents accidental breakage (critical in the case of testing out different serialization strategies). This is the tip of the iceberg - I believe the entire API should be covered in these kids of specs.
This PR adds a JSON-Schema specification for Tag and the nested history object that I've called
TrendHistory, and validates that the API responses match the schema.This PR also introduces some failing tests worth additional discussion. When fleshing the specs out, I expected that the
followingflag in the JSON body would be associated with theread:followsscope, but that is apparently not the case. The/api/v1/tagsendpoint always returns hashtag following information whenever there is an authenticated user, regardless of what scope the user has. I have a separate PR to tackle this, but I'm not sure that my expectations are indeed the actual expected behavior or not. I did raise this issue privately, and was given the green light to publicly discuss the behavior of it.So, a few questions to help spark discussion:
followingflag? Should it move behind an oauth scope? If so, should it re-useread:follows, or should a new one be made, such asread:tag_follows? I do have a PR I can submit that omits the flag from the response unless the user hasread:followsscope granted, but I'm not sure that's desirable.