| Message ID | patch-v3-03.10-045d58d8201-20211013T222329Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | progress: assert "global_progress" + test fixes / cleanup | expand |
On Thu, Oct 14, 2021 at 12:28:19AM +0200, Ævar Arnfjörð Bjarmason wrote: > Subject: [PATCH v3 03/10] progress.c tests: make start/stop verbs on stdin s/verbs/commands/ or instructions. Please call them what they are in the context of Git in general, or in a test script in particular, instead of what part of speech they would be if they were to appear in a sentence. > Change the usage of the "test-tool progress" introduced in > 2bb74b53a49 (Test the progress display, 2019-09-16) to take command > like "start" and "stop" on stdin, instead of running them implicitly. > > This makes for tests that are easier to read, since the recipe will > mirror the API usage, and allows for easily testing invalid usage that > would yield (or should yield) a BUG(), e.g. providing two "start" > calls in a row. A subsequent commit will add such tests. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > t/helper/test-progress.c | 37 ++++++++++++++++------- > t/t0500-progress-display.sh | 58 +++++++++++++++++++++++-------------- > 2 files changed, 63 insertions(+), 32 deletions(-) > > diff --git a/t/helper/test-progress.c b/t/helper/test-progress.c > index 50fd3be3dad..45ccbafa9da 100644 > --- a/t/helper/test-progress.c > +++ b/t/helper/test-progress.c > @@ -3,6 +3,9 @@ > * > * Reads instructions from standard input, one instruction per line: > * > + * "start <total>[ <title>]" - Call start_progress(title, total), > + * Uses the default title of "Working hard" > + * if the " <title>" is omitted. > * "progress <items>" - Call display_progress() with the given item count > * as parameter. > * "throughput <bytes> <millis> - Call display_throughput() with the given > @@ -10,6 +13,7 @@ > * specify the time elapsed since the > * start_progress() call. > * "update" - Set the 'progress_update' flag. > + * "stop" - Call stop_progress(). > * > * See 't0500-progress-display.sh' for examples. > */ > @@ -19,34 +23,43 @@ > #include "parse-options.h" > #include "progress.h" > #include "strbuf.h" > +#include "string-list.h" > > int cmd__progress(int argc, const char **argv) > { > - int total = 0; > - const char *title; > + const char *const default_title = "Working hard"; > + struct string_list list = STRING_LIST_INIT_DUP; > + const struct string_list_item *item; > struct strbuf line = STRBUF_INIT; > - struct progress *progress; > + struct progress *progress = NULL; > > const char *usage[] = { > - "test-tool progress [--total=<n>] <progress-title>", > + "test-tool progress <stdin", > NULL > }; > struct option options[] = { > - OPT_INTEGER(0, "total", &total, "total number of items"), > OPT_END(), > }; > > argc = parse_options(argc, argv, NULL, options, usage, 0); > - if (argc != 1) > - die("need a title for the progress output"); > - title = argv[0]; > + if (argc) > + usage_with_options(usage, options); > > progress_testing = 1; > - progress = start_progress(title, total); > while (strbuf_getline(&line, stdin) != EOF) { > char *end; > > - if (skip_prefix(line.buf, "progress ", (const char **) &end)) { > + if (skip_prefix(line.buf, "start ", (const char **) &end)) { > + uint64_t total = strtoull(end, &end, 10); > + if (*end == '\0') { > + progress = start_progress(default_title, total); > + } else if (*end == ' ') { > + item = string_list_insert(&list, end + 1); > + progress = start_progress(item->string, total); Why is it necessary to use a string_list here? This is the only place where it is used, so I don't understand why we should store anything in it. > + } else { > + die("invalid input: '%s'\n", line.buf); > + } > + } else if (skip_prefix(line.buf, "progress ", (const char **) &end)) { > uint64_t item_count = strtoull(end, &end, 10); > if (*end != '\0') > die("invalid input: '%s'\n", line.buf); > @@ -65,12 +78,14 @@ int cmd__progress(int argc, const char **argv) > display_throughput(progress, byte_count); > } else if (!strcmp(line.buf, "update")) { > progress_test_force_update(); > + } else if (!strcmp(line.buf, "stop")) { > + stop_progress(&progress); > } else { > die("invalid input: '%s'\n", line.buf); > } > } > - stop_progress(&progress); > strbuf_release(&line); > + string_list_clear(&list, 0); > > return 0; > } > diff --git a/t/t0500-progress-display.sh b/t/t0500-progress-display.sh > index f37cf2eb9c9..27ab4218b01 100755 > --- a/t/t0500-progress-display.sh > +++ b/t/t0500-progress-display.sh > @@ -18,6 +18,7 @@ test_expect_success 'simple progress display' ' > EOF > > cat >in <<-\EOF && > + start 0 > update > progress 1 > update > @@ -26,8 +27,9 @@ test_expect_success 'simple progress display' ' > progress 4 > update > progress 5 > + stop > EOF > - test-tool progress "Working hard" <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -42,11 +44,13 @@ test_expect_success 'progress display with total' ' > EOF > > cat >in <<-\EOF && > + start 3 > progress 1 > progress 2 > progress 3 > + stop > EOF > - test-tool progress --total=3 "Working hard" <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -63,14 +67,14 @@ Working hard.......2.........3.........4.........5.........6: > EOF > > cat >in <<-\EOF && > + start 100000 Working hard.......2.........3.........4.........5.........6 > progress 100 > progress 1000 > progress 10000 > progress 100000 > + stop > EOF > - test-tool progress --total=100000 \ > - "Working hard.......2.........3.........4.........5.........6" \ > - <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -89,16 +93,16 @@ Working hard.......2.........3.........4.........5.........6: > EOF > > cat >in <<-\EOF && > + start 100000 Working hard.......2.........3.........4.........5.........6 > update > progress 1 > update > progress 2 > progress 10000 > progress 100000 > + stop > EOF > - test-tool progress --total=100000 \ > - "Working hard.......2.........3.........4.........5.........6" \ > - <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -117,14 +121,14 @@ Working hard.......2.........3.........4.........5.........6: > EOF > > cat >in <<-\EOF && > + start 100000 Working hard.......2.........3.........4.........5.........6 > progress 25000 > progress 50000 > progress 75000 > progress 100000 > + stop > EOF > - test-tool progress --total=100000 \ > - "Working hard.......2.........3.........4.........5.........6" \ > - <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -141,14 +145,14 @@ Working hard.......2.........3.........4.........5.........6.........7.........: > EOF > > cat >in <<-\EOF && > + start 100000 Working hard.......2.........3.........4.........5.........6.........7......... > progress 25000 > progress 50000 > progress 75000 > progress 100000 > + stop > EOF > - test-tool progress --total=100000 \ > - "Working hard.......2.........3.........4.........5.........6.........7........." \ > - <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -165,12 +169,14 @@ test_expect_success 'progress shortens - crazy caller' ' > EOF > > cat >in <<-\EOF && > + start 1000 > progress 100 > progress 200 > progress 1 > progress 1000 > + stop > EOF > - test-tool progress --total=1000 "Working hard" <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -186,6 +192,7 @@ test_expect_success 'progress display with throughput' ' > EOF > > cat >in <<-\EOF && > + start 0 > throughput 102400 1000 > update > progress 10 > @@ -198,8 +205,9 @@ test_expect_success 'progress display with throughput' ' > throughput 409600 4000 > update > progress 40 > + stop > EOF > - test-tool progress "Working hard" <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -215,6 +223,7 @@ test_expect_success 'progress display with throughput and total' ' > EOF > > cat >in <<-\EOF && > + start 40 > throughput 102400 1000 > progress 10 > throughput 204800 2000 > @@ -223,8 +232,9 @@ test_expect_success 'progress display with throughput and total' ' > progress 30 > throughput 409600 4000 > progress 40 > + stop > EOF > - test-tool progress --total=40 "Working hard" <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -240,6 +250,7 @@ test_expect_success 'cover up after throughput shortens' ' > EOF > > cat >in <<-\EOF && > + start 0 > throughput 409600 1000 > update > progress 1 > @@ -252,8 +263,9 @@ test_expect_success 'cover up after throughput shortens' ' > throughput 1638400 4000 > update > progress 4 > + stop > EOF > - test-tool progress "Working hard" <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -268,6 +280,7 @@ test_expect_success 'cover up after throughput shortens a lot' ' > EOF > > cat >in <<-\EOF && > + start 0 > throughput 1 1000 > update > progress 1 > @@ -277,8 +290,9 @@ test_expect_success 'cover up after throughput shortens a lot' ' > throughput 3145728 3000 > update > progress 3 > + stop > EOF > - test-tool progress "Working hard" <in 2>stderr && > + test-tool progress <in 2>stderr && > > show_cr <stderr >out && > test_cmp expect out > @@ -286,6 +300,7 @@ test_expect_success 'cover up after throughput shortens a lot' ' > > test_expect_success 'progress generates traces' ' > cat >in <<-\EOF && > + start 40 > throughput 102400 1000 > update > progress 10 > @@ -298,10 +313,11 @@ test_expect_success 'progress generates traces' ' > throughput 409600 4000 > update > progress 40 > + stop > EOF > > - GIT_TRACE2_EVENT="$(pwd)/trace.event" test-tool progress --total=40 \ > - "Working hard" <in 2>stderr && > + GIT_TRACE2_EVENT="$(pwd)/trace.event" test-tool progress \ > + <in 2>stderr && > > # t0212/parse_events.perl intentionally omits regions and data. > test_region progress "Working hard" trace.event && > -- > 2.33.1.1346.g48288c3c089 >
On Thu, Oct 14, 2021 at 12:28:19AM +0200, Ævar Arnfjörð Bjarmason wrote: > > Change the usage of the "test-tool progress" introduced in > 2bb74b53a49 (Test the progress display, 2019-09-16) to take command > like "start" and "stop" on stdin, instead of running them implicitly. > > This makes for tests that are easier to read, since the recipe will > mirror the API usage, and allows for easily testing invalid usage that > would yield (or should yield) a BUG(), e.g. providing two "start" > calls in a row. A subsequent commit will add such tests. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > t/helper/test-progress.c | 37 ++++++++++++++++------- > t/t0500-progress-display.sh | 58 +++++++++++++++++++++++-------------- > 2 files changed, 63 insertions(+), 32 deletions(-) > > diff --git a/t/helper/test-progress.c b/t/helper/test-progress.c > index 50fd3be3dad..45ccbafa9da 100644 > --- a/t/helper/test-progress.c > +++ b/t/helper/test-progress.c > @@ -19,34 +23,43 @@ > #include "parse-options.h" > #include "progress.h" > #include "strbuf.h" > +#include "string-list.h" > > int cmd__progress(int argc, const char **argv) > { > - int total = 0; > - const char *title; > + const char *const default_title = "Working hard"; > + struct string_list list = STRING_LIST_INIT_DUP; > + const struct string_list_item *item; I suspect the string_list is there to enable multiple progress lines later, no? I saw SZEDER ask about it in another reply... If it's for later, is there a reason not to add the extra structure alongside the series adding multiple progress lines? > diff --git a/t/t0500-progress-display.sh b/t/t0500-progress-display.sh > index f37cf2eb9c9..27ab4218b01 100755 > --- a/t/t0500-progress-display.sh > +++ b/t/t0500-progress-display.sh > @@ -18,6 +18,7 @@ test_expect_success 'simple progress display' ' > EOF > > cat >in <<-\EOF && > + start 0 Seems better to me, more explicit for the test reader. Thanks for the change. - Emily
On Fri, Oct 22 2021, SZEDER Gábor wrote: > On Thu, Oct 14, 2021 at 12:28:19AM +0200, Ævar Arnfjörð Bjarmason wrote: >> Subject: [PATCH v3 03/10] progress.c tests: make start/stop verbs on stdin > > s/verbs/commands/ or instructions. > > Please call them what they are in the context of Git in general, or in > a test script in particular, instead of what part of speech they would > be if they were to appear in a sentence. *nod*, although barring further issues I don't think I'll re-roll with just that commit message adjustment... >> progress_testing = 1; >> - progress = start_progress(title, total); >> while (strbuf_getline(&line, stdin) != EOF) { >> char *end; >> >> - if (skip_prefix(line.buf, "progress ", (const char **) &end)) { >> + if (skip_prefix(line.buf, "start ", (const char **) &end)) { >> + uint64_t total = strtoull(end, &end, 10); >> + if (*end == '\0') { >> + progress = start_progress(default_title, total); >> + } else if (*end == ' ') { >> + item = string_list_insert(&list, end + 1); >> + progress = start_progress(item->string, total); > > Why is it necessary to use a string_list here? This is the only place > where it is used, so I don't understand why we should store anything > in it. The progress.c API doesn't xstrdup() the title you give it, so you can't free() it after while it's alive. Here we're re-setting the same strbuf in a loop, so if we hand a "title" to progress.c we need to keep it around, when we later add a BUG assertion in 10/10: + if (global_progress) + BUG("'%s' progress still active when trying to start '%s'", + global_progress->title, progress->title); + global_progress = progress; The "old" title would point to already-free'd or to a nonsensical value if we didn't do that.
On Fri, Oct 22, 2021 at 04:18:16PM +0200, Ævar Arnfjörð Bjarmason wrote: > >> progress_testing = 1; > >> - progress = start_progress(title, total); > >> while (strbuf_getline(&line, stdin) != EOF) { > >> char *end; > >> > >> - if (skip_prefix(line.buf, "progress ", (const char **) &end)) { > >> + if (skip_prefix(line.buf, "start ", (const char **) &end)) { > >> + uint64_t total = strtoull(end, &end, 10); > >> + if (*end == '\0') { > >> + progress = start_progress(default_title, total); > >> + } else if (*end == ' ') { > >> + item = string_list_insert(&list, end + 1); > >> + progress = start_progress(item->string, total); > > > > Why is it necessary to use a string_list here? This is the only place > > where it is used, so I don't understand why we should store anything > > in it. > > The progress.c API doesn't xstrdup() the title you give it, so you can't > free() it after while it's alive. > > Here we're re-setting the same strbuf in a loop, so if we hand a "title" > to progress.c we need to keep it around, when we later add a BUG > assertion in 10/10: > > + if (global_progress) > + BUG("'%s' progress still active when trying to start '%s'", > + global_progress->title, progress->title); > + global_progress = progress; > > The "old" title would point to already-free'd or to a nonsensical value > if we didn't do that. *nod*. I figured as much, but a comment indicating that may be helpful for reviewers and future readers of this code. Thanks, Taylor
On Fri, Oct 22, 2021 at 06:14:34PM -0400, Taylor Blau wrote: > On Fri, Oct 22, 2021 at 04:18:16PM +0200, Ævar Arnfjörð Bjarmason wrote: > > >> progress_testing = 1; > > >> - progress = start_progress(title, total); > > >> while (strbuf_getline(&line, stdin) != EOF) { > > >> char *end; > > >> > > >> - if (skip_prefix(line.buf, "progress ", (const char **) &end)) { > > >> + if (skip_prefix(line.buf, "start ", (const char **) &end)) { > > >> + uint64_t total = strtoull(end, &end, 10); > > >> + if (*end == '\0') { > > >> + progress = start_progress(default_title, total); > > >> + } else if (*end == ' ') { > > >> + item = string_list_insert(&list, end + 1); > > >> + progress = start_progress(item->string, total); > > > > > > Why is it necessary to use a string_list here? This is the only place > > > where it is used, so I don't understand why we should store anything > > > in it. > > > > The progress.c API doesn't xstrdup() the title you give it, so you can't > > free() it after while it's alive. > > > > Here we're re-setting the same strbuf in a loop, so if we hand a "title" > > to progress.c we need to keep it around, when we later add a BUG > > assertion in 10/10: > > > > + if (global_progress) > > + BUG("'%s' progress still active when trying to start '%s'", > > + global_progress->title, progress->title); > > + global_progress = progress; > > > > The "old" title would point to already-free'd or to a nonsensical value > > if we didn't do that. > > *nod*. I figured as much, but a comment indicating that may be helpful > for reviewers and future readers of this code. Yeah, a comment would be nice; though perhaps giving that list variable a descriptive name, e.g. 'titles' (plural!) would have been enough to set me on the right track.
diff --git a/t/helper/test-progress.c b/t/helper/test-progress.c index 50fd3be3dad..45ccbafa9da 100644 --- a/t/helper/test-progress.c +++ b/t/helper/test-progress.c @@ -3,6 +3,9 @@ * * Reads instructions from standard input, one instruction per line: * + * "start <total>[ <title>]" - Call start_progress(title, total), + * Uses the default title of "Working hard" + * if the " <title>" is omitted. * "progress <items>" - Call display_progress() with the given item count * as parameter. * "throughput <bytes> <millis> - Call display_throughput() with the given @@ -10,6 +13,7 @@ * specify the time elapsed since the * start_progress() call. * "update" - Set the 'progress_update' flag. + * "stop" - Call stop_progress(). * * See 't0500-progress-display.sh' for examples. */ @@ -19,34 +23,43 @@ #include "parse-options.h" #include "progress.h" #include "strbuf.h" +#include "string-list.h" int cmd__progress(int argc, const char **argv) { - int total = 0; - const char *title; + const char *const default_title = "Working hard"; + struct string_list list = STRING_LIST_INIT_DUP; + const struct string_list_item *item; struct strbuf line = STRBUF_INIT; - struct progress *progress; + struct progress *progress = NULL; const char *usage[] = { - "test-tool progress [--total=<n>] <progress-title>", + "test-tool progress <stdin", NULL }; struct option options[] = { - OPT_INTEGER(0, "total", &total, "total number of items"), OPT_END(), }; argc = parse_options(argc, argv, NULL, options, usage, 0); - if (argc != 1) - die("need a title for the progress output"); - title = argv[0]; + if (argc) + usage_with_options(usage, options); progress_testing = 1; - progress = start_progress(title, total); while (strbuf_getline(&line, stdin) != EOF) { char *end; - if (skip_prefix(line.buf, "progress ", (const char **) &end)) { + if (skip_prefix(line.buf, "start ", (const char **) &end)) { + uint64_t total = strtoull(end, &end, 10); + if (*end == '\0') { + progress = start_progress(default_title, total); + } else if (*end == ' ') { + item = string_list_insert(&list, end + 1); + progress = start_progress(item->string, total); + } else { + die("invalid input: '%s'\n", line.buf); + } + } else if (skip_prefix(line.buf, "progress ", (const char **) &end)) { uint64_t item_count = strtoull(end, &end, 10); if (*end != '\0') die("invalid input: '%s'\n", line.buf); @@ -65,12 +78,14 @@ int cmd__progress(int argc, const char **argv) display_throughput(progress, byte_count); } else if (!strcmp(line.buf, "update")) { progress_test_force_update(); + } else if (!strcmp(line.buf, "stop")) { + stop_progress(&progress); } else { die("invalid input: '%s'\n", line.buf); } } - stop_progress(&progress); strbuf_release(&line); + string_list_clear(&list, 0); return 0; } diff --git a/t/t0500-progress-display.sh b/t/t0500-progress-display.sh index f37cf2eb9c9..27ab4218b01 100755 --- a/t/t0500-progress-display.sh +++ b/t/t0500-progress-display.sh @@ -18,6 +18,7 @@ test_expect_success 'simple progress display' ' EOF cat >in <<-\EOF && + start 0 update progress 1 update @@ -26,8 +27,9 @@ test_expect_success 'simple progress display' ' progress 4 update progress 5 + stop EOF - test-tool progress "Working hard" <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -42,11 +44,13 @@ test_expect_success 'progress display with total' ' EOF cat >in <<-\EOF && + start 3 progress 1 progress 2 progress 3 + stop EOF - test-tool progress --total=3 "Working hard" <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -63,14 +67,14 @@ Working hard.......2.........3.........4.........5.........6: EOF cat >in <<-\EOF && + start 100000 Working hard.......2.........3.........4.........5.........6 progress 100 progress 1000 progress 10000 progress 100000 + stop EOF - test-tool progress --total=100000 \ - "Working hard.......2.........3.........4.........5.........6" \ - <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -89,16 +93,16 @@ Working hard.......2.........3.........4.........5.........6: EOF cat >in <<-\EOF && + start 100000 Working hard.......2.........3.........4.........5.........6 update progress 1 update progress 2 progress 10000 progress 100000 + stop EOF - test-tool progress --total=100000 \ - "Working hard.......2.........3.........4.........5.........6" \ - <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -117,14 +121,14 @@ Working hard.......2.........3.........4.........5.........6: EOF cat >in <<-\EOF && + start 100000 Working hard.......2.........3.........4.........5.........6 progress 25000 progress 50000 progress 75000 progress 100000 + stop EOF - test-tool progress --total=100000 \ - "Working hard.......2.........3.........4.........5.........6" \ - <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -141,14 +145,14 @@ Working hard.......2.........3.........4.........5.........6.........7.........: EOF cat >in <<-\EOF && + start 100000 Working hard.......2.........3.........4.........5.........6.........7......... progress 25000 progress 50000 progress 75000 progress 100000 + stop EOF - test-tool progress --total=100000 \ - "Working hard.......2.........3.........4.........5.........6.........7........." \ - <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -165,12 +169,14 @@ test_expect_success 'progress shortens - crazy caller' ' EOF cat >in <<-\EOF && + start 1000 progress 100 progress 200 progress 1 progress 1000 + stop EOF - test-tool progress --total=1000 "Working hard" <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -186,6 +192,7 @@ test_expect_success 'progress display with throughput' ' EOF cat >in <<-\EOF && + start 0 throughput 102400 1000 update progress 10 @@ -198,8 +205,9 @@ test_expect_success 'progress display with throughput' ' throughput 409600 4000 update progress 40 + stop EOF - test-tool progress "Working hard" <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -215,6 +223,7 @@ test_expect_success 'progress display with throughput and total' ' EOF cat >in <<-\EOF && + start 40 throughput 102400 1000 progress 10 throughput 204800 2000 @@ -223,8 +232,9 @@ test_expect_success 'progress display with throughput and total' ' progress 30 throughput 409600 4000 progress 40 + stop EOF - test-tool progress --total=40 "Working hard" <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -240,6 +250,7 @@ test_expect_success 'cover up after throughput shortens' ' EOF cat >in <<-\EOF && + start 0 throughput 409600 1000 update progress 1 @@ -252,8 +263,9 @@ test_expect_success 'cover up after throughput shortens' ' throughput 1638400 4000 update progress 4 + stop EOF - test-tool progress "Working hard" <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -268,6 +280,7 @@ test_expect_success 'cover up after throughput shortens a lot' ' EOF cat >in <<-\EOF && + start 0 throughput 1 1000 update progress 1 @@ -277,8 +290,9 @@ test_expect_success 'cover up after throughput shortens a lot' ' throughput 3145728 3000 update progress 3 + stop EOF - test-tool progress "Working hard" <in 2>stderr && + test-tool progress <in 2>stderr && show_cr <stderr >out && test_cmp expect out @@ -286,6 +300,7 @@ test_expect_success 'cover up after throughput shortens a lot' ' test_expect_success 'progress generates traces' ' cat >in <<-\EOF && + start 40 throughput 102400 1000 update progress 10 @@ -298,10 +313,11 @@ test_expect_success 'progress generates traces' ' throughput 409600 4000 update progress 40 + stop EOF - GIT_TRACE2_EVENT="$(pwd)/trace.event" test-tool progress --total=40 \ - "Working hard" <in 2>stderr && + GIT_TRACE2_EVENT="$(pwd)/trace.event" test-tool progress \ + <in 2>stderr && # t0212/parse_events.perl intentionally omits regions and data. test_region progress "Working hard" trace.event &&
Change the usage of the "test-tool progress" introduced in 2bb74b53a49 (Test the progress display, 2019-09-16) to take command like "start" and "stop" on stdin, instead of running them implicitly. This makes for tests that are easier to read, since the recipe will mirror the API usage, and allows for easily testing invalid usage that would yield (or should yield) a BUG(), e.g. providing two "start" calls in a row. A subsequent commit will add such tests. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/helper/test-progress.c | 37 ++++++++++++++++------- t/t0500-progress-display.sh | 58 +++++++++++++++++++++++-------------- 2 files changed, 63 insertions(+), 32 deletions(-)