hyrax icon indicating copy to clipboard operation
hyrax copied to clipboard

Fix Sipity::Role#destroy spec

Open randalldfloyd opened this issue 11 months ago • 2 comments

The spec Sipity::Role#destroy almost always fails in the context of the entire test suite, but will succeed when ran again individually. The failure is always the same:

./spec/models/sipity/role_spec.rb
 
 Sipity::Role#destroy will not allow registered role names to be destroyed
 
 PG::UniqueViolation: ERROR:  duplicate key value violates unique constraint "index_sipity_roles_on_name" DETAIL:  Key (name)=(managing) already exists.
Failure/Error: role = described_class.create!(name: Hyrax::RoleRegistry::MANAGING)
...
/app/bundle/ruby/3.3.0/gems/activerecord-7.2.2.1/lib/active_record/persistence.rb:55:in `create!'
./spec/models/sipity/role_spec.rb:32:in `block (3 levels) in <module:Sipity>'

There is likely a race condition where a previous database operation hasn't finished cleaning up before this test starts. Here are a couple of ideas from @dmolesUC :

In terms of just getting this spec to pass, it seems like the easiest fix would be to replace the create! with find_or_create_by

I notice we're using different DatabaseCleaner strategies for different tests : https://github.com/samvera/hyrax/blob/5b03f594d8ac0b1122f85a67e50fb2798fdcf9da/spec/spec_helper.rb#L199-L205 — maybe sometimes a Capybara test hasn't finished cleaning up when the Role test starts?

It might be worth trying just using truncation for everything, I know it's supposed to be slower but IDK if it's enough slower that we need to care

randalldfloyd avatar Feb 07 '25 14:02 randalldfloyd

@randalldfloyd I wasn't able to replicate the error with a full or targeted run. However, I would've recommended the same find_or_create_by fix mentioned above.

rkuehn-uofl avatar Apr 16 '25 11:04 rkuehn-uofl

@rkuehn-uofl Thanks for having a look at that. It's been a while since I've seen this happen in the test suite, so I think we're safe to hold it back for a follow-on release to 5.1.

randalldfloyd avatar Apr 18 '25 19:04 randalldfloyd