org.testng.reporters.XMLReporter doesnt support multiple <test> with same name in same suite

[nullin]

If I run the following suite:

<suite name="QS AtomFeed tests"> <test name="QS:AtomFeed:Tests" preserve-order="true"> <classes> <class name="testng.Test1"/> <class name="testng.Test2"/> <class name="testng.Test3"/> </classes> </test> <test name="QS:AtomFeed:Tests" preserve-order="true"> <classes> <class name="testng.Test3"/> </classes> </test> </suite>

I see the following in the xml report:

<suite name="QS AtomFeed tests" duration-ms="2" started-at="2011-05-25T12:39:33Z" finished-at="2011-05-25T12:39:33Z"> <groups> </groups> <test name="QS:AtomFeed:Tests" duration-ms="2" started-at="2011-05-25T12:39:33Z" finished-at="2011-05-25T12:39:33Z"> <class name="testng.Test3"> <test-method status="PASS" signature="test1()" name="test1" duration-ms="1" started-at="2011-05-25T12:39:33Z" finished-at="2011-05-25T12:39:33Z"> </test-method> </class> </test> </suite>

As you can see results for only one of the is reported. The TestNG plugin for eclipse correctly handles this and merges the results for <test>s with same name.

[nullin]

The problem is actually not in org.testng.reporters.XMLReporter but in SuiteRunner.runTest():

  private void runTest(TestRunner tr) {
    tr.run();

    ISuiteResult sr = new SuiteResult(m_suite, tr);
    m_suiteResults.put(tr.getName(), sr);
  }

The result is of TestRunner execution is added to m_suiteResults using test name as key, which is incorrect because TestNG doesn't enforce against having multiple tests with same name within a single suite.

Solution:

1. When reading the XML suite, only create a single XmlTest for multiple tests with same name? This might not work because user might specify different set of parameters with different tests.
2. Start enforcing that tests within same suite will have unique names. This has potential of breaking existing tests for many people.
3. Append random character or something like "_1" at end of test name if m_suiteResults.get(testName) returns non-null value. Not sure how this will go down with folks when it comes down to comparing input xmls and outputs from reporters and listeners
4. Implement something where TestRunners can be merged ???

What do you suggest Cedric?

[cbeust]

I also thought about all these options and neither is really straightforward. The best place to do this is as far up stream as possible, i.e. 1), but the XML parsing needs to be updated in several places (because of all the m_current*\ fields I am maintaining because of SAX).

[nullin]

unless you see a real need for having multiple test tags with same name, we should maybe break the backward compatibility in this case by going with option 2. It's the cleanest and simplest solution. I don't see a lot of people running into this issue, otherwise it would have been reported earlier.

Otherwise, If you do go with option 1, won't you run into the issue with different set of parameters defined for different tests. How do you think that would be handled?

[cbeust]

Well, it's already undefined behavior so we wouldn't make things much worse, but yes, maybe we should just catch this in ContentTestNGHandler and abort if we detect two tags with the same name.

[nullin]

ok. I'll make the change.

[cbeust]

Actually, a correction: we should catch this slightly higher than at XML parsing so that the fix will catch this both in XML and YAML documents.

[nullin]

Should be fixed with https://github.com/nullin/testng/commit/7f570c9e60aad5e8b77d2864f14d2db6f24faf7e.

Opening a pull request now.

[JIGD]

I think this issue should be closed too.