Skip to content

Retry the yarn-classic corepack probe in e2e tests - #1673

Merged
Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
ci-perf/1671-yarn-classic-probe-retry
Oct 23, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 1 commit into
mainfrom
ci-perf/1671-yarn-classic-probe-retry

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #1671

Problem

On 2026-10-22, merge-group run 39840895419 failed in yarn-classic 1.22.22. Two tests in e2e_redirect_yarn_classic_build panicked because corepack yarn@1.22.22 --version failed once, during a DNS timeout on the Depot host (lookup cache.depot.dev ... i/o timeout 90 s earlier). require_yarn_classic probed only once, and SOCKET_PATCH_YARN_E2E_REQUIRED=1 turns a failed probe into a panic. One blip:

Change

crates/socket-patch-cli/tests/common/yarn_classic_vex.rs, require_yarn_classic:

  • Retries the probe after 2 s and then 5 s, so 3 attempts in total, before it counts the release as unavailable.
  • Prints each failed attempt's exit status and stderr. Before this change, stderr went to Stdio::null(). The SKIP/panic message now includes the last attempt's error.
  • Does not retry when the corepack binary can't be spawned, because retrying can't help.
  • A wrong release still panics on the first attempt.

I left out the script warm-up the issue also proposed. Each suite uses its own sandbox COREPACK_HOME, and the retry alone covers the observed failure. I didn't change the yarn-berry, pnpm or rush probes, to keep this PR to one topic.

Expected saving

At ~0.5 hits/day (n=1 so far), about 15 merge-queue min/day amortized on the queue's critical path and ~50 Linux job-min/day. A green run costs nothing extra: the retry only runs after a probe fails. PR feedback latency and macOS minutes don't change.

Measured result

  • Locally, happy path: SOCKET_PATCH_YARN_E2E_REQUIRED=1 cargo test -p socket-patch-cli --test e2e_redirect_yarn_classic_build passed 40/40 in 22.4 s, the same as before the change.
  • Locally, failure path: an unreachable registry (COREPACK_NPM_REGISTRY=http://127.0.0.1:9) with an uncached release (1.22.19). Each probing test logged 3 failed attempts with corepack's stderr, then still panicked under REQUIRED=1. That adds ~7 s per probe to a leg that is already failing.
  • CI (this PR, head 28a5f04): all 54 checks passed. ci-ok and clippy are green. yarn-classic 1.22.22 took 1.87 min (20:41:30 → 20:43:22), within the baseline (p50 2.15 / p90 3.34 min, n=55), so the happy path is no slower. The saving comes from avoided merge-queue evictions, which show up only after merge. The profiler will check the eviction rate.

Where each test runs

All tests run where they ran before. No test moved, and no assertion was weakened. A release that really can't be fetched still fails the REQUIRED leg.

Risk

Low. The change touches only test-support code. Required checks ci-ok and clippy are unchanged.

https://claude.ai/code/session_016RS4Qh9aCHhzeMGKNok1oN


Generated by Claude Code

require_yarn_classic probed `corepack yarn@<release> --version` once
and, under SOCKET_PATCH_YARN_E2E_REQUIRED=1, panicked on any failure.
A single DNS timeout on a Depot host therefore failed the
yarn-classic 1.22.22 merge-queue leg on 2026-10-22, evicted #1653 and
forced the green group behind it to rebuild (~32 queue-min, ~94
job-min).

Retry the probe after 2 s and 5 s before treating the release as
unavailable, and keep the last attempt's stderr in the SKIP/panic
message instead of discarding it. A missing corepack binary is not
retried, a wrong release still panics at once, and a release that
stays unfetchable still fails the REQUIRED leg.

Refs #1671

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mikolalysenko Mikola Lysenko (mikolalysenko) added the ci-perf CI / merge-queue performance finding (profiler routine) label Oct 22, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 28a5f04. Configure here.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 22, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at 28a5f04.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. The yarn-classic e2e suites probe corepack yarn@1.22.22 --version before they run. This PR retries that probe up to 3 times (waits of 0 s, 2 s, then 5 s), so one DNS blip on a CI host no longer fails the REQUIRED=1 leg and evicts the merge-queue entry (#1671). Each failed attempt now prints corepack's stderr, which used to be discarded.

Risk: low. Only test-support code changes (1 file, +39/−23). The behavior that matters is kept: a wrong yarn version still panics on the first attempt, and a release that really can't be fetched still fails the REQUIRED leg, about 7 s later than before.

Look here

  • yarn_classic_vex.rs:124:`` the retry loop and its backoff schedule
  • yarn_classic_vex.rs:133:``` if the corepack binary can't be spawned, it stops right away with no retry
  • yarn_classic_vex.rs:146:```` a version mismatch still panics without retrying
  • yarn_classic_vex.rs:156:````` the final skip, whose message now includes the last error (skip still panics under REQUIRED, L103)

Verified

  • Read the full diff and checked it against the issue and the description; they match.
  • Checked all 14 call sites across the 3 yarn-classic test binaries. Only the fail path gets slower: up to ~7 s per probe, and the probes run in parallel threads.
  • CHANGELOG.md is untouched, and nothing outside the one file changed.
  • CI 54/54 settled on 28a5f04 (35 success, 19 skipped), with ci-ok and clippy green.
  • Bugbot is clean on 28a5f04. There are 0 review threads.

Changes I made: none.

Open questions: none.

Auto-merge (squash) is armed, so approving sends it straight to the merge queue.


Generated by Claude Code

Merged via the queue into main with commit bf7ef6a Oct 23, 2026
54 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the ci-perf/1671-yarn-classic-probe-retry branch October 23, 2026 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-perf CI / merge-queue performance finding (profiler routine) Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI perf: yarn-classic 1.22.22 — one failed corepack yarn@1.22.22 fetch fails the leg and evicts a merge-queue entry (~30 MQ-min, ~94 job-min per hit)

3 participants