Skip to content

test(cli): retry flaky subprocess evaluations in CI - #5909

Open
vianmora wants to merge 1 commit into
webstudio-is:mainfrom
webstudio-community:test/cli-retry-flaky-evaluations
Open

test(cli): retry flaky subprocess evaluations in CI#5909
vianmora wants to merge 1 commit into
webstudio-is:mainfrom
webstudio-community:test/cli-retry-flaky-evaluations

Conversation

@vianmora

Copy link
Copy Markdown
Contributor

Description

The CLI evaluation tests under packages/cli/evaluations/high-impact/* spawn real CLI subprocesses (cold-started through tsx) against a local fixture API. On a loaded CI runner those cold starts occasionally exceed the default 15s test timeout, so fixture-api.test.ts fails with Test timed out in 15000ms even though nothing is broken. Re-running the same commit passes. It is a timing/flaky failure, not a regression.

This adds a CI-only test retry so a transient timeout is absorbed instead of turning the whole run red and requiring a manual re-run:

// packages/cli/vite.config.ts
test: {
  retry: process.env.CI ? 2 : 0,
}
  • A real failure still fails every attempt, so this hides no bug.
  • Local runs stay strict (retry: 0).
  • The defineConfig import moves from vite to vitest/config (a superset; the existing build block is unaffected) so the test field type-checks.
  • It lives in packages/cli/vite.config.ts because that is the config Vitest loads for this package. A test.retry in the root vitest.config.ts does not reach it (verified locally with a probe test: the root value is ignored, the package-local one takes effect).

Steps for reproduction

  1. Run the CLI package tests repeatedly, ideally on a busy machine: pnpm --filter=webstudio test.
  2. Occasionally evaluations/high-impact/fixture-api.test.ts fails with Test timed out in 15000ms; a re-run passes.
  3. With this change and CI=1, the transient timeout is retried up to 2x instead of failing the run.

Code Review

  • hi @kof, I need you to do
    • conceptual review (architecture, feature-correctness)

Before requesting a review

  • made a self-review
  • added inline comments where things may be not obvious (the "why", not "what")

Before merging

  • tested locally (verified the retry applies from the package config; the root config value is ignored)
  • tested on preview environment (n/a, test-runner config change with no preview surface)
  • updated test cases document (n/a, no feature change)
  • added tests (n/a, adjusts existing test-runner behavior)
  • if any new env variables are added, added them to .env file (none added; process.env.CI is set by CI providers)

The evaluations/high-impact/* tests spawn CLI subprocesses (cold-started via
tsx) and occasionally exceed the 15s timeout on a loaded runner, failing
fixture-api.test.ts with a spurious timeout that passes on re-run. Add
retry: 2 in CI (retry: 0 locally) so a transient timeout is absorbed; a real
failure still fails every attempt.

The defineConfig import switches to vitest/config (a superset of vite's; the
build block is unaffected) so the test field type-checks.
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.

1 participant