From e7c3300e80d815fb2d8e852af32a31ccf1e5b26c Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sun, 19 Jul 2026 19:13:51 +0200 Subject: [PATCH] Repair two stale shell tests; CI now runs all six MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/shell-tests.yml | 13 ++----- ...ell-tests-stale-assertions-2026-07-19.json | 35 +++++++++++++++++++ .../tests/test_music_parallelism.sh | 6 ++-- ...t_usage_monitoring_installer_efficiency.sh | 17 +++++++-- 4 files changed, 55 insertions(+), 16 deletions(-) create mode 100644 docs/superpowers/evidence/shell-tests-stale-assertions-2026-07-19.json diff --git a/.github/workflows/shell-tests.yml b/.github/workflows/shell-tests.yml index 8a960f3f..ccba7983 100644 --- a/.github/workflows/shell-tests.yml +++ b/.github/workflows/shell-tests.yml @@ -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 diff --git a/docs/superpowers/evidence/shell-tests-stale-assertions-2026-07-19.json b/docs/superpowers/evidence/shell-tests-stale-assertions-2026-07-19.json new file mode 100644 index 00000000..f0e3bbd4 --- /dev/null +++ b/docs/superpowers/evidence/shell-tests-stale-assertions-2026-07-19.json @@ -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/", + "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." + ] +} diff --git a/linux_configuration/tests/test_music_parallelism.sh b/linux_configuration/tests/test_music_parallelism.sh index 60ab34f2..542ee657 100755 --- a/linux_configuration/tests/test_music_parallelism.sh +++ b/linux_configuration/tests/test_music_parallelism.sh @@ -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" diff --git a/linux_configuration/tests/test_usage_monitoring_installer_efficiency.sh b/linux_configuration/tests/test_usage_monitoring_installer_efficiency.sh index 0f660e63..044ecfb7 100755 --- a/linux_configuration/tests/test_usage_monitoring_installer_efficiency.sh +++ b/linux_configuration/tests/test_usage_monitoring_installer_efficiency.sh @@ -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" \