Listener's "before" and "after" methods should be executed in the same thread as the test itself #3044

asolntsev · 2024-01-28T19:15:50Z

TestNG Version

7.9.0

Expected behavior

I have a listener with methods beforeInvocation and afterInvocation (or, say, onTestStart/onTestSuccess/onTestFailure).
They prepare some ThreadLocal objects (used during the test or after the test).

I expect TestNG to execute them in the same thread as the test itself.

Actual behavior

TestNG runs all Listeners' methods in main thread, but my test - in different thread (TestNG-method=mySuperTest-1).
ThreadLocals stay untouched.

Is the issue reproducible on runner?

Test case sample

A sample project that reproduces the problem:
https://github.com/asolntsev/testng-threads-sample/

The text was updated successfully, but these errors were encountered:

krmahadevan · 2024-01-29T03:17:26Z

Duplicate of #914

krmahadevan · 2024-01-29T03:28:57Z

@asolntsev - Can you please track this via #914 ?

In the meantime please check if using InheritableThreadLocal helps you get past the problem.

Here's a sample

import org.testng.IInvokedMethod;
import org.testng.IInvokedMethodListener;
import org.testng.ITestResult;

import java.util.UUID;

public class SampleListener implements IInvokedMethodListener {

    public static final ThreadLocal<String> tracker = new InheritableThreadLocal<>() {
        @Override
        protected String initialValue() {
            String text = UUID.randomUUID().toString();
            System.err.println("Setting the initial value :" + text);
            return text;
        }
    };

    @Override
    public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
        String testMethod = testResult.getMethod().getMethodName();
        System.err.println("[beforeInvocation()] Thread id " + Thread.currentThread().getId() + " value = " +
        tracker.get() + " test-method = " + testMethod);
    }
}

Test class

import org.testng.annotations.Listeners;
import org.testng.annotations.Test;

import static com.rationaleemotions.github.issue3044.SampleListener.tracker;

@Listeners(SampleListener.class)
public class SampleTestCase {

    @Test(timeOut = 1000L)
    public void a() {
        System.err.println("[a()] Thread id " + Thread.currentThread().getId() + " value = " + tracker.get());
    }

    @Test
    public void b() {
        System.err.println("[b() ]Thread id " + Thread.currentThread().getId() + " value = " + tracker.get());
    }
}

suite file

<!DOCTYPE suite SYSTEM "https://testng.org/testng-1.0.dtd">
<suite name="2980_suite" verbose="2" parallel="methods">
    <test name="2980_test" verbose="2" parallel="methods">
        <classes>
            <class name="com.github.issue3044.SampleTestCase"/>
        </classes>
    </test>
</suite>

Output

...
... TestNG 7.9.0 by Cédric Beust ([email protected])
...

Setting the initial value :6c5c8d77-5b92-4281-b550-3cef65fdd168
Setting the initial value :2f705c3f-4326-44f5-93ba-af58e94afbc0
[beforeInvocation()] Thread id 15 value = 2f705c3f-4326-44f5-93ba-af58e94afbc0 test-method = a
[beforeInvocation()] Thread id 16 value = 6c5c8d77-5b92-4281-b550-3cef65fdd168 test-method = b
[a()] Thread id 15 value = 2f705c3f-4326-44f5-93ba-af58e94afbc0
[b() ]Thread id 16 value = 6c5c8d77-5b92-4281-b550-3cef65fdd168
PASSED: com.rationaleemotions.github.issue3044.SampleTestCase.b
PASSED: com.rationaleemotions.github.issue3044.SampleTestCase.a

===============================================
    2980_test
    Tests run: 2, Failures: 0, Skips: 0
===============================================


===============================================

asolntsev · 2024-01-29T07:38:49Z

@krmahadevan Thank you for the quick reply. Will continue in #914 .

P.S. No, InheritableThreadLocal doesn't help because the "after" method who needs to check to threadlocals is executed in "main" thread. Thus, it sees only the "parent" thread's ThreadLocal which stays empty (tests add values only to "child" thread's ThreadLocals).

krmahadevan · 2024-01-29T08:04:19Z

@asolntsev - The sample you shared makes use of onTestStart() and onTestSuccess(). I don't think that ITestListener was guaranteed to run on the same thread as the @Test. But the IInvokedMethodListener is guaranteed to run on the same thread as the @Test method.

Can you please switch to using beforeInvocation() and afterInvocation() and internally check if the method is a @Test method and then proceed with inspecting its test status and incorporate the same logic?

The sample that I shared uses the IInvokedMethodListener`

So if you make the below changes and try running ./gradlew test then you will see both the tests fail (which matches the expectation documented in the README.md)

diff --git a/build.gradle b/build.gradle
index 252ce24..80e3cd4 100644
--- a/build.gradle
+++ b/build.gradle
@@ -1,5 +1,6 @@
 plugins {
     id 'java'
+    id 'idea'
 }
 
 group = 'io.github.asolntsev'
@@ -15,5 +16,7 @@ dependencies {
 }
 
 test {
-    useTestNG()
+    useTestNG() {
+        suites("src/test/resources/testng.xml")
+    }
 }
\ No newline at end of file
diff --git a/src/test/java/io/github/asolntsev/ErrorsCollector.java b/src/test/java/io/github/asolntsev/ErrorsCollector.java
index e61ab23..e4fef99 100644
--- a/src/test/java/io/github/asolntsev/ErrorsCollector.java
+++ b/src/test/java/io/github/asolntsev/ErrorsCollector.java
@@ -6,7 +6,12 @@ import java.util.ArrayList;
 import java.util.List;
 
 public class ErrorsCollector {
-  private static final ThreadLocal<List<String>> errors = withInitial(ArrayList::new);
+  private static final ThreadLocal<List<String>> errors = new InheritableThreadLocal<>() {
+    @Override
+    protected List<String> initialValue() {
+      return new ArrayList<>();
+    }
+  };
 
   public static void reset() {
     errors.remove();
diff --git a/src/test/java/io/github/asolntsev/MyListener.java b/src/test/java/io/github/asolntsev/MyListener.java
index e734920..7626da6 100644
--- a/src/test/java/io/github/asolntsev/MyListener.java
+++ b/src/test/java/io/github/asolntsev/MyListener.java
@@ -6,24 +6,33 @@ import java.util.List;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.testng.IInvokedMethod;
+import org.testng.IInvokedMethodListener;
 import org.testng.ITestListener;
 import org.testng.ITestResult;
 
-public class MyListener implements ITestListener {
-  private static final Logger log = LoggerFactory.getLogger(MyListener.class);
+public class MyListener implements ITestListener, IInvokedMethodListener {
+    private static final Logger log = LoggerFactory.getLogger(MyListener.class);
 
-  @Override
-  public void onTestStart(ITestResult result) {
-    log.info("Before test {} in {}", result.getName(), currentThread().getName());
-    ErrorsCollector.reset();
-  }
 
-  @Override
-  public void onTestSuccess(ITestResult result) {
-    log.info("After test {} in {}", result.getName(), currentThread().getName());
-    List<String> errors = ErrorsCollector.getErrors();
-    if (errors != null && !errors.isEmpty()) {
-      throw new AssertionError("Errors found: " + errors);
+    @Override
+    public void beforeInvocation(IInvokedMethod method, ITestResult result) {
+        if (method.isConfigurationMethod()) {
+            return;
+        }
+        log.info("Before test {} in {}", result.getName(), currentThread().getName());
+        ErrorsCollector.reset();
+    }
+
+    @Override
+    public void afterInvocation(IInvokedMethod method, ITestResult result) {
+        if (method.isConfigurationMethod()) {
+            return;
+        }
+        log.info("After test {} in {}", result.getName(), currentThread().getName());
+        List<String> errors = ErrorsCollector.getErrors();
+        if (errors != null && !errors.isEmpty()) {
+            throw new AssertionError("Errors found: " + errors);
+        }
     }
-  }
 }

Execution output

➜  testng-threads-sample git:(main) ✗ ./gradlew test

> Task :test FAILED

2980_suite > 2980_test > io.github.asolntsev.WithTimeoutTest > myTest FAILED
    java.lang.AssertionError at MyListener.java:35

2980_suite > 2980_test > io.github.asolntsev.WithTimeoutTest > myTestWithTimeout FAILED
    java.lang.AssertionError at MyListener.java:35

4 tests completed, 2 failed, 2 skipped

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':test'.
> There were failing tests. See the report at: file:///Users/kmahadevan/githome/playground/github_issues/testng-threads-sample/build/reports/tests/test/index.html

* Try:
> Run with --scan to get full insights.

BUILD FAILED in 1s
3 actionable tasks: 1 executed, 2 up-to-date

The suite file that is being used here looks like below

<!DOCTYPE suite SYSTEM "https://testng.org/testng-1.0.dtd">
<suite name="2980_suite" verbose="2" parallel="methods">
    <test name="2980_test" verbose="2" parallel="methods">
        <classes>
            <class name="io.github.asolntsev.WithTimeoutTest"/>
        </classes>
    </test>
</suite>

asolntsev · 2024-01-29T11:28:51Z

@krmahadevan Thanks, but it didn't help.
Added methods beforeInvocation and afterInvocation are still executed in different thread (compared to test).
I committed my changes to https://github.com/asolntsev/testng-threads-sample/

krmahadevan · 2024-01-29T12:38:32Z

@asolntsev - I forgot to call out one important aspect. The parallel is set to methods in the suite file. So can you please alter your gradle build file and configure TestNG as below and try again?

test {
    useTestNG {
        options ->
            options.parallel = 'methods'
    }
}

asolntsev · 2024-01-29T21:33:58Z

@krmahadevan Yes, after adding parallel = 'methods' "before" and "after" methods are executed in the same thread with test.
But

What if I don't want to run tests in parallel?
What about all these arguments from Thread ID changes between test and *Invocation events when the test timeout set #914 (comment) that "it breaks contract", "its going to be unfair" etc. ?

krmahadevan · 2024-01-31T02:28:36Z

Yes thats the caveat that we have to use parallel as methods to get this to work which is not optimal.

I believe for the other part i have added my thoughts as a reply to your comment.

asolntsev changed the title ~~Listener's methods "beforeInvocation" and "afterInovcations" should be executed in the same thread as the test itself~~ Listener's "before" and "after" methods should be executed in the same thread as the test itself Jan 28, 2024

asolntsev mentioned this issue Jan 28, 2024

TextReport listener for TestNg generates an empty report for tests with timeout selenide/selenide#2609

Closed

krmahadevan marked this as a duplicate of #914 Jan 29, 2024

krmahadevan closed this as completed Jan 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Listener's "before" and "after" methods should be executed in the same thread as the test itself #3044

Listener's "before" and "after" methods should be executed in the same thread as the test itself #3044

asolntsev commented Jan 28, 2024 •

edited

Loading

krmahadevan commented Jan 29, 2024 •

edited

Loading

krmahadevan commented Jan 29, 2024

asolntsev commented Jan 29, 2024

krmahadevan commented Jan 29, 2024

asolntsev commented Jan 29, 2024

krmahadevan commented Jan 29, 2024

asolntsev commented Jan 29, 2024

krmahadevan commented Jan 31, 2024

Listener's "before" and "after" methods should be executed in the same thread as the test itself #3044

Listener's "before" and "after" methods should be executed in the same thread as the test itself #3044

Comments

asolntsev commented Jan 28, 2024 • edited Loading

TestNG Version

Expected behavior

Actual behavior

Is the issue reproducible on runner?

Test case sample

krmahadevan commented Jan 29, 2024 • edited Loading

krmahadevan commented Jan 29, 2024

asolntsev commented Jan 29, 2024

krmahadevan commented Jan 29, 2024

asolntsev commented Jan 29, 2024

krmahadevan commented Jan 29, 2024

asolntsev commented Jan 29, 2024

krmahadevan commented Jan 31, 2024

asolntsev commented Jan 28, 2024 •

edited

Loading

krmahadevan commented Jan 29, 2024 •

edited

Loading