ljmotta commented on code in PR #3556: URL: https://github.com/apache/incubator-kie-tools/pull/3556#discussion_r3203699105
########## packages/bpmn-editor/tests-e2e/__fixtures__/propertiesPanel/dataObjectPropertiesPanel.ts: ########## @@ -0,0 +1,73 @@ +/* + * 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 { PropertiesPanelBase } from "./propertiesPanelBase"; +import { Diagram } from "../diagram"; +import { NameProperties } from "./parts/nameProperties"; +import { DocumentationProperties } from "./parts/documentationProperties"; + +export class DataObjectPropertiesPanel extends PropertiesPanelBase { + private nameProperties: NameProperties; + private documentationProperties: DocumentationProperties; + + constructor( + public diagram: Diagram, + public page: Page + ) { + super(diagram, page); + this.nameProperties = new NameProperties(this.panel(), page); + this.documentationProperties = new DocumentationProperties(this.panel(), page); + } + + public async setName(args: { newName: string }) { + await this.nameProperties.setName({ ...args }); + } + + public async getName(): Promise<string> { + return await this.nameProperties.getName(); + } + + public async setDocumentation(args: { newDocumentation: string }) { + await this.documentationProperties.setDocumentation({ ...args }); + } + + public async getDocumentation(): Promise<string> { + return await this.documentationProperties.getDocumentation(); + } + + public async setItemSubjectRef(args: { itemSubjectRef: string }) { + const dataTypeInput = this.panel().locator('input[role="combobox"]').first(); + + await dataTypeInput.click(); Review Comment: Please, use the recommeded Playwright Locator API. For this case `getByRole("checkbox", { name: "<optional name>" })` This could be possible inlined: `this.panel().getByRole("checkbox").first().click();` ########## packages/bpmn-editor/tests-e2e/__fixtures__/propertiesPanel/dataObjectPropertiesPanel.ts: ########## @@ -0,0 +1,73 @@ +/* + * 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 { PropertiesPanelBase } from "./propertiesPanelBase"; +import { Diagram } from "../diagram"; +import { NameProperties } from "./parts/nameProperties"; +import { DocumentationProperties } from "./parts/documentationProperties"; + +export class DataObjectPropertiesPanel extends PropertiesPanelBase { + private nameProperties: NameProperties; + private documentationProperties: DocumentationProperties; + + constructor( + public diagram: Diagram, + public page: Page + ) { + super(diagram, page); + this.nameProperties = new NameProperties(this.panel(), page); + this.documentationProperties = new DocumentationProperties(this.panel(), page); + } + + public async setName(args: { newName: string }) { + await this.nameProperties.setName({ ...args }); + } + + public async getName(): Promise<string> { + return await this.nameProperties.getName(); + } + + public async setDocumentation(args: { newDocumentation: string }) { + await this.documentationProperties.setDocumentation({ ...args }); + } + + public async getDocumentation(): Promise<string> { + return await this.documentationProperties.getDocumentation(); + } + + public async setItemSubjectRef(args: { itemSubjectRef: string }) { + const dataTypeInput = this.panel().locator('input[role="combobox"]').first(); + + await dataTypeInput.click(); + await this.page.keyboard.type(args.itemSubjectRef); + + const createOption = this.page.getByText(`Create Data Type "${args.itemSubjectRef}"`, { exact: true }); + if (await createOption.isVisible().catch(() => false)) { + await createOption.click(); + } else { + await this.page.getByRole("option", { name: args.itemSubjectRef, exact: true }).click(); + } + } + + public async getItemSubjectRef(): Promise<string> { + const dataTypeInput = this.panel().locator('input[role="combobox"]').first(); + return (await dataTypeInput.inputValue()).trim(); Review Comment: Same here. ########## packages/bpmn-editor/tests-e2e/__fixtures__/propertiesPanel/intermediateEventPropertiesPanel.ts: ########## @@ -0,0 +1,245 @@ +/* + * 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 { expect, Page } from "@playwright/test"; +import { PropertiesPanelBase } from "./propertiesPanelBase"; +import { Diagram } from "../diagram"; +import { Nodes } from "../nodes"; +import { NameProperties } from "./parts/nameProperties"; +import { DocumentationProperties } from "./parts/documentationProperties"; + +export class IntermediateEventPropertiesPanel extends PropertiesPanelBase { + private nameProperties: NameProperties; + private documentationProperties: DocumentationProperties; + + constructor( + public diagram: Diagram, + public page: Page, + public nodes: Nodes + ) { + super(diagram, page); + this.nameProperties = new NameProperties(this.panel(), page); + this.documentationProperties = new DocumentationProperties(this.panel(), page); + } + + public async setName(args: { newName: string }) { + await this.nameProperties.setName({ ...args }); + } + + public async getName(): Promise<string> { + return await this.nameProperties.getName(); + } + + public async setDocumentation(args: { newDocumentation: string }) { + await this.documentationProperties.setDocumentation({ ...args }); + } + + public async getDocumentation(): Promise<string> { + return await this.documentationProperties.getDocumentation(); + } + + public async selectEventDefinition(args: { eventType: string }) { + const selectedNode = this.page + .locator(".kie-bpmn-editor--intermediate-catch-event-node, .kie-bpmn-editor--intermediate-throw-event-node") + .first(); + + await expect(selectedNode).toBeVisible({ timeout: 5000 }); + + await this.nodes.morphNode({ + nodeLocator: selectedNode, + targetMorphType: args.eventType, + }); + } + + public async setTimerDefinition(args: { type: "date" | "duration" | "cycle"; value: string }) { + if (args.type === "duration") { + await this.panel().getByLabel("Fire once after duration").click(); + const valueInput = this.panel().locator("#fire-once-input"); Review Comment: We shouldn't test using Element ID. Same as above. If any other locartor is not available, we can introduce `testid`. ########## packages/bpmn-editor/tests-e2e/__fixtures__/propertiesPanel/endEventPropertiesPanel.ts: ########## @@ -0,0 +1,194 @@ +/* + * 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, Locator } from "@playwright/test"; +import { PropertiesPanelBase } from "./propertiesPanelBase"; +import { Diagram } from "../diagram"; +import { Nodes } from "../nodes"; +import { NameProperties } from "./parts/nameProperties"; +import { DocumentationProperties } from "./parts/documentationProperties"; + +export class EndEventPropertiesPanel extends PropertiesPanelBase { + private nameProperties: NameProperties; + private documentationProperties: DocumentationProperties; + + constructor( + public diagram: Diagram, + public page: Page, + public nodes: Nodes + ) { + super(diagram, page); + this.nameProperties = new NameProperties(this.panel(), page); + this.documentationProperties = new DocumentationProperties(this.panel(), page); + } + + private async morphToEventType(args: { endEventLocator: Locator; eventType: string }) { + await this.nodes.morphNode({ + nodeLocator: args.endEventLocator, + targetMorphType: args.eventType, + }); + } + + public async setName(args: { newName: string }) { + await this.nameProperties.setName({ ...args }); + } + + public async getName(): Promise<string> { + return await this.nameProperties.getName(); + } + + public async setDocumentation(args: { newDocumentation: string }) { + await this.documentationProperties.setDocumentation({ ...args }); + } + + public async getDocumentation(): Promise<string> { + return await this.documentationProperties.getDocumentation(); + } + + public async setTerminateDefinition(args: { endEventLocator: Locator }) { + await this.morphToEventType({ endEventLocator: args.endEventLocator, eventType: "Terminate" }); + } + + public async setMessageDefinition(args: { messageName: string; endEventLocator: Locator }) { + await this.morphToEventType({ endEventLocator: args.endEventLocator, eventType: "Message" }); + + const messageInput = this.panel().locator('input[role="combobox"]').first(); + await messageInput.waitFor({ state: "visible", timeout: 10000 }); + await messageInput.click(); Review Comment: Why are you using `waitFor` instead of relying on Playwright auto-wait for this case? You could possibly inline this too. ########## packages/bpmn-editor/tests-e2e/__fixtures__/propertiesPanel/intermediateEventPropertiesPanel.ts: ########## @@ -0,0 +1,245 @@ +/* + * 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 { expect, Page } from "@playwright/test"; +import { PropertiesPanelBase } from "./propertiesPanelBase"; +import { Diagram } from "../diagram"; +import { Nodes } from "../nodes"; +import { NameProperties } from "./parts/nameProperties"; +import { DocumentationProperties } from "./parts/documentationProperties"; + +export class IntermediateEventPropertiesPanel extends PropertiesPanelBase { + private nameProperties: NameProperties; + private documentationProperties: DocumentationProperties; + + constructor( + public diagram: Diagram, + public page: Page, + public nodes: Nodes + ) { + super(diagram, page); + this.nameProperties = new NameProperties(this.panel(), page); + this.documentationProperties = new DocumentationProperties(this.panel(), page); + } + + public async setName(args: { newName: string }) { + await this.nameProperties.setName({ ...args }); + } + + public async getName(): Promise<string> { + return await this.nameProperties.getName(); + } + + public async setDocumentation(args: { newDocumentation: string }) { + await this.documentationProperties.setDocumentation({ ...args }); + } + + public async getDocumentation(): Promise<string> { + return await this.documentationProperties.getDocumentation(); + } + + public async selectEventDefinition(args: { eventType: string }) { + const selectedNode = this.page + .locator(".kie-bpmn-editor--intermediate-catch-event-node, .kie-bpmn-editor--intermediate-throw-event-node") + .first(); Review Comment: Do not rely on CSS classes for selecting elements. They are pretty volatile and changing then shouldn't break any tests. Playwright test should rely on UI. CSS selectors should be used only on last case scenario. If any other locator is not available, we can introduce the `testid`. ########## packages/bpmn-editor/tests-e2e/flowElements/addBoundaryEvent.spec.ts: ########## @@ -0,0 +1,229 @@ +/* + * 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 { NodeType, DefaultNodeName } from "../__fixtures__/nodes"; + +test.beforeEach(async ({ editor }) => { + await editor.open(); +}); + +test.describe("Add Boundary Event", () => { + test.describe("Basic attachment", () => { + test("should attach intermediate catch event to task", async ({ palette, jsonModel, page }) => { + await palette.dragNewNode({ type: NodeType.TASK, targetPosition: { x: 300, y: 300 } }); + await palette.dragNewNode({ type: NodeType.INTERMEDIATE_CATCH_EVENT, targetPosition: { x: 450, y: 300 } }); + + const boundaryEvent = await jsonModel.getFlowElement({ elementIndex: 1 }); + expect(boundaryEvent.__$$element).toBe("boundaryEvent"); + expect(boundaryEvent["@_attachedToRef"]).toBeDefined(); Review Comment: Please, move all `jsonModel` assertions to the end of the tests. ########## packages/bpmn-editor/tests-e2e/flowElements/addBoundaryEvent.spec.ts: ########## @@ -0,0 +1,229 @@ +/* + * 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 { NodeType, DefaultNodeName } from "../__fixtures__/nodes"; + +test.beforeEach(async ({ editor }) => { + await editor.open(); +}); + +test.describe("Add Boundary Event", () => { + test.describe("Basic attachment", () => { + test("should attach intermediate catch event to task", async ({ palette, jsonModel, page }) => { + await palette.dragNewNode({ type: NodeType.TASK, targetPosition: { x: 300, y: 300 } }); + await palette.dragNewNode({ type: NodeType.INTERMEDIATE_CATCH_EVENT, targetPosition: { x: 450, y: 300 } }); + + const boundaryEvent = await jsonModel.getFlowElement({ elementIndex: 1 }); + expect(boundaryEvent.__$$element).toBe("boundaryEvent"); + expect(boundaryEvent["@_attachedToRef"]).toBeDefined(); + + const boundaryEventNode = page.locator(".kie-bpmn-editor--intermediate-catch-event-node").first(); Review Comment: Shouldn't we have a `node` fixture to retrieve the nodes? Also, you're using CSS classes, and this could be inlined, we don't need to create unecessary variables. -- 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]
