Extract stub_model/mock_model to a gem #894

Closed
alindeman opened this Issue Dec 27, 2013 · 12 comments

Comments

Projects
None yet
4 participants
@alindeman
Contributor

alindeman commented Dec 27, 2013

I feel like using stub_model and mock_model are rarely a good idea. They are also really difficult to maintain since ActiveRecord seems to change in ways that break them nearly every time (currently they are broken on Rails 4.1-beta, which I'll address separately).

In the interest of discouraging their use, I propose extracting them to a gem in RSpec 3.0.0.

@thomas-holmes

This comment has been minimized.

Show comment
Hide comment
@thomas-holmes

thomas-holmes Dec 29, 2013

Member

Have you done any work on this yet? I've started tinkering with it. I'll reference something here when I've got it pushed.

Member

thomas-holmes commented Dec 29, 2013

Have you done any work on this yet? I've started tinkering with it. I'll reference something here when I've got it pushed.

thomas-holmes pushed a commit to thomas-holmes/rspec-rails that referenced this issue Dec 29, 2013

Thomas Holmes
Remove mock_model and stub_model from rspec-rails
Some tests still fail without them as I have not yet figured out
a good way to fix them yet. View scaffold generators are also
currently broken.

rspec/rspec-rails#894

thomas-holmes pushed a commit to thomas-holmes/rspec-activemodel-mocks that referenced this issue Dec 29, 2013

Thomas Holmes
A new gem that captures mock_model and stub_model.
This is a first pass extraction of the mock_model and
stub_model code. It doesn't have a lot of the supporting
configuration in it that other rspec projects currently
have and has not yet been stripped down to remove things
that are not necessary. I'm not happy with the need to
replicate the generation of a sample rails project but
it was the quickest way to get this off the ground for
now.

For rspec/rspec-rails#894
@alindeman

This comment has been minimized.

Show comment
Hide comment
@alindeman

alindeman Dec 29, 2013

Contributor

Haven't started yet. Looking forward to it, thanks :)

Contributor

alindeman commented Dec 29, 2013

Haven't started yet. Looking forward to it, thanks :)

@thomas-holmes

This comment has been minimized.

Show comment
Hide comment
@thomas-holmes

thomas-holmes Dec 29, 2013

Member

I've pushed up the first set of changes. Still plenty to do. I should be getting more done this evening.

Member

thomas-holmes commented Dec 29, 2013

I've pushed up the first set of changes. Still plenty to do. I should be getting more done this evening.

@thomas-holmes

This comment has been minimized.

Show comment
Hide comment
@thomas-holmes

thomas-holmes Dec 31, 2013

Member

I think this is at a pretty decent spot right now if you can find some time to review it. I don't want to do too much more without some feedback as it might be mostly unnecessary.
Modified rspec-rails:
https://github.com/thomas-holmes/rspec-rails/tree/remove-mocks

New gem repo:
https://github.com/thomas-holmes/rspec-activemodel-mocks

(Fixed the new gem repo link, I renamed it)

Member

thomas-holmes commented Dec 31, 2013

I think this is at a pretty decent spot right now if you can find some time to review it. I don't want to do too much more without some feedback as it might be mostly unnecessary.
Modified rspec-rails:
https://github.com/thomas-holmes/rspec-rails/tree/remove-mocks

New gem repo:
https://github.com/thomas-holmes/rspec-activemodel-mocks

(Fixed the new gem repo link, I renamed it)

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Dec 31, 2013

Member

Does your new gem actually depend on rails? Or simply activerecord? Or maybe even just activemodel?

If it's one of the latter, it would be good to name the gem rspec-activerecord-mocks or rspec-activemodel-mocks.

Member

myronmarston commented Dec 31, 2013

Does your new gem actually depend on rails? Or simply activerecord? Or maybe even just activemodel?

If it's one of the latter, it would be good to name the gem rspec-activerecord-mocks or rspec-activemodel-mocks.

@thomas-holmes

This comment has been minimized.

Show comment
Hide comment
@thomas-holmes

thomas-holmes Dec 31, 2013

Member

Thanks for the suggestion. I'm not completely certain, I'll need to reread through the actual mocking code. At a quick glance it looks like it relies on ActiveRecord and ActiveSupport.

I'll rename it if that's the case once I've verified.

Member

thomas-holmes commented Dec 31, 2013

Thanks for the suggestion. I'm not completely certain, I'll need to reread through the actual mocking code. At a quick glance it looks like it relies on ActiveRecord and ActiveSupport.

I'll rename it if that's the case once I've verified.

@thomas-holmes

This comment has been minimized.

Show comment
Hide comment
@thomas-holmes

thomas-holmes Jan 2, 2014

Member

A lot of its tests will have to be rewritten (they were being run inside the context of a rails app) but I don't see any aspect of the functionality the gem itself provides that requires a full rails stack. I'll start modifying the gem/tests to remove its dependency on Rails.

Member

thomas-holmes commented Jan 2, 2014

A lot of its tests will have to be rewritten (they were being run inside the context of a rails app) but I don't see any aspect of the functionality the gem itself provides that requires a full rails stack. I'll start modifying the gem/tests to remove its dependency on Rails.

@thomas-holmes

This comment has been minimized.

Show comment
Hide comment
@thomas-holmes

thomas-holmes Jan 3, 2014

Member

I've got the gem now pared down to only having explicit runtime dependencies on ActiveModel and ActiveSupport. I've also renamed the repo and the gem to rspec-activemodel-mocks.

I'm not really happy with the testing situation, it feels pretty hackish but at least it's working. I have some ideas for improving the tests in a way that a database might not be required but I'm not sure yet if that will pan out.

https://github.com/thomas-holmes/rspec-activemodel-mocks

Member

thomas-holmes commented Jan 3, 2014

I've got the gem now pared down to only having explicit runtime dependencies on ActiveModel and ActiveSupport. I've also renamed the repo and the gem to rspec-activemodel-mocks.

I'm not really happy with the testing situation, it feels pretty hackish but at least it's working. I have some ideas for improving the tests in a way that a database might not be required but I'm not sure yet if that will pan out.

https://github.com/thomas-holmes/rspec-activemodel-mocks

thomas-holmes pushed a commit to thomas-holmes/rspec-rails that referenced this issue Jan 5, 2014

alindeman added a commit that referenced this issue Jan 5, 2014

Remove mock_model and stub_model from rspec-rails
Some tests still fail without them as I have not yet figured out
a good way to fix them yet. View scaffold generators are also
currently broken.

rspec/rspec-rails#894

thomas-holmes added a commit to thomas-holmes/rspec-rails that referenced this issue Jan 16, 2014

Remove mock_model and stub_model from rspec-rails
Some tests still fail without them as I have not yet figured out
a good way to fix them yet. View scaffold generators are also
currently broken.

rspec/rspec-rails#894

thomas-holmes added a commit to thomas-holmes/rspec-rails that referenced this issue Jan 16, 2014

Remove mock_model and stub_model from rspec-rails
Some tests still fail without them as I have not yet figured out
a good way to fix them yet. View scaffold generators are also
currently broken.

rspec/rspec-rails#894

thomas-holmes added a commit to thomas-holmes/rspec-rails that referenced this issue Jan 16, 2014

thomas-holmes added a commit to thomas-holmes/rspec-rails that referenced this issue Feb 9, 2014

thomas-holmes added a commit to thomas-holmes/rspec-rails that referenced this issue Feb 9, 2014

thomas-holmes added a commit to thomas-holmes/rspec-rails that referenced this issue Feb 10, 2014

@jezstephens

This comment has been minimized.

Show comment
Hide comment
@jezstephens

jezstephens Feb 18, 2014

Hi guys. In what sense is using these features "discouraged"? If the goal is to discourage people from adding the new gem en masse, I think some guidance is necessary. It is not clear to me what is considered an appropriate use case for these features.

Though I use plain doubles or OpenStructs where I can, I often find myself using mock_model to avoid raising ActiveRecord::AssociationTypeMismatch when assigning to an association. I am not aware of a simple alternative but I would be very happy to hear one. This must be a very common use case.

Perhaps something can be added to the upgrade guide?

Hi guys. In what sense is using these features "discouraged"? If the goal is to discourage people from adding the new gem en masse, I think some guidance is necessary. It is not clear to me what is considered an appropriate use case for these features.

Though I use plain doubles or OpenStructs where I can, I often find myself using mock_model to avoid raising ActiveRecord::AssociationTypeMismatch when assigning to an association. I am not aware of a simple alternative but I would be very happy to hear one. This must be a very common use case.

Perhaps something can be added to the upgrade guide?

@thomas-holmes

This comment has been minimized.

Show comment
Hide comment
@thomas-holmes

thomas-holmes Feb 18, 2014

Member

I agree, some more guidance would be good. The idea is that doubles can often fill the role during isolated unit tests and real instances should work fine in larger integration style tests. If you are aware of any other situations in which you feel their use is necessary that would be great information to have.

Member

thomas-holmes commented Feb 18, 2014

I agree, some more guidance would be good. The idea is that doubles can often fill the role during isolated unit tests and real instances should work fine in larger integration style tests. If you are aware of any other situations in which you feel their use is necessary that would be great information to have.

@jezstephens

This comment has been minimized.

Show comment
Hide comment
@jezstephens

jezstephens Feb 18, 2014

The problem as I see it is that Rails makes isolated testing a bit difficult when it comes to models and controllers because they are so tied up with ActiveRecord and ActionController. Sometimes we have to compromise, and mock_model can make this a lot easier.

I hate to throw a spanner in the works, but I wonder if it might be better to deprecate only stub_model. I have not personally found a use for that one. Any thoughts?

The problem as I see it is that Rails makes isolated testing a bit difficult when it comes to models and controllers because they are so tied up with ActiveRecord and ActionController. Sometimes we have to compromise, and mock_model can make this a lot easier.

I hate to throw a spanner in the works, but I wonder if it might be better to deprecate only stub_model. I have not personally found a use for that one. Any thoughts?

alindeman added a commit that referenced this issue Mar 3, 2014

Remove mock_model and stub_model from rspec-rails
Some tests still fail without them as I have not yet figured out
a good way to fix them yet. View scaffold generators are also
currently broken.

rspec/rspec-rails#894
@alindeman

This comment has been minimized.

Show comment
Hide comment
@alindeman

alindeman Mar 3, 2014

Contributor

Fixed with #913 and #902

Contributor

alindeman commented Mar 3, 2014

Fixed with #913 and #902

@alindeman alindeman closed this Mar 3, 2014

@yujinakayama yujinakayama referenced this issue in yujinakayama/transpec Apr 10, 2014

Closed

Review supported conversions for RSpec changes #58

@awead awead referenced this issue in samvera/sufia Jun 17, 2014

Merged

Issue fixes #526

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment