Refactoring your legacy code - Part four: Fixture this!
Another day in paradise. The right time to continue with our small series about refactoring. Today Tomas and me will talk a bit about the right (and wrong) way of preparing your data for the tests - the so called “Fixtures”. If you will read in the text the context “we” then Tomas wrote this part about his experience with the OXID refactoring.
Remember - we learned how to test simple 2×2 functions but when we tried to test more complex ones initialy we felt like stalled with no or negative progress. We tried to identify what is the difference between simple functions returning a value from supplied parameters and the real life ones.
The problem with those functions is that they depend in most cases on your application state (script globals, config vars, session vars, object state, existing data etc..).
As already shown in the previous postings we are able to separate all the code into a few abstraction layers (db access, fetching config vars,..) and we did that, but finally we still wanted to test in a simple way the high-level functions. Eg. createOrderFromBasket()->saveOrder()->loadOrder() flow. Lets identify the issues preventing us from simply testing these functions and see the way how OXID handled them.
Time consuming fixture creation:
It is quite easy and fast to prepare a small data set for tests, but when you try to test a bit more complex functions like the calculation of values in a shopping basket, then you have to prepare the full basket and define all the needed values (products, amounts, vat… ). And if you have to do this for every test, this is a pretty boring task.
OXID decided to solve this issue by creating a test database filled with all the common data: users, orders, various combination’s of products, discounts etc. For each test where we needed this data we created on the fly fixtures out of this database - some sort of “fixture” generator with a static source (our database). This helped a lot in speeding up fixture creation. I’ve heard some opinions and complaints that our big demo data is a big NO, not a really valid “fixture” and you should prepare fixtures for every test separately, but why not to consider our test data as one fixture - only bigger? Leave us a comment if you think different!
Actually the way described above violate the theory behind TDD. In the swoodoo team we do it a bit different - but still I want to share this with you, as it’s a real world example and - not to forget - a very successful one.
Test data is altered via another test:
And here we have the next problem. When you have one big test data chunk, it gets altered by other tests. One of the (very wrong) ways would be adjusting tests, depending on which order they are run. But it’s clear you will reach a dead end very soon. Do not even try this. Wrong!
How did OXID solve this? First of all we created a rule: use test data for reading relaxed (actually the nature of our business logic is that it is “read data” oriented) and - if we need to change test data for test – make sure we change it back on tearDown(). This is still dangerous as one failed test may result of other test failing. Tracking this kind of failures is most difficult task. So additionally we use a small script which is run after each test (test class), which diffs existing db with initial and restores it.
Actually the much better way is to create the fixtures each time manually with tools like yaml. This way is the correct way and it can be solved also without too much work if you create yourself a generator for yaml. Doing so will avoid many problems like the ones described above. On the other hand - this is pure theory. And refactoring an existing application is something totally different than creating a new one with the latest state of the art technology.
Comments
6 Responses to “Refactoring your legacy code - Part four: Fixture this!”
Leave a Reply
I think the first main challenge with having a large chunk of “Shared Fixture” is keeping the tests independent of each other. Diffing and resetting the fixture like you do would be one way, but if this is too slow, it will lead to the tests running slowly and being run less often.
This is often the case with testing against an actual database, which is always slower than testing against an in-memory fixture. This is the second challenge.
The third challenge is that the test data simply is not visible in the test, not even in a fixture file next to it. That means I can’t see the test data when looking at the test, so I can’t really understand what the test is supposed to be testing, unless I put the information in comments, which again is redundant and prone to get out of sync with the fixture. This endangers the use of tests as documentation and makes tests harder to understand and to maintain.
How do you meet those challenges?
(The book “xUnit Test Patterns: Refactoring Test Code” by Gerard Meszaros has some excellent advice for everyone dealing with these and related problems. Highly recommended.)
Jochen,
thnx for your feedback. You are absolutly right with your concerns. The best way is - as I wrote - to have the fixtures per test/file and not using a database. On the other hand if you refactor a large and old application you need to find the right balance between fixtures and progress - otherwise it is easy to spend years - and I mean really years - creating perfect tests. I will ask Tomas to answer your concners as he is working with the test database each day
Jochen, thanks for the good points. I would like to emphasize that we do not _rely_ on the script to fully restore the test database. Indeed, this would oppose to the principle to test independently and atomic and furthermore running all tests would take much longer time. The script rather assures that the data has not been changed during tests (particulary we execute the check after each test case). We even plan to initiate the test to fail when data change is detected in order to keep sufficient test quality. But overall, having in mind that our application functionality is much more often based on database reading rather than writing, also the fact that we have hundreds of tests requiring very similar sets or subsets of test data, I think, having a shared fixture is a good compromise on a theoretically perfect TDD model and our reality of constant time pressure.
That could be definitely improved in future, but for the moment the primary and the main goal of automatic testing has been reached - the tests keep us alert on each functionality break.
[...] our modern test driven development world. Der Vortrag wurde zwar als ein Gemeinschaftsprodukt von Lars Jankowfsky, Johann-Peter Hartmann und Thorsten Rinne, aber Lars hat das ganze so ziemlich alleine gemeistert. [...]
Hello dodger,
I think you are mixing few things. Well, maybe mixing is not adequate word, but try to take a look from aside.
From my point of view, there are always 2 types of tests. They are _unit_ tests and _integrational_ tests. They are all written with xUnit, and I found that that is why many people don’t separate them.
When you are writing unit tests, you should test only 1 class, only 1 method. That means, you don’t need any basket. If you need basket object somewhere, just fake (mock/stub) it. You don’t need many fixtures. The purpose of unit test is to make code stable.
Integrational tests are one step higher. When you are writing integrational tests, you are testing code (code of the whole system) for some behavior. Let’s say you are writing test to ensure, that your order cancel process is working correcly (suppose it has to delete order from db, session and empty the basket). Then, and only then keeping “shared test data” for many tests is correct. Actually, “shared test data” should be called “state of system”. It can be situation, like user with id 8, have confirmed order with id 10, that contains objects with ids 1, 2, 3, 4, 5. User with id 8 have 3 objects in her basket, today saw objects with … and so on. Such state could be used to check cancel order, empty basket and many other bussiness processes. You can keep such system states in sql files and switch fast enough between them. If you look to integrational tests as separate type of tests, it won’t violate TDD rules (like Tomas mentioned). The purpose of integrational test is to make sure, that system (not class or method!) is conforming some bussines rule.
PS. Integrational tests should use real objects (and test data from real world), not fake. It means, that they should really write to database (HINT: mysql memory engine). You should call cancelOrder($id), let it pass, and then check db/session for real results.
Andrius,
sure you are right. I was talking about refactoring old crappy code. The problem there is to separate first the code in a way that it IS testable at all.
And about real world data - I do not agree here. This data can change and then tests will fail.