Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

While there are ways of keeping unit tests from being executed, what is the value of checking in failing unit tests?

I will use a simple example: Case Sensitivity. The current code is case sensitive. A valid input into the method is "Cat" and it would return an enum of Animal.Cat. However, the desired functionality of the method should not be case sensitive. So if the method described was passed "cat" it could possibly return something like Animal.Null instead of Animal.Cat and the unit test would fail. Though a simple code change would make this work, a more complex issue may take weeks to fix, but identifying the bug with a unit test could be a less complex task.

The application currently being analyzed has 4 years of code that "works". However, recent discussions regarding unit tests have found flaws in the code. Some just need explicit implementation documentation (ex. case sensitive or not), or code that does not execute the bug based on how it is currently called. But unit tests can be created executing specific scenarios that will cause the bug to be seen and are valid inputs.

What is the value of checking in unit tests that exercise the bug until someone can get around to fixing the code?

Should this unit test be flagged with ignore, priority, category etc, to determine whether a build was successful based on tests executed? Eventually the unit test should be created to execute the code once someone fixes it.

On one hand it shows that identified bugs have not been fixed. On the other, there could be hundreds of failed unit tests showing up in the logs and weeding through the ones that should fail vs. failures due to a code check-in would be difficult to find.

share|improve this question
    
That's one way to increase those test coverage numbers. – JeffO Jan 19 at 20:33
    
If you already put in the effort to write the unit test, why would you want to have to rewrite it when you do decide to fix the issue? Just because it is checked in, does not mean that it has to be run in the suite. (You could make a category for "Known Issues", and treat those tests as a backlog/TODO list.) – Caleb Jan 20 at 2:20

But the broken ones that identify bugs in a new project, named as such. That way you can see that they SHOULD break... As they are being fixed, they would be turned green, and moved into the normal test suite.

