From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 09 Oct 2026 11:52:38 GMT
Subject: Re: [PATCH v2 2/3] mergesort: move sorting tests to the unit-test framework
Message-ID: <asjVhlTj3MqHcGm4@pks.im>
In-Reply-To: <0429552774367ddcc3c2fda78e09a83650ccfa02.1791365181.git.dilsheddilu123@gmail.com>

On Wed, Oct 07, 2026 at 07:20:24PM +0530, Muhammed Dilshad A wrote:
> The mergesort certification checks exercise C code directly, so they do
> not need a shell test and test-tool command. Move their distributions and
> transformations to Clar, retaining the sorted-value, stability and list
> length checks. Add small cases for both list sort macros and debug hooks.
> 
> Keep node storage available to the cleanup fixture and bound validation
> so a failed assertion can release it without walking a broken list.

Wat...? I have no idea what this means.

I would appreciate it if you would read through the AI generated
messages and ask yourself whether a normal human being would understand
what was being generated. In general, we ask you to fully vet all of the
stuff that is being generated, understand it and convert it into a form
that normal human beings understand.

A commit message is _your_ chance to demonstrate that you understand
what you're contributing. If it's this obviously AI generated it raises
a huge red flag as I will immediately assume that you haven't read any
of the code it wrote.

> diff --git a/t/unit-tests/u-mergesort.c b/t/unit-tests/u-mergesort.c
> new file mode 100644
> index 0000000000..e621c9ec21
> --- /dev/null
> +++ b/t/unit-tests/u-mergesort.c
> @@ -0,0 +1,369 @@
> +#include "unit-test.h"
> +#include "mergesort.h"
> +
> +static uint32_t minstd_rand(uint32_t *state)

All of these distributions are kinda cute. But is it really required to
test the merge sort with half a dozen different distributions? I dunno,
color me sceptical.

That being said, you just retain the old status quo, so okay.

[snip]
> +#define DIST(name) { #name, dist_##name }
> +
> +static struct dist {
> +	const char *name;
> +	void (*fn)(int *arr, int n, int m);
> +} dist[] = {
> +	DIST(sawtooth),
> +	DIST(rand),
> +	DIST(stagger),
> +	DIST(plateau),
> +	DIST(shuffle),
> +};

This also feels quite overengineered now for the unit test infra.

[snip]
> +#define MODE(name) { #name, mode_##name }
> +
> +static struct mode {
> +	const char *name;
> +	void (*fn)(int *arr, int n);
> +} mode[] = {
> +	MODE(copy),
> +	MODE(reverse),
> +	MODE(reverse_1st_half),
> +	MODE(reverse_2nd_half),
> +	MODE(sort),
> +	MODE(dither),
> +	MODE(unriffle),
> +	MODE(unriffle_skewed),
> +};

Same. All of this is way too overengineered. It probably was useful at
one point in time to show performance with these different modes and
distributions. But even with the performance test we don't use those at
all anymore, so it just feels needlessly complex by now.

Patrick