system tests: remove unhelpful assertions

Regular primitive bats uses assertions like '[ $foo = something ]'. These are worthless for debugging: when they fail, all you know is that foo is not "something" but you don't know what foo _is_. Find and replace those assertions with 'assert', which is more informative. Instances found via: $ ack '^ *\[' tests/*.bats There are many matches for 'test' (instead of '[') but those mostly look like file-existence ones, which are less evil than string-check tests. I'm leaving those be for now. Signed-off-by: Ed Santiago <[email protected]>
containers · Jan 5, 2023 · 44dc562 · 44dc562
1 parent d2e7a94
commit 44dc562
Show file tree

Hide file tree

Showing 13 changed files with 75 additions and 65 deletions.
diff --git a/tests/basic.bats b/tests/basic.bats
@@ -128,7 +128,7 @@ load helpers
   run_buildah rmi containers-storage:other-new-image
   run_buildah rmi another-new-image
   run_buildah images -q
-  [ "$output" != "" ]
+  assert "$output" != "" "images -q"
   run_buildah rmi -a
   run_buildah images -q
   expect_output ""

diff --git a/tests/bud.bats b/tests/bud.bats
@@ -2064,7 +2064,7 @@ function _test_http() {
   root=$output
   test ! -s $root/vol/subvol/subvolfile
   run stat -c %f $root/vol/subvol
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status code from stat $root/vol/subvol"
   expect_output "41ed" "stat($root/vol/subvol) [0x41ed = 040755]"
 }
 
@@ -2180,12 +2180,12 @@ function _test_http() {
   run_buildah mount ${cid}
   root=$output
   run ls $root/data/log
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from ls $root/data/log"
   expect_output --substring "test"     "ls \$root/data/log"
   expect_output --substring "blah.txt" "ls \$root/data/log"
 
   run ls -al $root
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from ls -al $root"
   expect_output --substring "test-log -> /data/log" "ls -l \$root/data/log"
   expect_output --substring "blah -> /test-log"     "ls -l \$root/data/log"
 }
@@ -2199,11 +2199,11 @@ function _test_http() {
   run_buildah mount ${cid}
   root=$output
   run ls $root/log
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from ls $root/log"
   expect_output --substring "test" "ls \$root/log"
 
   run ls -al $root
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from ls -al $root"
   expect_output --substring "test-log -> ../log" "ls -l \$root/log"
   test -r $root/var/data/empty
 }
@@ -2217,20 +2217,20 @@ function _test_http() {
   run_buildah mount ${cid}
   root=$output
   run ls $root/data/log
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from ls $root/data/log"
   expect_output --substring "bin"      "ls \$root/data/log"
   expect_output --substring "blah.txt" "ls \$root/data/log"
 
   run ls -al $root/myuser
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from ls -al $root/myuser"
   expect_output --substring "log -> /test" "ls -al \$root/myuser"
 
   run ls -al $root/test
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from ls -al $root/test"
   expect_output --substring "bar -> /test-log" "ls -al \$root/test"
 
   run ls -al $root/test-log
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from ls -al $root/test-log"
   expect_output --substring "foo -> /data/log" "ls -al \$root/test-log"
 }
 
@@ -2250,11 +2250,11 @@ function _test_http() {
   run_buildah mount ${cid}
   root=$output
   run ls $root/bin
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from ls $root/bin"
   expect_output --substring "myexe" "ls \$root/bin"
 
   run cat $root/bin/myexe
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from cat $root/bin/myexe"
   expect_output "symlink-test" "cat \$root/bin/myexe"
 }
 
@@ -2267,7 +2267,7 @@ function _test_http() {
   run_buildah mount ${cid}
   root=$output
   run ls $root/data
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status from ls $root/data"
   expect_output --substring "myexe" "ls \$root/data"
 }
 

diff --git a/tests/commit.bats b/tests/commit.bats
@@ -132,7 +132,7 @@ load helpers
   config="$output"
   run python3 -c 'import json, sys; config = json.load(sys.stdin); print(config["history"][len(config["history"])-1]["created_by"])' <<< "$config"
   echo "$output"
-  [ "${status}" -eq 0 ]
+  assert "$status" -eq 0 "status from python command 1"
   expect_output "untracked actions"
 
   run_buildah config --created-by "" $cid
@@ -141,7 +141,7 @@ load helpers
   config="$output"
   run python3 -c 'import json, sys; config = json.load(sys.stdin); print(config["history"][len(config["history"])-1]["created_by"])' <<< "$config"
   echo "$output"
-  [ "${status}" -eq 0 ]
+  assert "$status" -eq 0 "status from python command 2"
   expect_output "/bin/sh"
 }
 

diff --git a/tests/help.bats b/tests/help.bats
@@ -68,8 +68,8 @@ function check_help() {
 
     # This can happen if the output of --help changes, such as between
     # the old command parser and cobra.
-    [ $count -gt 0 ] || \
-        die "Internal error: no commands found in 'buildah help $@' list"
+    assert "$count" -gt 0 \
+           "Internal error: no commands found in 'buildah help $@' list"
 
     # Sanity check: make sure the special loops above triggered at least once.
     # (We've had situations where a typo makes the conditional never run in podman)
@@ -85,8 +85,8 @@ function check_help() {
 
     # This can happen if the output of --help changes, such as between
     # the old command parser and cobra.
-    [ $count -gt 0 ] || \
-        die "Internal error: no commands found in 'buildah help list"
+    assert "$count" -gt 0 \
+           "Internal error: no commands found in 'buildah help list"
 
 }
 

diff --git a/tests/images.bats b/tests/images.bats
@@ -138,7 +138,7 @@ load helpers
 
   run_buildah images --json
   run python3 -m json.tool <<< "$output"
-  [ "${status}" -eq 0 ]
+  assert "$status" -eq 0 "status from python json.tool"
 }
 
 @test "specify an existing image" {

diff --git a/tests/lists.bats b/tests/lists.bats
@@ -81,8 +81,10 @@ IMAGE_LIST_S390X_INSTANCE_DIGEST=sha256:882a20ee0df7399a445285361d38b711c299ca09
     expect_output --substring ${IMAGE_LIST_ARM_INSTANCE_DIGEST}
     expect_output --substring ${IMAGE_LIST_PPC64LE_INSTANCE_DIGEST}
     expect_output --substring ${IMAGE_LIST_S390X_INSTANCE_DIGEST}
-    run grep ${IMAGE_LIST_ARM64_INSTANCE_DIGEST} <<< "$output"
-    [ $status -ne 0 ]
+
+    # ARM64 should now be gone
+    arm64=$(grep ${IMAGE_LIST_ARM64_INSTANCE_DIGEST} <<< "$output" || true)
+    assert "$arm64" = "" "arm64 instance digest found in manifest list"
 }
 
 @test "manifest-remove-not-found" {
@@ -108,8 +110,9 @@ IMAGE_LIST_S390X_INSTANCE_DIGEST=sha256:882a20ee0df7399a445285361d38b711c299ca09
 	    s390x) IMAGE_LIST_EXPECTED_INSTANCE_DIGEST=${IMAGE_LIST_S390X_INSTANCE_DIGEST} ;;
 	    *) skip "current arch \"$(go env GOARCH 2> /dev/null)\" not present in manifest list" ;;
     esac
+
     run grep ${IMAGE_LIST_EXPECTED_INSTANCE_DIGEST##sha256} ${TEST_SCRATCH_DIR}/pushed/manifest.json
-    [ $status -eq 0 ]
+    assert "$status" -eq 0 "status code of grep for expected instance digest"
 }
 
 @test "manifest-push-all" {

diff --git a/tests/namespaces.bats b/tests/namespaces.bats
@@ -84,7 +84,7 @@ idmapping_check_map() {
   local _expect_idmap=$2
   local _testname=$3
 
-  [ -n "$_output_idmap" ]
+  assert "$_output_idmap" != "" "Internal error: output_idmap is empty"
   local _idmap=$(sed -E -e 's, +, ,g' -e 's,^ +,,g' <<< "${_output_idmap}")
   expect_output --from="$_idmap" "${_expect_idmap}" "$_testname"
 
@@ -190,13 +190,13 @@ idmapping_check_permission() {
       local _output=$1
       local _testname=$2
 
-      [ "$_output" != "" ]
+      assert "$_output" != "" "Internal error: _output is empty"
       if [ -z "${uidmapargs[$i]}${gidmapargs[$i]}" ]; then
         if test "$BUILDAH_ISOLATION" != "chroot" -a "$BUILDAH_ISOLATION" != "rootless" ; then
           expect_output --from="$_output" "$mynamespace" "/proc/self/ns/user ($_testname)"
         fi
       else
-        [ "$_output" != "$mynamespace" ]
+        assert "$_output" != "$mynamespace" "_output vs mynamespace"
       fi
     }
 
@@ -236,7 +236,7 @@ idmapping_check_permission() {
     run_buildah mount "$ctr"
     mnt="$output"
     run stat -c '%u:%g %a' "$mnt"/somedir/someotherfile
-    [ $status -eq 0 ]
+    assert "$status" -eq 0 "status of stat $mnt/somedir/someotherfile"
     expect_output "$rootuid:$rootgid 4700"
 
     # Check that a container with mapped-layer can be committed.
@@ -299,15 +299,16 @@ general_namespace() {
   for namespace in "${types[@]}" ; do
     # Specify the setting for this namespace for this container.
     run_buildah from $WITH_POLICY_JSON --quiet --"$nsflag"=$namespace alpine
-    [ "$output" != "" ]
+    assert "$output" != "" "Internal error: buildah-from produced no output"
     ctr="$output"
 
     # Check that, unless we override it, we get that setting in "run".
     run_buildah run $RUNOPTS "$ctr" readlink /proc/self/ns/"$nstype"
-    [ "$output" != "" ]
+    assert "$output" != "" "readlink /proc/self/ns/$nstype must not be empty"
     case "$namespace" in
     ""|container|private)
-      [ "$output" != "$mynamespace" ]
+      assert "$output" != "$mynamespace" \
+             "readlink /proc/self/ns/$nstype, with namespace=$namespace"
       ;;
     host)
       expect_output "$mynamespace"
@@ -321,11 +322,12 @@ general_namespace() {
     if [ "$nsflag" != "userns" ]; then
       for different in ${types[@]} ; do
         # Check that, if we override it, we get what we specify for "run".
-       run_buildah run $RUNOPTS --"$nsflag"=$different "$ctr" readlink /proc/self/ns/"$nstype"
-        [ "$output" != "" ]
+        run_buildah run $RUNOPTS --"$nsflag"=$different "$ctr" readlink /proc/self/ns/"$nstype"
+        assert "$output" != "" "readlink /proc/self/ns/$nstype must not be empty"
         case "$different" in
         ""|container|private)
-          [ "$output" != "$mynamespace" ]
+          assert "$output" != "$mynamespace" \
+                 "readlink /proc/self/ns/$nstype, with different=$different"
           ;;
        host)
           expect_output "$mynamespace"
@@ -346,7 +348,7 @@ _EOF
     result=$(grep -A1 "TargetOutput" <<< "$output" | tail -n1)
     case "$namespace" in
     ""|container|private)
