buildah-bud tests: simplify

Experience this week has shown that managing .diff files
is too difficult for humans, and too fragile. Opportunities
for errors abound. So, let's try to minimize the diffs.

We can't eliminate the diffs to helpers.bash: those are
true code changes that are absolutely required for running
tests using podman instead of buildah. We need to carry
those ourselves: they are not appropriate for the buildah
repo itself.

What we can do is simplify the patching of bud.bats. That
is fragile, because bud.bats changes often, and context-
sensitive git patch files can easily get confused.

Recognizing that the changes to bud.bats fall under two types:

  - tests that are skipped
  - tests in which podman error messages differ from buildah's

...we now have a new script, apply-podman-deltas, which
is (I hope) much user-friendlier. It understands two directives:

  errmsg - alter the expected error message
  skip   - skip a test

Both operate based on a bats test name. The test name must
match exactly. These directives use 'sed' to update bud.bats.
If any directive fails, the script will keep going (so you
get as many errors as possible in a run), then exits failure.

Instructions (README.md) now explain the process for dealing
with all expected test failures.

(Sneak checkin: add '--filter=NAME' option to test runner,
allowing for targeted and much shorter test runs).

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

5 files changed, 243 insertions, 149 deletions

diff --git a/test/buildah-bud/README.md b/test/buildah-bud/README.md
index 88e4bbc3c..ba776c4d9 100644
--- a/test/buildah-bud/README.md
+++ b/test/buildah-bud/README.md
@@ -1,5 +1,4 @@
-buildah-bud tests under podman
-==============================
+# buildah-bud tests under podman
 
 This directory contains tools for running 'buildah bud' tests
 under podman. The key concept of the workflow is:
@@ -13,8 +12,7 @@ It's a teeny bit more complicated than that, but that's really most of
 what you need to know for most purposes. The tests run in podman CI,
 and for the most part are expected to just pass.
 
-Troubleshooting
----------------
+## Troubleshooting
 
 If you're reading this, it's probably because something went wrong.
 At the time of this writing (March 2021, initial commit) it is
@@ -26,8 +24,7 @@ my prediction is that they will fit one of two categories:
 
 Let's examine those in reverse order:
 
-Failure when not vendoring
---------------------------
+## Failure when not vendoring
 
 Aside from flakes, my only guess here is that you broke 'podman build'.
 If this is the case, it is very likely that you are aware of what you
@@ -41,13 +38,12 @@ If neither of those is the case, then I'm sorry, you're on your own.
 When you figure it out, please remember to update these instructions.
 
 
-Failure when vendoring new buildah
-----------------------------------
+## Failure when vendoring new buildah
 
 This is what I predict will be the usual case; and I predict that
 failures will fall into one of two bins:
 
-* failure to apply the patch
+* failure to apply the patches; and/or
 * failure because there are new buildah tests for functionality not in podman
 
 In either case, the process for solving is the same:
@@ -59,24 +55,52 @@ Presumably, something will fail here. Whatever the failure, your next step is:
 
 * `cd test-buildah-v<TAB>` (this is a new directory created by the script)
 
-If the failure was in `git am`, solve it (left as exercise for the reader).
+Now there are three possible failures:
 
-If the failure was in tests run, solve it (either by adding `skip`s to
-failing tests in bud.bats, or less preferably, by making other tweaks
-to the test code).
+### Failure in `git am`
 
-You now have modified files. THOSE SHOULD ONLY BE test/bud.bats or
-test/helpers.bash! If you changed any other file, that is a sign that
-something is very wrong!
+If the failure was in `git am`, it probably means that buildah
+`tests/helpers.bash` got updated in such a way as to cause a conflict
+with the patches we apply. Your best bet is to:
 
-Commit your changes: `git commit --all --amend`
+* Look at `tests/*.rej`
+* For each rejected patch, try to figure out where it should go and how to apply it. Do so.
+* `git add tests/helpers.bash` - this is for `git am`, next
+* `git am --continue` - this continues the failed patch. Make sure it succeeds.
+* `./make-new-buildah-diffs` - this updates your podman working directory
+* `cd ..; git diff test/buildah-bud`. This will show you a diff of a .diff file, which is really painful to read. I'm sorry. Just try to confirm that the changes look like what you expect.
 
-Push those changes to the podman repo: `./make-new-buildah-diffs`
+Proceed with 'In all cases' below.
 
-cd back up to the podman repo
+### Failure when applying podman-custom deltas
 
