fix(cli): validate option values instead of dropping bad ones

CHANGELOG.md

@@ -14,6 +14,7 @@
 - The `--parallel` unsupported-OS warning no longer claims Alpine is excluded: Alpine has been a supported parallel platform since the race conditions were fixed, the message was simply never updated
 
 ### Fixed
+- The numeric options (`--jobs`, `--retry`, `--test-timeout`, `--coverage-min`) and `--output` now reject an invalid value with a clear error and a non-zero exit, instead of dropping the setting. `--jobs abc` was the worst case: `[ n -lt abc ]` errors rather than returning false, so the Bash 3.x job-slot poll never reached its `break` and the run **hung**, while Bash 4.3+ silently ignored the concurrency limit. `--coverage-min abc` leaked a raw `[: abc: integer expression expected` and still exited `0`, so a CI job never enforced its threshold. Validation runs on the resolved configuration, so it covers the `BASHUNIT_*` environment variables too (#873)
 - An unknown option is now rejected with a clear error and a non-zero exit instead of being silently filed under test paths. A mistyped flag used to degrade the run while still reporting success: `bashunit --parralel tests/` ran sequentially and exited `0`, and `bashunit --filterr foo tests/` swallowed both the flag and its value and ran the whole suite. Applies to `test` and `bench`; the `assert` subcommand parses its own arguments and is unchanged (#871)
 - The quickstart's sample output showed durations as `16 ms` / `90 ms`; bashunit prints `16ms` / `90ms`
 - `.env.example` documented `BASHUNIT_SHOW_EXECUTION_TIME` as defaulting to `true`; the actual default has been `auto` since #765

src/main.sh

@@ -10,6 +10,54 @@ function bashunit::main::abort_unknown_option() {
   exit 1
 }
 
+##
+# Exits non-zero unless the value is a non-negative integer.
+# Arguments: $1 - value, $2 - the setting name to quote in the error
+##
+function bashunit::main::require_non_negative_int_or_exit() {
+  case "$1" in
+  '' | *[!0-9]*)
+    printf "%sError: %s must be a non-negative integer, got '%s'.%s\n" \
+      "${_BASHUNIT_COLOR_FAILED}" "$2" "$1" "${_BASHUNIT_COLOR_DEFAULT}" >&2
+    exit 1
+    ;;
+  esac
+}
+
+##
+# Validates the resolved configuration and exits non-zero on a bad value.
+# Runs after flag parsing so it covers both the flags and the BASHUNIT_* env
+# vars, which bypass the parser entirely. The numeric settings are compared with
+# `[ -lt ]`, which errors instead of returning false on a non-integer operand: the
+# job-slot poll looped forever and the rest silently dropped the setting (#873).
+##
+function bashunit::main::validate_config_or_exit() {
+  if [ "${BASHUNIT_PARALLEL_JOBS:-0}" != "0" ]; then
+    bashunit::main::require_non_negative_int_or_exit \
+      "${BASHUNIT_PARALLEL_JOBS}" "BASHUNIT_PARALLEL_JOBS (--jobs)"
+  fi
+  bashunit::main::require_non_negative_int_or_exit \
+    "${BASHUNIT_RETRY:-0}" "BASHUNIT_RETRY (--retry)"
+  bashunit::main::require_non_negative_int_or_exit \
+    "${BASHUNIT_TEST_TIMEOUT:-0}" "BASHUNIT_TEST_TIMEOUT (--test-timeout)"
+  # Empty is the documented "no minimum" default, so only a set value is checked.
+  if [ -n "${BASHUNIT_COVERAGE_MIN:-}" ]; then
+    bashunit::main::require_non_negative_int_or_exit \
+      "${BASHUNIT_COVERAGE_MIN}" "BASHUNIT_COVERAGE_MIN (--coverage-min)"
+  fi
+
+  # Only TAP is implemented; an unrecognised name used to fall back to the
+  # default renderer without a word, so `--output tpa` looked like it worked.
+  case "${BASHUNIT_OUTPUT_FORMAT:-}" in
+  '' | tap) ;;
+  *)
+    printf "%sError: unsupported output format '%s' for --output. Supported: tap.%s\n" \
+      "${_BASHUNIT_COLOR_FAILED}" "${BASHUNIT_OUTPUT_FORMAT}" "${_BASHUNIT_COLOR_DEFAULT}" >&2
+    exit 1
+    ;;
+  esac
+}
+
 ##
 # Validates a `--shard <index>/<total>` spec and exports the parts, or prints an
 # error and exits non-zero. Requires numeric index/total with 1 <= index <= total.
