Yicong-Huang commented on code in PR #7623:
URL: https://github.com/apache/texera/pull/7623#discussion_r3840871924
##########
.github/workflows/build.yml:
##########
@@ -86,7 +86,24 @@ jobs:
frontend:
if: ${{ inputs.run_frontend }}
runs-on: ${{ matrix.os }}
+ # Only the "Install dependency" step below is bounded, so a spec that
+ # truly hangs would otherwise run to GitHub's implicit 6h cap — billed at
Review Comment:
`macos-latest` is a standard runner, and this repository is public.
[GitHub's runner
policy](https://docs.github.com/en/actions/reference/runners/github-hosted-runners#standard-github-hosted-runners-for-public-repositories)
therefore makes this usage free and unlimited, not billed at a 10x multiplier.
Please remove the billing claim here and in the matching PR-description row;
the six-hour resource/concurrency bound still justifies the cap.
##########
frontend/TESTING.md:
##########
@@ -47,6 +47,7 @@ For repo-wide testing philosophy (TDD, characterization
tests, "every test must
| Coverage | `@vitest/coverage-v8`
|
| Test setup | `src/test-zone-setup.ts` wraps `it`/`test` in an
Angular ProxyZone (Vitest does not provide one and Angular's `fakeAsync`
requires it) |
| Globals | `globals: true` in `vitest.config.ts`, so
`describe / it / expect / vi / beforeEach` come from the runtime — no per-file
imports |
+| Timeouts | Raised over Vitest's defaults because macOS CI
runners stall for seconds at a time (#6073, #7713). jsdom: 20s per test / 30s
per hook (`vitest.config.ts`, defaults 5s/10s). Browser mode: 30s per test
(`vitest.browser.config.ts`), hooks left at the 30s `browser.enabled` already
resolves — its per-test default is 15s |
Review Comment:
`the 30s` needs a noun for the following `browser.enabled` clause to modify.
```suggestion
| Timeouts | Raised over Vitest's defaults because macOS CI
runners stall for seconds at a time (#6073, #7713). jsdom: 20s per test / 30s
per hook (`vitest.config.ts`, defaults 5s/10s). Browser mode: 30s per test
(`vitest.browser.config.ts`), hooks left at the 30s default that
`browser.enabled` already resolves — its per-test default is 15s |
```
--
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]
