Re: [PATCH 1/2] t/unit-tests: match functions signature with trailing code

[Junio C Hamano <gitster@pobox.com>]

Seyi Kuforiji <kuforiji98@gmail.com> writes:

> The `generate-clar-decls.sh` script extracts signatures of test
> functions from our unit tests, which will later get used by the clar to
> automatically wire up tests. The sed command only matches lines that
> ended immediately after `void)`, causing it to miss declarations with
> additional content such as comments or annotations.
>
> Relax the regular expression by making it match lines with trailing data
> after the function signature. This ensures that all valid function
> declarations are captured and formatted as `extern` declarations
> regardless of their formatting style, improving the robustness of the
> script when parsing `$suite` files.
>
> This will be used in subsequent commits to match and capture the
> function signature correctly, regardless of any trailing content.

I am not sure if this is going in the right direction, though.

Especially for things like test suites that are looked at and worked
on only by develoeprs *and* these tools, being uniform and consistent
weighs more than being more flexible.

Let me state it in another way.  How many of the existing test
pieces are picked up by the current pattern, and among them how many
of them would see vast improvements if they are allowed to have
arbitrary garbage after their "I do not take any arguments" function
signature?  Are new tests you are migrating from outside the clar
world lose a lot if they are no longer allowed to have comments
there, or would it be suffice to have the comments before the
functions (which many of our function definition do anyway)?

A quick peek at [PATCH 2/2] tells me that this is not even something
that would make it easier to port the existing tests by allowing
more straight line-by-line copies or something.  The patch splits
many in-line test pieces in the "main" into separate functions, and
it does so in a rather unusual format, e.g.,

  void test_hash__multi_character(void) TEST_HASH_STR("abc",
          "a9993e364706816aba3e25717850c26c9cd0d89d",
          "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad")

where TEST_HASH_STR() expands to the function body that starts with
a "{" and ends with a "}".  It can well be written more like

    void test_hash__multi_character(void)
    {
	TEST_HASH_STR("abc",
        	"a9993e364706816aba3e25717850c26c9cd0d89d",
		"ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad");
    }

and we do not need this step at all if we did so.  Such a construct
would be a lot friendlier to the editors that auto-indent, too.

So, I do not quite see much value in this particular change.

> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Seyi Kuforiji <kuforiji98@gmail.com>
> ---
>  t/unit-tests/generate-clar-decls.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/unit-tests/generate-clar-decls.sh b/t/unit-tests/generate-clar-decls.sh
> index 3b315c64b3..02e45cf0ba 100755
> --- a/t/unit-tests/generate-clar-decls.sh
> +++ b/t/unit-tests/generate-clar-decls.sh
> @@ -14,6 +14,6 @@ do
>  	suite_name=$(basename "$suite")
>  	suite_name=${suite_name%.c}
>  	suite_name=${suite_name#u-}
> -	sed -ne "s/^\(void test_${suite_name}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\)$/extern \1;/p" "$suite" ||
> +	sed -ne "s/^\(void test_${suite_name}__[a-zA-Z_0-9][a-zA-Z_0-9]*(void)\).*/extern \1;/p" "$suite" ||
>  	exit 1
>  done >"$OUTPUT"
> --
> 2.34.1