fix(closes #59): switch to xmlbuilder2 (#63)

BREAKING CHANGE: xmlbuilder2 does not support xml version 1.1, so junit-report-merger won't be able to process xml files with version 1.1 anymore. To my knowledge no tools use xml version 1.1 in their test reports, so this should not affect people. However, technically, this is a breaking change.

xmlbuilder2 is faster than xmldom, is maintained better and is successor of extremely popular xmlbuilder package. Also, it is more standard compliant.

package.json

@@ -18,7 +18,7 @@
   },
   "dependencies": {
     "fast-glob": "3.2.4",
-    "xmldom": "^0.4.0"
+    "xmlbuilder2": "2.4.0"
   },
   "repository": {
     "type": "git",
@@ -41,7 +41,7 @@
   "homepage": "https://github.com/bhovhannes/junit-report-merger#readme",
   "devDependencies": {
     "husky": "4.3.5",
-    "lint-staged": "10.5.2",
+    "lint-staged": "10.5.3",
     "prettier": "2.2.1",
     "@jest/globals": "26.6.2",
     "jest": "26.6.3"

src/mergeToString.js

@@ -1,4 +1,4 @@
-const { DOMParser, XMLSerializer } = require("xmldom");
+const { create } = require("xmlbuilder2");
 
 /**
  * Merges contents of given XML strings and returns resulting XML string.
@@ -7,12 +7,9 @@ const { DOMParser, XMLSerializer } = require("xmldom");
  * @return {String}
  */
 function mergeToString(srcStrings, options) {
-    const {
-        documentElement: combinedTestSuitesNode,
-    } = new DOMParser().parseFromString(
-        '<?xml version="1.0"?>\n<testsuites></testsuites>',
-        "text/xml"
-    );
+    const targetDoc = create({
+        testsuites: {},
+    });
 
     const attrs = {
         failures: 0,
@@ -21,30 +18,33 @@ function mergeToString(srcStrings, options) {
     };
 
     srcStrings.forEach((srcString) => {
-        const doc = new DOMParser().parseFromString(srcString, "text/xml");
-        const nodes = doc.getElementsByTagName("testsuite"),
-            nodeCount = nodes.length;
-        for (let i = 0; i < nodeCount; ++i) {
-            const testSuiteNode = nodes[i];
-            for (const attr in attrs) {
-                attrs[attr] += Number(testSuiteNode.getAttribute(attr));
-            }
-            combinedTestSuitesNode.appendChild(testSuiteNode);
-        }
-    });
+        const doc = create(srcString, {});
 
-    for (const attr in attrs) {
-        combinedTestSuitesNode.setAttribute(attr, attrs[attr]);
-    }
+        doc.root().each(
+            (xmlBuilder) => {
+                if (xmlBuilder.node.nodeName.toLowerCase() === "testsuite") {
+                    for (const attrNode of xmlBuilder.node.attributes) {
+                        const name = attrNode.name;
+                        if (name in attrs) {
+                            attrs[name] += Number(attrNode.value);
+                        }
+                    }
+                    targetDoc.root().import(xmlBuilder);
+                }
+            },
+            true,
+            true
+        );
 
-    let xmlString = new XMLSerializer().serializeToString(
-        combinedTestSuitesNode
-    );
-    if (xmlString.indexOf("<?") !== 0) {
-        xmlString = '<?xml version="1.0"?>\n' + xmlString;
-    }
+        for (const attr in attrs) {
+            targetDoc.root().att(attr, attrs[attr]);
+        }
+    });
 
-    return xmlString;
+    return targetDoc.toString({
+        prettyPrint: true,
+        noDoubleEncoding: true,
+    });
 }
 
 module.exports = { mergeToString };

test/e2e.spec.js

@@ -7,19 +7,19 @@ const {
 } = require("@jest/globals");
 const path = require("path");
 const fsPromises = require("fs").promises;
-const xmldom = require("xmldom");
+const { create } = require("xmlbuilder2");
 const { mergeFiles } = require("../index.js");
 
 describe("e2e", function () {
     let fixturePaths;
     beforeEach(() => {
         fixturePaths = {
             inputs: [
-                path.join(__dirname, "fixtures", "1.xml"),
-                path.join(__dirname, "fixtures", "2.xml"),
-                path.join(__dirname, "fixtures", "3.xml"),
+                path.join(__dirname, "fixtures", "m1.xml"),
+                path.join(__dirname, "fixtures", "m2.xml"),
+                path.join(__dirname, "fixtures", "m3.xml"),
             ],
-            output: path.join(__dirname, "fixtures", "output.xml"),
+            output: path.join(__dirname, "output", "actual-combined-1-3.xml"),
         };
     });
 
@@ -29,18 +29,22 @@ describe("e2e", function () {
 
     async function assertOutput() {
         const contents = await fsPromises.readFile(fixturePaths.output, "utf8");
-        const doc = new xmldom.DOMParser().parseFromString(
-            contents,
-            "text/xml"
-        );
-        const rootNode = doc.documentElement;
-        const testSuiteNodes = rootNode.getElementsByTagName("testsuite");
-        expect(testSuiteNodes).toHaveLength(4);
-
-        expect(rootNode.tagName.toLowerCase()).toBe("testsuites");
-        expect(rootNode.getAttribute("tests")).toBe("6");
-        expect(rootNode.getAttribute("errors")).toBe("0");
-        expect(rootNode.getAttribute("failures")).toBe("2");
+        const doc = create(contents).root();
+        expect(doc.node.childNodes).toHaveLength(4);
+
+        expect(doc.node.nodeName.toLowerCase()).toBe("testsuites");
+        const foundAttrs = {};
+        for (const attrNode of doc.node.attributes) {
+            const name = attrNode.name;
+            if (["tests", "errors", "failures"].includes(name)) {
+                foundAttrs[name] = attrNode.value;
+            }
+        }
+        expect(foundAttrs).toEqual({
+            tests: "6",
+            errors: "0",
+            failures: "2",
+        });
     }
 
     describe("mergeFiles", function () {
@@ -55,7 +59,7 @@ describe("e2e", function () {
         });
 
         it("merges xml reports matching given glob pattern", async () => {
-            await mergeFiles(fixturePaths.output, ["./**/fixtures/*.xml"]);
+            await mergeFiles(fixturePaths.output, ["./**/fixtures/m*.xml"]);
             await assertOutput();
         });
 
@@ -67,13 +71,7 @@ describe("e2e", function () {
                 fixturePaths.output,
                 "utf8"
             );
-            const doc = new xmldom.DOMParser().parseFromString(
-                contents,
-                "text/xml"
-            );
-            const rootNode = doc.documentElement;
-            const testSuiteNodes = rootNode.getElementsByTagName("testsuite");
-            expect(testSuiteNodes).toHaveLength(0);
+            expect(create(contents).root().node.childNodes).toHaveLength(0);
         });
 
         it("merges xml reports (options passed, callback style)", async () => {
@@ -108,14 +106,49 @@ describe("e2e", function () {
 
             await assertOutput();
         });
+
+        it("preserves xml entities", async () => {
+            await mergeFiles(
+                fixturePaths.output,
+                [path.join(__dirname, "fixtures", "with-entity-char.xml")],
+                {}
+            );
+
+            const contents = await fsPromises.readFile(
+                fixturePaths.output,
+                "utf8"
+            );
+            expect(contents).toContain("failure attr with ]]>");
+            expect(contents).toContain("failure message with ]]>");
+        });
+
+        it("merges m*.xml files into one, matching predefined snapshot", async () => {
+            await mergeFiles(fixturePaths.output, fixturePaths.inputs);
+            const actualContents = await fsPromises.readFile(
+                fixturePaths.output,
+                "utf8"
+            );
+            const expectedContents = await fsPromises.readFile(
+                path.join(
+                    __dirname,
+                    "fixtures",
+                    "expected",
+                    "expected-combined-1-3.xml"
+                ),
+                "utf8"
+            );
+            expect(create(actualContents).toObject()).toEqual(
+                create(expectedContents).toObject()
+            );
+        });
     });
 
     describe("cli", function () {
         it("merges xml reports", async () => {
             await new Promise((resolve, reject) => {
                 const { exec } = require("child_process");
                 exec(
-                    'node ./cli.js ./test/fixtures/output.xml "./test/**/1.xml" "./test/**/*.xml"',
+                    'node ./cli.js ./test/output/actual-combined-1-3.xml "./test/**/m1.xml" "./test/**/m?.xml"',
                     function (error, stdout, stderr) {
                         if (error) {
                             reject(error);

test/fixtures/expected/expected-combined-1-3.xml

@@ -0,0 +1,27 @@
+<?xml version="1.0"?>
+<testsuites failures="2" errors="0" tests="6"><testsuite name="PhantomJS 1.9.8" package="" timestamp="2016-01-22T18:25:08" id="0" hostname="MacBook-Pro.local" tests="2" errors="0" failures="1" time="0.026">
+        <properties>
+            <property name="browser.fullName" value="PhantomJS/1.9.8 Safari/534.34"/>
+        </properties>
+        <testcase name="testcase1.1" time="0.003" classname="PhantomJS 1.9.8.add"/>
+        <testcase name="testcase1.2" time="0.001" classname="PhantomJS 1.9.8.FAIL">
+            <failure type="">
+                AssertionError: expected undefined not to be an undefined
+            </failure>
+        </testcase>
+        <system-out><![CDATA[ foo ]]></system-out>
+        <system-err/>
+    </testsuite><testsuite name="FAIL" timestamp="2016-01-22T18:25:02" tests="2" failures="1" time="0.001">
+        <testcase name="testcase2.1" time="0.001" classname="should fail on client">
+        </testcase>
+        <testcase name="testcase2.2" time="0" classname="should fail on server">
+            <failure><![CDATA[ReferenceError: window is not defined
+                at Context.<anonymous> (src/fail.spec.js:14:21)]]></failure>
+        </testcase>
+    </testsuite><testsuite name="pass1" timestamp="2016-01-22T18:25:02" tests="1" failures="0" time="0.001">
+        <testcase name="testcase3.1" time="0.001" classname="should pass 1">
+        </testcase>
+    </testsuite><testsuite name="pass2" timestamp="2016-01-22T18:25:02" tests="1" failures="0" time="0.001">
+        <testcase name="testcase3.2" time="0.001" classname="should pass 2">
+        </testcase>
+    </testsuite></testsuites>