push.cx
Peter Bhat Harkins
Integration as Composition
I’m puzzling over the design for a worker and would appreciate your comments on it. I started with the pain of an ugly test, made an interesting refactoring, and decided to drop the test entirely, but I’m not at all sure this is the right decision.
In my mailing list archive Chibrary, I want to sum up the number of threads and messages in a month to present on archive pages. The MonthCountWorker takes a Sym, a unique identifier for a list’s slug + year + month, fetchs the threads for that month, sums them, and stores the sum. The code is trivial, right?:
class MonthCountWorker include Sidekiq::Worker
def perform serialized_sym sym = SymRepo.deserialize serialized_sym month = ThreadRepo.month sym mc = MonthCount.from month MonthCountRepo.new(mc).store end end
But the test I started writing is awful! The SymRepo.deserialize and MonthCount.from calls are pure queries that work in-memory. Following Metz’s excellent guidelines I want to write tests to check their return values. ThreadRepo.month is a nullipotent query that hits the database, so I’d like to stub it out. And finally MonthCountRepo is a command to store the result; I want to set an expectation that it happened.
Imagine writing tests for this. Everything has to stub out the database query, and to confirm that sym, month, and mc have the right values I’d have to set expectations on the next thing in line. I don’t truly want an expectation, but this procedural code has no seams to get at the values flowing through it. There are no boundaries.
I decided to introduce method boundaries and suddenly perform looks like functional composition (and Mike Busch mentioned it reminds him of literate programming):
class MonthCountWorker include Sidekiq::Worker
def perform serialized_sym store monthcount of_month identified_as serialized_sym end
def identified_as serialized_sym SymRepo.deserialize serialized_sym end
def of_month sym ThreadRepo.month(sym) end
def monthcount month MonthCount.from month end
def store mc MonthCountRepo.new(mc).store end end
Yes, that’s valid Ruby, I checked. The fact that it was such unfamiliar style I had to check tells me I’m either doing something awful or something awesome.
After reflecting on it a moment, I recognize it as function composition, though Ruby doesn’t have first-class support for it.
And now all the little pieces are isolated. The perform method is pure, pointfree composition; I don’t know if I’d test it at all. The individual methods can be easily tested with whatever technique is appropriate.
What tests are appropriate, though? All these methods are already covered over in the unit tests for each class; any test would be redundant, wasted effort.
The only new code here is the composition. This is high-level integration; the only appropriate test would be an integration test that runs against a prepopulated database and queries that the right results are stored back to it.
Or I could take the route Bernhardt mentions in his Boundaries talk and not bother testing it at all. There’s only one way these parts can be composed (not that Ruby has the compile-time type checking to enforce it...), and there really isn’t much value in a test that I would never expect to fail. I want tests to return the investment of time I put into writing and maintaining them. This composition is unlikely to fail, can only cause cosmetic issues, and would be straightforward to debug because it has many seams and no conditionals.
Ultimately my answer to testing this was to rewrite it to a compositional style and not bother testing it. If I get breaks and spend more than a minute or two fixing it in the next few months, I’ll know I chose the wrong answer. What’s your answer?