Take the 2-minute tour ×

I'm currently building a CI build script for a legacy application. There are sporadic JUnit tests available and I will be integrating a JUnit execution of all tests into the CI build. However, I'm wondering what to do with the 100'ish failures I'm encountering in the non-maintained JUnit tests. Do I:

1) Comment them out as they appear to have reasonable, if unmaintained, business logic in them in the hopes that someone eventually uncomments them and fixes them

2) Delete them as its unlikely that anyone will fix them and the commented out code will only be ignored or be clutter for evermore

3) Track down those who have left this mess in my hands and whack them over the heads with the printouts of the code (which due to long-method smell will be sufficently suited to the task) while preaching the benefits of a well maintained and unit tested code base

share|improve this question
You would like to beat up the people that gave you the code or the people that wrote unit tests? P.S. Bad programmers can only understand long methods. –  IAdapter Mar 23 '10 at 17:03
add comment

8 Answers

up vote 1 down vote accepted
  • Comment them out so that they can be fixed later.
  • Generate test coverage reports (with Cobertura for example). The methods that were supposed to be covered by the tests that you commented out will then be indicated as not covered by tests.
share|improve this answer
No need to comment out things, that's what revision control is for. delete them or @Ignore them. Commenting them out just leaves more cruft. –  thekbb Jun 18 '11 at 3:38
add comment

If you use Junit 4 you can annotate that tests with @Ignore annotation.

If you use JUnit 3 you can just rename tests so they don't start with test.

Also, try to fix tests for functionality you are modifying in order to not make code mess larger.

share|improve this answer
The @Ignore annotation is interesting. As for the removal of test from the method name, you'd then have to be careful and add a comment to explain that this is not a forgotten/unused method (otherwise somebody could delete it), but a test method that should be fixed. –  b.roth Mar 23 '10 at 16:15
You can rename method to something like fixItLaterTestBlahBlah, isn't it? –  uthark Mar 23 '10 at 16:40
We use JUnit 3 and I had considered renaming the tests to brokenSuchAndSuch, but to me this requires more work by future maintainers to understand what I did and why I did it. With commenting out or deletion, its much more in your face. –  Chris Knight Mar 23 '10 at 21:22
If you comment out tests then it may be possible that they will be uncompilable when you uncomment them because of API changes. If they will be always uncommentable you will fix compilation errors when your API changes. –  uthark Mar 24 '10 at 0:43
add comment

Follow the no broken window principle and take some action towards a solution of the problem. If you can't fix the tests, at least:

  1. Ignore them from the unit tests (there are different ways to do this).
  2. Enter as many issue as necessary and assign people to fix the tests.

Then to prevent such situation from happening in the future, install a plug in similar to Hudson Game Plugin. People gets assigned points during continuous integration, e.g.

  • -10 break the build <-- the worse
  • -1 break a test
  • +1 fix a test
  • etc.

Really cool tool to create a sense of responsibility about unit tests within a team.

share|improve this answer
The Hudson Game plugin link is momentary down. Here is another link clintshank.javadevelopersjournal.com/ci_build_game.htm –  ewernli Mar 23 '10 at 16:19
add comment

The failing JUnit tests indicate that either

  1. The source code under test has been worked on without the tests being maintained. In this case option 3 is definitely worth considering, or
  2. You have a genuine failure.

Either way you need to fix/review the tests/source. Since it sounds like your job is to create the CI system and not to fix the tests, in your position i would leave a time-bomb in the tests. You can get very fancy with annotated methods with JUnit 4 (something like @IgnoreUntil(date="2010/09/16")) and a custom runner, so or you can simply add an an if statement to the first line of each test:

  if (isBeforeTimeBomb()) {

Where isBeforeTimeBomb() can simply check the current date against a future date of your choosing. Then you follow the advice given by others here and notify your development team that the build is green now, but is likely to explode in X days unless the timebombed tests are fixed.

share|improve this answer
Loving the timebomb concept! –  Chris Knight Sep 16 '10 at 16:05
add comment

If they compile but fail: leave them in. That will get you a good history of test improvements over time when using CI. If the tests do not compile but break the build, comment them out and poke the developers to fix them.

This obviously does not preclude using option 3 (hitting them over the head), you should do that anyway, regardless of what you do with the tests.

share|improve this answer
add comment

You should definitely disable them in some way for now. Whether that's done by commenting, deleting (assuming you can get them back from source control) or some other means is up to you. You do not want these failing tests to be an obstacle for people trying to submit new changes.

If there are few enough that you feel you can fix them yourself, great -- do it. If there are too many of them, then I'd be inclined to use a "crowdsourcing" approach. File a bug for each failing test. Try to assign these bugs to the actual owners/authors of the tests/tested code if possible, but if that's too hard to determine then randomly selecting is fine as long as you tell people to reassign the bugs that were mis-assigned to them. Then encourage people to fix these bugs either by giving them a deadline or by periodically notifying everyone of the progress and encouraging them to fix all of the bugs.

share|improve this answer
add comment

A CI system that is steady red is pretty worthless. The main benefit is to maintain a quality bar, and that's made much more difficult if there's no transition to mark a quality drop.

So the immediate effort should be to disable the failing tests, and create a tracking ticket/work item for each. Each of those is resolved however you do triage - if nobody cares about the test, get rid of it. If the failure represents a problem that needs to be addressed before ship, then leave the test disabled.

Once you are in this state, you can now rely on the CI system to tell you that urgent action is required - roll back the last change, or immediately put a team on fixing the problem, or whatever.

share|improve this answer
add comment

I don't know your position in the company, but if it's possible leave them in and file the problems as errors in your ticket system. Leave it up to the developers to either fix them or remove the tests.

If that doesn't work remove them (you have version control, right?) and close the ticket with a comment like 'removed failing junit tests which apparently won't be fixed' or something a bit more polite.

The point is, junit tests are application code and as such should work. That's what developers get paid for. If a test isn't appropriate anymore (something that doesn't exist anymore got tested) developers should signal that and remove the test.

share|improve this answer
add comment

Your Answer


