Repository navigation
Flaky tests: fix router navigate URL fragment race - #80559
CoderAbhinav wants to merge 1 commit into
Conversation
The test "should update the URL from the last navigation if only varies in the URL fragment" could release a navigation's response before both requests had reached the route handler. Both navigations target the same page path (they only differ in the URL fragment, which is stripped when building the cache key), so both fetches go through a single route handler and their resolvers are collected in a shared array. Asserting the pending navigations counter is `2` only guarantees both navigate actions have started; the actual fetches happen later (after a dynamic import). As a result, `resolvers.pop()` could release the first navigation's request instead of the last one's. That earlier navigation bails out because its destination is no longer the current one, so the title stays "Main" and the assertion for "Link 1" times out. Wait for both requests to reach the route handler before releasing any of them, mirroring the fix applied to the sibling test in WordPress#80178. Reported issues: - Fixes WordPress#80447 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @CoderAbhinav! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
What
Fixes #80447.
This fixes a race in the router navigation E2E test
should update the URL from the last navigation if only varies in the URL fragment. The test could fail intermittently with the title stuck on"Main"instead of updating to"Link 1":Why
Both navigations in this test target the same page path — they only differ in the URL fragment, which is stripped when building the router's cache key (
getPagePath). As a result, both requests go through the samepage.route()handler and their resolvers are collected in a sharedresolversarray.Asserting that the pending navigations counter is
2only guarantees bothnavigateactions have started (the counter is incremented synchronously in the view action). The actualfetch()calls happen later, inside the router action after a dynamicimport(). Soresolvers.pop()could release the first navigation's request instead of the last one's, before the second request had even reached the route handler.That earlier navigation then bails out — its destination is no longer the current one (
if ( navigatingTo !== href ) return;) — so nothing re-renders, the title stays"Main", and the assertion for"Link 1"times out.This is the same class of race that was fixed for the sibling test in this file in #80178.
How
Wait until both requests have reached the route handler before releasing any of them, using a
bothRequestsbarrier that resolves once two resolvers have been collected. Releasing responses only after both requests are intercepted makes the ordering deterministic: the last navigation (the one with the hash) is released first and wins, then the earlier one is released and correctly bails out without replacing the latest HTML.No production code changes — the router already handles this correctly at runtime; only the test's timing assumptions were unsound.
Testing Instructions
npm run test:e2e -- test/e2e/specs/interactivity/router-navigate.spec.ts -g "should update the URL from the last navigation if only varies in the URL fragment" --repeat-each=30npm run test:e2e -- test/e2e/specs/interactivity/router-navigate.spec.ts