System tests: tighten 'is' operator

Fix day-one sloppiness: when I first wrote this framework
it compared strings using 'expr', not '=', to be more
forgiving of extra cruft in output. This was a bad decision.
It means that warnings or additional text are ignored:

    is "all is ok, NOT!"  "all is ok"  <-- this would pass

Solution: tighten up the 'is' check. Use '=' (direct
compare) first. If it fails, look for wild cards ('*')
or character classes ('[') in the expect string. If
so, and only then, use 'expr'. And, thanks to a clever
suggestion from Luap99, include '(using expr)' in the
error message when we do so; this could make it easier
for a developer to understand a string mismatch.

This change exposes a lot of instances in which we weren't
doing proper comparisons. Fix those. Thankfully, there
weren't as many as I'd feared.

Also, and completely unrelated, add '-T' flag to bats
helper, for showing timing results. (I will open this
as a separate PR if requested. I too find it offensive
to jumble together unrelated commits.)

Signed-off-by: Ed Santiago <santiago@redhat.com>

16 files changed, 75 insertions, 40 deletions

diff --git a/hack/bats b/hack/bats
index 45b8cf6f2..7cc3b9bde 100755
--- a/hack/bats
+++ b/hack/bats
@@ -19,6 +19,8 @@ $0 is a wrapper for invoking podman system tests.
                   version of bats installed, runs with '--filter pattern'
                   which runs only subtests that match 'pattern'
 
+   -T             Passed on to bats, which will then show timing data
+
    --help         display usage message
 
 By default, tests ./bin/podman. To test a different podman, do:
@@ -60,6 +62,8 @@ REMOTE=
 ROOT_ONLY=
 ROOTLESS_ONLY=
 
+declare -a bats_opts=()
+
 declare -a bats_filter=()
 
 for i;do
@@ -69,6 +73,7 @@ for i;do
         --root)     ROOT_ONLY=1 ;;
         --rootless) ROOTLESS_ONLY=1 ;;
         --remote)   REMOTE=remote; echo "--remote is TBI"; exit 1;;
