Usually I just throw my unit tests together using copy and paste and all kind of other bad practices. The unit tests usually end up looking quite ugly, they're full of "code smell," but does this really matter? I always tell myself as long as the "real" code is "good" that's all that matters. Plus, unit testing usually requires various "smelly hacks" like stubbing functions.
How concerned should I be over poorly designed ("smelly") unit tests?
Are unit test smells important? Yes, definitely. However, they are different from code smells because unit tests serve a different purpose and have a different set of tensions that inform their design. Many smells in code don't apply to tests. Given my TDD mentality, I would actually argue that unit test smells are more important than code smells because the code is just there to satisfy the tests.
Here are some common unit testing smells:
The importance of smells is that that they are useful indicators of design or other more fundamental issues, i.e. "where there's smoke, there's fire". Don't just look for test smells, look for their underlying cause as well.
Here, on the other hand, are some good practices for unit tests:
Be concerned. You write unit tests to prove your code acts the way you expect. They allow for you to refactor quickly with confidence. If your tests are fragile, difficult to understand, or if they are difficult to maintain, you will ignore failing tests or turn them off as your code base evolves, negating many of the benefits of writing tests in the first place.
I just finished reading The Art of Unit Testing a couple of days ago. The author advocates putting as much care into your unit tests as you do your production code.
I've experienced poorly-written, unmaintainable tests firsthand. I've written some of my own. It's virtually guaranteed that if a test is a pain in the ass to maintain, it won't be maintained. Once the tests are out of sync with the code under test, they've become nests of lies and deceit. The whole point of unit tests is to instill confidence that we haven't broken anything (that is, they create trust). If the tests can't be trusted, they're worse than useless.
As long as your unit test actually tests your code in "most" cases. (Said most on purpose since its hard sometimes to find all possible outcomes). I think "smelly" code is your personal preference. I always write code in a way I can read it and understand it within few seconds, rather than dig through junk and try to understand what is what. Especially when you come back to it after a significant amount of time.
Bottom line - its testing, it suppose to be easy. You don't want to confuse yourself with "smelly" code.
Definitely. Some people say "any test is better than no test at all". I strongly disagree - badly written tests bog down your development time, and you end up wasting days fixing "broken" tests because they weren't good unit tests in the first place. For me at the moment, the two things I'm focusing on to make my tests valuable, rather than a burden, are:
You should be testing the outcome (what happens), not the method (how it happens). Your set-up for the test should be as decoupled from the implementation as possible: only set up results for service calls etc that are absolutely necessary.
It's okay to allow a bit more repetition in your tests, which you wouldn't normally allow in your production code, if it makes them more readable. Just balance this out with the maintainability stuff above. Be explicit in what the test is doing!
In conclusion, you should be very concerned with "smelly" tests - they can end up just a waste of your time, providing no value.
You've said:
Unit testing usually requires various "smelly hacks" like stubbing functions.
Sounds like you could definitely do with reading up on some Unit Testing techniques, such as using a Mocking framework, to make your life a heck of a lot easier. I'd very very strongly recommend The Art of Unit Testing, which covers the above and much much more. I found it enlightening after struggling with poorly-written, unmaintainable, "smelly" tests for a long time. It's one of the best time investments I've made this year!
Two questions for you:
There are ways of dealing with repetitive tasks in your unit tests which are most commonly setup and teardown code. Essentially, you have a test set up method and a test tear down method--all unit test frameworks have support for this.
Unit tests should be small and easily understood. If they aren't and the test fails, how are you going to fix the problem in a reasonably short period of time. Make it easy on yourself for a couple months down the road when you have to come back to the code.
When the trash starts smelling, it's time to take it out. Your test code should be as clean as your production code. Would you show it to your mother?
In addition to the other answers here.
Poor quality code in unit tests is not restricted to your unit test suite.
One of the roles filled by Unit Testing is Documentation.
The Unit Test suite is one of the places one looks to find out how an API is intended to be used.
Callers of your API are not unlikely to copy parts of your unit test suite, leading to your bad test-suite code infecting live code elsewhere.
I almost didn't submit my answer as it took me awhile to figure out how to even approach this as a legitimate question.
The reasons programmers engage in "best practices" are not for aesthetic reasons, or because they want "perfect" code. It is because it saves them time.
So the question your asking (from my point of view) is should I save myself time or write code that will waste my time?
To that question I can only say, how important is your time?
FYI: stubbing, mocking, and monkey patching all have legitimate uses. They only "smell" when their use isn't appropriate.
I try specifically to NOT make my unit tests too robusts. I have seen unit tests that start doing error handling in the name of being robust. What you end up with is tests that swallow up the bugs they are trying to catch. Unit tests also have to do some funky things pretty often to make things work. Think about the entire idea of private accessors using reflection...if I saw a bunch of those all over in production code, I would be worried 9 times out of 10. I think more time should be put into thinking about what is being tested, rather than code cleanliness. The tests will need to be changed pretty frequently anyway if you do any major refactoring, so why not just hack them together, have a little less ownership feelings, and be more motivated to rewrite or rework them when the time comes?
If you are going for heavy, low-level coverage you will be spending as much time or more in the test code as in the product code when doing any serious modifications.
Depending on the complexity of the test set-up the code there may be more complex. (httpcontext.current vs. the horrible monster of trying to build a false one accurately for example)
Unless your product is something where you rarely make a breaking change to existing interfaces and your unit-level inputs are very simple to set up I would be at LEAST as worried about the understanding of the tests as the actual product.
Code Smells are only an issue in code that make need to be change or understood at some point later in time.
So I think you should fix code smells when you have to go "close" to the given unit test, but not before.
