Skip to content

Commit

Permalink
With ExpectedException, document that there should be no statements a…
Browse files Browse the repository at this point in the history
…fter the exceptional one. (junit-team#1199)

* Add text to the documentation of ExpectedException that points out that it is a mistake to have statements after the one that is expected to throw the exception. A common bug pattern is to have verification statements after that statement, like Mockito.verifyNoMoreInteractions. These will of course never be executed, but an imperfect understanding of exactly how ExpectedException works leads people not to realize that.
* In ExpectedException, recommend the use of Assert.assertThrows.
* In new ExpectedException javadoc text, spell out {@link} tags with complete parameter lists, and remove parentheses around the reference to expectThrows. Also move each <p> to the start of its paragraph text rather than a line on its own.
* Refine the suggestion to use Assert.assertThrows, saying that it applies starting with Java 8.
  • Loading branch information
eamonnmcmanus authored and kcooney committed Sep 18, 2016
1 parent 756959c commit 61f8c83
Showing 1 changed file with 20 additions and 14 deletions.
34 changes: 20 additions & 14 deletions src/main/java/org/junit/rules/ExpectedException.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,14 @@

/**
* The {@code ExpectedException} rule allows you to verify that your code
* throws a specific exception.
* throws a specific exception. Note that, starting with Java 8,
* {@link org.junit.Assert#assertThrows(java.lang.Class, org.junit.Assert.ThrowingRunnable)
* Assert.assertThrows}
* is often a better choice since it allows you to express exactly where you
* expect the exception to be thrown. Use
* {@link org.junit.Assert#expectThrows(java.lang.Class,
* org.junit.Assert.ThrowingRunnable) expectThrows}
* if you need to assert something about the thrown exception.
*
* <h3>Usage</h3>
*
Expand All @@ -34,15 +41,18 @@
* }
* }</pre>
*
* <p>
* You have to add the {@code ExpectedException} rule to your test.
* <p>You have to add the {@code ExpectedException} rule to your test.
* This doesn't affect your existing tests (see {@code throwsNothing()}).
* After specifiying the type of the expected exception your test is
* After specifying the type of the expected exception your test is
* successful when such an exception is thrown and it fails if a
* different or no exception is thrown.
*
* <p>
* Instead of specifying the exception's type you can characterize the
* <p>This rule does not perform any special magic to make execution continue
* as if the exception had not been thrown. So it is nearly always a mistake
* for a test method to have statements after the one that is expected to
* throw the exception.
*
* <p>Instead of specifying the exception's type you can characterize the
* expected exception based on other criteria, too:
*
* <ul>
Expand All @@ -52,8 +62,7 @@
* <li>The exception itself complies with a Hamcrest matcher: {@link #expect(Matcher)}</li>
* </ul>
*
* <p>
* You can combine any of the presented expect-methods. The test is
* <p>You can combine any of the presented expect-methods. The test is
* successful if all specifications are met.
* <pre> &#064;Test
* public void throwsException() {
Expand All @@ -63,8 +72,7 @@
* }</pre>
*
* <h3>AssumptionViolatedExceptions</h3>
* <p>
* JUnit uses {@link AssumptionViolatedException}s for indicating that a test
* <p>JUnit uses {@link AssumptionViolatedException}s for indicating that a test
* provides no useful information. (See {@link org.junit.Assume} for more
* information.) You have to call {@code assume} methods before you set
* expectations of the {@code ExpectedException} rule. In this case the rule
Expand All @@ -79,8 +87,7 @@
*
* <h3>AssertionErrors</h3>
*
* <p>
* JUnit uses {@link AssertionError}s for indicating that a test is failing. You
* <p>JUnit uses {@link AssertionError}s for indicating that a test is failing. You
* have to call {@code assert} methods before you set expectations of the
* {@code ExpectedException} rule, if they should be handled by the framework.
* E.g. the following test fails because of the {@code assertTrue} statement.
Expand All @@ -92,8 +99,7 @@
* }</pre>
*
* <h3>Missing Exceptions</h3>
* <p>
* By default missing exceptions are reported with an error message
* <p>By default missing exceptions are reported with an error message
* like "Expected test to throw an instance of foo". You can configure a different
* message by means of {@link #reportMissingExceptionWithMessage(String)}. You
* can use a {@code %s} placeholder for the description of the expected
Expand Down

0 comments on commit 61f8c83

Please sign in to comment.