From add6cd2949a0f89968abae3e3ad95060ecd8678f Mon Sep 17 00:00:00 2001 From: Michael Diamond Date: Wed, 22 Apr 2020 23:30:09 -0700 Subject: [PATCH] fix: reimplement load to be more useable and consistent Existing support is preserved (absolute paths and relative paths with a `.bash` suffix), and now *any* relative path is supported, as well as scripts on the system `PATH`. Fixes #79 --- lib/bats-core/test_functions.bash | 26 ++++++++++++++++---------- test/bats.bats | 18 ++++++++++++++++++ test/fixtures/bats/ambiguous | 5 +++++ test/fixtures/bats/ambiguous.bash | 6 ++++++ 4 files changed, 45 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/bats/ambiguous create mode 100644 test/fixtures/bats/ambiguous.bash diff --git a/lib/bats-core/test_functions.bash b/lib/bats-core/test_functions.bash index 371e3dd9..aa00bb20 100644 --- a/lib/bats-core/test_functions.bash +++ b/lib/bats-core/test_functions.bash @@ -3,24 +3,30 @@ BATS_TEST_DIRNAME="${BATS_TEST_FILENAME%/*}" BATS_TEST_NAMES=() +# Shorthand for source-ing files relative to the BATS_TEST_DIRNAME, +# optionally with a .bash suffix appended. If the argument doesn't +# resolve relative to BATS_TEST_DIRNAME it is sourced as-is. load() { - local name="$1" - local filename + local file="${1:?}" - if [[ "${name:0:1}" == '/' ]]; then - filename="${name}" - else - filename="$BATS_TEST_DIRNAME/${name}.bash" + # For backwards-compatibility first look for a .bash-suffixed file. + # TODO consider flipping the order here; it would be more consistent + # and less surprising to look for an exact-match first. + if [[ -f "${BATS_TEST_DIRNAME}/${file}.bash" ]]; then + file="${BATS_TEST_DIRNAME}/${file}.bash" + elif [[ -f "${BATS_TEST_DIRNAME}/${file}" ]]; then + file="${BATS_TEST_DIRNAME}/${file}" fi - if [[ ! -f "$filename" ]]; then - printf 'bats: %s does not exist\n' "$filename" >&2 + if [[ ! -f "$file" ]] && ! type -P "$file" >/dev/null; then + printf 'bats: %s does not exist\n' "$file" >&2 exit 1 fi - # Dynamically loaded user files provided outside of Bats. + # Dynamically loaded user file provided outside of Bats. + # Note: 'source "$file" || exit' doesn't work on bash3.2. # shellcheck disable=SC1090 - source "${filename}" + source "${file}" } run() { diff --git a/test/bats.bats b/test/bats.bats index 61244288..db22d4b9 100755 --- a/test/bats.bats +++ b/test/bats.bats @@ -196,6 +196,11 @@ fixtures bats [ $status -eq 0 ] } +@test "load sources relative scripts with filename extension" { + HELPER_NAME="test_helper.bash" run bats "$FIXTURE_ROOT/load.bats" + [ $status -eq 0 ] +} + @test "load aborts if the specified script does not exist" { HELPER_NAME="nonexistent" run bats "$FIXTURE_ROOT/load.bats" [ $status -eq 1 ] @@ -211,6 +216,19 @@ fixtures bats [ $status -eq 1 ] } +@test "load relative script with ambiguous name" { + HELPER_NAME="ambiguous" run bats "$FIXTURE_ROOT/load.bats" + [ $status -eq 0 ] +} + +@test "load supports scripts on the PATH" { + path_dir="$BATS_TMPNAME/path" + mkdir -p "$path_dir" + cp "${FIXTURE_ROOT}/test_helper.bash" "${path_dir}/on_path" + PATH="${path_dir}:$PATH" HELPER_NAME="on_path" run bats "$FIXTURE_ROOT/load.bats" + [ $status -eq 0 ] +} + @test "output is discarded for passing tests and printed for failing tests" { run bats "$FIXTURE_ROOT/output.bats" [ $status -eq 1 ] diff --git a/test/fixtures/bats/ambiguous b/test/fixtures/bats/ambiguous new file mode 100644 index 00000000..68aee573 --- /dev/null +++ b/test/fixtures/bats/ambiguous @@ -0,0 +1,5 @@ +# Helper to detect regressions in load's lookup ordering. +# An exact name match should be prioritized over name.bash. + +echo "Should not have loaded this file!" >&2 +exit 1 diff --git a/test/fixtures/bats/ambiguous.bash b/test/fixtures/bats/ambiguous.bash new file mode 100644 index 00000000..df0c0947 --- /dev/null +++ b/test/fixtures/bats/ambiguous.bash @@ -0,0 +1,6 @@ +# Helper to detect regressions in load's lookup ordering. +# An exact name match should be prioritized over name.bash. + +echo "This is the expected file to load." + +help_me() { :; }