From 8f2adea8d9340146aa4cfd8d7366cdb18a5d4de0 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Sat, 19 Sep 2026 19:23:30 +0200 Subject: [PATCH 1/3] Shard race detection by test, and check that the shards cover the suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Race Detection is nineteen minutes and runs on every push, and seventeen of those minutes are one package: internal/connector is 1031s of a 1061s critical path. That is why sharding by package group, which the card proposed, buys about 4% — whichever shard gets that package still takes seventeen minutes. The unit has to be the test. Four shards. Each enumerates the suite with `go test -list -json`, which gives package-and-test pairs; a bare -list prints names with no package and six names here exist in more than one. 4988 pairs across 41 packages, sorted, sliced by index, and run in one `go test -race -run` over ./... so Go's own package parallelism survives inside the shard. The six duplicated names cost eight extra test runs in total. The hazard that comes with sharding is the one this repository keeps finding: a split that silently stops running some tests is green and measures nothing. Two checks answer it, and neither is a warning. Each shard compares what actually ran, from the run events, against what it was assigned, and fails on any shortfall. A -run regex that matches nothing exits 0 saying "no tests to run", which is exactly the shape this has to rule out. That check earned itself immediately: the first real shard ran 1247 of its 1251 tests, and the four missing were benchmarks — -list enumerates them and -run does not run them without -bench, so they were being assigned to a shard that could not run them. They are out of the enumeration now. The aggregate then fails unless every shard passed, all four enumerations are byte-identical, the slices are pairwise disjoint, and their union is the whole list. It is the `Race Detection` job itself, so the required status check keeps its name and the ruleset is untouched however many shards there are; `if: always()` so a failed shard cannot skip it, because a skipped required check is not a failed one. The union check has its own tests, in make check, because a check that cannot fail is worse than no check. Eight cases: one complete disjoint split accepted, and seven refused — a shard assigned nothing, a shard with no record, the same test in two shards, a test in no shard, shards that enumerated different sets, a test that does not exist, and an empty enumeration. No wall-clock claim here yet. The point of this commit is that the split is provably complete; what it is worth is the next thing to measure, on CI, before the shard count is touched. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test.yml | 77 ++++++++++++++++++--- Makefile | 10 ++- scripts/race-shard-union-test.sh | 60 +++++++++++++++++ scripts/race-shard-union.sh | 54 +++++++++++++++ scripts/race-shard.sh | 111 +++++++++++++++++++++++++++++++ 5 files changed, 303 insertions(+), 9 deletions(-) create mode 100755 scripts/race-shard-union-test.sh create mode 100755 scripts/race-shard-union.sh create mode 100755 scripts/race-shard.sh diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4abd110f9..e3f325a87 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -126,11 +126,20 @@ jobs: go install golang.org/x/vuln/cmd/govulncheck@latest govulncheck -tags dev ./... - race-check: - name: Race Detection + # The shards, and then Race Detection over them. The required status check + # is the aggregate, so its name never changes and the ruleset never has to + # be edited when the shard count does. + race-shard: + name: Race Detection (shard ${{ matrix.shard }} of 4) runs-on: ubuntu-latest permissions: contents: read + strategy: + # Every shard runs: stopping the others on the first failure would + # leave the aggregate unable to tell a real gap from a cancelled job. + fail-fast: false + matrix: + shard: [1, 2, 3, 4] env: BASECAMP_NO_KEYRING: "1" steps: @@ -143,12 +152,64 @@ jobs: with: go-version-file: 'go.mod' - - name: Run tests with race detector - # 20 minutes, not Go's default 10 per package. internal/connector runs - # real sockets, real processes and the recovery harness; under -race it - # takes about five minutes on a fast box and roughly twice that on a - # runner. The default was not a hung test, it was the budget. - run: go test -tags dev -race -v -timeout 20m ./... + # 20 minutes, not Go's default 10 per package. internal/connector runs + # real sockets, real processes and the recovery harness; under -race it + # takes about five minutes on a fast box and roughly twice that on a + # runner. The default was not a hung test, it was the budget. + - name: Run this shard under the race detector + run: scripts/race-shard.sh "${{ matrix.shard }}" 4 "shard-records/shard-${{ matrix.shard }}" + + # Uploaded even when the shard failed: the aggregate has to be able to + # tell "this shard ran its tests and one of them failed" from "this + # shard never got as far as knowing what it was meant to run". + - name: Record what this shard was assigned + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: race-shard-${{ matrix.shard }} + path: | + shard-records/shard-${{ matrix.shard }}/all.txt + shard-records/shard-${{ matrix.shard }}/shard.${{ matrix.shard }}.txt + retention-days: 3 + if-no-files-found: error + + race-check: + name: Race Detection + runs-on: ubuntu-latest + needs: [race-shard] + # always(), or a failed shard would skip this job — and a skipped required + # check is not a failed one. + if: always() + permissions: + contents: read + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Download what each shard recorded + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + pattern: race-shard-* + path: shard-records + + # Naming what came back before judging it: a check that runs over + # nothing is the failure this whole arrangement exists to prevent. + - name: Show the records + run: find shard-records -type f | sort + + - name: Every shard passed + env: + SHARDS: ${{ needs.race-shard.result }} + run: | + echo "shards: $SHARDS" + [ "$SHARDS" = success ] || { + echo "a race-detection shard did not pass" >&2 + exit 1 + } + + - name: The shards between them ran every test, once + run: scripts/race-shard-union.sh shard-records 4 integration: name: Integration Tests diff --git a/Makefile b/Makefile index 72da89f25..910221fd9 100644 --- a/Makefile +++ b/Makefile @@ -407,6 +407,13 @@ replace-check: fi @echo "Replace check passed (no local replace directives)" +# The race job is sharded, and the union check is what makes that safe: it +# refuses unless the shards between them ran every test, once. A check that +# cannot fail would be worse than no check, so it has its own tests. +.PHONY: check-race-shards +check-race-shards: + @scripts/race-shard-union-test.sh + # Verify every leaf command is accounted for in smoke tests .PHONY: check-smoke-coverage check-smoke-coverage: build @@ -429,7 +436,7 @@ check-eval-patterns: # Run all checks (local CI gate) .PHONY: check -check: fmt-check vet lint lint-actions test test-e2e test-sync-skills check-naming check-surface check-skill-drift test-skill-drift check-bare-groups check-lint-lockstep check-smoke-coverage check-eval-patterns provenance-check tidy-check +check: fmt-check vet lint lint-actions test test-e2e test-sync-skills check-naming check-surface check-skill-drift test-skill-drift check-bare-groups check-lint-lockstep check-smoke-coverage check-eval-patterns check-race-shards provenance-check tidy-check # Lint GitHub Actions workflows (requires actionlint + zizmor) .PHONY: lint-actions @@ -650,6 +657,7 @@ help: @echo " coverage Run tests with coverage and open in browser" @echo " record-cassettes Record happy-path cassettes (TOKEN+TARGET+ACCOUNT+PROJECT)" @echo " smoke Run pre-release smoke suite (BASECAMP_TOKEN=...)" + @echo " check-race-shards Test the race shards' union check" @echo " qa-report Show QA coverage report from smoke traces" @echo "" @echo "Performance:" diff --git a/scripts/race-shard-union-test.sh b/scripts/race-shard-union-test.sh new file mode 100755 index 000000000..e377336d9 --- /dev/null +++ b/scripts/race-shard-union-test.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# race-shard-union-test.sh — the union check's own tests. +# +# The union check is the reason it is safe to shard this job at all, so it +# is worth knowing it fails. Each case below is a way the shards could +# silently stop covering the suite; all of them must be refused. +set -euo pipefail +cd "$(dirname "$0")/.." +union=scripts/race-shard-union.sh + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +plant() { + rm -rf "${work:?}"/* + for i in 1 2 3 4; do + mkdir -p "$work/shard-$i" + printf 'pkg/a\tTestOne\npkg/a\tTestTwo\npkg/b\tTestThree\npkg/b\tTestFour\n' > "$work/shard-$i/all.txt" + done + printf 'pkg/a\tTestOne\n' > "$work/shard-1/shard.1.txt" + printf 'pkg/a\tTestTwo\n' > "$work/shard-2/shard.2.txt" + printf 'pkg/b\tTestThree\n' > "$work/shard-3/shard.3.txt" + printf 'pkg/b\tTestFour\n' > "$work/shard-4/shard.4.txt" +} + +refuses() { + local what="$1" + if "$union" "$work" 4 >/dev/null 2>&1; then + echo "FAIL: the union check accepted $what" >&2 + exit 1 + fi + echo "ok - refuses $what" +} + +plant +"$union" "$work" 4 >/dev/null || { echo "FAIL: the union check refused a complete, disjoint split" >&2; exit 1; } +echo "ok - accepts a complete, disjoint split" + +plant; : > "$work/shard-3/shard.3.txt" +refuses "a shard assigned nothing" + +plant; rm "$work/shard-2/shard.2.txt" +refuses "a shard that recorded no assignment" + +plant; printf 'pkg/a\tTestOne\n' >> "$work/shard-3/shard.3.txt" +refuses "the same test assigned to two shards" + +plant; printf 'pkg/b\tTestFive\n' >> "$work/shard-1/all.txt" +refuses "a test in the enumeration and in no shard" + +plant; printf 'pkg/b\tTestFive\n' >> "$work/shard-2/all.txt" +refuses "shards that enumerated different sets of tests" + +plant; printf 'pkg/z\tTestGhost\n' >> "$work/shard-4/shard.4.txt" +refuses "a test assigned that the repository does not have" + +plant; : > "$work/shard-1/all.txt" +refuses "a shard whose enumeration is empty" + +echo "All union checks behaved." diff --git a/scripts/race-shard-union.sh b/scripts/race-shard-union.sh new file mode 100755 index 000000000..c4e9b984b --- /dev/null +++ b/scripts/race-shard-union.sh @@ -0,0 +1,54 @@ +#!/usr/bin/env bash +# race-shard-union.sh - the aggregate job's check that the shards between +# them ran every test, once. +# +# This is the point of sharding the race job rather than a nicety attached to +# it. A split that silently stops running some tests reports a fast green and +# measures nothing, which is the defect this repository has spent a lot of +# effort closing elsewhere. So every one of these is a failure, not a warning: +# +# - a shard that enumerated a different set of tests than its siblings, +# which means they did not all see the same repository +# - the same test assigned to two shards, which means the split is wrong +# even where it looks complete +# - a test in no shard at all, which is the one that matters +# +# race-shard-union.sh +set -euo pipefail + +dir="${1:?directory holding the shard records}" +total="${2:?number of shards}" + +fail() { echo "SHARD COVERAGE FAILED: $*" >&2; exit 1; } + +sorted() { LC_ALL=C sort -u "$1"; } + +expected="" +for i in $(seq 1 "$total"); do + [ -s "$dir/shard-$i/all.txt" ] || fail "shard $i recorded no enumeration; it did not get far enough to have one" + sum=$(sorted "$dir/shard-$i/all.txt" | md5sum | cut -d' ' -f1) + if [ -z "$expected" ]; then + expected="$sum" + elif [ "$sum" != "$expected" ]; then + fail "shard $i enumerated a different set of tests than shard 1, so the shards did not all see the same repository" + fi +done + +for i in $(seq 1 "$total"); do + [ -s "$dir/shard-$i/shard.$i.txt" ] || fail "shard $i was assigned no tests" + for j in $(seq $((i + 1)) "$total"); do + overlap=$(LC_ALL=C comm -12 <(sorted "$dir/shard-$i/shard.$i.txt") <(sorted "$dir/shard-$j/shard.$j.txt")) + [ -z "$overlap" ] || fail "shards $i and $j were both assigned $(echo "$overlap" | wc -l) test(s), e.g. $(echo "$overlap" | head -1)" + done +done + +cat "$dir"/shard-*/shard.*.txt | LC_ALL=C sort -u > "$dir/union.txt" +sorted "$dir/shard-1/all.txt" > "$dir/expected.txt" + +missing=$(LC_ALL=C comm -23 "$dir/expected.txt" "$dir/union.txt") +[ -z "$missing" ] || fail "$(echo "$missing" | wc -l) test(s) were in no shard and so did not run, e.g. $(echo "$missing" | head -1)" + +extra=$(LC_ALL=C comm -13 "$dir/expected.txt" "$dir/union.txt") +[ -z "$extra" ] || fail "$(echo "$extra" | wc -l) test(s) were assigned that the repository does not have, e.g. $(echo "$extra" | head -1)" + +echo "shard coverage OK: $(wc -l < "$dir/expected.txt") tests across $total shards, disjoint and complete" diff --git a/scripts/race-shard.sh b/scripts/race-shard.sh new file mode 100755 index 000000000..7181e5555 --- /dev/null +++ b/scripts/race-shard.sh @@ -0,0 +1,111 @@ +#!/usr/bin/env bash +# race-shard.sh - run one shard of the race-detector suite. +# +# Race detection was one job of nineteen minutes, and seventeen of them were +# one package: internal/connector is 1031s of a 1061s critical path. Sharding +# by package therefore buys nothing — whichever shard gets that package still +# takes seventeen minutes — so the unit here is the test, not the package. +# +# The hazard that comes with that is the one this repository keeps finding: +# a shard scheme that silently stops running some tests is green and measures +# nothing. Two checks answer it. This script refuses to report success unless +# every test it was assigned actually ran, and race-shard-union.sh, in the +# aggregate job, refuses unless the shards between them covered the lot. +# +# race-shard.sh +set -euo pipefail + +index="${1:?shard index, 1-based}" +total="${2:?number of shards}" +out="${3:?directory to write the shard record into}" +mkdir -p "$out" + +all="$out/all.txt" +plan="$out/shard.$index.txt" + +# -json, not a bare -list: a plain listing prints test names with no package, +# and six names in this repository exist in more than one. The union check +# downstream compares package-and-test pairs, so the attribution has to be +# unambiguous here. +go test -tags dev -list '.*' -json ./... | + python3 -c ' +import sys, json +for line in sys.stdin: + try: + event = json.loads(line) + except ValueError: + continue + if event.get("Action") != "output": + continue + name = event.get("Output", "").strip() + # Benchmarks are listed but -run does not run them without -bench, so + # assigning one would be assigning work no shard can do. The race job + # has never run them; it does not start now. + if name and " " not in name and name.startswith(("Test", "Fuzz", "Example")): + print(event["Package"] + "\t" + name) +' | LC_ALL=C sort -u > "$all" + +[ -s "$all" ] || { echo "enumerated no tests at all" >&2; exit 1; } +awk -v i="$index" -v n="$total" 'NR % n == i % n' "$all" > "$plan" +[ -s "$plan" ] || { echo "shard $index of $total was assigned no tests" >&2; exit 1; } + +echo "shard $index of $total: $(wc -l < "$plan") of $(wc -l < "$all") tests" + +# One invocation over ./... keeps go test's own package parallelism. A name +# that exists in two packages runs in both, which costs eight extra tests +# across the repository and keeps the command line to a single regex. +regex="^($(cut -f2 "$plan" | LC_ALL=C sort -u | paste -sd'|'))$" + +set +e +go test -tags dev -race -timeout 20m -json -run "$regex" ./... | tee "$out/events.$index.json" | + python3 -c ' +import sys, json +for line in sys.stdin: + try: + event = json.loads(line) + except ValueError: + sys.stdout.write(line) + continue + if event.get("Action") == "output": + sys.stdout.write(event.get("Output", "")) +' +status=${PIPESTATUS[0]} +set -e + +# A -run regex that matches nothing exits 0 with "no tests to run". That is +# exactly the shape of failure this scheme has to rule out, so what ran is +# compared against what was assigned. +python3 - "$plan" "$out/events.$index.json" <<'PY' +import json, sys + +plan, events = sys.argv[1], sys.argv[2] + +assigned = set() +with open(plan) as handle: + for line in handle: + package, _, name = line.rstrip("\n").partition("\t") + if name: + assigned.add((package, name)) + +ran = set() +with open(events) as handle: + for line in handle: + try: + event = json.loads(line) + except ValueError: + continue + name = event.get("Test", "") + if event.get("Action") == "run" and name and "/" not in name: + ran.add((event["Package"], name)) + +missing = assigned - ran +if missing: + print(f"SHARD RAN {len(assigned) - len(missing)} OF ITS {len(assigned)} TESTS", file=sys.stderr) + for package, name in sorted(missing)[:20]: + print(f" did not run: {package}\t{name}", file=sys.stderr) + sys.exit(1) + +print(f"shard ran all {len(assigned)} tests it was assigned") +PY + +exit "$status" From 8a2636d650432f3d8b08456780bef69b2234e3c3 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Sat, 19 Sep 2026 19:52:32 +0200 Subject: [PATCH 2/3] The aggregate could not read the reports it judges MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two from review, and the first is this change's own defect aimed back at it. download-artifact extracts each artifact to //, and the uploads were named race-shard-N while the union check reads shard-N. So the aggregate would have found no reports at all — on every run. It would have failed rather than passed, which is the one mercy: the check refuses a shard whose enumeration it cannot read. But a checker that can never see its inputs is not a checker, and "shard 1 recorded no enumeration" would have sent whoever met it looking in the wrong place. The artifact is named for the directory the aggregate reads, and the message now allows for a report that did not arrive. Proved rather than reasoned about: the union check's tests gained the two cases that would have caught this. A shard whose report never arrived is refused, and so is a report that arrived under a name the checker does not read — which is exactly the defect, kept as a test so it cannot come back. Ten cases now, one accepted and nine refused. And md5sum is not on stock macOS, which would have made check-race-shards fail before testing anything for anyone not on Linux — a local check that only runs on Linux quietly stops being run. The enumerations are compared as sorted files with cmp instead, which is POSIX and needs no hash at all. Nothing GNU-only is left in the script. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test.yml | 7 +++++-- scripts/race-shard-union-test.sh | 10 ++++++++++ scripts/race-shard-union.sh | 13 ++++++------- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e3f325a87..f63b31c93 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -166,7 +166,10 @@ jobs: if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: race-shard-${{ matrix.shard }} + # The name is the directory the aggregate reads: download-artifact + # extracts each one to //, so this has to be the + # shard-N the union check looks for, not a name of its own. + name: shard-${{ matrix.shard }} path: | shard-records/shard-${{ matrix.shard }}/all.txt shard-records/shard-${{ matrix.shard }}/shard.${{ matrix.shard }}.txt @@ -190,7 +193,7 @@ jobs: - name: Download what each shard recorded uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: - pattern: race-shard-* + pattern: shard-* path: shard-records # Naming what came back before judging it: a check that runs over diff --git a/scripts/race-shard-union-test.sh b/scripts/race-shard-union-test.sh index e377336d9..c26e46a57 100755 --- a/scripts/race-shard-union-test.sh +++ b/scripts/race-shard-union-test.sh @@ -57,4 +57,14 @@ refuses "a test assigned that the repository does not have" plant; : > "$work/shard-1/all.txt" refuses "a shard whose enumeration is empty" +# The aggregate reads the shards through uploaded artifacts, so a report +# that never arrives looks exactly like a shard with nothing to say. It +# must not: an aggregator that cannot read a shard's report would have an +# opinion about it anyway. +plant; rm -rf "$work/shard-2" +refuses "a shard whose report never arrived" + +plant; mv "$work/shard-3" "$work/race-shard-3" +refuses "a report that arrived under a name the checker does not read" + echo "All union checks behaved." diff --git a/scripts/race-shard-union.sh b/scripts/race-shard-union.sh index c4e9b984b..0f5584508 100755 --- a/scripts/race-shard-union.sh +++ b/scripts/race-shard-union.sh @@ -23,13 +23,12 @@ fail() { echo "SHARD COVERAGE FAILED: $*" >&2; exit 1; } sorted() { LC_ALL=C sort -u "$1"; } -expected="" +# Compared as sorted files rather than hashed: cmp is POSIX and md5sum is +# not on macOS, and this check is in make check, which people run locally. for i in $(seq 1 "$total"); do - [ -s "$dir/shard-$i/all.txt" ] || fail "shard $i recorded no enumeration; it did not get far enough to have one" - sum=$(sorted "$dir/shard-$i/all.txt" | md5sum | cut -d' ' -f1) - if [ -z "$expected" ]; then - expected="$sum" - elif [ "$sum" != "$expected" ]; then + [ -s "$dir/shard-$i/all.txt" ] || fail "shard $i recorded no enumeration; either it did not get far enough to have one, or its report did not reach here" + sorted "$dir/shard-$i/all.txt" > "$dir/all.$i.sorted" + if [ "$i" != 1 ] && ! cmp -s "$dir/all.1.sorted" "$dir/all.$i.sorted"; then fail "shard $i enumerated a different set of tests than shard 1, so the shards did not all see the same repository" fi done @@ -43,7 +42,7 @@ for i in $(seq 1 "$total"); do done cat "$dir"/shard-*/shard.*.txt | LC_ALL=C sort -u > "$dir/union.txt" -sorted "$dir/shard-1/all.txt" > "$dir/expected.txt" +cp "$dir/all.1.sorted" "$dir/expected.txt" missing=$(LC_ALL=C comm -23 "$dir/expected.txt" "$dir/union.txt") [ -z "$missing" ] || fail "$(echo "$missing" | wc -l) test(s) were in no shard and so did not run, e.g. $(echo "$missing" | head -1)" From 7ad38a7ddc0437d8543528cce6c5e7e06fca38e3 Mon Sep 17 00:00:00 2001 From: Jorge Manrubia Date: Sat, 19 Sep 2026 20:08:29 +0200 Subject: [PATCH 3/3] The union check refused every correct split on macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review flagged seq as unavailable on macOS. That part is wrong: macOS ships /usr/bin/seq, and make check already runs seq twice in test-e2e long before it reaches this script. But looking at the loop for that reason found a worse bug behind it. BSD seq reads "first larger than last" as a request to count down. GNU seq prints nothing for `seq 5 4`; BSD seq prints "5" and "4". The overlap loop ends on exactly that range — at i == total it asks for seq $((total+1)) total — so on macOS the last iteration ran with j == total+1 and j == total, read a shard directory that does not exist, and compared the last shard against itself. Every correct split was refused, with the message "shards 4 and 4 were both assigned 1 test(s)". So the fix the review asked for is right, for a reason it did not give. All three ranges now count in the shell. Proved it with a shim implementing BSD's documented rule: with seq the new case fails exactly as macOS would, naming shards 4 and 4; counting in the shell, it passes. The shim is the test, so a reintroduced seq cannot pass. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/race-shard-union-test.sh | 24 ++++++++++++++++++++++++ scripts/race-shard-union.sh | 12 +++++++++--- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/scripts/race-shard-union-test.sh b/scripts/race-shard-union-test.sh index c26e46a57..d12f9fa4c 100755 --- a/scripts/race-shard-union-test.sh +++ b/scripts/race-shard-union-test.sh @@ -67,4 +67,28 @@ refuses "a shard whose report never arrived" plant; mv "$work/shard-3" "$work/race-shard-3" refuses "a report that arrived under a name the checker does not read" +# The check must not depend on how seq reads a backwards range. macOS ships +# seq, but BSD seq treats "first larger than last" as counting down, and the +# overlap loop ends on exactly that range. With seq, this case compared the +# last shard against itself and refused every correct split on macOS. +plant +bsd=$work/bsdbin +mkdir -p "$bsd" +cat > "$bsd/seq" <<'SEQ' +#!/usr/bin/env bash +# BSD seq(1): "If first is larger than last the default incr is -1." +first=$1; last=$2 +if [ "$first" -gt "$last" ]; then + for ((n = first; n >= last; n--)); do echo "$n"; done +else + for ((n = first; n <= last; n++)); do echo "$n"; done +fi +SEQ +chmod +x "$bsd/seq" +PATH="$bsd:$PATH" "$union" "$work" 4 >/dev/null || { + echo "FAIL: the union check refused a complete, disjoint split where seq counts down over a backwards range, as BSD seq does on macOS" >&2 + exit 1 +} +echo "ok - accepts a complete, disjoint split under BSD seq semantics" + echo "All union checks behaved." diff --git a/scripts/race-shard-union.sh b/scripts/race-shard-union.sh index 0f5584508..7cfc8cce3 100755 --- a/scripts/race-shard-union.sh +++ b/scripts/race-shard-union.sh @@ -25,7 +25,13 @@ sorted() { LC_ALL=C sort -u "$1"; } # Compared as sorted files rather than hashed: cmp is POSIX and md5sum is # not on macOS, and this check is in make check, which people run locally. -for i in $(seq 1 "$total"); do +# +# The loops count in the shell rather than through seq for the same reason. +# macOS does ship seq, but BSD seq reads "first larger than last" as a +# request to count down, so `seq 5 4` prints "5 4" where GNU seq prints +# nothing. The inner loop below ends on exactly that range, so on macOS +# every correct run was refused, comparing the last shard against itself. +for ((i = 1; i <= total; i++)); do [ -s "$dir/shard-$i/all.txt" ] || fail "shard $i recorded no enumeration; either it did not get far enough to have one, or its report did not reach here" sorted "$dir/shard-$i/all.txt" > "$dir/all.$i.sorted" if [ "$i" != 1 ] && ! cmp -s "$dir/all.1.sorted" "$dir/all.$i.sorted"; then @@ -33,9 +39,9 @@ for i in $(seq 1 "$total"); do fi done -for i in $(seq 1 "$total"); do +for ((i = 1; i <= total; i++)); do [ -s "$dir/shard-$i/shard.$i.txt" ] || fail "shard $i was assigned no tests" - for j in $(seq $((i + 1)) "$total"); do + for ((j = i + 1; j <= total; j++)); do overlap=$(LC_ALL=C comm -12 <(sorted "$dir/shard-$i/shard.$i.txt") <(sorted "$dir/shard-$j/shard.$j.txt")) [ -z "$overlap" ] || fail "shards $i and $j were both assigned $(echo "$overlap" | wc -l) test(s), e.g. $(echo "$overlap" | head -1)" done