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.

There was a problem

You must be a member of this account.

This project is archived and is in readonly mode.

Rails::Generators::GeneratedAttribute: tests, cleanups and a bugfix

#4631

After starting writing some tests for Rails::Generators::GeneratedAttribute.field_type in the Bugmash ( #2377 time scaffolding uses datetime_select, stores as time ), I decided to go ahead and write more tests and do some cleaning up.

Also, I've fixed a little bug in Rails::Generators::GeneratedAttribute.default (You can't do .to_s(:db) on a Date object).

The patch is in the comments. :)

Reported by Jeff Kreeftmeijer · May 17th, 2010 @ 09:44 PM

State: resolved
Milestone: 3.0.2
Assigned to: José Valim José Valim
Importance: Medium

Activity

  1. Jeff Kreeftmeijer
    Jeff Kreeftmeijer
    • Tag changed from bug, cleanup, generators, patch, tests to 3.x, bug, cleanup, generators, patch, tests

    May 17th, 2010 @ 09:45 PM

  2. Rohit Arondekar
    Rohit Arondekar
    • State changed from new to resolved
    • Assigned user set to Rizwan Reza

    +1 Nice DRY'ing up, looks cleaner and all tests pass(on 1.9.2-head).

    However I got a warning from Git:

    rohit@rohit-desktop:~/remote-repos/rails_patches/working3$ git am < ../others_patches/generated_attribute_tests.diff
    Applying: Rails::Generators::GeneratedAttribute: tests, cleanups and a bugfix [#4631 Rails::Generators::GeneratedAttribute: tests, cleanups and a bugfix state:resolved]
    /home/rohit/remote-repos/rails_patches/working3/.git/rebase-apply/patch:176: trailing whitespace.

    /home/rohit/remote-repos/rails_patches/working3/.git/rebase-apply/patch:183: trailing whitespace.

    /home/rohit/remote-repos/rails_patches/working3/.git/rebase-apply/patch:192: trailing whitespace.

    warning: 3 lines add whitespace errors.

    May 20th, 2010 @ 10:32 AM

  3. Ryan Bigg
    Ryan Bigg
    • State changed from resolved to open

    Please be careful with putting bracketted stuff like [#ticket_no resolved] as it will mark that ticket as resolved.

    May 20th, 2010 @ 11:14 AM

  4. Rohit Arondekar
    Rohit Arondekar

    Sorry about that, and thanks Ryan for changing state to open. Will take care not to repeat this again.

    May 20th, 2010 @ 11:26 AM

  5. Jeff Kreeftmeijer
    Jeff Kreeftmeijer

    Checked the trailing whitespace problem out, seemed that there were some trailing spaces in railties/test/generators/generated_attribute_test.rb. Here's a new patch. :)

    May 20th, 2010 @ 03:35 PM

  6. Rohit Arondekar
    Rohit Arondekar

    One test fails on 1.9.2-head, see => http://pastie.org/969479
    It appears that in 1.9.2 Proc doesn't have marshal_dump defined. But playing in irb I noticed both 1.8.7 and 1.9.2-head, Proc doesn't have marshal_dump defined, so where is it coming from?

    Someone might want to look at marshal.rb in activesupport, it has some 1.9.2 stuff in it, I don't know if it's relevant though.

    May 20th, 2010 @ 04:15 PM

  7. Rohit Arondekar
    Rohit Arondekar

    I made a mistake in my original comment, I had tested on 1.8.7 and not 1.9.2-head.

    May 20th, 2010 @ 04:44 PM

  8. Rizwan Reza
    Rizwan Reza
    • No changes were found…

    May 20th, 2010 @ 05:09 PM

  9. Jeff Kreeftmeijer
    Jeff Kreeftmeijer

    I can't seem to reproduce Rohit's test fail on master with and without the patch on Ruby 1.8.7-p249 and 1.9.1-p376.

    Can anybody else reproduce this? Also, isn't it really unlikely that my patch would cause this?

    May 22nd, 2010 @ 09:24 AM

  10. Rohit Arondekar
    Rohit Arondekar

    I had tested on 1.9.2-head. I tried again both before and after applying the patch and all tests pass.

    P.S I have no idea why it failed the last time, maybe my environment is borked. :(

    May 22nd, 2010 @ 10:21 AM

  11. Anil Wadghule
    Anil Wadghule

    +1 tests pass (1.8.7)

    One thing, is it right to replace then Date.today.to_s(:db) with then Date.today.to_s?

    If I recall correctly, Date object also supported .to_s(:db).

    May 24th, 2010 @ 08:44 PM

  12. Jeff Kreeftmeijer
    Jeff Kreeftmeijer

    Anil was right, I've put .to_s(:db) back, and included active_support/time in Rails::Generators::GeneratedAttribute. Here's a new patch. :)

    May 24th, 2010 @ 09:20 PM

  13. Rohit Arondekar
    Rohit Arondekar

    +1 Tests pass on
    ruby 1.9.2dev (2010-05-08 trunk 27665)
    ruby 1.8.7 (2009-06-12 patchlevel 174)

    Like the DRY'ing up and the tests look cleaner.

    May 27th, 2010 @ 12:10 PM

  14. Shih-gian Lee
    Shih-gian Lee

    +1 for me too on 1.8.7. Agree with Rohit. Like the code refactoring to make it more DRY.

    May 27th, 2010 @ 04:11 PM

  15. Rizwan Reza
    Rizwan Reza
    • Milestone cleared.
    • Tag changed from 3.x, bug, cleanup, generators, patch, tests to 3.x, bug, bugmash-review, cleanup, generators, patch, tests
    • State changed from open to verified

    June 6th, 2010 @ 10:38 AM

  16. Santiago Pastorino
    Santiago Pastorino
    • Assigned user changed from Rizwan Reza to José Valim

    June 10th, 2010 @ 02:14 AM

  17. Repository
    Repository
    • State changed from verified to resolved

    June 10th, 2010 @ 07:08 PM

  18. Rizwan Reza
    Rizwan Reza
    • Tag changed from 3.x, bug, bugmash-review, cleanup, generators, patch, tests to 3.x, bug, cleanup, generators, patch, tests

    June 13th, 2010 @ 08:15 PM

  19. Jeremy Kemper
    Jeremy Kemper
    • Milestone set to 3.0.2
    • Importance changed from to Medium

    October 15th, 2010 @ 11:01 PM