4

Recently, in a code review, a colleague said "Tests should not have try-catch blocks". He also pointed me to information about ExpectedException, and I was able to use it to rewrite some of my tests that were performing checks on the exception data.

However, I'm encountering a situation where I'm not sure I can eliminate the try-catch. I'm writing a unit test for a method that will throw an exception and that will perform some behavior that I need to verify using JMockit. Suppose the code under test contains something like

try {
    ...code...
} catch (SomeException e) {
    statusMessage.send("Failure", <parameters with some data>);
    throw e;
}

The JUnit test currently looks like

@Test
public void test() {
    ... set things up ...
    try {
        callTheMethod();
        fail("No exception thrown");
    } catch (Exception e) {
        assertEquals(SomeException.class, e.getClass());
    }
    new Verifications() { {
        statusMessage.send("Failure", <expected data>);
    } }
}

I can't find a good way to get rid of the try-catch. I don't see a problem with having it there, personally, but if I don't at least try to get rid of it I'm likely to catch heck from my colleague :) :) :)

The fail and assertEquals aren't necessary. If there's a simple way to tell JUnit/JMockit to simply ignore any exception from callTheMethod() and continue, that would be fine--I can create another test to check the exception.

Is there some JUnit or JMockit feature that would let me accomplish what I'm trying to do?

[Note: Edited to make it clear that the behavior I'm testing is a call on a mocked object, not a static method call. It really isn't relevant to the question.]

ajb
  • 31,309
  • 3
  • 58
  • 84
  • 1
    It's stupid to make a blanket statement that tests should not have try-catch blocks. Rather, tests should not catch exceptions unless that specific exception is expected as part of that test. – gknicker Nov 18 '15 at 04:34
  • 2
    @gknicker You could have stopped your comment after the first seven words. – ajb Nov 18 '15 at 04:36
  • Maybe some middleground: instead of `assertEquals(..` you could potentially put the `new Verification` part into the catch block and then `throw e` afterwards. Wouldn't bother personally because you're not only interested in the exception but also what happens in between and that's simply not the standard "exception expected" test. – zapl Nov 18 '15 at 04:40

4 Answers4

2

Your colleague may be right in the general sense, using try-catch to test your exception handling is less desirable than using the provided ExpectedException utilities. However, I don't think that rule applies to your case; catching the exception so you can verify other things is perfectly reasonable. There's no other way to stop an exception from from propagating, AFAIK.

Rosa
  • 642
  • 6
  • 20
1

Currently with plain JUnit you have to use try-catch if you want to verify something after the assertion is thrown. With JUnit 4.13 (not released yet) you can use

expectThrows(Exception.class, () -> callTheMethod());
new Verifications() { {
    statusMessage.send("Failure", <expected data>);
} }

As an alternative you can use the Fishbowl's ignoreException.

ignoreException(() -> callTheMethod());
new Verifications() { {
    statusMessage.send("Failure", <expected data>);
} }

Full disclosure: I'm the author of Fishbowl.

Stefan Birkner
  • 24,059
  • 12
  • 57
  • 72
  • Cool! That will be useful. Don't think we have Fishbowl available for production use, so I'll look forward to 4.13 coming out. – ajb Nov 18 '15 at 15:48
  • You can simply copy the `ignoreException` method from Fishbowl if you don't have Fishbowl available. – Stefan Birkner Nov 18 '15 at 20:01
-1

Based on the API, I'd say you could do something like:

class AjbUnitTest
{
    @Rule 
    public ExpectedException thrown = ExpectedException.none();

    @Test
    public void test()
    {
        ... set things up ...
        thrown.expect(SomeException.class)
        callTheMethod();  / Preumably this throws SomeException
        ...
    }
}
dave
  • 11,641
  • 5
  • 47
  • 65
  • 1
    But when `callTheMethod()` throws the exception, it's all over for the test. If there are `Verifications` following that line, they won't get checked. I believe I already tested this, and that's the behavior I observed. – ajb Nov 18 '15 at 04:26
  • Oh okay. If you need to keep going, then you need the `try .. catch`. I think the rule is sensible, but rules can be broken if warranted. I think this is such a situation. – dave Nov 18 '15 at 04:52
-2

For exception specific assertions AssertJ has a good API:

@Test
public void testException() {
   assertThatThrownBy(() -> { throw new Exception("error!") })
     .isInstanceOf(Exception.class)
     .hasMessageContaining("error");
}

Basically, you can "collect" the exception without disrupting the test execution.

S.D.
  • 29,290
  • 3
  • 79
  • 130
  • While you're probably right about the `StatusMessage` (and I did use a mock in the real code, I was just trying to throw a simplified example together quickly), this in no way answers the question. – ajb Nov 18 '15 at 04:38
  • @ajb Code provided in question, is assumed to be part of the question. – S.D. Nov 18 '15 at 04:44
  • I think it's pretty well established on SO that if you want to say something about a questioner's code that doesn't relate to the question they asked, it should be a comment and not a separate answer. – ajb Nov 18 '15 at 04:49