before configuration and before invocation should be 'SKIP' when beforeMethod is 'skip' #2880

bobshie · 2023-03-09T08:33:19Z

TestNG Version

Note: only the latest version is supported
7.6.1

Expected behavior

16:02:12.583 [main] INFO  TestNgDemoListener - after invocation executed, method:beforeClass,testResultStatus:FAILURE/2
java.lang.ArithmeticException: / by zero
16:02:12.631 [main] INFO  TestNgDemoListener - before configuration executed, method:beforeMethod,testResultStatus:**SKIP/3**
16:02:12.631 [main] INFO  TestNgDemoListener - before invocation executed, method:beforeMethod,testResultStatus:**SKIP/3**
16:02:12.632 [main] INFO  TestNgDemoListener - after invocation executed, method:beforeMethod,testResultStatus:SKIP/3
16:02:12.633 [main] INFO  TestNgDemoListener - Skip Config Method,beforeMethod,testResultStatus:SKIP/3
14:03:16.659 [main] INFO  TestNgDemoListener - onTestStart executed, method:issueTest,testResultStatus:STARTED/16
14:03:16.661 [main] INFO  TestNgDemoListener - onTestSkipped executed, method:issueTest,testResultStatus:SKIP/3
14:03:16.660 [main] INFO  TestNgDemoListener - before invocation executed, method:issueTest,testResultStatus:SKIP/3
14:03:16.660 [main] INFO  TestNgDemoListener - after invocation executed, method:issueTest,testResultStatus:SKIP/3

Actual behavior

16:02:12.583 [main] INFO  TestNgDemoListener - after invocation executed, method:beforeClass,testResultStatus:FAILURE/2
java.lang.ArithmeticException: / by zero
16:02:12.631 [main] INFO  TestNgDemoListener - before configuration executed, method:beforeMethod,testResultStatus:**STARTED/16**
16:02:12.631 [main] INFO  TestNgDemoListener - before invocation executed, method:beforeMethod,testResultStatus:**STARTED/16**
16:02:12.632 [main] INFO  TestNgDemoListener - after invocation executed, method:beforeMethod,testResultStatus:SKIP/3
16:02:12.633 [main] INFO  TestNgDemoListener - Skip Config Method,beforeMethod,testResultStatus:SKIP/3
14:03:16.659 [main] INFO  TestNgDemoListener - onTestStart executed, method:issueTest,testResultStatus:STARTED/16
14:03:16.661 [main] INFO  TestNgDemoListener - onTestSkipped executed, method:issueTest,testResultStatus:SKIP/3
14:03:16.660 [main] INFO  TestNgDemoListener - before invocation executed, method:issueTest,testResultStatus:**SKIP/3**
14:03:16.660 [main] INFO  TestNgDemoListener - after invocation executed, method:issueTest,testResultStatus:SKIP/3

Is the issue reproducible on runner?

Test case sample

Please, share the test case (as small as possible) which shows the issue
Simply use listener:

public class TestNgDemoListener implements IInvokedMethodListener, IConfigurationListener, ITestListener

import org.slf4j.LoggerFactory;
import org.testng.IConfigurationListener;
import org.testng.IInvokedMethod;
import org.testng.IInvokedMethodListener;
import org.testng.ITestListener;
import org.testng.ITestNGMethod;
import org.testng.ITestResult;

public class TestNgDemoListener implements IInvokedMethodListener, IConfigurationListener, ITestListener {
    org.slf4j.Logger logger = LoggerFactory.getLogger(TestNgDemoListener.class);

    @Override
    public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
        logger.info(
                "before invocation executed, method:" + testResult.getMethod().getMethodName() + ",testResultStatus:"
                        + getStatusString(testResult.getStatus()));

    }

    @Override
    public void beforeConfiguration(ITestResult testResult) {
        logger.info(
                "before configuration executed, method:" + testResult.getMethod().getMethodName() + ",testResultStatus:"
                        + getStatusString(testResult.getStatus()));
    }

    @Override
    public void onConfigurationSkip(ITestResult testResult, ITestNGMethod method) {
        logger.info("Skip Config Method," + testResult.getMethod().getMethodName() + ",testResultStatus:"
                + getStatusString(testResult.getStatus()));
    }

    @Override
    public void afterInvocation(IInvokedMethod method, ITestResult testResult) {
        logger.info(
                "after invocation executed, method:" + testResult.getMethod().getMethodName() + ",testResultStatus:"
                        + getStatusString(testResult.getStatus()));
    }

    @Override
    public void onTestStart(ITestResult result) {
        logger.info(
                "onTestStart executed, method:" + result.getMethod().getMethodName() + ",testResultStatus:"
                        + getStatusString(result.getStatus()));
    }

    @Override
    public void onTestSkipped(ITestResult result) {
        logger.info(
                "onTestSkipped executed, method:" + result.getMethod().getMethodName() + ",testResultStatus:"
                        + getStatusString(result.getStatus()));
    }

    public static String getStatusString(int status) {
        return switch (status) {
        case -1 -> "CREATED/" + status;
        case 1 -> "SUCCESS/" + status;
        case 2 -> "FAILURE/" + status;
        case 3 -> "SKIP/" + status;
        case 4 -> "SUCCESS_PERCENTAGE_FAILURE/" + status;
        case 16 -> "STARTED/" + status;
        default -> "Others/" + status;
        };
    }
}

