Tests for issue #6659 - Unvalidated saves and Foreign Keys #12525

Closed
wants to merge 14 commits into
from

Conversation

Projects
None yet
3 participants

This adds tests, one of which fails currently (with descriptive comments of current behaviour) for the behavior of associations. This is the test code referenced in #6659

The test test_save_validate_false_after_association_created is currently failing and I believe that this is a minor Activerecord problem rather than a problem with this test. When I looked I couldn't find the right place to hook in to offer a fix to the underlying bug but I expect someone more familiar with the framework internals could do so quickly.

Note that the failures on mysql and postgresql in Travis are the expected result of this PR (this test expects enforced Foreign Key constraints so shouldn't be run on sqllite). These tests describe the behaviour as I believe it should be. rather than the current behaviour of Activerecord.

 1) Failure:
UnvalidatedSaveTest#test_save_validate_false_after_association_created [test/cases/associations/unvalidated_save_test.rb:43]:
ActiveRecord::InvalidForeignKey expected but nothing was raised.
5 runs, 21 assertions, 1 failures, 0 errors, 0 skips

rake aborted!
Command failed with status (1): [/home/travis/.rvm/rubies/ruby-2.0.0-p247/b...]
/home/travis/build/rails/rails/activerecord/Rakefile:64:in `block (3 levels) in <top (required)>
/home/travis/build/rails/rails/activerecord/Rakefile:63:in `each'
/home/travis/build/rails/rails/activerecord/Rakefile:63:in `all?'
/home/travis/build/rails/rails/activerecord/Rakefile:63:in `block (2 levels) in <top (required)>'
Tasks: TOP => postgresql:isolated_test => isolated_test_postgresql
(See full trace by running task with --trace)
Rails build FAILED
Failed components: activerecord:postgresql, activerecord:postgresql:isolated
The command "ci/travis.rb" exited with 1.
Done. Your build exited with 1.

The test that fails (on mysql and postgresql) is:

  def test_save_validate_false_after_association_created
    # This fails due to the association having been set before the id is set to something invalid
    door  = Door.new
    door.house = House.new # This makes the test fail below!!!
    bad_house_id = House.order(:id).last.id + 10000
    house_count = House.count
    door.house_id = bad_house_id
    # door.house # This would make it pass even with the above
    # door.valid? # This would also make it pass even with the above
    assert_raise(ActiveRecord::InvalidForeignKey) { door.save(validate: false) }  # Fails here
    assert_raise(ActiveRecord::RecordNotFound) { door.reload }
    assert_equal bad_house_id, door.house_id
    assert_raises(ActiveRecord::RecordNotFound) { House.find bad_house_id }
    assert_equal house_count, House.count
  end
@josephlord josephlord Skip the added tests when running on sqlite as they depend on Foreign…
… Key constraints which I don't believe are supported in sqlite.
7fc5342

@laurocaetano laurocaetano commented on the diff Mar 27, 2014

...cord/test/cases/associations/unvalidated_save_test.rb
+ assert_equal bad_house_id, door.house_id
+ assert_raises(ActiveRecord::RecordNotFound) { House.find bad_house_id }
+ assert_equal house_count, House.count
+ end
+
+ def test_save_validate_false_after_association_created
+ return skip("foreign key support required - sqlite not supported") if current_adapter?(:SQLite3Adapter)
+ # This fails due to the association having been set before the id is set to something invalid
+ door = Door.new
+ door.house = House.new # This makes the test fail below!!!
+ bad_house_id = House.order(:id).last.id + 10000
+ house_count = House.count
+ door.house_id = bad_house_id
+ # door.house # This would make it pass even with the above
+ # door.valid? # This would also make it pass even with the above
+ assert_raise(ActiveRecord::InvalidForeignKey) { door.save(validate: false) } # Fails here
@laurocaetano

laurocaetano Mar 27, 2014

Contributor

It's failing using validate: false because it's ignoring the house_id that you have set and using the association that you have assigned before.

Not using validate: false and setting the house_id will also not raise ActiveRecord::InvalidForeignKey, because it will clear the association and make the door invalid (because you have the presence validation for house).

@josephlord

josephlord Mar 27, 2014

Thanks laurocaetano.

As I said initially in #6659 I wasn't sure it was incorrect behaviour. I suppose I was just surprised that validation had side effects and that setting foo_id did not clear a previously set foo association. I couldn't at the time find documentation that stated such behaviour but even if nothing else exists (I haven't searched recently) at least this issue is here to refer to.

Owner

rafaelfranca commented Mar 27, 2014

Agree with #12525 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment