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

test: add test for Flow issue #19006 #6164

Merged
merged 7 commits into from
Apr 4, 2024
Merged

Conversation

tepi
Copy link
Contributor

@tepi tepi commented Mar 28, 2024

grid.setPageSize(10);
grid.addHierarchyColumn(String::toString).setHeader("Item");

var data = new TreeData<String>();
Copy link
Contributor

@vursen vursen Mar 28, 2024

Choose a reason for hiding this comment

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

I'm inclined to recommend this alternative test setup – it reproduces the issue in a way that is less likely to be affected by future refactorings:

  1. Expand All
  2. Scroll to 0-250
  3. Scroll to start
TreeGrid grid = new TreeGrid<String>();
grid.setPageSize(10);
grid.addHierarchyColumn(String::toString).setHeader("Item");

var data = new TreeData<String>();

for (int i = 0; i < 500; i++) {
    data.addRootItems("Item-" + i);
}

for (int i = 0; i < 500; i++) {
    data.addItems("Item-0", "Item-0-" + i);
}

for (int j = 0; j < 500; j++) {
    data.addItems("Item-0-0", "Item-0-0-" + j);
}

grid.setDataProvider(new TreeDataProvider<>(data));

add(grid);

add(new NativeButton("Expand all",
        e -> grid.expandRecursively(data.getRootItems(), 1)));

add(new NativeButton("Scroll to 0-250",
        e -> grid.scrollToIndex(0, 250)));

add(new NativeButton("Scroll to start",
        e -> grid.scrollToStart()));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified the test as recommended. Also added id checks in every step of the test.

@tepi tepi requested a review from vursen April 3, 2024 10:15
Copy link
Contributor

@ugur-vaadin ugur-vaadin left a comment

Choose a reason for hiding this comment

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

LGTM apart from the unused imports.

tepi and others added 7 commits April 4, 2024 17:04
…/main/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysPage.java
…/main/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysPage.java
…/main/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysPage.java
…/test/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysIT.java
…/test/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysIT.java
@ugur-vaadin ugur-vaadin force-pushed the add-test-for-flow-issue-19006 branch from 9dfd13c to 2fb5999 Compare April 4, 2024 14:04
@ugur-vaadin ugur-vaadin enabled auto-merge (squash) April 4, 2024 14:04
Copy link

sonarcloud bot commented Apr 4, 2024

Quality Gate Passed Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@ugur-vaadin ugur-vaadin merged commit fcbc2ee into main Apr 4, 2024
5 checks passed
@ugur-vaadin ugur-vaadin deleted the add-test-for-flow-issue-19006 branch April 4, 2024 14:22
vaadin-bot pushed a commit that referenced this pull request Apr 4, 2024
* test: add test for Flow issue #19006

* change test as recommended in review

* Update vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysPage.java

* Update vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysPage.java

* Update vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysPage.java

* Update vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysIT.java

* Update vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysIT.java

---------

Co-authored-by: Ugur Saglam <106508695+ugur-vaadin@users.noreply.github.com>
tepi added a commit that referenced this pull request Apr 5, 2024
* test: add test for Flow issue #19006

* change test as recommended in review

* Update vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysPage.java

* Update vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysPage.java

* Update vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysPage.java

* Update vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysIT.java

* Update vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/treegrid/it/TreeGridExpandedKeysIT.java

---------

Co-authored-by: Teppo Kurki <teppo.kurki@vaadin.com>
Co-authored-by: Ugur Saglam <106508695+ugur-vaadin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants