Rails Testing with Factory Girl • Hacker News Board

Re: Rails Testing with Factory Girl

#31

Post by sanderjd » Wed, Aug 22, 2012, 9:25 PM UTC

mattgreenrocks wrote: post #21

sanderjd wrote: post #18

Earlier quoted context omitted.

As an FYI to your last point - in Rails master (which will be Rails 4), you can active-record-ify a class by including ActiveRecord::Model, rather than inheriting from ActiveRecord::Base. In fact, that is all ActiveRecord::Base does itself - https://github.com/rails/rails/blob/master/activerecord/lib/... .

But your core domain classes are still coupled to their persistence mechanism in the name of 'pragmatism.'

Who says they have to be? That's a design decision. There is literally nothing (besides maybe inertia?) that enforces that "rule".

Re: Rails Testing with Factory Girl

#32

Post by davidcuddeback » Wed, Aug 22, 2012, 9:47 PM UTC

chrislaco wrote: post #28

davidcuddeback wrote: post #27

Earlier quoted context omitted.

I would argue that those aren't unit tests, because they depend on the collaborators that you're passing into the unit under test. A change to those objects could break your unit test. That makes it an integration test. If you're using your unit tests to inform your design, then the complexity in the extraneous objects is relevant, because they tell you how coupled your unit under test is to its dependencies. This is…

And I would argue that arguing bout what a unit test is and isn't has missed the topic of this parent post entirely. I have seen those links and I know both people as they worked where I work now. Arguing over speed and/or what a pure unit test is has never been my cup of tea because every single codebase is different for a variety of reasons. Tests and code don't always fall into these neat cult buckets we've drawn…

> And I would argue that arguing bout what a unit test is and isn't has missed the topic of this parent post entirely.

You do raise a valid point: we're arguing about a definition. I don't think that misses the point though, because this thread is specifically discussing the use of FactoryGirl in unit tests. Where I disagree with you is on the importance of definitions. I think it's important to understand the definition of a unit test vs an integration test vs a system test, because without agreeing on those definitions, we don't stand a chance of having a reasonable discussion. If we have different definitions for a word, then our arguments will be talking past each other.

> Tests and code don't always fall into these neat cult buckets we've drawn up for ourselves.

(I'll ignore the loaded language, because that's not conducive to rational discussion.) From this point on, you use the word "tests" instead of "unit tests," and your points are valid because you're talking about the broader category of tests. I'm not. I'm specifically discussing unit tests, which has been the topic of discussion for this entire thread, going all the way up to the top-level comment by indrekju.

> because every single codebase is different for a variety of reasons

I agree with you here. Every codebase is different, and so are their testing requirements. For small gems, I tend to write all integration tests with few unit tests. For larger Rails projects, I write mostly unit tests with some integration tests to catch the cases where an interface changed but I forgot to update a fake object. If you're saying that different codebases differ in their relative requirements of different types of tests (unit vs integration, etc), then I agree. In fact, I would take that a step further, and say the testing requirements also depend on the team. But if you're arguing that the definition of a unit test changes depending on the codebase's testing requirements, then I can't agree.

I'm not refuting the usefulness of FactoryGirl in integration, system, or acceptance tests. Just unit tests.

Having definitions for jargon gives us tools to learn and discuss things at a more abstract level. That doesn't necessitate applying those ideas dogmatically. If anything, I think it helps you understand the trade-offs so that you can be pragmatic, because pragmatism depends on understanding.

Re: Rails Testing with Factory Girl

#33

Post by davidcuddeback » Wed, Aug 22, 2012, 10:46 PM UTC

samd wrote: post #11

urbanautomaton wrote: post #8

Earlier quoted context omitted.

If I could upvote the parent any harder, it would hit orbit. If you need a complex network of collaborators to test a method, the answer is not to obscure the creation of those collaborators, it's to refactor so that each object talks only to its immediate neighbours, and then stub the heck out of them. If it absolutely has to be something that acts like an ActiveModel, then use mock_model().

And I'd upvote it to Mars. FactoryGirl is completely an anti-pattern, unless maybe you're doing acceptance testing, but you know even then I wouldn't want FactoryGirl in my codebase, people would be too damn tempted to use it everywhere.

