Fix #permit so #on works and is properly tested

Why:

* When using #permit with the #on qualifier to assert that #permit was
  called on a subset of the params hash (selected using #require), the
  matcher would fail. The matcher didn't properly detect that #permit
  was called on the slice -- it thought it was called on the parent
  params object.
* It turns out this was a bug in Doublespeak. When #require is called,
  we take the subset of the params, which is also an
  ActionController::Parameters object like params, and we stub #permit
  on it at runtime. Unfortunately the Double object created here was
  never activated, so when #permit was called, this Double wasn't the
  one run. Instead, the Double on #permit within the
  ActionController::Parameters class (which is stubbed at the beginning)
  was the one that was run, and it's this object that recorded the call
  on #permit incorrectly.
* This bug slipped through because it turns out when #on was added it
  wasn't tested very well.

To satisfy the above:

* Modify Doublespeak so that when it is in activated mode, whenever
  new doubles are created, activate them immediately.
* Fix all of the tests for #permit around operating on a slice of the
  params hash so that they use the #on qualifier.
This commit is contained in:
Elliot Winkler 2015-09-28 16:48:14 -06:00
parent 8253d4b4c9
commit 0d43835b61
6 changed files with 66 additions and 25 deletions

View File

@ -3,7 +3,7 @@
### Backward-incompatible changes
* We've dropped support for Rails 3.x, Ruby 1.9.2, and Ruby 1.9.3, and RSpec 2.
All of these have been end-of-lifed. ([a4045a1], [b7fe87a])
All of these have been end-of-lifed. ([a4045a1], [b7fe87a], [32c0e62])
* The gem no longer detects the test framework you're using or mixes itself into
that framework automatically. [History][no-auto-integration-1] has
@ -214,6 +214,7 @@
[#755]: https://github.com/thoughtbot/shoulda-matchers/pull/755
[#752]: https://github.com/thoughtbot/shoulda-matchers/pull/752
[9d9dc4e]: https://github.com/thoughtbot/shoulda-matchers/commit/9d9dc4e6b9cf2c19df66a1b4ba432ad8d3e5dded
[32c0e62]: https://github.com/thoughtbot/shoulda-matchers/commit/32c0e62596b87e37a301f87bbe21cfcc77750552
### Bug fixes
@ -253,6 +254,8 @@
* Fix failure message for `validate_numericality_of` as it sometimes didn't
provide the reason for failure. ([#699])
* Fix `permit` + `on` as it did not work correctly. ([#771])
### Features
* Add `on` qualifier to `permit`. This allows you to make an assertion that
@ -296,6 +299,7 @@
[#694]: https://github.com/thoughtbot/shoulda-matchers/pull/694
[#692]: https://github.com/thoughtbot/shoulda-matchers/pull/692
[#699]: https://github.com/thoughtbot/shoulda-matchers/pull/699
[#771]: https://github.com/thoughtbot/shoulda-matchers/pull/771
# 2.8.0

View File

@ -12,6 +12,14 @@ module Shoulda
@implementation = implementation
@activated = false
@calls = []
if world.doubles_activated?
activate
end
end
def activated?
@activated
end
def to_return(value = nil, &block)

View File

@ -3,6 +3,10 @@ module Shoulda
module Doublespeak
# @private
class World
def initialize
@doubles_activated = false
end
def double_collection_for(klass)
double_collections_by_class[klass] ||=
DoubleCollection.new(self, klass)
@ -20,12 +24,18 @@ module Shoulda
end
def with_doubles_activated
@doubles_activated = true
activate
yield
ensure
@doubles_activated = false
deactivate
end
def doubles_activated?
@doubles_activated
end
private
def activate

View File

@ -1,4 +1,4 @@
require 'shoulda-matchers'
require 'shoulda/matchers/doublespeak'
PROJECT_ROOT = File.expand_path('../..', __FILE__)
$LOAD_PATH << File.join(PROJECT_ROOT, 'lib')

View File

@ -7,7 +7,7 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
params_with_conditional_require(ctrl.params).permit(:name, :age)
end
expect(controller).to permit_with_conditional_params(
expect(controller).to permit_with_conditional_slice_of_params(
permit(:name).for(:create)
)
end
@ -17,7 +17,7 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
params_with_conditional_require(ctrl.params).permit(:name, :age)
end
expect(controller).to permit_with_conditional_params(
expect(controller).to permit_with_conditional_slice_of_params(
permit(:name, :age).for(:create)
)
end
@ -27,7 +27,7 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
params_with_conditional_require(ctrl.params).permit(:name)
end
expect(controller).not_to permit_with_conditional_params(
expect(controller).not_to permit_with_conditional_slice_of_params(
permit(:name, :admin).for(:create)
)
end
@ -35,7 +35,7 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
it 'rejects when #permit has not been called' do
define_controller_with_strong_parameters(action: :create)
expect(controller).not_to permit_with_conditional_params(
expect(controller).not_to permit_with_conditional_slice_of_params(
permit(:name).for(:create)
)
end
@ -46,11 +46,6 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
[:phone_number, :address_1, :address_2, :city, :state, :zip]
]
params = {
order: { some: 'value' },
diner: { some: 'value' }
}
define_controller_with_strong_parameters(action: :create) do |ctrl|
params_with_conditional_require(ctrl.params, :order).
permit(sets_of_attributes[0])
@ -59,14 +54,16 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
permit(sets_of_attributes[1])
end
expect(controller).to permit_with_conditional_params(
expect(controller).to permit_with_conditional_slice_of_params(
permit(*sets_of_attributes[0]).for(:create),
params
all_params: [:order, :diner],
selected_param: :order
)
expect(controller).to permit_with_conditional_params(
expect(controller).to permit_with_conditional_slice_of_params(
permit(*sets_of_attributes[1]).for(:create),
params
all_params: [:order, :diner],
selected_param: :diner
)
end
end
@ -91,7 +88,7 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
context 'when operating on the entire params hash' do
include_context 'basic tests' do
def permit_with_conditional_params(permit, _params = {})
def permit_with_conditional_slice_of_params(permit, options = {})
permit
end
@ -103,9 +100,16 @@ describe Shoulda::Matchers::ActionController::PermitMatcher, type: :controller d
context 'when operating on a slice of the params hash' do
include_context 'basic tests' do
def permit_with_conditional_params(permit, params = nil)
params ||= { user: { some: 'value' } }
permit.add_params(params)
def permit_with_conditional_slice_of_params(
permit,
all_params: [:user],
selected_param: :user
)
params = all_params.reduce({}) do |hash, param|
hash.merge(param => { any: 'value' })
end
permit.add_params(params).on(selected_param)
end
def params_with_conditional_require(params, *filters)

View File

@ -2,6 +2,20 @@ require 'doublespeak_spec_helper'
module Shoulda::Matchers::Doublespeak
describe Double do
describe 'initializer' do
context 'if doubles are currently activated on the world level' do
it 'immediately activates the new Double' do
world = build_world(doubles_activated?: true)
klass = create_class(a_method_name: nil)
implementation = build_implementation
double = described_class.new(world, klass, :a_method_name, implementation)
expect(double).to be_activated
end
end
end
describe '#to_return' do
it 'tells its implementation to call the given block' do
sent_block = -> { }
@ -12,7 +26,7 @@ module Shoulda::Matchers::Doublespeak
actual_block = block
end
double = described_class.new(
:a_world,
build_world,
:klass,
:a_method,
implementation
@ -24,7 +38,7 @@ module Shoulda::Matchers::Doublespeak
it 'tells its implementation to return the given value' do
implementation = build_implementation
double = described_class.new(
:a_world,
build_world,
:klass,
:a_method,
implementation
@ -43,7 +57,7 @@ module Shoulda::Matchers::Doublespeak
actual_block = block
end
double = described_class.new(
:a_world,
build_world,
:klass,
:a_method,
implementation
@ -127,7 +141,7 @@ module Shoulda::Matchers::Doublespeak
klass = create_class(a_method: 42)
instance = klass.new
double = described_class.new(
:a_world,
build_world,
klass,
:a_method,
build_implementation
@ -141,7 +155,7 @@ module Shoulda::Matchers::Doublespeak
describe '#record_call' do
it 'adds the given call to the list of calls' do
double = described_class.new(
:a_world,
build_world,
:a_klass,
:a_method,
:an_implementation
@ -247,7 +261,8 @@ module Shoulda::Matchers::Doublespeak
def build_world(methods = {})
defaults = {
original_method_for: nil,
store_original_method_for: nil
store_original_method_for: nil,
doubles_activated?: nil
}
double('world', defaults.merge(methods))
end