From e62277eabc31d075d4f5b8f303ee7b3950e41953 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Thu, 13 May 2021 15:08:35 -0600 Subject: [PATCH] WIP: tests: new assert() This is much scarier than I had intended. The intention is to start migrating from 'expect_output' and to use 'assert' instead; the reason is that 'assert' allows much more flexibility, particularly negative tests. We've long wanted something like "assert that output does not contain 'foo'". I've been too lazy to implement it, but last week I noticed code in bud.bats that does: ! expect_output "sdfsdfsdf" This is a really super bad idea: although it works fine when things are good and tests pass, it's a disaster when tests fail because the poor person debugging test output now sees a test failure log, and starts tracking it down, and then much later realizes that it was an intentional failure but the '!' was negating it, and all that track-down work was wasted. To keep this PR reasonable, I'm keeping all positive uses of expect_output untouched. We can migrate those (if desired) over time. All negative uses are now assertions. Signed-off-by: Ed Santiago --- tests/bud.bats | 29 ++++--- tests/config.bats | 6 +- tests/helpers.bash | 171 +++++++++++++++++++++++++++--------------- tests/helpers.bash.t | 87 +++++++++++++++++++++ tests/namespaces.bats | 14 ++-- tests/pull.bats | 8 +- tests/selinux.bats | 5 +- 7 files changed, 224 insertions(+), 96 deletions(-) create mode 100755 tests/helpers.bash.t diff --git a/tests/bud.bats b/tests/bud.bats index 178dba568c..aeb14d5807 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -1510,7 +1510,7 @@ function _test_http() { run_buildah bud --signature-policy ${TESTSDIR}/policy.json -t ${target} --build-arg foo=bar --build-arg foo2=bar2 -f ${TESTSDIR}/bud/build-arg ${TESTSDIR}/bud/build-arg expect_output --substring "one or more build args were not consumed: \[foo2\]" run_buildah bud --signature-policy ${TESTSDIR}/policy.json -t ${target} --build-arg IMAGE=alpine -f ${TESTSDIR}/bud/build-arg/Dockerfile2 ${TESTSDIR}/bud/build-arg - ! expect_output --substring "one or more build args were not consumed: \[IMAGE\]" + assert "$output" !~ "one or more build args were not consumed: \[IMAGE\]" expect_output --substring "FROM alpine" } @@ -1807,7 +1807,7 @@ _EOF run_buildah bud -t testbud --signature-policy ${TESTSDIR}/policy.json ${mytmpdir} expect_output --substring "file1" expect_output --substring "file2" - ! expect_output --substring "file3" + assert "$output" !~ "file3" } @test "bud copy with .dockerignore #2" { @@ -1827,8 +1827,8 @@ RUN find /tmp/stuff -type f _EOF run_buildah bud -t testbud --signature-policy ${TESTSDIR}/policy.json ${mytmpdir} - ! expect_output --substring "file1" - ! expect_output --substring "file2" + assert "$output" !~ file1 + assert "$output" !~ file2 } @test "bud-copy-workdir" { @@ -1869,9 +1869,8 @@ _EOF run_buildah bud --signature-policy ${TESTSDIR}/policy.json --layers -t ${target} -f Dockerfile3 --build-arg=UID=17122 --build-arg=CODE=/copr/coprs_frontend --build-arg=USERNAME=praiskup --build-arg=PGDATA=/pgdata ${TESTSDIR}/bud/build-arg run_buildah inspect -f '{{.FromImageID}}' ${target} argsid="$output" - if [[ "$argsid" == "$initialid" ]]; then - die ".FromImageID of test-img-2 ($argsid) == same as test-img, it should be different" - fi + assert "$argsid" != "$initialid" \ + ".FromImageID of test-img-2 ($argsid) == same as test-img, it should be different" # With build args, even in a different order, we should end up using the previous build as a cached result. run_buildah bud --signature-policy ${TESTSDIR}/policy.json --layers -t ${target} -f Dockerfile3 --build-arg=UID=17122 --build-arg=CODE=/copr/coprs_frontend --build-arg=USERNAME=praiskup --build-arg=PGDATA=/pgdata ${TESTSDIR}/bud/build-arg @@ -1905,18 +1904,16 @@ _EOF run_buildah bud --signature-policy ${TESTSDIR}/policy.json --layers -t test-img-2 --build-arg TEST=foo -f Dockerfile4 ${TESTSDIR}/bud/build-arg run_buildah inspect -f '{{.FromImageID}}' test-img-2 argsid="$output" - if [[ "$argsid" == "$initialid" ]]; then - die ".FromImageID of test-img-2 ($argsid) == same as test-img, it should be different" - fi + assert "$argsid" != "$initialid" \ + ".FromImageID of test-img-2 ($argsid) == same as test-img, it should be different" # Set the build-arg via an ENV in the local environment and verify that the cached layers are not used export TEST=bar run_buildah bud --signature-policy ${TESTSDIR}/policy.json --layers -t test-img-3 --build-arg TEST -f Dockerfile4 ${TESTSDIR}/bud/build-arg run_buildah inspect -f '{{.FromImageID}}' test-img-3 argsid="$output" - if [[ "$argsid" == "$initialid" ]]; then - die ".FromImageID of test-img-3 ($argsid) == same as test-img, it should be different" - fi + assert "$argsid" != "$initialid" \ + ".FromImageID of test-img-3 ($argsid) == same as test-img, it should be different" } @test "bud test RUN with a privileged command" { @@ -2869,12 +2866,12 @@ RUN echo "$SECRET" _EOF run_buildah bud -t testbud --signature-policy ${TESTSDIR}/policy.json --file ${mytmpdir} . - ! expect_output --substring '\-\-build-arg SECRET=' + assert "$output" !~ '--build-arg SECRET=' expect_output --substring '\-\-build-arg NEWSECRET=' run_buildah bud -t testbud --signature-policy ${TESTSDIR}/policy.json --build-arg NEWSECRET="VerySecret" --file ${mytmpdir} . - ! expect_output --substring '\-\-build-arg SECRET=' - ! expect_output --substring '\-\-build-arg NEWSECRET=' + assert "$output" !~ '--build-arg SECRET=' + assert "$output" !~ '--build-arg NEWSECRET=' } @test "bud with --runtime and --runtime-flag" { diff --git a/tests/config.bats b/tests/config.bats index 931b3785c0..7b89bad8fe 100644 --- a/tests/config.bats +++ b/tests/config.bats @@ -23,19 +23,19 @@ load helpers run_buildah config --annotation ANNOTATION $cid run_buildah 125 config --healthcheck 'AB "CD' $cid - expect_output --substring 'error parsing --healthcheck "AB \"CD": invalid command line string' + expect_output --substring 'error parsing --healthcheck "AB \\"CD": invalid command line string' run_buildah 125 config --healthcheck-interval ABCD $cid expect_output --substring 'error parsing --healthcheck-interval "ABCD": time: invalid duration "?ABCD"?' run_buildah 125 config --cmd 'AB "CD' $cid - expect_output --substring 'error parsing --cmd "AB \"CD": invalid command line string' + expect_output --substring 'error parsing --cmd "AB \\"CD": invalid command line string' run_buildah 125 config --env ENV $cid expect_output --substring 'error setting env "ENV": no value given' run_buildah 125 config --shell 'AB "CD' $cid - expect_output --substring 'error parsing --shell "AB \"CD": invalid command line string' + expect_output --substring 'error parsing --shell "AB \\"CD": invalid command line string' } function check_matrix() { diff --git a/tests/helpers.bash b/tests/helpers.bash index 11deb3672d..689b3995e0 100644 --- a/tests/helpers.bash +++ b/tests/helpers.bash @@ -19,17 +19,23 @@ export GPG_TTY=/dev/null function setup() { pushd "$(dirname "$(readlink -f "$BASH_SOURCE")")" - suffix=$(dd if=/dev/urandom bs=12 count=1 status=none | od -An -tx1 | sed -e 's, ,,g') - TESTDIR=${BATS_TMPDIR}/tmp${suffix} - rm -fr ${TESTDIR} - mkdir -p ${TESTDIR}/{root,runroot,sigstore,registries.d,cache} - echo "default-docker: " >> ${TESTDIR}/registries.d/default.yaml - echo " sigstore-staging: file://${TESTDIR}/sigstore " >> ${TESTDIR}/registries.d/default.yaml - echo "docker: " >> ${TESTDIR}/registries.d/default.yaml - echo " registry.access.redhat.com: " >> ${TESTDIR}/registries.d/default.yaml - echo " sigstore: https://access.redhat.com/webassets/docker/content/sigstore " >> ${TESTDIR}/registries.d/default.yaml - echo " registry.redhat.io: " >> ${TESTDIR}/registries.d/default.yaml - echo " sigstore: https://registry.redhat.io/containers/sigstore " >> ${TESTDIR}/registries.d/default.yaml + # buildah/podman: "repository name must be lowercase". + # me: "but it's a local file path, not a repository name!" + # buildah/podman: "i dont care. no caps anywhere!" + TESTDIR=$(mktemp -d --dry-run --tmpdir=${BATS_TMPDIR:-${TMPDIR:-/tmp}} buildah_tests.XXXXXX | tr A-Z a-z) + mkdir --mode=0700 $TESTDIR + + mkdir -p ${TESTDIR}/{root,runroot,sigstore,registries.d} + cat >${TESTDIR}/registries.d/default.yaml <&2 + printf "#| FAIL: %s\n" "$testname" >&2 + printf "#| expected: %s'%s'\n" "$op" "$expect_string" >&2 + printf "#| actual: %s'%s'\n" "$ws" "${actual_split[0]}" >&2 + local line + for line in "${actual_split[@]:1}"; do + printf "#| > %s'%s'\n" "$ws" "$line" >&2 + done + printf "#\\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" >&2 + false +} + +################### +# expect_output # [obsolete; kept for compatibility] +################### +# +# An earlier version of assert(). # function expect_output() { # By default we examine $output, the result of run_buildah local actual="$output" - local check_substring= + local operator='==' # option processing: recognize --from="...", --substring local opt @@ -256,43 +338,14 @@ function expect_output() { local value=$(expr "$opt" : '[^=]*=\(.*\)') case "$opt" in --from=*) actual="$value"; shift;; - --substring) check_substring=1; shift;; + --substring) operator='=~'; shift;; --) shift; break;; -*) die "Invalid option '$opt'" ;; *) break;; esac done - local expect="$1" - local testname="${2:-${MOST_RECENT_BUILDAH_COMMAND:-[no test name given]}}" - - if [ -z "$expect" ]; then - if [ -z "$actual" ]; then - return - fi - expect='[no output]' - elif [ "$actual" = "$expect" ]; then - return - elif [ -n "$check_substring" ]; then - if [[ "$actual" =~ $expect ]]; then - return - fi - fi - - # This is a multi-line message, which may in turn contain multi-line - # output, so let's format it ourselves, readably - local -a actual_split - readarray -t actual_split <<<"$actual" - printf "#/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv\n" >&2 - printf "#| FAIL: %s\n" "$testname" >&2 - printf "#| expected: '%s'\n" "$expect" >&2 - printf "#| actual: '%s'\n" "${actual_split[0]}" >&2 - local line - for line in "${actual_split[@]:1}"; do - printf "#| > '%s'\n" "$line" >&2 - done - printf "#\\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" >&2 - false + assert "$actual" "$operator" "$@" } ####################### diff --git a/tests/helpers.bash.t b/tests/helpers.bash.t new file mode 100755 index 0000000000..ec49519978 --- /dev/null +++ b/tests/helpers.bash.t @@ -0,0 +1,87 @@ +#!/bin/bash +# +# tests for helpers.bash +# + +. $(dirname ${BASH_SOURCE})/helpers.bash + +INDEX=1 +RC=0 + +# t (true) : tests that should pass +function t() { + result=$(assert "$@" 2>&1) + status=$? + + if [[ $status -eq 0 ]]; then + echo "ok $INDEX $*" + else + echo "not ok $INDEX $*" + echo "$result" + RC=1 + fi + + INDEX=$((INDEX + 1)) +} + +# f (false) : tests that should fail +function f() { + result=$(assert "$@" 2>&1) + status=$? + + if [[ $status -ne 0 ]]; then + echo "ok $INDEX ! $*" + else + echo "not ok $INDEX ! $* [passed, should have failed]" + RC=1 + fi + + INDEX=$((INDEX + 1)) +} + + + +t "" = "" +t "a" != "" +t "" != "a" + +t "a" = "a" +t "aa" == "aa" +t "a[b]{c}" = "a[b]{c}" + +t "abcde" =~ "a" +t "abcde" =~ "b" +t "abcde" =~ "c" +t "abcde" =~ "d" +t "abcde" =~ "e" +t "abcde" =~ "ab" +t "abcde" =~ "abc" +t "abcde" =~ "abcd" +t "abcde" =~ "bcde" +t "abcde" =~ "cde" +t "abcde" =~ "de" + +t "foo" =~ "foo" +t "foobar" =~ "foo" +t "barfoo" =~ "foo" + +t 'a "AB \"CD": ef' = 'a "AB \"CD": ef' +t 'a "AB \"CD": ef' =~ 'a "AB \\"CD": ef' + +t 'abcdef' !~ 'efg' +t 'abcdef' !~ 'x' + +########### + +f "a" = "b" +f "a" == "b" + +f "abcde" =~ "x" + +f "abcde" !~ "a" +f "abcde" !~ "ab" +f "abcde" !~ "abc" + +f "" != "" + +exit $RC diff --git a/tests/namespaces.bats b/tests/namespaces.bats index 907ef4f899..8bfd66d861 100644 --- a/tests/namespaces.bats +++ b/tests/namespaces.bats @@ -44,9 +44,7 @@ load helpers # Check that with settings that require a user namespace, we also get a new network namespace by default. run_buildah run $RUNOPTS "$ctr" readlink /proc/self/ns/net - if [[ $output == $mynetns ]]; then - expect_output "[output should not be '$mynetns']" - fi + assert "$output" != "$mynetns" "we should get a new network namespace" # Check that with settings that require a user namespace, we can still try to use the host's network namespace. run_buildah run $RUNOPTS --net=host "$ctr" readlink /proc/self/ns/net @@ -62,14 +60,12 @@ load helpers # Check that with settings that don't require a user namespace, we can request to use a per-container network namespace. run_buildah run $RUNOPTS --net=container "$ctr" readlink /proc/self/ns/net - if [[ $output == $mynetns ]]; then - die "[/proc/self/ns/net (--net=container) should not be '$mynetns']" - fi + assert "$output" != "$mynetns" \ + "[/proc/self/ns/net (--net=container) should not be '$mynetns']" run_buildah run $RUNOPTS --net=private "$ctr" readlink /proc/self/ns/net - if [[ $output == $mynetns ]]; then - die "[/proc/self/ns/net (--net=private) should not be '$mynetns']" - fi + assert "$output" != "$mynetns" \ + "[/proc/self/ns/net (--net=private) should not be '$mynetns']" } # Helper for idmapping test: check UID or GID mapping diff --git a/tests/pull.bats b/tests/pull.bats index 8b667bde8b..d732a25786 100644 --- a/tests/pull.bats +++ b/tests/pull.bats @@ -298,9 +298,7 @@ load helpers # Pull image by default should change the image id run_buildah pull -q --policy always --signature-policy ${TESTSDIR}/policy.json alpine - if [[ $output == $iid ]]; then - expect_output "[output should not be '$iid']" - fi + assert "$output" != "$iid" "pulled image should have a new IID" # Recreate image run_buildah commit -q $cid docker.io/library/alpine @@ -359,7 +357,5 @@ load helpers run_buildah pull -q --signature-policy ${TESTSDIR}/policy.json --policy missing --arch arm64 alpine armiid=$output - if [[ $amdiid == $armiid ]]; then - expect_output "[different arch images were not pulled]" - fi + assert "$amdiid" != "$armiid" "AMD and ARM ids should differ" } diff --git a/tests/selinux.bats b/tests/selinux.bats index 39ec62cec1..cfb858cb3a 100644 --- a/tests/selinux.bats +++ b/tests/selinux.bats @@ -27,9 +27,8 @@ load helpers run_buildah from --quiet --quiet --signature-policy ${TESTSDIR}/policy.json $image cid1=$output run_buildah run $cid1 sh -c 'tr \\0 \\n < /proc/self/attr/current' - if [ "$output" = "$firstlabel" ]; then - die "Second container has the same label as first (both '$output')" - fi + assert "$output" != "$firstlabel" \ + "Second container has the same label as first (both '$output')" } @test "selinux spc" {