Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion .github/scripts/shard-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,44 @@ while [ "$i" -lt "$total" ]; do
i=$((i + 1))
done

# What a pinned file is seeded with, and why it is not its @test count.
#
# The pin exists because these files' @test count does not describe their cost
# (see above). Seeding `load` with that same count therefore hands the balancer
# the one number it was told not to trust: a 630s file that contains 9 @test
# blocks reports a load of 9, the shard reads as very nearly empty, and the
# ordinary weighted pass fills it right back up. Measured on the terminal-driver
# tree (macOS, run 33850939213 attempt 4): the four shards came out
# 32.2/17.1/10.8/12.2 minutes against a 30-minute cap, and the 32.2 was the
# shard holding the 630s pin plus a full share of everything else. It timed out
# four attempts in a row while two other shards sat idle for fifteen minutes.
#
# So a pinned file is seeded at ONE SHARD'S AVERAGE SHARE instead: it is treated
# as already worth about what a shard is supposed to hold, which is the honest
# reading of "we cannot weigh this one". Same tree, same measured per-file
# seconds: 14.5/15.8/21.0/21.1, a 21-minute worst case.
#
# This is deliberately not a table of per-file seconds. That alternative was
# considered when the weights were first written and rejected because it goes
# stale silently -- it would be wrong the moment a fixed `sleep` becomes a poll
# loop, with nothing to say so. The share is derived from the tree on every run
# and cannot drift away from it.
#
# The failure mode to watch is over-reservation: a pinned file that is actually
# light leaves its shard underfilled and pushes the others up. That is bounded
# while the pins are few relative to `total` -- at 2 pins in 4 shards the worst
# case is the remaining work over two shards, which the numbers above clear with
# room. Pinning a majority of the shards would invert this, and nothing here
# detects that; keep PINNED_APART short.
total_tests=0
while IFS= read -r f; do
[ -n "$f" ] || continue
total_tests=$((total_tests + $(file_weight "$f")))
done <<EOF
$files
EOF
pin_seed=$((total_tests / total))

pinned_paths=" "
slot=0
for p in $PINNED_APART; do
Expand All @@ -163,7 +201,7 @@ $files
EOF
[ -n "$match" ] || continue
s=$((slot % total))
load[s]=$((load[s] + $(file_weight "$match")))
load[s]=$((load[s] + pin_seed))
if [ "$s" -eq "$((index - 1))" ]; then
printf '%s\n' "$match"
fi
Expand Down
37 changes: 37 additions & 0 deletions tests/test_ci_sharding.bats
Original file line number Diff line number Diff line change
Expand Up @@ -179,3 +179,40 @@ union_of_shards() {
# log claims a split the workflow is not performing.
grep -q "bats (\${{ matrix.os }} \${{ matrix.shard }}/$total)" "$wf"
}

# The pin's whole point is that these files' @test count does not describe their
# cost. Seeding the balancer with that count undoes the pin: the shard reads as
# empty and gets refilled, which is how one shard reached 32 minutes against a
# 30-minute cap while two others sat at 11 and 12 (macOS, run 33850939213).
#
# What this asserts is the reservation itself: a shard holding a pinned file
# must end up carrying MEASURABLY FEWER tests than the average shard, because
# the seed already spent that shard's budget. A test that only checked the
# pinned files land on different shards -- which the suite above already does --
# stays green through exactly this regression, since they still land apart when
# seeded by count. That is the break this one is here to catch.
@test "a pinned file's shard is reserved, not refilled (pin seed is not the count)" {
local script="$BATS_TEST_DIRNAME/../.github/scripts/shard-tests.sh"
local dir="$BATS_TEST_DIRNAME/.."
local total=4 n f count mean grand=0

for f in "$BATS_TEST_DIRNAME"/*.bats; do
grand=$((grand + $(grep -c '^[[:space:]]*@test' "$f" || true)))
done
mean=$((grand / total))
[ "$mean" -gt 0 ]

# Shard 1 and shard 2 hold the two pinned files (slot 0 and slot 1).
for n in 1 2; do
count=0
while IFS= read -r f; do
[ -n "$f" ] || continue
count=$((count + $(grep -c '^[[:space:]]*@test' "$dir/$f" || true)))
done < <(cd "$dir" && bash "$script" "$n" "$total")

# Strictly below the average, with margin: seeded by count these shards come
# out at roughly the mean, so a bare "<= mean" would be a coin flip rather
# than a check. Two thirds is well clear of both states.
[ "$count" -lt $((mean * 2 / 3)) ]
done
}
Loading