Skip to content

Conversation

amanmahajan7
Copy link
Collaborator

@amanmahajan7 amanmahajan7 commented Sep 23, 2025

https://vitest.dev/guide/browser/assertion-api.html#assertion-api

We recommend to always use expect.element when working with page.getBy* locators to reduce test flakiness.

https://vitest.dev/guide/browser/locators.html#nth

Before resorting to nth, you may find it useful to use chained locators to narrow down your search. Sometimes there is no better way to distinguish than by element position; although this can lead to flake, it's better than nothing.

I think we should find cell/headercell/row using name instead of relying on index when possible

@amanmahajan7 amanmahajan7 self-assigned this Sep 23, 2025
@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.84%. Comparing base (8b46566) to head (f90009c).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3871   +/-   ##
=======================================
  Coverage   97.84%   97.84%           
=======================================
  Files          42       42           
  Lines        1253     1253           
  Branches      348      348           
=======================================
  Hits         1226     1226           
  Misses         27       27           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@amanmahajan7 amanmahajan7 changed the title Rewrite some tests to reduce flakiness Rewrite some tests to reduce flakiness: part 1 Sep 23, 2025
@amanmahajan7 amanmahajan7 marked this pull request as ready for review September 23, 2025 18:57
await toggleSelection(1);
testSelection(1, false);
await testSelection(0, true);
await toggleSelection(1, false, true); // force click even if disabled
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

forcing click causes flaky test in FF.

await page.mouse.up();
};

const scrollGrid: BrowserCommand<[{ scrollLeft?: number; scrollTop?: number }]> = async (
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

await userEvent.keyboard('abc');

await scrollGrid({ scrollTop: 1500 });
expect(getCellsAtRowIndex(40)[1]).toHaveTextContent(/^40$/);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For some reason current scrolling logic is flaky in FF
https://github.com/Comcast/react-data-grid/actions/runs/17952854675/job/51056628087

Hopefully the new commands.scrollGrid will help

@amanmahajan7 amanmahajan7 marked this pull request as draft September 24, 2025 02:55
@amanmahajan7 amanmahajan7 marked this pull request as ready for review September 24, 2025 15:52

await scrollGrid({ scrollTop: 1500 });
expect(getCellsAtRowIndex(40)[1]).toHaveTextContent(/^40$/);
await userEvent.click(getCellsAtRowIndex(40)[1]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This can still fail when the grid is properly scrolled. Using locators fixes it

@amanmahajan7 amanmahajan7 merged commit 20d6730 into main Sep 26, 2025
2 checks passed
@amanmahajan7 amanmahajan7 deleted the fix-flaky branch September 26, 2025 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants