Rails Ruby on Rails → 3.0.2

Comments and changes to this ticket

• 

  Mike Riley July 29th, 2010 @ 06:31 PM

  • 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

• 

  Rohit Arondekar October 7th, 2010 @ 07:04 AM

  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? :)

• 

  Aditya Sanghi October 7th, 2010 @ 07:33 AM

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

• 

  Aditya Sanghi October 7th, 2010 @ 07:47 AM

  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?

• 

  Rohit Arondekar October 7th, 2010 @ 10:52 AM

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

• 

  David Trasbo October 8th, 2010 @ 08:58 AM

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

• 

  Aditya Sanghi October 8th, 2010 @ 11:01 AM

  Here's a rewritten patch. Please verify.

  • skip_ar_fixtures.diff 4.3 KB
• 

  Rohit Arondekar October 8th, 2010 @ 11:32 AM

  • State changed from “incomplete” to “open”
  • Milestone cleared.
  • Assigned user set to “Santiago Pastorino”

  +1

  Applies, test passes and it works.

• 

  Santiago Pastorino October 8th, 2010 @ 06:45 PM

  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

• 

  Santiago Pastorino October 8th, 2010 @ 07:56 PM

  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.

• 

  Aditya Sanghi October 8th, 2010 @ 08:01 PM

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

  • skip_ar.diff 3.3 KB

• You flagged this item as spam.
  

  Peter Bui October 8th, 2010 @ 08:36 PM

  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!

• 

  José Valim October 8th, 2010 @ 08:42 PM

  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.

• 

  Santiago Pastorino October 8th, 2010 @ 08:43 PM

  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.

• 

  Aditya Sanghi October 8th, 2010 @ 08:57 PM

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

  • skip_ar_only_test.diff 1 KB
• 

  Santiago Pastorino October 8th, 2010 @ 11:09 PM

  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 ;)

• 

  Aditya Sanghi October 9th, 2010 @ 08:51 AM

  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.

• 

  Repository October 9th, 2010 @ 04:12 PM

  (from [e7f911d5ca58e83b94f58d5f44352794b27b2a10]) Test if test/performance/browsing_test.rb exists when --skip-activerecord is used

  [#4104] http://github.com/rails/rails/commit/e7f911d5ca58e83b94f58d5f443527...

• 

  Repository October 9th, 2010 @ 04:32 PM

  • State changed from “open” to “committed”

  (from [36862511e065d5ea02dae9d2dcb7f498af99cab1]) test that skip active records does not load fixtures

  [#4104 state:committed]

  Signed-off-by: Santiago Pastorino santiago@wyeworks.com
  http://github.com/rails/rails/commit/36862511e065d5ea02dae9d2dcb7f4...

• 

  Santiago Pastorino October 9th, 2010 @ 04:34 PM

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

• 

  Jeremy Kemper October 15th, 2010 @ 11:01 PM

  • Milestone set to “3.0.2”

  [bulk edit]