Expose IErrorContext in ErrorInfo...

Prior to this commit, IRunListener.error(ErrorInfo) didn't give any context where the error happened.
spockframework · Feb 16, 2023 · 9bc3a7a · 9bc3a7a
1 parent bbdc5d9
commit 9bc3a7a
Show file tree

Hide file tree

Showing 12 changed files with 289 additions and 32 deletions.
diff --git a/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java b/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java
@@ -17,6 +17,7 @@
 package org.spockframework.compiler;
 
 import org.codehaus.groovy.syntax.Token;
+import org.codehaus.groovy.syntax.Types;
 import org.spockframework.lang.Wildcard;
 import org.spockframework.util.*;
 import spock.lang.Specification;
@@ -376,4 +377,16 @@ public static Expression createGetAtWithMapSupportMethodCall(Expression expressi
       createMethodCall(expression, GET_AT_METHOD_NAME, new ConstantExpression(index))
     );
   }
+  public static BinaryExpression createVariableNotNullExpression(VariableExpression var) {
+    return new BinaryExpression(
+      new VariableExpression(var),
+      Token.newSymbol(Types.COMPARE_NOT_EQUAL, -1, -1),
+      new ConstantExpression(null));
+  }
+  public static BinaryExpression createVariableIsNullExpression(VariableExpression var) {
+    return new BinaryExpression(
+      new VariableExpression(var),
+      Token.newSymbol(Types.COMPARE_EQUAL, -1, -1),
+      new ConstantExpression(null));
+  }
 }
diff --git a/spock-core/src/main/java/org/spockframework/compiler/DeepBlockRewriter.java b/spock-core/src/main/java/org/spockframework/compiler/DeepBlockRewriter.java
@@ -16,6 +16,9 @@
 
 package org.spockframework.compiler;
 
+import groovy.lang.GroovyObject;
+import org.codehaus.groovy.ast.ClassHelper;
+import org.codehaus.groovy.syntax.Token;
 import org.spockframework.compiler.model.*;
 import org.spockframework.util.*;
 
@@ -29,6 +32,7 @@
 
 import static org.codehaus.groovy.ast.expr.MethodCallExpression.NO_ARGUMENTS;
 import static org.spockframework.compiler.AstUtil.createDirectMethodCall;
