Skip to content

Conversation

@mgadewoll
Copy link
Contributor

@mgadewoll mgadewoll commented Jun 10, 2025

Summary

This PR updates the data_grid_cell.styles.spec.tsx cypress test which would fail when run locally only.
It looks like the call to getComputedStyle that gets the style information might happen too early. Adding a wait() resolves the issue.

Note

The checks to styles should probably not happen in a cypress test as we're rather verifying functionality in those.
We should consider migrating those tests to VRT instead.

QA

Checkout this PR and run cypress tests locally in eui/packages/eui: yarn test-cypress-dev and choose data_grid_cell.styles.spec.tsx to run.

  • verify no tests fail
    • (optional) compare the behavior when reverting the PR changes - the focus trap tests would fail.

- it looks like the call to getComputedStyle happens when the style is not updated yet, adding a wait() here returns expected behavior
@mgadewoll mgadewoll self-assigned this Jun 10, 2025
@mgadewoll mgadewoll added the skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) label Jun 10, 2025
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

cc @mgadewoll

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

cc @mgadewoll

@mgadewoll mgadewoll marked this pull request as ready for review June 10, 2025 17:59
@mgadewoll mgadewoll requested a review from a team as a code owner June 10, 2025 17:59
@acstll acstll self-requested a review June 11, 2025 07:21
Copy link
Contributor

@acstll acstll left a comment

Choose a reason for hiding this comment

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

Ran locally 3 times (a magic number), passed. Reverted, failed.

Thank you very much!

@mgadewoll mgadewoll merged commit c4079af into elastic:main Jun 11, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants