Re: [PATCH v3 03/10] progress.c tests: make start/stop verbs on stdin
- From
Emily Shaffer <emilyshaffer@google.com>
- Date
- Oct 22, 2021, 03:34 UTC
- Message-ID
- <YXIxLHJa6uzYAQZU@google.com>
- In-Reply-To
- <patch-v3-03.10-045d58d8201-20211013T222329Z-avarab@gmail.com>
On Thu, Oct 14, 2021 at 12:28:19AM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 33 quoted lines
>
> 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?
Show 9 quoted 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