TestListener is being lost when implenting both IClassListener and ITestListener

[jmoreira18]

TestNG Version 7.0.0 and 7.5

How to reproduce?

1. Create a TestListener that implements both IClassListener and ITestListener
2. Override the onTestStart method to skip if certain conditions are matched (ex: test contains a specific group)
3. Create a TestFile that uses TestListener as Listener
4. Add a test that meets the skip conditions under onTestStart is executed
5. Create a testng.xml file with 2 tags, calling the same test (also works if you call another test that should be skipped)

Expected behavior

All tests that match the skip conditions are skipped when tried to be executed

Actual behavior

Only the first one is skipped.

Is the issue reproducible on runner?

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

Test case sample

Here you have a small IntelliJ project containing two testng xmls, one has the listener explicit in the suite, the other one doesn't:
https://github.com/jmoreira18/TestNG-ClassListener-issue-demo

Provisional workarounds

1. If you explicit the TestListener on the testng.xml file, this issue doesn't happen
2. I haven't tried doing it, but I'm pretty sure if you divide the class (TestListener) and have 2 listeners (one implementing the IClassListener and one with the ITestListener) the issue won't happen. I tried removing the IClassListener from the TestListener, and this stopped happening

Code comments

I've been debugging the TestNG library in my project and this are a few things I found:

This continue (inside the if) is skipping the TestListener, even though it's both implementing the ITestListener and IClassListener

    // Instantiate all the listeners
    for (Class<? extends ITestNGListener> c : listenerClasses) {
      if (IClassListener.class.isAssignableFrom(c) && m_classListeners.containsKey(c)) {
        continue;
      }
      ITestNGListener listener = factory.createListener(c);

      addListener(listener);
    }

TestRunner.java code fragment from initListeners() function

I noticed this previous if -> continue, was added due to a fix, basically, trying not to add a ClassListeners two times.

I propose to remove that continue from there and put that check on addListener(ITestNGListener listener) instead:

    // Instantiate all the listeners
    for (Class<? extends ITestNGListener> c : listenerClasses) {
      ITestNGListener listener = factory.createListener(c);      
      addListener(listener);
    }

And inside addListener(ITestNGListener listener), all the adds should check that the listener is not already in each one of the lists and if not, add it (like it's done in IClassListener):

if (listener instanceof IClassListener) {
      IClassListener classListener = (IClassListener) listener;
      if (!m_classListeners.containsKey(classListener.getClass())) {
        m_classListeners.put(classListener.getClass(), classListener);
      }
    }

(Although this might not be needed, cause put basically replaces items if they already exist)

Thanks in advance!