New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Easy to read matcher #12
Comments
Your example uses the |
|
I'd assert that a better practice would be instead: describe "#save!" do
context "with invalid data" do
subject { model = User.new(:email => 'a@invalid.com) }
lambda do
subject.save!
end.should change(User, :count).by(0)
end
end
end Setting integers in a spec is a code smell to me |
i think we can argue that point, i'd say that your lambda and it's chain is way too complex for many developers to quickly scan and grasp the intent. i understand how you want to restrict magic numbers, and brittle code, but the idea (imho) is that a test should be the simplest expression of intent, even if it's convoluted and non performant -- because this should be as good as documentation for your project. whilst i understand what your test does, i had to read it carefully to manually unroll it in my head -- and i'm not a compiler. |
yeah, I agree that the test should be the simplest expression of intent. There's a lot of value in that. Perhaps the lambda should be written as a custom matcher (or added rspec proper) to read more like:
|
I think you should utilize the new RSpec expectations. They read much nicer. This is how I have been testing some of my things with ActiveRecord but it's probably not the best solution, but it makes sense. expect { model.save! }.to raise_error Mongoid::Errors::DocumentNotFound
expect(collection.count).to eq(2) |
+1 to @myronmarston's comment. Elaborating on why ... collection.size.should == 2
collection.should have(2).items The latter was added to RSpec back in the day when there was no Cucumber and RSpec was trying to walk the line between readability for tech and non-tech folks. Today I'm pretty convinced that a) nearly no non-tech folk read rspec code, b) some non-tech folk do read the output, but that's all from the docstrings, and c) |
I agree with @myronmarston, thanks for pointing it out. Right now I've just removed the second example and I've left only the lambda/expect one. If you have some more good examples, I'll be to add them. |
Write your thoughts about the "Easy to read matcher" best practice.
The text was updated successfully, but these errors were encountered: