buildah-bud tests: simplify #9977
Conversation
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 <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
| @@ -1,5 +1,4 @@ | |||
| buildah-bud tests under podman | |||
| ============================== | |||
| # buildah-bud tests under podman | |||
There was a problem hiding this comment.
Review recommendation for this file: '...' menu (top right) -> View file, instead of trying to read the diffs
| @@ -0,0 +1,157 @@ | |||
| #!/bin/bash | |||
| # | |||
| # *** NOTE TO READER: Please skip down to "user-customizable section" below! | |||
There was a problem hiding this comment.
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?
| 2 files changed, 37 insertions(+), 13 deletions(-) | ||
| 1 file changed, 24 insertions(+), 4 deletions(-) |
There was a problem hiding this comment.
You don't need to review this diff file, either. The important part is: there are no longer any diffs to bud.bats, only helpers.bash.
|
Since I am the only one who has gone through this. I will say LGTM. /lgtm We will truly know the next time buildah updates. |
github-actions
bot
added
the
locked - please file new issue/PR
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:
...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 [email protected]