From 5c2070d7cf981e2e103aef6fd0ef64c473331325 Mon Sep 17 00:00:00 2001 From: pimterry Date: Sun, 17 Feb 2013 14:23:26 +0000 Subject: [PATCH 1/4] Fixes #449, stopping AllMembersSupplier hiding DataPoint method exceptions Also includes a rewrite of the Theory nullsAccepted code, since that relied on the previous behaviour of this. --- .../theories/ParameterSupplier.java | 2 +- .../junit/experimental/theories/Theories.java | 23 +++++--- .../theories/internal/AllMembersSupplier.java | 10 +--- .../theories/internal/Assignments.java | 25 +++----- .../theories/AssumingInTheoriesTest.java | 9 +-- .../theories/TheoryTestUtils.java | 14 ++++- .../extendingwithstubs/StubbedTheories.java | 2 +- .../internal/AllMembersSupplierTest.java | 58 ++++++++++++------- .../SpecificDataPointsSupplierTest.java | 12 ++-- .../theories/runner/WithDataPointMethod.java | 48 +++++++++------ .../runner/WithExtendedParameterSources.java | 43 ++++++++++++-- .../theories/runner/WithNamedDataPoints.java | 2 +- .../runner/WithParameterSupplier.java | 2 +- 13 files changed, 158 insertions(+), 92 deletions(-) diff --git a/src/main/java/org/junit/experimental/theories/ParameterSupplier.java b/src/main/java/org/junit/experimental/theories/ParameterSupplier.java index c2981ee01b10..11916b31699e 100644 --- a/src/main/java/org/junit/experimental/theories/ParameterSupplier.java +++ b/src/main/java/org/junit/experimental/theories/ParameterSupplier.java @@ -3,5 +3,5 @@ import java.util.List; public abstract class ParameterSupplier { - public abstract List getValueSources(ParameterSignature sig); + public abstract List getValueSources(ParameterSignature sig) throws Throwable; } diff --git a/src/main/java/org/junit/experimental/theories/Theories.java b/src/main/java/org/junit/experimental/theories/Theories.java index b4d8a28052f2..056ee1788f3e 100644 --- a/src/main/java/org/junit/experimental/theories/Theories.java +++ b/src/main/java/org/junit/experimental/theories/Theories.java @@ -9,7 +9,7 @@ import java.util.List; import org.junit.Assert; -import org.junit.experimental.theories.PotentialAssignment.CouldNotGenerateValueException; +import org.junit.Assume; import org.junit.experimental.theories.internal.Assignments; import org.junit.experimental.theories.internal.ParameterizedAssertionError; import org.junit.internal.AssumptionViolatedException; @@ -202,8 +202,13 @@ protected Statement methodInvoker(FrameworkMethod method, Object test) { @Override public Object createTest() throws Exception { - return getTestClass().getOnlyConstructor().newInstance( - complete.getConstructorArguments(nullsOk())); + Object[] params = complete.getConstructorArguments(); + + if (!nullsOk()) { + Assume.assumeNotNull(params); + } + + return getTestClass().getOnlyConstructor().newInstance(params); } }.methodBlock(fTestMethod).evaluate(); } @@ -213,13 +218,13 @@ private Statement methodCompletesWithParameters( return new Statement() { @Override public void evaluate() throws Throwable { - try { - final Object[] values = complete.getMethodArguments( - nullsOk()); - method.invokeExplosively(freshInstance, values); - } catch (CouldNotGenerateValueException e) { - // ignore + final Object[] values = complete.getMethodArguments(); + + if (!nullsOk()) { + Assume.assumeNotNull(values); } + + method.invokeExplosively(freshInstance, values); } }; } diff --git a/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java b/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java index 873e07c6af2a..ba12fa2f5813 100644 --- a/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java +++ b/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java @@ -58,7 +58,7 @@ public AllMembersSupplier(TestClass type) { } @Override - public List getValueSources(ParameterSignature sig) { + public List getValueSources(ParameterSignature sig) throws Throwable { List list = new ArrayList(); addSinglePointFields(sig, list); @@ -69,13 +69,9 @@ public List getValueSources(ParameterSignature sig) { return list; } - private void addMultiPointMethods(ParameterSignature sig, List list) { + private void addMultiPointMethods(ParameterSignature sig, List list) throws Throwable { for (FrameworkMethod dataPointsMethod : getDataPointsMethods(sig)) { - try { - addMultiPointArrayValues(sig, dataPointsMethod.getName(), list, dataPointsMethod.invokeExplosively(null)); - } catch (Throwable e) { - // ignore and move on - } + addMultiPointArrayValues(sig, dataPointsMethod.getName(), list, dataPointsMethod.invokeExplosively(null)); } } diff --git a/src/main/java/org/junit/experimental/theories/internal/Assignments.java b/src/main/java/org/junit/experimental/theories/internal/Assignments.java index 2c7fa5191ba6..78ec44398259 100644 --- a/src/main/java/org/junit/experimental/theories/internal/Assignments.java +++ b/src/main/java/org/junit/experimental/theories/internal/Assignments.java @@ -61,21 +61,17 @@ public Assignments assignNext(PotentialAssignment source) { fUnassigned.size()), fClass); } - public Object[] getActualValues(int start, int stop, boolean nullsOk) + public Object[] getActualValues(int start, int stop) throws CouldNotGenerateValueException { Object[] values = new Object[stop - start]; for (int i = start; i < stop; i++) { - Object value = fAssigned.get(i).getValue(); - if (value == null && !nullsOk) { - throw new CouldNotGenerateValueException(); - } - values[i - start] = value; + values[i - start] = fAssigned.get(i).getValue(); } return values; } public List potentialsForNextUnassigned() - throws Exception { + throws Throwable { ParameterSignature unassigned = nextUnassigned(); return getSupplier(unassigned).getValueSources(unassigned); } @@ -107,20 +103,17 @@ private ParameterSupplier buildParameterSupplierFromClass( return cls.newInstance(); } - public Object[] getConstructorArguments(boolean nullsOk) + public Object[] getConstructorArguments() throws CouldNotGenerateValueException { - return getActualValues(0, getConstructorParameterCount(), nullsOk); + return getActualValues(0, getConstructorParameterCount()); } - public Object[] getMethodArguments(boolean nullsOk) - throws CouldNotGenerateValueException { - return getActualValues(getConstructorParameterCount(), - fAssigned.size(), nullsOk); + public Object[] getMethodArguments() throws CouldNotGenerateValueException { + return getActualValues(getConstructorParameterCount(), fAssigned.size()); } - public Object[] getAllArguments(boolean nullsOk) - throws CouldNotGenerateValueException { - return getActualValues(0, fAssigned.size(), nullsOk); + public Object[] getAllArguments() throws CouldNotGenerateValueException { + return getActualValues(0, fAssigned.size()); } private int getConstructorParameterCount() { diff --git a/src/test/java/org/junit/tests/experimental/theories/AssumingInTheoriesTest.java b/src/test/java/org/junit/tests/experimental/theories/AssumingInTheoriesTest.java index 1972d828fa7a..fa6f28524323 100644 --- a/src/test/java/org/junit/tests/experimental/theories/AssumingInTheoriesTest.java +++ b/src/test/java/org/junit/tests/experimental/theories/AssumingInTheoriesTest.java @@ -1,16 +1,14 @@ package org.junit.tests.experimental.theories; +import static org.junit.tests.experimental.theories.TheoryTestUtils.runTheoryClass; import org.junit.Assert; import org.junit.Assume; import org.junit.Test; import org.junit.experimental.theories.DataPoint; import org.junit.experimental.theories.Theories; import org.junit.experimental.theories.Theory; -import org.junit.runner.JUnitCore; -import org.junit.runner.Request; import org.junit.runner.Result; import org.junit.runner.RunWith; -import org.junit.runner.Runner; import org.junit.runners.model.InitializationError; @RunWith(Theories.class) @@ -23,10 +21,7 @@ public void noTheoryAnnotationMeansAssumeShouldIgnore() { @Test public void theoryMeansOnlyAssumeShouldFail() throws InitializationError { - JUnitCore junitRunner = new JUnitCore(); - Runner theoryRunner = new Theories(TheoryWithNoUnassumedParameters.class); - Request request = Request.runner(theoryRunner); - Result result = junitRunner.run(request); + Result result = runTheoryClass(TheoryWithNoUnassumedParameters.class); Assert.assertEquals(1, result.getFailureCount()); } diff --git a/src/test/java/org/junit/tests/experimental/theories/TheoryTestUtils.java b/src/test/java/org/junit/tests/experimental/theories/TheoryTestUtils.java index 5bd105a76656..6e1b35165cc2 100644 --- a/src/test/java/org/junit/tests/experimental/theories/TheoryTestUtils.java +++ b/src/test/java/org/junit/tests/experimental/theories/TheoryTestUtils.java @@ -4,7 +4,13 @@ import java.util.List; import org.junit.experimental.theories.PotentialAssignment; +import org.junit.experimental.theories.Theories; import org.junit.experimental.theories.internal.Assignments; +import org.junit.runner.JUnitCore; +import org.junit.runner.Request; +import org.junit.runner.Result; +import org.junit.runner.Runner; +import org.junit.runners.model.InitializationError; import org.junit.runners.model.TestClass; public final class TheoryTestUtils { @@ -12,10 +18,16 @@ public final class TheoryTestUtils { private TheoryTestUtils() { } public static List potentialAssignments(Method method) - throws Exception { + throws Throwable { return Assignments.allUnassigned(method, new TestClass(method.getDeclaringClass())) .potentialsForNextUnassigned(); } + + public static Result runTheoryClass(Class testClass) throws InitializationError { + Runner theoryRunner = new Theories(testClass); + Request request = Request.runner(theoryRunner); + return new JUnitCore().run(request); + } } diff --git a/src/test/java/org/junit/tests/experimental/theories/extendingwithstubs/StubbedTheories.java b/src/test/java/org/junit/tests/experimental/theories/extendingwithstubs/StubbedTheories.java index 5dafd744d735..1b68da858c5a 100644 --- a/src/test/java/org/junit/tests/experimental/theories/extendingwithstubs/StubbedTheories.java +++ b/src/test/java/org/junit/tests/experimental/theories/extendingwithstubs/StubbedTheories.java @@ -50,7 +50,7 @@ protected void runWithIncompleteAssignment(Assignments incomplete) } private GuesserQueue createGuesserQueue(Assignments incomplete) - throws Exception { + throws Throwable { ParameterSignature nextUnassigned = incomplete.nextUnassigned(); if (nextUnassigned.hasAnnotation(Stub.class)) { diff --git a/src/test/java/org/junit/tests/experimental/theories/internal/AllMembersSupplierTest.java b/src/test/java/org/junit/tests/experimental/theories/internal/AllMembersSupplierTest.java index 0901cf84ae7f..4eeb7584c569 100644 --- a/src/test/java/org/junit/tests/experimental/theories/internal/AllMembersSupplierTest.java +++ b/src/test/java/org/junit/tests/experimental/theories/internal/AllMembersSupplierTest.java @@ -5,14 +5,19 @@ import java.util.List; +import org.junit.Rule; import org.junit.Test; import org.junit.experimental.theories.DataPoints; import org.junit.experimental.theories.ParameterSignature; import org.junit.experimental.theories.PotentialAssignment; import org.junit.experimental.theories.internal.AllMembersSupplier; +import org.junit.rules.ExpectedException; import org.junit.runners.model.TestClass; public class AllMembersSupplierTest { + @Rule + public ExpectedException expected = ExpectedException.none(); + public static class HasDataPoints { @DataPoints public static Object[] objects = {1, 2}; @@ -22,13 +27,9 @@ public HasDataPoints(Object obj) { } @Test - public void dataPointsAnnotationMeansTreatAsArrayOnly() - throws SecurityException, NoSuchMethodException { - List valueSources = new AllMembersSupplier( - new TestClass(HasDataPoints.class)) - .getValueSources(ParameterSignature.signatures( - HasDataPoints.class.getConstructor(Object.class)) - .get(0)); + public void dataPointsAnnotationMeansTreatAsArrayOnly() throws Throwable { + List valueSources = allMemberValuesFor( + HasDataPoints.class, Object.class); assertThat(valueSources.size(), is(2)); } @@ -41,13 +42,9 @@ public HasDataPointsFieldWithNullValue(Object obj) { } @Test - public void dataPointsArrayFieldMayContainNullValue() - throws SecurityException, NoSuchMethodException { - List valueSources = new AllMembersSupplier( - new TestClass(HasDataPointsFieldWithNullValue.class)) - .getValueSources(ParameterSignature.signatures( - HasDataPointsFieldWithNullValue.class.getConstructor(Object.class)) - .get(0)); + public void dataPointsArrayFieldMayContainNullValue() throws Throwable { + List valueSources = allMemberValuesFor( + HasDataPointsFieldWithNullValue.class, Object.class); assertThat(valueSources.size(), is(2)); } @@ -62,13 +59,34 @@ public HasDataPointsMethodWithNullValue(Integer i) { } @Test - public void dataPointsArrayMethodMayContainNullValue() - throws SecurityException, NoSuchMethodException { - List valueSources = new AllMembersSupplier( - new TestClass(HasDataPointsMethodWithNullValue.class)) + public void dataPointsArrayMethodMayContainNullValue() throws Throwable { + List valueSources = allMemberValuesFor( + HasDataPointsMethodWithNullValue.class, Integer.class); + assertThat(valueSources.size(), is(2)); + } + + public static class HasFailingDataPointsArrayMethod { + @DataPoints + public static Object[] objects() { + throw new RuntimeException("failing method"); + } + + public HasFailingDataPointsArrayMethod(Object obj) { + } + } + + @Test + public void allMembersFailsOnFailingDataPointsArrayMethod() throws Throwable { + expected.expect(RuntimeException.class); + expected.expectMessage("failing method"); + allMemberValuesFor(HasFailingDataPointsArrayMethod.class, Object.class); + } + + private List allMemberValuesFor(Class testClass, + Class... constructorParameterTypes) throws Throwable { + return new AllMembersSupplier(new TestClass(testClass)) .getValueSources(ParameterSignature.signatures( - HasDataPointsMethodWithNullValue.class.getConstructor(Integer.class)) + testClass.getConstructor(constructorParameterTypes)) .get(0)); - assertThat(valueSources.size(), is(2)); } } diff --git a/src/test/java/org/junit/tests/experimental/theories/internal/SpecificDataPointsSupplierTest.java b/src/test/java/org/junit/tests/experimental/theories/internal/SpecificDataPointsSupplierTest.java index 3e668c7b58dd..3ccf902c9bcf 100644 --- a/src/test/java/org/junit/tests/experimental/theories/internal/SpecificDataPointsSupplierTest.java +++ b/src/test/java/org/junit/tests/experimental/theories/internal/SpecificDataPointsSupplierTest.java @@ -56,7 +56,7 @@ public static String[] getOtherValues() { } @Test - public void shouldReturnOnlyTheNamedDataPoints() throws Exception { + public void shouldReturnOnlyTheNamedDataPoints() throws Throwable { SpecificDataPointsSupplier supplier = new SpecificDataPointsSupplier(new TestClass(TestClassWithNamedDataPoints.class)); List assignments = supplier.getValueSources(signature("methodWantingAllNamedStrings")); @@ -67,7 +67,7 @@ public void shouldReturnOnlyTheNamedDataPoints() throws Exception { } @Test - public void shouldReturnOnlyTheNamedFieldDataPoints() throws Exception { + public void shouldReturnOnlyTheNamedFieldDataPoints() throws Throwable { SpecificDataPointsSupplier supplier = new SpecificDataPointsSupplier(new TestClass(TestClassWithNamedDataPoints.class)); List assignments = supplier.getValueSources(signature("methodWantingNamedFieldString")); @@ -78,7 +78,7 @@ public void shouldReturnOnlyTheNamedFieldDataPoints() throws Exception { } @Test - public void shouldReturnOnlyTheNamedMethodDataPoints() throws Exception { + public void shouldReturnOnlyTheNamedMethodDataPoints() throws Throwable { SpecificDataPointsSupplier supplier = new SpecificDataPointsSupplier(new TestClass(TestClassWithNamedDataPoints.class)); List assignments = supplier.getValueSources(signature("methodWantingNamedMethodString")); @@ -89,7 +89,7 @@ public void shouldReturnOnlyTheNamedMethodDataPoints() throws Exception { } @Test - public void shouldReturnOnlyTheNamedSingleFieldDataPoints() throws Exception { + public void shouldReturnOnlyTheNamedSingleFieldDataPoints() throws Throwable { SpecificDataPointsSupplier supplier = new SpecificDataPointsSupplier(new TestClass(TestClassWithNamedDataPoints.class)); List assignments = supplier.getValueSources(signature("methodWantingNamedSingleFieldString")); @@ -100,7 +100,7 @@ public void shouldReturnOnlyTheNamedSingleFieldDataPoints() throws Exception { } @Test - public void shouldReturnOnlyTheNamedSingleMethodDataPoints() throws Exception { + public void shouldReturnOnlyTheNamedSingleMethodDataPoints() throws Throwable { SpecificDataPointsSupplier supplier = new SpecificDataPointsSupplier(new TestClass(TestClassWithNamedDataPoints.class)); List assignments = supplier.getValueSources(signature("methodWantingNamedSingleMethodString")); @@ -111,7 +111,7 @@ public void shouldReturnOnlyTheNamedSingleMethodDataPoints() throws Exception { } @Test - public void shouldReturnNothingIfTheNamedDataPointsAreMissing() throws Exception { + public void shouldReturnNothingIfTheNamedDataPointsAreMissing() throws Throwable { SpecificDataPointsSupplier supplier = new SpecificDataPointsSupplier(new TestClass(TestClassWithNamedDataPoints.class)); List assignments = supplier.getValueSources(signature("methodWantingWrongNamedString")); diff --git a/src/test/java/org/junit/tests/experimental/theories/runner/WithDataPointMethod.java b/src/test/java/org/junit/tests/experimental/theories/runner/WithDataPointMethod.java index 581cac89037c..f79eff94703f 100644 --- a/src/test/java/org/junit/tests/experimental/theories/runner/WithDataPointMethod.java +++ b/src/test/java/org/junit/tests/experimental/theories/runner/WithDataPointMethod.java @@ -17,6 +17,7 @@ import org.hamcrest.Matcher; import org.junit.Test; import org.junit.experimental.theories.DataPoint; +import org.junit.experimental.theories.DataPoints; import org.junit.experimental.theories.Theories; import org.junit.experimental.theories.Theory; import org.junit.runner.JUnitCore; @@ -33,36 +34,52 @@ public static int oneHundred() { @Theory public void allIntsOk(int x) { - } } + @Test + public void pickUpDataPointMethods() { + assertThat(testResult(HasDataPointMethod.class), isSuccessful()); + } + @RunWith(Theories.class) - public static class HasUglyDataPointMethod { + public static class HasFailingSingleDataPointMethod { @DataPoint - public static int oneHundred() { - return 100; - } + public static int num = 10; @DataPoint - public static int oneUglyHundred() { + public static int failingDataPoint() { throw new RuntimeException(); } @Theory public void allIntsOk(int x) { - } } @Test - public void pickUpDataPointMethods() { - assertThat(testResult(HasDataPointMethod.class), isSuccessful()); + public void shouldFailFromExceptionsInSingleDataPointMethods() { + assertThat(testResult(HasFailingSingleDataPointMethod.class), not(isSuccessful())); + } + + @RunWith(Theories.class) + public static class HasFailingDataPointArrayMethod { + @DataPoints + public static int[] num = { 1, 2, 3 }; + + @DataPoints + public static int[] failingDataPoints() { + throw new RuntimeException(); + } + + @Theory + public void allIntsOk(int x) { + } } @Test - public void ignoreExceptionsFromDataPointMethods() { - assertThat(testResult(HasUglyDataPointMethod.class), isSuccessful()); + public void shouldFailFromExceptionsInDataPointArrayMethods() { + assertThat(testResult(HasFailingDataPointArrayMethod.class), not(isSuccessful())); } @RunWith(Theories.class) @@ -93,32 +110,29 @@ public void mutableObjectsAreCreatedAfresh() { @RunWith(Theories.class) public static class HasDateMethod { @DataPoint - public int oneHundred() { + public static int oneHundred() { return 100; } - public Date notADataPoint() { + public static Date notADataPoint() { return new Date(); } @Theory public void allIntsOk(int x) { - } @Theory public void onlyStringsOk(String s) { - } @Theory public void onlyDatesOk(Date d) { - } } @Test - public void ignoreDataPointMethodsWithWrongTypes() throws Exception { + public void ignoreDataPointMethodsWithWrongTypes() throws Throwable { assertThat(potentialAssignments( HasDateMethod.class.getMethod("onlyStringsOk", String.class)) .toString(), not(containsString("100"))); diff --git a/src/test/java/org/junit/tests/experimental/theories/runner/WithExtendedParameterSources.java b/src/test/java/org/junit/tests/experimental/theories/runner/WithExtendedParameterSources.java index e44d91a632f5..bcda189588bf 100644 --- a/src/test/java/org/junit/tests/experimental/theories/runner/WithExtendedParameterSources.java +++ b/src/test/java/org/junit/tests/experimental/theories/runner/WithExtendedParameterSources.java @@ -1,6 +1,7 @@ package org.junit.tests.experimental.theories.runner; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.Assert.assertThat; import static org.junit.experimental.results.PrintableResult.testResult; @@ -33,12 +34,13 @@ public void testedOnLimitsParameters() throws Exception { } @RunWith(Theories.class) - public static class ShouldFilterNull { - @DataPoint - public static String NULL = null; + public static class ShouldFilterOutNullSingleDataPoints { @DataPoint public static String A = "a"; + + @DataPoint + public static String NULL = null; @Theory(nullsAccepted = false) public void allStringsAreNonNull(String s) { @@ -47,10 +49,41 @@ public void allStringsAreNonNull(String s) { } @Test - public void shouldFilterNull() { - assertThat(testResult(ShouldFilterNull.class), isSuccessful()); + public void shouldFilterOutNullSingleDataPoints() { + assertThat(testResult(ShouldFilterOutNullSingleDataPoints.class), isSuccessful()); + } + + @RunWith(Theories.class) + public static class ShouldFilterOutNullElementsFromDataPointArrays { + @DataPoints + public static String[] SOME_NULLS = { "non-null", null }; + + @Theory(nullsAccepted = false) + public void allStringsAreNonNull(String s) { + assertThat(s, notNullValue()); + } } + @Test + public void shouldFilterOutNullElementsFromDataPointArrays() { + assertThat(testResult(ShouldFilterOutNullElementsFromDataPointArrays.class), isSuccessful()); + } + + @RunWith(Theories.class) + public static class ShouldRejectTheoriesWithOnlyDisallowedNullData { + @DataPoints + public static String value = null; + + @Theory(nullsAccepted = false) + public void allStringsAreNonNull(String s) { + } + } + + @Test + public void ShouldRejectTheoriesWithOnlyDisallowedNullData() { + assertThat(testResult(ShouldRejectTheoriesWithOnlyDisallowedNullData.class), not(isSuccessful())); + } + @RunWith(Theories.class) public static class DataPointArrays { public static String log = ""; diff --git a/src/test/java/org/junit/tests/experimental/theories/runner/WithNamedDataPoints.java b/src/test/java/org/junit/tests/experimental/theories/runner/WithNamedDataPoints.java index 4b88eb7b7072..249ef1041c4b 100644 --- a/src/test/java/org/junit/tests/experimental/theories/runner/WithNamedDataPoints.java +++ b/src/test/java/org/junit/tests/experimental/theories/runner/WithNamedDataPoints.java @@ -60,7 +60,7 @@ public void theory(@FromDataPoints("named") String param) { } @Test - public void onlyUseSpecificDataPointsIfSpecified() throws Exception { + public void onlyUseSpecificDataPointsIfSpecified() throws Throwable { List assignments = potentialAssignments(HasSpecificDatapointsParameters.class .getMethod("theory", String.class)); diff --git a/src/test/java/org/junit/tests/experimental/theories/runner/WithParameterSupplier.java b/src/test/java/org/junit/tests/experimental/theories/runner/WithParameterSupplier.java index 53760278dc0f..1802390a87d4 100644 --- a/src/test/java/org/junit/tests/experimental/theories/runner/WithParameterSupplier.java +++ b/src/test/java/org/junit/tests/experimental/theories/runner/WithParameterSupplier.java @@ -73,7 +73,7 @@ public void theoryMethod(@ParametersSuppliedBy(SimpleSupplier.class) String para } @Test - public void shouldPickUpDataPointsFromParameterSupplier() throws Exception { + public void shouldPickUpDataPointsFromParameterSupplier() throws Throwable { List assignments = potentialAssignments(TestClassUsingParameterSupplier.class .getMethod("theoryMethod", String.class)); From 50e3d1bc87b4b16f0617b6212252363f499397ab Mon Sep 17 00:00:00 2001 From: pimterry Date: Sun, 17 Mar 2013 00:30:46 +0000 Subject: [PATCH 2/4] Added parameter to annotation to mark exceptions in datapoint methods as ignorable. --- .../experimental/theories/DataPoint.java | 5 +- .../experimental/theories/DataPoints.java | 1 + .../theories/internal/AllMembersSupplier.java | 26 +++- .../internal/SpecificDataPointsSupplier.java | 2 +- .../runner/FailingDataPointMethods.java | 136 ++++++++++++++++++ .../theories/runner/WithDataPointMethod.java | 41 ------ 6 files changed, 165 insertions(+), 46 deletions(-) create mode 100644 src/test/java/org/junit/tests/experimental/theories/runner/FailingDataPointMethods.java diff --git a/src/main/java/org/junit/experimental/theories/DataPoint.java b/src/main/java/org/junit/experimental/theories/DataPoint.java index 95dd6050c10d..8e6043b601c8 100644 --- a/src/main/java/org/junit/experimental/theories/DataPoint.java +++ b/src/main/java/org/junit/experimental/theories/DataPoint.java @@ -10,5 +10,6 @@ @Retention(RetentionPolicy.RUNTIME) @Target({FIELD, METHOD}) public @interface DataPoint { - String[] value() default {}; -} + String[] value() default {}; + Class[] ignoredExceptions() default {}; +} \ No newline at end of file diff --git a/src/main/java/org/junit/experimental/theories/DataPoints.java b/src/main/java/org/junit/experimental/theories/DataPoints.java index 68a3c9e650d9..61d46030f662 100644 --- a/src/main/java/org/junit/experimental/theories/DataPoints.java +++ b/src/main/java/org/junit/experimental/theories/DataPoints.java @@ -51,4 +51,5 @@ @Target({FIELD, METHOD}) public @interface DataPoints { String[] value() default {}; + Class[] ignoredExceptions() default {}; } diff --git a/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java b/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java index ba12fa2f5813..23a2b549f96b 100644 --- a/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java +++ b/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java @@ -1,11 +1,15 @@ package org.junit.experimental.theories.internal; +import static org.hamcrest.CoreMatchers.not; +import static org.hamcrest.core.IsInstanceOf.instanceOf; + import java.lang.reflect.Array; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import org.junit.Assume; import org.junit.experimental.theories.DataPoint; import org.junit.experimental.theories.DataPoints; import org.junit.experimental.theories.ParameterSignature; @@ -37,8 +41,14 @@ public Object getValue() throws CouldNotGenerateValueException { throw new RuntimeException( "unexpected: getMethods returned an inaccessible method"); } catch (Throwable e) { + DataPoint annotation = fMethod.getAnnotation(DataPoint.class); + if (annotation != null) { + for (Class ignorable : annotation.ignoredExceptions()) { + Assume.assumeThat(e, not(instanceOf(ignorable))); + } + } + throw new CouldNotGenerateValueException(); - // do nothing, just look for more values } } @@ -71,7 +81,19 @@ public List getValueSources(ParameterSignature sig) throws private void addMultiPointMethods(ParameterSignature sig, List list) throws Throwable { for (FrameworkMethod dataPointsMethod : getDataPointsMethods(sig)) { - addMultiPointArrayValues(sig, dataPointsMethod.getName(), list, dataPointsMethod.invokeExplosively(null)); + try { + addMultiPointArrayValues(sig, dataPointsMethod.getName(), list, dataPointsMethod.invokeExplosively(null)); + } catch (Throwable t) { + DataPoints annotation = dataPointsMethod.getAnnotation(DataPoints.class); + if (annotation != null) { + for (Class ignored : annotation.ignoredExceptions()) { + if (ignored.isAssignableFrom(t.getClass())) { + return; + } + } + } + throw t; + } } } diff --git a/src/main/java/org/junit/experimental/theories/internal/SpecificDataPointsSupplier.java b/src/main/java/org/junit/experimental/theories/internal/SpecificDataPointsSupplier.java index cd18292efa57..7b571e335941 100644 --- a/src/main/java/org/junit/experimental/theories/internal/SpecificDataPointsSupplier.java +++ b/src/main/java/org/junit/experimental/theories/internal/SpecificDataPointsSupplier.java @@ -33,7 +33,7 @@ protected Collection getSingleDataPointFields(ParameterSignature sig) { } } - return fieldsWithMatchingNames; + return fieldsWithMatchingNames; } @Override diff --git a/src/test/java/org/junit/tests/experimental/theories/runner/FailingDataPointMethods.java b/src/test/java/org/junit/tests/experimental/theories/runner/FailingDataPointMethods.java new file mode 100644 index 000000000000..5abc0ea480fb --- /dev/null +++ b/src/test/java/org/junit/tests/experimental/theories/runner/FailingDataPointMethods.java @@ -0,0 +1,136 @@ +package org.junit.tests.experimental.theories.runner; + +import static org.hamcrest.CoreMatchers.not; +import static org.junit.Assert.assertThat; +import static org.junit.experimental.results.PrintableResult.testResult; +import static org.junit.experimental.results.ResultMatchers.isSuccessful; +import org.junit.Test; +import org.junit.experimental.theories.DataPoint; +import org.junit.experimental.theories.DataPoints; +import org.junit.experimental.theories.Theories; +import org.junit.experimental.theories.Theory; +import org.junit.runner.RunWith; + +public class FailingDataPointMethods { + + @RunWith(Theories.class) + public static class HasFailingSingleDataPointMethod { + @DataPoint + public static int num = 10; + + @DataPoint + public static int failingDataPoint() { + throw new RuntimeException(); + } + + @Theory + public void theory(int x) { + } + } + + @Test + public void shouldFailFromExceptionsInSingleDataPointMethods() { + assertThat(testResult(HasWronglyIgnoredFailingSingleDataPointMethod.class), not(isSuccessful())); + } + + @RunWith(Theories.class) + public static class HasFailingDataPointArrayMethod { + @DataPoints + public static int[] num = { 1, 2, 3 }; + + @DataPoints + public static int[] failingDataPoints() { + throw new RuntimeException(); + } + + @Theory + public void theory(int x) { + } + } + + @Test + public void shouldFailFromExceptionsInDataPointArrayMethods() { + assertThat(testResult(HasFailingDataPointArrayMethod.class), not(isSuccessful())); + } + + @RunWith(Theories.class) + public static class HasIgnoredFailingSingleDataPointMethod { + @DataPoint + public static int num = 10; + + @DataPoint(ignoredExceptions=Throwable.class) + public static int failingDataPoint() { + throw new RuntimeException(); + } + + @Theory + public void theory(int x) { + } + } + + @Test + public void shouldIgnoreSingleDataPointMethodExceptionsOnRequest() { + assertThat(testResult(HasIgnoredFailingSingleDataPointMethod.class), isSuccessful()); + } + + @RunWith(Theories.class) + public static class HasIgnoredFailingMultipleDataPointMethod { + @DataPoint + public static int num = 10; + + @DataPoints(ignoredExceptions=Throwable.class) + public static int[] failingDataPoint() { + throw new RuntimeException(); + } + + @Theory + public void theory(int x) { + } + } + + @Test + public void shouldIgnoreMultipleDataPointMethodExceptionsOnRequest() { + assertThat(testResult(HasIgnoredFailingMultipleDataPointMethod.class), isSuccessful()); + } + + @RunWith(Theories.class) + public static class HasWronglyIgnoredFailingSingleDataPointMethod { + @DataPoint + public static int num = 10; + + @DataPoint(ignoredExceptions=NullPointerException.class) + public static int failingDataPoint() { + throw new RuntimeException(); + } + + @Theory + public void theory(int x) { + } + } + + @Test + public void shouldNotIgnoreNonMatchingSingleDataPointExceptions() { + assertThat(testResult(HasWronglyIgnoredFailingSingleDataPointMethod.class), not(isSuccessful())); + } + + @RunWith(Theories.class) + public static class HasWronglyIgnoredFailingMultipleDataPointMethod { + @DataPoint + public static int num = 10; + + @DataPoint(ignoredExceptions=NullPointerException.class) + public static int failingDataPoint() { + throw new RuntimeException(); + } + + @Theory + public void theory(int x) { + } + } + + @Test + public void shouldNotIgnoreNonMatchingMultipleDataPointExceptions() { + assertThat(testResult(HasWronglyIgnoredFailingMultipleDataPointMethod.class), not(isSuccessful())); + } + +} diff --git a/src/test/java/org/junit/tests/experimental/theories/runner/WithDataPointMethod.java b/src/test/java/org/junit/tests/experimental/theories/runner/WithDataPointMethod.java index f79eff94703f..c814508de897 100644 --- a/src/test/java/org/junit/tests/experimental/theories/runner/WithDataPointMethod.java +++ b/src/test/java/org/junit/tests/experimental/theories/runner/WithDataPointMethod.java @@ -17,7 +17,6 @@ import org.hamcrest.Matcher; import org.junit.Test; import org.junit.experimental.theories.DataPoint; -import org.junit.experimental.theories.DataPoints; import org.junit.experimental.theories.Theories; import org.junit.experimental.theories.Theory; import org.junit.runner.JUnitCore; @@ -42,46 +41,6 @@ public void pickUpDataPointMethods() { assertThat(testResult(HasDataPointMethod.class), isSuccessful()); } - @RunWith(Theories.class) - public static class HasFailingSingleDataPointMethod { - @DataPoint - public static int num = 10; - - @DataPoint - public static int failingDataPoint() { - throw new RuntimeException(); - } - - @Theory - public void allIntsOk(int x) { - } - } - - @Test - public void shouldFailFromExceptionsInSingleDataPointMethods() { - assertThat(testResult(HasFailingSingleDataPointMethod.class), not(isSuccessful())); - } - - @RunWith(Theories.class) - public static class HasFailingDataPointArrayMethod { - @DataPoints - public static int[] num = { 1, 2, 3 }; - - @DataPoints - public static int[] failingDataPoints() { - throw new RuntimeException(); - } - - @Theory - public void allIntsOk(int x) { - } - } - - @Test - public void shouldFailFromExceptionsInDataPointArrayMethods() { - assertThat(testResult(HasFailingDataPointArrayMethod.class), not(isSuccessful())); - } - @RunWith(Theories.class) public static class DataPointMethodReturnsMutableObject { @DataPoint From 38d91308c302eb4f0b5ed24e064ee56395f9fc01 Mon Sep 17 00:00:00 2001 From: pimterry Date: Sun, 17 Mar 2013 12:26:09 +0000 Subject: [PATCH 3/4] Failing single-datapoint methods keep their original stack trace --- .../junit/experimental/theories/PotentialAssignment.java | 7 +++++++ .../experimental/theories/internal/AllMembersSupplier.java | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/junit/experimental/theories/PotentialAssignment.java b/src/main/java/org/junit/experimental/theories/PotentialAssignment.java index d4e401062ec6..f25fda07e60b 100644 --- a/src/main/java/org/junit/experimental/theories/PotentialAssignment.java +++ b/src/main/java/org/junit/experimental/theories/PotentialAssignment.java @@ -5,6 +5,13 @@ public abstract class PotentialAssignment { public static class CouldNotGenerateValueException extends Exception { private static final long serialVersionUID = 1L; + + public CouldNotGenerateValueException() { + } + + public CouldNotGenerateValueException(Throwable t) { + super(t); + } } public static PotentialAssignment forValue(final String name, final Object value) { diff --git a/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java b/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java index 23a2b549f96b..cf5ef75d0624 100644 --- a/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java +++ b/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java @@ -40,15 +40,15 @@ public Object getValue() throws CouldNotGenerateValueException { } catch (IllegalAccessException e) { throw new RuntimeException( "unexpected: getMethods returned an inaccessible method"); - } catch (Throwable e) { + } catch (Throwable t) { DataPoint annotation = fMethod.getAnnotation(DataPoint.class); if (annotation != null) { for (Class ignorable : annotation.ignoredExceptions()) { - Assume.assumeThat(e, not(instanceOf(ignorable))); + Assume.assumeThat(t, not(instanceOf(ignorable))); } } - throw new CouldNotGenerateValueException(); + throw new CouldNotGenerateValueException(t); } } From dbe771125873b707dfbc95f066649b4be8f247b2 Mon Sep 17 00:00:00 2001 From: pimterry Date: Fri, 22 Mar 2013 15:14:16 +0000 Subject: [PATCH 4/4] Refactored out ignorable exception type-matching to minimise duplicating of that logic --- .../theories/internal/AllMembersSupplier.java | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java b/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java index 1ad53f8fa01c..a8d7c9a6bd9d 100644 --- a/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java +++ b/src/main/java/org/junit/experimental/theories/internal/AllMembersSupplier.java @@ -1,8 +1,5 @@ package org.junit.experimental.theories.internal; -import static org.hamcrest.CoreMatchers.not; -import static org.hamcrest.core.IsInstanceOf.instanceOf; - import java.lang.reflect.Array; import java.lang.reflect.Field; import java.util.ArrayList; @@ -40,15 +37,11 @@ public Object getValue() throws CouldNotGenerateValueException { } catch (IllegalAccessException e) { throw new RuntimeException( "unexpected: getMethods returned an inaccessible method"); - } catch (Throwable t) { - DataPoint annotation = fMethod.getAnnotation(DataPoint.class); - if (annotation != null) { - for (Class ignorable : annotation.ignoredExceptions()) { - Assume.assumeThat(t, not(instanceOf(ignorable))); - } - } + } catch (Throwable throwable) { + DataPoint annotation = fMethod.getAnnotation(DataPoint.class); + Assume.assumeTrue(annotation == null || !isAssignableToAnyOf(annotation.ignoredExceptions(), throwable)); - throw new CouldNotGenerateValueException(t); + throw new CouldNotGenerateValueException(throwable); } } @@ -56,7 +49,7 @@ public Object getValue() throws CouldNotGenerateValueException { public String getDescription() throws CouldNotGenerateValueException { return fMethod.getName(); } - } + } private final TestClass fClass; @@ -86,16 +79,13 @@ private void addMultiPointMethods(ParameterSignature sig, List ignored : annotation.ignoredExceptions()) { - if (ignored.isAssignableFrom(t.getClass())) { - return; - } - } + if (annotation != null && isAssignableToAnyOf(annotation.ignoredExceptions(), throwable)) { + return; + } else { + throw throwable; } - throw t; } } } @@ -145,6 +135,15 @@ private Object getStaticFieldValue(final Field field) { "unexpected: getFields returned an inaccessible field"); } } + + private static boolean isAssignableToAnyOf(Class[] typeArray, Object target) { + for (Class type : typeArray) { + if (type.isAssignableFrom(target.getClass())) { + return true; + } + } + return false; + } protected Collection getDataPointsMethods(ParameterSignature sig) { return fClass.getAnnotatedMethods(DataPoints.class);