test: Significantly improve test suite

This adds a bunch of tests, and refactors most of the current ones to use
the `test_util.setup_pkg` and `test_util.fake_install` functions. This conversion
is being done because it makes the tests significantly easier to reason about
without common function helper abstractions

Author: hyperupcall

README.md

@@ -49,7 +49,6 @@ Why not use `bpkg` or `Basher`? Because `bpm`...
 - Is faster (bpm considers exec and subshell creation overhead)
 - Has a _much_ improved help output (basher)
 - Prints why a command failed, rather than just printing the help menu (basher)
-- Has actually been updated recently
 - Better bpm completion scripts
 - More flexibly parses command line arguments (basher)
 

docs/recepies.md

@@ -1,6 +1,6 @@
 # Recepies
 
-### Installing packages from Github
+### Installing a package from Github
 
 ```sh
 bpm install sstephenson/bats

pkg/lib/util/abstract-bins.sh

@@ -102,7 +102,9 @@ abstract.bins_do_action() {
 			chmod +x "$BPM_INSTALL_BIN/$binName"
 			;;
 		unlink)
-			unlink "$BPM_INSTALL_BIN/$binName"
+			if [ -f "$BPM_INSTALL_BIN/$binName" ]; then
+				unlink "$BPM_INSTALL_BIN/$binName"
+			fi
 			;;
 	esac
 }

pkg/lib/util/abstract-completions.sh

@@ -91,7 +91,9 @@ abstract.completions_do_action_bash() {
 		ln -sf "$file" "$BPM_INSTALL_COMPLETIONS/bash/${file##*/}"
 		;;
 	unlink)
-		unlink "$BPM_INSTALL_COMPLETIONS/bash/${file##*/}"
+		if [ -f "$BPM_INSTALL_COMPLETIONS/bash/${file##*/}" ]; then
+			unlink "$BPM_INSTALL_COMPLETIONS/bash/${file##*/}"
+		fi
 		;;
 	esac
 
@@ -103,15 +105,15 @@ abstract.completions_do_action_zsh() {
 
 	if grep -qs "^#compdef" "$file"; then
 		# TODO: run mkdir outside of loop
-		# TODO: unlink?
 		case "$action" in
 		link)
 			mkdir -p "$BPM_INSTALL_COMPLETIONS/zsh/compsys"
 			ln -sf "$file" "$BPM_INSTALL_COMPLETIONS/zsh/compsys/${file##*/}"
 			;;
 		unlink)
-			rm -f "$BPM_INSTALL_COMPLETIONS/zsh/compsys/${file##*/}"
-			# unlink "$BPM_INSTALL_COMPLETIONS/zsh/compsys/${file##*/}"
+			if [ -f "$BPM_INSTALL_COMPLETIONS/zsh/compsys/${file##*/}" ]; then
+				unlink "$BPM_INSTALL_COMPLETIONS/zsh/compsys/${file##*/}"
+			fi
 			;;
 		esac
 	else
@@ -121,8 +123,9 @@ abstract.completions_do_action_zsh() {
 			ln -sf "$file" "$BPM_INSTALL_COMPLETIONS/zsh/compctl/${file##*/}"
 			;;
 		unlink)
-			rm -f "$BPM_INSTALL_COMPLETIONS/zsh/compctl/${file##*/}"
-			# unlink "$BPM_INSTALL_COMPLETIONS/zsh/compctl/${file##*/}"
+			if [ -f "$BPM_INSTALL_COMPLETIONS/zsh/compctl/${file##*/}" ]; then
+				unlink "$BPM_INSTALL_COMPLETIONS/zsh/compctl/${file##*/}"
+			fi
 			;;
 		esac
 	fi

pkg/lib/util/abstract-mans.sh

@@ -66,7 +66,9 @@ abstract.mans_search_heuristics() {
 	done
 }
 
-# @arg $1 The man file to symlink or remove
+# @arg $1 The man file to symlink or remove. Not all the files passed
+# in here are man pages, which is why the regex check exists, to extract
+# the file ending (and the man category)
 abstract.mans_do_action() {
 	local action="$1"
 	local full_man_file="$2"
@@ -83,9 +85,12 @@ abstract.mans_do_action() {
 				ln -sf "$full_man_file" "$BPM_INSTALL_MAN/man$n/$manFile"
 				;;
 			unlink)
-				# TODO unlink?
-				rm -f "$BPM_INSTALL_MAN/man$n/$file"
-				# unlink "$BPM_INSTALL_MAN/man$n/$manFile"
+				# Because 'abstract.mans_search_heuristics' sometimes repeats
+				# directories, and sometimes the stat's are out of dates, we add this
+				# check in case a file was deleted in the meantime
+				if [ -f "$BPM_INSTALL_MAN/man$n/$manFile" ]; then
+					unlink "$BPM_INSTALL_MAN/man$n/$manFile"
+				fi
 				;;
 		esac
 	fi

pkg/lib/util/util.sh

@@ -198,7 +198,7 @@ util.extract_shell_variable() {
 	local shellFile="$1"
 	local variableName="$2"
 
-	if [ ! -f "$shellFile" ]; then
+	if [ ! -e "$shellFile" ]; then
 		die "File '$shellFile' not found"
 	fi
 

pkg/share/include.sh

@@ -15,7 +15,7 @@ include() {
 	fi
 
 	if [ ! -d "$BPM_PREFIX/packages/$package" ]; then
-		printf "%s" "Error: Package not installed: $package" >&2
+		printf "%s" "Error: Package '$package' not installed" >&2
 		return 1
 	fi
 

tests/do-init.bats

@@ -6,28 +6,28 @@ load './util/init.sh'
 	BPM_ROOT=/lol run do-init bash
 
 	assert_success
-	assert_line -e 'export BPM_ROOT="/lol"'
+	assert_line -p 'export BPM_ROOT="/lol"'
 }
 
 @test "exports BPM_PREFIX" {
 	BPM_PREFIX=/lol run do-init bash
 
 	assert_success
-	assert_line -e 'export BPM_PREFIX="/lol"'
+	assert_line -p 'export BPM_PREFIX="/lol"'
 }
 
 @test "exports BPM_PACKAGES_PATH" {
 	BPM_PACKAGES_PATH=/lol run do-init bash
 
 	assert_success
-	assert_line -e 'export BPM_PACKAGES_PATH="/lol"'
+	assert_line -p 'export BPM_PACKAGES_PATH="/lol"'
 }
 
 @test "errors if shell is not available" {
 	run do-init fakesh
 
 	assert_failure
-	assert_line -e "Shell 'fakesh' is not a valid shell"
+	assert_line -p "Shell 'fakesh' is not a valid shell"
 }
 
 @test "bash completion works" {
@@ -47,10 +47,6 @@ load './util/init.sh'
 }
 
 @test "is sh-compatible" {
-	if ! command -v sh &>/dev/null; then
-		skip "Command 'sh' not in PATH"
-	fi
-
-	run sh -ec 'eval "$(bpm init - sh)"'
+	run eval "$(do-init - sh)"
 	assert_success
 }

tests/do-link.bats

@@ -9,10 +9,10 @@ load 'util/init.sh'
 	assert_output -p "Directory 'invalid' not found"
 }
 
-@test "fails with a file path instead of a directory path" {
-	touch file1
+@test "fails with a file" {
+	touch 'file1'
 
-	run do-link file1
+	run do-link 'file1'
 
 	assert_failure
 	assert_output -p "Directory 'file1' not found"
@@ -24,11 +24,11 @@ load 'util/init.sh'
 	test_util.mock_command do-plumbing-link-completions
 	test_util.mock_command do-plumbing-link-man
 
-	mkdir -p subdir/theta
-	do-link subdir/theta
+	mkdir -p 'subdir/theta'
+	do-link 'subdir/theta'
+	mkdir 'theta'
 
-	mkdir theta
-	run do-link theta
+	run do-link 'theta'
 
 	assert_failure
 	assert_line -n 0 -p "Package 'bpm-local/theta' is already present"
@@ -42,26 +42,26 @@ load 'util/init.sh'
 
 	mkdir -p touch "$BPM_PACKAGES_PATH/bpm-local"
 	touch "$BPM_PACKAGES_PATH/bpm-local/theta"
+	mkdir 'theta'
 
-	mkdir theta
-	run do-link theta
+	run do-link 'theta'
 
 	assert_failure
 	assert_line -n 0 -p "Package 'bpm-local/theta' is already present"
 }
 
-@test "links the package to packages under the correct namespace" {
+@test "links the package to packages under the correct namespace (bpm-local)" {
 	test_util.mock_command do-plumbing-add-deps
 	test_util.mock_command do-plumbing-link-bins
 	test_util.mock_command do-plumbing-link-completions
 	test_util.mock_command do-plumbing-link-man
 
-	mkdir package1
+	mkdir 'package1'
 
-	run do-link package1
+	run do-link 'package1'
 
 	assert_success
-	assert [ "$(test_util.readlink $BPM_PACKAGES_PATH/bpm-local/package1)" = "$(test_util.readlink "$PWD/package1")" ]
+	assert [ "$(readlink -f $BPM_PACKAGES_PATH/bpm-local/package1)" = "$(readlink -f "$PWD/package1")" ]
 }
 
 @test "calls link-bins, link-completions, link-man and deps in order" {
@@ -70,9 +70,9 @@ load 'util/init.sh'
 	test_util.mock_command do-plumbing-link-completions
 	test_util.mock_command do-plumbing-link-man
 
-	mkdir package2
+	mkdir 'package2'
 
-	run do-link package2
+	run do-link 'package2'
 
 	assert_success
 	assert_line -n 0 -e "Linking '/(.*)/bpm/cwd/package2'"
@@ -83,15 +83,15 @@ load 'util/init.sh'
 
 }
 
-@test "respects --no-deps option, in order, with --nodeps" {
+@test "respects the --no-deps option in the correct order" {
 	test_util.mock_command do-plumbing-add-deps
 	test_util.mock_command do-plumbing-link-bins
 	test_util.mock_command do-plumbing-link-completions
 	test_util.mock_command do-plumbing-link-man
 
-	mkdir package2
+	mkdir 'package2'
 
-	run do-link --no-deps package2
+	run do-link --no-deps 'package2'
 
 	assert_success
 	assert_line -n 0 -e "Linking '/(.*)/bpm/cwd/package2'"
@@ -101,60 +101,59 @@ load 'util/init.sh'
 }
 
 
-@test "respects --no-deps option" {
+@test "respects the --no-deps option" {
 	test_util.mock_command do-plumbing-add-deps
 	test_util.mock_command do-plumbing-link-bins
 	test_util.mock_command do-plumbing-link-completions
 	test_util.mock_command do-plumbing-link-man
 
-	mkdir package2
+	mkdir 'package2'
 
-	run do-link --no-deps package2
+	run do-link --no-deps 'package2'
 
 	assert_success
 	refute_line "do-plumbing-add-deps bpm-local/package2"
 }
 
-@test "resolves current directory (dot) path" {
+@test "links the current directory" {
 	test_util.mock_command do-plumbing-add-deps
 	test_util.mock_command do-plumbing-link-bins
 	test_util.mock_command do-plumbing-link-completions
 	test_util.mock_command do-plumbing-link-man
 
-	mkdir package3
-	cd package3
+	mkdir 'package3'
+	cd 'package3'
 
 	run do-link .
 
 	assert_success
-	assert [ "$(test_util.readlink "$BPM_PACKAGES_PATH/bpm-local/package3")" = "$(test_util.readlink "$PWD")" ]
+	assert [ "$(readlink -f "$BPM_PACKAGES_PATH/bpm-local/package3")" = "$(readlink -f "$PWD")" ]
 }
 
-@test "resolves parent directory (dotdot) path" {
+@test "links the parent directory" {
 	test_util.mock_command do-plumbing-add-deps
 	test_util.mock_command do-plumbing-link-bins
 	test_util.mock_command do-plumbing-link-completions
 	test_util.mock_command do-plumbing-link-man
 
-	mkdir package3
-	cd package3
+	mkdir -p 'sierra/tango'
+	cd 'sierra/tango'
 
-	run do-link ../package3
+	run do-link ..
 
 	assert_success
-	assert [ "$(test_util.readlink "$BPM_PACKAGES_PATH/bpm-local/package3")" = "$(test_util.readlink "$PWD")" ]
+	assert [ "$(readlink -f "$BPM_PACKAGES_PATH/bpm-local/sierra")" = "$(readlink -f "$PWD/..")" ]
 }
 
-@test "resolves arbitrary complex relative path" {
+@test "links an arbitrary complex relative path" {
 	test_util.mock_command do-plumbing-add-deps
 	test_util.mock_command do-plumbing-link-bins
 	test_util.mock_command do-plumbing-link-completions
 	test_util.mock_command do-plumbing-link-man
 
-	mkdir package3
-
+	mkdir 'package3'
 	run do-link ./package3/.././package3
 
 	assert_success
-	assert [ "$(test_util.readlink "$BPM_PACKAGES_PATH/bpm-local/package3")" = "$(test_util.readlink "$PWD/package3")" ]
+	assert [ "$(readlink -f "$BPM_PACKAGES_PATH/bpm-local/package3")" = "$(readlink -f "$PWD/package3")" ]
 }

tests/do-list.bats

@@ -2,12 +2,12 @@
 
 load 'util/init.sh'
 
-@test "list installed packages" {
-	create_package username/p1
-	create_package username2/p2
-	create_package username2/p3
-	test_util.fake_clone username/p1
-	test_util.fake_clone username2/p2
+@test "properly list for 2 installed packages" {
+	create_package 'username/p1'
+	create_package 'username2/p2'
+	create_package 'username2/p3'
+	test_util.fake_clone 'username/p1'
+	test_util.fake_clone 'username2/p2'
 
 	run do-list
 
@@ -17,7 +17,7 @@ load 'util/init.sh'
 	refute_line "username2/p3"
 }
 
-@test "displays nothing if there are no packages" {
+@test "properly list for no installed packages" {
 	create_package username/p1
 
 	run do-list
@@ -26,7 +26,7 @@ load 'util/init.sh'
 	assert_output ""
 }
 
-@test "displays outdated packages" {
+@test "properly list outdated packages" {
 	create_package username/outdated
 	create_package username/uptodate
 	test_util.fake_clone username/outdated