import org.slf4j.LoggerFactory;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.BeforeGroups;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.BeforeSuite;
import org.testng.annotations.BeforeTest;
import org.testng.annotations.Listeners;
import org.testng.annotations.Test;

@Listeners({ TestNgDemoListener.class})
public class TestNgDemoTest {
    org.slf4j.Logger logger = LoggerFactory.getLogger(TestNgDemoTest.class);

    @BeforeClass
    public void beforeClass() {
        logger.info("beforeClass");
        int i = 5/0; // throw an exception
    }

    @BeforeMethod
    public void beforeMethod() {
        logger.info("beforeMethod");
    }

    @BeforeTest
    public void beforeTest() {
        logger.info("beforeTest");
    }

    @BeforeSuite
    public void beforeSuite() {
        logger.info("beforeSuite");
    }

    @BeforeGroups
    public void beforeGroups(){
        logger.info("beforeGroups");
    }


    @Test
    public void issueTest() {
        logger.info("Test BeforeConfiguration behavior");
    }

}

Contribution guidelines

Incase you plan to raise a pull request to fix this issue, please make sure you refer our Contributing section for detailed set of steps.

Description

After one of our test project uplift TestNG from 7.5 to 7.6 version, there are some unexpected behavior from before. the related issue is:
beforeConfiguration() listener method should be invoked for skipped configurations as well · Issue #2729 · cbeust/testng · GitHub

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

krmahadevan · 2023-03-09T08:36:40Z

@bobshie -Please include a sample (preferably a sample project) that can be used to reproduce the problem

bobshie · 2023-03-09T09:02:37Z

@krmahadevan , Thanks, I add an example and there is conflict with:

it should be the same status in before invocation of 'beforeMethod' and 'issueTest'

16:02:12.631 [main] INFO  TestNgDemoListener - before invocation executed, method:beforeMethod,testResultStatus:STARTED/16

14:03:16.660 [main] INFO  TestNgDemoListener - before invocation executed, method:issueTest,testResultStatus:SKIP/3

Hkh9966 · 2023-03-09T09:37:00Z

Hi @krmahadevan , in order to simply reproduce this problem, I did some simple code and drew a flow of the issue that is being discussed now:

If there are three methods:

  @BeforeClass
    public void beforeClass() {
        int i = 5/0; //throw an exception
    }

    @BeforeMethod
    public void beforeMethod() {

    }

    @Test
    public void issueTest() {
        System.out.println("Test BeforeConfiguration behavior");
    }

And a customized listener:

public class DemoListener implements IInvokedMethodListener, IConfigurationListener,ITestListener {

    @Override
    public void beforeConfiguration(ITestResult testResult) {
        System.out.println(
                "before configuration executed, method:" + testResult.getMethod().getMethodName() + ",testResultStatus:"
                        + testResult.getStatus());
    }

    @Override
    public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
        System.out.println(
                "before invocation executed, method:" + testResult.getMethod().getMethodName() + ",testResultStatus:"
                        + testResult.getStatus());
    }

    @Override
    public void onConfigurationSkip(ITestResult testResult, ITestNGMethod method) {
        System.out.println("Skip Config Method," + testResult.getMethod().getMethodName() + ",testResultStatus:"
                + testResult.getStatus());
    }

    @Override
    public void onTestStart(ITestResult testResult) {
        System.out.println("OnTestStart Status:" + testResult.getMethod().getMethodName() + ",testResultStatus:"
                + testResult.getStatus());
    }

}

The execution process is performed as shown in the figure:

In my opinion,since a config method(just like beforeMethod in this demo) has been determined to be skipped, the status obtained in beforeConfiguration() and beforeInvocation() for config method should be skip. In this way, config method and @test method are consistent.

And I also found a phenomenon in which @OnTestStart and beforeInvocation for @test method also behave inconsistently. I’m not sure if TestNg’s definition of these two methods is different.

As you know, since this issue was solved: #2729. If there is a situation in the demo, beforeConfiguration() will also be invoked, and we will get the status as STARTED, which will add a lot of changes for our project.

krmahadevan · 2023-03-09T09:40:21Z

@Hkh9966 - Since you have spent time on analysing this, would you be willing to help fix it by raising a PR?

bobshie · 2023-03-10T00:44:43Z

@krmahadevan , I'll try to do it.

Can we fix it as:
the expected status in beforeConfiguration & beforeInvocation should be 'SKIP' when beforeMethod is 'SKIP'.
and the expected status in onTestStart & beforeInvocation should be 'SKIP' when test method is 'SKIP'.

krmahadevan · 2023-03-10T02:15:44Z

@bobshie sure. It also goes without saying that this fix should not cause regression and all existing tests work fine.

@juherr what do you think ?

Hkh9966 · 2023-03-10T03:41:22Z

If we can change it to:
"The status in beforeConfiguration & beforeInvocation should be 'SKIP' when beforeMethod is 'SKIP'."
It is enough to solve our problems.

For "onTestStart & beforeInvocation should be 'SKIP' when test method is 'SKIP'", it may be that the changes will be large. And I'm not sure if this change is justified.

krmahadevan · 2023-03-10T03:44:22Z

@Hkh9966

For "onTestStart & beforeInvocation should be 'SKIP' when test method is 'SKIP'", it may be that the changes will be large. And I'm not sure if this change is justified.

In that case, can we please file do the following ?

Update this defect's description to explicitly track configuration methods discrepancy
Create a new github issue for the other one ?

bobshie · 2023-03-10T04:13:53Z

@krmahadevan , I think there is clear description in this thread, we only need to change the first one:
The status in beforeConfiguration & beforeInvocation should be 'SKIP' when beforeMethod is 'SKIP'."

about the next issue, testng has onTestStart() and onTestSkip() when test method is skipped:

12:02:55.353 [main] INFO  TestNgDemoListener - onTestStart executed, method:issueTest,testResultStatus:STARTED/16
Test ignored.
12:02:55.354 [main] INFO  TestNgDemoListener - before invocation executed, method:issueTest,testResultStatus:SKIP/3
12:02:55.354 [main] INFO  TestNgDemoListener - after invocation executed, method:issueTest,testResultStatus:SKIP/3
12:02:55.355 [main] INFO  TestNgDemoListener - onTestSkipped executed, method:issueTest,testResultStatus:SKIP/3

so we can't change onTestStart(STARTED/16) to onTestStart(SKIP/3). it's also very confusing.

I can just fix the first one if it's agreed.

Hkh9966 · 2023-03-10T05:32:54Z

In that case, can we please file do the following ?

Update this defect's description to explicitly track configuration methods discrepancy

Create a new github issue for the other one ?

@krmahadevan I agree with you.

Signed-off-by: Bob Shi <bob.shi@ericsson.com>

Closes #2880 Ensure that before configuration and before invocation should be 'SKIP' when beforeMethod is 'skip'

krmahadevan added the Need: sample label Mar 9, 2023

bobshie added a commit to bobshie/testng that referenced this issue Mar 10, 2023

change before config/invocation status 'SKIP' testng-team#2880

f66abde

Signed-off-by: Bob Shi <bob.shi@ericsson.com>

bobshie mentioned this issue Mar 11, 2023

Bobshie beforeconfig skip #2881

Merged

krmahadevan closed this as completed in #2881 Mar 14, 2023

krmahadevan pushed a commit that referenced this issue Mar 14, 2023

Skip config listener calls for skipped configs

8b5f4d1

Closes #2880 Ensure that before configuration and before invocation should be 'SKIP' when beforeMethod is 'skip'

krmahadevan added Feature: listener and removed Need: sample labels Mar 14, 2023

krmahadevan added this to the 7.8.0 milestone Mar 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

before configuration and before invocation should be 'SKIP' when beforeMethod is 'skip' #2880

before configuration and before invocation should be 'SKIP' when beforeMethod is 'skip' #2880

bobshie commented Mar 9, 2023 •

edited by juherr

Loading

krmahadevan commented Mar 9, 2023

bobshie commented Mar 9, 2023 •

edited by juherr

Loading

Hkh9966 commented Mar 9, 2023 •

edited by krmahadevan

Loading

krmahadevan commented Mar 9, 2023

bobshie commented Mar 10, 2023

krmahadevan commented Mar 10, 2023

Hkh9966 commented Mar 10, 2023 •

edited

Loading

krmahadevan commented Mar 10, 2023

bobshie commented Mar 10, 2023 •

edited by juherr

Loading

Hkh9966 commented Mar 10, 2023

before configuration and before invocation should be 'SKIP' when beforeMethod is 'skip' #2880

before configuration and before invocation should be 'SKIP' when beforeMethod is 'skip' #2880

Comments

bobshie commented Mar 9, 2023 • edited by juherr Loading

TestNG Version

Expected behavior

Actual behavior

Is the issue reproducible on runner?

Test case sample

Contribution guidelines

Description

krmahadevan commented Mar 9, 2023

bobshie commented Mar 9, 2023 • edited by juherr Loading

Hkh9966 commented Mar 9, 2023 • edited by krmahadevan Loading

krmahadevan commented Mar 9, 2023

bobshie commented Mar 10, 2023

krmahadevan commented Mar 10, 2023

Hkh9966 commented Mar 10, 2023 • edited Loading

krmahadevan commented Mar 10, 2023

bobshie commented Mar 10, 2023 • edited by juherr Loading

Hkh9966 commented Mar 10, 2023

bobshie commented Mar 9, 2023 •

edited by juherr

Loading

bobshie commented Mar 9, 2023 •

edited by juherr

Loading

Hkh9966 commented Mar 9, 2023 •

edited by krmahadevan

Loading

Hkh9966 commented Mar 10, 2023 •

edited

Loading

bobshie commented Mar 10, 2023 •

edited by juherr

Loading