when reviewing tests, there are “easy finds” you can have.

Here’s how I might start search for the following issues:

Trust issues – can I trust the test (is it repeatable, conssistent and tests the right thing?

 

• Search the test project for use of “DateTime.” 

using DateTime in your tests means that everytime you run the test, you’re essentially running a different unit test. if it somehow fails due to the datetime, there might not be an easy way to recreate the failing test (if you’ve never logged the datetime being used)

• Search the Test Project for any word to do with Threads 

if you’re using threads, then your test might break due to threading issues, and not logic issues. even running it on a slow machine, or a machine with multiple CPUs changes the test execution. It’s ok for an integration test, but not for a unit test, which should always have the same result.

• Search for “Environment.” (ignore newline)

If you’re using anything from the current environment, then there are two possible things happening: You run a different a test every time you change your test machine, and your test may be repeating the logic that exists in the production code. Instead, try to fake the environment values (abstract them away) and have hard coded expected values in your tests. otherwise- you have an integration test.

Maintainability issues – how fragile are the tests? will the break even if the code under test changes, but isn’t broken?

• Use Resharper to search for the “.Verifiable” method, and group by member. there should be no more than one per test.  If there is then you are testing more than one thing – and your test is more fragile since it over-specifies internal behavior. any internal change in the code, even if the end behavior is OK, can break your test quite easily.
• If using Moq: Search for “.VerifyAll”  - you are again over specifying the test, making it fragile and easy to break for the wrong reason. You should verify only on a single mock object.
• If using Rhino Mocks: Search for multiple .AssertWasCalled or multiple Verify() or VerifyAll() for the same reason above.
• If using Isolator: Search for multiple Verify() calls in one test for the same reasons
• Search for creation of the objects under test inside the test – this is a big chance for refactoring – if you don’t, and the constructor for the object changes – you will have to change all these tests as well. Refactor to a factory method, or use the Setup method in the tests.

 

Again – these are all “easy wins”. there are things which take a bit more digging up to find, which will greatly improve your tests.

for more examples see:

• My book, the art of unit testing, has a bunch of test review guidelines at the last page
• My test review videos