TechEd 2008 notes: How not to write a unit test
How not to write a unit test
Roy Osherove
Typemock
Blog: ISerializable.com
All the things no one ever told you about unit testing.
Will have two parts: presentation-like (what really works), and interactive (questions, prioritized).
Early questions:
- Data access
- Legacy code (what not to do)
- Duplication between unit tests and functional tests
- Testing non-.NET code, e.g. ASP.NET
- Testing other languages, e.g. F#, IronRuby)
- Unit tests and refactoring
- Testing UI
- How do you mock the world?
- How important are tools? Mocking tools, refactoring, etc. Can you write unit tests with just what VS provides?
- Did you bring your guitar? — No. Wanted as much time for information as possible.
- Where did you get your T-shirt? — Was being given away at another conference.
- the.artofunittesting.com
- www.typemock.com — Typemock Isolator
A unit test is a test of a small functional piece of code
- If a method returns a boolean, you probably want at least two tests
Unit testing makes your developer lives easier
- Easier to find bugs.
- That’s the common line. But not necessarily — e.g. if your test has bugs, or if you’re testing the wrong things
- If you can’t trust your tests to find bugs (and especially if you don’t know you can’t trust them), then the opposite may be true — you may be confident you don’t have bugs when you do.
- If you don’t trust them, then you won’t run them, they’ll get stale, and your investment in writing them was wasted.
- Easier to maintain.
- But 1,000 tests = 1,000 tests to maintain
- Change a constructor on a class with 50 tests — if you didn’t remove enough duplication in the tests, it will take longer than you think to maintain the tests
- We will look at ways to make tests more maintainable
- Easier to understand
- Unit tests are (micro-level) use cases for a class. If they’re understandable and readable, you can use them as behavior documentation.
- Most devs give really bad names to tests. That’s not on purpose.
- Tests need to be understandable for this to be true.
- Easier to develop
- When even one of the above is not true, this one isn’t true.
Make tests trustworthy
- Or people won’t run them
- Or people will still debug for confidence
Test the right thing
- Some people who are starting with test-driven development will write something like:
[Test]
public void Sum()
{
int result = calculator.Sum(1, 2);
Assert.AreEqual(4, result, "bad sum");
}
- Maybe not the best way to start with a failing test
- People don’t understand why you want to make the test fail
- Test needs to test that something in the real world is true: should reflect the required reality
- Good test fails when it should, passes when it should. Should pass without changing it later. The only way to make the test pass should be changing production code.
- If you do TDD, do test reviews.
- Test review won’t show you the fail-first. But you can ask, “So can you show me the test failing?”
Removing/Changing Tests
- Don’t remove the test as soon as it starts passing
- If it’s a requirement today, chances are it’ll still be a requirement tomorrow
- Duplicate tests are OK to remove
- Can refactor a test: better name, more maintainability
- When can a test fail?
- Production bug — right reason (don’t touch test)
- Test bug (fix test, do something to production code to make the corrected test fail, watch it fail, fix production code and watch it pass)
- Happens a lot with tests other people wrote (or with tests you don’t remember writing)
- Semantics of using the class have changed (fix/refactor)
- E.g., adding an Initialize method that you have to call before you use the class
- Why did they make that change without refactoring the tests?
- Make a shared method on the test class that instantiates and Initializes
- Feature conflict
- You wrote a new test that’s now passing, but the change made an old test fail
- Go to the customer and say, “Which of these requirements do you want to keep?”
- Remove whichever one is now obsolete
Assuring code coverage
- Maybe unorthodox, but Roy doesn’t like to use code-coverage tools
- 100% code coverage doesn’t mean anything. Finds the exceptions, but doesn’t prove the logic.
- Better: change production code and see what happens
- Make params into consts
- Remove “if” checks — or make into consts (
if (true)). Will a test fail? If not, you don’t have good coverage. - Do just enough of these kinds of tweaks to make sure the test is okay.
- Test reviews are still valuable if you do pair programming, just maybe less often. Good to bring in someone else who didn’t write the code, with an objective eye.
- Quick test review of yesterday’s code at end of stand-up meeting?
Avoid test logic
- No ifs, switches or cases
- Yes, there are always exceptions, but it should be very rare
- Probably only in testing infrastructure
- Most of the time, there are better ways to test it
- Sometimes people write conditionals when they should be writing two tests
- Don’t repeat the algorithm you’re testing in the test. That’s overspecifying the test.
- Only create, configure, act and assert
- No random numbers, no threads
- Test logic == test bugs
- Fail first also assures your test is correct
Make it easy to run
- Integration vs. Unit tests
- Configuration vs. ClickOnce
- Laziness is key
- Should be able to check out, run all the unit tests with one click, and have them pass.
- Might need to do configuration for the integration tests, so separate them out.
- Never check in with failing tests. If you do, you’re telling people it’s okay to have a failing test.
- Don’t write a lot of tests to begin with, and have them all failing until you finish everything. If you do that, you can’t check in (see previous point). Write one test at a time, make it pass, check in, repeat.
Creating maintainable tests
- Avoid testing private/protected members.
- This makes your test less brittle. You’re more committed to public APIs than private APIs.
- Testing only publics makes you think about the design and usability of a feature.
- Publics are probably feature interactions, rather than helpers.
- Testing privates is overspecification. You’re tying yourself to a specific implementation, so it’s brittle, and makes it hard to change the algorithm later.
- Sometimes there’s no choice; be pragmatic.
- Re-use test code (Create, Manipulate, Assert) — most powerful thing you can do to make tests more maintainable
- Enforce test isolation
- Avoid multiple asserts
Re-use test code
- Most common types:
- make_XX
- MakeDefaultAnalyzer()
- May have others: one already initialized, with specific parameters, etc.
- init_XX
- Once you’ve already created it, initialize it into a specific state
- verify_XX
- May invoke a method, then do an assert on the result. Pulling out common code.
- Suggestion: by default, the word
newshould not appear in your test methods. - As soon as you have two or more tests that create the same object, you should refactor the
newout into amakemethod.
Suggestion: don’t call the method directly from Assert.AreEqual(...). Introduce a temp variable instead. (This relates back to the 3A test pattern.)
Aside: Test structure
- One possibility: Each project, e.g. Demo.Logan, has tests Demo.Logan.Tests. Benefit: they’re next to each other in Solution Manager.
- Test files: one file per tested class?
- That’s a good way to do it. Aim for a convention like MyClassTests so it’s easy to find.
- If you have multiple test classes for one test, make multiple classes.
- Consider nested classes: making a MyClassTests, and putting nested classes, one per feature. Make the nested classes be the
TestFixtures. - Be careful of readability, though.
- Roy said his preference would be to keep one test class per source file, rather than using nested classes to put them all in one file.
- Decide for yourself whether you’d prefer one class per file, or all tests for one class in one place.
Enforce test isolation
- No dependency between tests!
- If you run into an unintended dependency between tests, prepare for a long day or two to track it down
- Don’t run a test from another test!
- You should be able to run one test alone…
- …or all of your tests together…
- …in any order.
- Otherwise, leads to nasty “what was that?” bugs
- Almost like finding a multithreading problem
- The technical solution (once you find the problem) is easy. That’s why God created
SetUpandTearDown. Roll back any state that your test changed.
Avoid multiple asserts
- Like having multiple tests
- But the first assert that fails — kills the others. If one test fails, the others still run. Asserts aren’t built that way.
- Exception: testing one big logical thing. Might have three or four asserts on the same object. That’s possible, and doesn’t necessarily hurt.
- Consider replacing multiple asserts with comparing two objects (and also overriding ToString so you can see how they’re different when the test fails).
- My experience: this doesn’t work well when the objects get really big and complicated. Could work well for small objects, though.
- Hard to name
- You won’t get the big picture (just some symptoms)
Don’t over-specify
- Interaction testing is risky
- Stubs -> rule. Mocks -> exception.
- Mocks can make you over-specify.
- “Should I be able to use a stub instead of a mock?”
- A stub is something you never assert against.
- There’s only one time when you have to use mocks: when A calls a void method on B, and you have no way to later observe what B was asked to do. Then you have to mock B and verify that it was called with the right parameter.
- If the other class does return a value, then you can test what your class did with that result. You’re testing your class, after all, not that other object — that’s why you’re faking it.
Readability
- If you do another test that tests basically the same thing, but with different parameters, he suggests appending “
2” to the end of the test name. But that’s assuming you already have a really good naming convention for the base test name! (Remember the serial killer.) - Bad:
Assert.AreEqual(1003, calc.Parse("-1")); - Better:
int parseResult = Calc.Parse(NEGATIVE_ILLEGAL_NUMBER); Assert.AreEqual(NEGATIVE_PARSE_RETURN_CODE, parseResult);
- If you can send any kind of number, and the specific value you pass doesn’t matter, either use a constant, or use the simplest input that could possibly work (e.g.
1).
Separate Assert from Action
- Previous example
- Assert call is less cluttered