NOTE: That project would have to be set to not be built on the build server, if your build server prevents checkins that break the build (assuming you define a broken build as one in which all tests don't pass)

share|improve this answer
    
+1 although there is no answer if to checkin or not there is an important argument: build server – k3b Mar 16 '11 at 6:21
    
I'd rather use an attribute to mark such a test instead of moving it to a separate project. – CodesInChaos Jan 19 at 15:05

I won't pretend that this is industry standard, but I check in broken tests as a way of reminding me or my other project members that there is still a problem with either the code or the unit test itself.

I suppose one thing to consider is whether your development policies allow for failed tests without penalty. I have a friend who works at a shop that does test-driven development, so they always start out with failing tests...

share|improve this answer
4  
But you should never check in a failing test, as your build server shouldn't build a project with a broken test. – CaffGeek Mar 15 '11 at 15:20
    
@Chad: Building and testing are two separate pieces of one automated steps. Building ensures that everything compiles. Test ensures that the result of the build is valid. My interpretation of the question was not, "should I check in code that doesn't compile?" Instead it was, "should I check in a test I know will fail?" – unholysampler Mar 15 '11 at 15:33
1  
I was just adding a point to consider, some continuous integration build servers run the tests, and if they fail, they don't get deployed. Rightfully, as if the build fails, the code fails, and there is no point in deploying a product known to be broken. – CaffGeek Mar 15 '11 at 15:40
    
@Chad: Right, I totally forgot about CI servers. That would definitely be a point to consider. It is also worth clarifying what we mean by "broken" tests; are they just plain "bad" tests, or is the test failing because the API changed in some way? – Tieson T. Mar 15 '11 at 15:58
    
The question should have been clearer. It should be the The test will compile, but the expected result will fail. – user20194 Mar 15 '11 at 18:00

Unit tests should test for error cases in addition to success cases of a function. A function should explicitly reject bad input or should have documentation to explain what input is considered valid.

If you have a function that is not doing either of those things, it is a bug and you should have a way of recording that it exists. Creating a unit test that demonstrates this issue is one way to do that. Filing a bug ticket is another option.

The point of unit testing is not to have 100% success, the point is to find and fix bugs in the code. Not making tests is an easy way to have 100% success, but that is not very beneficial to the project.

share|improve this answer
    
Woah... "The point of unit testing is not to have 100% success", are you saying they don't all need to pass!? – CaffGeek Mar 15 '11 at 16:01
2  
@Chad: The point is that it is better to have tests that you know will fail, but are demonstrating a real issue instead of not having the test just so you can have the green check mark at the end of the nightly build/test. – unholysampler Mar 15 '11 at 17:03
8  
@unholysampler, never have broken tests, unless they are CLEARLY marked as "should" break (by being in a different project). Otherwise, they become noise, and you don't know when a test that should be passing, has been broken. It completely defeats the purpose of Continuous Integration and having UnitTests – CaffGeek Mar 15 '11 at 17:18
2  
@Chad: I think this is getting into semantics of definitions. Based on the OP, it sounded like he was talking about creating a valid test that exercises a bug. However, the bug is low priority and not likely to to be fixed immediately. You are the one that brought up Continuous Integration, which puts much stricter restrictions on the automated process. – unholysampler Mar 15 '11 at 17:33
4  
@unholysampler, CI, or no CI, the point is, when you run the tests, and are used to seeing some Red lights, you become accustomed to it. So, when something that was green, turns red...how do you know?!? It's a horrible practice, and one of the reasons testing fails to be accepted in so many organizations. – CaffGeek Mar 15 '11 at 17:36

Failing unit tests give the development team visibility into what must be done to conform to the agreed upon specifications.

In short, failing unit tests give the team a "TODO" list.

For this reason, failing unit tests are far better than no unit tests at all.*
The absence of unit tests leaves the development team in the dark; specifications must be repeatedly confirmed manually.

[*Provided that the unit tests actually test something useful.]

share|improve this answer

File a bug for every failure, and note that in the test result. Then if you ever get your act together and fix the bug, your test passes and you delete that from the test result. Never just ignore problems.

share|improve this answer

I do not like broken unittests checked in because they produce unnecessary noise. After every unittest i would have to check all failed issues(red). Is it red because there is a new problem or because there is an old open to do/to fix. This is not okay if there are more than 20 unittests.

Instead I use

  • [Ignore("Reason")] attribute that makes the result yellow or
  • throw new NotImplementedException() that makes the result grey

Note: I am using NUnit for .net. I am not sure if the "gray" feature is there in other unittest frameworks.

So I like the following meaning of unit test-results.

  • green : all finished
  • gray : planned new features that need to be done but with low priority
  • yellow : bugs not fixed yet. Should be fixed soon
  • red : new bugs. Should be fixed immediately

Everything except "red" can be checked in.

To answer the question: there is more harm than value to check in "red-failed-testes" but checking in "yellow-ignored-tests" or "grey-NotImplementedYet-tests" can be usefull as a todo-list.

share|improve this answer
    
the problem I see with this approach is, that the ignored tests will probably never be fixed. You could also just remove the whole test code, what would be the difference (I'm being a bit cocky here) – Lovis Jan 19 at 14:52
2  
will probably never be fixed is a political decision if you want to spend efford in automated tests or not. With "ignored tests" you have the chance to fix them. Throwing away "ignored tests" means "abandon automated tests by and by until there are no more " – k3b Jan 19 at 15:05

The purpose of unit tests is to assert the expected behavior of a system, not to document defects. If we use unit tests to document defects, then the usefulness of them to assert expected behavior is lessened. The answer to the question "Why did this test fail?" is not a simple "Oh, something is broken that I didn't expect to be broken." It has become unknown whether the test failure is expected or unexpected.

Here is a paragraph from the beginning of chapter 13 of Working Effectively with Legacy Code:

Automated unit tests are a very important tool, but not for bug finding--not directly, at least. In general, automated tests should specify a goal that we'd like to fulfill or attempt to preserve behavior that is already there. In the natural flow of development, tests that specify become tests that preserve. You will find bugs, but usually not the first time that a test is run. You find bugs in later runs when you change behavior that you didn't expect to.

share|improve this answer

How I see TDD done with implementing tests for an unfinished code is, write tests first with [ExpectedException] attribute or similar. This should pass initially as the incomplete code would not have any logic in it and write a throw new Exception() code in it. Though throughing exception is wrong, this would atleast make the tests pass initially and fit for checkin. We may forget an ignored test but may definitely tidy up or fill the incomplete code. When we do so, automatically the corresponding test that was expecting an excpetion would now start failing and alarm you to fix it. This may involve a slight alteration of test to get rid of ExpectException and instead do real asserts. CI, Dev, testers and customers all happy and a win-win situation?

share|improve this answer
    
This does not answer question. Its not asking what is TDD and why test for expected exceptions. – Andy Wiesendanger Jan 28 at 16:36

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.