views:

148

answers:

7

I wanted to start a discussion about the details that you cover in your unit tests.

Do you test major functionalities, that consist of several methods doing one task at once with one test? or maybe you even test automatic properties?

Because, for example, I see little value in writing a test that would test only this:

   public Email
   {
      set
      {
         if(Regex.Match(/*....*/))
             email = value;
      }
      get
      {
        return email;
      }
   }

As it's really clear and it's just a waste of time. Usually when I do unit tests I test a whole task - like on this example - a whole registration procedure.

I'm asking this because, currently I'm reading the book "Applying Domain Driven Design and Patterns" authored by Jimmy Nilsson and there he points out that he's testing even such small details with a dedicated test.

Isn't such level of coverage an overuse?

+1  A: 

One could argue that testing the whole registration process is too high-level. Small pieces of functionality tend to be far easier to write tests for whereas more complicated tests often require a fair amount of scaffolding.

The strict approach would be to aim for 100% (or close to it) code coverage on your unit tests (many IDEs either have the capability to measure this directly or via some kind of plug-in). In practice you may not want to be so strict but instead pick key classes that you need to work because so much else depends on them.

One of the advantages of dependency injection ("DI") or inversion of control ("IoC") is that it encourages you to break your code up into pluggable pieces, which not only makes your code easier to test (because you can easily plug-in mocks) but also tends to make those pieces smaller. Writing tests for many small pieces tends to be much quicker than for fewer large pieces.

Testing a whole registration process is more about systems or integration testing.

cletus
+2  A: 

I try to test whatever could throw an exception or may fail in any other way.

For example, in your example, what if someone enters something that is not an email address, do you just quietly fail?

More important, I called Email with a valid email address, so email has a value. Then it gets called with an invalid email address. What should be the expected behavior? Is the original email address still there (I think so), and is that the correct behavior?

If there is a specification on how it should behave, then that sets the grounds for a unit test, IMO.

If you test this function at a higher level then you just need to make certain you are testing all possible situations for this setter, but testing the getter part is pointless.

James Black
Karim
+1 for the tests sanity checking the API design
Mark Simpson
@Karim - I tend to test anything that may have an error or unexpected response. For example, if you always assume that adding two ints is positive, then you should properly handle if it is negative, so, then a test is needed. But, if you don't care positive, negative, overflow, then no need to test.
James Black
@Karim - If an exception is thrown in your example, then the value isn't changed, so you still have a valid email address in your property, unless you set 'email=""' before using the regular expression. But, regardless, there should be some document explaining what the state should be in the event of an invalid email address. That would be the test case.
James Black
+1  A: 

An important attribute of unit tests is that they show clearly what is wrong when they fail. If they test very small functionality, this is possible. If you test, for example, a whole registration process, you can't be sure what line went wrong.

And as cletus said, testing a process would generally be considered systems or integration testing. Integration tests aren't expected to point out exactly what is wrong, but more that something is wrong.

JeffH
+8  A: 

Tests are not just there to test that what you wrote works. Tests are also there to test that what you wrote STILL works. Like, many years and many developers later. What you think is a dead simple class might in the future become more complicated.

For example, lets say it started with no filter. Just a simple get/set. Why test that? Then later some other developer adds in the regex filter, and its faulty. Then suddenly the class is broken, but its untested so nobody knows. This reveals itself as mysterious failures further up the call stack which now take more time to debug. Probably more than it would have taken to write a few tests.

And then, in the future, somebody's going to try and get clever and optimize your code. Or reformat it, regexes have a tendency to be written unreadable and can often do with some cleanup. This is ripe for subtle breakage. Small targeted unit tests will catch this.

In your example above, the regex is presumably there to filter for things that look like email addresses. That necessitates checking that regex is working, else your Email class stops accepting emails. Or it starts taking gibberish. Maybe your regex doesn't cover a valid email address, that's worth testing once you discover it. Eventually, somebody is going to replace your regex with an actual parser. Maybe it works, maybe it doesn't. A good test will have a simple list of valid and invalid email addresses which you can easily add to as corner cases are discovered.

