Subject: Re: [PATCH v3 03/10] progress.c tests: make start/stop verbs on stdin

From: Emily Shaffer <[email-masked]>
Date: Thu, 21 Oct 2021 20:34:04 -0700

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 <[email-masked]>
> ---
>  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