The `delegate_method` matcher uses Doublespeak internally to do its job.
The delegate object is completely stubbed using an ObjectDouble,
available from Doublespeak, which is shoulda-matchers's own test double
library. ObjectDouble is a class that responds to every
method by implementing `method_missing`.
Say you are using Ruby 2.4 and you are testing a class that uses the
Forwardable module to do some delegation, and you are using
`delegate_method` in your test. When you run your test you will a
warning that looks something like:
Courier#deliver at ~/.rvm/rubies/ruby-2.4.0/lib/ruby/2.4.0/forwardable.rb:156 forwarding to private method #deliver
Why is this happening? When the code in your class gets exercised,
Forwardable will delegate the delegate method in question to
ObjectDouble, and the method will get intercepted by its
`method_missing` method. The fact that this is actually a private method
is what Forwardable is complaining about.
To fix this, all we need to do is add `respond_to_missing?` in addition
to `method_missing?`. This is explained here:
https://bugs.ruby-lang.org/issues/13326
Why:
* There were architectural issues with how the permit matcher kept track
of params instances on which doubles had been placed. Previously we
were starting off by taking the ActionController::Parameters class and
stubbing the #permit and #require instance method on it -- in other
words, we were stubbing #require for all instances of
ActionController::Parameters -- then we would stub #permit on a
particular instance of ActionController::Parameters that #require
returned. What this means is that if for some reason the #permit stub
on an individual instance isn't working properly, then the #permit
stub on ActionController::Parameters will respond to the invocation.
This is exactly what happened for the issue we recently fixed --
if the stubbing were done a different way we wouldn't have run into
that issue.
* Also, there's no reason to have both ParametersDoubles and
SliceOfParametersDoubles classes around. While it's nice that we have
a simpler option to use if we don't need the more complex one, we
actually don't need a distinction here, and we can afford one class
that does both.
To satisfy the above:
* When stubbing #permit or #require, always do so on an instance of
ActionController::Parameters and not the whole class. This way we know
exactly which methods are being doubled and it's easier to debug things
in the future.
* This means that we now stub ActionController::Parameters.new and then
place stubs on the returned instance.
* Refactor ParametersDoubles and SliceOfParametersDoubles: combine them
into a ParametersDoubleRegistry class, but extract the code that stubs
ActionController::Parameters.new into
a CompositeParametersDoubleRegistry class.
* Since this broke one of the tests, modify DoubleCollection so that a
method cannot be doubled more than once -- if the method is already
doubled then `register_stub` or `register_proxy` does nothing and
returns the original Double.
Why:
* When debugging #permit it's been helpful to loop through all of the
doubles that Doublespeak has registered and spit out all of the calls
on those doubles (specifically which arguments they've received).
* It's also been helpful to know where the methods the doubles represent
have been called.
To satisfy the above:
* Add #calls_by_method_name to DoubleCollection
* Add #caller to MethodCall
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.
If a method for a class is doubled more than once within the same test
run, the original implementation of that method will change from double
to double, even if the double is deactivated correctly. Say we have two
distinct tests that both double the same method. Here is how that method
will be overridden as it goes along:
A) START: original method
B) ACTIVATE (1st test): method is doubled
C) DEACTIVATE (1st test): calls original method (A)
D) ACTIVATE (2nd test): original method (C) stored; method is again
doubled
E) DEACTIVATE (2nd test): calls original method (C)
With this commit, this changes to:
A) START: original method
B) ACTIVATE (1st test): method is doubled
C) DEACTIVATE (1st test): calls original method (A)
D) ACTIVATE (2nd test): original method not stored again; method is
again doubled
E) DEACTIVATE (2nd test): calls original method (A)
MethodCall is used to represent a method call made on a double. It
stores the object the call was made on, the name of the method, any
arguments or block sent to the method, and the double itself. There were
quite a few places where we were using an (args, block) or (object,
args, block) tuple. We also already had a MethodCall class, and
additionally a MethodCallWithName class, and the two were basically the
same. These usages have all been collapsed.
The provided RSpec example looks a bit contrived, but it corresponds to
this perfectly reasonable Minitest test case that was not working:
class UserControllerTest < ActionController::TestCase
should permit(:name).for(:create)
should_not permit(:admin).for(:create)
end
This instantiates two matchers at class load time, which resulted in the
second one overriding the double collection of the first one. Then when
the tests are executed, the first matcher would fail, because its double
is not listening to ActionController::Parameters#permit -- only the
second matcher's double collection gets activated.
This is fixed in Doublespeak by only creating one double collection per
class.
This provides a robust solution for temporarily stubbing (and
unstubbing) methods. It will be internally by the strong parameters and
delegation matchers.
Elliot Winkler