testng-team · juherr · Jul 30, 2026 · Jul 30, 2026 · Jul 30, 2026 · Jul 30, 2026
@@ -1,4 +1,11 @@
 Current (7.13.0)
+Fixed: DTD validation of suite files was silently disabled: the SAX validation feature was probed under an "https" identifier that no parser recognizes, so setValidating(true) was never reached and violations went unreported. Validation is enabled again, with a new testng.xml.validation=off|warn|strict system property; the default "warn" reports violations without failing the run (Julien Herr)
+Fixed: XmlSuite.toXml() dropped the "description" attribute of <include>, so regenerating a suite (testng-failed.xml, for instance) lost method descriptions (Julien Herr)
+Fixed: XmlSuite.toXml() dropped a <selector-class> priority of -1 while the parser reads a missing priority as 0. Since a negative method-selector priority changes selector evaluation, serializing a suite and reading it back altered its behaviour (Julien Herr)
+Fixed: The doctype written by XmlSuite.toXml() advertised testng-1.0.dtd although the parser always resolves testng-1.1.dtd (Julien Herr)
+Fixed: XmlSuite.toXml() emitted two sibling <groups> elements for a suite that has suite-level groups, which the DTD allows only once, so TestNG's own output did not validate (Julien Herr)
+Fixed: DTD violations were discarded for suite files pointing at their own copy or a mirror of the DTD rather than at testng.org, so those suites were never validated (Julien Herr)
+New: Added round trip characterization tests covering every suite file of the test corpus, so that XML serialization can be refactored safely (Julien Herr)
 New: Added OpenRewrite to the build with a hand-picked recipe list (see rewrite.yml), and applied it to the main sources (Julien Herr)
 Fixed: Remove leftover dead JUnit code: the deprecated unused ConversionUtils and orphaned JUnit test samples, following the removal of JUnit execution support in 7.10.0 (Julien Herr)
 Update: Dependency refresh: Guice 6.0.0, JCommander 2.0, snakeyaml 2.6, slf4j-api 2.0.18. Guice 7 and JCommander 3 were skipped: they require jakarta.inject and Java 17 respectively

