Re: [PATCH v3 02/10] trailer: add unit tests for trailer iterator

From: Christian Couder <christian.couder@gmail.com>
Date: Apr 26, 2024, 14:51 UTC
Message-ID: <CAP8UFD0ZHpo7US6dx_WK6F_1JYsBdHunBaW86qM1CzOKaPC-aA@mail.gmail.com>
In-Reply-To: <4ad0fbbb33cab9d5841689cc5660befe6921d515.1714091170.git.gitgitgadget@gmail.com>

On Fri, Apr 26, 2024 at 2:26 AM Linus Arver via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 6 quoted lines

>
> From: Linus Arver <linusa@google.com>
>
> Test the number of trailers found by the iterator (to be more precise,
> the parsing mechanism which the iterator just walks over) when given
> some some arbitrary log message.

s/some some/some/

Show 6 quoted lines

> We test the iterator because it is a public interface function exposed
> by the trailer API (we generally don't want to test internal
> implementation details which are, unlike the API, subject to drastic
> changes).
>
> Signed-off-by: Linus Arver <linusa@google.com>

Show 7 quoted lines

> +static void run_t_trailer_iterator(void)
> +{
> +       static struct test_cases {
> +               const char *name;
> +               const char *msg;
> +               size_t num_expected_trailers;
> +       } tc[] = {

...

Show 6 quoted lines

> +       };
> +
> +       for (int i = 0; i < sizeof(tc) / sizeof(tc[0]); i++) {
> +               TEST(t_trailer_iterator(tc[i].msg,
> +                                       tc[i].num_expected_trailers),
> +                    "%s", tc[i].name);

Nit: the members of struct test_cases are used in the (msg,
num_expected_trailers, name) order, while they are declared in the
(name, msg, num_expected_trailers) order. I think it would make it a
bit easier to use in struct test_cases the same order in which they
are used in the TEST() macro.

Show 8 quoted lines

> +       }
> +}
> +
> +int cmd_main(int argc, const char **argv)
> +{
> +       run_t_trailer_iterator();
> +       return test_done();
> +}

LGTM otherwise.