Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kie-issues#946: Action plan - Exploratory phase of the strategy for implementing the E2E test suite for the new DMN Editor - Part 3 #2211

Merged
merged 16 commits into from
Apr 11, 2024

Conversation

ljmotta
Copy link
Contributor

@ljmotta ljmotta commented Mar 26, 2024

Related to apache/incubator-kie-issues#946

This PR continues the exploratory phase and continue the work started on #2158

Description

This PR creates introduce and fix a few things:

  • Use the transform over zoom due to Playwright compatibility.
  • Set a y offset to the viewport.
  • Remove variable from Knowledge Source nodes.
  • Add add[Edge]Waypoint tests.
  • Add delete[Edge]Waypoint tests.
  • Add invalid[Edge] tests.
  • Add jsonModel validation for simple cases.

Improvements for the next interation

  • Add Decision Service node tests
  • Add property panel tests
  • Add resize node tests
  • Improve json model fixture and tests

ljmotta and others added 12 commits March 8, 2024 11:28
* Update addAssociationWaypoint.spec.ts

* simplify edges fixture

* report webkit issue and skip current test on it

* authority requirement

* information requirement

* remove .only()

* knowledge requirement
* Split move and add waypoint tests

* refactor edges fixtures
* Introduce delete[edge]Waypoint.spec.ts

* finish deleteEdgeWaypoint.spec.ts

* fix tests after merging with feature branch

* test delete waypoint without its movement

* Update packages/dmn-editor/tests/e2e/drgRequirements/deleteAuthorityRequirementWaypoint.spec.ts

Co-authored-by: Luiz João Motta <[email protected]>

* Update packages/dmn-editor/tests/e2e/drgRequirements/deleteAuthorityRequirementWaypoint.spec.ts

Co-authored-by: Luiz João Motta <[email protected]>

* do not use new line in given phase

* test names simplified and unified

* introduce addWaypoint2 fixture

* addAssociationWaypoint

* use new 'addWaypoint' fixture

---------

Co-authored-by: Luiz João Motta <[email protected]>
* initialize spec file

* invalid edge tests

* remove not needed test

* drop 'From' from test describe

* fix GROUP tests

* fix test describe - remove From

* add dim tests

* refactor dimmed class assertion

* incorporate review feedback

* remove custom assertion

* add dimmed class to the DOM top level lement of the InputData node

* test alternative input data shape
Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found some parts that we need to address probably. Please take a look when you have a moment and also sorry, probably myself is the author of problematic lines.

Comment on lines 36 to 48
export enum DataType {
Undefined = "<Undefined>",
Any = "Any",
Boolean = "boolean",
Context = "context",
Date = "date",
DateTime = "date and time",
DateTimeDuration = "days and time duration",
Number = "number",
String = "string",
Time = "time",
YearsMonthsDuration = "years and months duration",
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have this in __fixtures__/jsonModel.ts

Comment on lines +167 to +171
public async startDraggingEdge(args: { from: string; edgeType: EdgeType }) {
await this.select({ name: args.from, position: NodePosition.TOP });
await this.get({ name: args.from }).getByTitle(this.getAddEdgeTitle(args.edgeType)).hover();
await this.page.mouse.down();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public async startDraggingEdge(args: { from: string; edgeType: EdgeType }) {
await this.select({ name: args.from, position: NodePosition.TOP });
await this.get({ name: args.from }).getByTitle(this.getAddEdgeTitle(args.edgeType)).hover();
await this.page.mouse.down();
}
/**
* Used for testing invalid edges. No edge is created using this method.
*/
public async startDraggingEdge(args: { from: string; edgeType: EdgeType }) {
await this.select({ name: args.from, position: NodePosition.TOP });
await this.get({ name: args.from }).getByTitle(this.getAddEdgeTitle(args.edgeType)).hover();
await this.page.mouse.down();
}

});
});

