Skip to content

Commit dba4efb

Browse files
subbaraojosevalim
authored andcommitted
nested attributes tests should rely on associated objects to verify results not on assert_difference [rails#5206 state:resolved]
Signed-off-by: José Valim <jose.valim@gmail.com>
1 parent 46c14a6 commit dba4efb

File tree

1 file changed

+64
-57
lines changed

1 file changed

+64
-57
lines changed

activerecord/test/cases/nested_attributes_test.rb

Lines changed: 64 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ def test_should_disable_allow_destroy_by_default
7474
pirate = Pirate.create!(:catchphrase => "Don' botharrr talkin' like one, savvy?")
7575
ship = pirate.create_ship(:name => 'Nights Dirty Lightning')
7676

77-
assert_no_difference('Ship.count') do
78-
pirate.update_attributes(:ship_attributes => { '_destroy' => true, :id => ship.id })
79-
end
77+
pirate.update_attributes(:ship_attributes => { '_destroy' => true, :id => ship.id })
78+
79+
assert_nothing_raised(ActiveRecord::RecordNotFound) { pirate.ship.reload }
8080
end
8181

8282
def test_a_model_should_respond_to_underscore_destroy_and_return_if_it_is_marked_for_destruction
@@ -194,28 +194,30 @@ def test_should_modify_an_existing_record_if_there_is_a_matching_composite_id
194194

195195
def test_should_destroy_an_existing_record_if_there_is_a_matching_id_and_destroy_is_truthy
196196
@pirate.ship.destroy
197+
197198
[1, '1', true, 'true'].each do |truth|
198-
@pirate.reload.create_ship(:name => 'Mister Pablo')
199-
assert_difference('Ship.count', -1) do
200-
@pirate.update_attributes(:ship_attributes => { :id => @pirate.ship.id, :_destroy => truth })
201-
end
199+
ship = @pirate.reload.create_ship(:name => 'Mister Pablo')
200+
@pirate.update_attributes(:ship_attributes => { :id => ship.id, :_destroy => truth })
201+
202+
assert_nil @pirate.reload.ship
203+
assert_raise(ActiveRecord::RecordNotFound) { Ship.find(ship.id) }
202204
end
203205
end
204206

205207
def test_should_not_destroy_an_existing_record_if_destroy_is_not_truthy
206208
[nil, '0', 0, 'false', false].each do |not_truth|
207-
assert_no_difference('Ship.count') do
208-
@pirate.update_attributes(:ship_attributes => { :id => @pirate.ship.id, :_destroy => not_truth })
209-
end
209+
@pirate.update_attributes(:ship_attributes => { :id => @pirate.ship.id, :_destroy => not_truth })
210+
211+
assert_equal @ship, @pirate.reload.ship
210212
end
211213
end
212214

213215
def test_should_not_destroy_an_existing_record_if_allow_destroy_is_false
214216
Pirate.accepts_nested_attributes_for :ship, :allow_destroy => false, :reject_if => proc { |attributes| attributes.empty? }
215217

216-
assert_no_difference('Ship.count') do
217-
@pirate.update_attributes(:ship_attributes => { :id => @pirate.ship.id, :_destroy => '1' })
218-
end
218+
@pirate.update_attributes(:ship_attributes => { :id => @pirate.ship.id, :_destroy => '1' })
219+
220+
assert_equal @ship, @pirate.reload.ship
219221

220222
Pirate.accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
221223
end
@@ -236,12 +238,15 @@ def test_should_work_with_update_attributes_as_well
236238
end
237239

238240
def test_should_not_destroy_the_associated_model_until_the_parent_is_saved
239-
assert_no_difference('Ship.count') do
240-
@pirate.attributes = { :ship_attributes => { :id => @ship.id, :_destroy => '1' } }
241-
end
242-
assert_difference('Ship.count', -1) do
243-
@pirate.save
244-
end
241+
@pirate.attributes = { :ship_attributes => { :id => @ship.id, :_destroy => '1' } }
242+
243+
assert !@pirate.ship.destroyed?
244+
assert @pirate.ship.marked_for_destruction?
245+
246+
@pirate.save
247+
248+
assert @pirate.ship.destroyed?
249+
assert_nil @pirate.reload.ship
245250
end
246251

247252
def test_should_automatically_enable_autosave_on_the_association
@@ -254,39 +259,42 @@ def test_should_accept_update_only_option
254259

255260
def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true
256261
@ship.delete
257-
assert_difference('Ship.count', 1) do
258-
@pirate.reload.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower' })
259-
end
262+
263+
@pirate.reload.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower' })
264+
265+
assert_not_nil @pirate.ship
260266
end
261267

262268
def test_should_update_existing_when_update_only_is_true_and_no_id_is_given
263269
@ship.delete
264270
@ship = @pirate.create_update_only_ship(:name => 'Nights Dirty Lightning')
265271

266-
assert_no_difference('Ship.count') do
267-
@pirate.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower' })
268-
end
272+
@pirate.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower' })
273+
269274
assert_equal 'Mayflower', @ship.reload.name
275+
assert_equal @ship, @pirate.reload.ship
270276
end
271277

272278
def test_should_update_existing_when_update_only_is_true_and_id_is_given
273279
@ship.delete
274280
@ship = @pirate.create_update_only_ship(:name => 'Nights Dirty Lightning')
275281

276-
assert_no_difference('Ship.count') do
277-
@pirate.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower', :id => @ship.id })
278-
end
282+
@pirate.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower', :id => @ship.id })
283+
279284
assert_equal 'Mayflower', @ship.reload.name
285+
assert_equal @ship, @pirate.reload.ship
280286
end
281287

282288
def test_should_destroy_existing_when_update_only_is_true_and_id_is_given_and_is_marked_for_destruction
283289
Pirate.accepts_nested_attributes_for :update_only_ship, :update_only => true, :allow_destroy => true
284290
@ship.delete
285291
@ship = @pirate.create_update_only_ship(:name => 'Nights Dirty Lightning')
286292

287-
assert_difference('Ship.count', -1) do
288-
@pirate.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower', :id => @ship.id, :_destroy => true })
289-
end
293+
@pirate.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower', :id => @ship.id, :_destroy => true })
294+
295+
assert_nil @pirate.reload.ship
296+
assert_raise(ActiveRecord::RecordNotFound) { Ship.find(@ship.id) }
297+
290298
Pirate.accepts_nested_attributes_for :update_only_ship, :update_only => true, :allow_destroy => false
291299
end
292300

@@ -375,27 +383,24 @@ def test_should_modify_an_existing_record_if_there_is_a_matching_composite_id
375383
def test_should_destroy_an_existing_record_if_there_is_a_matching_id_and_destroy_is_truthy
376384
@ship.pirate.destroy
377385
[1, '1', true, 'true'].each do |truth|
378-
@ship.reload.create_pirate(:catchphrase => 'Arr')
379-
assert_difference('Pirate.count', -1) do
380-
@ship.update_attributes(:pirate_attributes => { :id => @ship.pirate.id, :_destroy => truth })
381-
end
386+
pirate = @ship.reload.create_pirate(:catchphrase => 'Arr')
387+
@ship.update_attributes(:pirate_attributes => { :id => pirate.id, :_destroy => truth })
388+
assert_raise(ActiveRecord::RecordNotFound) { pirate.reload }
382389
end
383390
end
384391

385392
def test_should_not_destroy_an_existing_record_if_destroy_is_not_truthy
386393
[nil, '0', 0, 'false', false].each do |not_truth|
387-
assert_no_difference('Pirate.count') do
388-
@ship.update_attributes(:pirate_attributes => { :id => @ship.pirate.id, :_destroy => not_truth })
389-
end
394+
@ship.update_attributes(:pirate_attributes => { :id => @ship.pirate.id, :_destroy => not_truth })
395+
assert_nothing_raised(ActiveRecord::RecordNotFound) { @ship.pirate.reload }
390396
end
391397
end
392398

393399
def test_should_not_destroy_an_existing_record_if_allow_destroy_is_false
394400
Ship.accepts_nested_attributes_for :pirate, :allow_destroy => false, :reject_if => proc { |attributes| attributes.empty? }
395401

396-
assert_no_difference('Pirate.count') do
397-
@ship.update_attributes(:pirate_attributes => { :id => @ship.pirate.id, :_destroy => '1' })
398-
end
402+
@ship.update_attributes(:pirate_attributes => { :id => @ship.pirate.id, :_destroy => '1' })
403+
assert_nothing_raised(ActiveRecord::RecordNotFound) { @ship.pirate.reload }
399404

400405
Ship.accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? }
401406
end
@@ -409,10 +414,12 @@ def test_should_work_with_update_attributes_as_well
409414
end
410415

