-
Machinist + RSpec matcher
Posted on March 20th, 2010 14 commentsMost of the time when I’m putting the skeleton of my Rails app together, I end up with a test like this:
After discovering Remarkable this week, I decided it was worth putting that into a macro matcher*. So now you can just say it { should work_with_machinist }. Here’s the code. Not tested. Enjoy:
* Thanks @David for pointing out it’s a matcher, not a macro
** Clarification: From reading the comments, I realize that this doesn’t seem useful unless you’ve used machinist. Machinist’s make method works on an ActiveRecord model, and works something like this: calls .new() with the fake params, calls save!, and then calls reload. If any of those fail, it raises an exception, so this effectively tests your blueprints for validation and callback sanity.
14 responses to “Machinist + RSpec matcher”
-
Devil’s Advocate again: does this really need to be tested? You’re testing your tests here, not the app. If you ever forget to install Machinist or create a blueprint, the NoMethodError you get wouldn’t be all that hard to figure out.
I can’t really argue with the idea that too many tests is better than not enough tests, but in this case I’m unsure what risk you’re mitigating or what time you’re saving.
-
Oh, don’t worry, I have no intention of writing any tests for it, ever, figuring along the lines you did that either the tests will blow up once I include it or it’ll work.
-
@Terry – I think @Stephen was questioning the existence of this matcher (it’s a matcher btw, macros are a bit different). Why specify that your models respond to make() when you’ll certainly get a NoMethodError on make() anyway if they don’t?
Also – check out the matcher DSL. You can tidy that up a bit like this:
-
Hmm. I tried to embed a gist but it seems to have gotten filtered out.
http://gist.github.com/339335.js
-
I tend to agree with Stephen that you’re not testing much here and there’s not much added value.
Couln’t you test that the activerecord object created with .make() did not generate any validation errors?
-
I use the works_with_machinist more than just to make sure there is a blueprint for something. It also tells you, right off the bat, if your blueprint isn’t aligning to the validations in your model.
While you can (reasonably) argue that the other tests will fail as a result, I like to have a test that tells me right away, “hey you screwed up your blueprints.”
Especially as blueprints get more complicated, I like having something that just runs it to make sure things are sane before getting into some of the more application-relevant tests.
-
@David
Looking at the gist, it’s not testing quite the same thing. I care more than that there’s just a make method, I want to make sure it works up with validations and callbacks. -
Oh nice! I hadn’t realized that Machinist would throw an exception when make() fails to generate a valid object. I can see the value of such matcher then. I need to port this to Shoulda!
-
Stephen Eley March 21st, 2010 at 18:21
Ahh, okay, I get it now. What you’re saying is that you’re not testing Machinist, you’re testing your assumptions about the model (which are reflected in the blueprint.) That makes sense, and I usually do it too for anything non-trivial.
I still think you might have made it a bit too complicated though. I usually assert that expectation with either:
Model.make_unsaved.should be_valid
or:
Model.make.should be_a(Model)
Most often the former, as the way I usually test validation rules is to set up before(:each) block with something like:
before(:each) do
@this = Model.make_unsaved
end…and then torture the attributes of @this in various ways, verifying each time that the model is no longer valid. An initial “@this.should be_valid” is then sort of necessary to isolate the variables.
Back to the original issue, though: even if the test purpose is sound, making a matcher for it still seems like unnecessary complexity. Your initial example was one line of code. You wrote 20 lines of code, which will presumably be included in a separate file in every future project, so that you could simplify that one line of code into…one line of code.
Granted, the new line of code is prettier than the old. If that’s enough of a win for you to make it worth it, rock on. >8-> (Sometimes it is for me too.)
-
@Stephen
3 lines of code, and having to type a description to keep things expressive. I think specs should be expressive, and not “it { model.make_unsaved.should be_valid }*”So, yes, copying this once to my new projects and keeping everything 1 line of code from there on out is worth it (Callbacks can definitely have problems, and if those show up in your blueprints, your test hasn’t revealed that — the lambda and should_not raise_error gets to be tedious enough and hard enough to decipher that it’s worth abstracting, I think).
-
Would it make more sense to test if the Machinist Library is included in ActiveRecord ?
-
@Brian
No. The test is checking that the blueprints for the model work with validations and callbacks.
-
Stephen Eley March 20th, 2010 at 20:13