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

Fixes bug in Test::Schema when using filter_parameters #2046

Merged
merged 2 commits into from Feb 16, 2017

Conversation

Projects
None yet
3 participants
@leonelgalan
Contributor

leonelgalan commented Feb 6, 2017

By default all Rails apps are created with an initializer called filter_parameter_logging.rb and its default contents are:

Rails.application.config.filter_parameters += [:password]

If this line is executed, as it is on this PR, the class of request.filtered_parameters changes from ActiveSupport::HashWithIndifferentAccess to Hash. This causes Schema::Test not to be able to find schemas by controller path and action:

     ActiveModelSerializers::Test::Schema::MissingSchema:
       No Schema file at spec/support/schemas//.json

Not properly "mocking" the Rails app properly caused the bug to be hidden in this and leonelgalan/rspec-active_model_serializers test suite.

This bug was first reported by me in another in #1947 (review), but at the time I couldn't produce failing tests.

@mention-bot

This comment has been minimized.

mention-bot commented Feb 6, 2017

@leonelgalan, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bf4 and @maurogeorge to be potential reviewers.

@@ -1,5 +1,7 @@
require 'test_helper'
Rails.application.config.filter_parameters += [:password]

This comment has been minimized.

@bf4

bf4 Feb 6, 2017

Member

@leonelgalan so, this is enough to make the test fail? (this should be set in the test app, I think)

This comment has been minimized.

@leonelgalan

leonelgalan Feb 6, 2017

Contributor

I moved into a configure block inside test/support/rails_app.rb

@bf4

failure case

@@ -6,6 +6,8 @@ module ActiveModelSerializers
config.active_support.test_order = :random
config.action_controller.perform_caching = true
config.action_controller.cache_store = :memory_store
config.filter_parameters += [:password]

This comment has been minimized.

@bf4

bf4 Feb 6, 2017

Member

do the tests fail with just this change, and not the fix in lib/active_model_serializers/test/schema.rb ?

This comment has been minimized.

@leonelgalan

leonelgalan Feb 6, 2017

Contributor

Yes, they will fail with the message above. I'm not sure how to change this setting for a single test to write a failure case.

@bf4 bf4 merged commit 81a9fbd into rails-api:master Feb 16, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment