Repair two stale shell tests; CI now runs all six
Some checks are pending
Pre-commit checks / pre-commit (push) Waiting to run
Python tests / test (push) Waiting to run
Shell tests / shell-tests (push) Waiting to run

Both tests excluded from CI last commit turned out to be stale rather
than catching regressions. Neither covered script needed changing.

test_music_parallelism.sh built its temp worktree at
scripts/digital_wellbeing/, but music_parallelism.sh resolves its
library as $SCRIPT_DIR/../../lib/common.sh — two levels up, correct
for the real scripts/periodic_background/digital_wellbeing/ location.
From the shallower path that resolved outside the worktree, so the
stubbed common.sh was never sourced and the script fell through to the
real /usr/local/lib/common.sh on the machine. The test was exercising
the live library and ignoring its own MOCK_FOCUS_ACTIVE, which is why
idle mode looked like it waited 15s instead of 30s. The daemon was
correct all along.

test_usage_monitoring_installer_efficiency.sh demanded a literal
'sleep 60'. The logger template no longer polls: it sleeps once until
midnight via seconds_until_next_day, then blocks on wait for the pmon
child — strictly fewer wakeups than the assertion required. Replaced
with assertions on that property so a return to polling still fails.

Allowlist extended from four tests to six.
This commit is contained in:
Krzysztof Rudnicki 2026-07-19 19:13:51 +02:00
parent 93a466c8e4
commit e7c3300e80
4 changed files with 55 additions and 16 deletions

View File

@ -21,16 +21,7 @@ jobs:
# Only the side-effect-free tests run here. The rest of
# linux_configuration/tests/ drives pacman, /etc, systemd or adb and
# cannot run on an Ubuntu runner without root and an Arch container --
# see docs note in the accompanying evidence artifact.
#
# Deliberately NOT in this list (both fail today, and both look like they
# are catching real polling-efficiency regressions rather than being
# stale -- they need a fix in the scripts they cover, not in the tests):
# test_music_parallelism.sh
# 'idle mode uses the idle interval' expects 30s but the daemon
# waits 15s, i.e. it wakes twice as often as intended when idle.
# test_usage_monitoring_installer_efficiency.sh
# 'logger template must sleep between day rollover checks'.
# see the accompanying evidence artifacts for the full list.
- name: Run side-effect-free shell tests
run: |
set -euo pipefail
@ -38,7 +29,9 @@ jobs:
test_android_guardian_post_fs_data.sh
test_android_guardian_service.sh
test_i3blocks_persist_common.sh
test_music_parallelism.sh
test_setup_dns_blocker.sh
test_usage_monitoring_installer_efficiency.sh
)
failed=0
for t in "${tests[@]}"; do

View File

@ -0,0 +1,35 @@
{
"intent": "Repair the last two excluded shell tests so all six side-effect-free tests run in CI, after establishing that both failures were stale tests rather than regressions in the scripts they cover.",
"scope": [
"linux_configuration/tests/test_music_parallelism.sh: temp-worktree layout",
"linux_configuration/tests/test_usage_monitoring_installer_efficiency.sh: one stale assertion replaced with three property assertions",
".github/workflows/shell-tests.yml: allowlist grows from 4 tests to 6",
"Non-goal: any change to music_parallelism.sh or install_usage_monitoring.sh — both were found to be correct"
],
"changes": [
"test_music_parallelism.sh built its temp worktree at scripts/digital_wellbeing/, but the script resolves its library as $SCRIPT_DIR/../../lib/common.sh — two levels up, correct for the real scripts/periodic_background/digital_wellbeing/ location. From the shallower path that resolved outside the worktree, so the stubbed common.sh was never sourced and the script silently fell through to the real /usr/local/lib/common.sh installed on the machine. The test was therefore exercising the live library and ignoring its own MOCK_FOCUS_ACTIVE, which is why idle mode appeared to wait 15s instead of 30s. The worktree now mirrors the real layout.",
"test_usage_monitoring_installer_efficiency.sh asserted the logger template contains a literal 'sleep 60'. The template no longer polls: it sleeps once until midnight via seconds_until_next_day and then blocks on wait for the pmon child, which is strictly fewer wakeups. Replaced with three assertions on that property — sleeps until the day boundary, defines seconds_until_next_day, and blocks on wait — so a regression back to polling still fails.",
"Workflow allowlist extended to all six side-effect-free tests; the comment about known-red exclusions is removed because none remain."
],
"verification": [
{
"command": "bash linux_configuration/tests/<each of the 6 tests>",
"result": "pass",
"evidence": "All six PASS, up from four. Before: test_music_parallelism.sh failed 'idle mode uses the idle interval (expected 30, actual 15)'; test_usage_monitoring_installer_efficiency.sh failed 'logger template must sleep between day rollover checks'."
},
{
"command": "grep 'sleep \"$(seconds_until_next_day)\"' install_usage_monitoring.sh",
"result": "pass",
"evidence": "Confirmed the template sleeps exactly until midnight then blocks on `wait \"$pmon_pid\"`, with no busy-wait anywhere in the loop — better than the sleep-60 polling the old assertion demanded."
}
],
"risks": [
"Both failures were initially reported as polling-efficiency regressions in the covered scripts. That was wrong in both cases, and no script was changed. The lesson is that a test which cannot find its own stub may silently fall back to a real installed library and still appear to run — worth checking first whenever a test's expectations look inexplicably wrong.",
"test_music_parallelism.sh now depends on the temp worktree mirroring the real directory depth. Another reorganisation of scripts/ will break it again in the same silent way; the failure will look like a behavioural assertion rather than a path problem.",
"The new assertions match on specific template text, so harmless rewording of the logger template would fail them even though behaviour is unchanged."
],
"rollback": [
"git revert this commit: the two tests return to their stale expectations and CI's allowlist drops back to four tests.",
"Validate after rollback: both tests fail again with the original messages, and .github/workflows/shell-tests.yml lists four tests."
]
}

View File

@ -29,8 +29,8 @@ trap cleanup EXIT
WORKTREE="$TMP_DIR/worktree"
BIN_DIR="$TMP_DIR/bin"
mkdir -p "$WORKTREE/scripts/digital_wellbeing" "$WORKTREE/scripts/lib" "$BIN_DIR"
cp "$TARGET_SCRIPT" "$WORKTREE/scripts/digital_wellbeing/music_parallelism.sh"
mkdir -p "$WORKTREE/scripts/periodic_background/digital_wellbeing" "$WORKTREE/scripts/lib" "$BIN_DIR"
cp "$TARGET_SCRIPT" "$WORKTREE/scripts/periodic_background/digital_wellbeing/music_parallelism.sh"
cat >"$WORKTREE/scripts/lib/common.sh" <<'EOF'
#!/bin/bash
@ -87,7 +87,7 @@ run_case() {
XDOTOOL_LOG="${XDOTOOL_LOG:-}" \
PROC_ROOT="$proc_root" \
MOCK_FOCUS_ACTIVE="$focus_active" \
bash "$WORKTREE/scripts/digital_wellbeing/music_parallelism.sh" "$mode" \
bash "$WORKTREE/scripts/periodic_background/digital_wellbeing/music_parallelism.sh" "$mode" \
>/dev/null 2>&1 || true
assert_equals "$expected_wait" "$(<"$wait_log")" "music_parallelism.sh should pick the expected wait interval"

View File

@ -26,9 +26,20 @@ printf 'Checking pmon logger template avoids read -t busy-loop pattern...\n'
! grep -q 'read -r -t' <<< "$logger_template" \
|| fail 'logger template must not use read -t as sleep surrogate'
printf 'Checking pmon logger template uses sleep-based waiting...\n'
grep -q 'sleep 60' <<< "$logger_template" \
|| fail 'logger template must sleep between day rollover checks'
# The template used to poll with `sleep 60` until the day changed. It now
# sleeps exactly once, until the next midnight, and blocks on `wait` for the
# pmon child — strictly fewer wakeups. Assert that property rather than the
# old literal, so an accidental return to polling still fails the test.
printf 'Checking pmon logger template sleeps until rollover rather than polling...\n'
grep -q 'sleep "\$(seconds_until_next_day)"' <<< "$logger_template" \
|| fail 'logger template must sleep until the next day boundary'
grep -q 'seconds_until_next_day()' <<< "$logger_template" \
|| fail 'logger template must define seconds_until_next_day'
printf 'Checking pmon logger template blocks on wait instead of spinning...\n'
grep -qE '^\s*wait "\$pmon_pid"' <<< "$logger_template" \
|| fail 'logger template must block on wait for the pmon child'
printf 'Checking pmon logger template uses fork-free date builtin...\n'
grep -q "printf '%(%Y%m%d)T' -1" <<< "$logger_template" \