samvera / hyrax

Hyrax is a Ruby on Rails Engine built by the Samvera community. Hyrax provides a foundation for creating many different digital repository applications.
http://hyrax.samvera.org/
Apache License 2.0
184 stars 124 forks source link

Clean up unused stubs in collections_membership_actor_spec.rb #2620

Open no-reply opened 6 years ago

no-reply commented 6 years ago

Descriptive summary

Several stubs in this file are on a method name that doesn't exist.

allow(collection.collection_type).to receive(:share_applies_to_works).and_return(true) should stub :share_applies_to_new_works? instead. Additionally, because the tested actor uses ActiveFedora::Base#find to retrieve a new instance of the example collection, the stubs are not used by the code under test even when they do stub the correct method.

Compare:

allow(collection.collection_type).to receive(:share_applies_to_new_works?).and_return(:moomin)
collection.collection_type.share_applies_to_new_works? # => :moomin
loaded_collection = ::Collection.find(collection.id)
loaded_collection.collection_type.share_applies_to_new_works? # => false

At a glance, these stubs seem integral to test setup. Since the tests in question pass, this points to other potential issues with tests or their setup; i.e. it doesn't seem these tests exercise the behavior they are meant to.

The tests should use factory setup instead of stubbing the methods. See https://github.com/samvera/hyrax/pull/2612/files#diff-05ea185029fb2aa40e1887104f3031f3R288 for an example.

Related work

2612

vantuyls commented 6 years ago

@no-reply is this still an issue?

no-reply commented 5 years ago

This is still an issue, but i'm backlogging it.