Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Remove fixtures from tests when --skip-activerecord is used

#4104

Since fixtures use built with ActiveRecord, I modified the rails app generator to only include it if ActiveRecord is being used as the ORM. If --skip-activerecord is used, then the test/fixtures directory is removed and the test_helper.rb file no longer has "fixtures :all" and comments added.

Reported by Peter Bui · March 4th, 2010 @ 08:25 AM

State: committed
Milestone: 3.0.2
Assigned to: Santiago Pastorino Santiago Pastorino
Importance: Low

Activity

  1. Mike Riley
    Mike Riley
    • State changed from new to incomplete
    • Importance changed from to Low

    Hello Peter,

    I tried applying this, but got the following errors:

    macbook$ git apply no_ar_no_fixtures.diff
    error: railties/lib/generators/rails/app/app_generator.rb: No such file or directory
    error: railties/lib/generators/rails/app/templates/test/test_helper.rb: No such file or directory
    error: patch failed: railties/test/generators/app_generator_test.rb:98
    error: railties/test/generators/app_generator_test.rb: patch does not apply

    Can you provide a new patch for this? I was trying to apply this against 2.3.8.

    thanks
    Mike Riley

    July 29th, 2010 @ 06:31 PM

  2. Rohit Arondekar
    Rohit Arondekar

    I get the same errors while trying to apply to 3-0-stable.

    
    Applying: Generating rails app without activerecord will no longer use fixtures in tests.
    error: railties/lib/generators/rails/app/app_generator.rb: does not exist in index
    error: railties/lib/generators/rails/app/templates/test/test_helper.rb: does not exist in index
    error: patch failed: railties/test/generators/app_generator_test.rb:98
    error: railties/test/generators/app_generator_test.rb: patch does not apply
    Patch failed at 0001 Generating rails app without activerecord will no longer use fixtures in tests.
    

    Peter can you fix that patch? Or can somebody else volunteer to do it? :)

    October 7th, 2010 @ 07:04 AM

  3. Aditya Sanghi
    Aditya Sanghi

    i dont mind making the patch now but would be ashamed of getting credit. Can the credit still be given to Peter at commit? :)

    October 7th, 2010 @ 07:33 AM

  4. Aditya Sanghi
    Aditya Sanghi

    Not sure this is relevant anymore.

    Having a quick look, it seems the only thing this patch adds (that is not already there) is skipping the creation of an empty fixtures directory.

    The -skip-activerecord already skips writing of the "fixtures :all" in the test_helper.rb (the condition is already there in the app template).

    We could still re-write the patch to not create the redundant fixtures directory. Is the empty fixtures directory just an irritant or is it actually malignant?

    October 7th, 2010 @ 07:47 AM

  5. Rohit Arondekar
    Rohit Arondekar

    In my opinion the fixtures directory shouldn't be created. Any body else have thoughts on this?

    October 7th, 2010 @ 10:52 AM

  6. David Trasbo
    David Trasbo

    I agree. test/fixtures is Active Record specific and thus shouldn't be created if the user decided to skip using Active Record. If any other fixture libraries out there use the same directory, their generators can simply create it.

    Peter, please recreate the patch. If you don't someone else will, and will get the credit. Consider yourself warned. :)

    October 8th, 2010 @ 08:58 AM

  7. Aditya Sanghi
    Aditya Sanghi

    Here's a rewritten patch. Please verify.

    October 8th, 2010 @ 11:01 AM

  8. Rohit Arondekar
    Rohit Arondekar
    • State changed from incomplete to open
    • Milestone cleared.
    • Assigned user set to Santiago Pastorino

    +1

    Applies, test passes and it works.

    October 8th, 2010 @ 11:32 AM

  9. Santiago Pastorino
    Santiago Pastorino

    Aditya Sanghi you can give credit to Peter using git commit --author="Peter Bui peter@paydrotalks.com
    " and optionally using --amend to change the last commit

    October 8th, 2010 @ 06:45 PM

  10. Santiago Pastorino
    Santiago Pastorino

    Aditya Sanghi if you consider that the commits are completely different is ok having you as the author. One thing i'm going to ask you is to merge the commits in one, you're second patch is changing your first one. Is better have this in one commit to keep trace about patches later ;).
    Thank you.

    October 8th, 2010 @ 07:56 PM

  11. Aditya Sanghi
    Aditya Sanghi

    Here's another one squished into one. Conscience doesnt allow credit though.

    October 8th, 2010 @ 08:01 PM

  12. Peter Bui
    Peter Bui

    No worries Aditya, you can take credit for it. I've forgotten about this ticket and just noticed the emails going into a spam folder. Thanks for fixing it!

    October 8th, 2010 @ 08:36 PM

  13. José Valim
    José Valim

    It seems good but I believe test/fixtures should stay (fixtures does not only refers to AR fixtures). Also, in the final code, you are calling "empty_directory :test" while I believe it is not necessary.

    October 8th, 2010 @ 08:42 PM

  14. Santiago Pastorino
    Santiago Pastorino

    Hehe that's right. But now you can take some credit because the patch is wrong.

    performance dir is not empty it has a browsing_test.rb file, so make your own patch based on the previous changes adding this dir with this file. Please add tests for that.

    Thanks.

    October 8th, 2010 @ 08:43 PM

  15. Aditya Sanghi
    Aditya Sanghi

    Fine, reverting the code bits. Just fixing the test. The rest is unnecessary as it is all already working.

    October 8th, 2010 @ 08:57 PM

  16. Santiago Pastorino
    Santiago Pastorino

    You still need to change performance dir from an empty dir to a dir with the browsing_test.rb file and add a test for this ;)

    October 8th, 2010 @ 11:09 PM

  17. Aditya Sanghi
    Aditya Sanghi

    I must be going nuts, Santiago, but i'm not able to see the obvious here.

    As I investigated earlier, there were 3 main bits to Peter Bui's patch. One was fixing the test_helper.rb to not output "fixtures :all" in case skip-active-record is provided. The second was split out "directory 'test'" into a bigger code block in app_generator.rb. The third was a test case.

    Now as I investigated, the first part is already in place, so no change. The second part does not need to be updated if as Jose suggested we leave the fixtures directory in the test directory, so no change here too.

    The only part my patch puts in is a new test assertion that tests for non-existance of "fixtures :all" when --skip-active-record is provided.

    Perhaps you are looking at an older patch of mine? Or maybe in your local area, my earlier patch removed some directories?

    Feel free to point me to something i'm missing here.

    October 9th, 2010 @ 08:51 AM

  18. Repository
  19. Repository
  20. Santiago Pastorino
    Santiago Pastorino

    Aditya your patch was committed, sorry I wasn't understanding you. Well done!

    October 9th, 2010 @ 04:34 PM

  21. Jeremy Kemper