Skip to content

Conversation

@MattHandzel
Copy link

@MattHandzel MattHandzel commented Dec 9, 2025

The "renders context menu item" test was flaky due to two root causes:

  1. Incorrect waitFor() usage: The test had an async callback that executed user.pointer() and immediately checked the assertion without waiting for the DOM to update. The waitFor() callback should be synchronous.

  2. Missing PropertyGrid load wait: The test attempted to interact with the PropertyGrid before it finished loading, causing the "Test SpatialCategory" element to not be found.

Changes:

  • Enable the test
  • Added this.timeout(10000) to increase test timeout to 10 seconds
  • Added explicit waitFor() with 5-second timeout to wait for PropertyGrid to load and display the category before interaction
  • Separated user interaction (pointer event) from assertion
  • Made the assertion callback synchronous to properly wait for context menu

Verification:

  • Ran this test 100 times locally without failures
  • No regressions in other tests

@MattHandzel MattHandzel requested review from a team as code owners December 9, 2025 20:42
@CLAassistant
Copy link

CLAassistant commented Dec 9, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@grigasp grigasp left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, looks good.

@ben-polinsky
Copy link
Contributor

@MattHandzel thanks - could you run prettier --write to correct formatting issues?

[warn] apps/learning-snippets/src/test/property-grid/ContextMenuItem.test.tsx
[warn] Code style issues found in the above file. Run Prettier with --write to fix.

@MattHandzel
Copy link
Author

MattHandzel commented Dec 16, 2025

@MattHandzel thanks - could you run prettier --write to correct formatting issues?

Hello @ben-polinsky, I ran prettier, not sure if you get notified when I make a commit or if I should write you with a comment like this, but it should be ready to be merged :)

@grigasp
Copy link
Member

grigasp commented Dec 17, 2025

@MattHandzel thanks - could you run prettier --write to correct formatting issues?

Hello @ben-polinsky, I ran prettier, not sure if you get notified when I make a commit or if I should write you with a comment like this, but it should be ready to be merged :)

Thanks. I didn't get notified until you commented. Re-ran the pipelines and prettier is still complaining:

> viewer-components-react@ prettier /home/runner/work/viewer-components-react/viewer-components-react
> prettier --check .

Checking formatting...
[warn] apps/learning-snippets/src/test/property-grid/ContextMenuItem.test.tsx
[warn] Code style issues found in the above file. Run Prettier with --write to fix.

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.

4 participants