Merge pull request #758 from tomdcc/clean-stack-traces

Clean up groovy stack traces

Author: aslakhellesoy

groovy/src/main/java/cucumber/runtime/groovy/GroovyStepDefinition.java

@@ -8,6 +8,7 @@
 import gherkin.formatter.Argument;
 import gherkin.formatter.model.Step;
 import groovy.lang.Closure;
+import org.codehaus.groovy.runtime.StackTraceUtils;
 
 import java.lang.reflect.Type;
 import java.util.ArrayList;
@@ -63,13 +64,17 @@ private List<ParameterInfo> getParameterInfos() {
     }
 
     public void execute(I18n i18n, final Object[] args) throws Throwable {
-        Timeout.timeout(new Timeout.Callback<Object>() {
-            @Override
-            public Object call() throws Throwable {
-                backend.invoke(body, args);
-                return null;
-            }
-        }, timeoutMillis);
+        try {
+            Timeout.timeout(new Timeout.Callback<Object>() {
+                @Override
+                public Object call() throws Throwable {
+                    backend.invoke(body, args);
+                    return null;
+                }
+            }, timeoutMillis);
+        } catch(Throwable e) {
+            throw StackTraceUtils.deepSanitize(e);
+        }
     }
 
     public boolean isDefinedAt(StackTraceElement stackTraceElement) {

groovy/src/test/groovy/cucumber/runtime/groovy/ExceptionThrowingThing.groovy

@@ -0,0 +1,16 @@
+package cucumber.runtime.groovy
+
+class ExceptionThrowingThing {
+    def methodMissing(String name, args) {
+        throw new RuntimeException("Don't have method $name taking $args")
+    }
+
+    RuntimeException returnGroovyException() {
+        try {
+            this.foo()
+            null
+        } catch(RuntimeException e) {
+            e
+        }
+    }
+}

groovy/src/test/java/cucumber/runtime/groovy/GroovyStackTraceTest.java

@@ -0,0 +1,47 @@
+package cucumber.runtime.groovy;
+
+import groovy.lang.Closure;
+import org.codehaus.groovy.runtime.MethodClosure;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import static junit.framework.Assert.assertFalse;
+import static org.junit.Assert.fail;
+
+@RunWith(MockitoJUnitRunner.class)
+public class GroovyStackTraceTest {
+    GroovyStepDefinition groovyStepDefinition;
+
+    @Before
+    public void setUp() throws Throwable {
+        Closure body = new MethodClosure("the owner", "length");
+        groovyStepDefinition = new GroovyStepDefinition(null, 0, body, null, new ExceptionThrowingBackend());
+    }
+
+    @Test
+    public void should_sanitize_stacktrace() throws Throwable {
+        try {
+            groovyStepDefinition.execute(null, new Object[0]);
+            fail("step definition didn't throw an exception");
+        } catch(Throwable thrown) {
+            for (StackTraceElement stackTraceElement : thrown.getStackTrace()) {
+                // if there are none of these, pretty good chance it's cleaned up the stack trace
+                assertFalse("Stack trace has internal groovy callsite elements", stackTraceElement.getClassName().startsWith("org.codehaus.groovy.runtime.callsite"));
+            }
+        }
+
+    }
+
+    private static class ExceptionThrowingBackend extends GroovyBackend {
+        public ExceptionThrowingBackend() {
+            super(null);
+        }
+
+        @Override
+        public void invoke(Closure body, Object[] args) throws Throwable {
+            throw new ExceptionThrowingThing().returnGroovyException();
+        }
+    }
+}