You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kie.apache.org by "ljmotta (via GitHub)" <gi...@apache.org> on 2024/03/01 20:30:47 UTC
[PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
ljmotta opened a new pull request, #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189
Related to https://github.com/apache/incubator-kie-issues/issues/946
This PR continues the exploratory phase and continue the work started on https://github.com/apache/incubator-kie-tools/pull/2158
## Description
Wip
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento commented on code in PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189#discussion_r1515376734
##########
packages/dmn-editor/src/diagram/nodes/Nodes.tsx:
##########
@@ -239,20 +239,19 @@ export const InputDataNode = React.memo(
onDoubleClick={triggerEditing}
onKeyDown={triggerEditingIfEnter}
style={alternativeSvgStyle}
- className={selectedAlternativeClass}
+ className={`kie-dmn-editor--node ${selectedAlternativeClass}`}
Review Comment:
There's one `"kie-dmn-editor--node"` inside another `"kie-dmn-editor--node"`?
##########
packages/dmn-editor/env/index.js:
##########
@@ -23,14 +23,9 @@ module.exports = composeEnv([require("@kie-tools/root-env/env")], {
vars: varsWithName({}),
get env() {
return {
- dmnEditor: {
- dev: {
- port: 3001,
- },
- },
dmnEditor: {
storybook: {
- port: 9901,
Review Comment:
Are all ports on `env/index.js` files defined as strings? I know this had the color bug with the special characters and everything, but it's really bugging me that this could be the only place where we need these numbers to be defined as strings...
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on code in PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189#discussion_r1511271839
##########
packages/dmn-editor/tests/e2e/deleteGroupAndDrgElements.spec.ts:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { test, expect } from "./__fixtures__/base";
+import { DefaultNodeName, NodeType } from "./__fixtures__/nodes";
+
+test.beforeEach(async ({ editor }) => {
+ await editor.open();
+});
+
+test.describe("Delete node - Group and DRG Elements", () => {
+ test.describe("Group with DRG elements", () => {
+ test.beforeEach(async ({ palette, nodes }) => {
+ await palette.dragNewNode({ type: NodeType.GROUP, targetPosition: { x: 100, y: 100 } });
+ await palette.dragNewNode({ type: NodeType.DECISION, targetPosition: { x: 130, y: 130 } });
+
+ await expect(nodes.get({ name: DefaultNodeName.GROUP })).toBeAttached();
+ await expect(nodes.get({ name: DefaultNodeName.DECISION })).toBeAttached();
+ });
+
+ test("should delete the Group node", async ({ nodes }) => {
+ await nodes.delete({ name: DefaultNodeName.GROUP });
+
+ await expect(nodes.get({ name: DefaultNodeName.GROUP })).not.toBeAttached();
+ await expect(nodes.get({ name: DefaultNodeName.DECISION })).toBeAttached();
+ });
+
+ test("should delete the DRG node", async ({ nodes }) => {
+ await nodes.delete({ name: DefaultNodeName.DECISION });
+
+ await expect(nodes.get({ name: DefaultNodeName.GROUP })).toBeAttached();
+ await expect(nodes.get({ name: DefaultNodeName.DECISION })).not.toBeAttached();
+ });
+
+ test("should delete the Group and DRG node", async ({ diagram, nodes }) => {
+ await diagram.select({ startPosition: { x: 50, y: 50 }, endPosition: { x: 500, y: 500 } });
+ await diagram.get().press("Delete");
Review Comment:
@ljmotta ok, I am fine with having only one of the keys used in most of the `delete*.spec.ts`, however I am not sure if I understand the strategy of testing the same behavior of `Delete` and `Backspace`.
Is that each `delete*.spec.ts` should test `Delete` in all its scenarios and single test in such spec should test also `Backspace` ?
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "ljmotta (via GitHub)" <gi...@apache.org>.
ljmotta commented on code in PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189#discussion_r1515394527
##########
packages/dmn-editor/env/index.js:
##########
@@ -23,14 +23,9 @@ module.exports = composeEnv([require("@kie-tools/root-env/env")], {
vars: varsWithName({}),
get env() {
return {
- dmnEditor: {
- dev: {
- port: 3001,
- },
- },
dmnEditor: {
storybook: {
- port: 9901,
Review Comment:
The problem is happening because we use the STORYBOOK_BASE env variable inside the storybook wrapper. We set the env variable on the start script and them it's used inside the `storybook-base` bin, thus the terminal color. I choose this approach as I had some problems evaluating the `build-env boxedExpressionComponent.storybook.port` on Windows due to quotes and double quotes. I guess I need to revist this problem. Anyway, this change should affect only components that use storybook.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on code in PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189#discussion_r1510695323
##########
packages/dmn-editor/tests/e2e/__screenshots__/Google-Chrome/drdArtifacts/Add-edge-waypoint---Association-should-add-sin-836b4-should-not-move-when-the-ending-node-is-moved-1.png:
##########
Review Comment:
Maybe we could increase the distance of nodes to make the edge more visible?
##########
packages/dmn-editor/tests/e2e/__screenshots__/Google-Chrome/drgRequirements/Add-edge---Information-Requirement-should-add--3cb4e-nt-edge-from-Input-Data-node-to-Decision-node-1.png:
##########
Review Comment:
file name says we should connect input data with decision, however in screenshot we have two decisions
##########
packages/dmn-editor/tests/e2e/__fixtures__/nodes.ts:
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Locator, Page } from "@playwright/test";
+import { Diagram } from "./diagram";
+import { EdgeType } from "./edges";
+
+export enum NodeType {
+ INPUT_DATA,
+ DECISION,
+ KNOWLEDGE_SOURCE,
+ BKM,
+ DECISION_SERVICE,
+ GROUP,
+ TEXT_ANNOTATION,
+}
+
+export enum DefaultNodeName {
+ INPUT_DATA = "New Input Data",
+ DECISION = "New Decision",
+ KNOWLEDGE_SOURCE = "New Knowledge Source",
+ BKM = "New BKM",
+ DECISION_SERVICE = "New Decision Service",
+ GROUP = "New group",
+ TEXT_ANNOTATION = "New text annotation",
+}
+
+export enum NodePosition {
+ BOTTOM,
+ CENTER,
+ LEFT,
+ RIGHT,
+ TOP,
+ TOP_PADDING,
+}
+
+export class Nodes {
+ constructor(public page: Page, public diagram: Diagram, public browserName: string) {}
+
+ public get(args: { name: string }) {
+ return this.page.locator(`div[data-nodelabel="${args.name}"]`);
+ }
+
+ public async getId(args: { name: string }): Promise<string> {
+ return (await this.get({ name: args.name }).getAttribute("data-nodehref")) ?? "";
+ }
+
+ public async delete(args: { name: string }) {
+ await this.select({ name: args.name, position: NodePosition.TOP_PADDING });
+ await this.diagram.get().press("Delete");
+ }
+
+ public async deleteMultiple(args: { names: string[] }) {
+ await this.selectMultiple({ names: args.names, position: NodePosition.TOP_PADDING });
+ await this.diagram.get().press("Delete");
+ }
+
+ public async rename(args: { current: string; new: string }) {
+ await this.get({ name: args.current }).getByRole("textbox").nth(0).fill(args.new);
+ await this.diagram.get().press("Enter");
+ }
+
+ public async dragNewConnectedNode(args: {
+ type: NodeType;
+ from: string;
+ targetPosition: { x: number; y: number };
+ thenRenameTo?: string;
+ }) {
+ await this.hover({ name: args.from, position: NodePosition.TOP });
+ const node = this.get({ name: args.from });
+
+ switch (args.type) {
+ case NodeType.INPUT_DATA:
+ await node
+ .getByTitle("Add Input Data node")
+ .dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.INPUT_DATA });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.INPUT_DATA, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.DECISION:
+ await node.getByTitle("Add Decision node").dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.DECISION });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.DECISION, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.KNOWLEDGE_SOURCE:
+ await node
+ .getByTitle("Add Knowledge Source node")
+ .dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.KNOWLEDGE_SOURCE });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.KNOWLEDGE_SOURCE, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.BKM:
+ await node.getByTitle("Add BKM node").dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.BKM });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.BKM, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.TEXT_ANNOTATION:
+ await node
+ .getByTitle("Add Text Annotation node")
+ .dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.TEXT_ANNOTATION });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.TEXT_ANNOTATION, new: args.thenRenameTo });
+ }
+ }
Review Comment:
Maybe we could shorten the code slightly, please let me know what do you think about this idea.
```js
let nodeTitle;
let nodeName;
switch (args.type) {
case NodeType.INPUT_DATA:
nodeTitle = "Add Input Data node";
nodeName = DefaultNodeName.INPUT_DATA;
break;
case NodeType.DECISION:
nodeTitle = "Add Decision node";
nodeName = DefaultNodeName.DECISION;
break;
case NodeType.KNOWLEDGE_SOURCE:
// all cases here
// ...
}
await node
.getByTitle(nodeTitle)
.dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
await this.waitForNodeToBeFocused({ name: nodeName });
if (args.thenRenameTo) {
await this.rename({ current: nodeName, new: args.thenRenameTo });
}
```
##########
packages/dmn-editor/tests/e2e/__screenshots__/Google-Chrome/drdArtifacts/Add-edge---Association-Add-association-edge-fr-a1cb6-Text-Annotation-node-to-Decision-Service-node-1.png:
##########
Review Comment:
Maybe we could increase the distance of nodes to make the edge more visible?
##########
packages/dmn-editor/tests/e2e/drdArtifacts/addTextAnnotation.spec.ts:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { TestAnnotations } from "@kie-tools/playwright-base/annotations";
+import { test, expect } from "../__fixtures__/base";
+import { EdgeType } from "../__fixtures__/edges";
+import { DefaultNodeName, NodeType } from "../__fixtures__/nodes";
+
+test.beforeEach(async ({ editor }) => {
+ await editor.open();
+});
+
+test.describe("Add node - Text Annotation", () => {
+ test.describe("add from the palette", () => {
+ test("should add Text Annotation node from palette", async ({ palette, nodes, diagram }) => {
+ await palette.dragNewNode({ type: NodeType.GROUP, targetPosition: { x: 100, y: 100 } });
+
+ expect(nodes.get({ name: DefaultNodeName.GROUP })).toBeAttached();
Review Comment:
```suggestion
await palette.dragNewNode({ type: NodeType.TEXT_ANNOTATION, targetPosition: { x: 100, y: 100 } });
expect(nodes.get({ name: DefaultNodeName.TEXT_ANNOTATION })).toBeAttached();
```
##########
packages/dmn-editor/tests/e2e/__fixtures__/palette.ts:
##########
@@ -0,0 +1,94 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Page } from "@playwright/test";
+import { Diagram } from "./diagram";
+import { DefaultNodeName, Nodes, NodeType } from "./nodes";
+
+export class Palette {
+ constructor(public page: Page, public diagram: Diagram, public nodes: Nodes) {}
+
+ public async dragNewNode(args: { type: NodeType; targetPosition: { x: number; y: number }; thenRenameTo?: string }) {
+ switch (args.type) {
Review Comment:
Maybe we could apply the same code shortening also for this switch, what do you think?
##########
packages/dmn-editor/tests/e2e/deleteGroupAndDrgElements.spec.ts:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { test, expect } from "./__fixtures__/base";
+import { DefaultNodeName, NodeType } from "./__fixtures__/nodes";
+
+test.beforeEach(async ({ editor }) => {
+ await editor.open();
+});
+
+test.describe("Delete node - Group and DRG Elements", () => {
+ test.describe("Group with DRG elements", () => {
+ test.beforeEach(async ({ palette, nodes }) => {
+ await palette.dragNewNode({ type: NodeType.GROUP, targetPosition: { x: 100, y: 100 } });
+ await palette.dragNewNode({ type: NodeType.DECISION, targetPosition: { x: 130, y: 130 } });
+
+ await expect(nodes.get({ name: DefaultNodeName.GROUP })).toBeAttached();
+ await expect(nodes.get({ name: DefaultNodeName.DECISION })).toBeAttached();
+ });
+
+ test("should delete the Group node", async ({ nodes }) => {
+ await nodes.delete({ name: DefaultNodeName.GROUP });
+
+ await expect(nodes.get({ name: DefaultNodeName.GROUP })).not.toBeAttached();
+ await expect(nodes.get({ name: DefaultNodeName.DECISION })).toBeAttached();
+ });
+
+ test("should delete the DRG node", async ({ nodes }) => {
+ await nodes.delete({ name: DefaultNodeName.DECISION });
+
+ await expect(nodes.get({ name: DefaultNodeName.GROUP })).toBeAttached();
+ await expect(nodes.get({ name: DefaultNodeName.DECISION })).not.toBeAttached();
+ });
+
+ test("should delete the Group and DRG node", async ({ diagram, nodes }) => {
+ await diagram.select({ startPosition: { x: 50, y: 50 }, endPosition: { x: 500, y: 500 } });
+ await diagram.get().press("Delete");
Review Comment:
Should we update all `delete*.spec.ts` files to contain tests both for `Delete` and `Backspace` keys?
##########
packages/dmn-editor/tests/e2e/__screenshots__/Google-Chrome/drdArtifacts/Add-node---Text-Annotation-add-from-the-palette-should-add-Text-Annotation-node-from-palette-1.png:
##########
Review Comment:
In the picture we have group, however test file says we should have text annotation?
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on code in PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189#discussion_r1511274807
##########
packages/dmn-editor/tests/e2e/drdArtifacts/addAssociation.spec.ts:
##########
@@ -54,7 +54,7 @@ test.describe("Add edge - Association", () => {
expect(await edges.getType({ from: DefaultNodeName.BKM, to: DefaultNodeName.TEXT_ANNOTATION })).toEqual(
EdgeType.ASSOCIATION
);
- await expect(diagram.get()).toHaveScreenshot();
+ await expect(diagram.get()).toHaveScreenshot("add-association-edge-bkm-to-text-annotation.png");
Review Comment:
with similar change, shouldn't we also delete some previous version of screenshots?
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "ljmotta (via GitHub)" <gi...@apache.org>.
ljmotta commented on code in PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189#discussion_r1511160235
##########
packages/dmn-editor/tests/e2e/drdArtifacts/addTextAnnotation.spec.ts:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { TestAnnotations } from "@kie-tools/playwright-base/annotations";
+import { test, expect } from "../__fixtures__/base";
+import { EdgeType } from "../__fixtures__/edges";
+import { DefaultNodeName, NodeType } from "../__fixtures__/nodes";
+
+test.beforeEach(async ({ editor }) => {
+ await editor.open();
+});
+
+test.describe("Add node - Text Annotation", () => {
+ test.describe("add from the palette", () => {
+ test("should add Text Annotation node from palette", async ({ palette, nodes, diagram }) => {
+ await palette.dragNewNode({ type: NodeType.GROUP, targetPosition: { x: 100, y: 100 } });
+
+ expect(nodes.get({ name: DefaultNodeName.GROUP })).toBeAttached();
Review Comment:
Great catch, this was causing some of the weird screenshots.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "ljmotta (via GitHub)" <gi...@apache.org>.
ljmotta commented on code in PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189#discussion_r1511180658
##########
packages/dmn-editor/tests/e2e/__fixtures__/nodes.ts:
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Locator, Page } from "@playwright/test";
+import { Diagram } from "./diagram";
+import { EdgeType } from "./edges";
+
+export enum NodeType {
+ INPUT_DATA,
+ DECISION,
+ KNOWLEDGE_SOURCE,
+ BKM,
+ DECISION_SERVICE,
+ GROUP,
+ TEXT_ANNOTATION,
+}
+
+export enum DefaultNodeName {
+ INPUT_DATA = "New Input Data",
+ DECISION = "New Decision",
+ KNOWLEDGE_SOURCE = "New Knowledge Source",
+ BKM = "New BKM",
+ DECISION_SERVICE = "New Decision Service",
+ GROUP = "New group",
+ TEXT_ANNOTATION = "New text annotation",
+}
+
+export enum NodePosition {
+ BOTTOM,
+ CENTER,
+ LEFT,
+ RIGHT,
+ TOP,
+ TOP_PADDING,
+}
+
+export class Nodes {
+ constructor(public page: Page, public diagram: Diagram, public browserName: string) {}
+
+ public get(args: { name: string }) {
+ return this.page.locator(`div[data-nodelabel="${args.name}"]`);
+ }
+
+ public async getId(args: { name: string }): Promise<string> {
+ return (await this.get({ name: args.name }).getAttribute("data-nodehref")) ?? "";
+ }
+
+ public async delete(args: { name: string }) {
+ await this.select({ name: args.name, position: NodePosition.TOP_PADDING });
+ await this.diagram.get().press("Delete");
+ }
+
+ public async deleteMultiple(args: { names: string[] }) {
+ await this.selectMultiple({ names: args.names, position: NodePosition.TOP_PADDING });
+ await this.diagram.get().press("Delete");
+ }
+
+ public async rename(args: { current: string; new: string }) {
+ await this.get({ name: args.current }).getByRole("textbox").nth(0).fill(args.new);
+ await this.diagram.get().press("Enter");
+ }
+
+ public async dragNewConnectedNode(args: {
+ type: NodeType;
+ from: string;
+ targetPosition: { x: number; y: number };
+ thenRenameTo?: string;
+ }) {
+ await this.hover({ name: args.from, position: NodePosition.TOP });
+ const node = this.get({ name: args.from });
+
+ switch (args.type) {
+ case NodeType.INPUT_DATA:
+ await node
+ .getByTitle("Add Input Data node")
+ .dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.INPUT_DATA });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.INPUT_DATA, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.DECISION:
+ await node.getByTitle("Add Decision node").dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.DECISION });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.DECISION, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.KNOWLEDGE_SOURCE:
+ await node
+ .getByTitle("Add Knowledge Source node")
+ .dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.KNOWLEDGE_SOURCE });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.KNOWLEDGE_SOURCE, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.BKM:
+ await node.getByTitle("Add BKM node").dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.BKM });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.BKM, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.TEXT_ANNOTATION:
+ await node
+ .getByTitle("Add Text Annotation node")
+ .dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.TEXT_ANNOTATION });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.TEXT_ANNOTATION, new: args.thenRenameTo });
+ }
+ }
Review Comment:
We can shorten this method, but I don't think this approach is the ideal one, as creating mutable variables isn't very friendly. We can create a private method where we return the `{ nodeTitle, nodeName }` on the `case` statement.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues#946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "tiagobento (via GitHub)" <gi...@apache.org>.
tiagobento merged PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "jomarko (via GitHub)" <gi...@apache.org>.
jomarko commented on code in PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189#discussion_r1511267236
##########
packages/dmn-editor/tests/e2e/__fixtures__/nodes.ts:
##########
@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Locator, Page } from "@playwright/test";
+import { Diagram } from "./diagram";
+import { EdgeType } from "./edges";
+
+export enum NodeType {
+ INPUT_DATA,
+ DECISION,
+ KNOWLEDGE_SOURCE,
+ BKM,
+ DECISION_SERVICE,
+ GROUP,
+ TEXT_ANNOTATION,
+}
+
+export enum DefaultNodeName {
+ INPUT_DATA = "New Input Data",
+ DECISION = "New Decision",
+ KNOWLEDGE_SOURCE = "New Knowledge Source",
+ BKM = "New BKM",
+ DECISION_SERVICE = "New Decision Service",
+ GROUP = "New group",
+ TEXT_ANNOTATION = "New text annotation",
+}
+
+export enum NodePosition {
+ BOTTOM,
+ CENTER,
+ LEFT,
+ RIGHT,
+ TOP,
+ TOP_PADDING,
+}
+
+export class Nodes {
+ constructor(public page: Page, public diagram: Diagram, public browserName: string) {}
+
+ public get(args: { name: string }) {
+ return this.page.locator(`div[data-nodelabel="${args.name}"]`);
+ }
+
+ public async getId(args: { name: string }): Promise<string> {
+ return (await this.get({ name: args.name }).getAttribute("data-nodehref")) ?? "";
+ }
+
+ public async delete(args: { name: string }) {
+ await this.select({ name: args.name, position: NodePosition.TOP_PADDING });
+ await this.diagram.get().press("Delete");
+ }
+
+ public async deleteMultiple(args: { names: string[] }) {
+ await this.selectMultiple({ names: args.names, position: NodePosition.TOP_PADDING });
+ await this.diagram.get().press("Delete");
+ }
+
+ public async rename(args: { current: string; new: string }) {
+ await this.get({ name: args.current }).getByRole("textbox").nth(0).fill(args.new);
+ await this.diagram.get().press("Enter");
+ }
+
+ public async dragNewConnectedNode(args: {
+ type: NodeType;
+ from: string;
+ targetPosition: { x: number; y: number };
+ thenRenameTo?: string;
+ }) {
+ await this.hover({ name: args.from, position: NodePosition.TOP });
+ const node = this.get({ name: args.from });
+
+ switch (args.type) {
+ case NodeType.INPUT_DATA:
+ await node
+ .getByTitle("Add Input Data node")
+ .dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.INPUT_DATA });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.INPUT_DATA, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.DECISION:
+ await node.getByTitle("Add Decision node").dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.DECISION });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.DECISION, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.KNOWLEDGE_SOURCE:
+ await node
+ .getByTitle("Add Knowledge Source node")
+ .dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.KNOWLEDGE_SOURCE });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.KNOWLEDGE_SOURCE, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.BKM:
+ await node.getByTitle("Add BKM node").dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.BKM });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.BKM, new: args.thenRenameTo });
+ }
+ break;
+ case NodeType.TEXT_ANNOTATION:
+ await node
+ .getByTitle("Add Text Annotation node")
+ .dragTo(this.diagram.get(), { targetPosition: args.targetPosition });
+ await this.waitForNodeToBeFocused({ name: DefaultNodeName.TEXT_ANNOTATION });
+ if (args.thenRenameTo) {
+ await this.rename({ current: DefaultNodeName.TEXT_ANNOTATION, new: args.thenRenameTo });
+ }
+ }
Review Comment:
It sounds good to me @ljmotta !
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "ljmotta (via GitHub)" <gi...@apache.org>.
ljmotta commented on code in PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189#discussion_r1511189351
##########
packages/dmn-editor/tests/e2e/deleteGroupAndDrgElements.spec.ts:
##########
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { test, expect } from "./__fixtures__/base";
+import { DefaultNodeName, NodeType } from "./__fixtures__/nodes";
+
+test.beforeEach(async ({ editor }) => {
+ await editor.open();
+});
+
+test.describe("Delete node - Group and DRG Elements", () => {
+ test.describe("Group with DRG elements", () => {
+ test.beforeEach(async ({ palette, nodes }) => {
+ await palette.dragNewNode({ type: NodeType.GROUP, targetPosition: { x: 100, y: 100 } });
+ await palette.dragNewNode({ type: NodeType.DECISION, targetPosition: { x: 130, y: 130 } });
+
+ await expect(nodes.get({ name: DefaultNodeName.GROUP })).toBeAttached();
+ await expect(nodes.get({ name: DefaultNodeName.DECISION })).toBeAttached();
+ });
+
+ test("should delete the Group node", async ({ nodes }) => {
+ await nodes.delete({ name: DefaultNodeName.GROUP });
+
+ await expect(nodes.get({ name: DefaultNodeName.GROUP })).not.toBeAttached();
+ await expect(nodes.get({ name: DefaultNodeName.DECISION })).toBeAttached();
+ });
+
+ test("should delete the DRG node", async ({ nodes }) => {
+ await nodes.delete({ name: DefaultNodeName.DECISION });
+
+ await expect(nodes.get({ name: DefaultNodeName.GROUP })).toBeAttached();
+ await expect(nodes.get({ name: DefaultNodeName.DECISION })).not.toBeAttached();
+ });
+
+ test("should delete the Group and DRG node", async ({ diagram, nodes }) => {
+ await diagram.select({ startPosition: { x: 50, y: 50 }, endPosition: { x: 500, y: 500 } });
+ await diagram.get().press("Delete");
Review Comment:
I don't think we need to test the `Delete` x `Backspace` on all cases. We can have tests were we check if the `Backspace` behaves the same as the `Delete`, and then we can choose one for the tests.
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-issues-946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 2 [incubator-kie-tools]
Posted by "ljmotta (via GitHub)" <gi...@apache.org>.
ljmotta commented on code in PR #2189:
URL: https://github.com/apache/incubator-kie-tools/pull/2189#discussion_r1511163309
##########
packages/dmn-editor/tests/e2e/__screenshots__/Google-Chrome/drgRequirements/Add-edge---Information-Requirement-should-add--3cb4e-nt-edge-from-Input-Data-node-to-Decision-node-1.png:
##########
Review Comment:
Great catch!
--
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: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org