IceArdor
IceArdor

Reputation: 2041

Knowing when an @Ignore'd test becomes passing

On a medium-large open source project written in Java, we receive many bug reports, fewer unit test for those bug reports, and fewer patches that close those bugs.

When a unit test but no patch is provided, we verify the existence of the bug against the trunk by adding the test function to the suite. We commit it, but add a junit4 @Ignore annotation to it so as not to break the build for a known bug.

Some bugs may be related or a change made for one bug may fix another. We would like to know when a previous known-failing test case no longer fails so we can notify the people watching that bug and close the bug.

Let's say we have some buggy function--raises an exception, has un-intended side-effects, or returns the wrong value. (Real world example)

public void buggyFunction() {
    throw new UnsupportedOperationException("this will be implemented later");
}

And some unit test to test buggyFunction

@Test
public void knownIssue() {
    buggyFunction();
}

Option 1: @Ignore the test

@Ignore("this test is currently failing. see bug 12345")
@Test
public void knownIssue() {
    buggyFunction();
}

Option 2: The standard junit4 way is to use @Test(expected=MyException.class) or sprinkle @Rule ExpectedExceptions throughout the test function. Neither give the user a helpful message saying why a failing test means a bug has been fixed, and to update the unit test and close the bug. Additionally, the test passes if the expected exception is thrown, but the test is meaningless in this case. It would be better to either fail (when the bug is fixed) or skip (when the bug is still open).

// when this bug is fixed, it should not throw an exception
// TODO: delete expected=UnsupportedOperationException.class
@Test(expected=UnsupportedOperationException.class)
public void knownIssue() {
    buggyFunction();
}

OR

@Rule
public final ExpectedException thrown = ExpectedException.none();

@Test
public void knownIssue() {
    thrown.expect(UnsupportedOperationException.class);
    thrown.expectMessage("this will be implemented later");
    buggyFunction();
    thrown.expect(ExpectedException.none());
}

Option 3: Booger up a test with boiler plate scaffolding

@Test
public void knownIssue() {
    try {
        buggyFunction();
    } catch (UnsupportedOperationException e) {
        // we know that buggyFunction is broken, so skip this test
        assumeTrue("Skipping test. Expected exception: " + e, false);
    }
    // surprise! buggyFunction isn't broken anymore!
    fail("The function is no longer buggy! " +
         "Update the unit test and close bug 12345!");
}

Are there any better alternatives that:

I could accomplish something like this pretty easily in Python where unevaluated functions are first class objects. The same can be done in Java 6 (yes, that's the version we're on) but probably requires more boilerplate than Option 3. Please tell me I am wrong.

def alertWhenFixed(expected=Exception, bug=12345):
    def decorator(func):
        def func_wrapper(*args, **kwargs):
            try:
                func(*args, **kwargs)
            except Exception as e:
                if isinstance(e, expected):
                    assumeTrue("Skipping test. Expected exception: {}"
                               .format(e), false)
                else:
                    raise e
            fail("The function is no longer buggy! " +
                 "Update the unit test and close bug {}".format(bug))
        return func_wrapper
    return decorator

@alertWhenFixed(expected=UnsupportedOperationException, bug=12345)
def knownIssue():
    buggyFunctionThrowsException()

@alertWhenFixed(expected=AssertionFailed)
def knownIssue():
    assertEquals(42, buggyFunctionReturnsWrongValue())

@alertWhenFixed(expected=AssertionFailed)
def knownIssue():
    buggyFunctionHasWrongSideEffect()
    assertEquals(42, getSideEffect())

This decorator can test known issues that raise exceptions, return an incorrect value or have the wrong side-effect.

This decorator is 100% reusable, so no copy-pasta try/except scaffolding, I can delete one line of code when the known issue has been fixed, and most importantly I can leave the test case logic alone.

Any idea if this can be translated to Java 6 or 7?

Upvotes: 5

Views: 293

Answers (3)

Markus Mitterauer
Markus Mitterauer

Reputation: 1610

If you're using Testsuites or using them is an option for you, you could try the JUnit Category annotation. (Or depending on your buildsystem you might even be able to do without test suites. Find more details the linked article.)

Therefore you need to create a marker interface:

public interface KnowIssue {
    // JUnit category marker interface
}

And instead of adding the @Ignore annotation you apply that category to your failing test method (or even a whole test class):

@Test
@Category(KnowIssue.class)
public void myFailingTest() {
    // some test
}

Then in the suite(s) for your green tests you exclude the KnownIssue category:

@RunWith(Categories.class)
@ExcludeCategory(KnownIssue.class) // <- exclude known issues!
@Suite.SuiteClasses({ /** your Testclasses here */ })
public class AllMyGoodTests {
    // test suite
}

To check on your known issues you create a second test suite, and you runt hat (regularily) to see if any of your known issues got fixed (green) by incident:

@RunWith(Categories.class)
@IncludeCategory(KnownIssue.class) // <- only run known issues!
@Suite.SuiteClasses({ /** your Testclasses here */ })
public class KnownIssueTests {
    // test suite
}

Then you remove the category from that test and it is automatically executed with your 'good' tests again.

It might even be possible to inverse the test-result (red<->green) of that failing tests with a custom test runner (@RunWith(KnownIssueCategoryRunner.class) but I've never actually tried that.

Upvotes: 2

Joop Eggen
Joop Eggen

Reputation: 109613

Make a separate TestSuite of bugs recognizers, that pass only when the bug is present (!).

/**
 * @see ...original now ignored test
 */
@Test
public void defect123124() {
    expectThrows(UnsupportedOperationException.class, () -> buggyFunction());
}

When a test "fails", it means you can delete it, and cancel the defect. Of course the inverse test will be needed in the regular test suite. There the @Ignore removed.

Now you have a nice overview. Also the programmer repairing something different, and suddenly getting failed tests of the bugs suite gets his bonus.

Upvotes: 3

GhostCat
GhostCat

Reputation: 140613

I don't see other technical options besides the one outlined by yourself.

I think the solution comes from a different perspective: your tooling isn't giving the developers the feedback they need. You see, the point is: when a developer decides to fix that buggyFunction() then that needs to happen in a strictly organized way. Meaning:

The developer wants to fix a bug. So he should be fully aware of that bug, and all the work that relates to this task:

  • Making updates in your bug tracking system
  • More importantly: run all the tests!

In other words: you want that your developer receives quick feedback. He should be able to change buggyFunction, and then, a few minutes later he should be notified about now failing testcases.

So even when he fixes the bug by accident, the crucial point is that he is notified that his change broke a test elsewhere. And then it is his responsibility what that other test broke. And if your system doesn't support that workflow in an efficient manner, than changing your unit tests will not help at all. Because your unit tests are not the real problem here.

Coming from that perspective, the only sane choice is between options 2 and 3. As they give you the "best" what this part of your system can give you: somebody changes code, and then a test breaks. And the person who changed code is notified as soon as possible; to then figure what is going on. From there on, it is only about the quality of that test (to point out what it means when that test fails); and the skills of the people involved (to do right thing then).

Upvotes: 2

Related Questions