Skip to content

Commit f77b122

Browse files
committed
Retry extension does not mesh with TestAbortedException and PendingFeature
The Retry annotation now skips when a TestAbortedException is thrown. And it also interoperates now with the PendingFeature correctly. Fixes #1863
1 parent 12a35eb commit f77b122

File tree

6 files changed

+263
-3
lines changed

6 files changed

+263
-3
lines changed

docs/release_notes.adoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ will now require that the spec is annotated with <<parallel_execution.adoc#isola
6262
* Fix incompatibility with JUnit 6 in OSGi environment spockIssue:2231[]
6363
* OSGi: Pin the `Require-Capability:osgi.ee=JavaSE` to Java `8` spockPull:2233[]
6464
* Fix: Prevent NPE in SpecRunHistory.sortFeatures when duration is missing spockIssue:2234[]
65+
* Retry extension does not mesh with TestAbortedException and PendingFeature spockIssue:1863[]
6566

6667
== 2.4-M6 (2025-04-15)
6768

spock-core/src/main/java/org/spockframework/runtime/extension/builtin/RetryBaseInterceptor.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package org.spockframework.runtime.extension.builtin;
1818

19+
import org.opentest4j.TestAbortedException;
1920
import org.spockframework.runtime.GroovyRuntimeUtil;
2021
import org.spockframework.runtime.extension.*;
2122
import spock.lang.Retry;
@@ -58,6 +59,12 @@ protected boolean isExpected(IMethodInvocation invocation, Throwable failure) {
5859
}
5960

6061
private boolean hasExpectedClass(Throwable failure) {
62+
for (Class<? extends Throwable> exception : retry.skipRetryExceptions()) {
63+
if (exception.isInstance(failure)) {
64+
return false;
65+
}
66+
}
67+
6168
for (Class<? extends Throwable> exception : retry.exceptions()) {
6269
if (exception.isInstance(failure)) {
6370
return true;

spock-core/src/main/java/org/spockframework/runtime/extension/builtin/RetryIterationInterceptor.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,23 @@ public class RetryIterationInterceptor extends RetryBaseInterceptor implements I
3939

4040
public RetryIterationInterceptor(Retry retry, MethodInfo featureMethod) {
4141
super(retry);
42-
featureMethod.addInterceptor(new InnerRetryInterceptor(retry, condition));
42+
InnerRetryInterceptor interceptor = new InnerRetryInterceptor(retry, condition);
43+
addInterceptorToFeatureMethod(featureMethod, interceptor);
44+
}
45+
46+
private static void addInterceptorToFeatureMethod(MethodInfo featureMethod, InnerRetryInterceptor interceptor) {
47+
List<IMethodInterceptor> interceptors = featureMethod.getInterceptors();
48+
for (int i = 0; i < interceptors.size(); i++) {
49+
IMethodInterceptor existing = interceptors.get(i);
50+
if (existing instanceof PendingFeatureInterceptor) {
51+
//Make sure we insert the RetryInterceptor before the PendingFeatureInterceptor
52+
//otherwise the PendingFeatureInterceptor will fail, because the RetryInterceptor swallowed the exceptions.
53+
//This would lead to a PendingFeatureSuccessfulError
54+
interceptors.add(i, interceptor);
55+
return;
56+
}
57+
}
58+
interceptors.add(interceptor);
4359
}
4460

4561
@Override
@@ -59,7 +75,7 @@ public void intercept(IMethodInvocation invocation) throws Throwable {
5975

6076
private class InnerRetryInterceptor extends RetryBaseInterceptor implements IMethodInterceptor {
6177

62-
public InnerRetryInterceptor(Retry retry, Closure condition) {
78+
public InnerRetryInterceptor(Retry retry, Closure<?> condition) {
6379
super(retry, condition);
6480
}
6581

spock-core/src/main/java/spock/lang/Retry.java

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616

1717
package spock.lang;
1818

19+
import org.opentest4j.TestAbortedException;
1920
import org.spockframework.runtime.extension.ExtensionAnnotation;
21+
import org.spockframework.runtime.extension.builtin.PendingFeatureSuccessfulError;
2022
import org.spockframework.runtime.extension.builtin.RetryExtension;
2123
import org.spockframework.util.Beta;
2224

@@ -54,6 +56,23 @@
5456
*/
5557
Class<? extends Throwable>[] exceptions() default {Exception.class, AssertionError.class};
5658

59+
/**
60+
* Configures which types of Exceptions should skip any remaining retry.
61+
* If the retry is skipped, the exception will be thrown.
62+
*
63+
* <p>Subclasses are included if their parent class is listed.
64+
*
65+
* <p>These Exceptions are evaluated before {@link #exceptions()} are evaluated.
66+
*
67+
* @return array of Exception classes which cause the retry to skip.
68+
* @since 2.4
69+
*/
70+
@Beta
71+
Class<? extends Throwable>[] skipRetryExceptions() default {
72+
TestAbortedException.class, //TestAbortedException shall skip to allow interop with annotations like @IgnoreIf, see #1863
73+
PendingFeatureSuccessfulError.class //Iterop with @PendingFeature to skip the Retry
74+
};
75+
5776
/**
5877
* Condition that is evaluated to decide whether the feature should be
5978
* retried.
@@ -99,7 +118,7 @@ enum Mode {
99118
ITERATION,
100119

101120
/**
102-
* Retry the the feature together with the setup and cleanup methods.
121+
* Retry the feature together with the setup and cleanup methods.
103122
*/
104123
SETUP_FEATURE_CLEANUP
105124
}
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* https://www.apache.org/licenses/LICENSE-2.0
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
15+
package org.spockframework.smoke.extension
16+
17+
18+
import spock.lang.Issue
19+
import spock.lang.PendingFeature
20+
import spock.lang.PendingFeatureIf
21+
import spock.lang.Retry
22+
import spock.lang.Specification
23+
24+
import static spock.lang.Retry.Mode.ITERATION
25+
import static spock.lang.Retry.Mode.SETUP_FEATURE_CLEANUP
26+
27+
/**
28+
* This specification shall test the interoperation between {@link PendingFeature} and the {@link Retry} extension.
29+
*/
30+
@Issue("https://github.com/spockframework/spock/issues/1863")
31+
class RetryPendingFeatureInteropSpec extends Specification {
32+
33+
/**
34+
* The interop between @PendingFeature and @Retry is tricky.
35+
* If the PendingFeature is first the feature method interceptor is the outer one,
36+
* whereas the retry will swallow the failure, and the the PendingFeature fails with PendingFeatureSuccessfulError.
37+
* So this and the next test checks for the different interceptor combinations.
38+
*/
39+
@PendingFeature
40+
@Retry(mode = SETUP_FEATURE_CLEANUP)
41+
def "Retry feature shall be skipped when using PendingFeature"() {
42+
expect:
43+
false
44+
}
45+
46+
/**
47+
* See test above, where the order of the annotations is switch, where the retry is the outer interceptor.
48+
*/
49+
@Retry(mode = SETUP_FEATURE_CLEANUP)
50+
@PendingFeature
51+
def "Retry feature inverse annotation order shall be skipped when using PendingFeature"() {
52+
expect:
53+
false
54+
}
55+
56+
@PendingFeatureIf({ true })
57+
@Retry(mode = SETUP_FEATURE_CLEANUP)
58+
def "Retry feature shall be skipped when using PendingFeatureIf"() {
59+
expect:
60+
false
61+
}
62+
63+
@Retry(mode = SETUP_FEATURE_CLEANUP)
64+
@PendingFeatureIf({ true })
65+
def "Retry feature inverse annotation order shall be skipped when using PendingFeatureIf"() {
66+
expect:
67+
false
68+
}
69+
70+
@PendingFeature
71+
@Retry(mode = ITERATION)
72+
def "Retry iteration shall be skipped when using PendingFeature"() {
73+
expect:
74+
false
75+
}
76+
77+
@PendingFeature
78+
@Retry(mode = SETUP_FEATURE_CLEANUP)
79+
def "Retry feature parameterized shall be skipped when using PendingFeature"() {
80+
expect:
81+
if (i)
82+
throw new Exception()
83+
84+
where:
85+
i << [1, 2]
86+
}
87+
88+
@PendingFeature
89+
@Retry(mode = ITERATION)
90+
def "Retry iteration parameterized shall be skipped when using PendingFeature"() {
91+
expect:
92+
if (i)
93+
throw new Exception()
94+
95+
where:
96+
i << [1, 2]
97+
}
98+
99+
@PendingFeatureIf({ true })
100+
@Retry(mode = ITERATION)
101+
def "Retry iteration parameterized shall be skipped when using PendingFeatureIf"() {
102+
expect:
103+
if (i)
104+
throw new Exception()
105+
106+
where:
107+
i << [1, 2]
108+
}
109+
}
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
/*
2+
* Copyright 2025 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* https://www.apache.org/licenses/LICENSE-2.0
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
15+
package org.spockframework.smoke.extension
16+
17+
import org.opentest4j.TestAbortedException
18+
import spock.lang.IgnoreIf
19+
import spock.lang.Issue
20+
import spock.lang.Requires
21+
import spock.lang.Retry
22+
import spock.lang.Specification
23+
24+
import static spock.lang.Retry.Mode.ITERATION
25+
import static spock.lang.Retry.Mode.SETUP_FEATURE_CLEANUP
26+
27+
/**
28+
* This specification shall test the interoperation of API using the {@link TestAbortedException} and the {@link Retry} extension.
29+
*/
30+
@Issue("https://github.com/spockframework/spock/issues/1863")
31+
class RetryTestAbortedExceptionInteropSpec extends Specification {
32+
33+
@Retry(mode = ITERATION)
34+
def "Retry iteration shall be skipped if a TestAbortedException is thrown"() {
35+
expect:
36+
throw new TestAbortedException()
37+
}
38+
39+
@Retry(mode = SETUP_FEATURE_CLEANUP)
40+
def "Retry feature shall be skipped if a TestAbortedException is thrown"() {
41+
expect:
42+
throw new TestAbortedException()
43+
}
44+
45+
@Retry(mode = ITERATION)
46+
def "Retry iteration parameterized shall be skipped if a TestAbortedException is thrown"() {
47+
expect:
48+
if (i > 0)
49+
throw new TestAbortedException("$i")
50+
51+
where:
52+
i << [1, 2]
53+
}
54+
55+
@Retry(mode = SETUP_FEATURE_CLEANUP)
56+
def "Retry feature parameterized shall be skipped if a TestAbortedException is thrown"() {
57+
expect:
58+
if (i > 0)
59+
throw new TestAbortedException("$i")
60+
61+
where:
62+
i << [1, 2]
63+
}
64+
65+
@IgnoreIf({ true })
66+
@Retry
67+
def "Retry shall be skipped when feature is ignored"() {
68+
expect:
69+
false
70+
}
71+
72+
@IgnoreIf({ true })
73+
@Retry
74+
def "Retry parameterized shall be skipped when feature is ignored"() {
75+
expect:
76+
false
77+
78+
where:
79+
i << [1, 2]
80+
}
81+
82+
@Requires({ false })
83+
@Retry
84+
def "Retry shall be skipped when using Requires"() {
85+
expect:
86+
false
87+
}
88+
89+
@Requires({ data.get("i") && false })
90+
@Retry(mode = ITERATION)
91+
def "Retry iteration parameterized shall be skipped when using Requires"() {
92+
expect:
93+
false
94+
95+
where:
96+
i << [1, 2]
97+
}
98+
99+
@Requires({ data.get("i") && false })
100+
@Retry(mode = SETUP_FEATURE_CLEANUP)
101+
def "Retry feature parameterized shall be skipped when using Requires"() {
102+
expect:
103+
false
104+
105+
where:
106+
i << [1, 2]
107+
}
108+
}

0 commit comments

Comments
 (0)