@@ -354,6 +402,8 @@ function bashunit::main::cmd_test() {
     shift
   done
 
+  bashunit::main::validate_config_or_exit
+
   # Auto-enable coverage when any coverage output option is specified
   if [ "$_bashunit_coverage_opt_set" = true ]; then
     # shellcheck disable=SC2034

tests/acceptance/bashunit_invalid_option_value_test.sh

@@ -0,0 +1,100 @@
+#!/usr/bin/env bash
+set -euo pipefail
+
+function set_up_before_script() {
+  TEST_ENV_FILE="tests/acceptance/fixtures/.env.default"
+  TEST_FILE="tests/acceptance/fixtures/tests_path/a_test.sh"
+}
+
+# The numeric options used to accept any string. `[ n -lt abc ]` errors rather
+# than returning false, so the value was either ignored or -- on the Bash 3.x
+# job-slot poll -- looped forever (#873).
+function test_bashunit_fails_when_jobs_is_not_a_number() {
+  local ec=0
+  local output
+  output=$(./bashunit --env "$TEST_ENV_FILE" --jobs abc "$TEST_FILE" 2>&1) || ec=$?
+
+  assert_general_error "" "" "$ec"
+  assert_contains "--jobs" "$output"
+}
+
+function test_bashunit_fails_when_jobs_is_negative() {
+  local ec=0
+  local output
+  output=$(./bashunit --env "$TEST_ENV_FILE" --jobs -5 "$TEST_FILE" 2>&1) || ec=$?
+
+  assert_general_error "" "" "$ec"
+}
+
+function test_bashunit_fails_when_retry_is_not_a_number() {
+  local ec=0
+  local output
+  output=$(./bashunit --env "$TEST_ENV_FILE" --retry abc "$TEST_FILE" 2>&1) || ec=$?
+
+  assert_general_error "" "" "$ec"
+  assert_contains "--retry" "$output"
+}
+
+function test_bashunit_fails_when_test_timeout_is_not_a_number() {
+  local ec=0
+  local output
+  output=$(./bashunit --env "$TEST_ENV_FILE" --test-timeout abc "$TEST_FILE" 2>&1) || ec=$?
+
+  assert_general_error "" "" "$ec"
+  assert_contains "--test-timeout" "$output"
+}
+
+function test_bashunit_fails_when_coverage_min_is_not_a_number() {
+  local ec=0
+  local output
+  output=$(./bashunit --env "$TEST_ENV_FILE" --coverage-min abc "$TEST_FILE" 2>&1) || ec=$?
+
+  assert_general_error "" "" "$ec"
+  assert_contains "--coverage-min" "$output"
+}
+
+function test_bashunit_fails_when_output_format_is_unsupported() {
+  local ec=0
+  local output
+  output=$(./bashunit --env "$TEST_ENV_FILE" --output nonsense "$TEST_FILE" 2>&1) || ec=$?
+
+  assert_general_error "" "" "$ec"
+  assert_contains "--output" "$output"
+}
+
+# The environment path bypasses flag parsing entirely, so it needs the same gate.
+function test_bashunit_fails_when_parallel_jobs_env_var_is_not_a_number() {
+  local ec=0
+  local output
+  output=$(BASHUNIT_PARALLEL_JOBS=abc ./bashunit --env "$TEST_ENV_FILE" --parallel "$TEST_FILE" 2>&1) || ec=$?
+
+  assert_general_error "" "" "$ec"
+  assert_contains "BASHUNIT_PARALLEL_JOBS" "$output"
+}
+
+function test_bashunit_still_accepts_valid_option_values() {
+  local ec=0
+  local output
+  output=$(./bashunit --env "$TEST_ENV_FILE" --jobs 2 --retry 0 --test-timeout 0 "$TEST_FILE" 2>&1) || ec=$?
+
+  assert_successful_code "" "" "$ec"
+  assert_contains "All tests passed" "$output"
+}
+
+function test_bashunit_still_accepts_jobs_auto() {
+  local ec=0
+  local output
+  output=$(./bashunit --env "$TEST_ENV_FILE" --jobs auto "$TEST_FILE" 2>&1) || ec=$?
+
+  assert_successful_code "" "" "$ec"
+  assert_contains "All tests passed" "$output"
+}
+
+function test_bashunit_still_accepts_tap_output() {
+  local ec=0
+  local output
+  output=$(./bashunit --env "$TEST_ENV_FILE" --output tap "$TEST_FILE" 2>&1) || ec=$?
+
+  assert_successful_code "" "" "$ec"
+  assert_contains "TAP version 13" "$output"
+}