This project is archived and is in readonly mode.

#5171 ✓stale
Neeraj Singh

refactor tests for nested_attributes

Reported by Neeraj Singh | July 21st, 2010 @ 02:51 PM | in 3.x

This ticket is continuation of discussion from ticket #5053. Please read that ticket to get a background.

As part of fix for #5053 I created a personal branch where some discussion took place. Here I am going to copy paste some of the discussions that took place. It will not contain all the discussions, just some of them which seem relevant.

Alloy:

In nested_attributes_test.rb following should be changed from

  test "if association is not loaded and child doesn't change and I am saving a grandchild then in memory record should be used" do
    @ship.parts_attributes=[{:id => @part.id,:trinkets_attributes =>[{:id => @trinket.id, :name => 'Ruby'}]}]
    assert_equal 1, @ship.parts.proxy_target.size
    assert_equal 'Mast', @ship.parts[0].name
    assert_no_difference("@ship.parts[0].trinkets.proxy_target.size") do
      @ship.parts[0].trinkets.proxy_target.size
    end
    assert_equal 'Ruby', @ship.parts[0].trinkets[0].name
    @ship.save
    assert_equal 'Ruby', @ship.parts[0].trinkets[0].name
  end

to

What I actually meant, in the last comment, was to make the test look something like:

<pre>
  test "load_target does not replace updated attributes set on an in-memory version of the same record" do
    @trinket.name = 'Ruby'
    assert !@part.trinkets.loaded?
    assert_equal 'Ruby', @ship.parts[0].trinkets[0].name
    assert_no_difference("@part.trinkets.proxy_target.size") { @ship.save! }
    assert_equal 'Ruby', @ship.parts[0].trinkets[0].name
  end
</pre>

I also tried to make the test description a bit more clearer, but I'm not sure if I got that right.

Alloy:

As a final note, it seems that some of these patches are written because of an issue encountered when using nested attributes. However, they are not specifically about nested attributes. For instance the one mentioned above, which is really about association collection. So I would expect at least a test added to the association collection test, and maybe a regression one to the nested attributes tests. I especially was a bit surprised to find that there are a whole bunch of these ‘lost’ tests in the nested attributes tests.

All tests from line 811 down should not be there at all: http://github.com/neerajdotname/rails/blob/49cc2d682f17dcf74e11b56c...
They should either be in the autosave association tests, or in the association collection tests, as far as I have studied them. Furthermore, they should not use the nested attributes API, as again, it's unrelated and only adds noise.

Comments and changes to this ticket

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:34 PM

    This issue has been automatically marked as stale because it has not been commented on for at least three months.

    The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

    Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

  • Santiago Pastorino

    Santiago Pastorino February 2nd, 2011 @ 04:34 PM

    • State changed from “open” to “stale”

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>