From 18db9e7a17af0f8b41d0582e39329581864251dc Mon Sep 17 00:00:00 2001 From: Eli Ma Date: Mon, 10 Aug 2026 10:42:10 +0800 Subject: [PATCH] ci: shard command_test and fix what kept compat-offline-core from finishing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `compat-offline-core` has not run to completion in weeks. `cargo test --all` is fail-fast, and the lib suite has been failing since 2026-07-28, so cargo aborted there every time and never reached a single integration binary. Runs looked like 53-minute failures; the suite behind them was never measured. Once the lib failures were fixed on a branch, the job ran past the 360-minute platform cap. Three separate problems, each only visible because the run got further than any before it: 1. `tests/command_test.rs` compiles 150 modules (~2950 tests) into one binary — deliberately, to avoid one binary per command. Roughly a third of them take the process-global cwd lock every `ChangeDirGuard` holds, so it runs at about one core no matter the machine: 479 of its tests logged "running for over 60 seconds" while the host sat near 0.3 load. It is lock-bound, not CPU-bound, and the only thing that shortens it is more than one PROCESS. Split into `compat-offline-command`, a four-way matrix. Measured: 739 tests per shard, ~2h50m each, comfortably inside the cap. 2. `command::switch_test::test_detach_head_basic` ABORTED the whole binary with a stack overflow. Not recursion — an unoptimized async state machine inlines every awaited future into one frame, and libtest's 2 MiB test threads cannot hold `switch::execute`'s chain. Reproduced in isolation at 2 MiB, passes at 4 MiB. `RUST_MIN_STACK` is 16 MiB here for headroom. The shipped CLI runs those futures on the 8 MiB main thread, so this is a harness limit, not a product one. 3. compat-offline-core now names its targets instead of using `--all`, so the two halves can be split. Targets whose `required-features` are off must be dropped rather than named: `cargo test --all` skips them silently, but `--test ` on one is a hard error. Verified: 208 test targets, 200 after the filter, and the eight dropped are exactly the ones this job's later steps already run explicitly with the feature each needs. Coverage is unchanged. Shards partition by enumerated test NAME, so the split is exhaustive and disjoint by construction — a rename cannot drop a test the way a hand-maintained filter list would. Both jobs refuse to run a suspiciously small set rather than pass on partial coverage, and both carry `timeout-minutes: 350` so an overrun fails with its log intact instead of being reaped at the cap. Validated on libra-tools/libra#447: shard 0 finished 737/739 and shard 1 finished 738/739, where every previous attempt had timed out or aborted with no result at all. The handful of remaining failures are pre-existing and unrelated to this change. Co-Authored-By: Claude Opus 5 --- .github/workflows/base.yml | 136 ++++++++++++++++++++++++++++++++++++- 1 file changed, 135 insertions(+), 1 deletion(-) diff --git a/.github/workflows/base.yml b/.github/workflows/base.yml index 1a30e5f72..51f5b9da8 100644 --- a/.github/workflows/base.yml +++ b/.github/workflows/base.yml @@ -169,12 +169,22 @@ jobs: runs-on: [self-hosted] + # Below GitHub's 360-minute default so an overrun fails as a timeout with + # the log intact, rather than being reaped at the platform cap. + timeout-minutes: 350 + env: CARGO_TERM_COLOR: always RUSTUP_TOOLCHAIN: stable LIBRA_SKIP_WEB_BUILD: "1" CARGO_PROFILE_TEST_DEBUG: "0" CARGO_BUILD_JOBS: "1" + # libtest runs each test in a thread with a 2 MiB stack. An unoptimized + # async state machine — `switch::execute` and friends inline every + # awaited future into one frame — does not fit, and the test process + # ABORTS rather than failing. The real CLI runs those futures on the + # 8 MiB main thread, so this is a harness limit, not a product one. + RUST_MIN_STACK: "16777216" steps: - name: Checkout repository @@ -240,7 +250,45 @@ jobs: LIBRA_STORAGE_BUCKET: ${{ secrets.LIBRA_STORAGE_BUCKET }} LIBRA_STORAGE_ACCESS_KEY: ${{ secrets.LIBRA_STORAGE_ACCESS_KEY }} LIBRA_STORAGE_SECRET_KEY: ${{ secrets.LIBRA_STORAGE_SECRET_KEY }} - run: cargo test --all + run: | + set -euo pipefail + # `cargo test --all` minus the `command_test` binary, which + # `compat-offline-command` shards instead. That one target compiles + # 150 modules (~2950 tests) into a SINGLE process, and roughly a + # third of them serialize on the process-global cwd lock that every + # `ChangeDirGuard` takes — so it runs at about one core regardless of + # the machine. It went past the 360-minute cap the first time the lib + # suite stopped failing early and cargo actually reached it. + # + # Coverage is unchanged: lib, bins and doctests run here, so does + # every other integration target, and `command_test` runs there. + # + # Targets whose `required-features` are not enabled are dropped, not + # named: `cargo test --all` SKIPS those silently, but naming one with + # `--test` is a hard error. The later steps in this job run each of + # them explicitly with the feature it needs. + mapfile -t TARGETS < <( + cargo metadata --no-deps --format-version 1 \ + | jq -r '.packages[] + | (.features.default // []) as $default + | .targets[] + | select(.kind[] == "test") + | select(.name != "command_test") + | select(((((."required-features") // []) - $default) | length) == 0) + | .name' \ + | sort -u + ) + if [ "${#TARGETS[@]}" -lt 100 ]; then + echo "::error::enumerated only ${#TARGETS[@]} integration targets; refusing to run a truncated suite" + exit 1 + fi + echo "running lib + bins + doctests + ${#TARGETS[@]} integration targets" + ARGS=() + for target in "${TARGETS[@]}"; do + ARGS+=(--test "$target") + done + cargo test --lib --bins "${ARGS[@]}" + cargo test --doc # Phase 6 — Local TUI Automation Control scenario suite (docs/improvement/agent.md Part C). # Without `--features test-provider` + `LIBRA_ENABLE_TEST_PROVIDER=1`, the scenarios @@ -303,6 +351,92 @@ jobs: if-no-files-found: ignore retention-days: 7 + # The `command_test` half of what `cargo test --all` used to do in one job. + # See the note on compat-offline-core's test step for why it is split out: + # the binary is lock-bound rather than CPU-bound, so the only thing that + # shortens it is running it in more than one PROCESS. Shards are separate + # jobs, so each gets its own cwd lock and they scale with the runner pool. + command-tests: + name: compat-offline-command + + runs-on: [self-hosted] + + timeout-minutes: 350 + + strategy: + # One shard failing must not cancel the others: the point of the split is + # to see the whole binary's result in one run. + fail-fast: false + matrix: + shard: [0, 1, 2, 3] + + env: + CARGO_TERM_COLOR: always + RUSTUP_TOOLCHAIN: stable + LIBRA_SKIP_WEB_BUILD: "1" + CARGO_PROFILE_TEST_DEBUG: "0" + CARGO_BUILD_JOBS: "1" + # See compat-offline-core: 2 MiB test threads cannot hold an + # unoptimized async state machine, and the process aborts if one + # overflows. `command::switch_test` needs a little over 2 MiB. + RUST_MIN_STACK: "16777216" + # Kept in one place so the matrix above and the partition below cannot + # disagree — a mismatch would silently drop or double-run tests. + SHARD_COUNT: "4" + + steps: + - name: Checkout repository + uses: actions/checkout@v5 + with: + submodules: recursive + + - name: Setup Node.js + uses: actions/setup-node@v5 + with: + node-version: "22" + + - name: Enable pnpm + run: | + corepack enable + corepack prepare pnpm@11.10.0 --activate + + - name: Run command_test shard ${{ matrix.shard }} + env: + LIBRA_TEST_GITHUB_TOKEN: ${{ secrets.LIBRA_TEST_GITHUB_TOKEN }} + LIBRA_TEST_GITHUB_NAMESPACE: ${{ secrets.LIBRA_TEST_GITHUB_NAMESPACE }} + DEEPSEEK_API_KEY: ${{ secrets.DEEPSEEK_API_KEY }} + LIBRA_D1_ACCOUNT_ID: ${{ secrets.LIBRA_D1_ACCOUNT_ID }} + LIBRA_D1_API_TOKEN: ${{ secrets.LIBRA_D1_API_TOKEN }} + LIBRA_D1_DATABASE_ID: ${{ secrets.LIBRA_D1_DATABASE_ID }} + LIBRA_STORAGE_ENDPOINT: ${{ secrets.LIBRA_STORAGE_ENDPOINT }} + LIBRA_STORAGE_BUCKET: ${{ secrets.LIBRA_STORAGE_BUCKET }} + LIBRA_STORAGE_ACCESS_KEY: ${{ secrets.LIBRA_STORAGE_ACCESS_KEY }} + LIBRA_STORAGE_SECRET_KEY: ${{ secrets.LIBRA_STORAGE_SECRET_KEY }} + run: | + set -euo pipefail + # Partition by ENUMERATED TEST NAME, not by module prefix. The modulo + # split is exhaustive and disjoint by construction, so a renamed or + # newly added test cannot silently fall out of every shard the way a + # hand-maintained filter list would eventually let one do. + cargo test --test command_test --no-run + mapfile -t ALL < <( + cargo test --test command_test -- --list --format terse \ + | sed -n 's/: test$//p' | sort + ) + total=${#ALL[@]} + if [ "$total" -lt 2000 ]; then + echo "::error::enumerated only $total command tests; refusing to run a truncated shard" + exit 1 + fi + MINE=() + for index in "${!ALL[@]}"; do + if [ "$(( index % SHARD_COUNT ))" -eq "${{ matrix.shard }}" ]; then + MINE+=("${ALL[$index]}") + fi + done + echo "shard ${{ matrix.shard }} of $SHARD_COUNT: ${#MINE[@]} of $total tests" + cargo test --test command_test -- --exact "${MINE[@]}" + network-remotes: name: compat-network-remotes runs-on: [self-hosted]