Exception assertions improvement. #722

Closed
ghost opened this Issue Aug 19, 2013 · 8 comments

Projects

None yet

5 participants

@ghost
ghost commented Aug 19, 2013

Hi guys.

There are at least 3 well known ways to check that some exception is thrown and has specific properties (verifiable by matcher):

try-catch:

@Test
public void assertSomeExceptionIsThrown() {
   try {
       ... // block of code under test
       fail("Expected test to throw SomeException");
   } catch (SomeException e) {
       assertThat(e, matcher);
   } catch (Throwable t) {
       fail("Expected test to throw SomeException, but caught: " + t);
   }
}

@Test(expected = ...):

@Test(expected = SomeException.class)
public void assertSomeExceptionIsThrown() {
   ... // block of code under test
}

@Rule:

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

@Test
public void assertSomeExceptionIsThrown() {
   thrown.expect(matcher);
   ... // block of code under test
}

All the caveats of these methods are also well known. try-catch is very wordy and overcomplicated for the most cases. @Test(expected = ...) is too simple -- the only thing you can check is the class of exception. @Rule is a bit better but it has almost the same disadvantage. While with @Test(expected = ...) you cannot say where
exactly the exception was thrown from, with @Rule you have to be cautious. You'll have to state your expectations just before the intended block of code. Otherwise JUnit will be pretty happy while you will get a false-positive test:

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

@Test
public void assertSomeExceptionIsThrown() {
   thrown.expect(matcher);
   ... // another block of code that can throw SomeException
   ... // block of code under test
}

Both of them (@Test(expected = ...) and @Rule) have the same side effect that as soon as exception is thrown the test is over. So throwing an exception would be the last thing you can check in your test. That's not very handy for more sophisticated tests.

What I suggest is to provide new methods in org.junit.Assert and org.junit.Assume to address the limitations of @Test(expected = ...) and @Rule and to take the best of try-catch while cutting the amount of boilerplate code needed for it.
org.junit.Assert:

public static <T extends Throwable> void assertThrows(ThrowingBlock block, Class<T> clazz);
public static void assertNotThrows(ThrowingBlock block);
public static <T extends Throwable> void assertThrows(ThrowingBlock block, Class<T> clazz, Matcher<? super T> matcher);
public static <T extends Throwable> T assertThrowsAndReturn(ThrowingBlock block, Class<T> clazz);

Where ThrowingBlock is simply a:

/**
 * Piece of code which execution could result in throwing an exception.
 */
public interface ThrowingBlock {

    void execute() throws Throwable;
}

Similar methods could be included in org.junit.Assume.

OK, here is how it's supposed to work:

@Test
public void assertWithClass() {
    Assert.assertThrows(new ThrowingBlock() {
        @Override
        public void execute() throws Throwable {
            account.withdraw(10);
        }
    }, InsufficientFundsException.class);
}

@Test
public void assertWithClassAndMatcher() {
    Assert.assertThrows(new ThrowingBlock() {
    @Override
        public void execute() throws Throwable {
            account.withdraw(10);
        }
    }, InsufficientFundsException.class, hasMessage(startsWith("Not enough")));
}

@Test
public void assertAndReturn() {
    InsufficientFundsException e = Assert.assertThrowsAndReturn(new ThrowingBlock() {
        @Override
        public void execute() throws Throwable {
            account.withdraw(10);
        }
    }, InsufficientFundsException.class);
    // assertions go after the exception block
    // which is more natural than with @Rule
    assertThat(e.getMessage(), hasMessage(startsWith("Not enough")));
    assertTrue(e.getBalance() < e.getRequestedAmount());
}

It may seem wordy, but the only thing that is big here is the the anonymous class. Boilerplate code is completely gone.
The extra bonus here is that being 100% usable right now, after release of jdk8, these methods will become even more usable:

@Test
public void assertWithClass() {
    Assert.assertThrows(() -> account.withdraw(10), InsufficientFundsException.class);
}

@Test
public void assertWithClassAndMatcher() {
    Assert.assertThrows(() -> account.withdraw(10), InsufficientFundsException.class, hasMessage(startsWith("Not enough")));
}

