From: Michael Haggerty <mhagger@alum.mit.edu>
To: Jeff King <peff@peff.net>,
git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 08/16] t5304: use helper to report failure of "test foo = bar"
Date: Tue, 07 Oct 2014 15:21:15 +0200 [thread overview]
Message-ID: <5433E8CB.1050005@alum.mit.edu> (raw)
In-Reply-To: <20141003202743.GH16293@peff.net>
On 10/03/2014 10:27 PM, Jeff King wrote:
> For small outputs, we sometimes use:
>
> test "$(some_cmd)" = "something we expect"
>
> instead of a full test_cmp. The downside of this is that
> when it fails, there is no output at all from the script.
> Let's introduce a small helper to make tests easier to
> debug.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This is in the same boat as the last commit; we can drop it without
> hurting the rest of the series.
>
> Is test_eq too cutesy or obfuscated? I have often wanted it when
> debugging other tests, too. Our usual technique is to do:
>
> echo whatever >expect &&
> do_something >actual &&
> test_cmp expect actual
>
> That's a bit verbose. We could hide it behind something like test_eq,
> too, but it introduces several extra new processes. And I know people on
> some fork-challenged platforms are very sensitive to the number of
> spawned processes in the test suite.
I don't like the three-argument version of test_eq. Wouldn't using a
comparison operator other than "=" would be very confusing, given that
"eq" is in the name of the function? It also doesn't look like you use
this feature.
If you want to write a helper that allows arbitrary comparator
operators, then I think it would be more readable to put the comparison
operator in the middle, like
test_test foo = bar
And in fact once you've done that, couldn't we just make this a generic
wrapper for any `test` command?
test_test () {
if ! test "$@"
then
echo >&2 "test failed: $*"
false
fi
}
Feel free to bikeshed the function name.
An alternative direction to go would be to specialize the function for
equality testing and delegate to test_cmp to get better output for
failures, but optimized to avoid excess process creation in the happy path:
test_eq () {
if test "$1" != "$2"
then
printf "%s" "$1" >expect &&
printf "%s" "$2" >actual &&
test_cmp expect actual
fi
}
(but using properly-created temporary file names).
Finally, if we want a function intended mainly for checking program
output (like the test_eql function suggested by Junio), it might be
simpler to use if it accepts the function output on its stdin:
test_output () {
echo "$1" >expect &&
cat >actual &&
test_cmp expect actual
}
...
do_something | test_output whatever
This would make it easier to generate the input using an arbitrary shell
pipeline.
> [...]
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
next prev parent reply other threads:[~2014-10-07 13:21 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-03 20:20 [PATCH 0/16] make prune mtime-checking more careful Jeff King
2014-10-03 20:21 ` [PATCH 01/16] foreach_alt_odb: propagate return value from callback Jeff King
2014-10-03 22:55 ` René Scharfe
2014-10-04 0:31 ` Jeff King
2014-10-03 20:22 ` [PATCH 02/16] isxdigit: cast input to unsigned char Jeff King
2014-10-03 20:22 ` [PATCH 03/16] object_array: factor out slopbuf-freeing logic Jeff King
2014-10-07 11:25 ` Michael Haggerty
2014-10-08 7:36 ` Jeff King
2014-10-08 8:40 ` Michael Haggerty
2014-10-08 8:55 ` Jeff King
2014-10-03 20:22 ` [PATCH 04/16] object_array: add a "clear" function Jeff King
2014-10-03 20:23 ` [PATCH 05/16] clean up name allocation in prepare_revision_walk Jeff King
2014-10-03 20:24 ` [PATCH 06/16] reachable: clear pending array after walking it Jeff King
2014-10-03 20:25 ` [PATCH 07/16] t5304: use test_path_is_* instead of "test -f" Jeff King
2014-10-03 22:12 ` Junio C Hamano
2014-10-03 20:27 ` [PATCH 08/16] t5304: use helper to report failure of "test foo = bar" Jeff King
2014-10-03 22:17 ` Junio C Hamano
2014-10-04 0:13 ` Jeff King
2014-10-07 13:21 ` Michael Haggerty [this message]
2014-10-07 17:29 ` Junio C Hamano
2014-10-07 20:18 ` Jeff King
2014-10-07 20:35 ` Junio C Hamano
2014-10-07 21:29 ` Jeff King
2014-10-07 21:53 ` Junio C Hamano
2014-10-07 22:17 ` Michael Haggerty
2014-10-08 1:13 ` Jeff King
2014-10-08 16:58 ` Junio C Hamano
2014-10-07 21:16 ` Junio C Hamano
2014-10-03 20:29 ` [PATCH 09/16] prune: factor out loose-object directory traversal Jeff King
2014-10-03 22:19 ` Junio C Hamano
2014-10-04 0:24 ` Jeff King
2014-10-07 14:07 ` Michael Haggerty
2014-10-08 7:33 ` Jeff King
2014-10-03 20:31 ` [PATCH 10/16] count-objects: do not use xsize_t when counting object size Jeff King
2014-10-03 20:31 ` [PATCH 11/16] count-objects: use for_each_loose_file_in_objdir Jeff King
2014-10-03 20:32 ` [PATCH 12/16] sha1_file: add for_each iterators for loose and packed objects Jeff King
2014-10-05 8:15 ` René Scharfe
2014-10-05 10:47 ` Ramsay Jones
2014-10-03 20:39 ` [PATCH 13/16] prune: keep objects reachable from recent objects Jeff King
2014-10-03 21:47 ` Junio C Hamano
2014-10-04 0:09 ` Jeff King
2014-10-04 0:30 ` Jeff King
2014-10-04 3:04 ` Junio C Hamano
2014-10-07 16:29 ` Michael Haggerty
2014-10-08 7:19 ` Jeff King
2014-10-08 10:37 ` Michael Haggerty
2014-10-03 20:39 ` [PATCH 14/16] pack-objects: refactor unpack-unreachable expiration check Jeff King
2014-10-03 20:40 ` [PATCH 15/16] pack-objects: match prune logic for discarding objects Jeff King
2014-10-03 20:41 ` [PATCH 16/16] write_sha1_file: freshen existing objects Jeff King
2014-10-03 21:29 ` Junio C Hamano
2014-10-04 0:01 ` Jeff King
2014-10-05 9:12 ` René Scharfe
2014-10-03 22:20 ` [PATCH 0/16] make prune mtime-checking more careful Junio C Hamano
2014-10-04 22:22 ` Junio C Hamano
2014-10-05 9:19 ` René Scharfe
2014-10-06 1:42 ` Jeff King
2014-10-08 8:31 ` Jeff King
2014-10-08 17:03 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5433E8CB.1050005@alum.mit.edu \
--to=mhagger@alum.mit.edu \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.