Re: [PATCH v2 7/8] u-string-list: move "filter string" test to "u-string-list.c"

[shejialuo <shejialuo@gmail.com>]

On Mon, May 19, 2025 at 09:18:06AM +0200, Patrick Steinhardt wrote:
> 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.

> > +	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