From 2e62da529b6305ce3abba36fc1075158c5ed3807 Mon Sep 17 00:00:00 2001 From: l-1squared <30831153+l-1squared@users.noreply.github.com> Date: Thu, 20 Jun 2024 15:35:10 +0200 Subject: [PATCH] feat(Issue-1625): fix catching pending scenarios as aborted --- .../jgiven/junit/JGivenMethodRule.java | 144 +++++++++--------- 1 file changed, 76 insertions(+), 68 deletions(-) diff --git a/jgiven-junit/src/main/java/com/tngtech/jgiven/junit/JGivenMethodRule.java b/jgiven-junit/src/main/java/com/tngtech/jgiven/junit/JGivenMethodRule.java index b70e44f1312..e4b6eac3c5a 100644 --- a/jgiven-junit/src/main/java/com/tngtech/jgiven/junit/JGivenMethodRule.java +++ b/jgiven-junit/src/main/java/com/tngtech/jgiven/junit/JGivenMethodRule.java @@ -36,7 +36,7 @@ public class JGivenMethodRule implements MethodRule { private static final String DATAPROVIDER_FRAMEWORK_METHOD = "com.tngtech.java.junit.dataprovider.DataProviderFrameworkMethod"; private static final String JUNITPARAMS_STATEMENT = "junitparams.internal.InvokeParameterisedMethod"; - private static final Logger log = LoggerFactory.getLogger( JGivenMethodRule.class ); + private static final Logger log = LoggerFactory.getLogger(JGivenMethodRule.class); protected final ScenarioBase scenario; @@ -50,7 +50,7 @@ public JGivenMethodRule() { /** * @since 0.8.1 */ - public JGivenMethodRule(ScenarioBase scenario ) { + public JGivenMethodRule(ScenarioBase scenario) { this.scenario = scenario; } @@ -62,18 +62,26 @@ public ScenarioBase getScenario() { } @Override - public Statement apply( final Statement base, final FrameworkMethod method, final Object target ) { + public Statement apply(final Statement base, final FrameworkMethod method, final Object target) { return new Statement() { @Override public void evaluate() throws Throwable { - starting( base, method, target ); + //TODO: make this nice! + starting(base, method, target); try { base.evaluate(); - succeeded(); - } catch( Throwable t ) { - if(ThrowableUtil.isAssumptionException(t)) { + } catch (Throwable t) { + if (ThrowableUtil.isAssumptionException(t)) { aborted(t); - }else { + } else { + failed(t); + } + throw t; + } + try { + succeeded(); + } catch (Throwable t) { + if (!ThrowableUtil.isAssumptionException(t)) { failed(t); } throw t; @@ -86,95 +94,95 @@ protected void succeeded() throws Throwable { scenario.finished(); // ignore test when scenario is not implemented - assumeTrue( EnumSet.of( SUCCESS, FAILED, ABORTED).contains( scenario.getScenarioModel().getExecutionStatus() ) ); + assumeTrue(EnumSet.of(SUCCESS, FAILED, ABORTED).contains(scenario.getScenarioModel().getExecutionStatus())); } - protected void failed( Throwable e ) throws Throwable { - if( scenario.getExecutor().hasFailed() ) { + protected void failed(Throwable e) throws Throwable { + if (scenario.getExecutor().hasFailed()) { Throwable failedException = scenario.getExecutor().getFailedException(); - List errors = Lists.newArrayList( failedException, e ); - scenario.getExecutor().setFailedException( new MultipleFailureException( errors ) ); + List errors = Lists.newArrayList(failedException, e); + scenario.getExecutor().setFailedException(new MultipleFailureException(errors)); } else { - scenario.getExecutor().failed( e ); + scenario.getExecutor().failed(e); } scenario.finished(); } - protected void aborted( Throwable e) throws Throwable { - if( scenario.getExecutor().hasAborted() ) { + protected void aborted(Throwable e) throws Throwable { + if (scenario.getExecutor().hasAborted()) { Throwable failedException = scenario.getExecutor().getAbortedException(); - List errors = Lists.newArrayList( failedException, e ); - scenario.getExecutor().setAbortedException( new MultipleFailureException( errors ) ); + List errors = Lists.newArrayList(failedException, e); + scenario.getExecutor().setAbortedException(new MultipleFailureException(errors)); } else { - scenario.getExecutor().aborted( e ); + scenario.getExecutor().aborted(e); } scenario.finished(); } - protected void starting( Statement base, FrameworkMethod testMethod, Object target ) { - ReportModel reportModel = ScenarioModelHolder.getInstance().getReportModel( target.getClass() ); - scenario.setModel( reportModel ); - scenario.getExecutor().injectStages( target ); + protected void starting(Statement base, FrameworkMethod testMethod, Object target) { + ReportModel reportModel = ScenarioModelHolder.getInstance().getReportModel(target.getClass()); + scenario.setModel(reportModel); + scenario.getExecutor().injectStages(target); - scenario.startScenario( target.getClass(), testMethod.getMethod(), getNamedArguments( base, testMethod, target ) ); + scenario.startScenario(target.getClass(), testMethod.getMethod(), getNamedArguments(base, testMethod, target)); // inject state from the test itself - scenario.getExecutor().readScenarioState( target ); + scenario.getExecutor().readScenarioState(target); } @VisibleForTesting - static List getNamedArguments( Statement base, FrameworkMethod method, Object target ) { + static List getNamedArguments(Statement base, FrameworkMethod method, Object target) { AccessibleObject constructorOrMethod = method.getMethod(); List arguments = Collections.emptyList(); - if( DATAPROVIDER_FRAMEWORK_METHOD.equals( method.getClass().getCanonicalName() ) ) { - arguments = getArgumentsFrom( method, "parameters" ); + if (DATAPROVIDER_FRAMEWORK_METHOD.equals(method.getClass().getCanonicalName())) { + arguments = getArgumentsFrom(method, "parameters"); } - if( JUNITPARAMS_STATEMENT.equals( base.getClass().getCanonicalName() ) ) { - arguments = getArgumentsFrom( base, "params" ); + if (JUNITPARAMS_STATEMENT.equals(base.getClass().getCanonicalName())) { + arguments = getArgumentsFrom(base, "params"); } - if( isParameterizedTest( target ) ) { - Constructor constructor = getOnlyConstructor( target.getClass() ); + if (isParameterizedTest(target)) { + Constructor constructor = getOnlyConstructor(target.getClass()); constructorOrMethod = constructor; - arguments = getArgumentsFrom( constructor, target ); + arguments = getArgumentsFrom(constructor, target); } - return ParameterNameUtil.mapArgumentsWithParameterNames( constructorOrMethod, arguments ); + return ParameterNameUtil.mapArgumentsWithParameterNames(constructorOrMethod, arguments); } - private static List getArgumentsFrom( Object object, String fieldName ) { + private static List getArgumentsFrom(Object object, String fieldName) { Class methodClass = object.getClass(); try { - Field field = methodClass.getDeclaredField( fieldName ); - field.setAccessible( true ); - return Arrays.asList( (Object[]) field.get( object ) ); - - } catch( NoSuchFieldException e ) { - log.warn( format( "Could not find field containing test method arguments in '%s'. " - + "Probably the internal representation has changed. Consider writing a bug report.", - methodClass.getSimpleName() ), e ); - } catch( IllegalAccessException e ) { - log.warn( format( "Not able to access field containing test method arguments in '%s'. " - + "Probably the internal representation has changed. Consider writing a bug report.", - methodClass.getSimpleName() ), e ); + Field field = methodClass.getDeclaredField(fieldName); + field.setAccessible(true); + return Arrays.asList((Object[]) field.get(object)); + + } catch (NoSuchFieldException e) { + log.warn(format("Could not find field containing test method arguments in '%s'. " + + "Probably the internal representation has changed. Consider writing a bug report.", + methodClass.getSimpleName()), e); + } catch (IllegalAccessException e) { + log.warn(format("Not able to access field containing test method arguments in '%s'. " + + "Probably the internal representation has changed. Consider writing a bug report.", + methodClass.getSimpleName()), e); } return Collections.emptyList(); } - private static boolean isParameterizedTest( Object target ) { - RunWith runWith = target.getClass().getAnnotation( RunWith.class ); - return runWith != null && Parameterized.class.equals( runWith.value() ); + private static boolean isParameterizedTest(Object target) { + RunWith runWith = target.getClass().getAnnotation(RunWith.class); + return runWith != null && Parameterized.class.equals(runWith.value()); } - private static Constructor getOnlyConstructor( Class testClass ) { + private static Constructor getOnlyConstructor(Class testClass) { Constructor[] constructors = testClass.getConstructors(); - if( constructors.length != 1 ) { - log.warn( "Test class can only have one public constructor, " - + "see org.junit.runners.Parameterized.TestClassRunnerForParameters.validateConstructor(List)" ); + if (constructors.length != 1) { + log.warn("Test class can only have one public constructor, " + + "see org.junit.runners.Parameterized.TestClassRunnerForParameters.validateConstructor(List)"); } return constructors[0]; } @@ -187,35 +195,35 @@ private static Constructor getOnlyConstructor( Class testClass ) { * type, the order of {@link ReflectionUtil#getAllNonStaticFieldValuesFrom(Class, Object, String)} is used. * * @param constructor {@link Constructor} from which argument types should be retrieved - * @param target {@link Parameterized} test instance from which arguments tried to be retrieved + * @param target {@link Parameterized} test instance from which arguments tried to be retrieved * @return the determined arguments, never {@code null} */ - private static List getArgumentsFrom( Constructor constructor, Object target ) { + private static List getArgumentsFrom(Constructor constructor, Object target) { Class testClass = target.getClass(); Class[] constructorParamClasses = constructor.getParameterTypes(); - List fieldValues = ReflectionUtil.getAllNonStaticFieldValuesFrom( testClass, target, - " Consider writing a bug report." ); + List fieldValues = ReflectionUtil.getAllNonStaticFieldValuesFrom(testClass, target, + " Consider writing a bug report."); - return getTypeMatchingValuesInOrderOf( constructorParamClasses, fieldValues ); + return getTypeMatchingValuesInOrderOf(constructorParamClasses, fieldValues); } - private static List getTypeMatchingValuesInOrderOf( Class[] expectedClasses, List values ) { - List valuesCopy = Lists.newArrayList( values ); + private static List getTypeMatchingValuesInOrderOf(Class[] expectedClasses, List values) { + List valuesCopy = Lists.newArrayList(values); List result = new ArrayList<>(); - for( Class argumentClass : expectedClasses ) { - for( Iterator iterator = valuesCopy.iterator(); iterator.hasNext(); ) { + for (Class argumentClass : expectedClasses) { + for (Iterator iterator = valuesCopy.iterator(); iterator.hasNext(); ) { Object value = iterator.next(); - if( Primitives.wrap( argumentClass ).isInstance( value ) ) { - result.add( value ); + if (Primitives.wrap(argumentClass).isInstance(value)) { + result.add(value); iterator.remove(); break; } } } - if( result.size() < expectedClasses.length ) { - log.warn( format( "Couldn't find matching values in '%s' for expected classes '%s',", valuesCopy, - Arrays.toString( expectedClasses ) ) ); + if (result.size() < expectedClasses.length) { + log.warn(format("Couldn't find matching values in '%s' for expected classes '%s',", valuesCopy, + Arrays.toString(expectedClasses))); } return result; }