Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
# Copy this file to .env and customize as needed
# All values shown are defaults (leave empty to use default)
#
# Do NOT add a variable here just to document it. .env is loaded with
# `set -o allexport; source .env`, so every line is an unconditional assignment:
# listing a name makes an empty value OVERRIDE one the caller exported or set on
# the command line (`BASHUNIT_OUTPUT_FORMAT=tap ./bashunit` stops working).
# Settings that must stay overridable are documented in docs/configuration.md
# instead. `.bashunitrc` does not have this problem — it applies only when unset.
# Precedence: an entry left EMPTY here means "not configured", so a value the
# caller exported or set on the command line still wins
# (`BASHUNIT_OUTPUT_FORMAT=tap ./bashunit` keeps working). Give an entry a value
# and it takes effect for the project, overriding the ambient environment.
# Listing a name with no value is therefore safe and is how a setting is
# documented here.
################################################################################

#───────────────────────────────────────────────────────────────────────────────
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,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
- An empty entry in `.env` no longer overrides a value the caller exported or set on the command line. `.env` is sourced under `set -o allexport`, so every line was an unconditional assignment: merely *listing* a name blanked it, and `BASHUNIT_OUTPUT_FORMAT=tap ./bashunit` silently stopped working in any project whose `.env` mentioned that setting. An empty entry now means "not configured here"; an entry with a value still takes effect for the project. This had been actively concealing defects — two of the bugs fixed in #879 were not reproducible from inside a repo checkout for exactly this reason (#865)
- `BASHUNIT_COVERAGE_THRESHOLD_LOW`/`BASHUNIT_COVERAGE_THRESHOLD_HIGH` (env-only, no CLI flag) now validate as non-negative integers like the other numeric settings. They are compared with `[ -ge ]` in the coverage class lookup, which errors instead of returning false on a non-integer value: a bad threshold leaked a raw `integer expression expected` into the coverage report and silently mis-bucketed every file's high/medium/low class
- `@max_ms` benchmark annotations with a decimal value (e.g. `@max_ms=100.5`) no longer leak a raw `integer expression expected` into the results table and always render as failing (`>`) regardless of the actual average; the threshold comparison now tolerates fractional operands like the average itself already could
- `bashunit assert <name>` with no arguments now reports a clear error and exits non-zero. It used to compute a `-1` array index, which Bash 3.x rejects, so it leaked a raw `bad array subscript` and still exited `0` — a malformed standalone assertion in a deploy script reported success (#877)
Expand Down
7 changes: 6 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,17 @@ test/parallel: $(TEST_SCRIPTS)
# the target lints the repo N+1 times and appears to hang), and its "*.sh" glob
# never sees the extensionless entrypoint or the bin/ scripts. `find` remains
# the fallback outside a git checkout.
#
# --others --exclude-standard includes files that are new and not yet staged:
# without it a brand-new script is invisible to `make sa` and only fails once CI
# lints it, which is exactly backwards.
sa:
ifndef STATIC_ANALYSIS_CHECKER
@printf "\e[1m\e[31m%s\e[0m\n" "Shellcheck not installed: Static analysis not performed!" && exit 1
else
@{ if git rev-parse --is-inside-work-tree >/dev/null 2>&1; then \
git ls-files -z "*.sh" bashunit bin/pre-commit bin/create-pr; \
git ls-files -z --cached --others --exclude-standard \
"*.sh" bashunit bin/pre-commit bin/create-pr; \
else \
find . -name "*.sh" -not -path "./local/*" -not -path "./.claude/worktrees/*" -print0; \
fi; } \
Expand Down
20 changes: 13 additions & 7 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,19 @@ It is meant for committing sensible project defaults. Precedence, from highest
to lowest:

1. CLI flags (e.g. `--simple`)
2. Environment variables and the `.env` file
3. `.bashunitrc`
4. Built-in defaults

`.bashunitrc` only fills values that are not already set, so an exported
environment variable or a `.env` entry always wins. `--skip-env-file` skips
`.bashunitrc` as well.
2. `.env` entries that have a value
3. Environment variables
4. `.bashunitrc`
5. Built-in defaults

An entry left **empty** in `.env` means "not configured here" and does not
override the environment, so `BASHUNIT_OUTPUT_FORMAT=tap ./bashunit` keeps
working even when `.env` lists that name. Give the entry a value and it takes
effect for the whole project, overriding the ambient environment — that is what a
committed project config is for.

`.bashunitrc` only fills values that are not already set, so anything above it
always wins. `--skip-env-file` skips both `.env` and `.bashunitrc`.

## Default path

Expand Down
39 changes: 35 additions & 4 deletions src/env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,41 @@ function bashunit::env::positive_int_or_default() {
# Load .env file (skip if --skip-env-file is used to keep shell environment intact)
if [ "${BASHUNIT_SKIP_ENV_FILE:-false}" != "true" ]; then
bashunit::env::load_config_file ".bashunitrc"
set -o allexport
# shellcheck source=/dev/null
[ -f ".env" ] && source .env
set +o allexport

if [ -f ".env" ]; then
# `.env` is sourced under allexport so it can hold arbitrary shell, which
# means every `KEY=` line is an unconditional assignment: an empty entry used
# to wipe a value the caller exported, so a project could not document a
# setting without breaking it on the command line (#865).
#
# Snapshot the caller's non-empty BASHUNIT_* values and put back any the file
# blanked. A non-empty entry still wins -- that is what a project config is
# for -- and only an empty one is treated as "not configured here", matching
# how .bashunitrc already applies values.
_bashunit_env_preserved=""
for _bashunit_env_name in $(compgen -v BASHUNIT_ 2>/dev/null || true); do
eval "_bashunit_env_value=\${$_bashunit_env_name}"
if [ -n "$_bashunit_env_value" ]; then
_bashunit_env_preserved="$_bashunit_env_preserved $_bashunit_env_name"
eval "_bashunit_env_saved_$_bashunit_env_name=\$_bashunit_env_value"
fi
done

set -o allexport
# shellcheck source=/dev/null
source .env
set +o allexport

for _bashunit_env_name in $_bashunit_env_preserved; do
eval "_bashunit_env_value=\${$_bashunit_env_name}"
if [ -z "$_bashunit_env_value" ]; then
eval "export $_bashunit_env_name=\$_bashunit_env_saved_$_bashunit_env_name"
fi
eval "unset _bashunit_env_saved_$_bashunit_env_name"
done

unset _bashunit_env_preserved _bashunit_env_name _bashunit_env_value
fi
fi

_BASHUNIT_DEFAULT_DEFAULT_PATH="tests"
Expand Down
68 changes: 68 additions & 0 deletions tests/acceptance/bashunit_env_file_precedence_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#!/usr/bin/env bash

# `.env` is loaded with `set -o allexport; source .env`, so every line is an
# unconditional assignment. Listing a name with an empty value therefore
# OVERRODE a value the caller exported, which meant a project could not document
# a setting in .env/.env.example without breaking it on the command line (#865).
#
# An empty entry now means "not configured here" and leaves the caller's value
# alone, matching how .bashunitrc already applies values (`${key:-val}`).
# A non-empty entry still wins, which is what a project config is for.

BASHUNIT_PATH="$PWD/bashunit"

function set_up() {
WORK_DIR=$(mktemp -d)
printf '#!/usr/bin/env bash\n\nfunction test_ok() {\n assert_same "1" "1"\n}\n' \
>"$WORK_DIR/probe_test.sh"
}

function tear_down() {
rm -rf "$WORK_DIR"
}

function test_an_empty_env_entry_does_not_override_an_exported_value() {
printf 'BASHUNIT_OUTPUT_FORMAT=\n' >"$WORK_DIR/.env"

local output
pushd "$WORK_DIR" >/dev/null || return 1
output=$(BASHUNIT_OUTPUT_FORMAT=tap "$BASHUNIT_PATH" . 2>&1) || true
popd >/dev/null || return 1

assert_contains "TAP version 13" "$output"
}

function test_a_non_empty_env_entry_still_applies() {
printf 'BASHUNIT_OUTPUT_FORMAT=tap\n' >"$WORK_DIR/.env"

local output
pushd "$WORK_DIR" >/dev/null || return 1
output=$("$BASHUNIT_PATH" . 2>&1) || true
popd >/dev/null || return 1

assert_contains "TAP version 13" "$output"
}

# A project that pins a value must still win over the ambient environment: an
# empty caller value is not a deliberate choice, so the .env entry applies.
function test_a_non_empty_env_entry_applies_over_an_empty_exported_value() {
printf 'BASHUNIT_OUTPUT_FORMAT=tap\n' >"$WORK_DIR/.env"

local output
pushd "$WORK_DIR" >/dev/null || return 1
output=$(BASHUNIT_OUTPUT_FORMAT='' "$BASHUNIT_PATH" . 2>&1) || true
popd >/dev/null || return 1

assert_contains "TAP version 13" "$output"
}

function test_env_file_still_sets_a_value_the_caller_did_not_provide() {
printf 'BASHUNIT_OUTPUT_FORMAT=tap\n' >"$WORK_DIR/.env"

local output
pushd "$WORK_DIR" >/dev/null || return 1
output=$("$BASHUNIT_PATH" . 2>&1) || true
popd >/dev/null || return 1

assert_contains "ok 1 - Ok" "$output"
}
13 changes: 7 additions & 6 deletions tests/acceptance/bashunit_invalid_option_value_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,11 @@ function test_bashunit_fails_when_parallel_jobs_env_var_is_not_a_number() {
function test_bashunit_fails_when_coverage_threshold_high_env_var_is_not_a_number() {
local ec=0
local output
# --skip-env-file is required: .env / .env.example both list these two names,
# and an allexport `source .env` turns an empty listing into an unconditional
# assignment that overrides the exported value under test (#865).
output=$(BASHUNIT_COVERAGE_THRESHOLD_HIGH=abc ./bashunit --skip-env-file --coverage "$TEST_FILE" 2>&1) || ec=$?
# .env / .env.example both list this name with an empty value. That used to
# override the exported value and this test needed --skip-env-file to work;
# since #865 an empty entry no longer clobbers the caller, so the real path is
# exercised here.
output=$(BASHUNIT_COVERAGE_THRESHOLD_HIGH=abc ./bashunit --coverage "$TEST_FILE" 2>&1) || ec=$?

assert_general_error "" "" "$ec"
assert_contains "BASHUNIT_COVERAGE_THRESHOLD_HIGH" "$output"
Expand All @@ -92,8 +93,8 @@ function test_bashunit_fails_when_coverage_threshold_high_env_var_is_not_a_numbe
function test_bashunit_fails_when_coverage_threshold_low_env_var_is_not_a_number() {
local ec=0
local output
# See the note above: .env would otherwise override the value under test.
output=$(BASHUNIT_COVERAGE_THRESHOLD_LOW=abc ./bashunit --skip-env-file --coverage "$TEST_FILE" 2>&1) || ec=$?
# See the note above.
output=$(BASHUNIT_COVERAGE_THRESHOLD_LOW=abc ./bashunit --coverage "$TEST_FILE" 2>&1) || ec=$?

assert_general_error "" "" "$ec"
assert_contains "BASHUNIT_COVERAGE_THRESHOLD_LOW" "$output"
Expand Down
Loading