-      [ "$result" != "$mynamespace" ]
+      assert "$result" != "$mynamespace" "readlink /proc/self/ns/$nstype"
       ;;
     host)
       expect_output --from="$result" "$mynamespace"
@@ -422,14 +424,14 @@ _EOF
 
               echo "buildah from $WITH_POLICY_JSON --ipc=$ipc --net=$net --pid=$pid --userns=$userns --uts=$uts --cgroupns=$cgroupns alpine"
               run_buildah from $WITH_POLICY_JSON --quiet --ipc=$ipc --net=$net --pid=$pid --userns=$userns --uts=$uts --cgroupns=$cgroupns alpine
-              [ "$output" != "" ]
+              assert "$output" != "" "output from buildah-from"
               ctr="$output"
               run_buildah run $ctr pwd
-              [ "$output" != "" ]
+              assert "$output" != "" "output from pwd"
               run_buildah run --tty=true  $ctr pwd
-              [ "$output" != "" ]
+              assert "$output" != "" "output from pwd, with --tty=true"
               run_buildah run --terminal=false $ctr pwd
-              [ "$output" != "" ]
+              assert "$output" != "" "output from pwd, with --terminal=false"
             done
           done
         done
@@ -500,7 +502,7 @@ utsns = "$mode"
 EOF
 
     CONTAINERS_CONF="$containers_conf_file" run_buildah from $WITH_POLICY_JSON --quiet alpine
-    [ "$output" != "" ]
+    assert "$output" != "" "output from buildah-from"
     ctr="$output"
 
     local op="=="

diff --git a/tests/overlay.bats b/tests/overlay.bats
@@ -28,7 +28,7 @@ load helpers
 
   # This should fail
   run ls ${TEST_SCRATCH_DIR}/lower/bar
