On Tue, Dec 28, 2021 at 04:18:59PM +0100, Æ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@xxxxxxxxx>
> ---
> t/helper/test-progress.c | 46 ++++++++++++++++++++++-------
> t/t0500-progress-display.sh | 58 +++++++++++++++++++++++--------------
> 2 files changed, 72 insertions(+), 32 deletions(-)
>
> diff --git a/t/helper/test-progress.c b/t/helper/test-progress.c
> index 50fd3be3dad..becc163375f 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,52 @@
> #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 titles = STRING_LIST_INIT_DUP;
> 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);
> + const char *title;
> + const char *str;
> +
> + /*
> + * We can't use "end + 1" as an argument to
> + * start_progress(), it doesn't xstrdup() its
> + * "title" argument. We need to hold onto a
> + * valid "char *" for it until the end.
> + */
> + if (!*end)
> + title = default_title;
> + else if (*end == ' ')
> + title = string_list_insert(&titles, end + 1)->string;
> + else
> + die("invalid input: '%s'\n", line.buf);
> +
> + str = title ? title : default_title;
I don't think title is ever NULL, so we should be able to elide this variable.
(Did you want to fall back to the default title when the input is "start "?)
> + progress = start_progress(str, total);
> + } 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);