Tests also allow you to exercise your interface and discover holes. What happens when you put in something that isn't an email address? That's right, nothing. Email.set silently throws away the input. Garbage in, nothing out isn't very polite. Maybe it should throw an exception. This is something that would quickly become clear as you're trying to test it, because it would be necessary to test whether the set worked.

Testing can also reveal inflexibilities and things which cannot be overriden or customized. In your example, it would be handy to test the regex filter directly rather than having to instantiate an object each time. This is because the filter is the most complicated bit, and its easier to test and debug while going through as few layers as possible. By putting it into Email.is_email_address you can now test it directly. As a side-effect it can also be overriden in a subclass. This is handy because most people get email validation WRONG because EMAIL HATES THE LIVING!

Finally, you want your tests to be decoupled. Test one thing without it being effected by additional complexities so you can clearly see the root of the problem. Your Email class is an excellent candidate for a simple, decoupled unit test.

Schwern
+1. Good advice
Mark Simpson
A: 

I would certainly test the code snippet you showed. It may just be a single if statement, but Regular Expressions are extremely easy to get wrong. I'd use a data-driven test to verify the regex is correct.

Even if it was just a case of if(some simple condition), I would advocate writing a test to make sure it is correct. After all, if the logic is simple, you might switch off your brain while writing it. Similarly, it's easy to get a ! in the wrong place, use && instead of ||, add parenthesis in the wrong place or just make an elementary mistake. If you add a test to cover the new code before you write it (the new code), the test will often catch the mistake before you go any further.

Moreover, writing the test using the AAA pattern (arrange, act, assert) helps you sanity check your API because your test is using it! I've often tested simple-looking parts of my code and then realised, from another perspective (a user's), the API could be clearer.

I personally wouldn't test auto properties with dedicated tests unless I knew for a fact the implementation is likely to change. That time is probably better spent adding more tests covering potential edge cases, or more tests for the highest traffic areas of your code.

Mark Simpson
+1  A: 

There are a number of reasons why you should test this particular property. Here are the different tests and the reasoning behind those.

  1. Test all the possible correct values that the property could possibly get invoked with. This is primarily to build up a complete picture of what values this property expects. This will become invaluable for later refactoring (i.e. regression).
  2. Test that the property returns the value that was previously successfully set. Because this is not straigh forward get/set property you want to make sure that it returns the right value.
  3. Test that it throws exception whenever an invalid value was passed in. Since this particular bit of code really should handle invalid input, you will need to throw an exception if the value passed in is not valid.

Testing this particular proeprty is certainly not a waste of time since email validation is not all that straight forward , you should really test the bollocks out of this one.

Igor Zevaka
+5  A: 

Tests of simple functions are themselves simple. So they are easy to write and easy to run. They take very little time to create and are a place holder for when the module gets more complicated later. So, yes, test even the simple functions.

Now, the function you typed above contains a regular expression which you helpfully commented out. Regular expressions are notorious hard to predict. The regular expression to match a valid email address is probably very complicated. So I would be testing the hell out of it. I'd have unit test after unit test plying that regular expression with all the different email address variants that I could think of, and all the corner and negative cases as well.

The decision about whether to write a test or not is always a decision about short- and long-term benefits. Tests are always beneficial in the long term, because they anchor the behavior of the code and detect side effects and unintended consequences. The act of deferring a test is always made for short term gain so that you don't "waste time". The problem is that ever test you don't write is a hole in the system in which side effects and unintended consequences can accumulate undetected. Since tests for simple modules are quick and easy to write, and since Murphy will ensure that nasty bugs will hide in any hole you provide for them, it seems unlikely that those simple tests are truly a waste of time.

Robert C. Martin