aglinxinyuan commented on code in PR #7623:
URL: https://github.com/apache/texera/pull/7623#discussion_r3788744914
##########
frontend/vitest.browser.config.ts:
##########
@@ -64,6 +64,11 @@ export default defineConfig({
// browser-mode the runtime has neither, so we install the `buffer` npm
// package as a shim).
setupFiles: ["src/browser-buffer-polyfill.ts", "src/test-zone-setup.ts"],
+ // Same runner-stall headroom as the jsdom config (vitest.config.ts):
+ // driving a real Chromium through playwright is strictly slower than
+ // jsdom, so these specs need at least as much slack. See #6073.
+ testTimeout: 30_000,
+ hookTimeout: 30_000,
Review Comment:
Addressed in fb5ec5a. Confirmed against the pinned 4.1.10 in `node_modules`
(`vitest/dist/chunks/coverage.DM_a_rWm.js:538-539`) — with `browser.enabled`
the defaults resolve to 15s test / 30s hook, so `hookTimeout: 30_000` was
restating the default. Dropped it, and the comment now says what the remaining
line actually does: a 15s -> 30s bump so the Chromium leg matches the jsdom
ceiling.
TESTING.md is scoped per config now: 30s per test in both (up from 5s under
jsdom, 15s under `browser.enabled`), hooks 30s under jsdom (up from 10s) with
browser mode already there by default.
##########
frontend/vitest.config.ts:
##########
@@ -34,6 +34,16 @@ export default defineConfig({
// which Angular's `fakeAsync` requires. Karma+Jasmine installed this
// implicitly; the @angular/build:unit-test path doesn't.
setupFiles: ["src/test-zone-setup.ts"],
+ // Vitest defaults (5s per test, 10s per hook) are too tight for the
+ // macOS runners, which stall for seconds at a time under load: the same
+ // spec file that takes 240ms on ubuntu-latest has been observed taking
+ // 11.7s on macos-latest in the same commit's matrix. The stall lands on
+ // whichever test happens to be running, so raising the ceiling is the
+ // only fix that isn't whack-a-mole — three different specs have gone
+ // red this way. A test that legitimately needs >30s is broken, and the
+ // job's own timeout still bounds a true hang. See apache/texera#6073.
Review Comment:
Addressed in fb5ec5a. Added `timeout-minutes: 30` to the `frontend` job,
same as `amber-integration` does, so the sentence points at a real bound
instead of the implicit 6h cap. Sizing: recent green legs run 4-12 min (this
PR's own run was ubuntu 8, windows 11, macOS 12) and the "Install dependency"
step is already allowed 20 on its own, so 30 bounds a hang while still
absorbing a cold yarn cache on a slow runner. The vitest.config.ts comment now
names that key explicitly.
##########
.github/workflows/build.yml:
##########
@@ -87,6 +87,12 @@ jobs:
if: ${{ inputs.run_frontend }}
runs-on: ${{ matrix.os }}
strategy:
+ # An OS-specific failure should not cancel the other two legs: with the
+ # default fail-fast the surviving jobs report "The operation was
+ # canceled" and the run no longer says whether the failure reproduces
+ # off that OS — exactly the evidence needed to tell a runner flake from
+ # a real break. Every other multi-leg matrix here already opts out.
Review Comment:
You're right — I parsed the file rather than trust the claim: the jobs that
set `fail-fast: false` are exactly `platform`, `platform-integration`,
`agent-service` and `infra`, while `amber-integration` (2 OS legs) and
`pyamber` (3 Python versions) are multi-leg with no opt-out. Fixed in fb5ec5a
to name those four instead.
On the aside: left `amber-integration` alone here to keep this PR to the
frontend job — happy to open a follow-up for it and `pyamber`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
