From 9b00201b565986ad35d7578d9018a7735c686365 Mon Sep 17 00:00:00 2001 From: web-padawan Date: Fri, 17 Jul 2026 19:35:32 +0300 Subject: [PATCH 1/2] test: re-enable undefined-size grid scroll regression checks Re-enable the disabled steps in the ItemCountUnknownGridIT scroll-to-end test and drop the stale #1038 / #9166 FIXME. The grid no longer fetches extra earlier ranges after the item count shrinks from an estimate to the real size, so assert that scrolling around after the size is adjusted keeps the size and does not trigger unnecessary fetches. Part of #9166 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../dataview/AbstractItemCountGridIT.java | 14 +++++++++---- .../grid/dataview/ItemCountUnknownGridIT.java | 20 +++++++++++++++---- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/AbstractItemCountGridIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/AbstractItemCountGridIT.java index c8478e8616a..95aae8aff8a 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/AbstractItemCountGridIT.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/AbstractItemCountGridIT.java @@ -56,10 +56,12 @@ protected void open(int size) { protected void doScroll(int rowToScroll, int expectedRows, int fetchIndex, int start, int end) { grid.scrollToRow(rowToScroll); - // FIXME when grid reduces size, it does currently some extra fetches - // -> not checking the requested items until this is fixed - // verifyFetchForUndefinedSizeCallback(fetchIndex, - // Range.between(start, end)); + // Note: the per-scroll fetch-range assertion + // (verifyFetchForUndefinedSizeCallback) is intentionally not used here. + // The grid fetches page by page, so a single scroll may trigger several + // fetches, which the one-range-per-scroll fetchIndex model cannot + // express. The fetchIndex/start/end parameters are kept for readability + // of the intended range. Only the resulting row count is asserted. verifyRows(expectedRows); } @@ -100,6 +102,10 @@ protected void verifyRows(int size) { grid.getRowCount()); } + protected int getFetchQueryCount() { + return findElements(By.cssSelector("[id^='log-']")).size(); + } + protected void verifyFetchForUndefinedSizeCallback(int index, Range range) { WebElement log = findElement(By.id("log-" + index)); Assert.assertEquals("Invalid range for index " + index, diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/ItemCountUnknownGridIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/ItemCountUnknownGridIT.java index 21b0e1c5938..e1c28314b04 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/ItemCountUnknownGridIT.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/ItemCountUnknownGridIT.java @@ -48,10 +48,22 @@ public void undefinedSizeGrid_defaultPageSizeEvenToDatasetSize_scrollingToEnd() // scroll to actual end, no more items returned and size is adjusted doScroll(500, 500, 5, 450, 500); Assert.assertEquals(499, grid.getLastVisibleRowIndex()); - // TODO #1038 test further after grid is note fetching extra stuff when - // size has been adjusted to less than what it is - // doScroll(0, 500, 6, 0, 100); - // doScroll(450, 500, 7, 400, 500); + + // After the size was adjusted down from the estimate, scrolling back + // and forth must keep the adjusted size and must not make the grid + // fetch extra earlier ranges (regression test for #1038 / flow #9166). + int fetchCountAfterEnd = getFetchQueryCount(); + doScroll(0, 500, 6, 0, 100); + doScroll(450, 500, 7, 400, 500); + // Scrolling within the already-known size fetches at most the two + // visited viewports (top and around row 450), never a storm of + // earlier-range refetches. Observed delta is 4; a small margin guards + // against viewport/row-height variance while still catching the + // pre-fix behavior, which refetched many earlier ranges. + Assert.assertTrue( + "Scrolling after the size was adjusted must not trigger " + + "unnecessary fetches for earlier ranges", + getFetchQueryCount() - fetchCountAfterEnd <= 6); } @Test From cc54434c27dd2003c27dca7ad030deb5111ad489 Mon Sep 17 00:00:00 2001 From: web-padawan Date: Fri, 17 Jul 2026 19:50:08 +0300 Subject: [PATCH 2/2] test: assert scroll-back fetches only hit visited viewports Replace the total fetch-count ceiling with a check that scrolling after the size is adjusted only fetches the visited viewports, never the ranges in between. This catches an off-viewport refetch regardless of count and avoids flakiness from viewport variance. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../dataview/AbstractItemCountGridIT.java | 18 +++++++++----- .../grid/dataview/ItemCountUnknownGridIT.java | 24 +++++++++---------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/AbstractItemCountGridIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/AbstractItemCountGridIT.java index 95aae8aff8a..95178f13a6f 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/AbstractItemCountGridIT.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/AbstractItemCountGridIT.java @@ -56,12 +56,8 @@ protected void open(int size) { protected void doScroll(int rowToScroll, int expectedRows, int fetchIndex, int start, int end) { grid.scrollToRow(rowToScroll); - // Note: the per-scroll fetch-range assertion - // (verifyFetchForUndefinedSizeCallback) is intentionally not used here. - // The grid fetches page by page, so a single scroll may trigger several - // fetches, which the one-range-per-scroll fetchIndex model cannot - // express. The fetchIndex/start/end parameters are kept for readability - // of the intended range. Only the resulting row count is asserted. + // A single scroll may fetch several pages, so only the resulting row + // count is asserted here. fetchIndex/start/end document the range. verifyRows(expectedRows); } @@ -106,6 +102,16 @@ protected int getFetchQueryCount() { return findElements(By.cssSelector("[id^='log-']")).size(); } + /** + * Returns the start offset of the fetch query logged at the given index. + * Log entries are formatted as {@code ":Range [..]"}. + */ + protected int getFetchedOffset(int index) { + String text = findElement(By.id("log-" + index)).getText(); + return Integer.parseInt( + text.substring(text.indexOf('[') + 1, text.indexOf(".."))); + } + protected void verifyFetchForUndefinedSizeCallback(int index, Range range) { WebElement log = findElement(By.id("log-" + index)); Assert.assertEquals("Invalid range for index " + index, diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/ItemCountUnknownGridIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/ItemCountUnknownGridIT.java index e1c28314b04..31ca71eb7e0 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/ItemCountUnknownGridIT.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/dataview/ItemCountUnknownGridIT.java @@ -49,21 +49,19 @@ public void undefinedSizeGrid_defaultPageSizeEvenToDatasetSize_scrollingToEnd() doScroll(500, 500, 5, 450, 500); Assert.assertEquals(499, grid.getLastVisibleRowIndex()); - // After the size was adjusted down from the estimate, scrolling back - // and forth must keep the adjusted size and must not make the grid - // fetch extra earlier ranges (regression test for #1038 / flow #9166). - int fetchCountAfterEnd = getFetchQueryCount(); + // After the size is adjusted down, scrolling back and forth keeps the + // size and only fetches the visited viewports (near the top and near + // row 450) - never the ranges in between. + int fetchesBefore = getFetchQueryCount(); doScroll(0, 500, 6, 0, 100); doScroll(450, 500, 7, 400, 500); - // Scrolling within the already-known size fetches at most the two - // visited viewports (top and around row 450), never a storm of - // earlier-range refetches. Observed delta is 4; a small margin guards - // against viewport/row-height variance while still catching the - // pre-fix behavior, which refetched many earlier ranges. - Assert.assertTrue( - "Scrolling after the size was adjusted must not trigger " - + "unnecessary fetches for earlier ranges", - getFetchQueryCount() - fetchCountAfterEnd <= 6); + for (int i = fetchesBefore; i < getFetchQueryCount(); i++) { + int offset = getFetchedOffset(i); + Assert.assertTrue( + "Refetched an off-viewport range at offset " + offset + + " after the size was adjusted", + offset < 150 || offset >= 350); + } } @Test