@@ -13,6 +13,7 @@ public final class RuntimeBehavior {
   private static final String TEST_CLASSPATH = "testng.test.classpath";
   private static final String SKIP_CALLER_CLS_LOADER = "skip.caller.clsLoader";
   public static final String TESTNG_USE_UNSECURED_URL = "testng.dtd.http";
+  public static final String XML_VALIDATION_MODE = "testng.xml.validation";
   public static final String SHOW_TESTNG_STACK_FRAMES = "testng.show.stack.frames";
   private static final String MEMORY_FRIENDLY_MODE = "testng.memory.friendly";
   public static final String STRICTLY_HONOUR_PARALLEL_MODE = "testng.strict.parallel";
@@ -52,6 +53,14 @@ public static boolean useSecuredUrlForDtd() {
     return !Boolean.getBoolean(TESTNG_USE_UNSECURED_URL);
   }
 
+  /**
+   * @return the raw value of {@value #XML_VALIDATION_MODE}, or {@code null} when unset. Interpreted
+   *     by {@code org.testng.xml.XmlValidationMode}.
+   */
+  public static String getXmlValidationMode() {
+    return System.getProperty(XML_VALIDATION_MODE);
+  }
+
   public static boolean isMemoryFriendlyMode() {
     return Boolean.parseBoolean(System.getProperty(MEMORY_FRIENDLY_MODE, "false"));
   }

@@ -15,8 +15,13 @@
  */
 class DefaultXmlWeaver implements IWeaveXml {
   // TODO: move constants to XmlSuite?
-  /** The name of the TestNG DTD. */
-  private static final String TESTNG_DTD = "testng-1.0.dtd";
+  /**
+   * The name of the TestNG DTD. Must stay in sync with {@code Parser.TESTNG_DTD}, which is the
+   * version the reader resolves from the classpath. The two had drifted apart, so the emitted
+   * doctype advertised a schema that was never the one used to read the file back. They cannot
+   * share a constant: {@code Parser} lives in testng-core, which depends on this module.
+   */
+  private static final String TESTNG_DTD = "testng-1.1.dtd";
 
   private static final String HTTPS_TESTNG_DTD_URL = "https://testng.org/" + TESTNG_DTD;
 
@@ -99,23 +104,28 @@ public String asXml(XmlSuite xmlSuite) {
         DEFAULT_ALLOW_RETURN_VALUES.toString());
     xsb.push("suite", p);
 
-    List<String> included = xmlSuite.getIncludedGroups();
-    List<String> excluded = xmlSuite.getExcludedGroups();
-    if (hasElements(included) || hasElements(excluded)) {
-      xsb.push("groups");
-      xsb.push("run");
-      for (String g : included) {
-        xsb.addEmptyElement("include", "name", g);
-      }
-      for (String g : excluded) {
-        xsb.addEmptyElement("exclude", "name", g);
-      }
-      xsb.pop("run");
-      xsb.pop("groups");
-    }
-
     if (xmlSuite.getGroups() != null) {
       xsb.getStringBuffer().append(xmlSuite.getGroups().toXml("  "));
+    } else {
+      // Only synthesize a <groups> block when the suite has no XmlGroups of its own to write.
+      // getIncludedGroups()/getExcludedGroups() read through to that same XmlGroups, so emitting
+      // both produced two sibling <groups> elements -- which the DTD allows only once, making
+      // TestNG's own output invalid. When the groups come from a parent suite there is nothing
+      // else to write, and flattening them here is what keeps a generated suite self-contained.
+      List<String> included = xmlSuite.getIncludedGroups();
+      List<String> excluded = xmlSuite.getExcludedGroups();
+      if (hasElements(included) || hasElements(excluded)) {
+        xsb.push("groups");
+        xsb.push("run");
+        for (String g : included) {
+          xsb.addEmptyElement("include", "name", g);
+        }
+        for (String g : excluded) {
+          xsb.addEmptyElement("exclude", "name", g);
+        }
+        xsb.pop("run");
+        xsb.pop("groups");
+      }
     }
 
     XmlUtils.dumpParameters(xsb, xmlSuite.getParameters());

@@ -75,6 +75,9 @@ public String toXml(String indent) {
     XMLStringBuffer xsb = new XMLStringBuffer(indent);
     Properties p = new Properties();
     p.setProperty("name", getName());
+    if (m_description != null) {
+      p.setProperty("description", m_description);
+    }
     List<Integer> invocationNumbers = getInvocationNumbers();
     if (invocationNumbers != null && !invocationNumbers.isEmpty()) {
       p.setProperty("invocation-numbers", XmlClass.listToString(invocationNumbers));

@@ -6,9 +6,13 @@
 
 /** This class describes the tag <code>&lt;method-selector&gt;</code> in testng.xml. */
 public class XmlMethodSelector {
+
+  /** The priority assumed when the {@code priority} attribute is absent from the suite file. */
+  public static final int DEFAULT_PRIORITY = 0;
+
   // Either this:
   private String m_className;
-  private int m_priority;
+  private int m_priority = DEFAULT_PRIORITY;
 
   // Or that:
   private XmlScript m_script;
@@ -56,7 +60,10 @@ public String toXml(String indent) {
     if (null != m_className) {
       Properties clsProp = new Properties();
       clsProp.setProperty("name", getClassName());
-      if (getPriority() != -1) {
+      // Omit the value the parser falls back to when the attribute is absent, so that a
+      // round trip is lossless. A negative priority is meaningful (see RunInfo#includeMethod)
+      // and must therefore be written out.
+      if (getPriority() != DEFAULT_PRIORITY) {
         clsProp.setProperty("priority", String.valueOf(getPriority()));
       }
       xsb.addEmptyElement("selector-class", clsProp);

@@ -117,8 +117,15 @@ enum Location {
   private final String m_fileName;
   private final boolean m_loadClasses;
   private boolean m_validate = false;
+  private boolean m_doctypeDeclared = false;
   private boolean m_hasWarn = false;
 
+  /**
+   * Resolved once per parse rather than per violation, so a malformed suite cannot re-read the
+   * system property -- and re-log the "unknown value" warning -- for every error it produces.
+   */
+  private final XmlValidationMode m_validationMode = XmlValidationMode.current();
+
   public TestNGContentHandler(String fileName, boolean loadClasses) {
     m_fileName = fileName;
     m_loadClasses = loadClasses;
@@ -128,6 +135,12 @@ public TestNGContentHandler(String fileName, boolean loadClasses) {
   public InputSource resolveEntity(String publicId, String systemId)
       throws SAXException, IOException {
 
+    // The document declares a doctype, whoever ends up providing it. Tracked separately from
+    // m_validate, which means "TestNG substituted its own copy of the DTD": gating error reporting
+    // on m_validate silently discarded every violation for suites pointing at their own DTD copy
+    // or at a corporate mirror.
+    m_doctypeDeclared = true;
+
     if (skipConsideringSystemId(systemId)) {
       m_validate = true;
       InputStream is = loadDtdUsingClassLoader();
@@ -495,10 +508,8 @@ public void xmlSelectorClass(boolean start, Attributes attributes) {
     if (start) {
       m_currentSelector.setName(attributes.getValue("name"));
       String priority = attributes.getValue("priority");
-      if (priority == null) {
-        priority = "0";
-      }
-      m_currentSelector.setPriority(Integer.parseInt(priority));
+      m_currentSelector.setPriority(
+          priority == null ? XmlMethodSelector.DEFAULT_PRIORITY : Integer.parseInt(priority));
     }
   }
 
@@ -781,8 +792,29 @@ public void endElement(String uri, String localName, String qName) {
 
   @Override
   public void error(SAXParseException e) throws SAXException {
-    if (m_validate) {
-      throw e;
+    if (!m_doctypeDeclared) {
+      // Without a doctype a validating parser only ever complains that no grammar was found, which
+      // would turn the existing "you should add a <!DOCTYPE>" hint into a hard failure.
+      return;
+    }
+    switch (m_validationMode) {
+      case STRICT:
+        throw e;
+      case WARN:
+        Logger.getLogger(TestNGContentHandler.class)
+            .warn(
+                "The suite file ["
+                    + m_fileName
+                    + "] does not conform to "
+                    + Parser.TESTNG_DTD
+                    + ": "
+                    + e.getMessage()
+                    + ". Run with [-D"
+                    + RuntimeBehavior.XML_VALIDATION_MODE
+                    + "=strict] to turn this into a failure.");
+        break;
+      case OFF:
+        break;
     }
   }
 

@@ -16,13 +16,21 @@ public abstract class XMLParser<T> implements IFileParser<T> {
 
   private static final SAXParser m_saxParser;
 
+  /**
+   * Whether the shared parser was built with DTD validation enabled. Decided once, because the
+   * parser itself is a singleton, and exposed so that tests can tell "validation is off in this
+   * JVM" apart from "this file is valid" instead of inferring it from a parse that does not fail.
+   */
+  private static final boolean validating;
+
   static {
     SAXParserFactory spf = loadSAXParserFactory();
 
-    if (supportsValidation(spf)) {
-      spf.setNamespaceAware(true);
-      spf.setValidating(true);
-    }
+    // Namespace awareness is deliberately left off: DTD validation does not need it, suite files
+    // are not namespaced, and turning it on would make an unbound prefix fatal and an xmlns
+    // attribute a validity error -- neither of which has anything to do with validating a suite.
+    validating = XmlValidationMode.current().isValidating() && supportsValidation(spf);
+    spf.setValidating(validating);
 
     SAXParser parser = null;
     try {
@@ -35,6 +43,11 @@ public abstract class XMLParser<T> implements IFileParser<T> {
 
   private static final AutoCloseableLock lock = new AutoCloseableLock();
 
+  /** Whether the shared parser validates suite files against the TestNG DTD. */
+  static boolean isValidating() {
+    return validating;
+  }
+
   public void parse(InputStream is, DefaultHandler dh) throws SAXException, IOException {
     try (AutoCloseableLock ignore = lock.lock()) {
       m_saxParser.parse(is, dh);
@@ -57,12 +70,20 @@ private static SAXParserFactory loadSAXParserFactory() {
     }
   }
 
-  /** Tests if the current <code>SAXParserFactory</code> supports DTD validation. */
+  /**
+   * Tests if the current <code>SAXParserFactory</code> supports DTD validation.
+   *
+   * <p>The feature name is a plain identifier, not a URL to dereference, so it keeps its historical
+   * <code>http</code> scheme. Probing it under <code>https</code> makes every conforming parser
+   * raise <code>SAXNotRecognizedException</code>, which silently disabled validation altogether.
+   */
   private static boolean supportsValidation(SAXParserFactory spf) {
     try {
-      spf.getFeature("https://xml.org/sax/features/validation");
+      spf.getFeature("http://xml.org/sax/features/validation");
       return true;
     } catch (Exception ex) {
+      Logger.getLogger(XMLParser.class)
+          .warn("The XML parser in use does not support DTD validation: " + ex);
       return false;
     }
   }

@@ -0,0 +1,69 @@
+package org.testng.xml;
+
+import java.util.Arrays;
+import java.util.Locale;
+import org.testng.internal.RuntimeBehavior;
+import org.testng.log4testng.Logger;
+
+/**
+ * How strictly a suite file is checked against the TestNG DTD, selected with the {@code
+ * testng.xml.validation} system property.
+ *
+ * <p>Validation used to be silently disabled: {@code XMLParser} probed the SAX validation feature
+ * under an {@code https} identifier, which no parser recognizes, so {@code setValidating(true)} was
+ * never reached and DTD violations went unreported. Turning it back on means suite files that have
+ * been accepted for years can suddenly be rejected -- the DTD constrains the order of the children
+ * of {@code <suite>}, for instance -- so {@link #WARN} is the default for now and reports
+ * violations without failing the run.
+ *
+ * <p>The property is read at two different moments, which constrains when it can be changed. {@code
+ * XMLParser} decides <em>whether to validate</em> once, when it builds its single static {@code
+ * SAXParser}; {@code TestNGContentHandler.error} decides <em>how to report</em> a violation on
+ * every occurrence. Moving between {@link #WARN} and {@link #STRICT} at run time therefore takes
+ * effect, but moving away from {@link #OFF} does not, because no violation is ever raised to
+ * report. Set the property on the command line to be safe.
+ */
+public enum XmlValidationMode {
+
+  /** Do not validate at all. */
+  OFF,
+
+  /** Validate and report violations as warnings. The default. */
+  WARN,
+
+  /** Validate and fail on the first violation. */
+  STRICT;
+
+  private static final XmlValidationMode DEFAULT = WARN;
+
+  public boolean isValidating() {
+    return this != OFF;
+  }
+
+  /**
+   * The mode requested by the {@code testng.xml.validation} system property, falling back to {@link
+   * #WARN} when the property is absent or holds an unknown value.
+   */
+  public static XmlValidationMode current() {
+    String requested = RuntimeBehavior.getXmlValidationMode();
+    if (requested == null || requested.trim().isEmpty()) {
+      return DEFAULT;
+    }
+    try {
+      return valueOf(requested.trim().toUpperCase(Locale.ROOT));
+    } catch (IllegalArgumentException e) {
+      Logger.getLogger(XmlValidationMode.class)
+          .warn(
+              "Unknown value ["
+                  + requested
+                  + "] for the system property ["
+                  + RuntimeBehavior.XML_VALIDATION_MODE
+                  + "]. Expected one of "
+                  + Arrays.toString(values())
+                  + ". Falling back to ["
+                  + DEFAULT
+                  + "].");
+      return DEFAULT;
+    }
+  }
+}