On Tue, 28 Mar 2023 at 20:54, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
>
> Hi David,
>
> Thanks for the quick reply!
>
> > "established process" is probably overselling it, but we're more than
> > happy to accept improvements and additions to KUnit.
>
> Yeah I guess I was expecting that. Was more wondering if you had
> anything in mind other than sending it to kunit-dev/linux-kselftest
> lists. I'll assume no for now :-)
Those lists (plus the lists / maintainers of anything the feature is
going to heavily interact with) are the right spot. There's also
#kunit on oftc, or feel free to reach out to Brendan or I to try to
sort out a video meeting if you'd rather something more real-time.
But the lists are probably best. :-)
>
> > > For example, we have this macro that makes KUNIT_ARRAY_PARAM easier by
> > > letting you just declare an array of test cases:
> > >
> > > /* Similar to KUNIT_ARRAY_PARAM, but avoiding an extra function */
> > > #define KUNIT_ARRAY_PARAM_DESC(name, array, desc_member) \
> > > static const void *name##_gen_params(const void *prev, char *desc) \
> > > { \
> > > typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : (array); \
> > > if (__next - (array) < ARRAY_SIZE((array))) { \
> > > strscpy(desc, __next->desc_member, KUNIT_PARAM_DESC_SIZE); \
> > > return __next; \
> > > } \
> > > return NULL; \
> > > }
> > >
> >
> > Very neat! I'm more than happy to see this added.
>
> Credits to Benjamin, FWIW. We'll send a patch.
>
> > We're definitely in favour of adding these sorts of helpers. Thus far,
> > these have mostly lived alongside the tests or subsystem being tested,
> > but if they're widely useful then they can sit alongside the KUnit
> > code.
> >
> > My personal feeling is that it's better to have these sorts of
> > subsystem-specific helpers be written and maintained as part of the
> > subsystems (like the tests themselves), largely because that's where
> > the subsystem expertise lies, but we're definitely happy to review any
> > such patches to make sure they fit into the KUnit infrastructure well.
>
> Right, that all makes sense. But ...
>
> > (And, of course, if something spans several subsystems, then lib/kunit
> > may be the best place to keep it.)
>
> Exactly. Even for wifi, being split between cfg80211 and mac80211 makes
> things harder than they should be and less clean if it's part of
> cfg80211 and then somehow exposed to other bits, and then possibly
> refactored into somewhere else in net if that starts using it.
>
> So I think in the case of test-managed SKBs, it would make sense to put
> it into lib/kunit.
>
Yeah, my only other thought is 'somewhere in net', but you'd know net
better than me. :-)
> > Yeah, it's a combination of "no-one has needed it yet", "no-one
> > working on KUnit understands it well enough", and "we haven't had the
> > time yet". We are a bit hesitant to add these features without having
> > tests that use them, too: often things will be coded by hand for one
> > or two tests, and only then refactored out into a common helper.
>
> Right, we're already at that place adding tests to cfg80211 and mac80211
> now, basically.
>
> > There are a few other similar helpers being worked on at the moment,
> > mostly around providing test-managed "struct device"s, so this is
> > definitely an active field of development.
>
> Great.
>
> I think we'll just send a couple of patches to start out with, for SKBs
> and the test case array macro above.
>
Sounds good!
Cheers,
-- David
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
