Modifies mime-registration test not to interfere with real mime types #25123
Conversation
The tests introduced in https://github.com/rails/rails/pull/23816/files#diff-384a5a15d8d53de799fb6541688ea5f9R153 register the JSON API media type `application/vnd.api+json` with `Mime[:json]`. The JSON API media type should not be registered with `Mime[:json]`, as discussed in rails#23712. Moreover, since the actual mime type used in the test is incidental, I've changed this to a valid, but fictional `applcation/vnd.rails+json`. These tests were causing failures in rails#25050 (comment) where `Mime[:jsonapi]` is being added, so that JSON API request params are parsed with the JSONAPI gem.
|
r? @matthewd (@rails-bot has picked a reviewer for you, use r? to override) |
bf4
changed the title
|
Those two tests should probably be removed altogether, as unregistering and then re-registering the JSON MIME type will cause that test to fail (if the former are run before the latter). Or the latter should be modified not to take the order into account. |
|
We should modify the latter to not take into account the order of the mime types. |
|
@rafaelfranca any blocker to merging this? |
|
@rafaelfranca any chance this will be in 5.0.0? |
|
@bf4 please don't make these requests on PRs. It hasn't been merged, so you know the answer to your question already. Thanks! ❤️ |
|
Hard to know how to manage stale complete prs in Rails land. Maintainers burden is so great, I dont know if things are forgotten or on hold. Is a reasonable question in smaller projects, I think. Thanks @kaspth for your conment ❤️ |
|
Yeah, I hear you. Though rest assured, @rafaelfranca does not simply forget a PR 😄 Usually it's tough for us to communicate the PR state because then we'd have to comment on a lot of PRs, which is more work. Thanks for your understanding, Benjamin. I and the rest of the team appreciate that ❤️ |
|
Backported in b1115d3 |
Modifies mime-registration test not to interfere with real mime types
The tests introduced in
https://github.com/rails/rails/pull/23816/files#diff-384a5a15d8d53de799fb6541688ea5f9R153
register the JSON API media type
application/vnd.api+jsonwithMime[:json].The JSON API media type should not be registered with
Mime[:json], as discussed in #23712.Moreover, since the actual mime type used in the test is incidental, I've changed this to a valid, but fictional
applcation/vnd.rails+json.These tests were causing failures in #25050 (comment) where
Mime[:jsonapi]is being added, so that JSON API request params are parsed with the JSONAPI gem.