-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Provide better feedback to the user in case of invalid test classes #1286
Merged
marcphilipp
merged 20 commits into
junit-team:master
from
alb-i986:ErrorReportingRunner-provide-better-feedback
Jun 23, 2016
Merged
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
9f0e1a0
Provide better feedback to the user in case of invalid test classes
alb-i986 5c91a16
InvalidTestClassError: add javadoc
alb-i986 93a315a
clean code: move method 'createMessage' close to its caller
alb-i986 4a6e166
Verifying the failure count does not make sense anymore
alb-i986 fa8381b
revert refactoring: I'll handle that in #1285
alb-i986 2c3a896
improve unit tests
alb-i986 45e06ba
InvalidTestClassError: improve param names
alb-i986 ef98f6f
fix unused import
alb-i986 7e7787b
fix unused import
alb-i986 4765762
revert unrelated changes
alb-i986 d5fc61c
fixup! Verifying the failure count does not make sense anymore
alb-i986 2147b76
improve readability
alb-i986 e96a5a5
fix star imports
alb-i986 feec1be
Merge branch 'master' into ErrorReportingRunner-provide-better-feedback
alb-i986 800ad80
mv InvalidTestClassError to org.junit.runners.model
alb-i986 eedafa8
clanup: rm unused import
alb-i986 5e80291
mv InvalidTestClassError to org.junit.runners.model: mv test class as…
alb-i986 a1610b6
rm unused import
alb-i986 3c73d3e
Merge branch 'master' into ErrorReportingRunner-provide-better-feedback
alb-i986 a227269
no need for the test class to be part of the state
alb-i986 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
41 changes: 41 additions & 0 deletions
41
src/main/java/org/junit/runners/model/InvalidTestClassError.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
package org.junit.runners.model; | ||
|
||
import java.util.List; | ||
|
||
/** | ||
* Thrown by {@link org.junit.runner.Runner}s in case the class under test is not valid. | ||
* <p> | ||
* Its message conveniently lists all of the validation errors. | ||
* | ||
* @since 4.13 | ||
*/ | ||
public class InvalidTestClassError extends InitializationError { | ||
private static final long serialVersionUID = 1L; | ||
|
||
private final Class<?> testClass; | ||
private final String message; | ||
|
||
public InvalidTestClassError(Class<?> offendingTestClass, List<Throwable> validationErrors) { | ||
super(validationErrors); | ||
this.testClass = offendingTestClass; | ||
this.message = createMessage(testClass, validationErrors); | ||
} | ||
|
||
private static String createMessage(Class<?> testClass, List<Throwable> validationErrors) { | ||
StringBuilder sb = new StringBuilder(); | ||
sb.append(String.format("Invalid test class '%s':", testClass.getName())); | ||
int i = 1; | ||
for (Throwable error : validationErrors) { | ||
sb.append("\n " + (i++) + ". " + error.getMessage()); | ||
} | ||
return sb.toString(); | ||
} | ||
|
||
/** | ||
* @return a message with a list of all of the validation errors | ||
*/ | ||
@Override | ||
public String getMessage() { | ||
return message; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,24 @@ | ||
package org.junit.internal.runners; | ||
|
||
import org.junit.After; | ||
import org.junit.Before; | ||
import org.junit.Test; | ||
import org.junit.runner.JUnitCore; | ||
import org.junit.runner.Result; | ||
import org.junit.runner.notification.Failure; | ||
import org.junit.runner.notification.RunNotifier; | ||
import org.junit.runners.model.InvalidTestClassError; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.List; | ||
|
||
import static org.hamcrest.CoreMatchers.allOf; | ||
import static org.hamcrest.CoreMatchers.containsString; | ||
import static org.hamcrest.CoreMatchers.instanceOf; | ||
import static org.hamcrest.CoreMatchers.is; | ||
import static org.hamcrest.CoreMatchers.startsWith; | ||
import static org.hamcrest.MatcherAssert.assertThat; | ||
|
||
public class ErrorReportingRunnerTest { | ||
|
||
|
@@ -23,5 +41,48 @@ public void cannotCreateWithNullClasses() { | |
public void cannotCreateWithoutClass() { | ||
new ErrorReportingRunner(new RuntimeException()); | ||
} | ||
|
||
|
||
@Test | ||
public void givenInvalidTestClassErrorAsCause() { | ||
final List<Failure> firedFailures = new ArrayList<Failure>(); | ||
InvalidTestClassError testClassError = new InvalidTestClassError(TestClassWithErrors.class, | ||
Arrays.asList(new Throwable("validation error 1"), new Throwable("validation error 2"))); | ||
ErrorReportingRunner sut = new ErrorReportingRunner(TestClassWithErrors.class, testClassError); | ||
|
||
sut.run(new RunNotifier() { | ||
@Override | ||
public void fireTestFailure(Failure failure) { | ||
super.fireTestFailure(failure); | ||
firedFailures.add(failure); | ||
} | ||
}); | ||
|
||
assertThat(firedFailures.size(), is(1)); | ||
Throwable exception = firedFailures.get(0).getException(); | ||
assertThat(exception, instanceOf(InvalidTestClassError.class)); | ||
assertThat(((InvalidTestClassError) exception), is(testClassError)); | ||
} | ||
|
||
@Test | ||
public void givenInvalidTestClass_integrationTest() { | ||
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 note the integration test here. I'm not sure if this is a good place for it? 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. This is fine. Thanks! |
||
Result result = JUnitCore.runClasses(TestClassWithErrors.class); | ||
|
||
assertThat(result.getFailureCount(), is(1)); | ||
Throwable failure = result.getFailures().get(0).getException(); | ||
assertThat(failure, instanceOf(InvalidTestClassError.class)); | ||
assertThat(failure.getMessage(), allOf( | ||
startsWith("Invalid test class '" + TestClassWithErrors.class.getName() + "'"), | ||
containsString("\n 1. "), | ||
containsString("\n 2. ") | ||
)); | ||
} | ||
|
||
private static class TestClassWithErrors { | ||
@Before public static void staticBeforeMethod() {} | ||
@After public static void staticAfterMethod() {} | ||
|
||
@Test public String testMethodReturningString() { | ||
return "this should not be allowed"; | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
23 changes: 23 additions & 0 deletions
23
src/test/java/org/junit/runners/model/InvalidTestClassErrorTest.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package org.junit.runners.model; | ||
|
||
import org.junit.Test; | ||
|
||
import static java.util.Arrays.asList; | ||
import static org.hamcrest.CoreMatchers.equalTo; | ||
import static org.hamcrest.MatcherAssert.assertThat; | ||
|
||
public class InvalidTestClassErrorTest { | ||
|
||
@Test | ||
public void invalidTestClassErrorShouldListAllValidationErrorsInItsMessage() { | ||
InvalidTestClassError sut = new InvalidTestClassError(SampleTestClass.class, | ||
asList(new Throwable("validation error 1"), new Throwable("validation error 2"))); | ||
|
||
assertThat(sut.getMessage(), equalTo("Invalid test class '" + SampleTestClass.class.getName() + "':" + | ||
"\n 1. validation error 1" + | ||
"\n 2. validation error 2")); | ||
} | ||
|
||
private static class SampleTestClass { | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I don't think we have to store
testClass
since it's only used in the constructor.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.
Uhm ya right.