testng-team / testng

TestNG testing framework
https://testng.org
Apache License 2.0
1.99k stars 1.02k forks source link

@Ignore doesn't work when used on multiple child classes, and parent has @Test methods #2868

Open h-j-j opened 1 year ago

h-j-j commented 1 year ago

TestNG Version

Note: only the latest version is supported 7.5

Expected behavior

Actual behavior

Is the issue reproducible on runner?

Test case sample

Please, share the test case (as small as possible) which shows the issue Base class


import org.testng.annotations.Test;

public class ParentClassTest { @Test public void testc() { hook(); }

protected void hook() {};

}


First child class

import org.testng.Reporter;

public class ChildClassTest extends ParentClassTest { @Override protected void hook() { Reporter.log("ChildClassTest#hook()"); } }


Second child class

import org.testng.Reporter; import org.testng.annotations.Ignore;

@Ignore public class SecondChildClassTest extends ParentClassTest { @Override protected void hook() { Reporter.log("SecondChildClassTest#hook()"); } }



### Contribution guidelines

Incase you plan to raise a pull request to fix this issue, please make sure you refer our [Contributing](.github/CONTRIBUTING.md) section for detailed set of steps.
juherr commented 1 year ago

The implementation of ignore is basic with limitations. I recommend you to implement your own logic that fits with your needs.

ghost commented 1 year ago

I've bumped into this same issue too - alphabetical ordering of which tests might be ignored seems highly unintuitive to me!

atassaad commented 1 year ago

@Ignore does not work either for methods on implemented interfaces. This was not the behavior of 6.9.9. That is: For this interface:

import org.testng.annotations.Test;

public interface MyInterface {

    @Ignore
    default void ignoreMePlease()
    {
            System.out.println("do the hussle, but ignore it!");
         }
}

implemented by the test class

import org.testng.annotations.Test;

@Test
public class ParentClassTest implements MyInterface {
    @Test
    public void testc() {
        hook();
    }

    protected void hook() {};
}

TestNG will attempt to run every default method on every implemented interface, which leads to an explosion that practically disables it in the presence of any meaningful interface(s).

juherr commented 1 year ago

@atassaad I think your sample is not complete: The method from the interface is not supposed to be discovered as a test method, except if a class is annotated with @Test somewhere in the inheritance. And only test methods can be ignored.

atassaad commented 1 year ago

@juherr Good catch, I will edit it accordingly, but we are still in a bind: 6.9.9 never considered default methods from interfaces as Test methods when the implementing class was marked as Test. And currently there is no mechanism (barring a transformer I guess) to recover the behavior of 6.9.9 without modifying thousands of test classes.

juherr commented 1 year ago

@atassaad please share a full sample and a description of the actual and expected result.

atassaad commented 1 year ago

@juherr This sample is now reflective of our entire codebase (test classes that implement interfaces that "plug tests" into a cloud environment. I have removed the @Ignore annotation from the interface method (as it is invalid).

public interface MyInterface {

    default void ignoreMePlease()
    {
            System.out.println("do the hussle, but ignore it!");
         }
}

implemented by the test class

import org.testng.annotations.Test;

@Test
public class ParentClassTest implements MyInterface {
    @Test
    public void testc() {
        hook();
    }

    protected void hook() {};
}

Here is the behavior of this code under TestNG 6.9.9: only the method ParentClassTest.testc() is recognized as a test method, along with any public methods on the class, MyInterface.ignoreMePlease(), and any other default methods on an interface implemented by ParentClassTest, that are not explicitly marked as @Test in the implementing class, are ignored without any annotation. (This is the behavior of 6.9.9)

juherr commented 1 year ago

@atassaad I don't catch why you set @Test on the class and expect that a method of the inheritance will be ignored. 6.9.9 is working like that just because it didn't support Java 8 new features when it was released. But go ahead and propose a pull request that make the behavior configurable. (or better but harder, a hook like annotation transformer that allow to change a test method configuration and not just an annotation).

As a workaround, what if you annotate the default method with @Test, but disabled?

atassaad commented 1 year ago

@juherr just for completeness: the use case is that we have thousands of test classes annotated for grouping purposes, whereas the class methods are annotated for enable/disable of individual tests and for method dependencies. We used 6.x and previous versions without realizing that the behavior (ignoring default methods of implemented interfaces) was merely a side effect. I will attempt to mark the default methods as Test+@Ignored, but wonder if TestNG sifting through all the extra, spurious methods that were previously ignored will make the initialization drag as it is doing currently (with ample RAM, where we were previously running one hello world test, we now scroll through about 35K spurious methods, which kills eclipse reliably)

krmahadevan commented 1 year ago

@atassaad - Here's a sample annotation transformer that can help you filter out all the default methods that ended up getting marked as test methods due to the @Test annotation on the class.

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

public class MethodPruningTransformer implements IInvokedMethodListener {

  @Override
  public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
    boolean isDefault = method.getTestMethod().getConstructorOrMethod().getMethod().isDefault();
    String msg = String.format("Is %s() a default Method ? %s", method.getTestMethod().getQualifiedName(),
        isDefault);
    System.err.println(msg);
    if (isDefault) {
      throw new SkipException("Skipping default method");
    }
  }
}
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;
import org.testng.IReporter;
import org.testng.IResultMap;
import org.testng.ISuite;
import org.testng.ISuiteResult;
import org.testng.ITestContext;
import org.testng.ITestNGMethod;
import org.testng.xml.XmlSuite;

public class MethodLogger implements IReporter {

  private List<String> methods;

  public List<String> getMethods() {
    return methods;
  }

  @Override
  public void generateReport(List<XmlSuite> xmlSuites, List<ISuite> suites,
      String outputDirectory) {
    methods = suites.stream().flatMap(each -> each.getResults().values().stream())
        .map(ISuiteResult::getTestContext)
        .flatMap(each -> extract(each).stream())
        .map(ITestNGMethod::getQualifiedName)
        .collect(Collectors.toList());
  }

  private Collection<ITestNGMethod> extract(ITestContext context) {
    return new ArrayList<>(extract(context.getPassedTests()));
  }

  private Collection<ITestNGMethod> extract(IResultMap map) {
    return map.getAllMethods();
  }
}

Test-case looks like below

import static org.assertj.core.api.Assertions.assertThat;

import org.testng.TestNG;

public class TestCase {

  public static void main(String[] args) {
    TestNG testng = new TestNG();
    MethodLogger logger = new MethodLogger();
    MethodPruningTransformer transformer = new MethodPruningTransformer();
    testng.addListener(transformer);
    testng.addListener(logger);
    testng.setTestClasses(new Class<?>[]{ParentClassTest.class});
    testng.run();
    assertThat(logger.getMethods())
        .containsOnly("com.rationaleemotions.issue2868.ParentClassTest.testc");
  }
}
juherr commented 1 year ago

@krmahadevan Good catch, your workaround is clearly better! :+1: