IInvokedMethodListener should not be called for methods which aren't invoked

Opening this new issue after correspondence in #1870

I can send the revised tests as a PR but without a fix for now... let me know if I should anyway.

TestNG Version

7.0.0-SNAPSHOT

Description

In TestNG's own tests, in the test class test.listeners.ordering.ListenerInvocationListenerInvocationDisabledBehaviorTest which validates listener calls and their order, one of the test method has an inaccurate expected result.
That method is testOrderHasOnlyFailedAndSkippedConfigAndSkippedTestMethod in which the executed test class contains 1 failed config method, which leads to skipping another config method and a test method. This issue appears when using the alwaysrunlisteners set to false, which is not the default behavior. I also suggest that this becomes the default behavior for IInvokedMethodListener in 7.0.0

Expected behavior

IInvokedMethodListener should only be invoked for the first config method as it is the only method being invoked by TestNG

Actual behavior

IInvokedMethodListener is invoked for both config methods, but not for the test method. This is both unexpected and inconsistent.

Is the issue reproductible on runner?

[ ] Shell
[ ] Maven
[x] Gradle
[ ] Ant
[ ] Eclipse
[x] IntelliJ
[ ] NetBeans

Test case sample

Here is the revised test method with the correct listener calls:

  @Test(description = "Test class has only 1 failed config, 1 skipped config and skipped test method")
  public void testOrderHasOnlyFailedAndSkippedConfigAndSkippedTestMethod() {
    List<String> expected = Arrays.asList(
        IEXECUTIONLISTENER_ON_EXECUTION_START,
        IALTERSUITELISTENER_ALTER,
        IANNOTATIONTRANSFORMER_TRANSFORM_3_ARGS,
        ISUITELISTENER_ON_START,
        ITESTLISTENER_ON_START_TEST_TAG,
        METHODINTERCEPTOR_INTERCEPT,
        METHODINTERCEPTOR_INTERCEPT,
        ICLASSLISTENER_ON_BEFORE_CLASS,
        ICONFIGURATIONLISTENER_BEFORE_CONFIGURATION,
        IINVOKEDMETHODLISTENER_BEFORE_INVOCATION,
        IINVOKEDMETHODLISTENER_AFTER_INVOCATION,
        ICONFIGURATIONLISTENER_ON_CONFIGURATION_FAILURE,
        ICONFIGURATIONLISTENER_BEFORE_CONFIGURATION,
        ICONFIGURATIONLISTENER_ON_CONFIGURATION_SKIP,
        ITESTLISTENER_ON_START_TEST_METHOD,
        ITESTLISTENER_ON_TEST_SKIPPED_TEST_METHOD,
        ICLASSLISTENER_ON_AFTER_CLASS,
        IEXECUTION_VISUALISER_CONSUME_DOT_DEFINITION,
        ITESTLISTENER_ON_FINISH_TEST_TAG,
        ISUITELISTENER_ON_FINISH,
        IREPORTER_GENERATE_REPORT,
        IEXECUTIONLISTENER_ON_EXECUTION_FINISH
    );
    runTest(expected, SimpleTestClassWithFailedAndSkippedConfigAndSkippedTestMethod.class);
  }

Please also see the following test class which uses the same test classes and scenarios to validate calls to the IInvokedMethodListener on its own: package test.listeners;

import org.assertj.core.api.Assertions;
import org.testng.*;
import org.testng.annotations.Test;
import test.SimpleBaseTest;
import test.listeners.ordering.*;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
 * This test verifies that IInvokedMethodListeners before/after calls
 * are invoked if and only if a configuration/test methods are invoked
 */
public class IInvokedMethodListenerTest extends SimpleBaseTest {

    @Test(description = "Test class has only 1 passed config and test method")
    public void testClassHasOnlyPassedConfigAndTestMethod() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithPassConfigAndMethod.beforeClass",
                "test.listeners.ordering.SimpleTestClassWithPassConfigAndMethod.testWillPass"
        );
        runTest(expected, SimpleTestClassWithPassConfigAndMethod.class);
    }

    @Test(description = "Test class has only 1 failed config and skipped test method")
    public void testClassHasOnlyFailedConfigAndSkippedTestMethod() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithFailedConfigAndSkippedTestMethod.beforeClass"
        );
        runTest(expected, SimpleTestClassWithFailedConfigAndSkippedTestMethod.class);
    }

    @Test(description = "Test class has only 1 failed config, 1 skipped config and skipped test method")
    public void testClassHasOnlyFailedAndSkippedConfigAndSkippedTestMethod() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithFailedAndSkippedConfigAndSkippedTestMethod.beforeClass"
        );

        runTest(expected, SimpleTestClassWithFailedAndSkippedConfigAndSkippedTestMethod.class);
    }

    @Test(description = "Test class has only 1 passed test method powered by a data provider")
    public void testClassHasOnlyDataDrivenPassedMethod() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithDataDrivenPassMethod.testWillPass",
                "test.listeners.ordering.SimpleTestClassWithDataDrivenPassMethod.testWillPass"
        );
        runTest(expected, SimpleTestClassWithDataDrivenPassMethod.class);
    }

    @Test(description = "Test class has only 1 data provider driven test method with 1 passed and 1 failed iteration.")
    public void testClassHasOnlyDataDrivenMethodWithPassedAndFailedIteration() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithDataDrivenMethodPassAndFailedIterations.testWillPass",
                "test.listeners.ordering.SimpleTestClassWithDataDrivenMethodPassAndFailedIterations.testWillPass"
        );
        runTest(expected, SimpleTestClassWithDataDrivenMethodPassAndFailedIterations.class);
    }

    //Regular test methods without any configuration methods.

    @Test(description = "Test class has only 1 passed test method")
    public void testClassHasOnlyPassedMethod() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithPassMethod.testWillPass"
        );
        runTest(expected, SimpleTestClassWithPassMethod.class);
    }

    @Test(description = "Test class has only 1 failed test method")
    public void testClassHasOnlyFailedMethod() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithFailedMethod.testWillFail"
        );
        runTest(expected, SimpleTestClassWithFailedMethod.class);
    }

    @Test(description = "Test class has only 1 failed test method and uses invocation counts")
    public void testClassHasOnlyFailedMethodMultipleInvocations() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithFailedMethodMultipleInvocations.testWillFail",
                "test.listeners.ordering.SimpleTestClassWithFailedMethodMultipleInvocations.testWillFail"
        );
        runTest(expected, SimpleTestClassWithFailedMethodMultipleInvocations.class);
    }

    @Test(description = "Test class has only 1 failed test method and uses invocation counts but configured"
            + "to skip failed invocations")
    public void testClassHasOnlyFailedMethodMultipleInvocationsAndSkipFailedInvocations() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithFailedMethodMultipleInvocations.testWillFail"
        );
        runTest(expected, SimpleTestClassWithFailedMethodMultipleInvocations.class, true);
    }

    @Test(description = "Test class has passed/failed/skipped test methods")
    public void testClassHasPassedFailedSkippedMethods() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithPassFailMethods.testWillFail",
                "test.listeners.ordering.SimpleTestClassWithPassFailMethods.testWillPass"
        );
        runTest(expected, SimpleTestClassWithPassFailMethods.class);
    }

    @Test(description = "Test class has only 1 failed test method and is powered by factory")
    public void testClassHasOnlyFailedMethodPoweredByFactory() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassPoweredByFactoryWithFailedMethod.testWillFail",
                "test.listeners.ordering.SimpleTestClassPoweredByFactoryWithFailedMethod.testWillFail"
        );
        runTest(expected, SimpleTestClassPoweredByFactoryWithFailedMethod.class);
    }

    @Test(description = "Test class has only 1 failed test method with retry analyzer")
    public void testClassHasOnlyFailedMethodWithRetryMechanism() {
        List<String> expected = createExpectedList(
                "test.listeners.ordering.SimpleTestClassWithFailedMethodHasRetryAnalyzer.testWillFail",
                "test.listeners.ordering.SimpleTestClassWithFailedMethodHasRetryAnalyzer.testWillFail"
        );
        runTest(expected, SimpleTestClassWithFailedMethodHasRetryAnalyzer.class, true);
    }

    private List<String> createExpectedList(String... expectedMethods) {
        return Arrays.stream(expectedMethods)
                .flatMap(expectedMethod -> Stream.of(before(expectedMethod), after(expectedMethod)))
                .collect(Collectors.toList());
    }

    private static void runTest(List<String> expected, Class<?> clazz) {
        runTest(expected, clazz, false);
    }

    private static void runTest(List<String> expected, Class<?> clazz, boolean skipInvocationCount) {
        TestNG testng = create(clazz);
        testng.setSkipFailedInvocationCounts(skipInvocationCount);

        InvokedMethodListener listener = new InvokedMethodListener();
        testng.addListener(listener);
        testng.alwaysRunListeners(false);
        testng.run();

        Assertions.assertThat(listener.getListenerCalls()).containsExactlyElementsOf(expected);
    }

    private static String before(String methodName) {
        return "BEFORE_INVOCATION " + methodName;
    }

    private static String after(String methodName) {
        return "AFTER_INVOCATION " + methodName;
    }

    static class InvokedMethodListener implements IInvokedMethodListener {

        private List<String> listenerCalls = new ArrayList<>();

        @Override
        public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
            listenerCalls.add(before(method.getTestMethod().getQualifiedName()));
        }

        @Override
        public void afterInvocation(IInvokedMethod method, ITestResult testResult) {
            listenerCalls.add(after(method.getTestMethod().getQualifiedName()));
        }

        List<String> getListenerCalls() {
            return listenerCalls;
        }
    }

}

testng-team / testng