Last active
January 18, 2023 17:43
-
-
Save searls/15bb76492b6f4a1eb568982fa966938c to your computer and use it in GitHub Desktop.
Here's an example of a refactor I did while test-driving a class using Ruby and Mocktail
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# The core problem here is mixing levels of abstraction | |
# This unit has two jobs. It delegates one and retains (most of) the | |
# other. That means it's doomed to be partially hands-off | |
# (simply trusting whatever result ProposesAgent sends back) | |
# and otherwise hopelessly in the weeds (requiring | |
# a test to care not only whether the save was valid but how/why) | |
module Proposals | |
class ProposesAgent | |
Result = Struct.new(:success?, :proposal, :error_messages, keyword_init: true) | |
def initialize | |
@assesses_proposability = AssessesProposability.new | |
@saves_proposal = SavesProposal.new | |
end | |
def propose(agent:, user:, params:) | |
proposability = @assesses_proposability.assess(agent) | |
if proposability.proposable? | |
proposal = agent.proposals.new(params.merge(user: user)) | |
if @saves_proposal.save(proposal) | |
Result.new(success?: true, proposal: proposal) | |
else | |
Result.new(success?: false, error_messages: proposal.errors.full_messages) | |
end | |
else | |
Result.new(success?: false, error_messages: [proposability.error_message]) | |
end | |
end | |
end | |
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
require "test_helper" | |
# This test is a pain but it gets REALLY bad at line 33 | |
module Proposals | |
class ProposesAgentTest < UnitTest | |
setup do | |
@assesses_proposability = Mocktail.of_next(AssessesProposability) | |
@saves_proposal = Mocktail.of_next(SavesProposal) | |
@subject = ProposesAgent.new | |
end | |
def test_is_proposable_and_save_succeeds | |
agent = New.new(Agent) | |
user = New.new(User) | |
stubs { @assesses_proposability.assess(agent) }.with { AssessesProposability::Result.new(proposable?: true) } | |
stubs { |m| @saves_proposal.save(m.is_a(Proposal)) }.with { true } | |
result = @subject.propose(agent: agent, user: user, params: {description: "Cool"}) | |
assert result.success? | |
assert_equal agent.proposals.first, result.proposal | |
assert_equal agent, result.proposal.agent | |
assert_equal user, result.proposal.user | |
assert_equal "Cool", result.proposal.description | |
assert_nil result.error_messages | |
end | |
def test_is_proposable_and_save_fails | |
agent = New.new(Agent) | |
user = New.new(User) | |
stubs { @assesses_proposability.assess(agent) }.with { AssessesProposability::Result.new(proposable?: true) } | |
# Ok, this design pain REALLY hurts. I now have to emulate the way | |
# Rails's #save method violates command-query separation inside the | |
# side effect of a stubbed response. This shall not stand. | |
stubs { |m| @saves_proposal.save(m.is_a(Proposal)) }.with { |call| | |
call.args.first.errors.add(:base, "Welp") | |
false | |
} | |
result = @subject.propose(agent: agent, user: user, params: {}) | |
refute result.success? | |
assert_equal ["Welp"], result.error_messages | |
end | |
def test_is_not_proposable | |
agent = New.new(Agent) | |
user = New.new(User) | |
stubs { @assesses_proposability.assess(agent) }.with { | |
AssessesProposability::Result.new(proposable?: false, problems: ["Hrm"]) | |
} | |
result = @subject.propose(agent: agent, user: user, params: {}) | |
refute result.success? | |
assert result.error_messages.first.include?("Hrm") | |
end | |
end | |
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# The solution was to have both AssessesProposability and SavesProposal | |
# return a value that quacks the same way (responding to `success?` and `error_messages`) | |
# so that their value could simply be returned without ProposesAgent having to | |
# mix the levels of abstraction. This only required refactoring AssessesProposability | |
# slightly and pushing down the `proposal.errors.full_messages` branch into `SavesProposal` | |
module Proposals | |
class ProposesAgent | |
def initialize | |
@assesses_proposability = AssessesProposability.new | |
@saves_proposal = SavesProposal.new | |
end | |
def propose(agent:, user:, params:) | |
proposability = @assesses_proposability.assess(agent) | |
if proposability.success? | |
@saves_proposal.save(agent.proposals.new(params.merge(user: user))) | |
else | |
proposability | |
end | |
end | |
end | |
end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
require "test_helper" | |
# This test now couldn't be easier. Literally just a main branch and a | |
# second that's dependent on the response of the first call | |
module Proposals | |
class ProposesAgentTest < UnitTest | |
setup do | |
@assesses_proposability = Mocktail.of_next(AssessesProposability) | |
@saves_proposal = Mocktail.of_next(SavesProposal) | |
@subject = ProposesAgent.new | |
end | |
def test_is_proposable_and_save_succeeds | |
agent = New.new(Agent) | |
user = New.new(User) | |
stubs { @assesses_proposability.assess(agent) }.with { ProposalResult.new(success?: true) } | |
stubs { |m| @saves_proposal.save(m.is_a(Proposal)) }.with { :some_save_result } | |
result = @subject.propose(agent: agent, user: user, params: {description: "Cool"}) | |
assert_equal :some_save_result, result | |
end | |
def test_is_not_proposable | |
agent = New.new(Agent) | |
user = New.new(User) | |
stubs { @assesses_proposability.assess(agent) }.with { ProposalResult.new(success?: false, error_messages: ["Welp"]) } | |
result = @subject.propose(agent: agent, user: user, params: {}) | |
refute result.success? | |
assert_equal ["Welp"], result.error_messages | |
end | |
end | |
end |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment