From 15b4b28a02b813423e30d8614f27a9f2645a751f Mon Sep 17 00:00:00 2001
From: Michael McCracken <mikmccra@cisco.com>
Date: Thu, 9 Jan 2025 16:47:45 -0800
Subject: [PATCH] test: add shellcheck to bats tests, fix lint

A previous test bug would have been caught by linting the bats test
code, so let's do that now.

That error was using an undefined variable in a test. ($last_layer_hash
in atomfs.bats) It had been defined in a different test in the same
file, but was undefined in the test in question. Unfortunately because
of the way bats does bash, shellcheck only identifies that as an INFO
level possible error SC2031.

So this adds a new `lintbats` target that first checks for JUST SC2031,
then checks separately for only error level issues.

This commit also fixes a few error level issues in other files:
- asterisk and whiteout : use glob instead of looping over ls. /shrug
- bom: [ -n "${ZOT_HOST}:${ZOT_PORT}" ] will always be true because of
the colon in there, so this commit eliminates the colon.
- config.bats had a complaint about the indentation of the EOF because
it's in yaml in another here doc? OK then.

Also add the shellcheck package to the build deps.

Signed-off-by: Michael McCracken <mikmccra@cisco.com>
---
 Makefile              |  9 ++++++++-
 install-build-deps.sh |  1 +
 test/asterisk.bats    |  2 +-
 test/bom.bats         | 14 +++++++-------
 test/config.bats      |  1 +
 test/whiteout.bats    |  2 +-
 6 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index e123156b..8952655d 100644
--- a/Makefile
+++ b/Makefile
@@ -169,7 +169,7 @@ PRIVILEGE_LEVEL?=
 check: lint test go-test
 
 .PHONY: test
-test: stacker download-tools
+test: stacker download-tools lintbats
 	sudo -E PATH="$(PATH)" \
 		STACKER_BUILD_ALPINE_IMAGE=$(STACKER_BUILD_ALPINE_IMAGE) \
 		STACKER_BUILD_BUSYBOX_IMAGE=$(STACKER_BUILD_BUSYBOX_IMAGE) \
@@ -179,6 +179,13 @@ test: stacker download-tools
 		$(shell [ -z $(PRIVILEGE_LEVEL) ] || echo --privilege-level=$(PRIVILEGE_LEVEL)) \
 		$(patsubst %,test/%.bats,$(TEST))
 
+.PHONY: lintbats
+lintbats:
+	# check only SC2031 which finds undefined variables in bats tests but is only an INFO
+	shellcheck -i SC2031 $(patsubst %,test/%.bats,$(TEST))
+	# check all error level issues
+	shellcheck -S error  $(patsubst %,test/%.bats,$(TEST))
+
 .PHONY: check-cov
 check-cov: lint test-cov
 
diff --git a/install-build-deps.sh b/install-build-deps.sh
index bae0a887..17a75eaf 100755
--- a/install-build-deps.sh
+++ b/install-build-deps.sh
@@ -43,6 +43,7 @@ installdeps_ubuntu() {
         squashfs-tools
         squashfuse
         libarchive-tools
+        shellcheck
     )
 
     case "$VERSION_ID" in
diff --git a/test/asterisk.bats b/test/asterisk.bats
index 9a88d4e2..9128d55d 100644
--- a/test/asterisk.bats
+++ b/test/asterisk.bats
@@ -23,7 +23,7 @@ EOF
     [ "$status" -eq 0 ]
 
 
