Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buildah-bud tests: simplify #9977

Merged
merged 1 commit into from
Apr 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 47 additions & 23 deletions test/buildah-bud/README.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
buildah-bud tests under podman
==============================
# buildah-bud tests under podman
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review recommendation for this file: '...' menu (top right) -> View file, instead of trying to read the diffs


This directory contains tools for running 'buildah bud' tests
under podman. The key concept of the workflow is:
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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:
Expand All @@ -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`.
157 changes: 157 additions & 0 deletions test/buildah-bud/apply-podman-deltas
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
#!/bin/bash
#
# *** NOTE TO READER: Please skip down to "user-customizable section" below!
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review recommendation: this advice is suitable for reviewers, too. If you want, give a once-over to the bash cruft immediately below, but what I really truly care about is the actual user-maintainable errmsg and skip directives. I want to know: do you find that readable? Maintainable?

#
# 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
Loading