Skip to content

Commit 4c6be6d

Browse files
authored
make testng custom listener evidence work automatically (fixes #1150, via #1312)
1 parent 3e35d5c commit 4c6be6d

6 files changed

Lines changed: 290 additions & 0 deletions

File tree

allure-testng/src/main/java/io/qameta/allure/testng/AllureTestNg.java

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import org.testng.ITestListener;
5454
import org.testng.ITestNGMethod;
5555
import org.testng.ITestResult;
56+
import org.testng.TestRunner;
5657
import org.testng.annotations.Parameters;
5758
import org.testng.internal.ConstructorOrMethod;
5859
import org.testng.xml.XmlSuite;
@@ -76,6 +77,7 @@
7677
import java.util.UUID;
7778
import java.util.concurrent.ConcurrentHashMap;
7879
import java.util.concurrent.CopyOnWriteArrayList;
80+
import java.util.concurrent.atomic.AtomicBoolean;
7981
import java.util.function.Consumer;
8082
import java.util.stream.Collectors;
8183
import java.util.stream.Stream;
@@ -135,6 +137,19 @@ public class AllureTestNg
135137

136138
private static final boolean HAS_CUCUMBERJVM7_IN_CLASSPATH = isClassAvailableOnClasspath("io.qameta.allure.cucumber7jvm.AllureCucumber7Jvm");
137139

140+
/**
141+
* TestNG dispatches finish events (onTestSuccess/onTestFailure/onTestSkipped) to test listeners in reverse
142+
* registration order since 7.5, while onTestStart keeps registration order. The markers pin that behavior:
143+
* {@code org.testng.internal.invokers.TestInvoker} appeared in 7.5 together with the reversal, and
144+
* {@code org.testng.ListenerComparator} is a public-API companion from 7.10 kept as a fallback in case the
145+
* internal class moves again. On older TestNG both events run in registration order, where taking the front
146+
* seat would invert the intent and close tests before custom listeners run.
147+
*/
148+
private static final boolean FINISH_EVENTS_REVERSED = isClassAvailableOnClasspath("org.testng.internal.invokers.TestInvoker")
149+
|| isClassAvailableOnClasspath("org.testng.ListenerComparator");
150+
151+
private static final AtomicBoolean LISTENER_ORDER_PROBLEM_REPORTED = new AtomicBoolean();
152+
138153
/**
139154
* The test currently running on this thread: its allure uuid plus the testng result it belongs to. The result acts
140155
* as the invocation identity, so a terminal callback can tell an already-started test from one that was skipped
@@ -259,6 +274,8 @@ public void onStart(final ISuite suite) {
259274

260275
@Override
261276
public void onStart(final ITestContext context) {
277+
promoteSelfToFirstListener(context);
278+
262279
final String uuid = getUniqueUuid(context);
263280
getLifecycle().registerScope(scopeKey(uuid));
264281

@@ -271,6 +288,68 @@ public void onStart(final ITestContext context) {
271288
}
272289
}
273290

291+
/**
292+
* Moves this listener to the front of the test runner's listener list, so it handles onTestStart first (the
293+
* test is already scheduled when custom listeners run) and finish events last (the test is still running
294+
* while custom listeners add their screenshots-on-failure and other evidence through the Allure API).
295+
* Ordering is otherwise defined by the registration mechanism alone, and the popular ones — the
296+
* {@code @Listeners} annotation and runner properties — register custom listeners in the losing position.
297+
*
298+
* <p>The move rides on {@link TestRunner#getTestListeners()} exposing the live list, which is internal
299+
* behavior; when any step of it fails, the run continues with the default order and the problem is reported
300+
* once, so the fallback is never worse than not reordering at all.</p>
301+
*/
302+
private void promoteSelfToFirstListener(final ITestContext context) {
303+
if (!FINISH_EVENTS_REVERSED) {
304+
// pre-7.5 TestNG runs finish events in registration order too, so there is no winning position
305+
// and the front seat would close tests before custom listeners run
306+
return;
307+
}
308+
if (!(context instanceof TestRunner)) {
309+
reportListenerOrderProblem("the test context is not org.testng.TestRunner", null);
310+
return;
311+
}
312+
final TestRunner runner = (TestRunner) context;
313+
final List<ITestListener> listeners = runner.getTestListeners();
314+
// a defensive copy would swallow the reorder silently; a fresh instance per call gives it away
315+
if (listeners != runner.getTestListeners()) {
316+
reportListenerOrderProblem("TestRunner.getTestListeners no longer exposes the live list", null);
317+
return;
318+
}
319+
moveSelfToFront(listeners);
320+
}
321+
322+
/**
323+
* The list surgery of {@link #promoteSelfToFirstListener(ITestContext)}: adds this listener at the front
324+
* before removing the old occurrence, so a list that rejects mutation mid-way can end up with a duplicate
325+
* but never without the listener at all.
326+
*/
327+
void moveSelfToFront(final List<ITestListener> listeners) {
328+
try {
329+
final int index = listeners.indexOf(this);
330+
if (index > 0) {
331+
listeners.add(0, this);
332+
listeners.remove(index + 1);
333+
}
334+
} catch (RuntimeException e) {
335+
reportListenerOrderProblem("the listener list rejected the change", e);
336+
}
337+
}
338+
339+
private static void reportListenerOrderProblem(final String reason, final RuntimeException exception) {
340+
if (LISTENER_ORDER_PROBLEM_REPORTED.compareAndSet(false, true)) {
341+
LOGGER.warn(
342+
"Could not move the Allure listener to the front of the TestNG listener list: {}. "
343+
+ "Attachments and steps added from custom test listeners (for example a screenshot "
344+
+ "on failure) may be missing from the report. Please report an issue to "
345+
+ "https://github.com/allure-framework/allure-java/issues "
346+
+ "including your TestNG version",
347+
reason,
348+
exception
349+
);
350+
}
351+
}
352+
274353
protected void createFakeResult(final ITestContext context, final ITestNGMethod method) {
275354
final String uuid = UUID.randomUUID().toString();
276355
final String parentUuid = UUID.randomUUID().toString();

allure-testng/src/test/java/io/qameta/allure/testng/AllureTestNgTest.java

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import io.qameta.allure.testfilter.TestPlan;
4040
import io.qameta.allure.testfilter.TestPlanV1_0;
4141
import io.qameta.allure.testng.config.AllureTestNgConfig;
42+
import io.qameta.allure.testng.samples.CustomListenerAttachments;
4243
import io.qameta.allure.testng.samples.PriorityTests;
4344
import io.qameta.allure.testng.samples.TestsWithIdForFilter;
4445
import org.assertj.core.api.Condition;
@@ -49,6 +50,7 @@
4950
import org.junit.jupiter.params.provider.Arguments;
5051
import org.junit.jupiter.params.provider.MethodSource;
5152
import org.testng.ITestContext;
53+
import org.testng.ITestListener;
5254
import org.testng.ITestNGMethod;
5355
import org.testng.ITestResult;
5456
import org.testng.TestNG;
@@ -59,8 +61,10 @@
5961

6062
import java.lang.reflect.Method;
6163
import java.net.URL;
64+
import java.util.ArrayList;
6265
import java.util.Arrays;
6366
import java.util.Collection;
67+
import java.util.Collections;
6468
import java.util.Comparator;
6569
import java.util.HashSet;
6670
import java.util.List;
@@ -78,6 +82,7 @@
7882
import static java.util.Arrays.asList;
7983
import static java.util.Collections.singletonList;
8084
import static org.assertj.core.api.Assertions.assertThat;
85+
import static org.assertj.core.api.Assertions.assertThatCode;
8186
import static org.assertj.core.api.Assertions.tuple;
8287
import static org.junit.jupiter.params.provider.Arguments.arguments;
8388
import static org.mockito.Mockito.mock;
@@ -1126,6 +1131,94 @@ public void attachmentsTest() {
11261131
.containsExactly("String attachment");
11271132
}
11281133

1134+
@AllureFeatures.Attachments
1135+
@Issue("1150")
1136+
@Test
1137+
@DisplayName("Should keep steps and attachments added by a custom listener on test failure")
1138+
public void shouldKeepCustomListenerEvidenceOnTestFailure() {
1139+
final AllureResults results = runTestNgSuites("suites/custom-listener-attachments.xml");
1140+
1141+
final TestResult failed = findTestResultByName(results, "failingTest");
1142+
assertThat(failed.getStatus())
1143+
.isEqualTo(Status.FAILED);
1144+
assertThat(failed.getSteps())
1145+
.extracting(StepResult::getName)
1146+
.containsExactly("On test start", "Take screenshot on failure", "Screenshot on failure");
1147+
assertThat(failed.getSteps())
1148+
.flatExtracting(StepResult::getAttachments)
1149+
.extracting(Attachment::getName)
1150+
.containsExactly("On test start", "Screenshot on failure");
1151+
}
1152+
1153+
@AllureFeatures.Attachments
1154+
@Issue("1150")
1155+
@Test
1156+
@DisplayName("Should keep attachments added by a custom listener on test start and success")
1157+
public void shouldKeepCustomListenerAttachmentsOnTestSuccess() {
1158+
final AllureResults results = runTestNgSuites("suites/custom-listener-attachments.xml");
1159+
1160+
final TestResult passed = findTestResultByName(results, "passingTest");
1161+
assertThat(passed.getStatus())
1162+
.isEqualTo(Status.PASSED);
1163+
assertThat(passed.getSteps())
1164+
.extracting(StepResult::getName)
1165+
.containsExactly("On test start", "On test success");
1166+
assertThat(passed.getSteps())
1167+
.flatExtracting(StepResult::getAttachments)
1168+
.extracting(Attachment::getName)
1169+
.containsExactly("On test start", "On test success");
1170+
}
1171+
1172+
@AllureFeatures.Attachments
1173+
@Issue("1150")
1174+
@Test
1175+
@DisplayName("Should keep attachments added by a custom listener registered via suite xml")
1176+
public void shouldKeepCustomListenerEvidenceWithXmlRegistration() {
1177+
final AllureResults results = runTestNgSuites("suites/custom-listener-attachments-xml.xml");
1178+
1179+
final TestResult failed = findTestResultByName(results, "failingTest");
1180+
assertThat(failed.getSteps())
1181+
.extracting(StepResult::getName)
1182+
.containsExactly("On test start", "Take screenshot on failure", "Screenshot on failure");
1183+
1184+
final TestResult passed = findTestResultByName(results, "passingTest");
1185+
assertThat(passed.getSteps())
1186+
.extracting(StepResult::getName)
1187+
.containsExactly("On test start", "On test success");
1188+
}
1189+
1190+
@AllureFeatures.Base
1191+
@Issue("1150")
1192+
@Test
1193+
@DisplayName("Should move the Allure listener to the front of the TestNG listener list")
1194+
public void shouldMoveAllureListenerToFront() {
1195+
final AllureTestNg adapter = new AllureTestNg(new AllureLifecycle(new AllureResultsWriterStub()));
1196+
final ITestListener custom = new CustomListenerAttachments.EvidenceListener();
1197+
final List<ITestListener> listeners = new ArrayList<>(Arrays.asList(custom, adapter));
1198+
1199+
adapter.moveSelfToFront(listeners);
1200+
1201+
assertThat(listeners)
1202+
.containsExactly(adapter, custom);
1203+
}
1204+
1205+
@AllureFeatures.Base
1206+
@Issue("1150")
1207+
@Test
1208+
@DisplayName("Should keep the default listener order when the TestNG listener list is unmodifiable")
1209+
public void shouldNotFailOnUnmodifiableListenerList() {
1210+
final AllureTestNg adapter = new AllureTestNg(new AllureLifecycle(new AllureResultsWriterStub()));
1211+
final ITestListener custom = new CustomListenerAttachments.EvidenceListener();
1212+
final List<ITestListener> listeners = Collections.unmodifiableList(
1213+
new ArrayList<>(Arrays.asList(custom, adapter))
1214+
);
1215+
1216+
assertThatCode(() -> adapter.moveSelfToFront(listeners))
1217+
.doesNotThrowAnyException();
1218+
assertThat(listeners)
1219+
.containsExactly(custom, adapter);
1220+
}
1221+
11291222
@AllureFeatures.MarkerAnnotations
11301223
@Issue("42")
11311224
@Test
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
* Copyright 2016-2026 Qameta Software Inc
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+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.qameta.allure.testng.samples;
17+
18+
import io.qameta.allure.Allure;
19+
import org.testng.ITestListener;
20+
import org.testng.ITestResult;
21+
import org.testng.annotations.Listeners;
22+
import org.testng.annotations.Test;
23+
24+
/**
25+
* Reproduces the popular screenshot-on-failure pattern: a custom listener registered via
26+
* {@link Listeners} that reports evidence through the Allure API from test listener callbacks.
27+
*/
28+
@Listeners(CustomListenerAttachments.EvidenceListener.class)
29+
public class CustomListenerAttachments {
30+
31+
@Test
32+
public void passingTest() {
33+
}
34+
35+
@Test
36+
public void failingTest() {
37+
throw new AssertionError("failed on purpose");
38+
}
39+
40+
public static class EvidenceListener implements ITestListener {
41+
42+
@Override
43+
public void onTestStart(final ITestResult result) {
44+
Allure.attachment("On test start", "text/plain", "before " + result.getName());
45+
}
46+
47+
@Override
48+
public void onTestSuccess(final ITestResult result) {
49+
Allure.attachment("On test success", "text/plain", "after passed " + result.getName());
50+
}
51+
52+
@Override
53+
public void onTestFailure(final ITestResult result) {
54+
Allure.step("Take screenshot on failure");
55+
Allure.attachment("Screenshot on failure", "text/plain", "after failed " + result.getName());
56+
}
57+
}
58+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/*
2+
* Copyright 2016-2026 Qameta Software Inc
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+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package io.qameta.allure.testng.samples;
17+
18+
import org.testng.annotations.Test;
19+
20+
/**
21+
* The {@link CustomListenerAttachments} scenario wired through suite XML instead of the
22+
* {@code @Listeners} annotation: the suite file registers
23+
* {@link CustomListenerAttachments.EvidenceListener} via a {@code <listeners>} element.
24+
*/
25+
public class CustomListenerAttachmentsXml {
26+
27+
@Test
28+
public void passingTest() {
29+
}
30+
31+
@Test
32+
public void failingTest() {
33+
throw new AssertionError("failed on purpose");
34+
}
35+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
3+
4+
<suite name="Custom listener attachments xml suite">
5+
<listeners>
6+
<listener class-name="io.qameta.allure.testng.samples.CustomListenerAttachments$EvidenceListener"/>
7+
</listeners>
8+
<test name="Custom listener attachments xml test">
9+
<classes>
10+
<class name="io.qameta.allure.testng.samples.CustomListenerAttachmentsXml">
11+
</class>
12+
</classes>
13+
</test>
14+
</suite>
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
3+
4+
<suite name="Custom listener attachments suite">
5+
<test name="Custom listener attachments test">
6+
<classes>
7+
<class name="io.qameta.allure.testng.samples.CustomListenerAttachments">
8+
</class>
9+
</classes>
10+
</test>
11+
</suite>

0 commit comments

Comments
 (0)