@Test
public void assertAndReturn() {
    InsufficientFundsException e = Assert.assertThrowsAndReturn(() -> account.withdraw(10), InsufficientFundsException.class);
    assertThat(e.getMessage(), hasMessage(startsWith("Not enough")));
    assertTrue(e.getBalance() < e.getRequestedAmount());
}

@Test
public void atmHasNoMercy() {
    for (int i = 0; i < MAX_RETRIES; i++)
        Assert.assertThrows(() -> account.withdraw(10), InsufficientFundsException.class);
}

All this is possible with zero effort from JUnit because ThrowingBlock is a Functional Interface.

I have working prototype of it. If you're interested I can submit it for review.

Thanks.

@dsaff
Member
dsaff commented Aug 19, 2013

This has come up before, although this is an unusually helpful summary (I think I'll use it in the future). JUnit is in a hard position, having, as you say, 3 "official" ways to test for exceptions. Unfortunately, until the widespread usability of Java 8, I think this functional interface is not obviously better than try/catch. So my suggestion has been this:

  • Until availability of Java 8, publicize this as a good solution for those it makes sense for.
  • Once Java 8 is released, push this as the "official best" solution.
@ghost
ghost commented Aug 19, 2013

@dsaff

This has come up before

Sure. I've just googled a little:

I think this functional interface is not obviously better than try/catch

It's better in the same sense that using a procedure is better than copy-pasting its internals every time you need it. As for me these 2 invocation of fail and proper organising of try-catch is something you can easily mess up with.

Once Java 8 is released, push this as the "official best" solution.

OK, I'll be around... waiting :)

@ghost
ghost commented Aug 20, 2013

I don't know if you've seen it or not. Anyway. Pretty nice solution, very concise. Unfortunately involves bytecode magic: catch-exception.

@ghost ghost closed this Aug 20, 2013
@ghost ghost reopened this Aug 20, 2013
@lpandzic

Hello,

this issue was one of the main motivators why I started developing a rule to solve this problem in a simple and elegant way which conforms to the given``when``then syntax of BDD:

givenSomePreconditions()
whenMethodThrowsException()
thenAssertAgainstThrownException()

This idea evolved into the BddRule which I hope provides an alternative solution to make tests simpler and easier to read.

@Test(expected = ...) you cannot say where
exactly the exception was thrown from, with @Rule you have to be cautious. You'll have to state your expectations just before the intended block of code. Otherwise JUnit will be pretty happy while you will get a false-positive test:

It also aims to solve this issue and guarantees the isolation of the unit test in terms of exceptional behavior testing.

@Tibor17
Contributor
Tibor17 commented Feb 19, 2014

@lpandzic
I like the expression language in BDD, however it is some kind of bigger hammer than usual assert* statement.
I can see that we have innovative requirements but IMHO these would not be implemented that fast.
I think we should write concept in these features:

  • first block of code is expected not to throw exception, last one is.
  • exclusive exception in test method if @Test(expected = ...) is used
  • post execution (after exception: optional) finally run () -> {}. Purpose shave same local variables from the stack.
  • using regex on the exception
  • using Hamcrest Matchers on assert* statements, and BDD, e.g. when(Matcher ) then(Callable/Runnable)

Since the community is quite huge some JUnit functionalities can be developed by the same community in separate Maven module. As a result the main module does not have to wait for a sub-module release. When new version of sub-module is ready, the main one will be associated. So both would be developed by the same team but the version will be different. I think this should speed up the realease of JUnit core.

@lpandzic

Yes, I agree. I never meant for this rule to reside inside the JUnit project, otherwise I'd have forked the JUnit project. I only wanted to give an option to people seeking a simple and elegant solution to this problem. Sadly I haven't released it yet since I've been busy with other projects but I do plan to solve it, hopefully soon.

@adrianosimoes

Haven't this been fixed in Junit 5 with assertThrows?
Since as far as I know JUnit 4 will always be Java 7 compatible, should this be closed?

@kcooney
Member
kcooney commented Feb 28, 2017

@adrianosimoes JUnit 4.x also has assertThrows so yes, this can be closed.

@kcooney kcooney closed this Feb 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment