Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 21 additions & 16 deletions src/main/java/org/testng/TestNG.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
/**
* This class is the main entry point for running tests in the TestNG framework.
* Users can create their own TestNG object and invoke it in many different
* ways:
* ways
* <ul>
* <li>On an existing testng.xml
* <li>On a synthetic testng.xml, created entirely from Java
Expand Down Expand Up @@ -269,28 +269,33 @@ public void setXmlPathInJar(String xmlPathInJar) {
public void initializeSuitesAndJarFile() {
// The Eclipse plug-in (RemoteTestNG) might have invoked this method already
// so don't initialize suites twice.
System.out.println("ENTER");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I think this line is not expected ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New to Java and git :(, just fixed it up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:) You can try to squash all your commits into only one with git interactive rebase. It's for advanced git users but it's really useful.

if (m_isInitialized) {
return;
}

m_isInitialized = true;
if (m_suites.size() > 0) {
//to parse the suite files (<suite-file>), if any
for (XmlSuite s: m_suites) {
for (String suiteFile : s.getSuiteFiles()) {
Path rootPath = Paths.get(s.getFileName()).getParent();
try {
Collection<XmlSuite> childSuites = getParser(rootPath.resolve(suiteFile).normalize().toString()).parse();
for (XmlSuite cSuite : childSuites){
cSuite.setParentSuite(s);
s.getChildSuites().add(cSuite);
}
//to parse the suite files (<suite-file>), if any
for (XmlSuite s: m_suites) {
for (String suiteFile : s.getSuiteFiles()) {
Path suitePath = Paths.get(s.getFileName());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move suitePath and rootPath between the 2 for.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s.getFileName() can return null in surefire plugin suites. By putting it here we will avoid the surefire plugin case and avoid a null check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s.getFileName() can return null in surefire plugin suites

Are we agree it is a surefire issue? Do you have an example ?

Path rootPath = suitePath.getParent();
try {
Collection<XmlSuite> childSuites = getParser(rootPath.resolve(suiteFile).normalize().toString()).parse();
for (XmlSuite cSuite : childSuites){
Path childSuite = Paths.get(cSuite.getFileName()).normalize();
Path parentSuite = Paths.get(suiteFile).normalize();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the difference between Paths.get(suiteFile).normalize() and rootPath.resolve(suiteFile).normalize()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paths.get(suiteFile).normalize() will get it from the project dir and rootPath.resolve(suiteFile).normalize() will first append the suiteFIle path and the normalize will remove the extra path identifiers. For ex, some users may write .\testng.xml and some may write \tesng.xml.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think \testng.xml or /testng.xml is not legal.

Could you provide an example where Paths.get(suiteFile).normalize() and ootPath.resolve(suiteFile).normalize() are not equal?

if(!childSuite.getFileName().equals(parentSuite.getFileName()))
{
cSuite.setParentSuite(s);
s.getChildSuites().add(cSuite);
}
}
} catch (ParserConfigurationException | IOException | SAXException e) {
e.printStackTrace(System.out);
}
}

}
}
return;
}

Expand Down Expand Up @@ -1012,13 +1017,13 @@ private void checkSuiteNames(List<XmlSuite> suites) {
private void checkSuiteNamesInternal(List<XmlSuite> suites, Set<String> names) {
for (XmlSuite suite : suites) {
final String name = suite.getName();

int count = 0;

String tmpName = name;
while (names.contains(tmpName)) {
tmpName = name + " (" + count++ + ")";
}

if (count > 0) {
suite.setName(tmpName);
names.add(tmpName);
Expand Down
29 changes: 29 additions & 0 deletions src/test/java/test/sanitycheck/CheckSuiteNamesTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,16 @@
import org.testng.xml.XmlSuite;
import org.testng.xml.XmlTest;
import org.xml.sax.SAXException;

import test.SimpleBaseTest;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import javax.xml.parsers.ParserConfigurationException;

public class CheckSuiteNamesTest extends SimpleBaseTest {
Expand Down Expand Up @@ -81,4 +88,26 @@ public void checkXmlSuiteAddition() throws ParserConfigurationException, SAXExce
tng.setXmlSuites(parser.parseToList());
tng.initializeSuitesAndJarFile();
}

@Test(description = "Verify that same suite is not added multiple times")
public void validateDuplicateSuiteAddition() throws ParserConfigurationException, SAXException, IOException
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test for #829 too? We should check that it is still possible to add the same child suite many times.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same child suite will point to same tests, same tests will fail the check in checkTestNames this method. That is the confusion I have. I am seeing the exception about same test name for two tests in one suite. Lets deal with this in a seperate issue. I will open one for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me pick this up as a part of next issue. I will log one after understanding the problem in details.

{
SuiteListner suiteListner = new SuiteListner();
TestNG tng = create();
String testngXmlPath = getPathToResource("sanitycheck/test-s-b.xml");
Parser parser = new Parser(testngXmlPath);
tng.setXmlSuites(parser.parseToList());
tng.addListener(suiteListner);
tng.run();

Set<String> allSuite = new HashSet<String>();
List<XmlSuite> ranSuites = suiteListner.getAllTestSuite();

Assert.assertEquals(ranSuites.size(), 3, "Correct number of suites ran");
for(XmlSuite suite : ranSuites)
{
Assert.assertTrue(allSuite.add(suite.getFileName()),
String.format("No duplicate of suite %s added", suite.getFileName()));
}
}
}
28 changes: 28 additions & 0 deletions src/test/java/test/sanitycheck/SuiteListner.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package test.sanitycheck;

import java.util.ArrayList;
import java.util.List;

import org.testng.ISuite;
import org.testng.ISuiteListener;
import org.testng.xml.XmlSuite;

public class SuiteListner implements ISuiteListener {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: SuiteListener


List<XmlSuite> allSuite = new ArrayList<XmlSuite>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer private & final when possible. But it's clearly my personal taste.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Private should be the right modifier. In Java its public by default. I think final is not necessary because we wont be changing the reference of the array list to any new array list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we wont be changing the reference of the array list to any new array list.

Agree, but final will warn you if you try ;)
But as I said, it's a personal taste, and I'm ok to not have it in a test class :)


@Override
public void onStart(ISuite suite) {
allSuite.add(suite.getXmlSuite());
}

@Override
public void onFinish(ISuite suite) {

}

public List<XmlSuite> getAllTestSuite(){
return allSuite;
}

}