In TDD and BDD we write small, focused, technical tests, sometimes called micro-tests. One of the core ideas is that these tests should run fast, really fast—each one measured in milliseconds. If you’re writing plain Ruby code, that’s pretty easy to accomplish. However, when you’re using something like Rails or Merb and DataMapper or ActiveRecord, it can get a bit more challenging.
Why do we end up with model tests/specs that run slowly? In looking at how we write and test our code, let’s ignore controllers and views, and focus on models. After all, models are where you put your business logic.
There can be several reasons for a slow test suite: database access, unnecessary objects, a massive setup that takes a while to load—just to name a few. This time though, I want us to focus on a new reason, one that’s a result of operating in the context of ActiveRecord or DataMapper. One that tricks us into thinking we’re doing well. One that exposes a few somewhat major flaws in the design of AR and DM.
With ActiveRecord and DataMapper, each model class is responsible for its own persistence, hence DHH using the name “Active Record.” It was inspired by Fowler’s writing on the pattern in Patterns of Enterprise Application Architecture and is the enabler of our model addiction.
OverActive Record
Used properly, the active record pattern is great for persisting data. These objects are great at handling their own persistence. Accessors, associations, and validation make a good wrapper around the transactional nature of a database. This is fine when the model is a simple data object, but we all run into problems when we start adding other behavior to the model class.
Weighted Companion Cake: The cube was the cake!
Recall the Single Responsibility Principle: “There should never be more than one reason for a class to change.” As we commonly use them, ActiveRecord and DataMapper1 classes almost always violate this principle. There are usually at least two responsibilities handled by every AR/DM model: persistence and business logic. Carrying around that persistence behavior, and all the dependencies that go along with it, is what bloats and slows down our specs.
There have been various attempts at dealing with this: in memory databases, stubbing parts of the DM/AR frameworks, etc. None of these are ideal. Either they don’t speed things up as much as we’d like, they’re awkward, or they bulk up the specs an objectionable amount.
Is There a Better Way? Why, Yes! Yes There Is.
DM/AR lets you define a model’s properties, its persistent parts, and generates the associated accessors and mutators. All access to the persistent properties is handled through these accessors and mutators. This gives us a perfect seam along which to split the class. All of the persistence-dependent functionality is on one side of this seam, and the business logic of the model is on the other.
We need a good saving throw against this frosting.
Pretend… It’s a Cake.
Yes, you read that correctly. Keep on reading—it makes sense!
Each of our models is like a frosted cake. We have the nice fluffy cake batter, baked with our properties, validations, and persistence. Then we frost the outside with our business logic. We can take a simple ActiveRecord cake and make it into almost anything, with a good amount of frosting.
Now herein lies the problem: the frosting starts to take on a life of its own. Before we know it we’re baking a few 100 ActiveRecord cakes each time we need to test the frosting behavior… There are two really good ways to get around this: cupcakes and cardboard.
Strategy 1: Oh Look! Cupcakes!
When we build a web application with ActiveRecord or DataMapper, it’s really hard to think outside the persistence box. If something is persisted on that model, or if it uses something on that model, we automatically add the code to the model. This leads quickly to a gigantic cake that does everything.
This is especially bad with primary models like User. The poor bloated User gets tons of code stuffed into it because our app is usually focused around users. Users do things in our app. Users get affected by things. They control the app and most things belong to the user in some way. But this doesn’t mean our User class should be 1000 lines.
The solution is to break out chunks of code into their own classes. There are many common patterns that we can find in our classes: Factory, Provider, Policy, Strategy, and plenty of types that are custom to each domain (Cake Decorator?). Let’s lump these all into something that we’ll call Cupcake Classes.
At Engine Yard, one of the places we found an opportunity for a Cupcake was our Deployment initialize. The method had to do a lot of data mutating before it could create an instance. Blank strings needed to become nil, string representations of a type needed to be converted to the actual type, and defaults needed to be applied to fill out the incoming data. The initialize method was a ridiculous 50 lines long. This needed to become its own class.
The result is a really clean initialize:
def initialize(params={})
merger = Deployment::DefaultsMerger.new(params)
merger.each_attribute { |att, val| send("#{att}=", val) }
end
…and a class that knows all about how to merge incoming params and defaults. It has a clear API, and it’s easily testable. We know what comes in and what comes out and we don’t have to create a database record every time we test it.
class Deployment
class DefaultsMerger
def initialize(params)
@attributes = merge_with_defaults(params)
scrub_blank 0, :data_volume_size, :app_server_count
scrub_blank nil, :db_volume_id, :data_volume_id
coerce_value(:instance_size, :db_instance_size) do |val|
Instance::Size.coerce(val)
end
# ...
end
def each_attribute
@attributes.each do |k,v|
yield k, safe_to_i(v)
end
end
private
def defaults
Mash.new({
:data_volume_size => Volume::DEFAULT_SIZE,
:app_server_count => 1,
# ...
})
end
# ...
end
end
Cupcake Coding Rule of Thumb: Messy Code Makes for Good Cupcakes!
When you want to clean up your models, look for messy code. Long methods or sets of utility methods (methods that barely, if at all, touch instance state) are just begging to be cut up into smaller classes.
What tipped us off was that the initialize method was long, nasty, and really had nothing to do with the class it was in. Mostly, it dealt with params, a hash. Almost every line in the old method did hash operations, not operations dealing with the actual model.
We clearly needed a class that would know how to work with merging our defaults into an incoming hash. The resulting class would also improve our real test coverage. It’s much easier to test a bunch of branches when they don’t have to pass through the model life cycle.
describe Deployment::DefaultsMerger do
describe_attribute :data_volume_size do
it_defaults_to Volume::DEFAULT_SIZE
it_converts ["", nil, 0, "0"], :to => 0
it_converts [123, "123"], :to => 123
end
describe_attribute :app_server_count do
it_defaults_to 1
it_converts ["", nil, 0, "0"], :to => 0
it_converts [2, "2"], :to => 2
end
# ...
end
With a focused class, it’s easy to write focused specs. With a few custom methods in your specs, you have beautifully concise descriptions of behavior to match your simple code.
Looks yummy!
Strategy 2: Cardboard Cake
The next way to improve your models and your testing is to extract the model’s behavior from the persistence. The idea is to test the frosting without having to bake a cake every time. Our example class after the behavior has been extracted:
class Oven
include DataMapper::Resource
include Oven::Behavior
## Properties
property :id, Serial
property :oven_type, String
property :temp_setting, String
has 1, :heating_element
has 1, :latch
## ...
end
Here’s the frosting, or behavior, that has been extracted from it:
class Oven
module Behavior
def check_lock
latch.engage! if dangerous_temperature?
end
def dangerous_temperature?
Oven::Temperatures.dangerous? temp_setting
end
def turn_on(temp = 350) # ...
def turn_off # ...
def preheating? # ...
end
end
Once this is done, you can write specs that focus on the logic in isolation. Instead of testing every aspect of behavior on top of the persistence framework (slow), you can isolate the behavior (fast). Sort of like making the cake out of cardboard so you can test our frosting without the baking time (See? Told you it would make sense!).
How Do We Achieve This Isolation? Mock Object Trickery, But of Course!
Let’s create a mock to stand in for the model object being tested. Let’s call these Cake Mocks. Then, we’ll extend the mock object with with the Behavior we’re testing.
@oven = mock("Oven").extend(Oven::Behavior)
It still acts like an oven, but instead we’ve frosted the outside of a cardboard cake with the same business logic that we would add to a real ActiveRecord object. The trick is to have the mock object extend the module that contains the business logic. The result is an object containing the behavior of the model but not its persistence.
Now we can stub accessors on the mock (but only those involved in the example) and set expectations on the mutators. Then the example can call methods on the mock oven as if it was a real Oven instance. Those calls invoke the actual business logic methods that we want to test. When those methods access the internal behavior of our persistence layer, our mock answers with the methods we stubbed. The mock merely fills in the center of the cake.
describe Oven::Behavior do
describe "preheating" do
before(:each) do
@latch = mock("latch")
@oven = mock("Oven", :latch => @latch).extend(Oven::Behavior)
end
it "isn't locked when temperature setting is less than 500" do
@oven.stub!(:temp_setting).and_return(350)
@latch.should_receive(:engage!).never
@oven.check_lock
end
it "isn't locked when temperature setting is 500" do
@oven.stub!(:temp_setting).and_return(500)
@latch.should_receive(:engage!).never
@oven.check_lock
end
it "is locked when temperature setting is greater than 500" do
@oven.stub!(:temp_setting).and_return(505)
@latch.should_receive(:engage!).once
@oven.check_lock
end
end
end
This cake mock style encourages another good testing practice: integration testing. When the model is completely mixed up with behavior, the need for integration testing is easier to ignore, but no less important. With separated behavior and persistence we can write simple integration tests that make sure the seam between the cake and the frosting is still intact.
Give Your Models Some Attention
When looking at models this way, whether you’re breaking them up in to cupcakes or separating the frosting, it’s easier to see the difference between behavior and persistence. If the logic can stand on its own, make a new class. If it’s more like an extension of the model, make a module and mix it back in. Allow ActiveRecord and DataMapper models to take care of what they do best without all the extra weight.
Take a few minutes and look at the models in your project. If the classes weren’t backed by the database, would you still have all that stuff jammed into one place? Are your models more complex than they need to be? If so… do something about it!
[1] DataMapper may be named after Fowler’s DataMapper pattern, but that’s where the similarity ends. “DataMapper” is little more than a variation of ActiveRecord. The objects are responsible for their own persistence, while the DataMapper pattern uses a separate object (the mapper) that manages persistence: “A layer of Mappers (473) that moves data between objects and a database while keeping them independent of each other and the mapper itself.”
Loading ...
Watch a Live Demo of Engine Yard AppCloud
The Engine Yard Newsletter
Great article! There's some great prior thinking around domain patterns, and it's easy to cozy up with the "rails blinders" on, eventually winding for worse. How do you deal with validations in this light? It seems like they could fall somewhere in between persistence (not null ala validates_presense_of) and behavior (validates_inclusion/exclusion_of, etc.) Future AR stuff looks promising in that validations can be utilized outside of ActiveRecord::Base, but would be interested to hear your thoughts on this specifically.
Validations deal directly with the integrity of the data going into your database. I think it's definitely something that stays with the persistence layer no matter how you organize your models (at least with AR and DM)
I agree. But that still violates the Law of Demeter. Isn't 'persisting' data and 'validating' data two reasons to change?
This is nitpick, but I find it a fascinating one. The lines we draw in code organization can sometimes be quite clear. But often, we end up with code that sits in a grey area.
Another principle I apply is Least Surprise. Where would other developers expect this code to be. AR validations should therefore go in the Model.
This was a great post Martin. I hope everything is going well for you in San Francisco!
Validation is necessary whether or not an object is persisted. In Rails, we allow invalid objects primarily for UI purposes: we want to be able to use the object to show the user what was entered that was incorrect.
But in another environment, invalid objects are simply not allowed to be created. This is what the "constructor" is for in languages that have this feature.
In either case, it is necessary to know or guarantee that an in-memory object can be used in the Model layer of the application – even if it vanishes into thin air without being persisted.
Why not just use `@oven = Oven.new`? It has all the behaviours, and then you don't need Oven::Behavior at all.
I think the point of it is to prevent your tests from having to deal with the entire unused ActiveRecord bits.
I assume you only read "none" of the article. By using `Oven.new`, you get all of the persistence behavior that the object has. Considering that in most libraries like Active Record and Datamapper the persistence is very well tested and should not be retested. Therefore it makes sense to split your specific behavior for that object into another module, and then extend a mock object with it. You completely avoid a heavy and well tested portion of code.
"Considering that in most libraries like Active Record and Datamapper the persistence is very well tested and should not be retested. "
I've definitely caught regressions in AR because my "unit" tests hit something by dumb luck. I usually don't intend to test the simplest part of AR persistence, but sometimes it is worth testing that everything interacts like you expect, at the very least (such as callbacks working properly across complex hmt/STI/polymorphic combos).
Yes. It is worthwhile to do both unit tests and functional tests.
And if you are doing rails standard AR Models, unlike this article suggests, your "unit" tests are actually more model tests – both unit and functional.
We can quibble about terms if you'd like, I figured my scare quotes were obvious enough, but all I was pointing out was that even if "heavy and well tested" is not overly stated, "should not be retested" certainly is. Was my quotation and reply unclear in some way that I could clarify?
This is a valid point and you're not the first person to bring it up. This simple case doesn't really make in seem that useful. Not every method on Oven should be extracted to a module necessarily. I would separate a large class into many cupcakes before I attacked it with the cardboard strategy ;)
A lot of times breaking some of the methods out this way helps your to group them before you make a class out of it. There's very minimal overhead to your existing tests while you work on the extraction and the extracted module often morphs itself into its own class. On 600 line models this can be a decent starting strategy that leads to better "cupcaking."
Wonderful article! The analogies are quite fun. You might like my blog I wrote a while back, with a similar message: http://karwin.blogspot.com/2008/05/activerecord-d...
Dude, congrats on your first article!
And it's a good one, I learned a lot (not surprising) :)
"This cake mock style encourages another good testing practice: integration testing. When the model is completely mixed up with behavior, the need for integration testing is easier to ignore, but no less important. With separated behavior and persistence we can write simple integration tests that make sure the seam between the cake and the frosting is still intact."
I sometimes find it easy to get lost in all of the different types of testing unit/functional/integration/etc. I know sometimes when people talk about integration tests, they're talking about driving the scenario from the browser. I'm curious if you could clarify what you mean by integration testing with some code samples?
Thanks, I really enjoyed this article. I've been thinking about this a lot lately as our models have grown more and more over time.
Can you expand on this a little more? This post really jived with me, but the biggest conceptual roadblock I've been facing is the where exactly to draw the integration/unit line. As I understand it, drawn traditionally, integration tests blackbox your app from the highest possible level (e.g. Selenium), while the unit tests mock or stub everything outside the tested method. Sounds great, except in order to achieve good coverage, you've now written two entire suites that cover the same ground twice. Not very DRY, nor very cost-effective. In frustration, I've simply opted to go nearly 100% integration / 0% unit, giving me what I perceive to be the best bang for buck. But now my tests are slower than molasses. I feel like I'm not understanding something fundamental here.
My understanding of integration testing is testing anything that needs to work together without any level of mocking. On server provisioning this can be extremely slow (10 minutes per test or more). This is where the split becomes important.
Let's I have some complex logic in an app around something that takes 10 minutes per test. Next, say the 10 minute part is communicated with by an API that just calls 1 method with a few arguments and the part you care about is just what arguments are passed in depending on your logic. This is where I would write a spec that verifies the input into that method using a mock, and then write and "integration test" that runs the slow part in a few of the most different ways (i.e. the pass condition, fail condition, and anything that does something particularly important.
What happens with top level black box testing (what I would probably call acceptance tests) is that you end up with a bunch of processing just to check a model level validation. Integration specs that check 30+ different input formats for one validation from outside the controller are asking for horribly slow, and therefore, most likely, infrequently tested code.
it is good one . i learned a lot
DUI Attorney
Excellent article! It's very important to remember that business logic and persistence are tow separate concerns.
BTW, having had reason to dig around in DM's internals a lot lately, I'm not sure why you say it has no Mapper. DM does a great job of separating resources from storage. It has an internal identity map which is separate from the resource objects. And it separates storage out into "Repositories" backed by Adapters – a given Resource (like a User) can be persisted to one repository or many, or even copied from one repository to another, something which isn't possible with AR last I checked. And because the repositories are independent of the models, repository A might store a given Model completely differently than repository B does – using different field names, for instance. That's a pretty effective separation of model from persistence if you ask me.
You're right about the DataMapper internals following the pattern fairly well. It makes DataMapper a bit easier to use and I think it's a great library.
Still, the way that most people use DM really has nothing to do with the pattern. For the end user the persistence layer is accessed through "self." If you interfaced with the adapter directly to use the object, you would be using the DataMapper pattern, but almost every programmer using DataMapper is using it like ActiveRecord. The way I see it, if self is how you access your persistence, then you're on the persistence layer.
This is not to say you should interface with the adapter directly. That would be rather insane since you don't control that interface and so you're asking for trouble. It's just that you shouldn't assume that because the library you're using separates its concerns, that it necessarily means that *you* are separating your concerns.
Do I smell a POJO movement coming on for ruby? PORO just doesn't have the same bouncy name. Unless you roll the "r".
At OtherInbox, we actually have a directory named PORO, for just these sort of classes; it has grown huge.
This does seem to lead to an organizational concern. How do you store all of these classes in a way that is intuitive to find what you are looking for and create new ones?
Hi Aaron, sorry i didn't notice your comment until just now.
Do you guys namespace classes with their owner classes?
I.E.
class Pixar
class Up
end
end
- app
– models
– disney.rb
– pixar
– up.rb
– toystory.rb
– pixar.rb
Here's to hoping this comment's formatting doesn't explode.
(I'm a big fan of other inbox btw)
Well the indentations didn't work…but oh well.
Great post! Thanks.
Great article! There's some great prior thinking around domain patterns, and it's easy to cozy up with the "rails blinders" on, eventually winding for worse.
Dui Attorney Temecula
When extracting behavior into a module of an ActiveRecord class as you do in strategy #2, do typically define that module in a different file than the one you are defining the ActiveRecord class in? I'd like to extract the module defining the behavior into a separate file, but I'm struggling with the best way to do so in the context of a rails app while still making the use of autoloading and Textmate's shortcuts that let me jump back and forth between a spec and the subject code. Maybe I should just forget about trying to make use of the default autoloading behavior. Do you have any suggestions?