From c71c8784a0a5b95da6b372a71312aadb969f39e9 Mon Sep 17 00:00:00 2001 From: Alberto Scotto Date: Fri, 13 May 2016 00:46:43 +0100 Subject: [PATCH 1/7] Fix #1290 Tests annotated with `@Test(expected = AssumptionViolatedException.class)` which throw AssumptionViolatedException should be marked as passing, not skipped. --- .../junit/internal/runners/statements/ExpectException.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/junit/internal/runners/statements/ExpectException.java b/src/main/java/org/junit/internal/runners/statements/ExpectException.java index d0636bd78bbb..9a2a952858a3 100644 --- a/src/main/java/org/junit/internal/runners/statements/ExpectException.java +++ b/src/main/java/org/junit/internal/runners/statements/ExpectException.java @@ -19,7 +19,9 @@ public void evaluate() throws Exception { next.evaluate(); complete = true; } catch (AssumptionViolatedException e) { - throw e; + if (!expected.isAssignableFrom(e.getClass())) { + throw e; + } } catch (Throwable e) { if (!expected.isAssignableFrom(e.getClass())) { String message = "Unexpected exception, expected<" From 939ade5c4bba7b3b8a88162af2b22166bcf6dece Mon Sep 17 00:00:00 2001 From: Alberto Scotto Date: Fri, 13 May 2016 01:02:01 +0100 Subject: [PATCH 2/7] add unit test --- .../org/junit/internal/AllInternalTests.java | 2 ++ .../statements/ExpectExceptionTest.java | 25 +++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java diff --git a/src/test/java/org/junit/internal/AllInternalTests.java b/src/test/java/org/junit/internal/AllInternalTests.java index c6f7ce29cd96..b23ac466de73 100644 --- a/src/test/java/org/junit/internal/AllInternalTests.java +++ b/src/test/java/org/junit/internal/AllInternalTests.java @@ -4,6 +4,7 @@ import org.junit.internal.matchers.StacktracePrintingMatcherTest; import org.junit.internal.matchers.ThrowableCauseMatcherTest; import org.junit.internal.runners.ErrorReportingRunnerTest; +import org.junit.internal.runners.statements.ExpectExceptionTest; import org.junit.internal.runners.statements.FailOnTimeoutTest; import org.junit.runner.RunWith; import org.junit.runners.Suite; @@ -13,6 +14,7 @@ @SuiteClasses({ AnnotatedBuilderTest.class, ErrorReportingRunnerTest.class, + ExpectExceptionTest.class, FailOnTimeoutTest.class, MethodSorterTest.class, StacktracePrintingMatcherTest.class, diff --git a/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java b/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java new file mode 100644 index 000000000000..592f3eb4f2d1 --- /dev/null +++ b/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java @@ -0,0 +1,25 @@ +package org.junit.internal.runners.statements; + +import org.junit.Test; +import org.junit.internal.AssumptionViolatedException; +import org.junit.runners.model.Statement; + +public class ExpectExceptionTest { + + @Test + public void givenWeAreExpectingAssumptionViolatedExceptionAndAStatementThrowingAssumptionViolatedException() throws Exception { + ExpectException sut = new ExpectException(new StatementThrowingAssumptionViolatedException(), AssumptionViolatedException.class); + + sut.evaluate(); + + // then no exception should be thrown + } + + + private static class StatementThrowingAssumptionViolatedException extends Statement { + @Override + public void evaluate() throws Throwable { + throw new AssumptionViolatedException("expected"); + } + } +} \ No newline at end of file From 6def9630ac6d9738a0b16893d5b9770e6edc9b5c Mon Sep 17 00:00:00 2001 From: Alberto Scotto Date: Fri, 13 May 2016 01:07:02 +0100 Subject: [PATCH 3/7] fix unit test If the AssumptionVaiolatedException is thrown, the unit test should fail, instead it would have been marked as skipped! --- .../runners/statements/ExpectExceptionTest.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java b/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java index 592f3eb4f2d1..e21695ccf42a 100644 --- a/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java +++ b/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java @@ -4,15 +4,20 @@ import org.junit.internal.AssumptionViolatedException; import org.junit.runners.model.Statement; +import static org.junit.Assert.*; + public class ExpectExceptionTest { @Test - public void givenWeAreExpectingAssumptionViolatedExceptionAndAStatementThrowingAssumptionViolatedException() throws Exception { + public void givenWeAreExpectingAssumptionViolatedExceptionAndAStatementThrowingAssumptionViolatedException() { ExpectException sut = new ExpectException(new StatementThrowingAssumptionViolatedException(), AssumptionViolatedException.class); - sut.evaluate(); - - // then no exception should be thrown + try { + sut.evaluate(); + // then no exception should be thrown + } catch (Throwable e) { + fail("should not throw anything, but was thrown: " + e); + } } From e6aee89f5a5f3dedb0518d659588783a51279c69 Mon Sep 17 00:00:00 2001 From: Alberto Scotto Date: Fri, 13 May 2016 01:13:28 +0100 Subject: [PATCH 4/7] fix style: rm double return line --- .../junit/internal/runners/statements/ExpectExceptionTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java b/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java index e21695ccf42a..610dad99f219 100644 --- a/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java +++ b/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java @@ -20,7 +20,6 @@ public void givenWeAreExpectingAssumptionViolatedExceptionAndAStatementThrowingA } } - private static class StatementThrowingAssumptionViolatedException extends Statement { @Override public void evaluate() throws Throwable { From 4a79e7558589f4dace8d9c82b6b74982969c1082 Mon Sep 17 00:00:00 2001 From: Alberto Scotto Date: Fri, 13 May 2016 11:01:08 +0100 Subject: [PATCH 5/7] add integration test --- .../junit/tests/running/methods/ExpectedTest.java | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/test/java/org/junit/tests/running/methods/ExpectedTest.java b/src/test/java/org/junit/tests/running/methods/ExpectedTest.java index 236de9d72908..90e55f3157f3 100644 --- a/src/test/java/org/junit/tests/running/methods/ExpectedTest.java +++ b/src/test/java/org/junit/tests/running/methods/ExpectedTest.java @@ -5,6 +5,7 @@ import static org.junit.Assert.assertTrue; import org.junit.Test; +import org.junit.internal.AssumptionViolatedException; import org.junit.runner.JUnitCore; import org.junit.runner.Result; import org.junit.runner.notification.Failure; @@ -67,4 +68,16 @@ public void throwsSubclass() { public void expectsSuperclass() { assertTrue(new JUnitCore().run(ExpectSuperclass.class).wasSuccessful()); } + + public static class ExpectAssumptionViolatedException { + @Test(expected = AssumptionViolatedException.class) + public void throwsAssumptionViolatedException() { + throw new AssumptionViolatedException("expected"); + } + } + + @Test + public void expectsAssumptionViolatedException() { + assertTrue(new JUnitCore().run(ExpectAssumptionViolatedException.class).wasSuccessful()); + } } From aa582cbe596d5fbaf2a7d24065c496953e247c17 Mon Sep 17 00:00:00 2001 From: Alberto Scotto Date: Thu, 19 May 2016 15:31:55 +0100 Subject: [PATCH 6/7] addressing review comments - var name: sut -> expectException - rename test method name - class javadoc linking to the integration tests - add a test verifying that tests that expect AssumptionViolatedException but do not throw should fail - add scenario where the given statement passes - add scenario where the statement throws subclass --- .../statements/ExpectExceptionTest.java | 66 +++++++++++++++++-- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java b/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java index 610dad99f219..da7e245fd4fa 100644 --- a/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java +++ b/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java @@ -4,26 +4,78 @@ import org.junit.internal.AssumptionViolatedException; import org.junit.runners.model.Statement; -import static org.junit.Assert.*; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; +/** + * Integration tests can be found in {@link org.junit.tests.running.methods.ExpectedTest}. + * See e.g. {@link org.junit.tests.running.methods.ExpectedTest#expectsAssumptionViolatedException()} + */ public class ExpectExceptionTest { @Test - public void givenWeAreExpectingAssumptionViolatedExceptionAndAStatementThrowingAssumptionViolatedException() { - ExpectException sut = new ExpectException(new StatementThrowingAssumptionViolatedException(), AssumptionViolatedException.class); + public void whenExpectingAssumptionViolatedExceptionStatementsThrowingItShouldPass() { + Statement statementThrowingAssumptionViolatedException = new Fail(new AssumptionViolatedException("expected")); + ExpectException expectException = new ExpectException(statementThrowingAssumptionViolatedException, AssumptionViolatedException.class); try { - sut.evaluate(); + expectException.evaluate(); // then no exception should be thrown } catch (Throwable e) { fail("should not throw anything, but was thrown: " + e); } } - private static class StatementThrowingAssumptionViolatedException extends Statement { - @Override + @Test + public void whenExpectingAssumptionViolatedExceptionStatementsThrowingSubclassShouldPass() { + Statement statementThrowingAssumptionViolatedExceptionSubclass = new Fail(new org.junit.AssumptionViolatedException("expected")); + ExpectException expectException = new ExpectException(statementThrowingAssumptionViolatedExceptionSubclass, AssumptionViolatedException.class); + + try { + expectException.evaluate(); + // then no exception should be thrown + } catch (Throwable e) { + fail("should not throw anything, but was thrown: " + e); + } + } + + @Test + public void whenExpectingAssumptionViolatedExceptionStatementsThrowingDifferentExceptionShouldFail() { + Statement statementThrowingSomeException = new Fail(new SomeException("not expected")); + ExpectException expectException = new ExpectException(statementThrowingSomeException, AssumptionViolatedException.class); + + try { + expectException.evaluate(); + fail("should throw 'Unexpected exception' when statement throws an exception which is not the one expected"); + } catch (Exception e) { + assertThat(e.getMessage(), equalTo("Unexpected exception, expected " + + "but was")); + } + } + + @Test + public void whenExpectingAssumptionViolatedExceptionStatementsPassingShouldFail() throws Exception { + ExpectException expectException = new ExpectException(new PassingStatement(), AssumptionViolatedException.class); + + try { + expectException.evaluate(); + fail("ExpectException should throw when the given statement passes"); + } catch (AssertionError e) { + assertThat(e.getMessage(), containsString("Expected exception: " + AssumptionViolatedException.class.getName())); + } + } + + private static class PassingStatement extends Statement { public void evaluate() throws Throwable { - throw new AssumptionViolatedException("expected"); + // nop + } + } + + private static class SomeException extends RuntimeException { + public SomeException(String message) { + super(message); } } } \ No newline at end of file From 95800b671f1dc747ecc988e1342679d2245f84bc Mon Sep 17 00:00:00 2001 From: Alberto Scotto Date: Tue, 24 Jan 2017 00:31:08 +0100 Subject: [PATCH 7/7] adressing review comments --- .../statements/ExpectExceptionTest.java | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java b/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java index da7e245fd4fa..568d0b6bfa89 100644 --- a/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java +++ b/src/test/java/org/junit/internal/runners/statements/ExpectExceptionTest.java @@ -17,21 +17,21 @@ public class ExpectExceptionTest { @Test public void whenExpectingAssumptionViolatedExceptionStatementsThrowingItShouldPass() { - Statement statementThrowingAssumptionViolatedException = new Fail(new AssumptionViolatedException("expected")); - ExpectException expectException = new ExpectException(statementThrowingAssumptionViolatedException, AssumptionViolatedException.class); + Statement delegate = new Fail(new AssumptionViolatedException("expected")); + ExpectException expectException = new ExpectException(delegate, AssumptionViolatedException.class); try { expectException.evaluate(); - // then no exception should be thrown - } catch (Throwable e) { + // then AssumptionViolatedException should not be thrown + } catch (Throwable e) { // need to explicitly catch and re-throw as an AssertionError or it might be skipped fail("should not throw anything, but was thrown: " + e); } } @Test public void whenExpectingAssumptionViolatedExceptionStatementsThrowingSubclassShouldPass() { - Statement statementThrowingAssumptionViolatedExceptionSubclass = new Fail(new org.junit.AssumptionViolatedException("expected")); - ExpectException expectException = new ExpectException(statementThrowingAssumptionViolatedExceptionSubclass, AssumptionViolatedException.class); + Statement delegate = new Fail(new AssumptionViolatedExceptionSubclass("expected")); + ExpectException expectException = new ExpectException(delegate, AssumptionViolatedException.class); try { expectException.evaluate(); @@ -43,8 +43,8 @@ public void whenExpectingAssumptionViolatedExceptionStatementsThrowingSubclassSh @Test public void whenExpectingAssumptionViolatedExceptionStatementsThrowingDifferentExceptionShouldFail() { - Statement statementThrowingSomeException = new Fail(new SomeException("not expected")); - ExpectException expectException = new ExpectException(statementThrowingSomeException, AssumptionViolatedException.class); + Statement delegate = new Fail(new SomeException("not expected")); + ExpectException expectException = new ExpectException(delegate, AssumptionViolatedException.class); try { expectException.evaluate(); @@ -61,10 +61,11 @@ public void whenExpectingAssumptionViolatedExceptionStatementsPassingShouldFail( try { expectException.evaluate(); - fail("ExpectException should throw when the given statement passes"); } catch (AssertionError e) { assertThat(e.getMessage(), containsString("Expected exception: " + AssumptionViolatedException.class.getName())); + return; } + fail("ExpectException should throw when the given statement passes"); } private static class PassingStatement extends Statement { @@ -78,4 +79,10 @@ public SomeException(String message) { super(message); } } + + private static class AssumptionViolatedExceptionSubclass extends AssumptionViolatedException { + public AssumptionViolatedExceptionSubclass(String assumption) { + super(assumption); + } + } } \ No newline at end of file