-  [ "$status" -ne 0 ]
+  assert "$status" -ne 0 "status of ls ${TEST_SCRATCH_DIR}/lower/bar"
 }
 
 @test "overlay source permissions and owners" {
@@ -58,7 +58,7 @@ load helpers
 
   # This should fail since /tmp/test was an overlay, not a bind mount
   run ls ${TEST_SCRATCH_DIR}/lower/bar
-  [ "$status" -ne 0 ]
+  assert "$status" -ne 0 "status of ls ${TEST_SCRATCH_DIR}/lower/bar"
 }
 
 @test "overlay path contains colon" {
@@ -92,5 +92,5 @@ load helpers
 
   # This should fail
   run ls ${TEST_SCRATCH_DIR}/a:lower/bar
-  [ "$status" -ne 0 ]
-}
+  assert "$status" -ne 0 "status of ls ${TEST_SCRATCH_DIR}/a:lower/bar"
+}
diff --git a/tests/pull.bats b/tests/pull.bats
@@ -13,7 +13,7 @@ load helpers
   # force a failed pull and look at the error message which *must* include the
   # the resolved image name (localhost/image:latest).
   run_buildah 125 pull --policy=always image
-  [[ "$output" == *"initializing source docker://localhost/image:latest"* ]]
+  assert "$output" =~ "initializing source docker://localhost/image:latest"
   run_buildah rmi localhost/image ${iid}
 }
 
@@ -56,7 +56,7 @@ load helpers
   run_buildah --retry pull --registries-conf ${TEST_SOURCES}/registries.conf $WITH_POLICY_JSON alpine@sha256:e9a2035f9d0d7cee1cdd445f5bfa0c5c646455ee26f14565dce23cf2d2de7570
   run_buildah 125 pull --registries-conf ${TEST_SOURCES}/registries.conf $WITH_POLICY_JSON fakeimage/fortest
   run_buildah images --format "{{.Name}}:{{.Tag}}"
-  [[ ! "$output" =~ "fakeimage/fortest" ]]
+  assert "$output" !~ "fakeimage/fortest" "fakeimage/fortest found in buildah images"
 }
 
 @test "pull-from-docker-archive" {
@@ -100,7 +100,7 @@ load helpers
 
   run docker pull alpine
   echo "$output"
-  [ "$status" -eq 0 ]
+  assert "$status" -eq 0 "status of docker (yes, docker) pull alpine"
   run_buildah pull $WITH_POLICY_JSON docker-daemon:docker.io/library/alpine:latest
   run_buildah images --format "{{.Name}}:{{.Tag}}"
   expect_output --substring "alpine:latest"

diff --git a/tests/rmi.bats b/tests/rmi.bats
@@ -31,7 +31,7 @@ load helpers
   cid3=$output
   run_buildah 125 rmi alpine busybox
   run_buildah images -q
-  [ "$output" != "" ]
+  assert "$output" != "" "images -q"
 
   run_buildah rmi -f alpine busybox
   run_buildah images -q
@@ -66,7 +66,7 @@ load helpers
   cid3=$output
   run_buildah 125 rmi --all
   run_buildah images -q
-  [ "$output" != "" ]
+  assert "$output" != "" "images -q"
 
   run_buildah rmi --all --force
   run_buildah images -q

diff --git a/tests/run.bats b/tests/run.bats
@@ -814,7 +814,7 @@ _EOF
 		found_runtime=y
 		run_buildah '?' run --runtime=runc --runtime-flag=debug $cid true
 		if [ "$status" -eq 0 ]; then
-			[ -n "$output" ]
+			assert "$output" = "" "Output from running 'true'"
 		else
 			# runc fully supports cgroup v2 (unified mode) since v1.0.0-rc93.
 			# older runc doesn't work on cgroup v2.
@@ -825,7 +825,7 @@ _EOF
 	if [ -n "$(command -v crun)" ]; then
 		found_runtime=y
 		run_buildah run --runtime=crun --runtime-flag=debug $cid true
-		[ -n "$output" ]
+		assert "$output" = "" "Output from running 'true'"
 	fi
 
 	if [ -z "${found_runtime}" ]; then

diff --git a/tests/selinux.bats b/tests/selinux.bats
@@ -16,7 +16,7 @@ load helpers
   run_buildah from --quiet --quiet $WITH_POLICY_JSON $image
   cid=$output
   run_buildah run $cid sh -c 'tr \\0 \\n < /proc/self/attr/current'
-  [ "$output" != "" ]
+  assert "$output" != "" "/proc/self/attr/current cannot be empty"
   firstlabel="$output"
 
   # Ensure that we label the same container consistently across multiple "run" instructions.