Properly call IRunListener.specSkipped and .featureSkipped (#1811)

fixes #1662

Author: leonard84

spock-core/src/main/java/org/spockframework/runtime/FeatureNode.java

@@ -39,6 +39,11 @@ public SkipResult shouldBeSkipped(SpockExecutionContext context) throws Exceptio
     return shouldBeSkipped(getNodeInfo());
   }
 
+  @Override
+  public void nodeSkipped(SpockExecutionContext context, TestDescriptor testDescriptor, SkipResult result) {
+    context.getRunner().supervisor.featureSkipped(getNodeInfo());
+  }
+
   @Override
   public void around(SpockExecutionContext context, Invocation<SpockExecutionContext> invocation) {
     ErrorInfoCollector errorInfoCollector = new ErrorInfoCollector();

spock-core/src/main/java/org/spockframework/runtime/PlatformSpecRunner.java

@@ -184,8 +184,7 @@ public void runFeature(SpockExecutionContext context, Runnable feature) {
     if (currentFeature.isExcluded()) return;
 
     if (currentFeature.isSkipped()) {
-      supervisor.featureSkipped(currentFeature); // todo notify in node isSkipped
-      return;
+      throw new InternalSpockError("Invalid state, feature is executed although it should have been skipped");
     }
 
     supervisor.beforeFeature(currentFeature);

spock-core/src/main/java/org/spockframework/runtime/SpecNode.java

@@ -1,5 +1,6 @@
 package org.spockframework.runtime;
 
+import org.junit.platform.engine.TestDescriptor;
 import org.spockframework.runtime.model.SpecInfo;
 import spock.config.RunnerConfiguration;
 
@@ -21,14 +22,15 @@ public Type getType() {
 
   @Override
   public SpockExecutionContext prepare(SpockExecutionContext context) throws Exception {
+    PlatformParameterizedSpecRunner specRunner = context.getRunContext().createSpecRunner(getNodeInfo());
+    context = context.withRunner(specRunner).withSpec(getNodeInfo());
     if (getNodeInfo().isSkipped()) {
-      // Node.prepare is called before Node.shouldBeSkipped so we just skip the shared spec initialization
+      // Node.prepare is called before Node.shouldBeSkipped, so we just skip the shared spec initialization.
+      // We still set the runner, as we need it in the nodeSkipped callback.
       return context;
     }
-    PlatformParameterizedSpecRunner specRunner = context.getRunContext().createSpecRunner(getNodeInfo());
     ErrorInfoCollector errorInfoCollector = new ErrorInfoCollector();
     context = context.withErrorInfoCollector(errorInfoCollector);
-    context = context.withRunner(specRunner).withSpec(getNodeInfo());
     context = specRunner.runSharedSpec(context);
     errorInfoCollector.assertEmpty();
     return context;
@@ -39,6 +41,11 @@ public SkipResult shouldBeSkipped(SpockExecutionContext context) throws Exceptio
     return shouldBeSkipped(getNodeInfo());
   }
 
+  @Override
+  public void nodeSkipped(SpockExecutionContext context, TestDescriptor testDescriptor, SkipResult result) {
+    context.getRunner().supervisor.specSkipped(getNodeInfo());
+  }
+
   @Override
   public SpockExecutionContext before(SpockExecutionContext context) throws Exception {
     ErrorInfoCollector errorInfoCollector = new ErrorInfoCollector();

spock-specs/src/test/groovy/org/spockframework/runtime/RunListenerSpec.groovy

@@ -3,10 +3,18 @@ package org.spockframework.runtime
 import org.spockframework.EmbeddedSpecification
 import org.spockframework.runtime.extension.*
 import org.spockframework.runtime.model.SpecInfo
+import org.spockframework.runtime.model.parallel.ExecutionMode
+import spock.lang.Execution
+import spock.lang.Ignore
+import spock.lang.Issue
 import spock.lang.Specification
 
 import java.lang.annotation.*
 
+@Execution(
+  value = ExecutionMode.SAME_THREAD,
+  reason = "Uses a shared field on a delegate"
+)
 class RunListenerSpec extends EmbeddedSpecification {
 
   IRunListener runListener = Mock()
@@ -45,6 +53,73 @@ class ASpec extends Specification {
     cleanup:
     RunListenerDelegate.delegate = null
   }
+
+  def "IRunListener is called for skipped features"() {
+    given:
+    RunListenerDelegate.delegate = runListener
+    runner.addPackageImport(Specification.package)
+    runner.addClassImport(RegisterRunListener)
+    runner.addClassImport(Ignore)
+
+    when:
+    runner.runWithImports '''
+@RegisterRunListener
+class ASpec extends Specification {
+  @Ignore
+  def "a test"() {
+      expect: true
+  }
+
+  @Ignore
+  def "data driven test"() {
+      expect: true
+      where:
+      i << [1, 2]
+  }
+}
+'''
+
+
+    then:
+    1 * runListener.beforeSpec(_)
+    then:
+    2 * runListener.featureSkipped(_)
+    then:
+    1 * runListener.afterSpec(_)
+    then:
+    0 * runListener._
+
+    cleanup:
+    RunListenerDelegate.delegate = null
+  }
+
+  @Issue("https://github.com/spockframework/spock/issues/1662")
+  def "IRunListener is called for skipped specs"() {
+    given:
+    RunListenerDelegate.delegate = runListener
+    runner.addPackageImport(Specification.package)
+    runner.addClassImport(RegisterRunListener)
+    runner.addClassImport(Ignore)
+
+    when:
+    runner.runWithImports '''
+@RegisterRunListener
+@Ignore
+class ASpec extends Specification {
+  def "a test"() {
+      expect: true
+  }
+}
+'''
+
+    then:
+    1 * runListener.specSkipped(_)
+    then:
+    0 * runListener._
+
+    cleanup:
+    RunListenerDelegate.delegate = null
+  }
 }
 
 @Target(ElementType.TYPE)