From: Stefan Sperling <stsp@stsp.name>
Subject: run tests with set -u
To: gameoftrees@openbsd.org
Date: Tue, 8 Mar 2022 12:02:30 +0100

I think we should use set -u in command line tests to catch mistakes
related to use of unset variables.

The patch below does this and fixes some existing tests to keep passing.

ok?

-----------------------------------------------
commit b9aa2cb5715f027588c0779b474df2b736b1f8d9 (set-u)
from: Stefan Sperling <stsp@stsp.name>
date: Tue Mar  8 11:01:43 2022 UTC
 
 run all command line tests with set -u
 
 Run set -u in common.sh and adjust test scripts to keep passing.
 Fix occurances of unset variables in various tests.
 
diff 1ce77e2d7d2c2b9ef233ae3fc6dd12ba90e83bf6 7f39a4477ba44e4e10171b9f535e15330906c1b3
blob - a9916124c369cb2ba1bac4634cd2e80573716a5a
blob + 2da298bbc31afb238eb1590acafbb81297d71569
--- regress/cmdline/blame.sh
+++ regress/cmdline/blame.sh
@@ -19,7 +19,11 @@
 blame_cmp() {
 	local testroot="$1"
 	local file="$2"
-	local xfail="$3"
+	if [ $# -gt 2 ]; then
+		local xfail="$3"
+	else
+		local xfail=""
+	fi
 
 	(cd $testroot/wt && got blame "$file" | cut -d ' ' -f 2 \
 		> $testroot/${file}.blame.got)
@@ -794,7 +798,7 @@ test_blame_symlink() {
 
 	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret="$?"
-	if [ "$ret" != "0" -a "$xfail" = "" ]; then
+	if [ "$ret" != "0" ]; then
 		diff -u $testroot/stdout.expected $testroot/stdout
 		test_done "$testroot" "1"
 		return 1
@@ -815,7 +819,7 @@ test_blame_symlink() {
 
 	cmp -s $testroot/stdout.expected $testroot/stdout
 	ret="$?"
-	if [ "$ret" != "0" -a "$xfail" = "" ]; then
+	if [ "$ret" != "0" ]; then
 		diff -u $testroot/stdout.expected $testroot/stdout
 		test_done "$testroot" "1"
 		return 1
blob - d950aa95587abfc3c97d43aa360191d8fd9fc808
blob + 38d14fb58448ccab42ddfef43159d355ee919581
--- regress/cmdline/cleanup.sh
+++ regress/cmdline/cleanup.sh
@@ -17,7 +17,7 @@
 . ./common.sh
 
 # disable automatic packing for these tests
-export GOT_TEST_PACK=""
+export GOT_TEST_PACK="0"
 
 test_cleanup_unreferenced_loose_objects() {
 	local testroot=`test_init cleanup_unreferenced_loose_objects`
@@ -46,7 +46,7 @@ test_cleanup_unreferenced_loose_objects() {
 	echo 'modified alpha' > $testroot/wt/alpha
 	(cd $testroot/wt && got commit -m 'newbranch commit' > /dev/null)
 	local commit1=`git_show_branch_head $testroot/repo newbranch`
-	local tree1=`got cat -r $testroot/repo $newbranch_commit | \
+	local tree1=`got cat -r $testroot/repo $commit1 | \
 		grep ^tree | cut -d ' ' -f2`
 	local alpha1=`got tree -r $testroot/repo -i -c $commit1 | \
 		grep "[0-9a-f] alpha$" | cut -d' ' -f 1`
blob - 0e24ae3c91470f97788ef2e7a418a85ac613d339
blob + ef6c216480ca7a67e3c1fe8b7cc57218b7892456
--- regress/cmdline/clone.sh
+++ regress/cmdline/clone.sh
@@ -163,7 +163,7 @@ test_clone_branch() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -q -b foo $testurl/repo $testroot/repo-clone
 	ret="$?"
@@ -236,7 +236,7 @@ test_clone_all() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -q -a $testurl/repo $testroot/repo-clone
 	ret="$?"
@@ -313,7 +313,7 @@ test_clone_mirror() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -q -m $testurl/repo $testroot/repo-clone
 	ret="$?"
@@ -386,7 +386,7 @@ test_clone_mirror_all() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -q -m -a $testurl/repo $testroot/repo-clone
 	ret="$?"
@@ -459,7 +459,7 @@ test_clone_reference() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -q -R hoo $testurl/repo $testroot/repo-clone
 	ret="$?"
@@ -537,7 +537,7 @@ test_clone_branch_and_reference() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -q -R hoo/boo/zoo -b foo $testurl/repo $testroot/repo-clone
 	ret="$?"
@@ -613,7 +613,7 @@ test_clone_reference_mirror() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -q -R hoo -m $testurl/repo $testroot/repo-clone
 	ret="$?"
blob - df81db689facacce41e7257238b81e939bc87e2a
blob + a9cfb71adc2fdbf1404ff39d6d6afb08d138236f
--- regress/cmdline/common.sh
+++ regress/cmdline/common.sh
@@ -23,9 +23,17 @@ export GOT_AUTHOR_8="flan_hac"
 export GOT_AUTHOR_11="flan_hacker"
 export GOT_LOG_DEFAULT_LIMIT=0
 export GOT_TEST_ROOT="/tmp"
+if [ -z "$GOT_TEST_QUIET" ]; then
+	export GOT_TEST_QUIET="0"
+fi
+if [ -z "$GOT_TEST_PACK" ]; then
+	export GOT_TEST_PACK="0"
+fi
 
 export MALLOC_OPTIONS=S
 
+set -u
+
 git_init()
 {
 	git init -q "$1"
@@ -45,7 +53,7 @@ git_init()
 maybe_pack_repo()
 {
 	local repo="$1"
-	if [ -n "$GOT_TEST_PACK" ]; then
+	if [ "$GOT_TEST_PACK" != "0" ]; then
 		(cd $repo && gotadmin pack -a > /dev/null)
 		(cd $repo && gotadmin cleanup -a -q)
 	fi
@@ -83,7 +91,11 @@ git_show_branch_head()
 git_show_author_time()
 {
 	local repo="$1"
-	local object="$2"
+	if [ $# -gt 1 ]; then
+		local object="$2"
+	else
+		local object=""
+	fi
 	(cd $repo && git show --no-patch --pretty='format:%at' $object)
 }
 
@@ -98,7 +110,11 @@ git_show_tagger_time()
 git_show_parent_commit()
 {
 	local repo="$1"
-	local commit="$2"
+	if [ $# -gt 1 ]; then
+		local commit="$2"
+	else
+		local commit=""
+	fi
 	(cd $repo && git show --no-patch --pretty='format:%P' $commit)
 }
 
@@ -194,7 +210,11 @@ get_blob_id()
 test_init()
 {
 	local testname="$1"
-	local no_tree="$2"
+	if [ $# -gt 1 ]; then
+		local no_tree="$2"
+	else
+		local no_tree=""
+	fi
 	if [ -z "$testname" ]; then
 		echo "No test name provided" >&2
 		return 1
@@ -244,7 +264,7 @@ test_parseargs()
 run_test()
 {
 	testfunc="$1"
-	if [ -z "$GOT_TEST_QUIET" ]; then
+	if [ "$GOT_TEST_QUIET" = "0" ]; then
 		echo -n "$testfunc "
 	fi
 	$testfunc
@@ -256,7 +276,7 @@ test_done()
 	local result="$2"
 	if [ "$result" = "0" ]; then
 		test_cleanup "$testroot" || return 1
-		if [ -z "$GOT_TEST_QUIET" ]; then
+		if [ "$GOT_TEST_QUIET" = "0" ]; then
 			echo "ok"
 		fi
 	elif echo "$result" | grep -q "^xfail"; then
blob - 5b536e8cfc092fb0ac4d203d6360716d2f94ff5b
blob + 300ac2430c824074230f7cdefaf6355257354516
--- regress/cmdline/fetch.sh
+++ regress/cmdline/fetch.sh
@@ -172,7 +172,7 @@ test_fetch_branch() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	echo "modified alpha on master" > $testroot/repo/alpha
 	git_commit $testroot/repo -m "modified alpha"
@@ -279,7 +279,7 @@ test_fetch_all() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got ref -l -r $testroot/repo-clone > $testroot/stdout
 
@@ -335,7 +335,7 @@ test_fetch_empty_packfile() {
 
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -q $testurl/repo $testroot/repo-clone
 	ret="$?"
@@ -406,7 +406,7 @@ test_fetch_delete_branch() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -a -q $testurl/repo $testroot/repo-clone
 	ret="$?"
@@ -518,7 +518,7 @@ test_fetch_delete_branch_mirror() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -a -m -q $testurl/repo $testroot/repo-clone
 	ret="$?"
@@ -615,7 +615,7 @@ test_fetch_update_tag() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got clone -a -q $testurl/repo $testroot/repo-clone
 	ret="$?"
@@ -632,7 +632,7 @@ test_fetch_update_tag() {
 	got ref -r $testroot/repo -d "refs/tags/1.0"  >/dev/null
 	got tag -r $testroot/repo -c $commit_id2 -m tag "1.0" >/dev/null
 	local tag_id2=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	got ref -l -r $testroot/repo-clone > $testroot/stdout
 
@@ -782,7 +782,7 @@ test_fetch_reference() {
 	got ref -r $testroot/repo -c $commit_id refs/hoo/boo/zoo
 	got tag -r $testroot/repo -c $commit_id -m tag "1.0" >/dev/null
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	echo "modified alpha on master" > $testroot/repo/alpha
 	git_commit $testroot/repo -m "modified alpha"
blob - c944b6d2017466ab5b2bd1f3d69c28fff83eb0b1
blob + 368f9c1d0e864a40ee9695623426e17da280e375
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -981,7 +981,6 @@ test_histedit_path_prefix_drop() {
 		> $testroot/stdout)
 
 	local short_old_commit1=`trim_obj_id 28 $old_commit1`
-	local short_old_commit2=`trim_obj_id 28 $old_commit2`
 
 	echo "$short_old_commit1 ->  drop commit: changing zeta" \
 		> $testroot/stdout.expected
@@ -1578,7 +1577,6 @@ EOF
 	local short_old_commit2=`trim_obj_id 28 $old_commit2`
 	local short_old_commit3=`trim_obj_id 28 $old_commit3`
 	local short_new_commit1=`trim_obj_id 28 $new_commit1`
-	local short_new_commit2=`trim_obj_id 28 $new_commit2`
 
 	echo "G  alpha" > $testroot/stdout.expected
 	echo "D  beta" >> $testroot/stdout.expected
@@ -1695,7 +1693,6 @@ EOF
 	local short_old_commit2=`trim_obj_id 28 $old_commit2`
 	local short_old_commit3=`trim_obj_id 28 $old_commit3`
 	local short_new_commit1=`trim_obj_id 28 $new_commit1`
-	local short_new_commit2=`trim_obj_id 28 $new_commit2`
 
 	echo "G  alpha" > $testroot/stdout.expected
 	echo "D  beta" >> $testroot/stdout.expected
blob - 597e601e0fefb3fbe64e77e4c05e3ae7103dd8b3
blob + e6315753a928d3106098b77bef6bd5d25b527dc3
--- regress/cmdline/pack.sh
+++ regress/cmdline/pack.sh
@@ -17,7 +17,7 @@
 . ./common.sh
 
 # disable automatic packing for these tests
-export GOT_TEST_PACK=""
+export GOT_TEST_PACK="0"
 
 test_pack_all_loose_objects() {
 	local testroot=`test_init pack_all_loose_objects`
blob - 028eda58e7cab9cf077122c67e7736ce74a560b0
blob + 6e785863004f4ff0b7db0c3a0ae842d3cdf58c49
--- regress/cmdline/ref.sh
+++ regress/cmdline/ref.sh
@@ -299,7 +299,7 @@ test_ref_list() {
 		return 1
 	fi
 	local tag_id=`got ref -r $testroot/repo -l \
-		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+		| grep "^refs/tags/1.0" | tr -d ' ' | cut -d: -f2`
 
 	# Create a ref based on repository's HEAD reference
 	got ref -r $testroot/repo -c HEAD refs/foo/zoo
blob - 2f126fac8cca3afed8a2b489b7d7e3223c308752
blob + 6ce56c226cf19b641e78941f4704c1f10549bdac
--- regress/cmdline/stage.sh
+++ regress/cmdline/stage.sh
@@ -17,7 +17,7 @@
 . ./common.sh
 
 # disable automatic packing for these tests
-export GOT_TEST_PACK=""
+export GOT_TEST_PACK="0"
 
 test_stage_basic() {
 	local testroot=`test_init stage_basic`
blob - 9ca64b4d00d541ee2703cb9c58c93091e0ae020b
blob + 4dfd0868a78e552664caaa9ca8f04a0013586fd7
--- regress/cmdline/tree.sh
+++ regress/cmdline/tree.sh
@@ -123,7 +123,7 @@ test_tree_submodule_of_same_repo() {
 		test_done "$testroot" "1"
 		return 1
 	fi
-	if [ -n "$GOT_TEST_PACK" ]; then
+	if [ "$GOT_TEST_PACK" = "1" ]; then
 		echo "got-read-pack: bad object data" \
 			> $testroot/stderr.expected
 	else