diff --git a/CHANGELOG.md b/CHANGELOG.md index 2456d7f9..554c5116 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,8 +4,10 @@ ### Added - Display captured test output on assertion failures when `--show-output` is enabled (#637) +- `bashunit::env::supports_color` helper exposing a capability probe (`TERM=dumb` / `tput colors < 8`) for future auto-detection use; `bashunit::io::clear_screen` helper that prefers `tput clear` and falls back to the raw ANSI sequence (#247) ### Changed +- Centralize all ANSI escape emission through the existing `_BASHUNIT_COLOR_*` constants. `src/coverage.sh` and the `--watch` screen-clear in `src/main.sh` no longer hardcode escape sequences (#247) - Speed up coverage report generation by collapsing the per-line non-executable pattern checks in `bashunit::coverage::is_executable_line` into a single combined `grep` invocation (#636) - Speed up coverage report generation further by combining executable + hit counting into a single source-file pass (`bashunit::coverage::compute_file_coverage`) shared across text/lcov/html reporters, removing per-line `get_line_hits` scans of the coverage data file (#636) - Replace `echo | sed` / `echo | grep` subshells in `bashunit::coverage::extract_functions` with bash native regex matching and parameter expansion (#636) diff --git a/adrs/adr-006-tput-vs-ansi-escapes.md b/adrs/adr-006-tput-vs-ansi-escapes.md new file mode 100644 index 00000000..997664cb --- /dev/null +++ b/adrs/adr-006-tput-vs-ansi-escapes.md @@ -0,0 +1,55 @@ +# Keep ANSI for colors, use tput where it pays off + +* Status: accepted +* Deciders: @Chemaclass +* Date: 2026-04-29 + +Technical Story: [#247](https://github.com/TypedDevs/bashunit/issues/247) + +## Context + +bashunit prints colors and clears the screen with hardcoded ANSI escapes (`\e[31m`, `\033[2J\033[H`). The idea floated in #247 was to switch to `tput`, which reads terminfo and is in theory more portable. + +We tried something similar around PR #245 and it broke the test suite across CI envs. Lots of runners ship with `TERM=dumb` or no `TERM` at all, so `tput setaf` returns empty and colored output silently disappears. + +So the question is not really "tput or ANSI" but "where does tput actually help us, and where does it just break things?" + +## Options + +* A. Replace every ANSI escape with `tput`. +* B. Keep ANSI everywhere, change nothing. +* C. Keep ANSI for colors. Use tput only where it gives us something ANSI cannot. + +## Decision + +Option C. + +Reasoning: + +* For colors, tput just emits the same ANSI codes we already write by hand. The only thing it adds is breaking on dumb terminals. +* For things ANSI cannot do well, like probing whether the terminal supports color at all, or producing the right "clear screen" sequence on weird terminals, tput is genuinely useful. +* We already use `tput cols` in `src/env.sh` with an ANSI/`stty` fallback. Same pattern fits here. + +What this PR does: + +1. All color escapes go through `bashunit::sgr` and the `_BASHUNIT_COLOR_*` constants. No more raw `\033[...m` literals in `src/coverage.sh` or `src/main.sh`. +2. New `bashunit::env::supports_color` (false on `TERM=dumb` or `tput colors < 8`). Exposed but not wired into `colors.sh` init yet. The same auto-disable broke CI in PR #245 and again on the first push of this branch, so it waits until we add a `CI` / `FORCE_COLOR` override. +3. New `bashunit::io::clear_screen` runs `tput clear` and falls back to `\033[2J\033[H` if tput is missing or returns nothing. Replaces the hardcoded clear in `--watch` mode. + +## Consequences + +Good: + +* One place to change colors. +* Screen clear works on terminals where the literal ANSI is wrong. +* `supports_color` is ready for the next step (auto-detect with a CI override). + +Bad: + +* Two mechanisms (constants for colors, tput for clear/probe). Contributors need to know not to add raw escapes back. + +## Links + +* Issue [#247](https://github.com/TypedDevs/bashunit/issues/247) +* PR [#245](https://github.com/TypedDevs/bashunit/pull/245) +* [NO_COLOR spec](https://no-color.org/) diff --git a/src/coverage.sh b/src/coverage.sh index 1d6f7045..839512ac 100644 --- a/src/coverage.sh +++ b/src/coverage.sh @@ -159,6 +159,14 @@ function bashunit::coverage::get_coverage_class() { fi } +function bashunit::coverage::get_color_for_class() { + case "$1" in + high) printf '%s' "$_BASHUNIT_COLOR_PASSED" ;; + medium) printf '%s' "$_BASHUNIT_COLOR_SKIPPED" ;; + low) printf '%s' "$_BASHUNIT_COLOR_FAILED" ;; + esac +} + # Calculate percentage from hit and executable counts function bashunit::coverage::calculate_percentage() { local hit="$1" @@ -737,16 +745,8 @@ function bashunit::coverage::report_text() { total_executable=$((total_executable + executable)) total_hit=$((total_hit + hit)) - # Determine color based on class - local color="" reset="" - if [ "${BASHUNIT_NO_COLOR:-false}" != "true" ]; then - reset=$'\033[0m' - case "$class" in - high) color=$'\033[32m' ;; # Green - medium) color=$'\033[33m' ;; # Yellow - low) color=$'\033[31m' ;; # Red - esac - fi + local color reset="$_BASHUNIT_COLOR_DEFAULT" + color=$(bashunit::coverage::get_color_for_class "$class") # Display relative path local display_file="${file#"$(pwd)"/}" @@ -767,15 +767,8 @@ function bashunit::coverage::report_text() { total_pct=$(bashunit::coverage::calculate_percentage "$total_hit" "$total_executable") total_class=$(bashunit::coverage::get_coverage_class "$total_pct") - local color="" reset="" - if [ "${BASHUNIT_NO_COLOR:-false}" != "true" ]; then - reset=$'\033[0m' - case "$total_class" in - high) color=$'\033[32m' ;; - medium) color=$'\033[33m' ;; - low) color=$'\033[31m' ;; - esac - fi + local color reset="$_BASHUNIT_COLOR_DEFAULT" + color=$(bashunit::coverage::get_color_for_class "$total_class") printf "%sTotal: %d/%d (%d%%)%s\n" \ "$color" "$total_hit" "$total_executable" "$total_pct" "$reset" @@ -839,14 +832,8 @@ function bashunit::coverage::check_threshold() { pct=$(bashunit::coverage::get_percentage) if [ "$pct" -lt "$BASHUNIT_COVERAGE_MIN" ]; then - local color="" - local reset="" - if [ "${BASHUNIT_NO_COLOR:-false}" != "true" ]; then - color=$'\033[31m' - reset=$'\033[0m' - fi printf "%sCoverage %d%% is below minimum %d%%%s\n" \ - "$color" "$pct" "$BASHUNIT_COVERAGE_MIN" "$reset" + "$_BASHUNIT_COLOR_FAILED" "$pct" "$BASHUNIT_COVERAGE_MIN" "$_BASHUNIT_COLOR_DEFAULT" return 1 fi diff --git a/src/dependencies.sh b/src/dependencies.sh index c605167f..1d12c034 100644 --- a/src/dependencies.sh +++ b/src/dependencies.sh @@ -40,3 +40,7 @@ function bashunit::dependencies::has_python() { function bashunit::dependencies::has_node() { command -v node >/dev/null 2>&1 } + +function bashunit::dependencies::has_tput() { + command -v tput >/dev/null 2>&1 +} diff --git a/src/env.sh b/src/env.sh index 2a809131..8a1ab14f 100644 --- a/src/env.sh +++ b/src/env.sh @@ -182,6 +182,32 @@ function bashunit::env::is_no_color_enabled() { [ "$BASHUNIT_NO_COLOR" = "true" ] } +## +# Whether the current terminal can render ANSI color sequences. +# Returns 1 when TERM=dumb or when `tput colors` reports fewer than 8. +# Returns 0 when tput is missing (assume colors work, preserving prior behavior). +## +function bashunit::env::supports_color() { + if [ "${TERM:-}" = "dumb" ]; then + return 1 + fi + + if ! bashunit::dependencies::has_tput; then + return 0 + fi + + local n + n=$(tput colors 2>/dev/null) + case "$n" in + '' | *[!0-9]*) + return 0 + ;; + *) + [ "$n" -ge 8 ] + ;; + esac +} + function bashunit::env::is_coverage_enabled() { [ "$BASHUNIT_COVERAGE" = "true" ] } diff --git a/src/io.sh b/src/io.sh index bfe9d29e..c3f2a1e4 100644 --- a/src/io.sh +++ b/src/io.sh @@ -1,5 +1,22 @@ #!/usr/bin/env bash +## +# Clear the terminal screen and move the cursor to the home position. +# Uses `tput clear` when available (queries terminfo for the right sequence) +# and falls back to the ANSI sequence \033[2J\033[H otherwise. +## +function bashunit::io::clear_screen() { + if bashunit::dependencies::has_tput; then + local out + out=$(tput clear 2>/dev/null) + if [ -n "$out" ]; then + printf '%s' "$out" + return + fi + fi + printf '\033[2J\033[H' +} + function bashunit::io::download_to() { local url="$1" local output="$2" diff --git a/src/main.sh b/src/main.sh index d33d8335..b805511d 100644 --- a/src/main.sh +++ b/src/main.sh @@ -586,7 +586,7 @@ function bashunit::main::watch_loop() { if [ "$current_checksum" != "$last_checksum" ]; then last_checksum="$current_checksum" - printf '\033[2J\033[H' + bashunit::io::clear_screen printf "%s[watch] Running tests...%s\n\n" \ "${_BASHUNIT_COLOR_SKIPPED}" \ "${_BASHUNIT_COLOR_DEFAULT}" diff --git a/tests/unit/env_test.sh b/tests/unit/env_test.sh index b78e93ca..e68c1539 100644 --- a/tests/unit/env_test.sh +++ b/tests/unit/env_test.sh @@ -150,6 +150,47 @@ function test_find_terminal_width_fallback_returns_100() { assert_equals "100" "$result" } +function test_supports_color_returns_failure_when_TERM_is_dumb() { + local original_term="${TERM:-}" + export TERM="dumb" + + if bashunit::env::supports_color; then + export TERM="$original_term" + fail "Expected supports_color to fail when TERM=dumb" + return + fi + + export TERM="$original_term" + assert_successful_code 0 +} + +function test_supports_color_returns_failure_when_tput_reports_below_8_colors() { + local original_term="${TERM:-}" + export TERM="xterm" + bashunit::mock tput <<<"2" + + if bashunit::env::supports_color; then + export TERM="$original_term" + fail "Expected supports_color to fail when tput colors reports 2" + return + fi + + export TERM="$original_term" + assert_successful_code 0 +} + +function test_supports_color_returns_success_when_tput_reports_8_or_more_colors() { + local original_term="${TERM:-}" + export TERM="xterm" + bashunit::mock tput <<<"256" + + bashunit::env::supports_color + local result=$? + + export TERM="$original_term" + assert_equals 0 "$result" +} + function test_print_verbose_outputs_env_var_names() { local original="$BASHUNIT_VERBOSE" export BASHUNIT_VERBOSE="true" diff --git a/tests/unit/io_test.sh b/tests/unit/io_test.sh new file mode 100644 index 00000000..7945b6c6 --- /dev/null +++ b/tests/unit/io_test.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash + +function test_clear_screen_uses_tput_when_available() { + bashunit::mock tput <<<"CLEARED" + + local output + output=$(bashunit::io::clear_screen) + + assert_contains "CLEARED" "$output" +} + +function test_clear_screen_emits_non_empty_output() { + local output + output=$(bashunit::io::clear_screen) + + assert_not_empty "$output" +}