-As necessary, rerun `run-buildah-bud-tests`. You can use `--no-checkout`
-to run tests immediately, without rerunning the git checkout.
+Failure in the `apply-podman-deltas` script means that one of the
+hand-crafted exceptions was not found, e.g., there's a `skip` or
+`errmsg` looking for a specific `@test` in `bud.bats` that is
+no longer there.
 
-If you're happy with the diffs, `git add` the modified `.diff` file
-and submit it as part of your PR.
+Solution:
+* Inspect the error message(s) from `apply-podman-deltas`. Each message will list a specific `@test` name.
+* Look at the diffs in `tests/bud.bats` between master and your PR. (I'm really sorry; there's no quick easy command-line way to do that. You will need a checked-out buildah tree, and you will need to know the old and new buildah tags).
+  * In those diffs, look for changes related to each `@test` listed as an error. For example, a test being renamed or even removed.
+  * Update `test/buildah-bud/apply-podman-deltas` accordingly.
+
+Proceed with 'In all cases' below.
+
+### Failure when running tests
+
+If the failure was in tests run, and you're vendoring, your only real choice is to add a new `skip`:
+
+* Identify the failing test(s)
+* File a new podman issue, e.g. "podman build fails buildah XYZ test"
+* Edit `test/buildah/bud/apply-podman-deltas`. Search for "actual podman bugs" near the bottom, and add a new `skip` line with the reason (INCLUDE THE ISSUE NUMBER!) and the test name.
+
+### In all cases
+
+You will probably want to rerun `run-buildah-bud-tests` to save yourself
+the hassle of having it fail in CI. (`rm -rf test-buildah-v<TAB>` first).
+If you're debugging problems that run on a specific test, you can
+use `--filter="pattern"` to run only tests that match "pattern".
+
+If everything passes, `git commit --amend` your PR, adding the
+files you changed under `test/buildah-bud`, then `git push --force`.
diff --git a/test/buildah-bud/apply-podman-deltas b/test/buildah-bud/apply-podman-deltas
new file mode 100755
index 000000000..9f6f38190
--- /dev/null
+++ b/test/buildah-bud/apply-podman-deltas
@@ -0,0 +1,157 @@
+#!/bin/bash
+#
+# *** NOTE TO READER: Please skip down to "user-customizable section" below!
+#
+# Not all tests in buildah/tests/bud.bats work under podman.
+# Some work, but emit different error messages.
+#
+# This script is used to skip the former, and munge expect_output messages
+# for the latter.
+#
+ME=$(basename $0)
+
+BUD=tests/bud.bats
+
+if [[ ! -e $BUD ]]; then
+    echo "$ME: $BUD not found: please run me from buildah subdir" >&2
+    exit 1
+fi
+
+###############################################################################
+# BEGIN handlers
+#
+# *** NOTE TO READER (again): Please skip down to "user-customizable section"
+#
+# You almost certainly don't care about anything in this section.
+#
+set -e
+
+RC=0
+
+ECHO=':'
+if [[ -n $DEBUG_PODMAN_DELTAS ]]; then
+    ECHO='echo'
+fi
+
+# Issue a warning, and set exit status (but do not exit now)
+function warn() {
+    echo "$ME: ERROR: $*" >&2
+    RC=1
+}
+
+# errmsg: used to change the text of a message, probably in expect_output()
+function errmsg() {
+    local msg_orig=${1//\//\\/}; shift
+    local msg_new=${1//\//\\/};  shift
+
+    for t in "$@"; do
+        if fgrep -qx "@test \"$t\" {" $BUD; then
+            $ECHO "@test \"$t\" : updating to \"$msg_new\""
+            t=${t//\//\\/}
+            # FIXME: emit error if msg_orig not found
+            sed -i -e "/^\@test \"$t\" {/,/^}/s/\"$msg_orig\"/\"$msg_new\"/" $BUD
+        else
+            warn "[errmsg] Did not find test \"$t\" in $BUD"
+        fi
+    done
+}
+
+# skip: used to add a 'skip' to one specific test
+function skip() {
+    local reason=$1; shift
+
+    # All further arguments are test names
+    for t in "$@"; do
+        if fgrep -qx "@test \"$t\" {" $BUD; then
+            $ECHO "@test \"$t\" : skip \"$reason\""
+            t=${t//\//\\/}
+            sed -i -e "/^\@test \"$t\" {/ a \ \ skip \"$reason\"" $BUD
+        else
+            warn "[skip] Did not find test \"$t\" in $BUD"
+        fi
+    done
+}
+
+# END   handlers
+###############################################################################
+# BEGIN user-customizable section
+#
+# These are the hand-maintained exceptions. This is what you want to edit
+# or update as needed.
+#
+# There are two directives you can use below:
+#
+#    errmsg "old-message" "new-message" "test name" ["test name"...]
+#
+# This replaced "old-message" with "new-message" in @test "test name".
+# It is used when a podman error message differs from buildah's.
+#
+#    skip "reason" "test name" ["test name"...]
+#
+# This adds a 'skip' statement as the first line of @test "test name".
+# It is used when a test does not work in podman, either for permanent
+# design-related reasons or for hopefully-temporary bug-in-podman reasons.
+# (If the latter, please file an issue before adding the skip, and include
+# the issue number in your skip message. This makes it possible to remove
+# the skip once the issue is fixed).
+#
+# For both cases, you can list multiple "test names" at the end. This
+# is not used much right now, but will be once I file my podman-remote PR
+# because there are some cases where the same issue affects up to fifty
+# different bud.bats tests.
+#
+
+###############################################################################
+# BEGIN differences in error messages between buildah and podman
+errmsg "non-directory/Dockerfile: not a directory" \
+       "Error: context must be a directory:" \
+       "bud with a path to a Dockerfile (-f) containing a non-directory entry"
+
+errmsg "no such file or directory" \
+       "Error: context must be a directory:" \
+       "bud with dir for file but no Dockerfile in dir" \
+       "bud with bad dir Dockerfile"
+
+errmsg "no such file or directory" \
+       "Error: no context directory and no Containerfile specified" \
+       "bud without any arguments should fail when no Dockerfile exist"
+
+errmsg "is not a file" \
+       "Error: open .*: no such file or directory" \
+       "bud with specified context should fail if assumed Dockerfile is a directory"
+
+errmsg "no such file or directory" \
+       "context must be a directory" \
+       "bud with specified context should fail if context contains not-existing Dockerfile"
+
+###############################################################################
+# BEGIN tests that don't make sense under podman due to fundamental differences
+skip "N/A under podman" \
+     "bud-flags-order-verification"
+
+skip "does not work under podman" \
+     "bud without any arguments should succeed"
+
+skip "podman requires a directory, not a Dockerfile" \
+     "bud with specified context should succeed if context contains existing Dockerfile"
+
+# ...or due to Ed's laziness
+skip "Too much effort to spin up a local registry" \
+     "bud with encrypted FROM image"
+
+# ...or due to a fundamental arg-parsing difference between buildah and podman
+# which we could and perhaps should fix in the buildah repo via:
+#    - ... ${TESTSDIR}/bud/layers-squash/Dockerfile.hardlinks
+#    + ... -f Dockerfile.hardlinks ${TESTSDIR}/bud/layers-squash
+skip "FIXME FIXME FIXME: argument-order incompatible with podman" \
+     "bud-squash-hardlinks"
+
+###############################################################################
+# BEGIN tests which are skipped due to actual podman bugs.
+skip "FIXME: podman #9915" \
+     "bud with --arch flag"
+
+###############################################################################
+# Done.
+
+exit $RC
diff --git a/test/buildah-bud/buildah-tests.diff b/test/buildah-bud/buildah-tests.diff
index 141bd9266..bba737848 100644
--- a/test/buildah-bud/buildah-tests.diff
+++ b/test/buildah-bud/buildah-tests.diff
@@ -1,130 +1,13 @@
-From 47cc8f54d272039bdab389b29c8b2b5bb085e923 Mon Sep 17 00:00:00 2001
+From b948e99cb6cb4765987711e8d8948841f6d3f7e2 Mon Sep 17 00:00:00 2001
 From: Ed Santiago <santiago@redhat.com>
 Date: Tue, 9 Feb 2021 17:28:05 -0700
 Subject: [PATCH] tweaks for running buildah tests under podman
 
 Signed-off-by: Ed Santiago <santiago@redhat.com>
 ---
- tests/bud.bats     | 22 +++++++++++++---------
  tests/helpers.bash | 28 ++++++++++++++++++++++++----
- 2 files changed, 37 insertions(+), 13 deletions(-)
+ 1 file changed, 24 insertions(+), 4 deletions(-)
 
-diff --git a/tests/bud.bats b/tests/bud.bats
-index fe3af27a..d1ccced5 100644
---- a/tests/bud.bats
-+++ b/tests/bud.bats
-@@ -4,7 +4,7 @@ load helpers
-
- @test "bud with a path to a Dockerfile (-f) containing a non-directory entry" {
-   run_buildah 125 bud -f ${TESTSDIR}/bud/non-directory-in-path/non-directory/Dockerfile
--  expect_output --substring "non-directory/Dockerfile: not a directory"
-+  expect_output --substring "Error: context must be a directory:"
- }
-
- @test "bud with --dns* flags" {
-@@ -117,6 +117,7 @@ symlink(subdir)"
- }
-
- @test "bud-flags-order-verification" {
-+  skip "N/A under podman"
-   run_buildah 125 bud /tmp/tmpdockerfile/ -t blabla
-   check_options_flag_err "-t"
-
-@@ -1436,13 +1437,13 @@ function _test_http() {
- @test "bud with dir for file but no Dockerfile in dir" {
-   target=alpine-image
-   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/bud/empty-dir ${TESTSDIR}/bud/empty-dir
--  expect_output --substring "no such file or directory"
-+  expect_output --substring "Error: context must be a directory:"
- }
-
- @test "bud with bad dir Dockerfile" {
-   target=alpine-image
-   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json -t ${target} -f ${TESTSDIR}/baddirname ${TESTSDIR}/baddirname
--  expect_output --substring "no such file or directory"
-+  expect_output --substring "Error: context must be a directory:"
- }
-
- @test "bud with ARG before FROM default value" {
-@@ -1913,6 +1914,7 @@ _EOF
- }
-
- @test "bud without any arguments should succeed" {
-+  skip "does not work under podman"
-   cd ${TESTSDIR}/bud/from-scratch
-   run_buildah bud --signature-policy ${TESTSDIR}/policy.json
- }
-@@ -1920,7 +1922,7 @@ _EOF
- @test "bud without any arguments should fail when no Dockerfile exist" {
-   cd $(mktemp -d)
-   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json
--  expect_output --substring "no such file or directory"
-+  expect_output "Error: no context directory and no Containerfile specified"
- }
-
- @test "bud with specified context should fail if directory contains no Dockerfile" {
-@@ -1933,16 +1935,17 @@ _EOF
-   DIR=$(mktemp -d)
-   mkdir -p "$DIR"/Dockerfile
-   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json "$DIR"
--  expect_output --substring "is not a file"
-+  expect_output --substring "Error: open .*: no such file or directory"
- }
-
- @test "bud with specified context should fail if context contains not-existing Dockerfile" {
-   DIR=$(mktemp -d)
-   run_buildah 125 bud --signature-policy ${TESTSDIR}/policy.json "$DIR"/Dockerfile
--  expect_output --substring "no such file or directory"
-+  expect_output --substring "context must be a directory"
- }
-
- @test "bud with specified context should succeed if context contains existing Dockerfile" {
-+  skip "podman requires a directory, not a Dockerfile"
-   DIR=$(mktemp -d)
-   echo "FROM alpine" > "$DIR"/Dockerfile
-   run_buildah 0 bud --signature-policy ${TESTSDIR}/policy.json "$DIR"/Dockerfile
-@@ -1994,7 +1997,7 @@ _EOF
-
- @test "bud-squash-hardlinks" {
-   _prefetch busybox
--  run_buildah bud --signature-policy ${TESTSDIR}/policy.json --squash ${TESTSDIR}/bud/layers-squash/Dockerfile.hardlinks
-+  run_buildah bud --signature-policy ${TESTSDIR}/policy.json --squash -f Dockerfile.hardlinks ${TESTSDIR}/bud/layers-squash
- }
-
- @test "bud with additional directory of devices" {
-@@ -2159,6 +2162,7 @@ _EOF
- }
-
- @test "bud with Containerfile should fail with nonexistent authfile" {
-+  skip "FIXME: podman issue #9572"
-   target=alpine-image
-   run_buildah 125 bud --authfile /tmp/nonexistent --signature-policy ${TESTSDIR}/policy.json -t ${target} ${TESTSDIR}/bud/containerfile
- }
-@@ -2286,6 +2290,7 @@ EOM
- }
-
- @test "bud with encrypted FROM image" {
-+  skip "Too much effort to spin up a local registry"
-   _prefetch busybox
-   mkdir ${TESTDIR}/tmp
-   openssl genrsa -out ${TESTDIR}/tmp/mykey.pem 1024
-@@ -2358,8 +2363,6 @@ EOM
-   _prefetch alpine
-   run_buildah bud --timestamp=0 --quiet --pull=false --signature-policy ${TESTSDIR}/policy.json -t timestamp -f Dockerfile.1 ${TESTSDIR}/bud/cache-stages
-   cid=$output
--  run_buildah inspect --format '{{ .Docker.Created }}' timestamp
--  expect_output --substring "1970-01-01"
-   run_buildah inspect --format '{{ .OCIv1.Created }}' timestamp
-   expect_output --substring "1970-01-01"
-   run_buildah inspect --format '{{ .History }}' timestamp
-@@ -2619,6 +2622,7 @@ _EOF
- }
-
- @test "bud with --arch flag" {
-+  skip "FIXME: for podman, emergency skip to get CI going"
-   _prefetch alpine
-   mytmpdir=${TESTDIR}/my-dir
-   mkdir -p ${mytmpdir}
 diff --git a/tests/helpers.bash b/tests/helpers.bash
 index 99c290af..c5572840 100644
 --- a/tests/helpers.bash
diff --git a/test/buildah-bud/make-new-buildah-diffs b/test/buildah-bud/make-new-buildah-diffs
index 1191f4597..11987e376 100644
--- a/test/buildah-bud/make-new-buildah-diffs
+++ b/test/buildah-bud/make-new-buildah-diffs
@@ -37,11 +37,11 @@ if [[ $n_commits -gt 1 ]]; then
     die "Please squash your commits"
 fi
 
-# Scope check: make sure the only files changed are under tests/
-changes=$(git diff --name-status [BASETAG]..HEAD | egrep -v '\stests/')
+# Scope check: make sure the only files changed is helpers.bash
+changes=$(git diff --name-status [BASETAG]..HEAD | egrep -v '\stests/helpers.bash')
 if [[ -n "$changes" ]]; then
     echo $changes
-    die "Found modified files other than under 'tests/'"
+    die "Found modified files other than 'tests/helpers.bash'"
 fi
 
 ###############################################################################
diff --git a/test/buildah-bud/run-buildah-bud-tests b/test/buildah-bud/run-buildah-bud-tests
index 1265e67d5..b08b854ee 100755
--- a/test/buildah-bud/run-buildah-bud-tests
+++ b/test/buildah-bud/run-buildah-bud-tests
@@ -14,16 +14,25 @@ BASE_TAG=buildah-bud-in-podman
 # END   user-customizable section
 ###############################################################################
 
-usage="Usage: $ME [--help] [--no-checkout] [--no-test]
+usage="Usage: $ME [--help] [--no-checkout] [--no-test] [--filter=TESTNAME]
+
+Flags, useful for manual debugging:
+
+  --no-checkout   Skip checkout step, go directly to running tests
+  --no-test       Do checkout only, but do not run tests
+  --filter=NAME   Passed on to bats; runs only tests that match NAME
 "
 
 # Parse command-line options (used in development only, not in CI)
 do_checkout=y
 do_test=y
+declare -a bats_filter=()
 for i; do
+    value=$(expr "$i" : '[^=]*=\(.*\)')
     case "$i" in
         --no-checkout)  do_checkout= ; shift;;
         --no-test)      do_test=     ; shift;;
+        --filter=*)     bats_filter=("--filter" "$value"); shift;;
         -h|--help)      echo "$usage"; exit 0;;
         *)              echo "$ME: Unrecognized option '$i'" >&2; exit 1;;
     esac
@@ -150,12 +159,33 @@ if [[ -n $do_checkout ]]; then
     chmod 755 make-new-buildah-diffs
 
     # Apply custom patches. We do this _after_ building, although it shouldn't
-    # matter because these patches should only apply to test scripts.
+    # matter because these patches should only apply to test scripts and not
+    # to any buildah sources.
     failhint="
 Error applying patch file. This can happen when you vendor in a new buildah.
+You will want to:
 
-Look for '*.rej' files to resolve the conflict(s) manually."
+  - look for 'test/*.rej'
+  - resolve conflicts manually
+  - git add test/helpers.bash
+  - git am --continue
+  - ./make-new-buildah-diffs
+"
     (set -x;git am --reject <$PATCHES)
+
+    # Now apply our custom skips and error-message changes. This is maintained
+    # in a custom script, not a .diff file, because diffs are WAY too hard for
+    # humans to read and update.
+    APPLY=apply-podman-deltas
+    failhint="
+Error applying podman-specific deltas. This sometimes happens when you
+vendor in a new buildah. You will want to:
+
+  - inspect the errors shown above
+  - find the corresponding lines in $BUD_TEST_DIR_REL/$APPLY
+  - edit/delete them as necessary
+"
+    (set -x;$BUD_TEST_DIR/$APPLY)
 else
     # Called with --no-checkout
     test -d $buildah_dir || die "Called with --no-checkout, but $buildah_dir does not exist"
@@ -183,5 +213,5 @@ review the test failure and double-check your changes.
     (set -x;sudo env TMPDIR=/var/tmp \
                  PODMAN_BINARY=$PODMAN_BINARY \
                  BUILDAH_BINARY=$(pwd)/bin/buildah \
-                 bats tests/bud.bats)
+                 bats "${bats_filter[@]}" tests/bud.bats)
 fi