test("shouldn't add an Authority Requirement edge from Input Data node to Input Data node", async ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test("shouldn't add an Authority Requirement edge from Input Data node to Input Data node", async ({
test("shouldn't add an Authority Requirement edge from Knowledge Source node to Input Data node", async ({

await expect(nodes.get({ name: DefaultNodeName.INPUT_DATA })).toHaveClass(/.*dimmed/);
});

test("shouldn't add an Authority Requirement edge from Input Data node to Decision Service node", async ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test("shouldn't add an Authority Requirement edge from Input Data node to Decision Service node", async ({
test("shouldn't add an Authority Requirement edge from Knowledge Source node to Decision Service node", async ({

await expect(nodes.get({ name: DefaultNodeName.DECISION_SERVICE })).toHaveClass(/.*dimmed/);
});

test("shouldn't add an Authority Requirement edge from Input Data node to Group node", async ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test("shouldn't add an Authority Requirement edge from Input Data node to Group node", async ({
test("shouldn't add an Authority Requirement edge from Knowledge Source node to Group node", async ({

await expect(nodes.get({ name: DefaultNodeName.GROUP })).toHaveClass(/.*dimmed/);
});

test("shouldn't add an Authority Requirement edge from Input Data node to Text Annotation node", async ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test("shouldn't add an Authority Requirement edge from Input Data node to Text Annotation node", async ({
test("shouldn't add an Authority Requirement edge from Knowledge Source node to Text Annotation node", async ({

});
});

test("shouldn't add an Knowledge Requirement edge from Input Data node to Input Data node", async ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test("shouldn't add an Knowledge Requirement edge from Input Data node to Input Data node", async ({
test("shouldn't add an Knowledge Requirement edge from BKM node to Input Data node", async ({

await expect(nodes.get({ name: DefaultNodeName.INPUT_DATA })).toHaveClass(/.*dimmed/);
});

test("shouldn't add an Knowledge Requirement edge from Input Data node to Knowledge Source node", async ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test("shouldn't add an Knowledge Requirement edge from Input Data node to Knowledge Source node", async ({
test("shouldn't add an Knowledge Requirement edge from BKM node to Knowledge Source node", async ({

await expect(nodes.get({ name: DefaultNodeName.KNOWLEDGE_SOURCE })).toHaveClass(/.*dimmed/);
});

test("shouldn't add an Knowledge Requirement edge from Input Data node to Group node", async ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test("shouldn't add an Knowledge Requirement edge from Input Data node to Group node", async ({
test("shouldn't add an Knowledge Requirement edge from BKM node to Group node", async ({

await expect(nodes.get({ name: DefaultNodeName.GROUP })).toHaveClass(/.*dimmed/);
});

test("shouldn't add an Knowledge Requirement edge from Input Data node to Text Annotation node", async ({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test("shouldn't add an Knowledge Requirement edge from Input Data node to Text Annotation node", async ({
test("shouldn't add an Knowledge Requirement edge from BKM node to Text Annotation node", async ({

Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for updates

@tiagobento
Copy link
Contributor

@ljmotta Looking at the jsonModel.ts fixture you introduced, I have a couple of thoughts:

  1. The wait mechanism is flawed, we can do better with a new optional prop called onModelDebounceStateChanged only render the JSON once debounce is completed.
  2. The drgElements.ts file has too much logic. IMHO we shouldn't be abstracting direct access to the JSON model itself. IMHO it's alright to use hard-coded indexes to find stuff in the array, as we expect that the JSON model will be stable.
  3. The strategy of using the DOM to make the JSON available to the tests is a clever hack, well done.

@tiagobento
Copy link
Contributor

@ljmotta I'm having the exact same problem at #2230, I'll fix it there and this PR should go back to working normally.

@tiagobento
Copy link
Contributor

#2230 merged. I already re-triggered the CI here.

@tiagobento tiagobento merged commit 8a8d331 into apache:main Apr 11, 2024
8 checks passed
fantonangeli pushed a commit to fantonangeli/kie-tools that referenced this pull request Apr 23, 2024
…mplementing the E2E test suite for the new DMN Editor - Part 3 (apache#2211)

Co-authored-by: Jozef Marko <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants