[1/3] test-lib: consolidate naming of test-results paths

Commit Message

SZEDER Gábor Dec. 4, 2018, 4:34 p.m. UTC

There are two places where we strip off any leading path components
and the '.sh' suffix from the test script's pathname, and there are
two places where we construct the filename of test output files in
't/test-results/'.  The last patch in this series will add even more.

Factor these out into helper variables to avoid repeating ourselves.

Signed-off-by: SZEDER Gábor <[e-mail]>
---
 t/test-lib.sh | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

Comments

Jeff King Dec. 5, 2018, 4:57 a.m. UTC | #1

On Tue, Dec 04, 2018 at 05:34:55PM +0100, SZEDER Gábor wrote:

> There are two places where we strip off any leading path components
> and the '.sh' suffix from the test script's pathname, and there are
> two places where we construct the filename of test output files in
> 't/test-results/'.  The last patch in this series will add even more.
> 
> Factor these out into helper variables to avoid repeating ourselves.

Makes sense.

> +TEST_NAME="$(basename "$0" .sh)"
> +TEST_RESULTS_BASE="$TEST_OUTPUT_DIRECTORY/test-results/$TEST_NAME"

Hmm, since we are building up this BASE variable anyway, why not:

  TEST_RESULTS_DIR=$TEST_OUTPUT_DIRECTORY/test-results
  TEST_RESULTS_BASE=$TEST_RESULTS_DIR/$TEST_NAME

? That saves having to run `dirname` on it later.

I guess one could argue that saying "the directory name of the file I'm
writing" is more readable. I just generally try to avoid extra
manipulation of the strings when possible (especially in shell).

-Peff

Patch

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0f1faa24b2..49e4563405 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -71,6 +71,9 @@  then
 	exit 1
 fi
 
+TEST_NAME="$(basename "$0" .sh)"
+TEST_RESULTS_BASE="$TEST_OUTPUT_DIRECTORY/test-results/$TEST_NAME"
+
 # if --tee was passed, write the output not only to the terminal, but
 # additionally to the file test-results/$BASENAME.out, too.
 case "$GIT_TEST_TEE_STARTED, $* " in
@@ -78,12 +81,11 @@  done,*)
 	# do not redirect again
 	;;
 *' --tee '*|*' --va'*|*' -V '*|*' --verbose-log '*)
-	mkdir -p "$TEST_OUTPUT_DIRECTORY/test-results"
-	BASE="$TEST_OUTPUT_DIRECTORY/test-results/$(basename "$0" .sh)"
+	mkdir -p "$(dirname "$TEST_RESULTS_BASE")"
 
 	# Make this filename available to the sub-process in case it is using
 	# --verbose-log.
-	GIT_TEST_TEE_OUTPUT_FILE=$BASE.out
+	GIT_TEST_TEE_OUTPUT_FILE=$TEST_RESULTS_BASE.out
 	export GIT_TEST_TEE_OUTPUT_FILE
 
 	# Truncate before calling "tee -a" to get rid of the results
@@ -91,8 +93,8 @@  done,*)
 	>"$GIT_TEST_TEE_OUTPUT_FILE"
 
 	(GIT_TEST_TEE_STARTED=done ${TEST_SHELL_PATH} "$0" "$@" 2>&1;
-	 echo $? >"$BASE.exit") | tee -a "$GIT_TEST_TEE_OUTPUT_FILE"
-	test "$(cat "$BASE.exit")" = 0
+	 echo $? >"$TEST_RESULTS_BASE.exit") | tee -a "$GIT_TEST_TEE_OUTPUT_FILE"
+	test "$(cat "$TEST_RESULTS_BASE.exit")" = 0
 	exit
 	;;
 esac
@@ -818,12 +820,9 @@  test_done () {
 
 	if test -z "$HARNESS_ACTIVE"
 	then
-		test_results_dir="$TEST_OUTPUT_DIRECTORY/test-results"
-		mkdir -p "$test_results_dir"
-		base=${0##*/}
-		test_results_path="$test_results_dir/${base%.sh}.counts"
+		mkdir -p "$(dirname "$TEST_RESULTS_BASE")"
 
-		cat >"$test_results_path" <<-EOF
+		cat >"$TEST_RESULTS_BASE.counts" <<-EOF
 		total $test_count
 		success $test_success
 		fixed $test_fixed
@@ -1029,7 +1028,7 @@  then
 fi
 
 # Test repository
-TRASH_DIRECTORY="trash directory.$(basename "$0" .sh)"
+TRASH_DIRECTORY="trash directory.$TEST_NAME"
 test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY"
 case "$TRASH_DIRECTORY" in
 /*) ;; # absolute path is good