+import static org.spockframework.compiler.AstUtil.createMethodCall;
 
 /**
  * Walks the statement and expression tree to:
@@ -64,6 +68,7 @@ private void addBlockEnterCall(Block block) {
       || blockType == BlockParseInfo.METHOD_END
       || blockType == BlockParseInfo.ANONYMOUS) return;
 
+    // SpockRuntime.enterBlock(getSpecificationContext(), new BlockInfo(blockKind, [blockTexts]))
     MethodCallExpression enterBlockCall = createDirectMethodCall(
       new ClassExpression(resources.getAstNodeCache().SpockRuntime),
       resources.getAstNodeCache().SpockRuntime_CallEnterBlock,
@@ -82,7 +87,19 @@ private void addBlockEnterCall(Block block) {
             )
           ))
       ));
-    block.getAst().add(0, new ExpressionStatement(enterBlockCall));
+
+    // As the cleanup block finalizes the specification, it would override any previous block in ErrorInfo,
+    // so we only call enterBlock if there is no error yet.
+    if (blockType == BlockParseInfo.CLEANUP) {
+      block.getAst().add(0, new IfStatement(
+        // if ($spock_feature_throwable == null)
+        new BooleanExpression(AstUtil.createVariableIsNullExpression(new VariableExpression(SpecRewriter.SPOCK_FEATURE_THROWABLE, resources.getAstNodeCache().Throwable))),
+        new ExpressionStatement(enterBlockCall),
+        EmptyStatement.INSTANCE
+      ));
+    } else {
+      block.getAst().add(0, new ExpressionStatement(enterBlockCall));
+    }
   }
 
   @Override

diff --git a/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java b/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java
@@ -44,6 +44,9 @@ public class SpecRewriter extends AbstractSpecVisitor implements IRewriteResourc
   // https://issues.apache.org/jira/browse/GROOVY-10403
   // needed for groovy-4 compatibility and only available since groovy-4
   private static final java.lang.reflect.Method GET_PLAIN_NODE_REFERENCE = ReflectionUtil.getMethodBySignature(ClassNode.class, "getPlainNodeReference", boolean.class);
+  public static final String SPOCK_VALUE = "$spock_value";
+  public static final String SPOCK_FEATURE_THROWABLE = "$spock_feature_throwable";
+  public static final String SPOCK_TMP_THROWABLE = "$spock_tmp_throwable";
 
   private final AstNodeCache nodeCache;
   private final SourceLookup lookup;
@@ -159,7 +162,7 @@ private void createFinalFieldGetter(Field field) {
 
   private void createSharedFieldSetter(Field field) {
     String setterName = "set" + MetaClassHelper.capitalize(field.getName());
-    Parameter[] params = new Parameter[] { new Parameter(field.getAst().getType(), "$spock_value") };
+    Parameter[] params = new Parameter[] { new Parameter(field.getAst().getType(), SPOCK_VALUE) };
     MethodNode setter = spec.getAst().getMethod(setterName, params);
     if (setter != null) {
       errorReporter.error(field.getAst(),
@@ -180,7 +183,7 @@ private void createSharedFieldSetter(Field field) {
                     // use internal name
                     new ConstantExpression(field.getAst().getName())),
                 Token.newSymbol(Types.ASSIGN, -1, -1),
-                new VariableExpression("$spock_value"))));
+                new VariableExpression(SPOCK_VALUE))));
 
     setter.setSourcePosition(field.getAst());
     spec.getAst().addMethod(setter);
@@ -479,7 +482,7 @@ public void visitCleanupBlock(CleanupBlock block) {
     }
 
     VariableExpression featureThrowableVar =
-        new VariableExpression("$spock_feature_throwable", nodeCache.Throwable);
+        new VariableExpression(SPOCK_FEATURE_THROWABLE, nodeCache.Throwable);
     method.getStatements().add(createVariableDeclarationStatement(featureThrowableVar));
 
     List<Statement> featureStats = new ArrayList<>();
@@ -507,13 +510,6 @@ public void visitCleanupBlock(CleanupBlock block) {
     movedStatsBackToMethod = true;
   }
 
-  private BinaryExpression createVariableNotNullExpression(VariableExpression var) {
-    return new BinaryExpression(
-        new VariableExpression(var),
-        Token.newSymbol(Types.COMPARE_NOT_EQUAL, -1, -1),
-        new ConstantExpression(null));
-  }
-
   private Statement createVariableDeclarationStatement(VariableExpression var) {
     DeclarationExpression throwableDecl =
         new DeclarationExpression(
@@ -538,7 +534,7 @@ private TryCatchStatement createCleanupTryCatch(CleanupBlock block, VariableExpr
   }
 
   private CatchStatement createThrowableAssignmentAndRethrowCatchStatement(VariableExpression assignmentVar) {
-    Parameter catchParameter = new Parameter(nodeCache.Throwable, "$spock_tmp_throwable");
+    Parameter catchParameter = new Parameter(nodeCache.Throwable, SPOCK_TMP_THROWABLE);
 
     BinaryExpression assignThrowableExpr =
         new BinaryExpression(
@@ -555,9 +551,9 @@ private CatchStatement createThrowableAssignmentAndRethrowCatchStatement(Variabl
   }
 
   private CatchStatement createHandleSuppressedThrowableStatement(VariableExpression featureThrowableVar) {
-    Parameter catchParameter = new Parameter(nodeCache.Throwable, "$spock_tmp_throwable");
+    Parameter catchParameter = new Parameter(nodeCache.Throwable, SPOCK_TMP_THROWABLE);
 
-    BinaryExpression featureThrowableNotNullExpr = createVariableNotNullExpression(featureThrowableVar);
+    BinaryExpression featureThrowableNotNullExpr = AstUtil.createVariableNotNullExpression(featureThrowableVar);
 
     List<Statement> addSuppressedStats =
       singletonList(new ExpressionStatement(

diff --git a/spock-core/src/main/java/org/spockframework/lang/ISpecificationContext.java b/spock-core/src/main/java/org/spockframework/lang/ISpecificationContext.java
@@ -37,4 +37,6 @@ public interface ISpecificationContext {
   Throwable getThrownException();
 
   IMockController getMockController();
+
+  boolean isSharedContext();
 }
diff --git a/spock-core/src/main/java/org/spockframework/runtime/DataIteratorFactory.java b/spock-core/src/main/java/org/spockframework/runtime/DataIteratorFactory.java
@@ -36,10 +36,15 @@ protected Object invokeRaw(Object target, MethodInfo method, Object... arguments
       try {
         return method.invoke(target, arguments);
       } catch (Throwable throwable) {
-        supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(method, throwable));
+        supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(method, throwable, getErrorContext()));
         return null;
       }
     }
+
+
+    protected IErrorContext getErrorContext() {
+      return ErrorContext.from(context.getCurrentInstance().getSpecificationContext());
+    }
   }
 
   /**
@@ -127,7 +132,7 @@ public Object[] next() {
       try {
         return (Object[]) invokeRaw(context.getSharedInstance(), context.getCurrentFeature().getDataProcessorMethod(), next);
       } catch (Throwable t) {
-        supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProcessorMethod(), t));
+        supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProcessorMethod(), t, getErrorContext()));
         return null;
       }
     }
@@ -179,13 +184,17 @@ public boolean hasNext() {
             haveNext = hasNext;
           } else if (haveNext != hasNext) {
             DataProviderInfo provider = context.getCurrentFeature().getDataProviders().get(i);
-            supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(provider.getDataProviderMethod(),
-              createDifferentNumberOfDataValuesException(provider, hasNext)));
+            supervisor.error(context.getErrorInfoCollector(),
+              new ErrorInfo(
+                provider.getDataProviderMethod(),
+                createDifferentNumberOfDataValuesException(provider, hasNext),
+                getErrorContext())
+            );
             return false;
           }
 
         } catch (Throwable t) {
-          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProviders().get(i).getDataProviderMethod(), t));
+          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProviders().get(i).getDataProviderMethod(), t, getErrorContext()));
           return false;
         }
 
@@ -205,7 +214,7 @@ public Object[] next() {
         try {
           next[i] = iterators[i].next();
         } catch (Throwable t) {
-          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProviders().get(i).getDataProviderMethod(), t));
+          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProviders().get(i).getDataProviderMethod(), t, getErrorContext()));
           return null;
         }
       }
@@ -283,7 +292,7 @@ private Object[] createDataProviders() {
           break;
         } else if (provider == null) {
           SpockExecutionException error = new SpockExecutionException("Data provider is null!");
-          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(method, error));
+          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(method, error, getErrorContext()));
           break;
         }
 
@@ -331,12 +340,12 @@ private Iterator<?>[] createIterators() {
           Iterator<?> iter = GroovyRuntimeUtil.asIterator(dataProviders[i]);
           if (iter == null) {
             supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProviders().get(i).getDataProviderMethod(),
-              new SpockExecutionException("Data provider's iterator() method returned null")));
+              new SpockExecutionException("Data provider's iterator() method returned null"), getErrorContext()));
             return null;
           }
           iterators[i] = iter;
         } catch (Throwable t) {
-          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProviders().get(i).getDataProviderMethod(), t));
+          supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(context.getCurrentFeature().getDataProviders().get(i).getDataProviderMethod(), t, getErrorContext()));
           return null;
         }
 

diff --git a/spock-core/src/main/java/org/spockframework/runtime/ErrorContext.java b/spock-core/src/main/java/org/spockframework/runtime/ErrorContext.java
@@ -0,0 +1,50 @@
+package org.spockframework.runtime;
+
+import org.spockframework.lang.ISpecificationContext;
+import org.spockframework.runtime.model.*;
+
+class ErrorContext implements IErrorContext {
+  private final SpecInfo spec;
+  private final FeatureInfo feature;
+  private final IterationInfo iteration;
+  private final BlockInfo block;
+
+  private ErrorContext(SpecInfo spec, FeatureInfo feature, IterationInfo iteration, BlockInfo block) {
+    this.spec = spec;
+    this.feature = feature;
+    this.iteration = iteration;
+    this.block = block;
+  }
+
+  static ErrorContext from(ISpecificationContext context) {
+    if (context.isSharedContext()) {
+      return new ErrorContext(context.getCurrentSpec(), null, null, null);
+    }
+    return new ErrorContext(
+      context.getCurrentSpec(),
+      context.getCurrentFeature(),
+      context.getCurrentIteration(),
+      context.getCurrentBlock()
+    );
+  }
+
+  @Override
+  public SpecInfo getCurrentSpec() {
+    return spec;
+  }
+
+  @Override
+  public FeatureInfo getCurrentFeature() {
+    return feature;
+  }
+
+  @Override
+  public IterationInfo getCurrentIteration() {
+    return iteration;
+  }
+
+  @Override
+  public BlockInfo getCurrentBlock() {
+    return block;
+  }
+}
diff --git a/spock-core/src/main/java/org/spockframework/runtime/MasterRunSupervisor.java b/spock-core/src/main/java/org/spockframework/runtime/MasterRunSupervisor.java
@@ -45,7 +45,7 @@ public void error(ErrorInfoCollector errorInfoCollector, ErrorInfo error) {
 
     exception = transform(exception);
 
-    ErrorInfo transformedError = new ErrorInfo(error.getMethod(), exception);
+    ErrorInfo transformedError = new ErrorInfo(error.getMethod(), exception, error.getErrorContext());
     if (exception instanceof TestAbortedException || exception instanceof TestSkippedException) {
       // Spock has no concept of "aborted tests", so we don't notify Spock listeners
     } else {
@@ -58,7 +58,7 @@ public void error(ErrorInfoCollector errorInfoCollector, ErrorInfo error) {
   private void handleMultipleFailures(ErrorInfoCollector errorInfoCollector,  ErrorInfo error) {
     MultipleFailuresError multiFailure = (MultipleFailuresError) error.getException();
     for (Throwable failure : multiFailure.getFailures())
-      error(errorInfoCollector, new ErrorInfo(error.getMethod(), failure));
+      error(errorInfoCollector, new ErrorInfo(error.getMethod(), failure, error.getErrorContext()));
   }
 
 

diff --git a/spock-core/src/main/java/org/spockframework/runtime/PlatformSpecRunner.java b/spock-core/src/main/java/org/spockframework/runtime/PlatformSpecRunner.java
@@ -364,7 +364,7 @@ private void runIterationCleanups(SpockExecutionContext context) {
       try {
         cleanup.run();
       } catch (Throwable t) {
-        ErrorInfo error = new ErrorInfo(CollectionUtil.getFirstElement(context.getSpec().getCleanupMethods()), t);
+        ErrorInfo error = new ErrorInfo(CollectionUtil.getFirstElement(context.getSpec().getCleanupMethods()), t, ErrorContext.from(getSpecificationContext(context)));
         supervisor.error(context.getErrorInfoCollector(), error);
       }
     }
@@ -397,7 +397,7 @@ protected void invoke(SpockExecutionContext context, Object target, MethodInfo m
     try {
       invocation.proceed();
     } catch (Throwable throwable) {
-      ErrorInfo error = new ErrorInfo(method, throwable);
+      ErrorInfo error = new ErrorInfo(method, throwable, ErrorContext.from(getSpecificationContext(context)));
       supervisor.error(context.getErrorInfoCollector(), error);
     }
   }
@@ -406,7 +406,7 @@ protected Object invokeRaw(SpockExecutionContext context, Object target, MethodI
     try {
       return method.invoke(target, arguments);
     } catch (Throwable throwable) {
-      supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(method, throwable));
+      supervisor.error(context.getErrorInfoCollector(), new ErrorInfo(method, throwable, ErrorContext.from(getSpecificationContext(context))));
       return null;
     }
   }

diff --git a/spock-core/src/main/java/org/spockframework/runtime/SpecificationContext.java b/spock-core/src/main/java/org/spockframework/runtime/SpecificationContext.java
@@ -38,15 +38,15 @@ public void setCurrentSpec(SpecInfo currentSpec) {
 
   @Override
   public FeatureInfo getCurrentFeature() {
-    if (currentIteration == null) {
+    if (isSharedContext()) {
       throw new IllegalStateException("Cannot request current feature in @Shared context");
     }
     return getCurrentIteration().getFeature();
   }
 
   @Override
   public IterationInfo getCurrentIteration() {
-    if (currentIteration == null) {
+    if (isSharedContext()) {
       throw new IllegalStateException("Cannot request current iteration in @Shared context");
     }
     return currentIteration;
@@ -80,4 +80,9 @@ public void setThrownException(Throwable exception) {
   public IMockController getMockController() {
     return mockController;
   }
+
+  @Override
+  public boolean isSharedContext() {
+    return currentIteration == null;
+  }
 }
diff --git a/spock-core/src/main/java/org/spockframework/runtime/model/ErrorInfo.java b/spock-core/src/main/java/org/spockframework/runtime/model/ErrorInfo.java
@@ -14,13 +14,23 @@
 
 package org.spockframework.runtime.model;
 
+import org.spockframework.util.Nullable;
+
 public class ErrorInfo {
   private final MethodInfo method;
   private final Throwable error;
+  private final IErrorContext errorContext;
 
   public ErrorInfo(MethodInfo method, Throwable error) {
     this.method = method;
     this.error = error;
+    this.errorContext = null;
+  }
+
+  public ErrorInfo(MethodInfo method, Throwable error, IErrorContext errorContext) {
+    this.method = method;
+    this.error = error;
+    this.errorContext = errorContext;
   }
 
   public MethodInfo getMethod() {
@@ -30,4 +40,9 @@ public MethodInfo getMethod() {
   public Throwable getException() {
     return error;
   }
+
+  @Nullable
+  public IErrorContext getErrorContext() {
+    return errorContext;
+  }
 }