From c588b0841df440e1a5290c7150ee245f90d1788e Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Thu, 20 Aug 2026 06:01:20 -0700 Subject: feat(post-rebuild-check): let a machine declare which units it means to leave disabled Check 2 treats "enabled" as a proxy for "will actually run", and the proxy is wrong for a unit nobody intends to enable on that box. velox has four: geoclue-agent is redundant because hyprland's exec-once starts the binary itself, emacs is started on demand by emacsclient, obs-record-watchdog only matters while recording, and obsbot-wb-guard needs an OBSBOT the laptop doesn't have. All four reported at every run. Four permanent lines in front of every real finding teach me to skim the output, which is the argument check 4 already makes about CLAUDE.md. On velox this takes the run from 8 findings to 4. The list is machine-local, read from $XDG_CONFIG_HOME/post-rebuild-check/units-expected-disabled. It can't live in the unit file, because obsbot-wb-guard is correctly enabled on ratio. One unit, a different right answer per machine. An entry whose unit is enabled after all is reported too. The main loop can't catch that, since it skips any state that isn't disabled or linked, so the list gets its own pass. Without it a stale exemption sits there suppressing nothing, and the list becomes somewhere real findings go to die. The exemption is tested last, so it never hides the dangling-symlink finding decided on the filesystem above it. --- .../post-rebuild-check/test_post_rebuild_check.py | 106 ++++++++++++++++++++- 1 file changed, 105 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/post-rebuild-check/test_post_rebuild_check.py b/tests/post-rebuild-check/test_post_rebuild_check.py index 9902299..a034f87 100644 --- a/tests/post-rebuild-check/test_post_rebuild_check.py +++ b/tests/post-rebuild-check/test_post_rebuild_check.py @@ -31,6 +31,9 @@ probe"): running; "MISSING" = not installed on this machine) PRC_REPO_REMOTES newline list of "pathorigin-url" for the push-capability check ("" = no repos to check) + PRC_UNITS_EXPECTED_DISABLED + newline list of units whose not-enabled state is + deliberate on this machine ("" = no exemptions) Run from repo root: python3 -m unittest tests.post-rebuild-check.test_post_rebuild_check @@ -51,7 +54,8 @@ CHECK = os.path.join(REPO_ROOT, "scripts", "post-rebuild-check") def run_check(failed_units="", unit_states="", local_roots="", project_roots="", signal_accounts="+15045551234", ntp_sources="162.159.200.1\npool.ntp.org", - idle_daemon="4242", repo_remotes=""): + idle_daemon="4242", repo_remotes="", + units_expected_disabled=""): """Run the script with every probe stubbed; defaults are all-clean. Roots are newline-separated. Empty means "the seam is set and names no @@ -67,6 +71,7 @@ def run_check(failed_units="", unit_states="", local_roots="", env["PRC_NTP_SOURCES"] = ntp_sources env["PRC_IDLE_DAEMON"] = idle_daemon env["PRC_REPO_REMOTES"] = repo_remotes + env["PRC_UNITS_EXPECTED_DISABLED"] = units_expected_disabled return subprocess.run( ["sh", CHECK], capture_output=True, text=True, timeout=30, env=env, ) @@ -165,6 +170,7 @@ class NtpBootstrap(unittest.TestCase): "PRC_SIGNAL_ACCOUNTS": "+15045551234", "PRC_IDLE_DAEMON": "4242", "PRC_REPO_REMOTES": "", + "PRC_UNITS_EXPECTED_DISABLED": "", "PRC_CHRONY_CONF": chrony_conf}) env.pop("PRC_NTP_SOURCES", None) return subprocess.run(["sh", CHECK], capture_output=True, text=True, @@ -281,6 +287,100 @@ class IdleDaemon(unittest.TestCase): run_check(idle_daemon=pids).stdout.lower()) +class UnitsExpectedDisabled(unittest.TestCase): + """Check 2 — units nothing intends to enable on this machine. + + "Enabled" is the check's proxy for "will actually run", and the proxy is + wrong for a unit nobody means to enable here. velox carries four such + units, for four different reasons: geoclue-agent is redundant because + hyprland's exec-once starts the binary directly, emacs is started on demand + by emacsclient, obs-record-watchdog only matters while recording, and + obsbot-wb-guard needs an OBSBOT the machine doesn't have. + + Left unexempted they report at every run, which is the standing-findings + problem check 4's own comment already argues against: four permanent lines + in front of every real one teach you to skim the output. + + The exemption is machine-local rather than a marker in the shared unit + file, because obsbot-wb-guard is correctly ENABLED on ratio. Same unit, + different right answer per machine. + """ + + # --- Normal cases --------------------------------------------------- + + def test_an_exempt_unit_is_not_flagged(self): + r = run_check(unit_states="emacs.service linked", + units_expected_disabled="emacs.service") + self.assertEqual(r.returncode, 0, r.stdout) + self.assertNotIn("emacs.service", r.stdout) + + def test_a_non_exempt_unit_still_flags(self): + r = run_check(unit_states="roam-sync.timer linked", + units_expected_disabled="emacs.service") + self.assertEqual(r.returncode, 1) + self.assertIn("roam-sync.timer", r.stdout) + + def test_several_exemptions_all_apply(self): + r = run_check( + unit_states=("emacs.service linked\n" + "geoclue-agent.service linked\n" + "obsbot-wb-guard.service linked"), + units_expected_disabled=("emacs.service\n" + "geoclue-agent.service\n" + "obsbot-wb-guard.service")) + self.assertEqual(r.returncode, 0, r.stdout) + + # --- Boundary cases ------------------------------------------------- + + def test_an_exemption_that_is_actually_enabled_is_a_finding(self): + # A stale exemption must surface rather than sit there suppressing + # nothing. Otherwise the list rots into a place real findings go to + # die, which is worse than the noise it was added to remove. + r = run_check(unit_states="obsbot-wb-guard.service enabled", + units_expected_disabled="obsbot-wb-guard.service") + self.assertEqual(r.returncode, 1) + self.assertIn("obsbot-wb-guard.service", r.stdout) + + def test_comments_and_blank_lines_are_ignored(self): + # The reason a unit is exempt is the most useful thing about the + # entry, so the format has to hold a comment next to it. + r = run_check(unit_states="emacs.service linked", + units_expected_disabled=("# started on demand\n" + "\n" + "emacs.service # not by systemd\n")) + self.assertEqual(r.returncode, 0, r.stdout) + + def test_no_exemptions_flags_everything_as_before(self): + r = run_check(unit_states="emacs.service linked", + units_expected_disabled="") + self.assertEqual(r.returncode, 1) + self.assertIn("emacs.service", r.stdout) + + def test_an_exemption_does_not_suppress_a_dangling_link(self): + # A stowed unit pointing at a missing target is a different finding, + # decided on the filesystem. Exempting the name must not hide that. + d = tempfile.mkdtemp(prefix="prc-units-") + self.addCleanup(shutil.rmtree, d, True) + unit_dir = os.path.join(d, "systemd", "user") + os.makedirs(unit_dir) + link = os.path.join(unit_dir, "emacs.service") + os.symlink(os.path.join(d, "gone.service"), link) + env = dict(os.environ) + env.update({"PRC_FAILED_UNITS": "", "PRC_LOCAL_SCAN_ROOTS": "", + "PRC_PROJECT_ROOTS": "", + "PRC_SIGNAL_ACCOUNTS": "+15045551234", + "PRC_NTP_SOURCES": "162.159.200.1", + "PRC_IDLE_DAEMON": "4242", + "PRC_REPO_REMOTES": "", + "PRC_UNITS_EXPECTED_DISABLED": "emacs.service", + "XDG_CONFIG_HOME": d}) + env.pop("PRC_UNIT_STATES", None) + r = subprocess.run(["sh", CHECK], capture_output=True, text=True, + timeout=30, env=env) + self.assertIn("points at a missing target", r.stdout) + self.assertEqual(r.returncode, 1) + + class RepoPushCapability(unittest.TestCase): """Check 8 — a working repo cloned from the read-only endpoint. @@ -797,6 +897,7 @@ class SignalAccount(unittest.TestCase): "PRC_SIGNAL_ACCOUNTS": "+15045551234", "PRC_IDLE_DAEMON": "4242", "PRC_REPO_REMOTES": "", + "PRC_UNITS_EXPECTED_DISABLED": "", "signal_missing": "1"}) r = subprocess.run(["sh", CHECK], capture_output=True, text=True, timeout=30, env=env) @@ -867,6 +968,7 @@ class ProbeFailure(unittest.TestCase): "PRC_SIGNAL_ACCOUNTS": "+15045551234", "PRC_IDLE_DAEMON": "4242", "PRC_REPO_REMOTES": "", + "PRC_UNITS_EXPECTED_DISABLED": "", "TMPDIR": "/nonexistent-tmp-dir"}) r = subprocess.run(["sh", CHECK], capture_output=True, text=True, timeout=30, env=env) @@ -890,6 +992,7 @@ class RealUnitDirEnumeration(unittest.TestCase): "PRC_SIGNAL_ACCOUNTS": "+15045551234", "PRC_IDLE_DAEMON": "4242", "PRC_REPO_REMOTES": "", + "PRC_UNITS_EXPECTED_DISABLED": "", "XDG_CONFIG_HOME": config_home}) env.pop("PRC_UNIT_STATES", None) return subprocess.run(["sh", CHECK], capture_output=True, text=True, @@ -945,6 +1048,7 @@ class WedgedSystemctl(unittest.TestCase): "PRC_SIGNAL_ACCOUNTS": "+15045551234", "PRC_IDLE_DAEMON": "4242", "PRC_REPO_REMOTES": "", + "PRC_UNITS_EXPECTED_DISABLED": "", "PRC_SYSTEMCTL": fake, "PRC_SYSTEMCTL_TIMEOUT": timeout_s, "XDG_CONFIG_HOME": d}) -- cgit v1.2.3