tiagobento commented on code in PR #2207:
URL: 
https://github.com/apache/incubator-kie-tools/pull/2207#discussion_r1538040788


##########
packages/dmn-marshaller/tests/xsdSequence.test.ts:
##########
@@ -21,74 +21,57 @@ import * as fs from "fs";
 import * as path from "path";
 import { getMarshaller } from "@kie-tools/dmn-marshaller";
 
-const files = [
+const filesForAddition = [
   { path: "../tests-data--manual/other/decisionAndInput.dmn" },
+  { path: "../tests-data--manual/other/decisionAndInputWithAddition.dmn" },
+];
+
+const filesForNormalization = [
   { path: 
"../tests-data--manual/other/decisionAndInput_wrongSequenceOrder.dmn" },
+  { path: "../tests-data--manual/other/decisionAndInput.dmn" },
 ];
 
-describe("build always produces elements in the same order", () => {
-  for (const file of files) {
-    test(path.basename(file.path), () => {
-      const xml = fs.readFileSync(path.join(__dirname, file.path), "utf-8");
-      const marshaller = getMarshaller(xml, { upgradeTo: "1.5" });
-      const json = marshaller.parser.parse();
+describe("build always add elements in the same order", () => {
+  test("Addition", () => {
+    const fileSource = filesForAddition[0];
+    const fileExpected = filesForAddition[1];
+    const xml = fs.readFileSync(path.join(__dirname, fileSource.path), 
"utf-8");
+    const marshaller = getMarshaller(xml, { upgradeTo: "1.5" });
+    const json = marshaller.parser.parse();
 
-      // Adding some props in an arbitrary order shouldn't alter the end 
result.
+    // Adding some props in an arbitrary order shouldn't alter the end result.
 
-      json.definitions.import = [
-        {
-          "@_name": "some-import",
-          "@_namespace": "some-namespace",
-          "@_importType": "some-import-type",
-        },
-      ];
+    json.definitions.import = [
+      {
+        "@_name": "some-import",
+        "@_namespace": "some-namespace",
+        "@_importType": "some-import-type",
+      },
+    ];
 
-      json.definitions.artifact = [
-        {
-          __$$element: "group",
-          "@_name": "some-group",
-        },
-      ];
+    json.definitions.artifact = [
+      {
+        __$$element: "group",
+        "@_name": "some-group",
+      },
+    ];
+
+    const xmlRetrieved = marshaller.builder.build(json).replace(/\s/g, "");
+    const xmlExpected = fs.readFileSync(path.join(__dirname, 
fileExpected.path), "utf-8").replace(/\s/g, "");
+    expect(xmlRetrieved).toEqual(xmlExpected);
+  });
+});
+
+describe("build always produces elements in the same order", () => {
+  test("Normalization", () => {
+    const fileSource = filesForNormalization[0];
+    const fileExpected = filesForNormalization[1];
+    const xmlSource = fs.readFileSync(path.join(__dirname, fileSource.path), 
"utf-8");
+    const marshaller = getMarshaller(xmlSource, { upgradeTo: "1.5" });
+    const json = marshaller.parser.parse();
 
-      expect(marshaller.builder.build(json)).toStrictEqual(`<?xml 
version="1.0" encoding="UTF-8" ?>
-<definitions xmlns="https://www.omg.org/spec/DMN/20230324/MODEL/"; 
expressionLanguage="https://www.omg.org/spec/DMN/20230324/FEEL/"; 
namespace="https://kie.org/dmn/_D19C1092-7677-427F-A493-BCED38F74A9B"; 
id="_11655DE3-BEA5-45B1-B54E-8AD84FBBED25" 
name="DMN_1E889EDB-B967-4508-8DB1-E0DF5986E62F" 
xmlns:dmndi="https://www.omg.org/spec/DMN/20230324/DMNDI/"; 
xmlns:dc="http://www.omg.org/spec/DMN/20180521/DC/"; 
xmlns:di="http://www.omg.org/spec/DMN/20180521/DI/"; 
xmlns:kie="https://kie.org/dmn/extensions/1.0";>
-  <import name="some-import" namespace="some-namespace" 
importType="some-import-type" />
-  <inputData name="New Input Data" id="_154F9E03-B180-4C87-B7D3-8745DA4336F4">
-    <variable name="New Input Data" id="_A28401DD-9A87-4251-A1E4-C63FC3A7C729" 
typeRef="string" />
-  </inputData>
-  <decision name="New Decision" id="_392BEF3D-44B5-47DC-8A06-C36F15DB2984">
-    <variable id="_C2C9C21A-E708-46D9-876A-52BB25692B66" typeRef="string" 
name="New Decision" />
-    <informationRequirement id="_E781E253-D97E-4A1D-BE51-037B012B30F0">
-      <requiredInput href="#_154F9E03-B180-4C87-B7D3-8745DA4336F4" />
-    </informationRequirement>
-    <literalExpression id="_509ED9AF-3852-48F4-89A7-3CCF221B809C" label="New 
Decision" typeRef="string">
-      <text>&quot;New Decision&quot;</text>
-    </literalExpression>
-  </decision>
-  <group name="some-group" />
-  <dmndi:DMNDI>
-    <dmndi:DMNDiagram id="_0D2FD42B-91FF-4795-B71F-E501CE115389" name="Default 
DRD" useAlternativeInputDataShape="false">
-      <di:extension>
-        <kie:ComponentsWidthsExtension>
-          <kie:ComponentWidths 
dmnElementRef="_509ED9AF-3852-48F4-89A7-3CCF221B809C">
-            <kie:width>190</kie:width>
-          </kie:ComponentWidths>
-        </kie:ComponentsWidthsExtension>
-      </di:extension>
-      <dmndi:DMNShape id="_92B3305F-A892-4E38-BD92-398906A9BC24" 
dmnElementRef="_154F9E03-B180-4C87-B7D3-8745DA4336F4" isCollapsed="false" 
isListedInputData="false">
-        <dc:Bounds x="100" y="280" width="160" height="80" />
-      </dmndi:DMNShape>
-      <dmndi:DMNShape id="_1E49EEEB-9296-4AE5-B37C-2EE0044C0CC2" 
dmnElementRef="_392BEF3D-44B5-47DC-8A06-C36F15DB2984" isCollapsed="false" 
isListedInputData="false">
-        <dc:Bounds x="100" y="100" width="160" height="80" />
-      </dmndi:DMNShape>
-      <dmndi:DMNEdge id="_C54C8ED9-7DB2-47BC-844A-E79D7142844B-AUTO-TARGET" 
dmnElementRef="_E781E253-D97E-4A1D-BE51-037B012B30F0" 
sourceElement="_92B3305F-A892-4E38-BD92-398906A9BC24" 
targetElement="_1E49EEEB-9296-4AE5-B37C-2EE0044C0CC2">
-        <di:waypoint x="180" y="320" />
-        <di:waypoint x="180" y="140" />
-      </dmndi:DMNEdge>
-    </dmndi:DMNDiagram>
-  </dmndi:DMNDI>
-</definitions>
-`);
-    });
-  }
+    const xmlRetrieved = marshaller.builder.build(json).replace(/\s/g, "");
+    const xmlExpected = fs.readFileSync(path.join(__dirname, 
fileExpected.path), "utf-8").replace(/\s/g, "");
+    expect(xmlRetrieved).toStrictEqual(xmlExpected);
+  });

Review Comment:
   Yep, I got it, you have a good point, and we indeed need a good strategy to 
make sure we're covered. My observation was about `xsdSequence.test.ts`, where 
you introduced the `/\s/g` replace. If we ignore spaces and new lines, we are 
ignoring the new line treatment for FEEL expressions, for example, which is 
very important.
   
   IMHO we should normalize the files by formatting them with Prettier, 
programatically. We've done it in other places, no reason why we can't do it 
here as well. We trust Prettier won't break anything when formatting, so doing:
   
   1. Read unformatted file
   2. Formatting it with Prettier, using the XML rules.
   3. Parsing it with our Marshaller.
   4. Building it with our Marshaller, and formatting it with Prettier.
   5. Comparing formatted original XML with formatted built XML.
   
   should be the way to go, IMHO. WDYT?
   
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to