mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
1 commit
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
8402963198
|
fix(ci): shard platform-sensitive matrix + spawn built CLI to fix Windows cross-platform timeout (#2394)
* fix(ci): shard platform-sensitive matrix + spawn built CLI to fix Windows cross-platform timeout The `windows-latest (platform-sensitive)` job was hitting its 15-min internal vitest watchdog in run-cross-platform.ts. It's cumulative slowness, not a hang: the fixed 72-file suite is dominated by ~50 CLI/worker process spawns, and Windows is ~5x slower than macOS at process startup (macOS ran the same set in ~3min of tests). Two complementary changes bring it back under the watchdog with headroom, without touching any test assertion: - Shard the platform-sensitive matrix (windows/macos × shard [1,2]) and forward `--shard=i/2` through run-cross-platform.ts to vitest, which partitions the fixed file list deterministically (sha1, equal file-count) — halving each runner. macOS/Ubuntu were already under budget. - New test/helpers/cli-entry.ts (`CLI_SPAWN_PREFIX`): spawn the built `dist/cli/index.js` when `GITNEXUS_E2E_CLI=dist` (set on the cross-platform job, which already builds) instead of `node --import tsx src/cli/index.ts`, which re-transpiles the whole CLI on every spawn. Defaults to tsx-on-source so local runs always reflect current source; `GITNEXUS_E2E_CLI=dist` on an unbuilt tree throws an actionable "run npm run build" error. dist is opt-in only — never inferred from a generic `CI` env — so an ambient `CI=1` can't silently run a stale build. Converted 8 spawn-based e2e suites; added test/unit/cli-entry.test.ts. The Ubuntu coverage job leaves `GITNEXUS_E2E_CLI` unset, so the tsx-on-source path stays exercised in CI too (both entry points covered). Measured on Linux: cli-limit-e2e 121.5s→91s, cli-e2e 289s→217s (~25%); larger on Windows where the transpile is a bigger share of each spawn. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(ci): derive platform-sensitive shard count from one source (#2394) The shard total was hardcoded in three coupled, unenforced places (matrix length, job-name suffix, --shard denominator); editing one without the others silently dropped a shard's tests with green CI. Add a checkout-free shard-plan job whose single TOTAL generates both the shard index list (consumed via fromJSON) and the /N denominator (job name + --shard arg), so they cannot drift. Asserts TOTAL>=1 to rule out an empty-matrix silent skip. No behavior change — still 2 shards per OS. Addresses PR #2394 tri-review finding F2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): 3 shards for real Windows headroom + honest sharding comments (#2394) vitest shards by file COUNT, not runtime, so the heaviest spawn suites cluster into one shard: live CI showed Windows shard 1/2 at 12m12s (~81% of the 15-min watchdog) vs shard 2/2 at 3m0s. The old comments claimed "comfortable/generous headroom", which the count-based split doesn't deliver at 2 shards. Bump TOTAL to 3 (one line, single source) so even the busiest Windows shard clears the watchdog, and reword the comments to describe count-based (not time-based) sharding. Addresses PR #2394 tri-review finding F1. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ci): extract testable parseShardArg from run-cross-platform (#2394) The --shard parse/forward glue had no unit test. Extract it into a pure scripts/shard-arg.ts (mirroring the computeSpawnPrefix extraction precedent) so the branch logic is lockable without the script's top-level execFileSync, and add test/unit/shard-arg.test.ts (absent -> undefined, valid token -> passed through, found amid other args). Behavior unchanged; U4 adds the malformed fail-loud on top. Addresses PR #2394 tri-review finding F3. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): fail loud on a malformed --shard arg (#2394) A shard-shaped-but-malformed arg (--shard=1, --shard, --shard=abc) was silently ignored, dropping the shard flag so both legs ran the full unsharded ~50-spawn suite — re-arming the Windows watchdog timeout with no signal. parseShardArg now throws an actionable error on any --shard/--shard=… arg that fails the strict regex (unrelated flags like --shardx= pass through), and the call site in run-cross-platform.ts catches it into console.error + exit 1, kept outside the execFileSync try so the message isn't swallowed by that catch's watchdog-only branch. Addresses PR #2394 tri-review finding F4. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(test): fail loud on an unknown GITNEXUS_E2E_CLI value (#2394) computeSpawnPrefix silently degraded any unknown GITNEXUS_E2E_CLI value to tsx-on-source, so a typo (e.g. `dsit`) would make CI believe it tests the dist entry point while actually running src. Throw on any value other than 'dist'/'src'/unset (the safe tsx default is preserved for unset/''/'src', so it still never selects dist without an explicit opt-in). Flip the unknown-mode unit test to assert the throw and add the missing {mode:undefined, distExists:true} case. Only ci-tests.yml sets the var (=dist), so no existing suite is affected. Addresses PR #2394 tri-review findings minor-a/b. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(ci): run cli-entry.test.ts on the cross-platform matrix (#2394) cli-entry.test.ts resolves CLI_SPAWN_PREFIX from a real path, and its last assertion (cli[/\\]index) has a Windows backslash branch that only Ubuntu exercised. Register it in PLATFORM_LOGIC so it runs on the Windows/macOS matrix too. (shard-arg.test.ts stays out — pure string logic, OS-independent.) List grows 73 -> 74; the generated shard matrix keeps coverage complete. Addresses PR #2394 tri-review finding minor-c. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor(test): share tsxLoaderUrl(), dedup the last tsx-loader boilerplate (#2394) bridge-cache-reopen.test.ts carried its own copy of the tsx-loader-resolution boilerplate (createRequire -> resolve('tsx/package.json') -> pathToFileURL) — the one site the PR's CLI_SPAWN_PREFIX migration didn't cover (it spawns a seed script, not the CLI). Export the existing tsxLoaderUrl() from cli-entry.ts and reuse it here; the resolved loader URL is byte-identical. Addresses PR #2394 tri-review finding minor-d. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(test): make skipUnlessFtsAvailable install FTS on miss so shards are self-sufficient (#2394) Sharding the platform-sensitive suite into 3 exposed a latent test-isolation bug: load-only FTS primitives (test/integration/lbug-core-adapter.test.ts) only passed because a sibling installer test happened to co-locate in the same shard and install FTS into the shared ~/.lbdb first. At 3 shards, lbug-core-adapter landed in a shard with no installer sibling, so its load-only loadFTSExtension() failed deterministically on macOS+Windows shard 2/3 under GITNEXUS_REQUIRE_FTS=1. Make the gate self-sufficient: on a load-only miss under REQUIRE_FTS, install FTS with `auto` (LOAD-first, then one bounded network INSTALL) before treating it as a hard failure — mirroring withTestIndexedDB. A pre-installed extension still costs no network (auto is LOAD-first); offline/local runs (no env var) still skip gracefully. Verified: with a fresh HOME (no pre-installed FTS) + REQUIRE_FTS=1, lbug-core-adapter now passes 15/15 (previously threw). Addresses the 3-shard CI failure surfaced while validating PR #2394's F1 fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: warm-cache the LadybugDB FTS extension across platform shards (#2394) Follow-up to the FTS self-install fix: cache ~/.lbdb/extension per OS + lockfile so a warm run skips the network install entirely and the parallel shards share one download across runs. Pure reliability/speed — on a cache miss the tests still self-install FTS on demand (test/helpers/fts-availability.ts), so this is never a correctness dependency, just a way to cut the network-install surface that made the sharded FTS tests flaky. Keyed by lockfile hash (a LadybugDB version bump re-installs); per-OS since the extension is a native binary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): pass shard via env to clear zizmor template-injection (#2394) Interpolating ${{ matrix.shard }} (now sourced from the shard-plan job output) directly into the run: shell tripped zizmor's template-injection audit (code-scanning alert #824, ci-tests.yml:147). Move the value into a SHARD env var — assigned via ${{ }} but referenced as "$SHARD" in the shell, which is not an injection sink — and set shell: bash so the expansion is uniform across the windows + macOS matrix (the default run shell is pwsh on Windows, where $SHARD would be empty and trip the new malformed-shard fail-loud). Verified locally with zizmor: the :147 template-injection finding is gone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ci: shard the ubuntu coverage job and merge blobs before the threshold gate (#2394) The coverage job ran the full suite unsharded (~16 min). Shard it like the cross-platform matrix, then merge the per-shard coverage before enforcing the threshold gate: - shard-plan now also single-sources the coverage shard count (cov_total / cov_shards), so the coverage matrix + /N denominator can't drift. - The `tests` job becomes a coverage shard matrix: each shard runs `vitest run --shard --coverage --reporter=blob` with thresholds forced to 0 (a single shard's partial coverage can never meet the gate) and uploads its blob. FTS self-installs per shard, so sharding the full suite is safe. - New `coverage-merge` job (needs: tests) reduces the blobs with `vitest --mergeReports`, enforcing the REAL config thresholds on the combined ('new') coverage — this is the gate. It also emits the merged test-results.json and runs the unsharded web + docker suites, so the `test-reports` artifact keeps the exact shape ci-report.yml consumes for its base-branch ('baseline') vs new coverage delta. The shard arg goes through a SHARD env var + shell: bash (no template-injection). Validated locally: shard blobs write and merge into a coverage-summary.json + merged test-results.json; the merge enforces thresholds on the union. CI Gate still aggregates the coverage-merge result via the reusable-workflow call. Note: the coverage check names change (ubuntu / coverage 1/3 … + merge) — update any pinned branch-protection required checks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): include hidden files when uploading the coverage blob (#2394) The coverage shards write their blob to gitnexus/.vitest-reports/ (a dotdir). actions/upload-artifact excludes hidden files by default, so the coverage-blob-* artifacts uploaded empty — the merge job then downloaded 0 artifacts and vitest --mergeReports failed with ENOENT scandir '.vitest-reports'. Set include-hidden-files: true on the blob upload so the blobs actually ship. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): group shard-plan GITHUB_OUTPUT writes to satisfy shellcheck SC2129 (#2394) Adding the coverage shard outputs (cov_shards/cov_total) made the shard-plan gen step write four individual `>> "$GITHUB_OUTPUT"` redirects, which shellcheck (run by the actionlint check) flags as SC2129. Group the echoes into a single `{ …; } >> "$GITHUB_OUTPUT"` block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * perf(test): cost-balanced shard sequencer to cut CPU contention (#2394) vitest's default --shard hashes file paths and splits by file COUNT, which clustered the spawn-heavy suites onto one runner (Windows platform shard 1 ran ~4x the others). Add a custom sequence.sequencer that overrides only shard() and balances by estimated WORK instead: - specWeight() weights the fileParallelism:false spawn-heavy suites (cli-e2e, lbug-db — already isolated to run sequentially) far above the parallel default files, plus file size as a cheap finer signal. Deterministic per checkout. - assignShards() does greedy longest-processing-time bin-packing (heaviest file into the currently-lightest shard). The partition stays complete and disjoint — verified: on the 74-file cross-platform set the three shards weigh 7611/7610/8064 (the sequential-heavy files spread ~7/7/8) with zero overlap and no file dropped, vs the hash split's count-only balance. sort() is left to the base sequencer so project groupOrder / duration-cache ordering is untouched. Pure logic split into shard-balance.ts with a unit test locking the disjoint+complete, balance, and determinism properties. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(ci): install + cache FTS up front on the coverage (and cross-platform) shards (#2394) coverage 3/3 failed on extension-binary-real.test.ts: it uses the file-path FTS gate (requireFtsResourceOrSkip), which resolves ~/.lbdb/extension at MODULE LOAD and cannot self-install the way the load-path gate (skipUnlessFtsAvailable, U8) does. The coverage job had no FTS cache and relied on an installer test running first in the shard — the balancing sequencer reshuffled the shards and dropped extension-binary-real into a shard with no installer, so FTS was absent. Remove the ordering dependency: add scripts/ensure-fts.ts (init a throwaway lbug db, loadFTSExtension with policy:auto → LOAD-first, INSTALL on miss) and run it up front on every coverage AND cross-platform shard, after restoring the per-OS FTS cache. The coverage job now shares that same cache key (it previously had none — this is the "share the cached FTS with coverage" the failure pointed at). Cold cache installs once; warm cache is a no-network load. Verified locally: ensure-fts installs FTS into a fresh HOME and is a no-op when already present. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |