Dont honour params specified in suite-file tag

Closes #581 TestNG does not have any logic that would honour parameter tags specified within suite-files tag. But due to a parsing bug, we end up reading parameters that were specified within `<suite-file>` tag. This tag by definition is NOT meant to have any child tags inside of it. Fixed this discrepancy by adding a warning when this anomaly is detected and skipping of reading the `<parameter>` tag inside it as if it were specified within `<suite>` tag.
testng-team · Jun 7, 2023 · 60e9946 · 60e9946
1 parent d6767b5
commit 60e9946
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 0 deletions.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -1,5 +1,6 @@
 Current
 
+Fixed: GITHUB-581: Parameters of nested test suites are overridden(Krishnan Mahadevan)
 Fixed: GITHUB-727 : Fixing data races (Krishnan Mahadevan)
 Fixed: GITHUB-2913: Maps containing nulls can be incorrectly considered equal (Alex Heneveld)
 

diff --git a/testng-core/src/main/java/org/testng/xml/TestNGContentHandler.java b/testng-core/src/main/java/org/testng/xml/TestNGContentHandler.java
@@ -101,6 +101,7 @@ enum Location {
   }
 
   private final Stack<Location> m_locations = new Stack<>();
+  private boolean isSuiteFileTag = false;
 
   private XmlClass m_currentClass = null;
   private ArrayList<XmlInclude> m_currentIncludedMethods = null;
@@ -196,9 +197,11 @@ private void xmlSuiteFile(boolean start, Attributes attributes) {
       String path = attributes.getValue("path");
       pushLocation(Location.SUITE);
       m_suiteFiles.add(path);
+      isSuiteFileTag = true;
     } else {
       m_currentSuite.setSuiteFiles(m_suiteFiles);
       popLocation();
+      isSuiteFileTag = false;
     }
   }
 
@@ -611,6 +614,13 @@ public void startElement(String uri, String localName, String qName, Attributes
     } else if ("exclude".equals(qName)) {
       xmlExclude(true, attributes);
     } else if ("parameter".equals(qName)) {
+      if (isSuiteFileTag) {
+        // do-not process a <parameter> tag when it is specified inside <suite-files>
+        Logger.getLogger(getClass())
+            .warn(
+                "Ignoring the <parameter> tag because it is specified inside a <suite-file> tag.");
+        return;
+      }
       String value = expandValue(attributes.getValue("value"));
       Location location = m_locations.peek();
       switch (location) {

diff --git a/testng-core/src/test/java/test/parameters/ParameterTest.java b/testng-core/src/test/java/test/parameters/ParameterTest.java
@@ -2,9 +2,12 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import org.testng.ISuite;
+import org.testng.ISuiteListener;
 import org.testng.ITestResult;
 import org.testng.TestListenerAdapter;
 import org.testng.TestNG;
@@ -131,4 +134,21 @@ public void testNativeInjectionAndParamsInjection() {
     testng.run();
     assertThat(listener.getPassedTests().isEmpty()).isFalse();
   }
+
+  @Test(description = "GITHUB-581")
+  public void ensureParametersSpecifiedInsideSuiteFilesTagAreIgnored() {
+    TestNG testng = create();
+    testng.setTestSuites(
+        Collections.singletonList("src/test/resources/parametertest/issue_581/parent_suite.xml"));
+    Map<String, Map<String, String>> actualParameters = new HashMap<>();
+    testng.addListener(
+        new ISuiteListener() {
+          @Override
+          public void onStart(ISuite suite) {
+            actualParameters.put(suite.getName(), suite.getXmlSuite().getAllParameters());
+          }
+        });
+    testng.run();
+    actualParameters.values().forEach(each -> assertThat(each).isEmpty());
+  }
 }
diff --git a/testng-core/src/test/java/test/parameters/issue581/TestClassSample.java b/testng-core/src/test/java/test/parameters/issue581/TestClassSample.java
@@ -0,0 +1,12 @@
+package test.parameters.issue581;
+
+import org.testng.annotations.Optional;
+import org.testng.annotations.Parameters;
+import org.testng.annotations.Test;
+
+public class TestClassSample {
+
+  @Parameters(value = "url")
+  @Test
+  public void test(@Optional String ignored) {}
+}
diff --git a/testng-core/src/test/resources/parametertest/issue_581/parent_suite.xml b/testng-core/src/test/resources/parametertest/issue_581/parent_suite.xml
@@ -0,0 +1,11 @@
+<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
+<suite name="parent_suite">
+  <suite-files>
+    <suite-file path="suite_one.xml">
+      <parameter name="url" value="http://www.google.com"/>
+    </suite-file>
+    <suite-file path="suite_two.xml">
+      <parameter name="url" value="http://www.bing.com"/>
+    </suite-file>
+  </suite-files>
+</suite>
diff --git a/testng-core/src/test/resources/parametertest/issue_581/suite_one.xml b/testng-core/src/test/resources/parametertest/issue_581/suite_one.xml
@@ -0,0 +1,8 @@
+<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
+<suite name="suite_one">
+  <test name="test_one">
+    <classes>
+      <class name="test.parameters.issue581.TestClassSample"/>
+    </classes>
+  </test>
+</suite>
diff --git a/testng-core/src/test/resources/parametertest/issue_581/suite_two.xml b/testng-core/src/test/resources/parametertest/issue_581/suite_two.xml
@@ -0,0 +1,8 @@
+<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd" >
+<suite name="suite_two">
+  <test name="test_two">
+    <classes>
+      <class name="test.parameters.issue581.TestClassSample"/>
+    </classes>
+  </test>
+</suite>