-    for i in $(ls dest/rootfs/bin); do
+    for i in dest/rootfs/bin/*; do
         stat dest/rootfs/mybin/$i
     done
 }
diff --git a/test/bom.bats b/test/bom.bats
index add64e7e..45b736a5 100644
--- a/test/bom.bats
+++ b/test/bom.bats
@@ -1,13 +1,13 @@
 load helpers
 
 function setup_file() {
-  if [ -n "${ZOT_HOST}:${ZOT_PORT}" ]; then
+  if [ -n "${ZOT_HOST}${ZOT_PORT}" ]; then
     zot_setup
   fi
 }
 
 function teardown_file() {
-  if [ -n "${ZOT_HOST}:${ZOT_PORT}" ]; then
+  if [ -n "${ZOT_HOST}${ZOT_PORT}" ]; then
     zot_teardown
   fi
 }
@@ -154,7 +154,7 @@ EOF
     [ -f .stacker/artifacts/second/inventory.json ]
     # sbom for this image
     [ -f .stacker/artifacts/second/second.json ]
-    if [ -n "${ZOT_HOST}:${ZOT_PORT}" ]; then
+    if [ -n "${ZOT_HOST}${ZOT_PORT}" ]; then
       zot_setup
       stacker publish --skip-tls --url docker://${ZOT_HOST}:${ZOT_PORT} --tag latest --substitute CENTOS_OCI=${CENTOS_OCI}
       refs=$(regctl artifact tree ${ZOT_HOST}:${ZOT_PORT}/first:latest --format "{{json .}}" | jq '.referrer | length')
@@ -204,7 +204,7 @@ EOF
     [ -f .stacker/artifacts/bom-alpine/inventory.json ]
     # sbom for this image
     [ -f .stacker/artifacts/bom-alpine/bom-alpine.json ]
-    if [ -n "${ZOT_HOST}:${ZOT_PORT}" ]; then
+    if [ -n "${ZOT_HOST}${ZOT_PORT}" ]; then
       zot_setup
       stacker publish --skip-tls --url docker://${ZOT_HOST}:${ZOT_PORT} --tag latest --substitute ALPINE_OCI=${ALPINE_OCI}
       refs=$(regctl artifact tree ${ZOT_HOST}:${ZOT_PORT}/bom-alpine:latest --format "{{json .}}" | jq '.referrer | length')
@@ -258,7 +258,7 @@ EOF
     [ -f .stacker/artifacts/parent/inventory.json ]
     # sbom for this image
     [ -f .stacker/artifacts/parent/parent.json ]
-    if [ -n "${ZOT_HOST}:${ZOT_PORT}" ]; then
+    if [ -n "${ZOT_HOST}${ZOT_PORT}" ]; then
       stacker publish --skip-tls --url docker://${ZOT_HOST}:${ZOT_PORT} --tag latest
       refs=$(regctl artifact tree ${ZOT_HOST}:${ZOT_PORT}/parent:latest --format "{{json .}}" | jq '.referrer | length')
       [ $refs -eq 2 ]
@@ -296,7 +296,7 @@ EOF
     [ -f .stacker/artifacts/child/inventory.json ]
     # sbom for this image
     [ -f .stacker/artifacts/child/child.json ]
-    if [ -n "${ZOT_HOST}:${ZOT_PORT}" ]; then
+    if [ -n "${ZOT_HOST}${ZOT_PORT}" ]; then
       stacker publish --skip-tls --url docker://${ZOT_HOST}:${ZOT_PORT} --tag latest
       refs=$(regctl artifact tree ${ZOT_HOST}:${ZOT_PORT}/child:latest --format "{{json .}}" | jq '.referrer | length')
       [ $refs -eq 2 ]
@@ -450,7 +450,7 @@ EOF
     [ -f .stacker/artifacts/second/inventory.json ]
     # sbom for this image
     [ -f .stacker/artifacts/second/second.json ]
-    if [ -n "${ZOT_HOST}:${ZOT_PORT}" ]; then
+    if [ -n "${ZOT_HOST}${ZOT_PORT}" ]; then
       zot_setup
       stacker publish --skip-tls --url docker://${ZOT_HOST}:${ZOT_PORT} --tag latest --substitute CENTOS_OCI=${CENTOS_OCI}
       refs=$(regctl artifact tree ${ZOT_HOST}:${ZOT_PORT}/second:latest --format "{{json .}}" | jq '.referrer | length')
diff --git a/test/config.bats b/test/config.bats
index ad3e5571..63c5363a 100644
--- a/test/config.bats
+++ b/test/config.bats
@@ -69,6 +69,7 @@ EOF
         "found rootfs=$rdir" "found oci=$odir" "found stacker=$sdir" \
         "found name=my-build" > "$expected"
 
+    # shellcheck disable=SC1039
     cat > "$stacker_yaml" <<"EOF"
 my-build:
     build_only: true
diff --git a/test/whiteout.bats b/test/whiteout.bats
index 71fa0008..4108fd03 100644
--- a/test/whiteout.bats
+++ b/test/whiteout.bats
@@ -21,7 +21,7 @@ EOF
 
   stacker build
   echo "checking"
-  for f in $(ls oci/blobs/sha256/); do
+  for f in oci/blobs/sha256/*; do
     file oci/blobs/sha256/$f | grep "gzip" || {
       echo "skipping $f"
         continue