test: Improve tests

Author: hyperupcall

pkg/lib/util/pkg.sh

@@ -83,10 +83,6 @@ pkg.phase_download_tarball() {
 	local download_dest="$BASALT_GLOBAL_DATA_DIR/store/tarballs/$package_id.tar.gz"
 	mkdir -p "${download_dest%/*}"
 
-	if [ ${DEBUG+x} ]; then
-		print.indent-light-cyan "Downloading" "$package_id | $download_dest"
-	fi
-
 	# Use cache if it already exists
 	if [ -e "$download_dest" ]; then
 		print.indent-green "Downloaded" "$package_id (cached)"
@@ -104,13 +100,12 @@ pkg.phase_download_tarball() {
 				print.indent-die "File '$download_dest' is not actually a tarball"
 			fi
 
-			print.indent-green "Downloaded" "$site/$package@$version"
+			print.indent-green "Downloaded" "$package_id"
 			return
-		else
-			# This is OK, since the 'version' could be an actual ref. In that case,
-			# download the package as below
-			:
 		fi
+
+		# If cURL fails, this is OK, since the 'version' could be an actual ref. In that case,
+		# download the package as below. It does this automatically for 'local' packages
 	fi
 
 	# TODO Print warning if a local dependency has a dirty index
@@ -120,12 +115,12 @@ pkg.phase_download_tarball() {
 	fi
 
 	rm -rf "$BASALT_GLOBAL_DATA_DIR/scratch"
-	if ! git clone --quiet "$url" "$BASALT_GLOBAL_DATA_DIR/scratch/$package_id" 2>/dev/null; then
+	if ! git clone --quiet "$url" "$BASALT_GLOBAL_DATA_DIR/scratch/$package_id"; then
 		print.indent-die "Could not clone repository for $package_id"
 	fi
 
-	if ! git -C "$BASALT_GLOBAL_DATA_DIR/scratch/$package_id" archive --prefix="prefix/" -o "$download_dest" "$version"; then
-		rm -rf "$BASALT_GLOBAL_DATA_DIR/scratch"
+	if ! git -C "$BASALT_GLOBAL_DATA_DIR/scratch/$package_id" archive --prefix="prefix/" -o "$download_dest" "$version" 2>/dev/null; then
+		rm -rf "$BASALT_GLOBAL_DATA_DIR/scratch" "$download_dest"
 		print.indent-die "Could not download archive or extract archive from temporary Git repository of $package_id"
 	fi
 	rm -rf "$BASALT_GLOBAL_DATA_DIR/scratch"
@@ -146,10 +141,6 @@ pkg.phase_extract_tarball() {
 	local tarball_src="$BASALT_GLOBAL_DATA_DIR/store/tarballs/$package_id.tar.gz"
 	local tarball_dest="$BASALT_GLOBAL_DATA_DIR/store/packages/$package_id"
 
-	if [ ${DEBUG+x} ]; then
-		print.indent-light-cyan "Extracting" "$package_id | $tarball_dest"
-	fi
-
 	# Use cache if it already exists
 	if [ -d "$tarball_dest" ]; then
 		print.indent-green "Extracted" "$package_id (cached)"
@@ -178,10 +169,6 @@ pkg.phase_global_integration() {
 
 	local project_dir="$BASALT_GLOBAL_DATA_DIR/store/packages/$package_id"
 
-	if [ ${DEBUG+x} ]; then
-		print.indent-light-cyan "Transforming" "$project_dir"
-	fi
-
 	ensure.dir "$project_dir"
 	if [ -f "$project_dir/basalt.toml" ]; then
 		# Install dependencies

pkg/lib/util/symlink.sh

@@ -13,10 +13,6 @@ symlink.package() {
 	local target="$BASALT_GLOBAL_DATA_DIR/store/packages/$package_id"
 	local link_name="$install_dir/$package_id"
 
-	if [ ${DEBUG+x} ]; then
-		print.indent-light-cyan "Symlinking" "$link_name -> $target"
-	fi
-
 	mkdir -p "${link_name%/*}"
 	if ! ln -sf "$target" "$link_name"; then
 		print.indent-die "Could not symlink directory '${target##*/}' for package $package_id"
@@ -42,11 +38,6 @@ symlink.bin_strict() {
 					for target in "$package_dir/$dir"/*; do
 						local link_name="$install_dir/bin/${target##*/}"
 
-						if [ ${DEBUG+x} ]; then
-							print.indent-light-cyan "Symlinking" "target    $target"
-							print.indent-light-cyan "Symlinking" "link_name $link_name"
-						fi
-
 						if [ -e "$link_name" ]; then
 							print-indent 'Warning' "Executable file '${target##*/} for package $package_id will clobber an identically-named file owned by a different package"
 						fi

pkg/lib/util/util.sh

@@ -108,7 +108,7 @@ util.get_package_id() {
 	if [ "$repo_type" = 'remote' ]; then
 		REPLY="$site/$package@$version"
 	elif [ "$repo_type" = 'local' ]; then
-		REPLY="local/${url##*/}"
+		REPLY="local/${url##*/}@$version"
 	else
 		util.die_unexpected_value 'repo_type'
 	fi

tests/pkg-phase-download-tarball.bats

@@ -2,8 +2,68 @@
 
 load './util/init.sh'
 
+@test "Succeeds for valid repository" {
+	local package_id="github.com/hyperupcall/[email protected]"
+	util.get_package_info "https://$package_id"
+
+	run pkg.phase_download_tarball "$REPLY1" "$REPLY2" "$REPLY3" "$REPLY4" "$REPLY5"
+
+	assert_success
+	assert_line -p "Downloaded $package_id"
+	assert_file_exist "$BASALT_GLOBAL_DATA_DIR/store/tarballs/$package_id.tar.gz"
+}
+
+@test "Succeeds with caching" {
+	local package_id="github.com/hyperupcall/[email protected]"
+	util.get_package_info "https://$package_id"
+
+	run pkg.phase_download_tarball "$REPLY1" "$REPLY2" "$REPLY3" "$REPLY4" "$REPLY5"
+
+	assert_success
+	assert_line -p "Downloaded $package_id"
+	assert_file_exist "$BASALT_GLOBAL_DATA_DIR/store/tarballs/$package_id.tar.gz"
+
+	run pkg.phase_download_tarball "$REPLY1" "$REPLY2" "$REPLY3" "$REPLY4" "$REPLY5"
+	assert_line -p "Downloaded $package_id (cached)"
+	assert_file_exist "$BASALT_GLOBAL_DATA_DIR/store/tarballs/$package_id.tar.gz"
+
+	assert_file_exist "$BASALT_GLOBAL_DATA_DIR/store/tarballs/$package_id.tar.gz"
+}
+
+# Note that by the time we get to 'util.get_package_info', we expect the version (REPLY5)
+# to properly be calculated in the code via checking the version with -z, and calling
+# `util.get_latest_package_version`
+@test "Succeeds with local file" {
+	test_util.create_fake_remote 'user/repo' 'v0.0.1'; dir="$REPLY"
+	util.get_package_info "file://$dir"
+
+	run pkg.phase_download_tarball "$REPLY1" "$REPLY2" "$REPLY3" "$REPLY4" 'v0.0.1'
+
+	assert_success
+	assert_line -p "Downloaded local/[email protected]"
+	assert_file_exist "$BASALT_GLOBAL_DATA_DIR/store/tarballs/local/[email protected]"
+}
+
 @test "Fails for invalid repository" {
+	local package_id="github.com/hyperupcall/[email protected]"
+	util.get_package_info "https://$package_id"
+
+	run pkg.phase_download_tarball "$REPLY1" "$REPLY2" "$REPLY3" "$REPLY4" "$REPLY5"
+
+	assert_failure
+	assert_line -p "Could not clone repository for $package_id"
+	assert_not_exist "$BASALT_GLOBAL_DATA_DIR/store/tarballs/$package_id.tar.gz"
+}
+
+@test "Fails for invalid repository version" {
 	skip
 
-	pkg.phase_download_tarball 'remote' 'https://github.com/hyperupcall/bash-object.git' 'github.com' 'hyperupcall/bash-object' 'v0.3.0'
+	local package_id="github.com/hyperupcall/[email protected]"
+	util.get_package_info "https://$package_id"
+
+	run pkg.phase_download_tarball "$REPLY1" "$REPLY2" "$REPLY3" "$REPLY4" "$REPLY5"
+
+	assert_failure
+	assert_line -p "Could not download archive or extract archive from temporary Git repository of $package_id"
+	assert_not_exist "$BASALT_GLOBAL_DATA_DIR/store/tarballs/$package_id.tar.gz"
 }

tests/util/init.sh

@@ -1,12 +1,7 @@
 # shellcheck shell=bash
 
 # Source testing dependencies
-load './vendor/bats-assert/assert.sh'
-load './vendor/bats-file/file.sh'
-load './vendor/bats-file/temp.sh'
-load './vendor/bats-support/error.sh'
-load './vendor/bats-support/lang.sh'
-load './vendor/bats-support/output.sh'
+load './vendor/bats-common-utils/load.bash'
 load './util/test_util.sh'
 
 # Get the current directory of the Basalt git repository
@@ -27,6 +22,7 @@ basalt-package-init() { basalt-package-init.main "$@"; }
 basalt() { basalt.main "$@"; }
 
 # Testing variables
+export NO_COLOR=
 export XDG_DATA_HOME=
 export BASALT_GLOBAL_REPO="$BATS_TEST_TMPDIR/source"
 export BASALT_GLOBAL_DATA_DIR="$BATS_TEST_TMPDIR/localshare"

tests/util/test_util.sh

@@ -24,6 +24,7 @@ test_util.get_repo_root() {
 test_util.create_fake_remote() {
 	unset REPLY; REPLY=
 	local package="$1"
+	local version="${2:-v0.0.1}"
 
 	local git_dir="$BATS_TEST_TMPDIR/fake_remote_${package%/*}_${package#*/}"
 
@@ -35,8 +36,8 @@ test_util.create_fake_remote() {
 		git add .
 		git commit -m 'Initial commit'
 		git branch -M main
-		git commit --allow-empty -m 'v0.0.1'
-		git tag -m 'v0.0.1' 'v0.0.1'
+		git commit --allow-empty -m "$version"
+		git tag -m "$version" "$version"
 	} >/dev/null 2>&1
 
 	REPLY="$git_dir"