From be47247da7ff9113d355a9d07552d53d22cc1f7a Mon Sep 17 00:00:00 2001 From: Matthias Johnson Date: Sun, 15 Feb 2026 21:55:09 -0700 Subject: [PATCH] Fix test cleanup destroying user jobs, consolidate code - Test cleanup now only removes jobs it created (tracked via test_job_ids array) instead of nuking all systab_* units. Fixes bug where running tests would delete real user jobs. - Fix extract_id subshell issue: array appends in $() don't propagate to parent, so use _extracted_id variable instead. - Merge disable_job_by_id/enable_job_by_id into toggle_job_by_id. - Update usage text: -D/-E/-L/-S now show consistently. - Fix pre-commit hook sed regex that only captured last digit of multi-digit numbers; replaced with grep -oP. Co-Authored-By: Claude Opus 4.6 --- .githooks/pre-commit | 2 +- README.md | 10 ++++---- systab | 55 +++++++++++++++++++------------------------- test.sh | 46 ++++++++++++++++++++---------------- 4 files changed, 56 insertions(+), 57 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 73687f6..39e69cc 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -27,7 +27,7 @@ fi echo "Running tests..." if output=$(./test.sh 2>&1); then echo "$output" - count=$(sed -n 's/.*\([0-9]\+\) tests: \([0-9]\+\) passed.*/\2 passed/p' <<< "$output") + count=$(grep -oP '\d+ passed' <<< "$output" | tail -1) write_badge "${count:-passing}" "brightgreen" else echo "$output" diff --git a/README.md b/README.md index cf7f062..7896b19 100644 --- a/README.md +++ b/README.md @@ -167,12 +167,12 @@ Job Creation: -m Send email notification to address (via sendmail) -o [lines] Include job output in notifications (default: 10 lines) -Management: - -D Disable a job - -E Enable a disabled job +Management (accept hex ID or name): + -D Disable a job + -E Enable a disabled job -e Edit jobs in crontab-like format - -L [id] [filter] List job logs (optionally for a specific job and/or filtered) - -S [id] Show status of all managed jobs (or a specific job) + -L [id|name] [filter] List job logs (optionally for a specific job and/or filtered) + -S [id|name] Show status of all managed jobs (or a specific job) -C Clean up completed one-time jobs -h Show help ``` diff --git a/systab b/systab index 0c97e58..3a36de2 100755 --- a/systab +++ b/systab @@ -42,12 +42,12 @@ Job Creation Options: -m Send email notification to address (via sendmail) -o [lines] Include job output in notifications (default: 10 lines) -Management Options: - -D Disable a job - -E Enable a disabled job +Management Options (accept hex ID or name): + -D Disable a job + -E Enable a disabled job -e Edit jobs in crontab-like format - -L [id] [filter] List job logs (optionally for a specific job and/or filtered) - -S [id] Show status of all managed jobs (or a specific job) + -L [id|name] [filter] List job logs (optionally for a specific job and/or filtered) + -S [id|name] Show status of all managed jobs (or a specific job) -C Clean up completed one-time jobs TIME FORMATS: @@ -267,36 +267,29 @@ validate_job_id() { grep -q "^$MARKER" "$timer_file" 2>/dev/null || error "Not a managed job: $id" } -# Disable a job by short ID or name -disable_job_by_id() { - validate_job_id "$1" +# Toggle a job's enabled state by short ID or name +# Usage: toggle_job_by_id +toggle_job_by_id() { + local input="$1" action="$2" + validate_job_id "$input" local id="$_resolved_id" local name name=$(get_job_name "$SYSTEMD_USER_DIR/${_job_name}.service") local label label=$(format_job_id "$id" "$name") - if ! is_job_enabled "$_job_name"; then - echo "Already disabled: $label" - return + if [[ "$action" == "disable" ]]; then + if ! is_job_enabled "$_job_name"; then + echo "Already disabled: $label"; return + fi + disable_job "$_job_name" + echo "Disabled: $label" + else + if is_job_enabled "$_job_name"; then + echo "Already enabled: $label"; return + fi + enable_job "$_job_name" + echo "Enabled: $label" fi - disable_job "$_job_name" - echo "Disabled: $label" -} - -# Enable a job by short ID or name -enable_job_by_id() { - validate_job_id "$1" - local id="$_resolved_id" - local name - name=$(get_job_name "$SYSTEMD_USER_DIR/${_job_name}.service") - local label - label=$(format_job_id "$id" "$name") - if is_job_enabled "$_job_name"; then - echo "Already enabled: $label" - return - fi - enable_job "$_job_name" - echo "Enabled: $label" } # Get all managed unit files of a given type (service or timer) @@ -1120,9 +1113,9 @@ main() { # Determine mode based on options if [[ -n "$opt_disable" ]]; then - disable_job_by_id "$opt_disable" + toggle_job_by_id "$opt_disable" disable elif [[ -n "$opt_enable" ]]; then - enable_job_by_id "$opt_enable" + toggle_job_by_id "$opt_enable" enable elif $opt_edit; then edit_jobs elif $opt_list; then diff --git a/test.sh b/test.sh index 0cfd1f5..fa7215f 100755 --- a/test.sh +++ b/test.sh @@ -94,22 +94,29 @@ assert_file_contains() { fi } +# Track test-created job IDs for targeted cleanup +test_job_ids=() + # Extract job ID from "Job created: " or "Job created: ()" output +# Sets _extracted_id and appends to test_job_ids for cleanup tracking extract_id() { - sed -n 's/^Job created: \([0-9a-f]\{6\}\)\( .*\)\{0,1\}$/\1/p' <<< "$_last_output" + _extracted_id=$(sed -n 's/^Job created: \([0-9a-f]\{6\}\)\( .*\)\{0,1\}$/\1/p' <<< "$_last_output") + test_job_ids+=("$_extracted_id") } -# Remove all systab_* unit files and reload +# Remove only test-created systab units and reload cleanup() { local had_units=false - for f in "$SYSTEMD_USER_DIR"/systab_*.service "$SYSTEMD_USER_DIR"/systab_*.timer; do - [[ -f "$f" ]] || continue - local unit - unit=$(basename "$f") - systemctl --user stop "$unit" 2>/dev/null || true - systemctl --user disable "$unit" 2>/dev/null || true - rm -f "$f" - had_units=true + for id in "${test_job_ids[@]}"; do + local name="systab_${id}" + for ext in service timer; do + local f="$SYSTEMD_USER_DIR/${name}.${ext}" + [[ -f "$f" ]] || continue + systemctl --user stop "${name}.${ext}" 2>/dev/null || true + systemctl --user disable "${name}.${ext}" 2>/dev/null || true + rm -f "$f" + had_units=true + done done if $had_units; then systemctl --user daemon-reload 2>/dev/null || true @@ -120,7 +127,6 @@ cleanup() { _last_output="" trap cleanup EXIT -cleanup echo "${BOLD}Running systab tests...${RESET}" echo "" @@ -132,10 +138,10 @@ echo "" echo "${BOLD}--- Job creation ---${RESET}" assert_output "create recurring job" "Job created:" $SYSTAB -t "every 5 minutes" -c "echo test_recurring" -id_recurring=$(extract_id) +extract_id; id_recurring=$_extracted_id assert_output "create one-time job" "Job created:" $SYSTAB -t "in 30 minutes" -c "echo test_onetime" -id_onetime=$(extract_id) +extract_id; id_onetime=$_extracted_id if [[ -z "$id_recurring" || -z "$id_onetime" ]]; then echo "FATAL: could not extract job IDs, aborting" @@ -190,33 +196,33 @@ echo "" echo "${BOLD}--- Notifications ---${RESET}" assert_output "create with -i" "Job created:" $SYSTAB -t "every 10 minutes" -c "echo notify_test" -i -id_notify=$(extract_id) +extract_id; id_notify=$_extracted_id assert_file_contains "-i service has ExecStopPost" \ "$SYSTEMD_USER_DIR/systab_${id_notify}.service" "^ExecStopPost=" assert_file_contains "-i service has notify-send" \ "$SYSTEMD_USER_DIR/systab_${id_notify}.service" "notify-send" assert_output "create with -i -o" "Job created:" $SYSTAB -t "every 10 minutes" -c "echo output_test" -i -o -id_output=$(extract_id) +extract_id; id_output=$_extracted_id assert_file_contains "-i -o service has journalctl" \ "$SYSTEMD_USER_DIR/systab_${id_output}.service" "journalctl" assert_file_contains "-i -o service has %%s" \ "$SYSTEMD_USER_DIR/systab_${id_output}.service" "%%s" assert_output "create with -i -o 5" "Job created:" $SYSTAB -t "every 10 minutes" -c "echo output5_test" -i -o 5 -id_output5=$(extract_id) +extract_id; id_output5=$_extracted_id assert_file_contains "-i -o 5 service has -n 5" \ "$SYSTEMD_USER_DIR/systab_${id_output5}.service" "-n 5" # Email notification (only if sendmail/msmtp available) if command -v sendmail &>/dev/null || command -v msmtp &>/dev/null; then assert_output "create with -m" "Job created:" $SYSTAB -t "every 10 minutes" -c "echo mail_test" -m test@example.com - id_mail=$(extract_id) + extract_id; id_mail=$_extracted_id assert_file_contains "-m service has ExecStopPost" \ "$SYSTEMD_USER_DIR/systab_${id_mail}.service" "^ExecStopPost=" assert_output "create with -i -m" "Job created:" $SYSTAB -t "every 10 minutes" -c "echo both_test" -i -m test@example.com - id_both=$(extract_id) + extract_id; id_both=$_extracted_id local_count=$(grep -c "^ExecStopPost=" "$SYSTEMD_USER_DIR/systab_${id_both}.service") if [[ "$local_count" -ge 2 ]]; then pass "-i -m service has two ExecStopPost lines" @@ -279,7 +285,7 @@ echo "" echo "${BOLD}--- Job names ---${RESET}" assert_output "create job with name" "Job created:" $SYSTAB -t "every 10 minutes" -c "echo named_test" -n mytest -id_named=$(extract_id) +extract_id; id_named=$_extracted_id assert_last_output_contains "name appears in creation output" "(mytest)" assert_file_contains "service file has SYSTAB_NAME" \ @@ -316,7 +322,7 @@ assert_failure "invalid job ID for -E" $SYSTAB -E "zzzzzz" # -o without -i or -m: should succeed (flag accepted, just no notification lines) assert_output "-o without -i/-m creates job" "Job created:" $SYSTAB -t "every 10 minutes" -c "echo bare_output" -o -id_bare_o=$(extract_id) +extract_id; id_bare_o=$_extracted_id # Should have FLAGS comment but no ExecStopPost assert_file_contains "-o without -i/-m has FLAGS comment" \ "$SYSTEMD_USER_DIR/systab_${id_bare_o}.service" "SYSTAB_FLAGS=o"