From 79f1cf906337db63b80d33e2841410b320a46cd0 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Mon, 2 Dec 2024 02:07:42 +0100 Subject: [PATCH] Include root cause when using DataTable.asList and friends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `CucumberInvocationTargetException` was originally designed to be used as a wrapper around `InvocationTargetException` to track the step definition that caused the exception. The cause of `InvocationTargetException` would then be rewritten to appear to have originated from the step definition. This would effectively hide the whole backend from view. Unfortunately using DataTable.asList inside a step, this also invokes the backend. And because `CucumberInvocationTargetException` did not have cause, the cause of any exceptions would also be hidden. By exposing the cause of the `InvocationTargetException` as the cause of the `CucumberInvocationTargetException` and removing any frames before the step definition was invoked we can clean this up somewhat. ```log io.cucumber.datatable.CucumberDataTableException: 'java.util.List' could not transform | NAME | QUANTITY | UNITS | | Flour | 1 | KG | | Water | 0.65 | L | | Salt | 0.08 | KG | at io.cucumber.datatable.DataTableType.transform(DataTableType.java:158) at io.cucumber.datatable.DataTableTypeRegistryTableConverter.toListOrProblems(DataTableTypeRegistryTableConverter.java:158) at io.cucumber.datatable.DataTableTypeRegistryTableConverter.toList(DataTableTypeRegistryTableConverter.java:139) at io.cucumber.datatable.DataTable.asList(DataTable.java:199) at io.cucumber.skeleton.StepDefinitions.i_wait_hour(StepDefinitions.java:40) at ✽.I mix the following ingredients(classpath:io/cucumber/skeleton/belly.feature:6) Caused by: io.cucumber.core.backend.CucumberInvocationTargetException at io.cucumber.java.Invoker.doInvoke(Invoker.java:73) at io.cucumber.java.Invoker.invoke(Invoker.java:24) at io.cucumber.java.AbstractGlueDefinition.invokeMethod(AbstractGlueDefinition.java:47) at io.cucumber.java.JavaDataTableTypeDefinition.lambda$createDataTableType$2(JavaDataTableTypeDefinition.java:47) at io.cucumber.datatable.DataTableType$TableEntryTransformerAdaptor.transform(DataTableType.java:342) at io.cucumber.datatable.DataTableType$TableEntryTransformerAdaptor.transform(DataTableType.java:319) at io.cucumber.datatable.DataTableType.transform(DataTableType.java:155) ... 5 more Caused by: java.lang.IllegalArgumentException: This message is never shown during test execution at io.cucumber.skeleton.StepDefinitions.mySystemEntry(StepDefinitions.java:29) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at io.cucumber.java.Invoker.doInvoke(Invoker.java:66) ... 11 more ``` Unfortunately, it is not possible to remove the `CucumberInvocationTargetException` entirely. Fixes: #2948 --- CHANGELOG.md | 2 + .../CucumberInvocationTargetException.java | 10 ++- .../core/runner/StackManipulation.java | 90 +++++++++++-------- .../core/runner/StepDefinitionMatchTest.java | 2 +- .../cucumber/java/JavaStepDefinitionTest.java | 2 +- ...efinitionMarksCorrectStackElementTest.java | 2 +- 6 files changed, 65 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a453ff5562..04e773e70b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed +- [Core] Include root cause when using DataTable.asList and friends ([#2949](https://github.com/cucumber/cucumber-jvm/pull/2949) M.P. Korstanje) ### Changed - [JUnit Platform Engine] Use JUnit Platform 1.11.3 (JUnit Jupiter 5.11.3) diff --git a/cucumber-core/src/main/java/io/cucumber/core/backend/CucumberInvocationTargetException.java b/cucumber-core/src/main/java/io/cucumber/core/backend/CucumberInvocationTargetException.java index 5e58e35df7..7cfd04de76 100644 --- a/cucumber-core/src/main/java/io/cucumber/core/backend/CucumberInvocationTargetException.java +++ b/cucumber-core/src/main/java/io/cucumber/core/backend/CucumberInvocationTargetException.java @@ -20,12 +20,20 @@ public CucumberInvocationTargetException(Located located, InvocationTargetExcept this.invocationTargetException = invocationTargetException; } + /** + * @deprecated use {@link #getCause()} instead. + */ + @Deprecated public Throwable getInvocationTargetExceptionCause() { - return invocationTargetException.getCause(); + return getCause(); } public Located getLocated() { return located; } + @Override + public Throwable getCause() { + return invocationTargetException.getCause(); + } } diff --git a/cucumber-core/src/main/java/io/cucumber/core/runner/StackManipulation.java b/cucumber-core/src/main/java/io/cucumber/core/runner/StackManipulation.java index d169e6f440..851d16e7c8 100644 --- a/cucumber-core/src/main/java/io/cucumber/core/runner/StackManipulation.java +++ b/cucumber-core/src/main/java/io/cucumber/core/runner/StackManipulation.java @@ -3,60 +3,72 @@ import io.cucumber.core.backend.CucumberInvocationTargetException; import io.cucumber.core.backend.Located; +import java.util.function.Consumer; + final class StackManipulation { private StackManipulation() { } - static Throwable removeFrameworkFrames(CucumberInvocationTargetException invocationException) { - Throwable error = invocationException.getInvocationTargetExceptionCause(); - StackTraceElement[] stackTraceElements = error.getStackTrace(); - Located located = invocationException.getLocated(); - - int newStackTraceLength = findIndexOf(located, stackTraceElements); - if (newStackTraceLength == -1) { - return error; - } + static Throwable removeFrameworkFramesAndAppendStepLocation( + CucumberInvocationTargetException invocationException, StackTraceElement stepLocation + ) { + Throwable error = invocationException.getCause(); + walkException(error, appendStepLocation(invocationException.getLocated(), stepLocation)); + return error; + } - StackTraceElement[] newStackTrace = new StackTraceElement[newStackTraceLength]; - System.arraycopy(stackTraceElements, 0, newStackTrace, 0, newStackTraceLength); - error.setStackTrace(newStackTrace); + static Throwable removeFrameworkFrames(CucumberInvocationTargetException invocationException) { + Throwable error = invocationException.getCause(); + walkException(invocationException, removeFramesAfter(invocationException.getLocated())); return error; } - private static int findIndexOf(Located located, StackTraceElement[] stackTraceElements) { - if (stackTraceElements.length == 0) { - return -1; + private static void walkException(Throwable cause, Consumer action) { + while (cause != null) { + action.accept(cause); + cause = cause.getCause(); } + } - int newStackTraceLength; - for (newStackTraceLength = 1; newStackTraceLength < stackTraceElements.length; ++newStackTraceLength) { - if (located.isDefinedAt(stackTraceElements[newStackTraceLength - 1])) { - break; + static Consumer removeFramesAfter(Located located) { + return throwable -> { + StackTraceElement[] stackTrace = throwable.getStackTrace(); + int lastFrame = findIndexOf(located, stackTrace); + if (lastFrame == -1) { + return; } - } - return newStackTraceLength; + StackTraceElement[] newStackTrace = new StackTraceElement[lastFrame + 1]; + System.arraycopy(stackTrace, 0, newStackTrace, 0, lastFrame + 1); + throwable.setStackTrace(newStackTrace); + }; } - static Throwable removeFrameworkFramesAndAppendStepLocation( - CucumberInvocationTargetException invocationException, StackTraceElement stepLocation - ) { - Located located = invocationException.getLocated(); - Throwable error = invocationException.getInvocationTargetExceptionCause(); - if (stepLocation == null) { - return error; - } - StackTraceElement[] stackTraceElements = error.getStackTrace(); - int newStackTraceLength = findIndexOf(located, stackTraceElements); - if (newStackTraceLength == -1) { - return error; - } - StackTraceElement[] newStackTrace = new StackTraceElement[newStackTraceLength + 1]; - System.arraycopy(stackTraceElements, 0, newStackTrace, 0, newStackTraceLength); - newStackTrace[newStackTraceLength] = stepLocation; - error.setStackTrace(newStackTrace); - return error; + private static Consumer appendStepLocation(Located located, StackTraceElement stepLocation) { + return throwable -> { + if (located == null) { + return; + } + StackTraceElement[] stackTrace = throwable.getStackTrace(); + int lastFrame = findIndexOf(located, stackTrace); + if (lastFrame == -1) { + return; + } + // One extra for the step location + StackTraceElement[] newStackTrace = new StackTraceElement[lastFrame + 1 + 1]; + System.arraycopy(stackTrace, 0, newStackTrace, 0, lastFrame + 1); + newStackTrace[lastFrame + 1] = stepLocation; + throwable.setStackTrace(newStackTrace); + }; } + private static int findIndexOf(Located located, StackTraceElement[] stackTraceElements) { + for (int index = 0; index < stackTraceElements.length; index++) { + if (located.isDefinedAt(stackTraceElements[index])) { + return index; + } + } + return -1; + } } diff --git a/cucumber-core/src/test/java/io/cucumber/core/runner/StepDefinitionMatchTest.java b/cucumber-core/src/test/java/io/cucumber/core/runner/StepDefinitionMatchTest.java index 5fe6701fb7..2dfdd8e8ba 100644 --- a/cucumber-core/src/test/java/io/cucumber/core/runner/StepDefinitionMatchTest.java +++ b/cucumber-core/src/test/java/io/cucumber/core/runner/StepDefinitionMatchTest.java @@ -46,7 +46,7 @@ class StepDefinitionMatchTest { private final Located stubbedLocation = new Located() { @Override public boolean isDefinedAt(StackTraceElement stackTraceElement) { - return false; + return true; } @Override diff --git a/cucumber-java/src/test/java/io/cucumber/java/JavaStepDefinitionTest.java b/cucumber-java/src/test/java/io/cucumber/java/JavaStepDefinitionTest.java index 01f756934a..48923d9334 100644 --- a/cucumber-java/src/test/java/io/cucumber/java/JavaStepDefinitionTest.java +++ b/cucumber-java/src/test/java/io/cucumber/java/JavaStepDefinitionTest.java @@ -44,7 +44,7 @@ void can_provide_location_of_step() throws Throwable { JavaStepDefinition definition = new JavaStepDefinition(method, "three (.*) mice", lookup); CucumberInvocationTargetException exception = assertThrows(CucumberInvocationTargetException.class, () -> definition.execute(new Object[0])); - Optional match = stream(exception.getInvocationTargetExceptionCause().getStackTrace()) + Optional match = stream(exception.getCause().getStackTrace()) .filter(definition::isDefinedAt).findFirst(); StackTraceElement stackTraceElement = match.get(); diff --git a/cucumber-java8/src/test/java/io/cucumber/java8/Java8LambdaStepDefinitionMarksCorrectStackElementTest.java b/cucumber-java8/src/test/java/io/cucumber/java8/Java8LambdaStepDefinitionMarksCorrectStackElementTest.java index 7b24b5c91f..1b7815c509 100644 --- a/cucumber-java8/src/test/java/io/cucumber/java8/Java8LambdaStepDefinitionMarksCorrectStackElementTest.java +++ b/cucumber-java8/src/test/java/io/cucumber/java8/Java8LambdaStepDefinitionMarksCorrectStackElementTest.java @@ -29,7 +29,7 @@ void exception_from_step_should_be_defined_at_step_definition_class() { CucumberInvocationTargetException exception = assertThrows(CucumberInvocationTargetException.class, () -> stepDefinition.execute(new Object[0])); - assertThat(exception.getInvocationTargetExceptionCause(), + assertThat(exception.getCause(), new CustomTypeSafeMatcher("exception with matching stack trace") { @Override protected boolean matchesSafely(Throwable item) {