Subject: Re: [PATCH 3/9] test-mergesort: add test subcommand

From: Junio C Hamano <[email-masked]>
Date: Fri, 01 Oct 2021 13:26:18 -0700

René Scharfe <[email-masked]> writes:

> +static void dist_rand(int *arr, int n, int m)
> +{
> +	int i;
> +	for (i = 0; i < n; i++)
> +		arr[i] = rand() % m;
> +}
> ...
> +static void dist_shuffle(int *arr, int n, int m)
> +{
> +	int i, j, k;
> +	for (i = j = 0, k = 1; i < n; i++)
> +		arr[i] = (rand() % m) ? (j += 2) : (k += 2);
> +}

I briefly wondered if we want to seed the rand() in some way to make
the tests reproducible, but we'd need to ship our own rand() if we
wanted to go that route, which would probably be too much.

> +static int test(const struct dist *dist, const struct mode *mode, int n, int m)
> +{
> +...
> +	for (i = 0, curr = list; i < n && curr; i++, curr = curr->next) {
> +		if (arr[i] != curr->value)
> +			is_sorted = 0;
> +		if (curr->next && curr->value == curr->next->value &&
> +		    curr->rank >= curr->next->rank)
> +			is_stable = 0;
> +	}
> +	if (i < n) {
> +		verdict = "too short";
> +	} else if (curr) {
> +		verdict = "too long";
> +	} else if (!is_sorted) {
> +		verdict = "not sorted";
> +	} else if (!is_stable) {
> +		verdict = "unstable";
> +	} else {
> +		verdict = "OK";
> +		result = 0;
> +	}
> +
> +	printf("%-9s %-16s %8d %8d %8d %8d %8d %s\n",
> +	       dist->name, mode->name, n, m, stats.get_next, stats.set_next,
> +	       stats.compare, verdict);
> +
> +	clear_numbers(list);
> +	free(arr);
> +
> +	return result;
> +}

Nice.

>  int cmd__mergesort(int argc, const char **argv)
>  {
>  	if (argc == 2 && !strcmp(argv[1], "sort"))
>  		return sort_stdin();
> -	usage("test-tool mergesort sort");
> +	if (argc > 1 && !strcmp(argv[1], "test"))
> +		return run_tests(argc - 2, argv + 2);
> +	fprintf(stderr, "usage: test-tool mergesort sort\n");
> +	fprintf(stderr, "   or: test-tool mergesort test [<n>...]\n");
> +	return 129;

If you can live with OPT_CMDMODE to implement sort/test subcommands,
you'd get to have parse_options() to do the usage for you, I think.
I am not sure if it is worth it, as t/helper/ is not end-user
facing.