+972 52-548-6969
Cut the abstractions by putting test hooks
I have been hearing about testable code for a long time now. It looks somewhat like this, although I had to cut on the number of interfaces along the way.
We go through a lot of contortions to be able to do something fairly simple, avoid hitting a data store in our tests.
This is actually inaccurate, we are putting in a lot of effort into being able to do that without changing production code. There are even a lot of explanations how testable code is decoupled, and easy to change, etc.
In my experience, one common problem is that we put in too much abstraction in our code. Sometimes it actually serve a purpose, but in many cases, it is just something that we do to enable testing. But we still pay the hit in the design complexity anyway.
We can throw all of that away, and keep only what we need to run production code, but that would mean that we would have harder time with the tests. But we can resolve the issue very easily by making my infrastructure aware of testing, such as this example:
But now your production code was changed by tests?!
Yes, it was, so? I never really got the problem people had with this, but at this day and age, putting in the hooks to make testing easier just make sense. Yes, you can go with the “let us add abstractions until we can do it”, but it is much cheaper and faster to go with this approach.
Moreover, notice that this is part of the infrastructure code, which I don’t consider as production code (you don’t touch it very often, although of course it has to be production ready), so I don’t have any issue with this.
Nitpicker corner: Let us skip the whole TypeMock discussion, shall we?
TDD fanatic corner: I don’t really care about testable code, I care about tested code. If I have a regression suite, that is just what i need.
Comments
I can see the point, but its just wrong! if you don't want to call DoExecute in the tests, but you want to ensure that Execute was called with the correct parameters etc,
What you actually want is to be able to do is be able to mock this class.
Although we should strive for simplicity, I think putting hooks for testing so that "when it tests the code follows this path" actually reduces the use of the test in the first place, and could well hide bugs further down the chain of the program.
Adding the interface to allow for mocking adds complexity but I think the price for missing it out is to high, we do need a way of simplifying the structure of our code, but I am not sure this is the way to go.
I never did get the point of avoiding the database like a plague. Why not just setup a test database, and test against that? I'm currently working on some database layer, so our tests have three stages:
Setup - calls our interface function that sets up the table like we want it
Test - calls whichever function we want to test at the moment, be it the one that dumps a table or the one that replaces a single row.
Verify - calls the dump-table function and compares that to an expected output.
It's all very simple, and it works quickly enough (it doesn't slow down the build by any noticeable length of time). Maybe it's good for us because our app is very database-centric, but I just don't see why you'd go to all that trouble to avoid the database.
The point of "avoiding" the database is usually a simple one: even if we ignore the performance issue, each different method that have to be tested might expect the database to be in a certain different state - certain tables must contain some records, with some particular values etc. - to implement a separate script which populates the test database with the expected data, for each method (or group of methods) might be a huge overhead when somebody have hundred of different methods..
Ayende, you just spoke the pure truth. Many of the testing advocates who are especially visible in the community are fanatics. They are the people who argue if the test attribute should be called Test or Fact...
I assume you're not arguing against abstractions, only the excessive use of abstractions. Personally, I don't abstract something "in anticipation". Instead, it's usually when two people need to work simultaneously (both code to the interface), when the component needs to be tested separately, or when I might need to test it on a machine that doesn't have 100% of the environment setup.
In the model you gave above, I would create the IAuthenticationService every time. This might allow development in a non-connected environment.
Regarding the test hooks you illustrated:
The "if" statement reduces the code readability. It isn't as clear to me as "set my provider to a mock and execute".
I've seen the testing code get activated by accident in production environments before. This shouldn't happen, but everyone makes mistakes.
On a separate note: there are definitely times to just "hack it" and be done with it.
I see what you're saying, but I don't think I would want to use an if statement like that. In a lot of cases I have found that it's better to just forget the separate behaviour for production and testing altogether (ie: just hit the database in your tests). In fact, I wrote a whole article about how too much abstraction is a bad thing and you might as well access your db in automated tests: graemehill.ca/unit-testing-an-entity-framework-... (the title mentions Entity Framework, but it is not actually specific to EF)
So, your tests drove your design. In other words you practiced TDD. You have thought about code testability. Will you then kindly move over to the fanatics corner?
InMemoryUserRepository - SqlLite :memory: anyone?
I just don't understand the obsession people have with mocking out their database - just accept the fact you need to get, save and fetch data, and life becomes so much simpler :-)
Works great for state based testing:
Customer.Create, Order.Create(customer)
orderingSvc.CancelOrder
Assert.Whatever
No mocking, and hey because you have NH & SqlLite in memory the tests just became so much much faster.
Just recently we got our "unit tests" to run 5 times faster by moving to sql lite & also other optimizations like doing heavy weight container, AR initializations just once per binsor file per test-suite.
Ajai
In the post I've found two reasons why you want to go with code without abstractions and with that CurrentContext thing: "much cheaper and faster". How is that cheaper and faster than creating an interface (using resharper)? In which kind of projects it's cheaper and faster?
With the context thing you won't be able to test the protocol between the two objects (how they work together) if you need to.
I agree on the idea that creating abstractions just for the sake of testability all over the code is a doubtful thing though.
I can't say I agree with this opinion, but I can certainly understand that there's nothing wrong if it works for you.
If the end result is stable then it doesn't matter if or how you choose to test it.
My $0.02 on this approach would be that CurrentContext smells like a God-Object that would be expected to know about implementation details of every class type it provided test stub behaviour for.
It also implies that the behaviour of each call to the method will be treated exactly the same way. Part of testing any particular section of code independently via a mock is the ability to specify the results from dependent objects. For example, how does a test of some component dependent on AbstractQuery assert its behaviour when AbstractQuery throws an exception, or returns different supported, (or unexpected)enumerables? I don't want to boil over into an argument of how far testing should go, my point would be that there are often grounds to test around more than one state for a dependency.
Would making the Execute method virtual also succeed in allowing tests to fake that method while also avoiding the ceremony of unnecessary abstraction while also being even cheaper and faster than the test hook? I prefer that to having a potential liability in production.
I don't know, it kinda smells; but I can't specify what can go wrong with it...
What about this: the subject of the test is a descendant of AbstractQuery, and you have other objects that should operate in "test" mode, but you want the test subject to operate in "production" mode.
How have you been using this approach in you projects?
Err... I meant: How long have you been using this approach in your projects? didn't it cause you any maintainability problems?
If someone deletes the call to DoExecute(...) you're in trouble, and your tests wont pick it up.
Why not derive from this class in your tests and override your template method DoExecute() to return CurrentContext.GetResultsFor(this)?
I don't think the issue was ever about it not working. No testing whatsoever works too. The issue was about getting a good (aka SOLID) design. Whether that is a desirable goal is another issue (I happen to think it is desirable), but TDD is a methodology to get that SOLID design. If you have other, more important goals then yes, the whole abstracting thing is pointless.
What about aspect oriented programming ? Instead of writing test related hooks inside your method, why not writing an AOP aspect and then apply it to whatever method you want ? This aspect will tranform your method so that instead of calling a DB in order to get a list of entities, it will return a list of test entities.
Comment preview