I'm working on a patch for an ActiveRecord bug that's been annoying me, and I came across an issue with the ActiveRecord fixtures. I'm concerned that this issue may cause subtle test bugs for others.
=== The Problem ===
The following will fail on the second assertion:
firm = companies(:odegy) assert_equal firm.id, firm.account.firm_id assert_equal firm, firm.account.firm
The reason is that Account belongs_to :firm, but companies(:odegy) is actually an ExclusivelyDependentFirm which is not a subclass of Firm, hence firm.account.firm returns nil.
One of the results of this is that the following will fail:
companies(:odegy).account.destroy assert !Account.destroyed_account_ids.empty?
In other words, if destroy is called on the account of an ExclusivelyDependentFirm, the account won't be added to the destroyed_account_ids, and it will appear as though the account was deleted rather than destroyed.
=== The Solution ===
The simple solution for the particular test I'm writing is to change the before_destroy on Account as follows:
before_destroy do |account| - if account.firm - Account.destroyed_account_ids[account.firm.id] << account.id + if account.firm_id + Account.destroyed_account_ids[account.firm_id] << account.id end true end
This doesn't break any existing tests.
The bigger issue, however, is that it's not clear what the correct behaviour should be in this case. Having firm.account.firm be nil is confusing at best, dangerous at worst.
=== The Questions ===
Is my fix to the before_destroy appropriate?
Is this problem likely to bite other tests in subtle ways?
Is there some way to make all this neater and less confusing?