From accac4199c1d28dfd6c860b32d7111c3de8df7a6 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 5 May 2016 13:30:10 -0700 Subject: [PATCH 1/4] test-parse-options: fix output when callback option fails When test-parse-options detects an error on the command line, it gives the usage string just like any parse-options API users do, without showing any "variable dump". An exception is the callback test, where a "variable dump" for the option is done before the command line options are fully parsed. Do not expose this implementation detail by separating the handling of callback test into two phases, one to capture the fact that an option was given during the option parsing phase, and the other to show that fact as a part of normal "variable dump". The effect of this fix is seen in the patch to t/t0040 where it tried "test-parse-options --no-length" where "--length" is a callback that does not take a negative form. Signed-off-by: Junio C Hamano --- t/t0040-parse-options.sh | 4 +--- test-parse-options.c | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index fec3fef9a1..dbaee5526a 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -356,9 +356,7 @@ test_expect_success 'OPT_CALLBACK() and OPT_BIT() work' ' test_cmp expect output ' -cat >expect <<\EOF -Callback: "not set", 1 -EOF +>expect test_expect_success 'OPT_CALLBACK() and callback errors work' ' test_must_fail test-parse-options --no-length >output 2>output.err && diff --git a/test-parse-options.c b/test-parse-options.c index f02c275f33..b5f4e900ba 100644 --- a/test-parse-options.c +++ b/test-parse-options.c @@ -14,10 +14,18 @@ static char *file = NULL; static int ambiguous; static struct string_list list; +static struct { + int called; + const char *arg; + int unset; +} length_cb; + static int length_callback(const struct option *opt, const char *arg, int unset) { - printf("Callback: \"%s\", %d\n", - (arg ? arg : "not set"), unset); + length_cb.called = 1; + length_cb.arg = arg; + length_cb.unset = unset; + if (unset) return 1; /* do not support unset */ @@ -84,6 +92,12 @@ int main(int argc, char **argv) argc = parse_options(argc, (const char **)argv, prefix, options, usage, 0); + if (length_cb.called) { + const char *arg = length_cb.arg; + int unset = length_cb.unset; + printf("Callback: \"%s\", %d\n", + (arg ? arg : "not set"), unset); + } printf("boolean: %d\n", boolean); printf("integer: %d\n", integer); printf("magnitude: %lu\n", magnitude); From ab6b28b02f4db52ab5bad342592399f2559b4d81 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 5 May 2016 14:36:55 -0700 Subject: [PATCH 2/4] test-parse-options: --expect= option to simplify tests Existing tests in t0040 follow a rather verbose pattern: cat >expect <<\EOF boolean: 0 integer: 0 magnitude: 0 timestamp: 0 string: (not set) abbrev: 7 verbose: 0 quiet: 3 dry run: no file: (not set) EOF test_expect_success 'multiple quiet levels' ' test-parse-options -q -q -q >output 2>output.err && test_must_be_empty output.err && test_cmp expect output ' But the only thing this test cares about is if "quiet: 3" is in the output. We should be able to write the above 18 lines with just four lines, like this: test_expect_success 'multiple quiet levels' ' test-parse-options --expect="quiet: 3" -q -q -q ' Teach the new --expect= option to test-parse-options helper. Signed-off-by: Junio C Hamano --- t/t0040-parse-options.sh | 1 + test-parse-options.c | 90 +++++++++++++++++++++++++++++++++------- 2 files changed, 76 insertions(+), 15 deletions(-) diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index dbaee5526a..d678fbf1b7 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -45,6 +45,7 @@ Standard options -v, --verbose be verbose -n, --dry-run dry run -q, --quiet be quiet + --expect expected output in the variable dump EOF diff --git a/test-parse-options.c b/test-parse-options.c index b5f4e900ba..8a1235d03e 100644 --- a/test-parse-options.c +++ b/test-parse-options.c @@ -39,6 +39,61 @@ static int number_callback(const struct option *opt, const char *arg, int unset) return 0; } +static int collect_expect(const struct option *opt, const char *arg, int unset) +{ + struct string_list *expect; + struct string_list_item *item; + struct strbuf label = STRBUF_INIT; + const char *colon; + + if (!arg || unset) + die("malformed --expect option"); + + expect = (struct string_list *)opt->value; + colon = strchr(arg, ':'); + if (!colon) + die("malformed --expect option, lacking a colon"); + strbuf_add(&label, arg, colon - arg); + item = string_list_insert(expect, strbuf_detach(&label, NULL)); + if (item->util) + die("malformed --expect option, duplicate %s", label.buf); + item->util = (void *)arg; + return 0; +} + +__attribute__((format (printf,3,4))) +static void show(struct string_list *expect, int *status, const char *fmt, ...) +{ + struct string_list_item *item; + struct strbuf buf = STRBUF_INIT; + va_list args; + + va_start(args, fmt); + strbuf_vaddf(&buf, fmt, args); + va_end(args); + + if (!expect->nr) + printf("%s\n", buf.buf); + else { + char *colon = strchr(buf.buf, ':'); + if (!colon) + die("malformed output format, output lacking colon: %s", fmt); + *colon = '\0'; + item = string_list_lookup(expect, buf.buf); + *colon = ':'; + if (!item) + ; /* not among entries being checked */ + else { + if (strcmp((const char *)item->util, buf.buf)) { + printf("-%s\n", (char *)item->util); + printf("+%s\n", buf.buf); + *status = 1; + } + } + } + strbuf_release(&buf); +} + int main(int argc, char **argv) { const char *prefix = "prefix/"; @@ -46,6 +101,7 @@ int main(int argc, char **argv) "test-parse-options ", NULL }; + struct string_list expect = STRING_LIST_INIT_NODUP; struct option options[] = { OPT_BOOL(0, "yes", &boolean, "get a boolean"), OPT_BOOL('D', "no-doubt", &boolean, "begins with 'no-'"), @@ -86,34 +142,38 @@ int main(int argc, char **argv) OPT__VERBOSE(&verbose, "be verbose"), OPT__DRY_RUN(&dry_run, "dry run"), OPT__QUIET(&quiet, "be quiet"), + OPT_CALLBACK(0, "expect", &expect, "string", + "expected output in the variable dump", + collect_expect), OPT_END(), }; int i; + int ret = 0; argc = parse_options(argc, (const char **)argv, prefix, options, usage, 0); if (length_cb.called) { const char *arg = length_cb.arg; int unset = length_cb.unset; - printf("Callback: \"%s\", %d\n", - (arg ? arg : "not set"), unset); + show(&expect, &ret, "Callback: \"%s\", %d", + (arg ? arg : "not set"), unset); } - printf("boolean: %d\n", boolean); - printf("integer: %d\n", integer); - printf("magnitude: %lu\n", magnitude); - printf("timestamp: %lu\n", timestamp); - printf("string: %s\n", string ? string : "(not set)"); - printf("abbrev: %d\n", abbrev); - printf("verbose: %d\n", verbose); - printf("quiet: %d\n", quiet); - printf("dry run: %s\n", dry_run ? "yes" : "no"); - printf("file: %s\n", file ? file : "(not set)"); + show(&expect, &ret, "boolean: %d", boolean); + show(&expect, &ret, "integer: %d", integer); + show(&expect, &ret, "magnitude: %lu", magnitude); + show(&expect, &ret, "timestamp: %lu", timestamp); + show(&expect, &ret, "string: %s", string ? string : "(not set)"); + show(&expect, &ret, "abbrev: %d", abbrev); + show(&expect, &ret, "verbose: %d", verbose); + show(&expect, &ret, "quiet: %d", quiet); + show(&expect, &ret, "dry run: %s", dry_run ? "yes" : "no"); + show(&expect, &ret, "file: %s", file ? file : "(not set)"); for (i = 0; i < list.nr; i++) - printf("list: %s\n", list.items[i].string); + show(&expect, &ret, "list: %s", list.items[i].string); for (i = 0; i < argc; i++) - printf("arg %02d: %s\n", i, argv[i]); + show(&expect, &ret, "arg %02d: %s", i, argv[i]); - return 0; + return ret; } From 32d51d473f7a0a9f86b4d082fb4f9de78314a0bd Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 6 May 2016 10:52:34 -0700 Subject: [PATCH 3/4] t0040: remove unused test helpers 9a001381 (Fix tests under GETTEXT_POISON on parseopt, 2012-08-27) introduced check_i18n, but the helper was never used from the beginning. The same commit also introduced check_unknown_i18n to replace the helper check_unknown and changed all users of the latter to use the former, but failed to remove check_unknown itself. Signed-off-by: Junio C Hamano --- t/t0040-parse-options.sh | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index d678fbf1b7..5c8c72aa05 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -81,30 +81,6 @@ check() { test_cmp expect output } -check_i18n() { - what="$1" && - shift && - expect="$1" && - shift && - sed "s/^$what .*/$what $expect/" expect && - test-parse-options $* >output 2>output.err && - test_must_be_empty output.err && - test_i18ncmp expect output -} - -check_unknown() { - case "$1" in - --*) - echo error: unknown option \`${1#--}\' >expect ;; - -*) - echo error: unknown switch \`${1#-}\' >expect ;; - esac && - cat expect.err >>expect && - test_must_fail test-parse-options $* >output 2>output.err && - test_must_be_empty output && - test_cmp expect output.err -} - check_unknown_i18n() { case "$1" in --*) From 8ca65aebad55790d675f1eaead2dfc15fae60847 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 6 May 2016 11:17:05 -0700 Subject: [PATCH 4/4] t0040: convert a few tests to use test-parse-options --expect As a small example of using "test-parse-options --expect", rewrite the "check" helper using it, instead of comparing the whole variable dump. Signed-off-by: Junio C Hamano --- t/t0040-parse-options.sh | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 5c8c72aa05..db5f60d0c5 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -57,28 +57,12 @@ test_expect_success 'test help' ' mv expect expect.err -cat >expect.template <<\EOF -boolean: 0 -integer: 0 -magnitude: 0 -timestamp: 0 -string: (not set) -abbrev: 7 -verbose: -1 -quiet: 0 -dry run: no -file: (not set) -EOF - -check() { +check () { what="$1" && shift && expect="$1" && shift && - sed "s/^$what .*/$what $expect/" expect && - test-parse-options $* >output 2>output.err && - test_must_be_empty output.err && - test_cmp expect output + test-parse-options --expect="$what $expect" "$@" } check_unknown_i18n() {