411416
def test_should_not_destroy_the_associated_model_until_the_parent_is_saved
412-
assert_no_difference('Pirate.count') do
413-
@ship.attributes = { :pirate_attributes => { :id => @ship.pirate.id, '_destroy' => true } }
414-
end
415-
assert_difference('Pirate.count', -1) { @ship.save }
417+
pirate = @ship.pirate
418+
419+
@ship.attributes = { :pirate_attributes => { :id => pirate.id, '_destroy' => true } }
420+
assert_nothing_raised(ActiveRecord::RecordNotFound) { Pirate.find(pirate.id) }
421+
@ship.save
422+
assert_raise(ActiveRecord::RecordNotFound) { Pirate.find(pirate.id) }
416423
end
417424

418425
def test_should_automatically_enable_autosave_on_the_association
@@ -421,39 +428,39 @@ def test_should_automatically_enable_autosave_on_the_association
421428

422429
def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true
423430
@pirate.delete
424-
assert_difference('Pirate.count', 1) do
425-
@ship.reload.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr' })
426-
end
431+
@ship.reload.attributes = { :update_only_pirate_attributes => { :catchphrase => 'Arr' } }
432+
433+
assert @ship.update_only_pirate.new_record?
427434
end
428435

429436
def test_should_update_existing_when_update_only_is_true_and_no_id_is_given
430437
@pirate.delete
431438
@pirate = @ship.create_update_only_pirate(:catchphrase => 'Aye')
432439

433-
assert_no_difference('Pirate.count') do
434-
@ship.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr' })
435-
end
440+
@ship.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr' })
436441
assert_equal 'Arr', @pirate.reload.catchphrase
442+
assert_equal @pirate, @ship.reload.update_only_pirate
437443
end
438444

439445
def test_should_update_existing_when_update_only_is_true_and_id_is_given
440446
@pirate.delete
441447
@pirate = @ship.create_update_only_pirate(:catchphrase => 'Aye')
442448

443-
assert_no_difference('Pirate.count') do
444-
@ship.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr', :id => @pirate.id })
445-
end
449+
@ship.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr', :id => @pirate.id })
450+
446451
assert_equal 'Arr', @pirate.reload.catchphrase
452+
assert_equal @pirate, @ship.reload.update_only_pirate
447453
end
448454

449455
def test_should_destroy_existing_when_update_only_is_true_and_id_is_given_and_is_marked_for_destruction
450456
Ship.accepts_nested_attributes_for :update_only_pirate, :update_only => true, :allow_destroy => true
451457
@pirate.delete
452458
@pirate = @ship.create_update_only_pirate(:catchphrase => 'Aye')
453459

454-
assert_difference('Pirate.count', -1) do
455-
@ship.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr', :id => @pirate.id, :_destroy => true })
456-
end
460+
@ship.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr', :id => @pirate.id, :_destroy => true })
461+
462+
assert_raise(ActiveRecord::RecordNotFound) { @pirate.reload }
463+
457464
Ship.accepts_nested_attributes_for :update_only_pirate, :update_only => true, :allow_destroy => false
458465
end
459466
end

0 commit comments

Comments
 (0)