-
Notifications
You must be signed in to change notification settings - Fork 3.3k
RuleChain#around should not allow null args #1313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
38479a7
ea3217b
4c17e42
83510e1
e173ec0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,8 +77,12 @@ private RuleChain(List<TestRule> rules) { | |
* | ||
* @param enclosedRule the rule to enclose. | ||
* @return a new {@code RuleChain}. | ||
* @throws NullPointerException if the argument {@code enclosedRule} is null | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please change this line to |
||
*/ | ||
public RuleChain around(TestRule enclosedRule) { | ||
if (enclosedRule == null) { | ||
throw new NullPointerException("The enclosed rule should not be null"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please replace "should" with "must". |
||
} | ||
List<TestRule> rulesOfNewChain = new ArrayList<TestRule>(); | ||
rulesOfNewChain.add(enclosedRule); | ||
rulesOfNewChain.addAll(rulesStartingWithInnerMost); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,25 @@ | ||
package org.junit.rules; | ||
|
||
import static java.util.Arrays.asList; | ||
import static org.hamcrest.CoreMatchers.containsString; | ||
import static org.hamcrest.CoreMatchers.equalTo; | ||
import static org.hamcrest.MatcherAssert.assertThat; | ||
import static org.junit.Assert.assertEquals; | ||
import static org.junit.Assert.assertTrue; | ||
import static org.junit.Assert.fail; | ||
import static org.junit.experimental.results.PrintableResult.testResult; | ||
import static org.junit.rules.RuleChain.outerRule; | ||
|
||
import java.io.PrintWriter; | ||
import java.io.StringWriter; | ||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
import org.junit.Rule; | ||
import org.junit.Test; | ||
import org.junit.runner.Description; | ||
import org.junit.runner.JUnitCore; | ||
import org.junit.runner.Result; | ||
|
||
public class RuleChainTest { | ||
private static final List<String> LOG = new ArrayList<String>(); | ||
|
@@ -55,4 +63,41 @@ public void executeRulesInCorrectOrder() throws Exception { | |
"finished outer rule"); | ||
assertEquals(expectedLog, LOG); | ||
} | ||
|
||
@Test | ||
public void aroundShouldNotAllowNullRules() { | ||
RuleChain chain = RuleChain.emptyRuleChain(); | ||
try { | ||
chain.around(null); | ||
fail("around() should not allow null rules"); | ||
} catch (NullPointerException e) { | ||
assertThat(e.getMessage(), equalTo("The enclosed rule should not be null")); | ||
} | ||
} | ||
|
||
public static class RuleChainWithNullRules { | ||
@Rule | ||
public final RuleChain chain = outerRule(new LoggingRule("outer rule")) | ||
.around(null); | ||
|
||
@Test | ||
public void example() {} | ||
} | ||
|
||
@Test | ||
public void whenRuleChainHasNullRuleTheStacktraceShouldPointToIt() { | ||
Result result = JUnitCore.runClasses(RuleChainWithNullRules.class); | ||
|
||
assertThat(result.getFailures().size(), equalTo(1)); | ||
String stacktrace = stacktraceToString(result.getFailures().get(0).getException()); | ||
assertThat(stacktrace, containsString("\tat org.junit.rules.RuleChainTest$RuleChainWithNullRules.<init>(RuleChainTest.java:")); | ||
} | ||
|
||
// TODO delete as soon as #1312 is merged | ||
private static String stacktraceToString(Throwable e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case you are going to merge #1312 soon, please merge that before this, so that I can remove this method. |
||
StringWriter sw = new StringWriter(); | ||
PrintWriter pw = new PrintWriter(sw); | ||
e.printStackTrace(pw); | ||
return sw.toString(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change this line to
@param enclosedRule the rule to enclose; must not be {@code null}.