If you want to be sure to keep FactoryGirl out of unit tests but still have the benefit of using it in higher-level tests, you can use a different test/spec helper. I tend to do "spec_helper_lite", which sets up the bare minimum for unit tests (no FactoryGirl), and "spec_helper", which includes FactoryGirl for higher-level tests.

I say "higher-level" tests to refer to integration, system, and acceptance tests, because I don't have an opinion of where to draw the line for FactoryGirl. Is there a reason you would consider FactoryGirl for acceptance tests, but not integration or system tests? Just curious, because I might learn something.

Re: Rails Testing with Factory Girl

#34

Post by jeremyjh » Thu, Aug 23, 2012, 3:18 AM UTC

chrislaco wrote: post #29

urbanautomaton wrote: post #10

Earlier quoted context omitted.

Remember to ensure that your build strategy is set on all associations if you want this to work, e.g. FactoryGirl.define do factory :user do association :account, :strategy => :build end end Otherwise when you build a user, the account association will still get persisted to the database. Even if you do this, though, you're still spinning up a whole network of real objects in what is supposed to be a unit test.

I'll go one recommendation further. Just don't couple user and account. When making a factory named after a model, only set model attributes and never set relations. factory :user # sets only user fields, no relations factory :account # sets only account fields, no relations factory :user_account {creates both} When you have a large # of tests/factories, it's nearly impossible to keep track over time what the :user f…

That sounds like pretty good advice. I do not have a very large base of factories and tests yet but I can see how unwieldy it could get without really sane naming conventions.

Re: Rails Testing with Factory Girl

#35

Post by urbanautomaton » Thu, Aug 23, 2012, 9:59 AM UTC

chrislaco wrote: post #29

urbanautomaton wrote: post #10

Earlier quoted context omitted.

Remember to ensure that your build strategy is set on all associations if you want this to work, e.g. FactoryGirl.define do factory :user do association :account, :strategy => :build end end Otherwise when you build a user, the account association will still get persisted to the database. Even if you do this, though, you're still spinning up a whole network of real objects in what is supposed to be a unit test.

I'll go one recommendation further. Just don't couple user and account. When making a factory named after a model, only set model attributes and never set relations. factory :user # sets only user fields, no relations factory :account # sets only account fields, no relations factory :user_account {creates both} When you have a large # of tests/factories, it's nearly impossible to keep track over time what the :user f…

My preference is to have the base factories set everything that is needed for the generated record to be valid, and nothing more. If your User model validates the presence of the account, then the account association should be set - if not, it shouldn't. Otherwise, FactoryGirl.create(:user) is going to provide you with an unsaved, invalid record, which would certainly be surprising to me if I were using that factory for the first time. If all you want is a consistent set of initial attributes, then define a constant USER_ATTRIBUTES somewhere with a hash of values, and pass that to #new - using factories for this purpose is a bit sledgehammer/walnut.

I agree that excessive association building can get out of hand (and the latest version of FG even allows you to subscribe to factory events just so you can work out what the hell your test suite is building - a sure sign that you've lost the ability to reason about your tests if ever there was one), but completely abstaining from building associations seems to remove more or less all of the attraction of using factories as a fixture replacement in the first place.

All of the above applies only to integration/acceptance tests, though, where factories-as-fixture-replacements are at least arguably justifiable; my recommendation for unit tests is "don't use factories at all."

Re: Rails Testing with Factory Girl

#36

Post by samd » Thu, Aug 23, 2012, 2:31 PM UTC

davidcuddeback wrote: post #33

samd wrote: post #11

Earlier quoted context omitted.

And I'd upvote it to Mars. FactoryGirl is completely an anti-pattern, unless maybe you're doing acceptance testing, but you know even then I wouldn't want FactoryGirl in my codebase, people would be too damn tempted to use it everywhere.

If you want to be sure to keep FactoryGirl out of unit tests but still have the benefit of using it in higher-level tests, you can use a different test/spec helper. I tend to do "spec_helper_lite", which sets up the bare minimum for unit tests (no FactoryGirl), and "spec_helper", which includes FactoryGirl for higher-level tests. I say "higher-level" tests to refer to integration, system, and acceptance tests, becaus…

Integration tests just mean you are testing the integration of multiple components, say view and controller, model and database, controller and model. Acceptance tests are tests from the end-user's perspective and touch the entire stack like a real user would. Acceptance tests are integration tests. It's alright for these tests to hit the database because part of what you are testing is the integration with the database.