Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Subsequent skipped configurations do not set a throwable #3166

Open
1 of 7 tasks
aws-sde opened this issue Aug 18, 2024 · 2 comments
Open
1 of 7 tasks

Subsequent skipped configurations do not set a throwable #3166

aws-sde opened this issue Aug 18, 2024 · 2 comments

Comments

@aws-sde
Copy link

aws-sde commented Aug 18, 2024

TestNG Version

7.10.2

Expected behavior

All failed or skipped configuration methods should have a throwable set on their test result.

Actual behavior

Only the first failed or skipped configuration method has a throwable set in its test result. Subsequent configuration methods do get skipped (due to the initial failure/skip), but they do not set the throwable.

This has the consequence of intermittently leaving the throwable null on test results for skipped test methods. When TestInvoker detects a configuration failure, it marks the test method as skipped, and sets its throwable to be the configuration method's throwable. There are two problems with this:

  1. Failed and skipped configurations are stored in a set based on a ConcurrentHashMap, which does not guarantee ordering.
  2. Exception.getExceptionDetails naïvely uses the throwable of the first match for a configuration method's test result.

Is the issue reproducible on runner?

  • Shell
  • Maven
  • Gradle
  • Ant
  • Eclipse
  • IntelliJ
  • NetBeans

Test case sample

I'm not able to reproduce onTestSkipped showing a null throwable with this, but it absolutely happens in my actual code base. In any case, I think this sufficiently demonstrates that the throwable is null for the other configuration.

package org.example;

import org.testng.SkipException;
import org.testng.annotations.*;

@Listeners(SkipListener.class)
public class AppTest {
    @BeforeClass
    public void firstBeforeClass() {
        throw new SkipException("Skipping firstBeforeClass");
    }

    @BeforeClass(dependsOnMethods = "firstBeforeClass")
    public void secondBeforeClass() {}

    @Test
    public void appHasAGreeting() {}
}
package org.example;

import org.testng.IConfigurationListener;
import org.testng.ITestListener;
import org.testng.ITestResult;

public class SkipListener implements IConfigurationListener, ITestListener {
    @Override
    public void onConfigurationSkip(ITestResult tr) {
        System.out.printf("Configuration %s skipped: %s%n", tr.getMethod().getMethodName(), tr.getThrowable());
    }

    @Override
    public void onTestSkipped(ITestResult result) {
        System.out.printf("Test %s skipped: %s%n", result.getMethod().getMethodName(), result.getThrowable());
    }
}
Gradle suite > Gradle test > org.example.AppTest STANDARD_OUT
    Configuration firstBeforeClass skipped: org.testng.SkipException: Skipping firstBeforeClass
    Configuration secondBeforeClass skipped: null

Gradle suite > Gradle test > org.example.AppTest > appHasAGreeting SKIPPED

Gradle suite > Gradle test > org.example.AppTest STANDARD_OUT
    Test appHasAGreeting skipped: org.testng.SkipException: Skipping firstBeforeClass
@krmahadevan
Copy link
Member

@aws-sde Since you seem to have spent a good amount of time trying to debug this issue, would you like to raise a pull request to get this fixed? We can help you with the review and merge

@aws-sde
Copy link
Author

aws-sde commented Aug 18, 2024

Yes, I'll try to find time to submit a fix hopefully next weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants