Re: [PATCH v2 1/4] lib/test: Introduce cpumask KUnit test suite

From: Sander Vanheule
Date: Sun Jun 05 2022 - 02:23:10 EST


Hi Yury,

On Sat, 2022-06-04 at 12:31 -0700, Yury Norov wrote:
> On Sat, Jun 04, 2022 at 07:15:56PM +0200, Sander Vanheule wrote:
> > Add a basic suite of tests for cpumask, providing some tests for empty
> > and completely filled cpumasks.
> >
> > Signed-off-by: Sander Vanheule <sander@xxxxxxxxxxxxx>
>
> The test must go after fix, so that it's doesn't cause regressions
> while bisecting.

OK, I'll change the order of the patches.

>
> > ---
> >  lib/Kconfig.debug  |   9 ++++
> >  lib/Makefile       |   1 +
> >  lib/test_cpumask.c | 115 +++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 125 insertions(+)
> >  create mode 100644 lib/test_cpumask.c
>
> [..]
>
> > +#define FOR_EACH_ITER_EQ(_test, _iter, _expect, _loop)         \
> > +       do {                                                    \
> > +               (_iter) = 0;                                    \
> > +               _loop                                           \
> > +                       (_iter)++;                              \
> > +               KUNIT_EXPECT_EQ((_test), (_expect), (_iter));   \
> > +       } while (0)
>
> This one is harder to use than it should be.

Perhaps I tried to hard to make one macro to cover all the cases.

> Maybe like this? (not tested,
> just an idea)
>
> #define TEST_FOR_EACH_CPU_EQ(test, mask)                                \
>         do {                                                            \
>                 cpumask_t *m = (mask);                                  \
>                 int iter = 0, cpu;                                      \
>                 for_each_cpu(cpu, m)                                    \
>                         iter++;                                         \
>                 KUNIT_EXPECT_EQ((test), cpumask_weight(m), iter);       \
>         } while (0)
>
> static void test_cpumask_iterators(struct kunit *test)
> {
>         TEST_FOR_EACH_CPU(test, &mask_empty);
>         ...
> }
>  
> Similarly for NOT and WRAP.

Thanks for the suggestion, I forgot we don't have to declare all variables at the start of the
function. I'll send an update with a few more specific macros.

Best,
Sander