diff options
| author | Craig Jennings <c@cjennings.net> | 2026-06-27 13:44:13 -0400 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-06-27 13:44:13 -0400 |
| commit | ba07886525acfb78999d98c4c1a06bc4c08e8d95 (patch) | |
| tree | 1afe0919cbf0793a241ef004a0b9b0d8114b3f8f | |
| parent | e33abe6fd6bde995c3f067ca023dd8f158e8cfe9 (diff) | |
| download | archsetup-ba07886525acfb78999d98c4c1a06bc4c08e8d95.tar.gz archsetup-ba07886525acfb78999d98c4c1a06bc4c08e8d95.zip | |
refactor: collapse describe-run-warn idiom into run_task helper
The installer announced, ran, and warned on each operation with a
hand-written two-line pair, repeated ~35 times:
action="enabling rngd service" && display "task" "$action"
systemctl enable rngd >> "$logfile" 2>&1 || error_warn "$action" "$?"
I added a run_task "desc" cmd... helper that does this in one line, plus an
enable_service wrapper for the canonical "enabling <unit> service" case. The
35 single-command sites now call run_task. The three exact-wording service
enables (rngd, upower, fail2ban) use enable_service. Multi-line sites
(heredocs, subshells, intervening logic) keep the explicit form.
Behavior is unchanged: same descriptions, same commands, same logfile
redirection, same non-fatal warning on the real exit code. tests/run-task
covers the helper across Normal/Boundary/Error including exit-code
propagation, and the full unit suite stays green.
| -rwxr-xr-x | archsetup | 128 | ||||
| -rw-r--r-- | tests/run-task/test_run_task.py | 172 |
2 files changed, 230 insertions, 70 deletions
@@ -595,6 +595,29 @@ display() { ### Installation Helpers +# Describe-run-warn primitive. Announces a task, runs the command with +# stdout+stderr appended to $logfile, and on failure logs a non-fatal +# warning carrying the command's real exit code. Replaces the recurring +# action="desc" && display "task" "$action" +# cmd >> "$logfile" 2>&1 || error_warn "$action" "$?" +# idiom with a single call: +# run_task "desc" cmd arg... +run_task() { + local desc="$1" + shift + display "task" "$desc" + "$@" >> "$logfile" 2>&1 || error_warn "$desc" "$?" +} + +# Enable one or more systemd units with the conventional wording. +# Each unit is announced and warned independently via run_task. +enable_service() { + local unit + for unit in "$@"; do + run_task "enabling $unit service" systemctl enable "$unit" + done +} + MAX_INSTALL_RETRIES=3 retry_install() { local pkg="$1" @@ -1154,8 +1177,7 @@ user_customizations() { pacman_install fontconfig # Refresh font cache for any fonts in dotfiles - action="refreshing font cache" && display "task" "$action" - fc-cache -f >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "refreshing font cache" fc-cache -f # install desktop-file-utils before updating database (provides update-desktop-database) pacman_install desktop-file-utils @@ -1202,8 +1224,7 @@ EOF # (e.g. the /etc/skel .bashrc/.bash_profile a fresh user starts with). Runs # for every desktop_env, including none — minimal/ ships those skel-colliding # files too, so its --adopt needs the same restore. - action="restoring dotfile versions" && display "task" "$action" - git -C "$dotfiles_dir" restore . >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "restoring dotfile versions" git -C "$dotfiles_dir" restore . action="creating common directories" && display "task" "$action" # Create default directories and grant permissions @@ -1259,17 +1280,14 @@ essential_services() { display "subtitle" "Randomness" pacman_install rng-tools - action="enabling rngd service" && display "task" "$action" - systemctl enable rngd >> "$logfile" 2>&1 || error_warn "$action" "$?" - action="starting rngd service" && display "task" "$action" - systemctl start rngd >> "$logfile" 2>&1 || error_warn "$action" "$?" + enable_service rngd + run_task "starting rngd service" systemctl start rngd # Networking display "subtitle" "Networking" pacman_install networkmanager - action="enabling NetworkManager" && display "task" "$action" - systemctl enable NetworkManager.service >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling NetworkManager" systemctl enable NetworkManager.service action="configuring MAC address randomization" && display "task" "$action" mkdir -p /etc/NetworkManager/conf.d @@ -1319,28 +1337,23 @@ EOF # Note: If Docker containers have DNS issues, systemd-resolved's stub resolver # (127.0.0.53) may be the cause. Fix: configure Docker to use direct DNS, or # disable systemd-resolved and use /etc/resolv.conf directly. (2026-01-18) - action="enabling systemd-resolved" && display "task" "$action" - systemctl enable systemd-resolved >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling systemd-resolved" systemctl enable systemd-resolved # Create resolv.conf symlink to systemd-resolved - action="linking resolv.conf to systemd-resolved" && display "task" "$action" - ln -sf /run/systemd/resolve/stub-resolv.conf /etc/resolv.conf >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "linking resolv.conf to systemd-resolved" ln -sf /run/systemd/resolve/stub-resolv.conf /etc/resolv.conf # Power display "subtitle" "Power" pacman_install upower - action="enabling upower service" && display "task" "$action" - systemctl enable upower >> "$logfile" 2>&1 || error_warn "$action" "$?" + enable_service upower # Secure Shell display "subtitle" "Secure Shell" pacman_install openssh - action="enabling the openssh service to run at boot" && display "task" "$action" - systemctl enable sshd >> "$logfile" 2>&1 || error_warn "$action" "$?" - action="starting the openssh service" && display "task" "$action" - systemctl start sshd >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling the openssh service to run at boot" systemctl enable sshd + run_task "starting the openssh service" systemctl start sshd action="hardening sshd (root login by key only)" && display "task" "$action" cat << 'EOF' > /etc/ssh/sshd_config.d/10-hardening.conf @@ -1373,16 +1386,13 @@ maxretry = 3 bantime = 1h EOF - action="enabling fail2ban service" && display "task" "$action" - systemctl enable fail2ban >> "$logfile" 2>&1 || error_warn "$action" "$?" - action="starting fail2ban service" && display "task" "$action" - systemctl start fail2ban >> "$logfile" 2>&1 || error_warn "$action" "$?" + enable_service fail2ban + run_task "starting fail2ban service" systemctl start fail2ban display "subtitle" "Firewall" pacman_install ufw - action="configuring ufw to deny by default" && display "task" "$action" - ufw default deny incoming >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "configuring ufw to deny by default" ufw default deny incoming # Firewall rules - only open ports for services we actually run for protocol in \ @@ -1410,11 +1420,9 @@ EOF action="rate-limiting SSH to protect from brute force attacks" && display "task" "$action" (ufw limit 22/tcp >> "$logfile" 2>&1) || error_warn "$action" "$?" - action="enabling firewall" && display "task" "$action" - ufw --force enable >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling firewall" ufw --force enable - action="enabling firewall service to launch on boot" && display "task" "$action" - systemctl enable ufw.service >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling firewall service to launch on boot" systemctl enable ufw.service # Verify firewall is actually active # Note: In VM environments, UFW may show inactive due to missing kernel @@ -1436,17 +1444,14 @@ EOF display "task" "skipping avahi (already running)" else pacman_install avahi # service discovery on a local network using mdns - action="enabling avahi for mDNS discovery" && display "task" "$action" - systemctl enable avahi-daemon.service >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling avahi for mDNS discovery" systemctl enable avahi-daemon.service fi pacman_install wsdd - action="enabling wsdd for Windows network discovery" && display "task" "$action" - systemctl enable wsdd.service >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling wsdd for Windows network discovery" systemctl enable wsdd.service pacman_install geoclue # geolocation service for location-aware apps - action="enabling geoclue geolocation service" && display "task" "$action" - systemctl enable geoclue.service >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling geoclue geolocation service" systemctl enable geoclue.service # Enable BeaconDB as geoclue wifi location provider (default MLS/Ichnaea API is defunct) action="configuring geoclue to use BeaconDB location service" && display "task" "$action" @@ -1480,11 +1485,9 @@ EOF display "subtitle" "Job Scheduling" pacman_install cronie - action="enabling cronie to launch at boot" && display "task" "$action" - systemctl enable cronie >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling cronie to launch at boot" systemctl enable cronie pacman_install at - action="enabling the batch delayed command scheduler" && display "task" "$action" - systemctl enable atd >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling the batch delayed command scheduler" systemctl enable atd action="installing log cleanup cron job" && display "task" "$action" (sudo -u "$username" crontab -l 2>/dev/null; \ @@ -1496,8 +1499,7 @@ EOF display "subtitle" "Package Repository Cache Maintenance" pacman_install pacman-contrib - action="enabling the package cache cleanup timer" && display "task" "$action" - systemctl enable --now paccache.timer >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling the package cache cleanup timer" systemctl enable --now paccache.timer action="configuring paccache to keep 3 versions" && display "task" "$action" backup_system_file /etc/conf.d/pacman-contrib @@ -1610,8 +1612,7 @@ Persistent=true WantedBy=timers.target EOF - action="enabling sanoid timer" && display "task" "$action" - systemctl enable sanoid.timer >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling sanoid timer" systemctl enable sanoid.timer action="enabling weekly ZFS scrub" && display "task" "$action" # Get pool name dynamically (usually zroot) @@ -1665,16 +1666,13 @@ EOF snapper -c root set-config "TIMELINE_LIMIT_MONTHLY=1" >> "$logfile" 2>&1 snapper -c root set-config "TIMELINE_LIMIT_YEARLY=0" >> "$logfile" 2>&1 - action="enabling snapper timeline timer" && display "task" "$action" - systemctl enable snapper-timeline.timer >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling snapper timeline timer" systemctl enable snapper-timeline.timer systemctl enable snapper-cleanup.timer >> "$logfile" 2>&1 || error_warn "$action" "$?" - action="enabling grub-btrfsd for boot menu snapshots" && display "task" "$action" - systemctl enable grub-btrfsd >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling grub-btrfsd for boot menu snapshots" systemctl enable grub-btrfsd # Allow user to use snapper without root (required for snapper-gui) - action="allowing wheel group to use snapper" && display "task" "$action" - snapper -c root set-config "ALLOW_GROUPS=wheel" >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "allowing wheel group to use snapper" snapper -c root set-config "ALLOW_GROUPS=wheel" snapper -c root set-config "SYNC_ACL=yes" >> "$logfile" 2>&1 || error_warn "$action" "$?" # Set ACL on .snapshots directory for wheel group access setfacl -m g:wheel:rx /.snapshots >> "$logfile" 2>&1 || error_warn "$action" "$?" @@ -1692,8 +1690,7 @@ EOF # user-level IMAP/SMTP daemons over SSH or from remote agents. display "subtitle" "User Services" - action="enabling user-services lingering for $username" && display "task" "$action" - loginctl enable-linger "$username" >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling user-services lingering for $username" loginctl enable-linger "$username" } ### Xorg Display Manager @@ -1718,8 +1715,7 @@ Section "ServerFlags" Option "DontZap" "True" EndSection EOF - action="configuring xorg server" && display "task" "$action" - chmod 644 /etc/X11/xorg.conf.d/00-no-vt-or-zap.conf >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "configuring xorg server" chmod 644 /etc/X11/xorg.conf.d/00-no-vt-or-zap.conf # Install GPU-specific drivers install_gpu_drivers @@ -1968,8 +1964,7 @@ desktop_environment() { pacman_install "$software" done pacman_install solaar # Logitech device manager - action="enabling bluetooth to launch at boot" && display "task" "$action" - systemctl enable bluetooth.service >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling bluetooth to launch at boot" systemctl enable bluetooth.service # Command Line Utilities @@ -2085,8 +2080,7 @@ gaming() { pacman_install steam # Enable gamemode service for user - action="enabling gamemode for user" && display "task" "$action" - sudo -u "$username" systemctl --user enable gamemoded.service >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling gamemode for user" sudo -u "$username" systemctl --user enable gamemoded.service } ### Zig Toolchain Pin @@ -2327,8 +2321,7 @@ developer_workstation() { pacman_install proton-vpn-gtk-app # Proton VPN GUI client with system tray pacman_install tailscale # mesh VPN - run 'tailscale up' to authenticate - action="enabling tailscale service" && display "task" "$action" - systemctl enable tailscaled >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling tailscale service" systemctl enable tailscaled action="DevOps Utilities" && display "subtitle" "$action" @@ -2357,8 +2350,7 @@ developer_workstation() { } EOF fi - action="enabling docker service to launch on boot" && display "task" "$action" - systemctl enable docker.service >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling docker service to launch on boot" systemctl enable docker.service # podman (rootless containers for winvm) pacman_install podman @@ -2496,8 +2488,7 @@ supplemental_software() { # makepkg's integrity check fails on that file even though the package tarball # itself verifies. Rechecked 2026-06-24 — the original expired-PGP-signature # cause is gone, but this LICENSE-drift keeps the workaround necessary. - action="installing python-lyricsgenius (integrity workaround)" && display "task" "$action" - yay -S --noconfirm --mflags --skipinteg python-lyricsgenius >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "installing python-lyricsgenius (integrity workaround)" yay -S --noconfirm --mflags --skipinteg python-lyricsgenius aur_install tidal-dl # tidal-dl:tidal as yt-dlp:youtube aur_install tidaler # tidal downloader (tidal-dl-ng fork) aur_install freetube # privacy-focused YouTube desktop client @@ -2610,8 +2601,7 @@ PLATFORM_PROFILE_ON_BAT=low-power # Off by default — uncomment (and match the BAT name) to enable. #STOP_CHARGE_THRESH_BAT1=80 EOF - action="enabling TLP service" && display "task" "$action" - systemctl enable tlp.service >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "enabling TLP service" systemctl enable tlp.service systemctl mask systemd-rfkill.service systemd-rfkill.socket >> "$logfile" 2>&1 || \ error_warn "masking systemd-rfkill for TLP" "$?" fi @@ -2633,8 +2623,7 @@ EOF linux-firmware-mellanox linux-firmware-nfp linux-firmware-nvidia \ linux-firmware-other linux-firmware-qlogic linux-firmware-radeon \ >> "$logfile" 2>&1 || error_warn "$action" "$?" - action="rebuilding initramfs after firmware trim" && display "task" "$action" - mkinitcpio -P >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "rebuilding initramfs after firmware trim" mkinitcpio -P fi # GRUB: reset timeouts, adjust log levels, larger menu for HiDPI screens, and show splashscreen @@ -2654,8 +2643,7 @@ EOF # Regenerate GRUB config after all modifications if [ -f /etc/default/grub ]; then - action="generating grub configuration" && display "task" "$action" - grub-mkconfig -o /boot/grub/grub.cfg >> "$logfile" 2>&1 || error_warn "$action" "$?" + run_task "generating grub configuration" grub-mkconfig -o /boot/grub/grub.cfg fi } diff --git a/tests/run-task/test_run_task.py b/tests/run-task/test_run_task.py new file mode 100644 index 0000000..35036dd --- /dev/null +++ b/tests/run-task/test_run_task.py @@ -0,0 +1,172 @@ +"""Tests for the run_task / enable_service helpers in the archsetup installer. + +run_task is the installer's describe-run-warn primitive. It replaces the +hand-written idiom that recurs ~100 times across the script: + + action="enabling rngd service" && display "task" "$action" + systemctl enable rngd >> "$logfile" 2>&1 || error_warn "$action" "$?" + +as a single call: + + run_task "enabling rngd service" systemctl enable rngd + +It announces the task via display, runs the command with stdout+stderr +appended to $logfile, and on failure calls error_warn with the command's +real exit code (non-fatal). enable_service is a thin wrapper that enables +one or more systemd units with the conventional "enabling <unit> service" +wording. + +These tests exercise the REAL function bodies, extracted from the +`archsetup` script at run time (not a copy), with recording stubs standing +in for display, error_warn, and systemctl. The command run by run_task is +genuinely executed. + +Run from repo root: + python3 -m unittest tests.run-task.test_run_task +""" + +import os +import shutil +import subprocess +import tempfile +import unittest + + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +ARCHSETUP = os.path.join(REPO_ROOT, "archsetup") + +# A bash harness that sources the real run_task + enable_service out of the +# installer, with recording stubs for their dependencies. Each stub appends a +# tab-separated record to a file named by an env var, so the Python side can +# assert what was called. The real command passed to run_task still runs. +WRAPPER = r"""#!/bin/bash +ARCHSETUP="$1"; shift +logfile="$LOGFILE" + +display() { printf '%s\t%s\n' "$1" "$2" >> "$DISPLAY_LOG"; } +error_warn() { printf '%s\t%s\n' "$1" "$2" >> "$ERRWARN_LOG"; return 1; } +systemctl() { printf 'systemctl %s\n' "$*"; } + +source <(sed -n '/^run_task() {/,/^}/p' "$ARCHSETUP") +source <(sed -n '/^enable_service() {/,/^}/p' "$ARCHSETUP") + +"$@" +""" + + +class RunTaskHarness(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="run-task-test-") + self.wrapper = os.path.join(self.tmp, "run.sh") + with open(self.wrapper, "w") as f: + f.write(WRAPPER) + os.chmod(self.wrapper, 0o755) + self.logfile = os.path.join(self.tmp, "install.log") + self.display_log = os.path.join(self.tmp, "display.log") + self.errwarn_log = os.path.join(self.tmp, "errwarn.log") + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def call(self, *args): + env = dict(os.environ) + env["LOGFILE"] = self.logfile + env["DISPLAY_LOG"] = self.display_log + env["ERRWARN_LOG"] = self.errwarn_log + return subprocess.run( + ["bash", self.wrapper, ARCHSETUP, *args], + capture_output=True, text=True, timeout=10, env=env, + ) + + def read(self, path): + if not os.path.exists(path): + return "" + with open(path) as f: + return f.read() + + # --- Normal cases ----------------------------------------------------- + + def test_run_task_success_announces_and_runs(self): + result = self.call("run_task", "doing a thing", "true") + self.assertEqual(result.returncode, 0, result.stderr) + # Announced as a "task" with the exact description. + self.assertEqual(self.read(self.display_log), "task\tdoing a thing\n") + # No warning on success. + self.assertEqual(self.read(self.errwarn_log), "") + + def test_run_task_captures_command_output_to_logfile(self): + result = self.call("run_task", "echo something", "echo", "hello-from-cmd") + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("hello-from-cmd", self.read(self.logfile)) + # Command output is logged, not printed to the terminal. + self.assertNotIn("hello-from-cmd", result.stdout) + + def test_run_task_captures_stderr_to_logfile(self): + # `ls` of a missing path writes to stderr; it must land in the logfile. + missing = os.path.join(self.tmp, "no-such-path") + self.call("run_task", "listing", "ls", missing) + self.assertIn("no-such-path", self.read(self.logfile)) + + def test_run_task_preserves_multiple_arguments(self): + self.call("run_task", "multi-arg", "printf", "%s|%s|%s", "a", "b", "c") + self.assertIn("a|b|c", self.read(self.logfile)) + + def test_run_task_preserves_arguments_with_spaces(self): + self.call("run_task", "spacey", "printf", "[%s]", "two words") + self.assertIn("[two words]", self.read(self.logfile)) + + # --- enable_service --------------------------------------------------- + + def test_enable_service_single_unit(self): + self.call("enable_service", "rngd") + self.assertEqual(self.read(self.display_log), "task\tenabling rngd service\n") + self.assertIn("systemctl enable rngd", self.read(self.logfile)) + + def test_enable_service_multiple_units(self): + self.call("enable_service", "foo", "bar", "baz") + disp = self.read(self.display_log) + self.assertIn("task\tenabling foo service\n", disp) + self.assertIn("task\tenabling bar service\n", disp) + self.assertIn("task\tenabling baz service\n", disp) + log = self.read(self.logfile) + self.assertIn("systemctl enable foo", log) + self.assertIn("systemctl enable bar", log) + self.assertIn("systemctl enable baz", log) + + # --- Error cases ------------------------------------------------------ + + def test_run_task_failure_warns_with_description(self): + result = self.call("run_task", "failing thing", "false") + self.assertNotEqual(result.returncode, 0) + self.assertEqual(self.read(self.errwarn_log), "failing thing\t1\n") + + def test_run_task_failure_propagates_real_exit_code(self): + # `bash -c 'exit 42'` must surface 42 to error_warn, not a clobbered 0. + self.call("run_task", "exit-42", "bash", "-c", "exit 42") + self.assertEqual(self.read(self.errwarn_log), "exit-42\t42\n") + + def test_enable_service_failure_warns_per_unit(self): + # Override systemctl to fail; each unit should produce a warning. + env = dict(os.environ) + env["LOGFILE"] = self.logfile + env["DISPLAY_LOG"] = self.display_log + env["ERRWARN_LOG"] = self.errwarn_log + # Re-create wrapper with a failing systemctl stub for this case. + failing = os.path.join(self.tmp, "run-fail.sh") + with open(failing, "w") as f: + f.write(WRAPPER.replace( + "systemctl() { printf 'systemctl %s\\n' \"$*\"; }", + "systemctl() { printf 'systemctl %s\\n' \"$*\"; return 1; }", + )) + os.chmod(failing, 0o755) + subprocess.run( + ["bash", failing, ARCHSETUP, "enable_service", "alpha", "beta"], + capture_output=True, text=True, timeout=10, env=env, + ) + warns = self.read(self.errwarn_log) + self.assertIn("enabling alpha service\t1\n", warns) + self.assertIn("enabling beta service\t1\n", warns) + + +if __name__ == "__main__": + unittest.main() |
