From 9f01a02da86129662ccafaea4c1a8c1d563c0c62 Mon Sep 17 00:00:00 2001
From: Tim Yates <tim.yates@gmail.com>
Date: Fri, 16 Feb 2024 13:56:34 +0000
Subject: [PATCH] fix: add interceptor around parameterized features (#953)

When a feature is using `@Unroll` (which is implicit in recent versions of Spock), we needed to
add the interceptor to the iteration interceptor handler instead of the standard handler.

This bug was visible in #911 where specs that were parameterized did not properly revert the
transaction between iterations.

Closes #911
---
 .../spock/MicronautSpockExtension.java        | 47 +++++++++++--------
 .../spock/intercept/InterceptTestSpec.groovy  | 28 ++---------
 2 files changed, 31 insertions(+), 44 deletions(-)

diff --git a/test-spock/src/main/java/io/micronaut/test/extensions/spock/MicronautSpockExtension.java b/test-spock/src/main/java/io/micronaut/test/extensions/spock/MicronautSpockExtension.java
index c86f1f532..0243dbd73 100644
--- a/test-spock/src/main/java/io/micronaut/test/extensions/spock/MicronautSpockExtension.java
+++ b/test-spock/src/main/java/io/micronaut/test/extensions/spock/MicronautSpockExtension.java
@@ -65,26 +65,11 @@ public void visitSpecAnnotation(T annotation, SpecInfo spec) {
 
         spec.getAllFeatures().forEach(feature -> {
 
-            feature.addInterceptor(invocation -> {
-                TestContext testContext = buildContext(invocation, null);
-                try {
-                    beforeTestMethod(testContext);
-                    interceptTest(new TestMethodInvocationContext<Object>() {
-                        @Override
-                        public TestContext getTestContext() {
-                            return testContext;
-                        }
-
-                        @Override
-                        public Object proceed() throws Throwable {
-                            invocation.proceed();
-                            return null;
-                        }
-                    });
-                } finally {
-                    afterTestMethod(testContext);
-                }
-            });
+            if (feature.isParameterized()) {
+                feature.addIterationInterceptor(this::getInvocationInterceptor);
+            } else {
+                feature.addInterceptor(this::getInvocationInterceptor);
+            }
 
             feature.getFeatureMethod().addInterceptor(invocation -> {
                 try {
@@ -198,6 +183,28 @@ public Object proceed() throws Throwable {
         });
     }
 
+    @SuppressWarnings("java:S112") // Throwable is what we deal with here
+    private void getInvocationInterceptor(IMethodInvocation invocation) throws Throwable {
+        TestContext testContext = buildContext(invocation, null);
+        try {
+            beforeTestMethod(testContext);
+            interceptTest(new TestMethodInvocationContext<Object>() {
+                @Override
+                public TestContext getTestContext() {
+                    return testContext;
+                }
+
+                @Override
+                public Object proceed() throws Throwable {
+                    invocation.proceed();
+                    return null;
+                }
+            });
+        } finally {
+            afterTestMethod(testContext);
+        }
+    }
+
     private MicronautTestValue buildValueObject(MicronautTest micronautTest) {
         if (micronautTest != null) {
             return new MicronautTestValue(
diff --git a/test-spock/src/test/groovy/io/micronaut/test/spock/intercept/InterceptTestSpec.groovy b/test-spock/src/test/groovy/io/micronaut/test/spock/intercept/InterceptTestSpec.groovy
index 39ae7d764..54b5028b1 100644
--- a/test-spock/src/test/groovy/io/micronaut/test/spock/intercept/InterceptTestSpec.groovy
+++ b/test-spock/src/test/groovy/io/micronaut/test/spock/intercept/InterceptTestSpec.groovy
@@ -1,18 +1,3 @@
-/*
- * Copyright 2017-2020 original authors
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
 package io.micronaut.test.spock.intercept
 
 import io.micronaut.context.annotation.Property
@@ -21,7 +6,6 @@ import io.micronaut.test.spock.MathService
 import jakarta.inject.Inject
 import spock.lang.Specification
 import spock.lang.Stepwise
-import spock.lang.Unroll
 
 @Stepwise
 @MicronautTest
@@ -34,7 +18,6 @@ class InterceptTestSpec extends Specification {
     @Inject
     TestInterceptor testInterceptor
 
-    @Unroll
     void "should compute #num to #expected"() {
         when:
             def result = mathService.compute(num)
@@ -62,6 +45,8 @@ class InterceptTestSpec extends Specification {
                     'OUT BEFORE $spock_feature_0_0',
                     'IN AFTER $spock_feature_0_0',
                     'OUT AFTER $spock_feature_0_0',
+                    'OUT $spock_feature_0_0', // This is the implicit unroll
+                    'IN $spock_feature_0_0',
                     'IN BEFORE $spock_feature_0_0',
                     'OUT BEFORE $spock_feature_0_0',
                     'IN AFTER $spock_feature_0_0',
@@ -80,13 +65,8 @@ class InterceptTestSpec extends Specification {
                     'OUT BEFORE $spock_feature_0_2',
                     //  This test is $spock_feature_0_2
             ]
+
         then:
-            assert calls.size() == expected.size()
-            for (int i = 0; i < expected.size(); i++) {
-                def a = calls.get(i)
-                def b = expected[i]
-                assert a == b
-            }
+            calls == expected
     }
-
 }