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

Updates shoulda gems #2665

Closed
wants to merge 3 commits into from
Closed

Updates shoulda gems #2665

wants to merge 3 commits into from

Conversation

jsmadis
Copy link
Contributor

@jsmadis jsmadis commented Oct 25, 2021

What this PR does / why we need it:

Updates shoulda gems to newer versions.

THREESCALE-7756: upgrade shoulda-matchers to fix BigDecimal deprecation

@jsmadis jsmadis force-pushed the rails-upgrade-shoulda-matchers branch from 2ce4c06 to 0ef1e95 Compare October 26, 2021 13:38
@jsmadis jsmadis changed the title [WIP] Updates shoulda gems Updates shoulda gems Oct 26, 2021
@jsmadis jsmadis self-assigned this Oct 26, 2021
@jsmadis jsmadis force-pushed the rails-upgrade-shoulda-matchers branch from 0ef1e95 to 803475d Compare October 26, 2021 13:56
Comment on lines +1 to +20
# frozen_string_literal: true

# TODO: remove when you see similar error but actual format is: "format"=>:json
# PARAMS_TO_SYMBOLIZE causes errors in the tests regarding the format.
# It always converts format to symbol which causes a failure in the tests:
# Stats::Api::ApplicationsController should route GET /stats/api/applications/42/usage.json
# --- expected
# +++ actual
# @@ -1 +1 @@
# -{"application_id"=>"42", "action"=>"usage", "format"=>:json, "controller"=>"stats/api/applications"}
# +{"controller"=>"stats/api/applications", "action"=>"usage", "application_id"=>"42", "format"=>"json"}
module Shoulda
module Matchers
module ActionController
class RouteParams
PARAMS_TO_SYMBOLIZE = [].freeze
end
end
end
end
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 am not sure how to fix it on the other side, somehow the tests fail because shoulda-matchers is symbolizing format values and it crashes our tests. Our controllers are returning format as a string then tests like https://github.com/3scale/porta/blob/master/test/functional/stats/api/applications_controller_test.rb#L7 fails.

I am not unable to debug it ( it will fail with other [IO] issues). There is no stack trace and also the test result is not helpful at all.

Example test result:

# Stats::Api::ApplicationsController should route GET /stats/api/applications/42/usage.json
# --- expected
# +++ actual
# @@ -1 +1 @@
# -{"application_id"=>"42", "action"=>"usage", "format"=>:json, "controller"=>"stats/api/applications"}
# +{"controller"=>"stats/api/applications", "action"=>"usage", "application_id"=>"42", "format"=>"json"}

Copy link
Contributor

Choose a reason for hiding this comment

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

See this issue comment
thoughtbot/shoulda-matchers#809 (comment)

Perhaps we need to fix in routes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't do this in our case. We are using one route for multiple formats and default won't work.
One of the tests that are failing because of this: https://github.com/3scale/porta/blob/master/test/functional/stats/api/services_controller_test.rb#L7

On the other hand, when the tests are checking the controller they are parsing the path and the path contains the format which is parsed as string, not a symbol.
https://github.com/rails/rails/blob/v5.1.7/actionpack/lib/action_dispatch/routing/route_set.rb#L864

@jsmadis jsmadis requested a review from a team October 26, 2021 14:28
@github-actions github-actions bot added the Stale label Nov 18, 2021
@jsmadis jsmadis closed this Nov 18, 2021
@3scale 3scale deleted a comment from github-actions bot Feb 16, 2022
@josemigallas josemigallas reopened this Feb 16, 2022
@github-actions github-actions bot added the Stale label Mar 3, 2022
@3scale 3scale deleted a comment from github-actions bot Mar 3, 2022
@josemigallas josemigallas removed the Stale label Mar 3, 2022
@github-actions github-actions bot added the Stale label Mar 18, 2022
@3scale 3scale deleted a comment from github-actions bot Mar 18, 2022
@josemigallas
Copy link
Contributor

This was done in #2621

@josemigallas josemigallas deleted the rails-upgrade-shoulda-matchers branch March 18, 2022 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants