1

e.g, in verifySthIsSetCorrectly() of following code, should I use assertEquals() to check result or should I throw exception so that it is caught by try...catch of the caller and let caller to handle?

@Parameters
public static Collection<object[]> methodParams() {
    List<Object[]> params = new ArrayList<Object[]>();
    /* arg, errorString */
    params.add(new Object[] {"arg1", null /*errorString*/});
    params.add(new Object[] {"arg2", ERROR_STRING1 /*errorString*/});
}
@Test
public void sampleTest () {
    try {
        MethodAndWait(arg);
        assertNull("expect error String" + errorString, errorString);
    } catch (Exception ex) {
        assertNotNull("expect error String" + errorString, errorString);
        assertTrue(ex.getMessage().contains(errorString));
    }
}

private void MethodAndWait() {
    call_remote_server_to_have_sth_set;
    verifySthIsSetCorrectly();
}

private void verifySthIsSetCorrectly() {
    int sth = getSth();
    assertEquals(sth == "5");
}
kryger
  • 12,906
  • 8
  • 44
  • 65
user389955
  • 9,605
  • 14
  • 56
  • 98

3 Answers3

4

In a JUnit test, you should use assertions like assertEquals() to verify the result of a method call or the state of an object:

@Test
public void addingTwoNumbersShouldWork() {
  int result = calculator.add(5, 7);

  assertEquals(12, result);
  assertFalse(calculator.hasOverflow());
}

It's extremely rare to use try and catch in a JUnit test for anything other than testing that a code block throws an expected exception:

@Test
public void setColorShouldThrowNullPointerExceptionOnNullInput() {
  try {
    deathRay.setColor(null);
    fail("expected NullPointerException");
  } catch (NullPointerException expected) {
    assertThat(expected.getMessage(), contains("death ray color"));
  }
}

You do not need to use try and catch if the method you are testing happens to throw an exception:

@Test
public void fireDeathRay() throws DeathRayException {
  deathRay.fire();
}

In the above test, if fire() throws a DeathRayException (or a runtime exception) the fireDeathRay test will fail.

In JUnit4, it's even rarer to use try and catch, because you can use the ExpectedException rule to check if a call throws an expected exception.

Community
  • 1
  • 1
NamshubWriter
  • 23,549
  • 2
  • 41
  • 59
  • got it. so should use private void verifySthIsSetCorrectly() throw exception {} because the purpose is not to test its expected error, but to test expected error of sampleTest() – user389955 Jul 22 '15 at 07:03
  • @user389955 Sorry, I don't understand what your test is doing enough to agree or disagree with your comment – NamshubWriter Jul 22 '15 at 14:27
1

Your test should be

@Test
public void sampleTest () {
    call_remote_server_to_have_sth_set;
    int sth = getSth();
    assertEquals(5, sth);
}

I would recommend to read an introduction to testing with JUnit if you haven't already done it.

Stefan Birkner
  • 24,059
  • 12
  • 57
  • 72
  • Thanks Stefan. the code in verifySthIsSetCorrectly() is not as short as what I typed. that is why I put it in a function. – user389955 Jul 10 '15 at 19:08
0

I'm going to take the unusual step of adding another answer after my answer was accepted. The previous answer focused on the question raised in the summary, but I wanted to focus on the code.

I think one of the reasons why you were wondering what to do is because the test sampleTest() is testing two completely different things. Your test method is testing normal behavior and exceptional behavior in the same test method.

Instead, split out the testing of the exceptional cases into their own test methods. For example:

@RunWith(JUnit4.class)
public class SampleTest {

  @Test
  public void methodAndWaitShouldAcceptNonNullValue() {
    ClassUnderTest.methodAndWait("arg1")
  }

  @Test
  public void methodAndWaitShouldThrowWhenGivenNullValue() {
    try {
      ClassUnderTest.methodAndWait(null);
      fail("NullPointerException was not thrown");
    } catch (NullPointerException ex) {
      assertTrue(ex.getMessage().contains(ERROR_STRING1));
    }
  }
}

This has several advantages:

  1. If methodAndWait("arg1") throws an exception, the test will fail with a useful stack trace
  2. If methodAndWait(null) throws something other than NullPointerException, the test will fail with a useful stack trace
  3. If methodAndWait(null) doesn't throw anything, the test will fail with a useful message
  4. The intent of both tests is very clear

If you need to test with multiple arguments, you can use the Enclosed runner:

@RunWith(Enclosed.class)
public class SampleTest {

  @RunWith(Parameterized.class)
  public static class WhenPassingNonNull {
    @Parameters
    public static Collection<object[]> methodParams() {
      List<Object[]> params = new ArrayList<Object[]>();
      /* arg, errorString */
      params.add(new Object[] {"arg1", "result1"});
      params.add(new Object[] {"arg3", "result3"});
    }

    public final String arg;
    public final String actualResult;

    public SampleTest(String arg, String actualResult) {
      this.arg = arg;
      this.actualResult = actualResult;
    }

    @Test
    public void sampleTest() {
      String actualResult = ClassUnderTest.methodAndWait(arg);

      assertEquals(expectedResult, actualResult);
    }
  }

  @RunWith(JUnit4.class)
  public static class WhenPassingNull {
    @Test
    public void shouldThrowNullPointerException() {
      try {
        ClassUnderTest.methodAndWait(null);
        fail();
      } catch (NullPointerException ex) {
        assertTrue(ex.getMessage().contains(ERROR_STRING1));
      }
    }
  }
}
NamshubWriter
  • 23,549
  • 2
  • 41
  • 59
  • Hi, NamshubWriter: the question I really wanted to ask is: I want to test sampleTest() to make sure with different arguments(I have a lot of arguments, and corresponding errorMessage to test), we get corresponding errorMessage correctly. I do not mean to test methodAndWait() although I have to call methodAndWait() in sampleTest(). And I assume when testing sampleTest(), methodAndWait() should always return normally. But it is possible that methodAndWait() itself may catch exception. (think about replace methodAndWait() with createTable(). we – user389955 Jul 28 '15 at 01:33
  • want to run some test about a table and we call createTable() first to make table ready. but it is possible that createTable() throw an exception because table is not created successfully) in this case should I just make the test fails or should I pass the failure to sampleTest() and let sampleTest() to match the argument - errorMassage list and decide what to do? – user389955 Jul 28 '15 at 01:34
  • I wont use Enclose.class because it require to change everything as static – user389955 Jul 28 '15 at 01:36
  • @user389955 As long as you split tests where you expect the production code to throw from tests that expect the production code not to fail, I think you are fine. Don't worry about methods that should work but could fail with an exception; just update your throws clause to list any checked exceptions thrown by `createTable()`. If you don't like `Enclosed` then perhaps consider using JUnit-params so you can have per-method parameters. – NamshubWriter Jul 28 '15 at 02:35