Re: [PATCH v2 7/8] u-string-list: move "filter string" test to "u-string-list.c"
- From
shejialuo <shejialuo@gmail.com>
- Date
- May 26, 2025, 14:33 UTC
- Message-ID
- <aDR71ftk0KJVTA62@ArchLinux>
- In-Reply-To
- <aCrbLoElTJJq6jwZ@pks.im>
On Mon, May 19, 2025 at 09:18:06AM +0200, Patrick Steinhardt wrote:
Show 44 quoted lines
> On Sun, May 18, 2025 at 11:58:09PM +0800, shejialuo wrote:
> > diff --git a/t/unit-tests/u-string-list.c b/t/unit-tests/u-string-list.c
> > index e4b8e38fb8..be2bb5f103 100644
> > --- a/t/unit-tests/u-string-list.c
> > +++ b/t/unit-tests/u-string-list.c
> > @@ -103,3 +115,57 @@ void test_string_list__split_in_place(void)
> >
> > t_string_list_clear(&list, 0);
> > }
> > +
> > +static int prefix_cb(struct string_list_item *item, void *cb_data)
> > +{
> > + const char *prefix = (const char *)cb_data;
> > + return starts_with(item->string, prefix);
> > +}
> > +
> > +static void t_string_list_filter(struct string_list *list,
> > + string_list_each_func_t want, void *cb_data, ...)
> > +{
> > + struct string_list expected_strings = STRING_LIST_INIT_DUP;
> > + va_list ap;
> > +
> > + va_start(ap, cb_data);
> > + t_vcreate_string_list_dup(&expected_strings, 0, ap);
> > + va_end(ap);
> > +
> > + filter_string_list(list, 0, want, cb_data);
> > + t_string_list_equal(list, &expected_strings);
> > +
> > + string_list_clear(&expected_strings, 0);
> > +}
> > +
> > +void test_string_list__filter(void)
> > +{
> > + struct string_list list = STRING_LIST_INIT_DUP;
> > + const char *prefix = "y";
> > +
> > + t_create_string_list_dup(&list, 0, NULL);
>
> Okay, here we have to manually create a list because you cannot pass two
> vararg lists to `t_string_list_filter()`. It's not the prettiest and
> feels a bit repetitive, but the alternatives I can think of aren't a lot
> nicer, either.
> Yes, I am not very satisfied with this either. In the original shell script test, it uses "string_list_split" to create a new "string-list". However, this way is worse than the current, too hacky.
Show 6 quoted lines
> > + t_string_list_filter(&list, prefix_cb, (void*)prefix, NULL); > > Both the `prefix_cb` and `prefix` are the same across all function > calls, so I wondered whether we might want to move them into the > wrapper function directly. >
What if we want to use the different "prefix_cb" and "prefix"? I somehow think we should make "t_string_list_filter" more flexible.
> The `void *` casts are also all unnecessary. >
Right, I will improve this in the next version.
> Patrick