+        --ts|-T)    bats_opts+=("-T") ;;
         */*.bats)   TESTS=$i ;;
         *)
             if [[ $i =~ : ]]; then
@@ -94,7 +99,7 @@ if [ -z "$ROOTLESS_ONLY" ]; then
     sudo    --preserve-env=PODMAN \
             --preserve-env=PODMAN_TEST_DEBUG \
             --preserve-env=OCI_RUNTIME \
-            bats "${bats_filter[@]}" $TESTS
+            bats "${bats_opts[@]}" "${bats_filter[@]}" $TESTS
     rc=$?
 fi
 
@@ -102,7 +107,7 @@ fi
 echo "--------------------------------------------------"
 if [ -z "$ROOT_ONLY" ]; then
     echo "\$ bats ${bats_filter[@]} $TESTS"
-    bats "${bats_filter[@]}" $TESTS
+    bats "${bats_opts[@]}" "${bats_filter[@]}" $TESTS
     rc=$((rc | $?))
 fi
 
diff --git a/test/system/001-basic.bats b/test/system/001-basic.bats
index 888c075b8..2e5ebe4a3 100644
--- a/test/system/001-basic.bats
+++ b/test/system/001-basic.bats
@@ -101,7 +101,9 @@ function setup() {
     fi
 
     run_podman 125 --remote
-    is "$output" "Error: missing command 'podman COMMAND'" "podman remote show usage message without running endpoint"
+    is "$output" "Error: missing command 'podman COMMAND'
+Try 'podman --help' for more information." \
+       "podman --remote show usage message without running endpoint"
 }
 
 # This is for development only; it's intended to make sure our timeout
diff --git a/test/system/005-info.bats b/test/system/005-info.bats
index 0ea0f8356..045045fa6 100644
--- a/test/system/005-info.bats
+++ b/test/system/005-info.bats
@@ -37,7 +37,7 @@ cgroupVersion: v[12]
     # FIXME: if we're ever able to get package versions on Debian,
     #        add '-[0-9]' to all '*.package' queries below.
     tests="
-host.buildahVersion       | [0-9.]
+host.buildahVersion       | [1-9][0-9]*\.[0-9.]\\\+.*
 host.conmon.path          | $expr_path
 host.conmon.package       | .*conmon.*
 host.cgroupManager        | \\\(systemd\\\|cgroupfs\\\)
diff --git a/test/system/010-images.bats b/test/system/010-images.bats
index 649987dfd..1e9d5f181 100644
--- a/test/system/010-images.bats
+++ b/test/system/010-images.bats
@@ -19,8 +19,8 @@ load helpers
 
 @test "podman images - custom formats" {
     tests="
-{{.ID}}                  |        [0-9a-f]\\\{12\\\}
-{{.ID| upper}}           |        [0-9A-F]\\\{12\\\}
+{{.ID}}                  |        [0-9a-f]\\\{12\\\}\\\$
+{{.ID| upper}}           |        [0-9A-F]\\\{12\\\}\\\$
 {{.Repository}}:{{.Tag}} | $PODMAN_TEST_IMAGE_FQN
 {{.Labels.created_by}}   | test/system/build-testimage
 {{.Labels.created_at}}   | 20[0-9-]\\\+T[0-9:]\\\+Z
@@ -28,7 +28,7 @@ load helpers
 
     parse_table "$tests" | while read fmt expect; do
         run_podman images --format "$fmt"
-        is "$output" "$expect\$" "podman images $fmt"
+        is "$output" "$expect" "podman images --format '$fmt'"
     done
 
     run_podman images --format "{{.ID}}" --no-trunc
diff --git a/test/system/030-run.bats b/test/system/030-run.bats
index 3d9d834b3..4080f08b4 100644
--- a/test/system/030-run.bats
+++ b/test/system/030-run.bats
@@ -517,7 +517,8 @@ json-file | f
     cid="$output"
 
     run_podman inspect --format "{{.ImageName}}" $cid
-    is "$output" "$newtag" "container .ImageName is the container-create name"
+    is "$output" "$newtag:latest" \
+       "container .ImageName is the container-create name"
 
     # Same thing, but now with a :tag, and making sure it works with --name
     newtag2="${newtag}:$(random_string 6|tr A-Z a-z)"
@@ -526,7 +527,8 @@ json-file | f
     cname="$(random_string 14|tr A-Z a-z)"
     run_podman create --name $cname $newtag2
     run_podman inspect --format "{{.ImageName}}" $cname
-    is "$output" "$newtag2" "container .ImageName is the container-create name"
+    is "$output" "$newtag2" \
+       "container .ImageName is the container-create name, with :tag"
 
     # Clean up.
     run_podman rm $cid $cname
@@ -718,7 +720,7 @@ EOF
     run_podman 125 run --device-cgroup-rule="b 7:2" --rm $IMAGE
     is "$output" 'Error: invalid device cgroup rule requires type, major:Minor, and access rules: "b 7:2"'
     run_podman 125 run --device-cgroup-rule="x 7:* rmw" --rm $IMAGE
-    is "$output" "Error: invalid device type in device-access-add:"
+    is "$output" "Error: invalid device type in device-access-add: x"
     run_podman 125 run --device-cgroup-rule="a a:* rmw" --rm $IMAGE
     is "$output" "Error: strconv.ParseInt: parsing \"a\": invalid syntax"
 }
diff --git a/test/system/075-exec.bats b/test/system/075-exec.bats
index b7367d153..de767a1e8 100644
--- a/test/system/075-exec.bats
+++ b/test/system/075-exec.bats
@@ -77,7 +77,7 @@ load helpers
     is "${lines[1]}" "3000+0 records out" "dd: number of records out"
     # Verify sha. '% *' strips off the path, keeping only the SHA
     run_podman exec $cid sha512sum /tmp/bigfile
-    is "${output% *}" "$expect" "SHA of file in container"
+    is "${output% *}" "$expect " "SHA of file in container"
 
     # Clean up
     run_podman exec $cid touch /stop
diff --git a/test/system/120-load.bats b/test/system/120-load.bats
index 8be9ed5c5..e9959271f 100644
--- a/test/system/120-load.bats
+++ b/test/system/120-load.bats
@@ -11,7 +11,7 @@ load helpers
 # initialize, read image ID and name
 get_iid_and_name() {
     run_podman images -a --format '{{.ID}} {{.Repository}}:{{.Tag}}'
-    read iid img_name < <(echo "$output")
+    read iid img_name <<<"$output"
 
     archive=$PODMAN_TMPDIR/myimage-$(random_string 8).tar
 }
@@ -62,7 +62,7 @@ verify_iid_and_name() {
 
     # FIXME: cannot compare IID, see #7371, so we check only the tag
     run_podman images $fqin --format '{{.Repository}}:{{.Tag}}'
-    is "$output" "$fqin" "image preserves name across save/load"
+    is "${lines[0]}" "$fqin" "image preserves name across save/load"
 
     # Load with a new tag
     local new_name=x1$(random_string 14 | tr A-Z a-z)
diff --git a/test/system/125-import.bats b/test/system/125-import.bats
index 5995d71bf..5b8d84a2f 100644
--- a/test/system/125-import.bats
+++ b/test/system/125-import.bats
@@ -20,26 +20,26 @@ load helpers
     # Simple import
     run_podman import -q $archive
     iid="$output"
-    run_podman run -t --rm $iid cat /random.txt
+    run_podman run --rm $iid cat /random.txt
     is "$output" "$random_content" "simple import"
     run_podman rmi -f $iid
 
     # Simple import via stdin
     run_podman import -q - < <(cat $archive)
     iid="$output"
-    run_podman run -t --rm $iid cat /random.txt
+    run_podman run --rm $iid cat /random.txt
     is "$output" "$random_content" "simple import via stdin"
     run_podman rmi -f $iid
 
     # Tagged import
     run_podman import -q $archive $fqin
-    run_podman run -t --rm $fqin cat /random.txt
+    run_podman run --rm $fqin cat /random.txt
     is "$output" "$random_content" "tagged import"
     run_podman rmi -f $fqin
 
     # Tagged import via stdin
     run_podman import -q - $fqin < <(cat $archive)
-    run_podman run -t --rm $fqin cat /random.txt
+    run_podman run --rm $fqin cat /random.txt
     is "$output" "$random_content" "tagged import via stdin"
     run_podman rmi -f $fqin
 }
@@ -100,7 +100,7 @@ EOF
 
     # Confirm exit within timeout
     run_podman ps -a --filter name=$a_cnt --format '{{.Status}}'
-    is "$output" "Exited (33)" "Exit by non-TERM/KILL"
+    is "$output" "Exited (33) .*" "Exit by non-TERM/KILL"
 
     run_podman rm -f $a_cnt
     run_podman rmi $b_img $a_img
diff --git a/test/system/150-login.bats b/test/system/150-login.bats
index ed925044c..c003a0409 100644
--- a/test/system/150-login.bats
+++ b/test/system/150-login.bats
@@ -245,7 +245,7 @@ function _test_skopeo_credential_sharing() {
     is "$status" "0" "skopeo inspect - exit status"
 
     got_name=$(jq -r .Name <<<"$output")
-    is "$got_name" "$registry/$dest_name" "skopeo inspect -> Name"
+    is "$got_name" "$registry/$destname" "skopeo inspect -> Name"
 
     # Now try without a valid login; it should fail
     run_podman logout "$@" $registry
diff --git a/test/system/160-volumes.bats b/test/system/160-volumes.bats
index e21be9ea4..b48198a18 100644
--- a/test/system/160-volumes.bats
+++ b/test/system/160-volumes.bats
@@ -261,7 +261,8 @@ EOF
 
     # prune should remove v4
     run_podman volume prune --force
-    is "$output" "${v[4]}" "volume prune, with 1, 2, 3 in use, deletes only 4"
+    is "$(echo $(sort <<<$output))" "${v[4]} ${v[5]} ${v[6]}" \
+       "volume prune, with 1, 2, 3 in use, deletes only 4, 5, 6"
 
     # Remove the container using v2 and v3. Prune should now remove those.
     # The 'echo sort' is to get the output sorted and in one line.
diff --git a/test/system/330-corrupt-images.bats b/test/system/330-corrupt-images.bats
index 86da06cb0..7f2b81835 100644
--- a/test/system/330-corrupt-images.bats
+++ b/test/system/330-corrupt-images.bats
@@ -74,7 +74,7 @@ function _corrupt_image_test() {
         # Corruptify, and confirm that 'podman images' throws an error
         rm -v ${PODMAN_CORRUPT_TEST_WORKDIR}/root/*-images/$id/${rm_path}
         run_podman 125 images
-        is "$output" "Error: error retrieving label for image \"$id\": you may need to remove the image to resolve the error"
+        is "$output" "Error: error retrieving label for image \"$id\": you may need to remove the image to resolve the error.*"
 
         # Run the requested command. Confirm it succeeds, with suitable warnings
         run_podman $*
diff --git a/test/system/410-selinux.bats b/test/system/410-selinux.bats
index 0f7c35c65..7f7f23000 100644
--- a/test/system/410-selinux.bats
+++ b/test/system/410-selinux.bats
@@ -29,7 +29,7 @@ function check_label() {
     if [ -n "$2" ]; then
         # e.g. from the above example -> "s0:c45,c745"
         range=$(cut -d: -f4,5 <<<"$context")
-        is "$range" "$2" "SELinux range"
+        is "$range" "$2^@" "SELinux range"
     fi
 }
 
@@ -101,7 +101,7 @@ function check_label() {
                --security-opt label=level:s0 \
                $IMAGE sh -c 'while test ! -e /stop; do sleep 0.1; done'
     run_podman inspect --format='{{ .HostConfig.SecurityOpt }}' myc
-    is "$output" "\[label=type:spc_t,label=level:s0 seccomp=unconfined]" \
+    is "$output" "[label=type:spc_t,label=level:s0 seccomp=unconfined]" \
       "'podman inspect' preserves all --security-opts"
 
     run_podman exec myc touch /stop
@@ -117,6 +117,10 @@ function check_label() {
     # must use a pid namespace and not join an existing one.
     skip_if_rootless_cgroupsv1
 
+    if [[ $(podman_runtime) == "runc" ]]; then
+        skip "some sort of runc bug, not worth fixing (#11784)"
+    fi
+
     run_podman run -d --name myctr $IMAGE top
     run_podman exec myctr cat -v /proc/self/attr/current
     context_c1="$output"
@@ -225,24 +229,25 @@ function check_label() {
 
     run_podman run -v $tmpdir:/test $IMAGE cat /proc/self/attr/current
     run ls -dZ ${tmpdir}
-    is "$output" ${LABEL} "No Relabel Correctly"
+    is "$output" "${LABEL} ${tmpdir}" "No Relabel Correctly"
 
     run_podman run -v $tmpdir:/test:z --security-opt label=disable $IMAGE cat /proc/self/attr/current
     run ls -dZ $tmpdir
-    is "$output" ${RELABEL} "Privileged Relabel Correctly"
+    is "$output" "${RELABEL} $tmpdir" "Privileged Relabel Correctly"
 
     run_podman run -v $tmpdir:/test:z --privileged $IMAGE cat /proc/self/attr/current
     run ls -dZ $tmpdir
-    is "$output" ${RELABEL} "Privileged Relabel Correctly"
+    is "$output" "${RELABEL} $tmpdir" "Privileged Relabel Correctly"
 
     run_podman run -v $tmpdir:/test:Z $IMAGE cat /proc/self/attr/current
     level=$(secon -l $output)
     run ls -dZ $tmpdir
-    is "$output" "system_u:object_r:container_file_t:$level" "Confined Relabel Correctly"
+    is "$output" "system_u:object_r:container_file_t:$level $tmpdir" \
+       "Confined Relabel Correctly"
 
     run_podman run -v $tmpdir:/test:z $IMAGE cat /proc/self/attr/current
     run ls -dZ $tmpdir
-    is "$output" ${RELABEL} "Shared Relabel Correctly"
+    is "$output" "${RELABEL} $tmpdir" "Shared Relabel Correctly"
 }
 
 # vim: filetype=sh
diff --git a/test/system/450-interactive.bats b/test/system/450-interactive.bats
index 47bdff9ab..55c2afcd4 100644
--- a/test/system/450-interactive.bats
+++ b/test/system/450-interactive.bats
@@ -55,9 +55,11 @@ function teardown() {
     cols=$(( 15 + RANDOM % 60 & 126 ))
     stty rows $rows cols $cols <$PODMAN_TEST_PTY
 
+    CR=$'\r'
+
     # ...and make sure stty under podman reads that.
     run_podman run -it --name mystty $IMAGE stty size <$PODMAN_TEST_PTY
-    is "$output" "$rows $cols" "stty under podman run reads the correct dimensions"
+    is "$output" "$rows $cols$CR" "stty under podman run reads the correct dimensions"
 
     run_podman rm -f mystty
 
@@ -75,7 +77,7 @@ function teardown() {
 @test "podman load - will not read from tty" {
     run_podman 125 load <$PODMAN_TEST_PTY
     is "$output" \
-       "Error: cannot read from terminal. Use command-line redirection" \
+       "Error: cannot read from terminal. Use command-line redirection or the --input flag." \
        "Diagnostic from 'podman load' without redirection or -i"
 }
 
@@ -84,14 +86,15 @@ function teardown() {
     run_podman run --tty -i --rm $IMAGE echo hello < /dev/null
     is "$output" ".*The input device is not a TTY.*" "-it _without_ a tty"
 
+    CR=$'\r'
     run_podman run --tty -i --rm $IMAGE echo hello <$PODMAN_TEST_PTY
-    is "$output" "hello" "-it _with_ a pty"
+    is "$output" "hello$CR" "-it _with_ a pty"
 
     run_podman run --tty=false -i --rm $IMAGE echo hello < /dev/null
     is "$output" "hello" "-tty=false: no warning"
 
     run_podman run --tty -i=false --rm $IMAGE echo hello < /dev/null
-    is "$output" "hello" "-i=false: no warning"
+    is "$output" "hello$CR" "-i=false: no warning"
 }
 
 # vim: filetype=sh
diff --git a/test/system/500-networking.bats b/test/system/500-networking.bats
index 548f2d764..29fa309f3 100644
--- a/test/system/500-networking.bats
+++ b/test/system/500-networking.bats
@@ -417,7 +417,7 @@ load helpers
 
     # check network alias for container short id
     run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname\").Aliases}}"
-    is "$output" "\[${cid:0:12}\]" "short container id in network aliases"
+    is "$output" "[${cid:0:12}]" "short container id in network aliases"
 
     run_podman network disconnect $netname $cid
 
@@ -449,7 +449,7 @@ load helpers
 
     # check network2 alias for container short id
     run_podman inspect $cid --format "{{(index .NetworkSettings.Networks \"$netname2\").Aliases}}"
-    is "$output" "\[${cid:0:12}\]" "short container id in network aliases"
+    is "$output" "[${cid:0:12}]" "short container id in network aliases"
 
     # curl should work
     run curl --max-time 3 -s $SERVER/index.txt
diff --git a/test/system/700-play.bats b/test/system/700-play.bats
index 0785bffdf..f41c50e4c 100644
--- a/test/system/700-play.bats
+++ b/test/system/700-play.bats
@@ -30,7 +30,7 @@ spec:
   containers:
   - command:
     - sleep
-    - "100"
+    - \"100\"
     env:
     - name: PATH
       value: /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
@@ -49,7 +49,7 @@ spec:
       capabilities: {}
       privileged: false
       seLinuxOptions:
-         level: "s0:c1,c2"
+         level: \"s0:c1,c2\"
       readOnlyRootFilesystem: false
     volumeMounts:
     - mountPath: /testdir:z
@@ -73,7 +73,7 @@ RELABEL="system_u:object_r:container_file_t:s0"
     run_podman play kube - < $PODMAN_TMPDIR/test.yaml
     if [ -e /usr/sbin/selinuxenabled -a /usr/sbin/selinuxenabled ]; then
        run ls -Zd $TESTDIR
-       is "$output" ${RELABEL} "selinux relabel should have happened"
+       is "$output" "${RELABEL} $TESTDIR" "selinux relabel should have happened"
     fi
 
     run_podman stop -a -t 0
@@ -88,7 +88,7 @@ RELABEL="system_u:object_r:container_file_t:s0"
     run_podman play kube $PODMAN_TMPDIR/test.yaml
     if [ -e /usr/sbin/selinuxenabled -a /usr/sbin/selinuxenabled ]; then
        run ls -Zd $TESTDIR
-       is "$output" ${RELABEL} "selinux relabel should have happened"
+       is "$output" "${RELABEL} $TESTDIR" "selinux relabel should have happened"
     fi
 
     run_podman stop -a -t 0
diff --git a/test/system/helpers.bash b/test/system/helpers.bash
index 666735b0c..03e1ab82b 100644
--- a/test/system/helpers.bash
+++ b/test/system/helpers.bash
@@ -478,13 +478,30 @@ function is() {
     local expect="$2"
     local testname="${3:-${MOST_RECENT_PODMAN_COMMAND:-[no test name given]}}"
 
+    local is_expr=
     if [ -z "$expect" ]; then
         if [ -z "$actual" ]; then
+            # Both strings are empty.
             return
         fi
         expect='[no output]'
-    elif expr "$actual" : "$expect" >/dev/null; then
+    elif [[ "$actual" = "$expect" ]]; then
+        # Strings are identical.
         return
+    else
+        # Strings are not identical. Are there wild cards in our expect string?
+        if expr "$expect" : ".*[^\\][\*\[]" >/dev/null; then
+            # There is a '[' or '*' without a preceding backslash.
+            is_expr=' (using expr)'
+        elif [[ "${expect:0:1}" = '[' ]]; then
+            # String starts with '[', e.g. checking seconds like '[345]'
+            is_expr=' (using expr)'
+        fi
+        if [[ -n "$is_expr" ]]; then
+            if expr "$actual" : "$expect" >/dev/null; then
+                return
+            fi
+        fi
     fi
 
     # This is a multi-line message, which may in turn contain multi-line
@@ -493,7 +510,7 @@ function is() {
     readarray -t actual_split <<<"$actual"
     printf "#/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv\n" >&2
     printf "#|     FAIL: $testname\n"                          >&2
-    printf "#| expected: '%s'\n" "$expect"                     >&2
+    printf "#| expected: '%s'%s\n" "$expect" "$is_expr"        >&2
     printf "#|   actual: '%s'\n" "${actual_split[0]}"          >&2
     local line
     for line in "${actual_split[@]:1}"; do