Re: [PATCH v3 02/10] trailer: add unit tests for trailer iterator
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 26, 2024, 16:20 UTC
- Message-ID
- <xmqqle50ayeh.fsf@gitster.g>
- In-Reply-To
- <CAP8UFD0ZHpo7US6dx_WK6F_1JYsBdHunBaW86qM1CzOKaPC-aA@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
Show 10 quoted lines
> On Fri, Apr 26, 2024 at 2:26 AM Linus Arver via GitGitGadget > <gitgitgadget@gmail.com> wrote: >> >> 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/
Right.
Show 22 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[] = {
>
> ...
>
>> + };
>> +
>> + 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.I am not sure if I agree. In the array of struct, being able to identify each array item with its .name component makes quite a lot of sense, especially when the .name member is not really part of the data used in tests but is used as an identifier for the tuple made of other members (i.e., <msg, num_expected_trailers> in this case).
The TEST() macro is unable to take "name" as an early parameter than others due to how it wants to create the identifying string (i.e., doing an equivalent of strfmt() on tc[i].name), but reordering the struct members to match the peculiar order the members are used smells like a tail wagging a dog.
Show 11 quoted lines
>
>> + }
>> +}
>> +
>> +int cmd_main(int argc, const char **argv)
>> +{
>> + run_t_trailer_iterator();
>> + return test_done();
>> +}
>
> LGTM otherwise.Thanks.