G'day:
This is another "low-hanging fruit" kinda exercise (see also previous article: "A question about the overhead of OOP in CFML"). I'm extracting this one from the conversation in the CFML Slack channel so that it doesn't get lost. I have a growing concern about how Slack is a bit of a content-black-hole, and am cooling to the notion of helping people there. Especially when my answer is a fairly generic one. The question is a CFML-based one; the answer is just a general testing-strategy one. I say this so people are less likely to stop reading because it's about CFML ;-) Anyhoo: time for some copy and paste.
The question is a simple one:
Just dropping it in here. Unit testing DAO object using qb. How would you guys go about it or why not? Go!
I say "simple". I mean short and on-point.
Here's the rest of the thread. I've tidied up the English in some places, but have not changed any detail of what was said. I will prefix this saying I don't know "qb" from a bar of soap; it's some sort of DBAL. For the purposes of this, how it works doesn't really matter.
The participants here are myself, Kevin and Samuel Knowlton.
It depends on how you design this part of your application.
For me the interface between the application and the storage tier is via a Repository class, which talks to the higher part of the domain in object and collections thereof: no trace of any persistence DSL at all (no SQLish language, for example).
For the purposes of testing, I abstract the actual storage-tier communications: in CFML this would be queryExecute calls, or calls to an external library like qb. I fashion these as DAOs if they contain actual SQL statements, or an Adapter if it was something like qb.
So the repo has all the testable logic in it, the DAO or Adapter simply takes values, calls the persistence implementation, and returns whatever the persistence call returns. The repo converts that lot to/from domain-ready stuff for the application to use.
I'll just add an example of this for the blog version of this conversation:
selectFooById(id) {
return queryExecute("
SELECT col1, col2, col3, etc
FROM some_table
WHERE id = :id
", {id=id})
}
It's important to note that this method does nothing other than abstract the queryExecute call away from any application logic in the method that calls this. No manipulation of inputs; no remapping of outputs. Both of those are the job of the repository method that calls this DAO method. This DAO method only exists to remove the external call from the repo code. That's it.
To that end, there is no unit testable logic in the DAO. But I will do an integration test on it. For a CFML example if I call a method on the DAO, then I do get the kind of query I'd expect (correct columns).
I'd unit test the repo logic along the lines of the mapping to/from the domain/storage is correct, and any transformation logic is correct (converting datetime objects to just date-only strings and the like). I would test that if a DAO call returned a three-row query, then the repo returns a three-object collection, with the expected values.
NB: Repos don't have business logic; they only have mapping / transformation logic.
An initial task here might be to look at the DAO code, and even if it rolls-in transformation/mapping logic and persistence-tier calls, at the very least it doesn't do both in the same method.
One thing to be careful to not do is to re-test qb. Assume it's doing its job correctly, and just test what your code does with the data qb gives you.
In that case my current tests are not correct at all… They felt wrong from the start, but I was struggling with how to test the DAO layer (which is abstracted through an interface). My DAO layer at this point does have a hard reference to / dependency on qb, which in that case violates some OO principles… This is a brainbreaker
Ah yes everything I mentioned above is predicated on using DI and an IoC container which should really be the baseline for any application designed with testing in mind (which should be every application).
(NB: DI is the baseline; using an IoC container to manage the DI is a very-nice-to-have, but is not essential).
You can "fake it til you make it" with the DI, by extracting hard references to qb object creation into methods that solely do that. You can then mock-out those methods in your tests so you can return a testable object from it.
So instead of this:
// MyRepository
component {
getObjectCollection() {
qb = new QB()
raw = qb.getStuff()
collection = raw.map(() => {
// convert to domain model objects. this is what you want to test
})
return collection
}
}
One has this:
// MyRepository
component {
getObjectCollection() {
qb = getQb()
raw = qb.getStuff()
collection = raw.map(() => {
// convert to domain model objects. this is what you want to test
})
return collection
}
private getQb() {
return new QB()
}
}
And can test like this:
it("maps the data correctly", () => {
sut = new MyRepository()
mockbox.prepareMock(sut) // sut is still a MyRepository, but can selectively mock methods
rawData = [/* known values that exercise the logic in the mapper */]
sut.$("getQb").$results(rawData)
collection = sut.getObjectCollection()
expectedCollection = [/* expected values based on known values */]
expect(collection).toBe(expectedCollection)
})
You could also extract the mapping logic from the fetching logic, and test the mapping logic directly. That's getting a bit bitty for my liking though. However do what you need to do to be able to separate the logic into something testable, away from code that isn't testable. It's always doable with a bit of refactoring, even if the refactoring isn't perfect.
[…] I already have a getQueryBuilder method in the DAO object. So that's a good starting point. How would you test update / create / deletion?
Two sets of tests:
Integration tests
Test that your app integrates with QB and the DB. Call the actual live method, hit the DB, and test the results.
So…
- create a record using your app. Then use queryExecute to get the values back out, and test it's what you expect to have been saved.
- Insert a row into the DB (either via your create mechanism, or via queryExecute). Update it via your app. Use queryExecute to get the values back out, and test it's what you expect to have been updated.
- Insert a row into the DB (either via your create mechanism, or via queryExecute). Delete it via your app. Use queryExecute to try to get the values back out, and make sure they ain't there.
Unit tests
- When you call your app's create method with [given inputs] then those inputs (or equivalents, if you need to transform them) are the ones passed to the expected method of the (mocked) object returned from your mocked getQueryBuilder call.
- Same with update.
- Same with delete.
it("deletes the data from storage", () => {
testId = 17
sut = new MyRepository()
mockbox.prepareMock(sut)
mockedQb = createMock("QB")
expectedReturnFromDelete = "I dunno what QB returns; but use some predictable value"
mockedQb.$("delete").$args(testId).$results(expectedReturnFromDelete)
sut.$("getQueryBuilder").$results(mockedQb)
result = sut.delete(testId)
expect(result).toBe(expectedReturnFromDelete) // it won't have been returned unless testId was passed to QB.delete
})
That's a very simplistic test, obvs.
Oh: run all the integration tests in a transaction that you rollback once done.
Here the unit tests don't hit the actual storage, so are easier to write, maintain, and quick to run in your red/green/refactor cycle. You run these all the time when doing development work. The integration tests are fiddly and slow, so you only run those when you make changes to that tier of the app (DB schema changes that impact the app, generally; and then when putting in a pull request, merging it, and doing a build). It is vital the code being integration tested does not have any logic in it. Because then you need to unit test it, which defeats the purpose of separating-out yer concerns.
[…]To add our two cents: we use QB and Quick all the time in integration tests to make sure that objects are created, updated, or deleted the way we want. Sometimes we will use cfmigrations to pre-populate things so we are not doing a whole create/update/destroy cycle on each test run (which can be awesome, but also slow)
[Agreed]
A lot of information to take in. I'll take my time to read this through. Thank you @Adam Cameron and @sknowlton for taking your time to answer my questions!
NP. And ping away with any questions / need for clarification.
[a few days pass]
[…]Thanks to your input I was able to refactor the "dao" layer through test-driven design!
I do have a general testing question though. Let's assume I have method foo in a service with parameters x, y, z with the following pseudocode:
function foo( x, y, z ) {
/* does something */
var args = ...
/* validate, throw, etc... */
return bar(
argumentCollection = args
);
}
Should the unit test mock out the bar function and assert the args parameter(s) and / or throw scenarios or actually assert the return value of the foo function. Or is the latter case more of a integration test rather than a unit test?
Sorry if this is a noob question, been trying to wrap my head around this…
A handy thing to do here is to frame the tests as testing the requirement, not testing the code.
Based on your sample pseudo code, then you have a requirement which is "it does the foothing so that barthing can happen". I have purposely not used the method names there directly; those are implementation detail, but should describe the action taking place. It could be "it converts the scores into roman numerals": in your example "it converts the scores" is the call to foo, and "into roman numerals" is the call to bar.
If you frame it like that, your tests will be testing all the variations of foothing and barthing - on variation per test, eg:
- x, y, z are valid and you get the expected result (happy path)
- x is bung, expected exception is thrown
- y is bung [etc]
- z is bung [etc]
- say there's a conditional within bar… one test for each true / false part of that. The true variant might be part of that initial happy path test though.
This is how I read the pseudo code, given that bar looks like another method in the same class - probably a private method that is part of some refactoring to keep foo streamlined.
If your pseudocode was return dao.bar(argumentCollection = args), then I'd say that the DAO is a boundary of your application (eg, it is an adapter to a queryExecute call), and in this case you would mock-out dao.bar, and use spying to check that it received the correct arguments, based on the logic of foo. For example foo might multiple x by two... make sure bar receives 2x, etc
Does that cover it / make sense? Am off to a(~nother) meeting now, so had to be quick there. I'll check back in later on.
It covers my question, Adam, thanks!
One thing I didn't have time to say... I used a DAO in that example specifically to keep my answer short. A DAO is by definition a boundary.
If it was something else like a RomanNumeralWriterService and it was part of your own application, just not internal to the class the method you're testing is in... it's not so clear cut as to whether to mock or use a live object.
If RomanNumeralWriterService is easy to create (say its constructor take no or very simple arguments), and doesn't itself reach outside your application... then no reason to mock it: let it do its job. If however it's got a bunch of its own dependencies and config and will take a chunk of your test just to create it, or if it internally does its own logging or DB writing or reaching out to another service, or it's sloooow, then you mock it out. You don't want your tests messing around too much with dependencies that are not part of what yer testing, but it's not black or white whether to mock; it's a judgement call.
Yeah I was struggling with the thing you mention in your last paragraph. Service A has Service B as a dependency which itself has a lot of other dependencies (logging, dao, …). The setup for the test takes ages while the test method is 1 line of code :/.
Yeah, mock it out.
Make sure whatever you mock-out is well-test-covered as well in its own right, but it's not the job of these tests to faff around with that stuff.
That was it. I think that was a reasonably handy discussion, so was worth preserving here. YMMV of course: lemme know.
Righto.
--
Adam
