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.

Sign up for GitHub

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

Jump to bottom

chore: fix flaky test on CI #8756

Merged
merged 1 commit into from
Feb 24, 2025
Merged

chore: fix flaky test on CI #8756

merged 1 commit into from
Feb 24, 2025

Conversation

stipsan
Copy link
Member

@stipsan stipsan commented Feb 24, 2025 •
edited
Loading

Description

Fixes a flaky test, that fails on almost every single PR.
The core issue were that the hydrateRoot instance creates a React render tree that continues to live on after the test is tore down.
And in some cases it would try to re-render the <Studio /> test after teardown, at which point globals like window were no longer there and thus created this confusing error on builds:

⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯

Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯ Uncaught Exception ⎯⎯⎯⎯⎯

ReferenceError: window is not defined

This error originated in "src/core/studio/Studio.test.tsx" test file. It doesn't mean the error was thrown inside the file itself, but while it was running.
This error was caught after test environment was torn down. Make sure to cancel any running tasks before test finishes:
- cancel timeouts using clearTimeout and clearInterval
- wait for promises to resolve using the await keyword

There's both the case that act were missing an await, as well as no root.unmount() which lets react cleanup the test.
When using @testing-library/react, this cleanup happens automatically. However in this particular test we were calling hydrateRoot manually and thus have to do the cleanup ourselves.

What to review

Does it make sense?

Testing

If the CI passes every time, instead of some of the time, then we're good 👍

Notes for release

N/A

@vercel Vercel
Copy link

vercel bot commented Feb 24, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
page-building-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 24, 2025 9:06pm
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 24, 2025 9:06pm
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 24, 2025 9:06pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Feb 24, 2025 9:06pm
test-next-studio ⬜️ Ignored (Inspect) Feb 24, 2025 9:06pm

@github-actions GitHub Actions
Copy link
Contributor

No changes to documentation

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 24, 2025 •
edited
Loading

⚡️ Editor Performance Report

Updated Mon, 24 Feb 2025 21:11:49 GMT

Benchmark reference
latency of sanity@latest		 experiment
latency of this branch		 Δ (%)
latency difference
article (title) 22.2 efps (45ms) 24.4 efps (41ms) -4ms (-8.9%) ✅
article (body) 67.1 efps (15ms) 71.9 efps (14ms) -1ms (-/-%) ✅
article (string inside object) 25.0 efps (40ms) 23.8 efps (42ms) +2ms (+5.0%) ✅
article (string inside array) 22.2 efps (45ms) 22.2 efps (45ms) +0ms (-/-%) ✅
recipe (name) 40.0 efps (25ms) 50.0 efps (20ms) -5ms (-20.0%) ✅
recipe (description) 45.5 efps (22ms) 52.6 efps (19ms) -3ms (-13.6%) ✅
recipe (instructions) 99.9+ efps (6ms) 99.9+ efps (6ms) -0ms (-/-%) ✅
synthetic (title) 19.6 efps (51ms) 19.6 efps (51ms) +0ms (-/-%) ✅
synthetic (string inside object) 18.7 efps (54ms) 19.2 efps (52ms) -2ms (-2.8%) ✅

efps — editor "frames per second". The number of updates assumed to be possible within a second.

Derived from input latency. efps = 1000 / input_latency

Detailed information

🏠 Reference result

The performance result of sanity@latest

Benchmark latency p75 p90 p99 blocking time test duration
article (title) 45ms 71ms 81ms 525ms 1007ms 12.1s
article (body) 15ms 17ms 29ms 78ms 182ms 5.8s
article (string inside object) 40ms 45ms 51ms 225ms 270ms 7.5s
article (string inside array) 45ms 47ms 50ms 148ms 322ms 7.5s
recipe (name) 25ms 27ms 31ms 60ms 0ms 7.9s
recipe (description) 22ms 23ms 26ms 42ms 0ms 5.2s
recipe (instructions) 6ms 9ms 11ms 37ms 0ms 3.4s
synthetic (title) 51ms 53ms 57ms 128ms 745ms 17.8s
synthetic (string inside object) 54ms 58ms 62ms 437ms 1238ms 8.6s

🧪 Experiment result

The performance result of this branch

Benchmark latency p75 p90 p99 blocking time test duration
article (title) 41ms 44ms 71ms 466ms 914ms 11.1s
article (body) 14ms 17ms 24ms 256ms 382ms 5.7s
article (string inside object) 42ms 46ms 54ms 243ms 327ms 7.5s
article (string inside array) 45ms 48ms 50ms 151ms 246ms 7.4s
recipe (name) 20ms 22ms 24ms 43ms 0ms 7.0s
recipe (description) 19ms 20ms 22ms 44ms 0ms 4.7s
recipe (instructions) 6ms 8ms 10ms 24ms 0ms 3.3s
synthetic (title) 51ms 54ms 61ms 351ms 963ms 12.5s
synthetic (string inside object) 52ms 56ms 74ms 281ms 813ms 8.2s

📚 Glossary

column definitions

  • benchmark — the name of the test, e.g. "article", followed by the label of the field being measured, e.g. "(title)".
  • latency — the time between when a key was pressed and when it was rendered. derived from a set of samples. the median (p50) is shown to show the most common latency.
  • p75 — the 75th percentile of the input latency in the test run. 75% of the sampled inputs in this benchmark were processed faster than this value. this provides insight into the upper range of typical performance.
  • p90 — the 90th percentile of the input latency in the test run. 90% of the sampled inputs were faster than this. this metric helps identify slower interactions that occurred less frequently during the benchmark.
  • p99 — the 99th percentile of the input latency in the test run. only 1% of sampled inputs were slower than this. this represents the worst-case scenarios encountered during the benchmark, useful for identifying potential performance outliers.
  • blocking time — the total time during which the main thread was blocked, preventing user input and UI updates. this metric helps identify performance bottlenecks that may cause the interface to feel unresponsive.
  • test duration — how long the test run took to complete.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 24, 2025 •
edited
Loading

Component Testing Report Updated Feb 24, 2025 9:20 PM (UTC)

❌ Failed Tests (3) -- expand for details
File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 1m 24s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 14s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ❌ Failed (Inspect) 1m 22s 5 0 1
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 57s 11 7 0
formBuilder/inputs/PortableText/copyPaste/CopyPasteFields.spec.tsx ✅ Passed (Inspect) 0s 0 12 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 30s 6 0 0
formBuilder/inputs/PortableText/DisableFocusAndUnset.spec.tsx ✅ Passed (Inspect) 17s 3 0 0
formBuilder/inputs/PortableText/DragAndDrop.spec.tsx ✅ Passed (Inspect) 31s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ❌ Failed (Inspect) 1m 57s 14 0 1
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 57s 21 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ❌ Failed (Inspect) 2m 42s 20 0 1
formBuilder/inputs/PortableText/PresenceCursors.spec.tsx ✅ Passed (Inspect) 15s 3 9 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 31s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 2m 0s 21 0 0
formBuilder/tree-editing/TreeEditing.spec.tsx ✅ Passed (Inspect) 0s 0 3 0
formBuilder/tree-editing/TreeEditingNestedObjects.spec.tsx ✅ Passed (Inspect) 0s 0 3 0

@stipsan stipsan force-pushed the fix-flaky-studio-test branch 2 times, most recently from ad91f00 to f94f661 Compare February 24, 2025 20:35
@stipsan stipsan marked this pull request as ready for review February 24, 2025 20:41
@stipsan stipsan requested a review from a team as a code owner February 24, 2025 20:41
@stipsan stipsan requested review from cngonzalez and removed request for a team February 24, 2025 20:41
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 24, 2025 •
edited
Loading

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 42.42% 54334 / 128061
🔵 Statements 42.42% 54334 / 128061
🔵 Functions 47.3% 2726 / 5763
🔵 Branches 79.42% 10254 / 12910
File CoverageNo changed files found.
Generated in workflow #31021 for commit 054c65b by the Vitest Coverage Report Action

@stipsan stipsan enabled auto-merge (squash) February 24, 2025 20:45
@stipsan stipsan force-pushed the fix-flaky-studio-test branch from f94f661 to 054c65b Compare February 24, 2025 20:58
bjoerge
bjoerge approved these changes Feb 24, 2025
View reviewed changes
Copy link
Member

@bjoerge bjoerge left a comment

Choose a reason for hiding this comment

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

Great find, much appreciated <3

@stipsan stipsan merged commit a143928 into next Feb 24, 2025
62 checks passed
@stipsan stipsan deleted the fix-flaky-studio-test branch February 24, 2025 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjoerge bjoerge bjoerge approved these changes

@cngonzalez cngonzalez Awaiting requested review from cngonzalez cngonzalez is a code owner automatically assigned from sanity-io/studio

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stipsan @bjoerge