Last active
December 16, 2015 11:59
-
-
Save mipearson/5431613 to your computer and use it in GitHub Desktop.
Testing - I think I'm doing it wrong. 25 lines of test for very little logic. Fork & tell me how you'd do it instead.
This file contains 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
class LineItem < ActiveRecord::Base | |
belongs_to :invoice | |
def allocated? | |
status == 'allocated' | |
end | |
def refundable? | |
allocated? && invoice.refundable? | |
end | |
def cancellable? | |
allocated? && invoice.cancellable? | |
end | |
end |
This file contains 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
describe LineItem | |
describe "permissions" do | |
subject {LineItem.new(status: status, invoice: Invoice.new)} | |
context "with an invoice that allows for refunds and cancellations" do | |
before { subject.invoice.stub!(refundable?: true, cancellable?: true)} | |
context "and allocated" do | |
let(:status) {'allocated'} | |
it { should be_refundable } | |
it { should be_cancellable } | |
end | |
context "and not allocated" do | |
let(:status) {'cancelled'} | |
it { should_not be_refundable } | |
it { should_not be_cancellable } | |
end | |
end | |
context "with an invoice that doesn't allow for refunds and cancellations" do | |
before { subject.invoice.stub!(refundable?: false, cancellable?: false)} | |
let(:status) {'allocated'} | |
it { should_not be_refundable } | |
it { should_not be_cancellable } | |
end | |
end | |
end |
Perhaps this is an incremental improvement?
https://gist.github.com/pda/5431874/5143067b3baff5b929f70048626f61721cb2864f
It pushes the slight complexity of setting up the Invoice
up to the initial subject
declaration, allowing the contexts to declare their parameters as simply as possible.
Also, a bit of (arguable) whitespace improvement to make things clearer.
Of course, this isn't a fundmental improvement. I think it's one of those things which requires a lot of assertions to test all cases. Reducing the amount of code used to express those assertions would be adding indirection and complexity.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Yeah, I don't see any other way to do it if you want to test all variations. One bool expression requires 2 tests. Two bool expressions require four. I do not see any way to make the tests shorter and still testing all boolean values.