Skip to content

Commit

Permalink
Fix supports_helper stub_supports to handle unsupported_reason
Browse files Browse the repository at this point in the history
We are now getting away from supports?(:supported_feature).
Instead we are calling unsupported_reason(:supported_feature)

It cuts down on our calls into the supports_feature_mixin.

Before
======

When using stub_supports,
unsupported_reason(:supported_feature) was not stubbed (and unpredictable)
supports? was stubbed

After
=====

We are stubbing at the feature definition layer
all code in unsupported_reason is run/tested
all code in supports? is run/tested

This puts more code under test and allows flexibility for refactoring

Notes
=====

Wasn't thrilled about kwarg supports, but a few tests still use that.
I've changed them all but it wasn't too much work to keep this working
so we can backport with minimal fuss

This code better tests our features that defer to another feature
  • Loading branch information
kbrock committed Apr 5, 2024
1 parent dfbf8e7 commit bc0151b
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 22 deletions.
35 changes: 30 additions & 5 deletions spec/lib/supports_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,39 +6,64 @@
include SupportsFeatureMixin
supports :archive
supports_not :delete
supports(:archive_ref) { unsupported_reason(:archive) }
supports(:delete_ref) { unsupported_reason(:delete) }
end)
end

context "stub_supports" do
it "overrides supports from false to true" do
it "starts as false" do
expect(Post.supports?(:delete)).to be false

end
it "overrides supports from false to true" do
stub_supports(Post, :delete)

expect(Post.supports?(:delete)).to be true
expect(Post.unsupported_reason(:delete)).to be nil
expect(Post.new.supports?(:delete)).to be true
expect(Post.new.unsupported_reason(:delete)).to be nil
end

it "overrides supports (string) from false to true" do
expect(Post.supports?(:delete)).to be false

stub_supports(Post, "delete")

expect(Post.supports?(:delete)).to be true
expect(Post.new.supports?(:delete)).to be true
end

it "overrides supports references" do
stub_supports_all_others(Post)
stub_supports(Post, :delete)

expect(Post.new.supports?(:delete_ref)).to be true
expect(Post.new.unsupported_reason(:delete_ref)).to be nil
end
end

context "stub_supports_not" do
it "overrides supports from true to false" do
it "starts as true" do
expect(Post.supports?(:archive)).to be true
end

it "overrides supports from true to false" do
stub_supports_not(Post, :archive, "reasons")

expect(Post.supports?(:archive)).to be false
expect(Post.unsupported_reason(:archive)).to eq("reasons")
expect(Post.new.supports?(:archive)).to be false
expect(Post.new.unsupported_reason(:archive)).to eq("reasons")
end

it "ref starts sa true" do
expect(Post.supports?(:archive_ref)).to be true
end

it "overrides supports references" do
stub_supports_all_others(Post)
stub_supports_not(Post, :archive, "reasons")

expect(Post.new.supports?(:archive_ref)).to be false
expect(Post.new.unsupported_reason(:archive_ref)).to eq("reasons")
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

describe "#perf_capture_all_queue" do
context "with a provider not supporting metrics capture" do
before { stub_supports(ems, :metrics, :supported => false) }
before { stub_supports_not(ems, :metrics) }

it "doesn't queue any targets captures" do
ems.perf_capture_object.perf_capture_all_queue
Expand All @@ -17,7 +17,7 @@
end

context "with a provider supporting metrics capture" do
before { stub_supports(ems, :metrics, :supported => true) }
before { stub_supports(ems, :metrics) }

context "with no inventory" do
it "doesn't queue any targets captures" do
Expand Down
21 changes: 21 additions & 0 deletions spec/models/mixins/supports_feature_mixin_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,27 @@ def initialize(values = {})
expect(test_inst.supports?(:dynamic_feature)).to be_truthy
end

it "instance redirects work properly" do
test_class.instance_eval do
supports(:dynamic_attr) { unsupported_reason(:dynamic_operation) }
supports(:dynamic_operation) { "unsupported" }
end

child_class = define_model(nil, test_class)
child_class.instance_eval do
supports(:dynamic_operation) { nil }
end

# parent
expect(test_class.new.supports?(:dynamic_operation)).to be_falsey
expect(test_class.new.supports?(:dynamic_attr)).to be_falsey

# child
expect(child_class.new.supports?(:dynamic_operation)).to be_truthy
expect(child_class.new.supports?(:dynamic_attr)).to be_truthy
expect(child_class.new.unsupported_reason(:dynamic_attr)).to eq(nil)
end

context "with dynamic child class" do
let(:child_class) do
define_model(
Expand Down
20 changes: 5 additions & 15 deletions spec/support/supports_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,21 @@ module SupportsHelper
# when testing a model that receives multiple supports,
# put this down first to allow the other supports to work fine.
def stub_supports_all_others(model)
allow_any_instance_of(model).to receive(:supports?).and_call_original
allow(model.supports_features).to receive(:[]).and_call_original
end

# when testing requests, ensure the model supports a certain attribute
def stub_supports(model, feature = :update, supported: true)
model = model.class unless model.kind_of?(Class)
feature = feature.to_sym

receive_supports = receive(:supports?).with(feature).and_return(supported)
allow(model).to receive_supports
allow_any_instance_of(model).to receive_supports

allow(model).to receive(:types_supporting).with(feature).and_return([nil] + model.descendants.map(&:name))
allow(model.supports_features).to receive(:[]).with(feature).and_return(supported)
# TODO: verify this stub is necessary
allow(model).to receive(:types_supporting).with(feature).and_return([nil] + model.descendants.map(&:name)) if supported == true
end

def stub_supports_not(model, feature = :update, reason = nil)
model = model.class unless model.kind_of?(Class)
feature = feature.to_sym

stub_supports(model, feature, :supported => false)

reason ||= SupportsFeatureMixin.default_supports_reason
receive_reason = receive(:unsupported_reason).with(feature).and_return(reason)
allow(model).to(receive_reason)
allow_any_instance_of(model).to(receive_reason)
stub_supports(model, feature, :supported => (reason || false))
end
end
end
Expand Down

0 comments on commit bc0151b

Please sign in to comment.