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

Enable partial doubles verification for RSpec #3005

Merged

Conversation

cedum
Copy link
Contributor

@cedum cedum commented Dec 18, 2018

Enables the partial double verification for RSpec in Solidus Core and:

  • removes test cases that don't have an actual implementation (were passing because the implementation was mocked);
  • fixes test cases that have a refactored implementation (renamed method names for example);
  • disables partial doubles verification when testing runtime defined methods or user defined hooks (cannot find a more elegant solution to this, see Stub controller helper method on rspec rails 3 rspec/rspec-rails#1076. Any suggestions are welcome).

Being new to Solidus, it will be awesome if someone with more knowledge of the codebase could take a look at the changeset and ensure I didn't removed/changed too much 😄

RSpec 3 introduced partial double verification that checks arguments and
methods existence also for partial doubles.
This prevents having a double and its real object out of sync, which
usually happens when refactoring existing code and the tests continue to
pass, even if the refactored method or attribute has been renamed or
removed.
Its implementation has been refactored here:
solidusio#1033
First removed example:
`Spree::StockLocation` doesn't implement `#decrease_stock_variant`. It
was successful because the order object used in the example didn't
contain any associated shipments, hence it never checked the
expectation.

Second removed example:
`Spree::InventoryUnit` doesn't implement `.sell_units`. It was
successful because it was checking that the method isn't called when
finalizing the order.
The order object used for the `Spree::Order#finalize!` specs didn't
contain shipment objects.
This changes the initial order factory to ensure it covers all the test
cases.
Actually, `Spree::OrderUpdater#update_shipments` doesn't accept any
arguments.
Allows mocking user provided custom line item comparison hooks.
Adds `partial_double_verification` RSpec meta-tag, that, when set to
false, disables the partial double verification for the example.
Useful when testing unimplemented mocks like user-defined hooks.
Upgrades `RSpec::ActiveModel::Mocks` to version `~>1.1` which includes
the support for `inversed_from` when assigning an association for a mock.
See: rspec/rspec-activemodel-mocks#32
Disables partial double verification when testing mocked external
user-defined methods like hooks, for example.
By enabling `verify_partial_doubles` for RSpec, this test example was
failing because the `TestSorter#new` implementation was not accepting
any arguments. Fixed, by mocking a custom `#initialize` method.
Since `verify_partial_doubles` has been set to true for the entire
project, state machine defined dynamically methods aren't supported.
This disables the partial doubles check, for test cases dependent on the
state machine methods.
Reuse an already initialized Promotion object, instead of creating a new
one.
By enabling the `verify_partial_doubles` in RSpec, it's not allowed to
stub unimplemented methods.
In these cases `Spree::Promotion#rules` was stubbed to return a plain
`Array` object, instead it should return an
`ActiveRecord::Associations::CollectionProxy` object.
It was stubbed to return a plain `Array`, instead it must return a
`ActiveRecord::Associations::CollectionProxy` object.
By enabling `verify_partial_doubles` in RSpec, dynamic defined methods
by State Machine are not supported.
This disables the partial double verification for these cases.
Disables partial double verification, since it stubs unimplemented
methods.
Fixes the expectation for `#cancel` call check example.
By enabling `verify_partial_doubles`, when stubbing controller helpers,
the partial double check must be disabled, see: rspec/rspec-rails#1076
@cedum cedum force-pushed the cedum/enable_verify_partial_doubles branch from e39f16d to 11e3ca4 Compare December 18, 2018 17:29
Copy link
Contributor

@jacobherrington jacobherrington left a comment

Choose a reason for hiding this comment

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

This makes sense to me. I don't see any reason to not use this feature. Thanks! ❤️

Copy link
Contributor

@ericsaupe ericsaupe left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@jacobherrington jacobherrington merged commit 6ff96dc into solidusio:master Dec 19, 2018
@aitbw aitbw deleted the cedum/enable_verify_partial_doubles branch January 9, 2019 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants