4 ms·

I hate when people are using it in the unit tests. It just makes testing very convenient, so they start using it everywhere. In the end, the test suite gets ver

I hate when people are using it in the unit tests. It just makes testing very convenient, so they start using it everywhere. In the end, the test suite gets very slow. Unit tests should not hit the database. First example there: test_new_customer_defaults. Really, it needs 3 saved models to test this? FactoryGirl isn't solution here. It's just hiding the problem.

urbanautomaton 14y ago
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().
- samd 14y ago
  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.
  - mattgreenrocks 14y ago
    That's where we find ourselves currently: constructing 3-4 levels out due to Law of Demeter violations, and it becomes incredibly brittle when seemingly unrelated classes change. IMO, DHH (and others) argue too hard against core OO principles (like SRP and LoD) that have the possibility of helping out when the domain model becomes more complex. "Just use Rails" is fine when spinning up, but as the app grows, it becomes increasingly less pleasant. Coupling increases in the name of less code. Ideas like hexagonal architecture look nice, but I'm left wondering how to implement them when the framework insists on infecting core domain classes (AR::Base).
    - deafbybeheading 14y ago
      So can someone recommend a Ruby (edit: ideally Sinatra/Sequel, not Rails) app one could look at with a more principled test suite? I'm coming from a more traditional OO background and jumping into Ruby. It's good to see I'm not alone in finding this militant pragmatism somewhat off-putting, but I don't want to reinvent the wheel.
    - sanderjd 14y ago
      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/active_record/base.rb#L324 https://github.com/rails/rails/blob/master/activerecord/lib/....
      - mattgreenrocks 14y ago
        But your core domain classes are still coupled to their persistence mechanism in the name of 'pragmatism.'
        sanderjd 14y ago
        Who says they have to be? That's a design decision. There is literally nothing (besides maybe inertia?) that enforces that "rule".
  - davidcuddeback 14y ago
    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.
    - samd 14y ago
      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.
jeremyjh 14y ago
You can use the build method to instantiate the model without saving to the database. Very useful because you only need use create when you need persistence in your test - such as when testing a controller.
- urbanautomaton 14y ago
  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.
  - chrislaco 14y ago
    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 factory creates. Today it's one relation. Years from now, it could be 10 other tables relations. Someone later on decides they want to reuse the :user factory, but they don't want the account to be create, so then they do :user_only factory. Yada yada. Shenanigans ensue. If you stick to the above strategy, things are much faster, but more importantly, there are less unintended surprises, like associations doing a create instead of a build, etc.
    - jeremyjh 14y ago
      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.
    - urbanautomaton 14y ago
j_s 14y ago
One compromise (edit: on the performance problem) would be to use FactoryGirl to generate some type of (yaml?) cache pre-test-run, automatically refreshing only when the model changes.
- davidcuddeback 14y ago
  I think you're missing the point of what makes FactoryGirl an anti-pattern in unit tests. FactoryGirl reduces the feedback that you can get from unit tests by hiding complexity (of creating objects), which your unit tests are supposed to make apparent. Edited to add: Using FactoryGirl to generate a cache only addresses the performance issues without addressing the complexity-hiding problem.
  - j_s 14y ago
    I don't disagree; I was only trying to address the 'test suite gets very slow' part rather than the 'unit tests should not hit the database' part. If performance wasn't an issue, the convenience of auto-created objects would mean more 'unit' tests (edit: usually closer to integration tests) that wouldn't exist otherwise. Sure it's best to de-couple things, but there's room for something between no [unit] tests at all and perfect unit tests.
  - chrislaco 14y ago
    I could also argue that's exactly WHY to use factory girl in unit tests. You're not using FG to create the things under test, only the things to pass to the thing under test. Complexity is a moot point for those extraneous objects. You want that complexity out of the tests since it's not the focus of the test itself, and makes it more readable. But I'll take this one step further. To each their own. There is no one exact correct way. Do what's right for your code, domain, and tests. I wouldn't test an API codebase the same I would a site codebase or a gem codebase anyways.
    - davidcuddeback 14y ago
      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 what FactoryGirl hides from you. I tend to use fake objects for the collaborators. Usually just a vanilla test double will do, but sometimes I use RSpec's stub_model if the collaborator has to behave more "model-like". I'd recommend http://pyvideo.org/video/631/fast-test-slow-test http://pyvideo.org/video/631/fast-test-slow-test and http://www.confreaks.com/videos/641-gogaruco2011-fast-rails-tests http://www.confreaks.com/videos/641-gogaruco2011-fast-rails-... if you haven't seen them already.
gyardley 14y ago
Yep, I'm trying to dig myself out from this problem now. The Rails Tutorial (railstutorial.org) uses Factory Girl and integration tests for just about everything, which is a fast enough approach for its toy application, but I've found that when your application gets more complex your test suite takes forever to run, and setting up the tests gets more and more painful (especially when you start dealing with external APIs). Does anyone have any opinions on the best resource for teaching effective unit testing in Rails? I'd love to speed up my test suite while not losing coverage.
- davidcuddeback 14y ago
  It's not really focused on unit testing (though it does cover it) so much as it's focused on good design, but the two are related, so I found Avdi's "Objects on Rails" [1] helpful. [1]: http://objectsonrails.com/ http://objectsonrails.com/ I'm not sure if it's the best resource, though. I'd love to hear other opinions.

urbanautomaton 14y ago

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().

samd 14y ago
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.
- mattgreenrocks 14y ago
  That's where we find ourselves currently: constructing 3-4 levels out due to Law of Demeter violations, and it becomes incredibly brittle when seemingly unrelated classes change. IMO, DHH (and others) argue too hard against core OO principles (like SRP and LoD) that have the possibility of helping out when the domain model becomes more complex. "Just use Rails" is fine when spinning up, but as the app grows, it becomes increasingly less pleasant. Coupling increases in the name of less code. Ideas like hexagonal architecture look nice, but I'm left wondering how to implement them when the framework insists on infecting core domain classes (AR::Base).
  - deafbybeheading 14y ago
    So can someone recommend a Ruby (edit: ideally Sinatra/Sequel, not Rails) app one could look at with a more principled test suite? I'm coming from a more traditional OO background and jumping into Ruby. It's good to see I'm not alone in finding this militant pragmatism somewhat off-putting, but I don't want to reinvent the wheel.
  - sanderjd 14y ago
    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/active_record/base.rb#L324 https://github.com/rails/rails/blob/master/activerecord/lib/....
    - mattgreenrocks 14y ago
      But your core domain classes are still coupled to their persistence mechanism in the name of 'pragmatism.'
      - sanderjd 14y ago
        Who says they have to be? That's a design decision. There is literally nothing (besides maybe inertia?) that enforces that "rule".
- davidcuddeback 14y ago
  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.
  - samd 14y ago
    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.

jeremyjh 14y ago

You can use the build method to instantiate the model without saving to the database. Very useful because you only need use create when you need persistence in your test - such as when testing a controller.

urbanautomaton 14y ago
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.
- chrislaco 14y ago
  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 factory creates. Today it's one relation. Years from now, it could be 10 other tables relations. Someone later on decides they want to reuse the :user factory, but they don't want the account to be create, so then they do :user_only factory. Yada yada. Shenanigans ensue. If you stick to the above strategy, things are much faster, but more importantly, there are less unintended surprises, like associations doing a create instead of a build, etc.
  - jeremyjh 14y ago
    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.
  - urbanautomaton 14y ago

j_s 14y ago

One compromise (edit: on the performance problem) would be to use FactoryGirl to generate some type of (yaml?) cache pre-test-run, automatically refreshing only when the model changes.

davidcuddeback 14y ago
I think you're missing the point of what makes FactoryGirl an anti-pattern in unit tests. FactoryGirl reduces the feedback that you can get from unit tests by hiding complexity (of creating objects), which your unit tests are supposed to make apparent. Edited to add: Using FactoryGirl to generate a cache only addresses the performance issues without addressing the complexity-hiding problem.
- j_s 14y ago
  I don't disagree; I was only trying to address the 'test suite gets very slow' part rather than the 'unit tests should not hit the database' part. If performance wasn't an issue, the convenience of auto-created objects would mean more 'unit' tests (edit: usually closer to integration tests) that wouldn't exist otherwise. Sure it's best to de-couple things, but there's room for something between no [unit] tests at all and perfect unit tests.
- chrislaco 14y ago
  I could also argue that's exactly WHY to use factory girl in unit tests. You're not using FG to create the things under test, only the things to pass to the thing under test. Complexity is a moot point for those extraneous objects. You want that complexity out of the tests since it's not the focus of the test itself, and makes it more readable. But I'll take this one step further. To each their own. There is no one exact correct way. Do what's right for your code, domain, and tests. I wouldn't test an API codebase the same I would a site codebase or a gem codebase anyways.
  - davidcuddeback 14y ago
    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 what FactoryGirl hides from you. I tend to use fake objects for the collaborators. Usually just a vanilla test double will do, but sometimes I use RSpec's stub_model if the collaborator has to behave more "model-like". I'd recommend http://pyvideo.org/video/631/fast-test-slow-test http://pyvideo.org/video/631/fast-test-slow-test and http://www.confreaks.com/videos/641-gogaruco2011-fast-rails-tests http://www.confreaks.com/videos/641-gogaruco2011-fast-rails-... if you haven't seen them already.

gyardley 14y ago

Yep, I'm trying to dig myself out from this problem now. The Rails Tutorial (railstutorial.org) uses Factory Girl and integration tests for just about everything, which is a fast enough approach for its toy application, but I've found that when your application gets more complex your test suite takes forever to run, and setting up the tests gets more and more painful (especially when you start dealing with external APIs). Does anyone have any opinions on the best resource for teaching effective unit testing in Rails? I'd love to speed up my test suite while not losing coverage.

davidcuddeback 14y ago
It's not really focused on unit testing (though it does cover it) so much as it's focused on good design, but the two are related, so I found Avdi's "Objects on Rails" [1] helpful. [1]: http://objectsonrails.com/ http://objectsonrails.com/ I'm not sure if it's the best resource, though. I'd love to hear other opinions.