gitgabrio commented on code in PR #2207:
URL:
https://github.com/apache/incubator-kie-tools/pull/2207#discussion_r1537910937
##########
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>"New Decision"</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:
> Several comments here:
>
>
> Using an array for `fileSource` and `fileExpected` is not the way to go
here. I would inline the paths, to be honest.
>
Could you provide an example ? It is not clear to me (is that already used
somewhere else ?)
>
> Having one `describe` for each `test` looks weird too. I would do one
describe like this:
>
> `describe("build always produces elements in the same order"`
>
> and two tests like this:
>
> `test("after adding elements")` `test("after reading denormalized file")`
>
Fine
>
> Replacing blank spaces with an empty string is tempering with the tests.
My suggestion is that we match the files char by char, without exceptions.
The `toStrictEqual` method fails here, because the generated xml has
different formatting/white space then the original one.
One of the caveat of this test is to be sure that the generated xml is
absolutely identical to the original one as it is: if the "litmus test" xml is
somehow modified by the very code that is under test, then the result of the
test could be "invalid".
e.g.
```javascript
1 const xml_original = fs.readFileSync(fullPathOfFile, "utf-8");
2 const { parser, builder } = getMarshaller(xml_original, { upgradeTo:
"latest" });
3 const json = parser.parse();
4 const xml_firstPass = builder.build(json);
5 const xml_secondPass = builder.build(getMarshaller(xml_firstPass, {
upgradeTo: "latest" }).parser.parse());
6 expect(xml_firstPass).toStrictEqual(xml_secondPass);
```
1. we are testing the "marshaller" (the parser and the build const)
2. the string "xml_firstPass" (line 4) is a "product" of the marshaller,
created from the original string
3. the string "xml_secondPass" (line 5) is another "product" of the
marshaller, created from the "xml_firstPass" string
4. at line 6, the test verify if two products of the marshaller are exactly
the same
5. in that case the `toStrictEqual` succeed because both strings have been
generated by the marshaller, but at the same time it does not guarantee that
the marshaller itself does not introduce/modify/whatever something wrong in
both of them
6. that's why I wanted to test the raw, original xml with the one created by
the marshaller, and only way to do that comparison is to strip out whitespaces.
If there is a nicer command, fine for me, but what it is important IMO is
the logic and princiuple of the test itself.
Does this make sense ? Am I clear ?
--
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]
