Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Support stub_model(string) to return a temporary ActiveRecord double #607

Closed
avit opened this Issue Sep 11, 2012 · 12 comments

Comments

Projects
None yet
7 participants

avit commented Sep 11, 2012

mock_model("Dummy") will currently return a temporary ActiveModel double, but the object doesn't have very good fidelity with actual rails ActiveRecord::Base models since those define additional class methods, e.g. primary_key and base_class which are needed for id and type primary key attributes. (See issues #606, #435.)

stub_model should support a string argument for creating temporary ActiveRecord::Base doubles.

Would this make mock_model obsolete since the names are confusing too?

Contributor

patmaddox commented Sep 12, 2012

if you need primary_key and base_class on your double then you can define them...as you say, mock_model gives you an ActiveModel, which doesn't behave exactly like an ActiveRecord. If you want ActiveRecord then why not use mock_model with an actual ActiveRecord class, or pass in the extra methods that you want?

Owner

dchelimsky commented Sep 12, 2012

@patmaddox we already added support for primary_key (see #435), so @avit was asking us to also bend the rules for base_class.

@avit - @patmaddox is correct that you can pass an ActiveRecord subclass to mock_model, which might solve the problem for you.

avit commented Sep 12, 2012

@dchelimsky it's actually easier to use mock_model with a string and just stub the required base_class on its class. (At least in this case: I guess it could run into other shortcomings with ActiveModel vs. ActiveRecord::Base depending on how it's used.)

I showed an example of how I'm currently using it in #606. Your proposed alternative means creating an actual ActiveRecord subclass: then I might as well use the real thing. Is there any reason why stub_model should not build a dummy class from a string the same way mock_model does?

Contributor

alindeman commented Sep 12, 2012

What would stub_model mean if it built a dummy class? That'd be mock_model, right?

Is there too much tension between useful test doubles for use in Rails and keeping them looking close enough to ActiveRecord models to be useful (the interface for ActiveRecord is really wide and ever changing)?

Owner

dchelimsky commented Sep 13, 2012

What would stub_model mean if it built a dummy class? That'd be mock_model, right?

Sort of. The original distinction between mock_model and stub_model was that one is a double and the other is a real object that's not allowed to talk to a database. At one point we had an issue that led us in a direction in which mock_model would only support ActiveModel APIs so you could use it for non-ActiveRecord models. This meant: use stub_model for non-AR and use stub_model for AR.

The Rails team has been very good about maintaining decoupling between components by constraining ActionPack's interactions with models to ActiveModel APIs, but there is no parallel commitment for inter-model communication within ActiveRecord. This seems like a reasonable tradeoff, especially when you consider that the problems we're seeing here are related specifically to associations, which is an ActiveRecord concept AFAIK.

The benefit of adding support for a String to stub_model is that you could use it to generate an anonymous ActiveRecord::Base subclass (i.e. the class need not exist), but you'd still get the other benefits of stub_model.

We should probably re-think these names, however, since mock_model and stub_model don't really tell the whole story.

avit commented Jul 19, 2013

We should probably re-think these names, however, since mock_model and stub_model don't really tell the whole story.

So will it be double_model?

Owner

dchelimsky commented Jul 19, 2013

How about model_double? I see it as a noun, not a verb.

avit commented Jul 19, 2013

I was being a little facetious here. 😉 But actually, I think the freshly-deprecated stub method was a better name overall, despite the "mocks are not stubs" ambiguity: stub works fine either as a noun or verb. (To double something still implies arithmetic in my eyes.) We have a lot of our own stub_something helpers & FactoryGirl's build_stubbed in our code, so it'll take some time to get used to the mismatch or else rename everything to *_double...

Owner

myronmarston commented Jul 19, 2013

@avit: FWIW, there's a couple reasons I chose to deprecate stub:

  • stub was being used as a noun and a verb: stub() (noun) would return a test double and Object#stub would stub a method. The use of the same name to do two completely different things caused confusion.
  • In RSpec 3 we are going to have instance_double and class_double (more here), and the symmetry of double/instance_double/class_double is nice. I didn't want to also have instance_stub and class_stub (plus instance_mock and class_mock since mock was an alias for double as well).

As RSpec 3 is released, is this still an issue or could this be closed?

Owner

JonRowe commented Nov 17, 2013

Not quite, the functionality is different to what was requested, but it doesn't look like we reached a consensus on what we want to do with this.

Contributor

alindeman commented Mar 3, 2014

Closing this since we've deprecated/extracted stub_model/mock_model.

@alindeman alindeman closed this Mar 3, 2014

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