From 21f822d02de73f46a56d12193d05a13540712277 Mon Sep 17 00:00:00 2001 From: "Earl Tankard, Jr., Ph.D" Date: Sun, 12 Jul 2026 19:45:06 -0400 Subject: [PATCH] feat(flags): WI-3 --skip selective install + --only/--skip mutual exclusion (#468) ## What - bash scripts/linux/setup.sh: add ARG_SKIP_SET sentinel so --skip= (empty) exits 1 instead of falling through to a full install. Change mutual exclusion guard from -n "$ARG_ONLY" truthiness to ARG_ONLY_SET/ARG_SKIP_SET sentinels (mirrors --only pattern). Change build_final_toolset skip branch to use ARG_SKIP_SET so empty --skip= is caught by validate_csv_shape. - PS scripts/windows/setup.ps1: change mutual exclusion from ($Only -and $Skip) to $PSBoundParameters.ContainsKey('Only') -and ContainsKey('Skip'). Change elseif ($Skip) to elseif ($PSBoundParameters.ContainsKey('Skip')) -- mirrors the --only ContainsKey pattern, ensuring empty -Skip '' triggers exit 1. - tests/test_setup_flags.sh + test_setup_flags_pwsh.ps1: add 12 WI-3 tests (bash/pwsh parity): T_skip_single, T_skip_multi, T_skip_unknown, T_skip_empty, T_skip_conflict, T_skip_blank_trailing, T_skip_blank_leading, T_skip_blank_consecutive, T_list_plus_only, T_list_plus_skip, T_no_selection_persistence, T_git_hook_skip_path_safe. ## Why WI-3 (#468 flags-first): --skip=tool1,tool2 excludes tools from DEFAULT_TOOLS while preserving order; --only and --skip are mutually exclusive; --list takes precedence over both; no state leaks between independent invocations. ## Test evidence - PS: 36/36 PASS (24 WI-1/2 + 12 WI-3 new tests, including backward-compat gate) - Bash: WSL not installed in this environment; T_skip_empty confirmed RED by code analysis (ARG_SKIP_SET fix addresses it); remaining WI-3 tests expected GREEN based on WI-2 validate_csv_shape parity. - ASCII purity verified (no non-ASCII in .ps1 files). - LF line endings enforced on .sh files (no BOM). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- scripts/linux/setup.sh | 9 +- scripts/windows/setup.ps1 | 4 +- tests/test_setup_flags.sh | 174 +++++++++++++++++++++++++++++++ tests/test_setup_flags_pwsh.ps1 | 179 ++++++++++++++++++++++++++++++++ 4 files changed, 360 insertions(+), 6 deletions(-) diff --git a/scripts/linux/setup.sh b/scripts/linux/setup.sh index 253afd3..72d8d54 100755 --- a/scripts/linux/setup.sh +++ b/scripts/linux/setup.sh @@ -53,11 +53,12 @@ ARG_LIST=0 ARG_HELP=0 ARG_TOOLS_DIR="" # hidden test seam ARG_ONLY_SET=0 # tracks whether --only was explicitly provided +ARG_SKIP_SET=0 # tracks whether --skip was explicitly provided for arg in "$@"; do case "$arg" in --only=*) ARG_ONLY="${arg#--only=}"; ARG_ONLY_SET=1 ;; - --skip=*) ARG_SKIP="${arg#--skip=}" ;; + --skip=*) ARG_SKIP="${arg#--skip=}"; ARG_SKIP_SET=1 ;; --list) ARG_LIST=1 ;; --help) ARG_HELP=1 ;; --tools-dir=*) ARG_TOOLS_DIR="${arg#--tools-dir=}" ;; @@ -160,12 +161,12 @@ fi # --------------------------------------------------------------------------- # Mutual exclusion # --------------------------------------------------------------------------- -if [[ -n "$ARG_ONLY" && -n "$ARG_SKIP" ]]; then +if [[ $ARG_ONLY_SET -eq 1 && $ARG_SKIP_SET -eq 1 ]]; then log_error "--only and --skip are mutually exclusive." exit 1 fi -# (Note: ARG_ONLY_SET handles --only= with empty value; build_final_toolset validates.) +# (Note: ARG_ONLY_SET / ARG_SKIP_SET handle empty-value sentinels; build_final_toolset validates.) # --------------------------------------------------------------------------- # Build FinalToolSet -- populates global FINAL_TOOLS (bash 3.2 safe: no @@ -231,7 +232,7 @@ build_final_toolset() { FINAL_TOOLS+=("${_sorted_optin[@]}") fi - elif [[ -n "$ARG_SKIP" ]]; then + elif [[ $ARG_SKIP_SET -eq 1 ]]; then validate_csv_shape "$ARG_SKIP" local skip_list=() IFS=',' read -ra skip_list <<< "$ARG_SKIP" diff --git a/scripts/windows/setup.ps1 b/scripts/windows/setup.ps1 index d38545f..393c5e5 100644 --- a/scripts/windows/setup.ps1 +++ b/scripts/windows/setup.ps1 @@ -172,7 +172,7 @@ if ($List) { # --------------------------------------------------------------------------- # Mutual exclusion # --------------------------------------------------------------------------- -if ($Only -and $Skip) { +if ($PSBoundParameters.ContainsKey('Only') -and $PSBoundParameters.ContainsKey('Skip')) { Write-Err "-Only and -Skip are mutually exclusive." exit 1 } @@ -204,7 +204,7 @@ if ($PSBoundParameters.ContainsKey('Only')) { $optIn = @($names | Where-Object { $DefaultTools -notcontains $_ } | Sort-Object) foreach ($t in $optIn) { $FinalTools += $t } -} elseif ($Skip) { +} elseif ($PSBoundParameters.ContainsKey('Skip')) { $names = Split-ToolList -ToolList $Skip foreach ($name in $names) { if ($Available -notcontains $name) { diff --git a/tests/test_setup_flags.sh b/tests/test_setup_flags.sh index e8f20e2..5fa2581 100644 --- a/tests/test_setup_flags.sh +++ b/tests/test_setup_flags.sh @@ -407,6 +407,180 @@ else fi teardown_harness +# --------------------------------------------------------------------------- +# WI-3: --skip selective exclusion +# Stub defaults.txt order: prereqs, alpha, bravo, charlie, dotfiles, git-hook +# Opt-in stubs (in dir but NOT in defaults.txt): delta, uv +# --------------------------------------------------------------------------- + +# --------------------------------------------------------------------------- +# T_skip_single: --skip=bravo excludes bravo, installs remaining in order +# --------------------------------------------------------------------------- +echo "" +echo "--- T_skip_single ---" +setup_harness +bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" --skip=bravo 2>&1 | grep -q . || true +if assert_log_str "$(printf 'prereqs\nalpha\ncharlie\ndotfiles\ngit-hook')"; then + pass "T_skip_single: --skip=bravo excludes bravo; remaining tools installed in order" +else + fail "T_skip_single: unexpected run-log" +fi +teardown_harness + +# --------------------------------------------------------------------------- +# T_skip_multi: --skip=alpha,charlie excludes both, rest in default order +# --------------------------------------------------------------------------- +echo "" +echo "--- T_skip_multi ---" +setup_harness +bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" --skip=alpha,charlie 2>&1 | grep -q . || true +if assert_log_str "$(printf 'prereqs\nbravo\ndotfiles\ngit-hook')"; then + pass "T_skip_multi: --skip=alpha,charlie excludes both; order preserved" +else + fail "T_skip_multi: unexpected run-log" +fi +teardown_harness + +# --------------------------------------------------------------------------- +# T_skip_unknown: --skip=bogus exits 1 with helpful message +# --------------------------------------------------------------------------- +echo "" +echo "--- T_skip_unknown ---" +skip_unk_out="$(bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" --skip=bogus 2>&1)" && skip_unk_exit=$? || skip_unk_exit=$? +if [[ $skip_unk_exit -ne 0 ]]; then + if echo "$skip_unk_out" | grep -qi "unknown\|bogus\|available\|--list"; then + pass "T_skip_unknown: --skip=bogus exits non-zero with helpful message" + else + fail "T_skip_unknown: exits non-zero but message not helpful: $skip_unk_out" + fi +else + fail "T_skip_unknown: --skip=bogus exited 0 (expected non-zero)" +fi + +# --------------------------------------------------------------------------- +# T_skip_empty: --skip= exits 1 (*** EXPECTED RED before WI-3 ARG_SKIP_SET fix ***) +# --------------------------------------------------------------------------- +echo "" +echo "--- T_skip_empty ---" +bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" "--skip=" >/dev/null 2>&1 && skip_empty_exit=0 || skip_empty_exit=$? +if [[ $skip_empty_exit -ne 0 ]]; then + pass "T_skip_empty: --skip= exits non-zero" +else + fail "T_skip_empty: --skip= exited 0 (expected non-zero) *** RED until ARG_SKIP_SET fix ***" +fi + +# --------------------------------------------------------------------------- +# T_skip_conflict: --only=alpha --skip=bravo exits 1 (mutually exclusive) +# --------------------------------------------------------------------------- +echo "" +echo "--- T_skip_conflict ---" +bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" --only=alpha --skip=bravo >/dev/null 2>&1 && skip_conflict_exit=0 || skip_conflict_exit=$? +if [[ $skip_conflict_exit -ne 0 ]]; then + pass "T_skip_conflict: --only + --skip exits non-zero (mutually exclusive)" +else + fail "T_skip_conflict: --only + --skip exited 0 (expected non-zero)" +fi + +# --------------------------------------------------------------------------- +# T_skip_blank_trailing: --skip=alpha, exits 1 +# --------------------------------------------------------------------------- +echo "" +echo "--- T_skip_blank_trailing ---" +bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" "--skip=alpha," >/dev/null 2>&1 && sbt_exit=0 || sbt_exit=$? +if [[ $sbt_exit -ne 0 ]]; then + pass "T_skip_blank_trailing: --skip=alpha, exits non-zero" +else + fail "T_skip_blank_trailing: --skip=alpha, exited 0 (expected non-zero)" +fi + +# --------------------------------------------------------------------------- +# T_skip_blank_leading: --skip=,alpha exits 1 +# --------------------------------------------------------------------------- +echo "" +echo "--- T_skip_blank_leading ---" +bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" "--skip=,alpha" >/dev/null 2>&1 && sbl_exit=0 || sbl_exit=$? +if [[ $sbl_exit -ne 0 ]]; then + pass "T_skip_blank_leading: --skip=,alpha exits non-zero" +else + fail "T_skip_blank_leading: --skip=,alpha exited 0 (expected non-zero)" +fi + +# --------------------------------------------------------------------------- +# T_skip_blank_consecutive: --skip=alpha,,bravo exits 1 +# --------------------------------------------------------------------------- +echo "" +echo "--- T_skip_blank_consecutive ---" +bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" "--skip=alpha,,bravo" >/dev/null 2>&1 && sbc_exit=0 || sbc_exit=$? +if [[ $sbc_exit -ne 0 ]]; then + pass "T_skip_blank_consecutive: --skip=alpha,,bravo exits non-zero" +else + fail "T_skip_blank_consecutive: --skip=alpha,,bravo exited 0 (expected non-zero)" +fi + +# --------------------------------------------------------------------------- +# T_list_plus_only: --list --only=alpha exits 0 (--list takes precedence) +# --------------------------------------------------------------------------- +echo "" +echo "--- T_list_plus_only ---" +setup_harness +lpo_out="$(bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" --list --only=alpha 2>&1)" && lpo_exit=$? || lpo_exit=$? +if [[ $lpo_exit -eq 0 ]] && assert_contains "$lpo_out" "alpha" && [[ ! -s "$RUN_LOG" ]]; then + pass "T_list_plus_only: --list wins over --only (exits 0, list printed, no install)" +else + fail "T_list_plus_only: --list did not win over --only (exit=$lpo_exit)" + echo " Output: $lpo_out" +fi +teardown_harness + +# --------------------------------------------------------------------------- +# T_list_plus_skip: --list --skip=alpha exits 0 (--list takes precedence) +# --------------------------------------------------------------------------- +echo "" +echo "--- T_list_plus_skip ---" +setup_harness +lps_out="$(bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" --list --skip=alpha 2>&1)" && lps_exit=$? || lps_exit=$? +if [[ $lps_exit -eq 0 ]] && assert_contains "$lps_out" "alpha" && [[ ! -s "$RUN_LOG" ]]; then + pass "T_list_plus_skip: --list wins over --skip (exits 0, list printed, no install)" +else + fail "T_list_plus_skip: --list did not win over --skip (exit=$lps_exit)" + echo " Output: $lps_out" +fi +teardown_harness + +# --------------------------------------------------------------------------- +# T_no_selection_persistence: prior --only run does NOT poison a later no-arg run +# --------------------------------------------------------------------------- +echo "" +echo "--- T_no_selection_persistence ---" +setup_harness +bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" --only=alpha 2>&1 | grep -q . || true +teardown_harness +setup_harness +bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" 2>&1 | grep -q . || true +if assert_log_equals "${STUB_DIR}/defaults.txt"; then + pass "T_no_selection_persistence: no-arg run after --only run installs full defaults" +else + fail "T_no_selection_persistence: no-arg run after --only run did not produce full defaults" +fi +teardown_harness + +# --------------------------------------------------------------------------- +# T_git_hook_skip_path_safe: --skip=git-hook succeeds; git-hook not in run-log +# --------------------------------------------------------------------------- +echo "" +echo "--- T_git_hook_skip_path_safe ---" +setup_harness +bash "$LINUX_SETUP" "--tools-dir=${STUB_DIR}" --skip=git-hook 2>&1 | grep -q . || true +hook_log="$(cat "$RUN_LOG" 2>/dev/null || true)" +if echo "$hook_log" | grep -qF "git-hook"; then + fail "T_git_hook_skip_path_safe: git-hook appeared in run-log despite being skipped" +elif assert_contains "$hook_log" "prereqs"; then + pass "T_git_hook_skip_path_safe: --skip=git-hook succeeds; git-hook excluded from run" +else + fail "T_git_hook_skip_path_safe: unexpected run-log: $hook_log" +fi +teardown_harness + # --------------------------------------------------------------------------- # Results # --------------------------------------------------------------------------- diff --git a/tests/test_setup_flags_pwsh.ps1 b/tests/test_setup_flags_pwsh.ps1 index f0bfde5..f004611 100644 --- a/tests/test_setup_flags_pwsh.ps1 +++ b/tests/test_setup_flags_pwsh.ps1 @@ -427,6 +427,185 @@ Test-Scenario "T_backward_compat_gate: no-arg run logs all defaults in order" { finally { Teardown-Harness } } +# --------------------------------------------------------------------------- +# WI-3: -Skip selective exclusion +# Stub defaults.txt order: prereqs, alpha, bravo, charlie, dotfiles, git-hook +# Opt-in stubs (in dir but NOT in defaults.txt): delta, uv +# --------------------------------------------------------------------------- + +# --------------------------------------------------------------------------- +# T_skip_single: -Skip 'bravo' excludes bravo, installs remaining in order +# --------------------------------------------------------------------------- + +Test-Scenario "T_skip_single: -Skip 'bravo' excludes bravo; remaining tools in order" { + Setup-Harness + try { + powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -Skip 'bravo' 2>&1 | Out-Null + Assert-LogStr @('prereqs', 'alpha', 'charlie', 'dotfiles', 'git-hook') + } + finally { Teardown-Harness } +} + +# --------------------------------------------------------------------------- +# T_skip_multi: -Skip 'alpha,charlie' excludes both, remaining in default order +# --------------------------------------------------------------------------- + +Test-Scenario "T_skip_multi: -Skip 'alpha,charlie' excludes both; remaining in default order" { + Setup-Harness + try { + powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -Skip 'alpha,charlie' 2>&1 | Out-Null + Assert-LogStr @('prereqs', 'bravo', 'dotfiles', 'git-hook') + } + finally { Teardown-Harness } +} + +# --------------------------------------------------------------------------- +# T_skip_unknown: -Skip 'bogus' exits 1 with helpful message +# --------------------------------------------------------------------------- + +Test-Scenario "T_skip_unknown: -Skip 'bogus' exits non-zero with error" { + $out = powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -Skip 'bogus' 2>&1 | Out-String + if ($LASTEXITCODE -eq 0) { throw "Expected non-zero exit for unknown tool 'bogus'" } + if ($out -notmatch 'bogus|unknown|available') { + throw "Error message not helpful: $out" + } +} + +# --------------------------------------------------------------------------- +# T_skip_empty: -Skip '' exits 1 (*** EXPECTED RED before WI-3 ContainsKey fix ***) +# --------------------------------------------------------------------------- + +Test-Scenario "T_skip_empty: -Skip '' exits non-zero" { + $skipEmptyFailed = $false + try { + powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -Skip '' 2>&1 | Out-Null + if ($LASTEXITCODE -ne 0) { $skipEmptyFailed = $true } + } catch { + $skipEmptyFailed = $true + } + if (-not $skipEmptyFailed) { throw "Expected non-zero exit for empty -Skip (RED until ContainsKey fix)" } +} + +# --------------------------------------------------------------------------- +# T_skip_conflict: -Only 'alpha' -Skip 'bravo' exits 1 (mutually exclusive) +# --------------------------------------------------------------------------- + +Test-Scenario "T_skip_conflict: -Only + -Skip exits non-zero (mutually exclusive)" { + powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -Only 'alpha' -Skip 'bravo' 2>&1 | Out-Null + if ($LASTEXITCODE -eq 0) { throw "Expected non-zero exit when both -Only and -Skip are provided" } +} + +# --------------------------------------------------------------------------- +# T_skip_blank_trailing: -Skip 'alpha,' exits 1 +# --------------------------------------------------------------------------- + +Test-Scenario "T_skip_blank_trailing: -Skip 'alpha,' exits non-zero" { + powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -Skip 'alpha,' 2>&1 | Out-Null + if ($LASTEXITCODE -eq 0) { throw "Expected non-zero exit for trailing comma in -Skip" } +} + +# --------------------------------------------------------------------------- +# T_skip_blank_leading: -Skip ',alpha' exits 1 +# --------------------------------------------------------------------------- + +Test-Scenario "T_skip_blank_leading: -Skip ',alpha' exits non-zero" { + powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -Skip ',alpha' 2>&1 | Out-Null + if ($LASTEXITCODE -eq 0) { throw "Expected non-zero exit for leading comma in -Skip" } +} + +# --------------------------------------------------------------------------- +# T_skip_blank_consecutive: -Skip 'alpha,,bravo' exits 1 +# --------------------------------------------------------------------------- + +Test-Scenario "T_skip_blank_consecutive: -Skip 'alpha,,bravo' exits non-zero" { + powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -Skip 'alpha,,bravo' 2>&1 | Out-Null + if ($LASTEXITCODE -eq 0) { throw "Expected non-zero exit for consecutive commas in -Skip" } +} + +# --------------------------------------------------------------------------- +# T_list_plus_only: -List -Only 'alpha' exits 0 (-List takes precedence) +# --------------------------------------------------------------------------- + +Test-Scenario "T_list_plus_only: -List wins over -Only (exits 0, list printed, no install)" { + Setup-Harness + try { + $out = powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -List -Only 'alpha' 2>&1 | Out-String + $ec = $LASTEXITCODE + if ($ec -ne 0) { throw "-List -Only exited $ec (expected 0)" } + if ($out -notmatch 'alpha') { throw "-List output does not contain 'alpha'" } + $content = Get-Content $script:RunLog -ErrorAction SilentlyContinue + if ($content) { throw "-List wrote to run-log (install occurred): $($content -join ', ')" } + } + finally { Teardown-Harness } +} + +# --------------------------------------------------------------------------- +# T_list_plus_skip: -List -Skip 'alpha' exits 0 (-List takes precedence) +# --------------------------------------------------------------------------- + +Test-Scenario "T_list_plus_skip: -List wins over -Skip (exits 0, list printed, no install)" { + Setup-Harness + try { + $out = powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -List -Skip 'alpha' 2>&1 | Out-String + $ec = $LASTEXITCODE + if ($ec -ne 0) { throw "-List -Skip exited $ec (expected 0)" } + if ($out -notmatch 'alpha') { throw "-List output does not contain 'alpha'" } + $content = Get-Content $script:RunLog -ErrorAction SilentlyContinue + if ($content) { throw "-List wrote to run-log (install occurred): $($content -join ', ')" } + } + finally { Teardown-Harness } +} + +# --------------------------------------------------------------------------- +# T_no_selection_persistence: prior -Only run does NOT poison a later no-arg run +# --------------------------------------------------------------------------- + +Test-Scenario "T_no_selection_persistence: no-arg run after -Only run installs full defaults" { + Setup-Harness + try { + powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -Only 'alpha' 2>&1 | Out-Null + } + finally { Teardown-Harness } + Setup-Harness + try { + powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir 2>&1 | Out-Null + Assert-LogEquals (Join-Path $StubDir 'defaults.txt') + } + finally { Teardown-Harness } +} + +# --------------------------------------------------------------------------- +# T_git_hook_skip_path_safe: -Skip 'git-hook' succeeds; git-hook not in run-log +# --------------------------------------------------------------------------- + +Test-Scenario "T_git_hook_skip_path_safe: -Skip 'git-hook' succeeds; git-hook excluded from run" { + Setup-Harness + try { + powershell -NoProfile -ExecutionPolicy Bypass -File $WinSetup ` + -ToolsDir $StubDir -Skip 'git-hook' 2>&1 | Out-Null + $logContent = Get-Content $script:RunLog -ErrorAction SilentlyContinue + if ($logContent -contains 'git-hook') { + throw "git-hook appeared in run-log despite being skipped" + } + if ($logContent -notcontains 'prereqs') { + throw "Expected other tools to run; run-log: $($logContent -join ', ')" + } + } + finally { Teardown-Harness } +} + # --------------------------------------------------------------------------- # Results # ---------------------------------------------------------------------------