Test annotated with @Test(enabled=false) and @Ignore should be reported as skipped

[marcphilipp]

Currently such tests are not reported at all.

Related issue: gradle/gradle#1403

[marcphilipp]

I briefly looked into this. It seems methods with @Test(enabled=false) are available as part of ITestClass.getTestMethods but @Ignored ones are not.

[marcphilipp]

@juherr @krmahadevan Is that difference on purpose or accidental?

[juherr]

@Ignore is supposed to disable @Test thanks to an internal listener.
The only expected behavior is that @Ignore on a class will disable enabled @Test on methods but @Test(enabled=false) won't.

About reporting, I don't know what is the best option but not reported because disabled looks legit.

[marcphilipp]

When I'm in an implementation of IClassListener.onBeforeClass and call testClass.getTestMethods(), the returned array will contain an instance of ITestNGMethod for methods annotated with @Test(enabled=false) but not for methods annotated with @Ignore. Hence, we could report the former as skipped in the TestEngine but not the latter which would be inconsistent. I'm wondering why @Ignore methods are not accessible via testClass.getTestMethods(), though. 🤔

@krmahadevan Do you know?

[krmahadevan]

@marcphilipp - @Ignore on a class level is basically equivalent to having @Test(enabled=false) on all the test Methods.

That is why @Ignore at class level and @Test(enabled=false) for all test methods in the test class behave the same way i.e., testClass.getTestMethods() returns null.

The below example should demonstrate the behavior.

import org.testng.Assert;
import org.testng.IClassListener;
import org.testng.ITestClass;
import org.testng.ITestNGMethod;
import org.testng.TestNG;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Ignore;
import org.testng.annotations.Test;

public class LocalTestRunner {

  @Test(dataProvider = "testData")
  public void demonstrateBehavior(Class<?> clazz, boolean isNull) {
    TestNG testng = new TestNG();
    testng.setTestClasses(new Class[]{clazz});
    testng.setVerbose(2);
    LocalClassListener listener = new LocalClassListener();
    testng.addListener(listener);
    testng.run();
    Assert.assertEquals(listener.getTestMethods() == null, isNull);
  }

  @DataProvider(name = "testData")
  public Object[][] getData() {
    return new Object[][]{
        {TestClassWithAllTestsExplicitlyDisabled.class, true},
        {TestClassUsingIgnoreToDisableAllMethods.class, true},
        {TestClassWithMixedCombinations.class, false}
    };
  }

  public static class TestClassWithAllTestsExplicitlyDisabled {

    @Test(enabled = false)
    public void test1() {
    }

    @Test(enabled = false)
    public void test2() {
    }
  }

  @Ignore
  public static class TestClassUsingIgnoreToDisableAllMethods {

    @Test
    public void test1() {
    }

    @Test
    public void test2() {
    }
  }

  public static class TestClassWithMixedCombinations {

    @Test
    public void runnableTest() {
    }

    @Test(enabled = false)
    public void disabledTest() {
    }

    @Ignore
    @Test
    public void ignoredTest() {
    }
  }

  public static class LocalClassListener implements IClassListener {

    private ITestNGMethod[] testMethods;

    @Override
    public void onBeforeClass(ITestClass testClass) {
      testMethods = testClass.getTestMethods();
    }

    public ITestNGMethod[] getTestMethods() {
      return testMethods;
    }
  }
}

[juherr]

All the @Ignore logic is based on a listener.

So, it sounds like an issue when listeners are involved.

We should investigate and fix it on TestNG side.

[krmahadevan]

@juherr - Currently Testng is behaving the same way when

• @Ignore used on class or
• Test class has all test methods as disabled

That was what my sample was trying to show.

Since in the second case ( enabled=false) there is no listener involvement I didn't quite get you.

[marcphilipp]

@krmahadevan Thanks for the sample!

Given the status quo, I don't see how the engine could reliably report tests annotated with @Test(enabled=false) or @Ignore as skipped since they're only available from onBeforeClass in the TestClassWithMixedCombinations case above (and even then only the methods with @Test(enabled=false) not the ones with @Ignore).

[juherr]

That was what my sample was trying to show.

@krmahadevan Right! I didn't realize your sample already highlights similar behavior between @Ignore and @Test(enabled=false).

only available from onBeforeClass in the TestClassWithMixedCombinations case above (and even then only the methods with @Test(enabled=false) not the ones with @Ignore)

but it looks like some not-expected differences exist.

I don't see how the engine could reliably report tests annotated with @Test(enabled=false) or @Ignore as skipped

@marcphilipp Like https://github.com/cbeust/testng/blob/master/testng-core/src/main/java/org/testng/internal/annotations/IgnoreListener.java I think you can collect all methods into an IAnnotationTransformer and mark the not-run methods as skipped at the end of the run.

[marcphilipp]

Thanks for the suggestion! That could be done but it would mean somewhat duplicating TestNG's internals (i.e. what it considers a test method etc.) since the code currently gets most of its info from ITestNGMethod.

[juherr]

@krmahadevan @marcphilipp
If it is possible, WDYT if we add an internal listener which notifies every discovered ITestNGMethod?

[krmahadevan]

@marcphilipp - Can you please tell me if this is just a reporting requirement ? If yes, then can we consider building something on top of the IReporter interface that I believe will report skipped tests as well. Would that work here ?

[marcphilipp]

Yes, this is for reporting. Other testing frameworks such as JUnit and Spock report skipped tests including a reason so there's at least some indication that not all tests were executed and why. If you're interested, we can set up a call and I can walk you through the current engine implementation so you might get a better idea what would be necessary.

[krmahadevan]

@marcphilipp - Sure. I am ok with a call this saturday ( July 17).

[marcphilipp]

@krmahadevan @juherr I've sent you a meeting invite for tomorrow via email. Please let me know if that works for you.

[krmahadevan]

@marcphilipp - I have accepted the invite.

[juherr]

I can't be present but @krmahadevan knows everything :)

[krmahadevan]

@marcphilipp - In the last call we discussed the below two discrepancies for which I was to get back to you.

1. When a test class contains a mixture of enabled, disabled (enabled=false) and ignored @Test methods, then the @Ignore annotated @Test methods are not visible in the org.testng.IClassListener implementation. Ignored Tests are not retrieved for a mixed test class (test with enabled, disabled and ignored test method) testng-team/testng#2613 is tracking this and I have fixed this as part of the PR Ignored tests to be available in listeners testng-team/testng#2614
2. Provide a listener capability within TestNG wherein disabled and ignored tests can be retrieved from TestNG. This is still pending.

[krmahadevan]

@marcphilipp

On (2), Once my above mentioned PR gets merged, then you should be able to use the org.testng.ITestListener interface and then get hold of all the tests that have been ignored|disabled.

The below sample demonstrates this (For this sample to work, the above mentioned PR has to be merged)

import java.util.List;
import java.util.stream.Collectors;
import org.assertj.core.api.Assertions;
import org.testng.ITestContext;
import org.testng.ITestListener;
import org.testng.TestNG;
import org.testng.annotations.Ignore;
import org.testng.annotations.Test;

public class ExampleClass {

  @Test
  public void sampleTestCase() {
    TestNG testng = new TestNG();
    testng.setTestClasses(new Class[]{ExampleTest.class});
    ExampleListener l = new ExampleListener();
    testng.addListener(l);
    testng.run();
    String[] expected = new String[] {"disabledTest", "ignoredTest"};
    Assertions.assertThat(l.getMethods()).containsExactlyInAnyOrder(expected);
  }

  public static class ExampleTest {

    @Test(enabled = false)
    public void disabledTest() {
    }

    @Test
    @Ignore
    public void ignoredTest() {}
  }

  public static class ExampleListener implements ITestListener {

    private List<String> methods;

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

    @Override
    public void onStart(ITestContext context) {
      methods = context.getExcludedMethods().stream()
          .map(each -> each.getConstructorOrMethod().getMethod().getName())
          .collect(Collectors.toList());
    }
  }
}

[marcphilipp]

Thanks for the update! Would ITestContext.getExcludedMethods() also return all methods of classes that have @Test(enabled = false) or @Ignore on the class declaration?

[marcphilipp]

@krmahadevan I did a quick check and it seems only the mixed method use case is supported this way. In that sense ITestContext.getExcludedMethods() is equivalent to checking ITestClass.getTestMethods() for excluded, isn't it?

[krmahadevan]

@marcphilipp - I didnt try out the scenario you mentioned. Will check it out and get back. Please give me sometime.

[krmahadevan]

@marcphilipp

Here's what I have so far:

The example test classes that I have

import org.testng.annotations.Test;

@Test(enabled = false)
public class ClassWithOnlyDisabledTestsAtClassLevel {

  public void disabledTest() {}

  public void ignoredTest() {}
}

import org.testng.annotations.Test;

public class ClassWithOnlyDisabledTestsAtMethodLevel {

  @Test(enabled = false)
  public void disabledTest() {}

  @Test(enabled = false)
  public void ignoredTest() {}
}

import org.testng.annotations.Ignore;
import org.testng.annotations.Test;

@Test
@Ignore
public class ClassWithOnlyIgnoredTestsAtClassLevel {

  public void disabledTest() {}

  public void ignoredTest() {}
}

import org.testng.annotations.Ignore;
import org.testng.annotations.Test;

public class ClassWithOnlyIgnoredTestsAtMethodLevel {

  @Test
  @Ignore
  public void disabledTest() {}

  @Test
  @Ignore
  public void ignoredTest() {}
}

The listener that I am using

import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import org.testng.IClassListener;
import org.testng.ITestClass;
import org.testng.ITestContext;
import org.testng.ITestListener;

public class ExampleListener implements IClassListener, ITestListener {

  private List<String> methods;

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

  @Override
  public void onBeforeClass(ITestClass testClass) {
    methods = Arrays.stream(testClass.getTestMethods())
        .map(each -> each.getConstructorOrMethod().getMethod().getName())
        .collect(Collectors.toList());
  }

  @Override
  public void onFinish(ITestContext context) {
    List<String> excluded = context.getExcludedMethods().stream()
        .map(each -> each.getConstructorOrMethod().getMethod().getName())
        .collect(Collectors.toList());
    if (methods == null || methods.isEmpty()) {
      methods = excluded;
    } else {
      methods.addAll(excluded);
    }
  }
}

And here's the test class that works for all the above 4 combinations.

import org.assertj.core.api.Assertions;
import org.testng.TestNG;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

public class ExampleClass {

  @Test(dataProvider = "getData")
  public void sampleTestCase(Class<?> cls) {
    TestNG testng = new TestNG();
    testng.setTestClasses(new Class[]{cls});
    ExampleListener l = new ExampleListener();
    testng.addListener(l);
    testng.run();
    String[] expected = new String[]{"disabledTest", "ignoredTest"};
    Assertions.assertThat(l.getMethods()).containsExactlyInAnyOrder(expected);
  }

  @DataProvider
  public Object[][] getData() {
    return new Object[][]{
        {ClassWithOnlyDisabledTestsAtClassLevel.class},
        {ClassWithOnlyDisabledTestsAtMethodLevel.class},
        {ClassWithOnlyIgnoredTestsAtClassLevel.class},
        {ClassWithOnlyIgnoredTestsAtMethodLevel.class}
    };
  }
}

[krmahadevan]

@marcphilipp - Does the above shared listener example suffice for all the use cases ?

[marcphilipp]

Thanks for the example! I think that should be sufficient to track all disabled/ignored tests.

[bdrx312]

Being able to see in the report that tests were skipped because they were ignored or disabled would also be useful for us. It doesn't matter for us if the they are reported as skipped versus a new category like ignored or disabled; what is important is that we know that some of our tests were not run.