ljmotta commented on PR #2158:
URL:
https://github.com/apache/incubator-kie-tools/pull/2158#issuecomment-1947403357
Ok, I've taken a closer look at your changes Jozef, and I have a few
observations.
I could see that the Playwright extension wasn't working on the "dmn-editor"
folder due to the missing `lodash` dev dependency, as we use the `merge` on the
`playwright.config.ts`.
Regarding the constant files, I don't think it's a good approach as it will
not be easy to maintain it, and we're introducing test-specific code to the src
codebase. At least, we shouldn't reuse `classNames` as `data-testid`, even
though they have the same name, they are two completely different things.
Instead of creating the constants, we could reuse the constants from the DMN
Editor. For the edges, we have the `EDGE_TYPES`, and for the nodes we have
`NODE_TYPES`. Using the types we would have for the "KnowledgeRequirementPath":
`data-testid={EDGE_TYPES.knowledgeRequirement}`
For the "OutgoingStuffNodePanel", we could create a `data-testid` appending
the "add":
```
data-testid={`add_${edgeType}`}
data-testid={`add_${nodeType}`}
```
Another observation is that you're not taking advantage of the Playwright
fixture. For example, the
`page.getByTestId("kie-dmn-editor--diagram-container")` is something that will
be used extensively. This could be used as something like:
```
test("my test", async ({ page, diagram }) => {
await page
.getByTitle("Input Data", { exact: true })
.dragTo(diagram.getContainer(), { targetPosition: { x: 100, y: 300 }
});
})
```
This would reduce to one unique place you need to be worried about the
`data-testid`.
Now, after playing a little with the tests, I could find another approach
that I think would be more beneficial for us in the long run. The edges have a
`role=button` and an `aria-label` telling us the "from/to" with the node ids.
Given that, adding the name of the node as a `title` property and adding the
node id as another property (in this example for the Input Data node):
```
title={inputData["@_name"]}
data-nodeid={`#${inputData["@_id"]}`}
```
OBS: We could use the `data-testid` too, but as I could see, the RF uses
this `data-nodeid` on other elements as well. Also, I used the `title` because
it makes more sense to have a `title` property with the node name, but I'm not
sure if `data-testid` would be better as it's specific to tests.
With that in place, we could get the exact edge we want (this is for the
"From Input Data" -> "Add Decision" test):
```
const from = await page.getByTitle("New Input Data", { exact: true
}).getAttribute("data-nodeid");
const to = await page.getByTitle("New Decision", { exact: true
}).getAttribute("data-nodeid");
await expect(page.getByRole("button", { name: `Edge from ${from} to ${to}`
})).toBeAttached();
await expect(page.getByRole("button", { name: `Edge from ${from} to ${to}`
}).getByTestId(EDGE_TYPES.informationRequirement)).toBeAttached();
```
And looking at this snippet, we could see a potential Playwright fixture 😃
```
await expect(await diagram.getEdge(from, to)).toBeAttached();
await expect(await diagram.getEdge(from,
to).getByTestId(EDGE_TYPES.informationRequirement)).toBeAttached();
```
At least, while running the tests I didn't generate any "playwright-report"
folder, which command did you run?
--
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]
