Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230707210947.1208717-1-rmoar@google.com> <20230707210947.1208717-5-rmoar@google.com>
 <CABVgOS=X+C2g6_q-=3qdzp1YMPjazq8F2C5ZqvFhub+t3_sUdQ@mail.gmail.com>
In-Reply-To: <CABVgOS=X+C2g6_q-=3qdzp1YMPjazq8F2C5ZqvFhub+t3_sUdQ@mail.gmail.com>
From:   Rae Moar <rmoar@google.com>
Date:   Tue, 18 Jul 2023 16:40:19 -0400
Message-ID: <CA+GJov4=9T0io-7a=zO=gmabS+93=4EOtABYosz=Aq2Nj-riRw@mail.gmail.com>
Subject: Re: [RFC v2 4/9] kunit: Add ability to filter attributes
To:     David Gow <davidgow@google.com>
Cc:     shuah@kernel.org, dlatypov@google.com, brendan.higgins@linux.dev,
        linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com,
        linux-kernel@vger.kernel.org, keescook@chromium.org,
        linux-hardening@vger.kernel.org, jstultz@google.com,
        tglx@linutronix.de, sboyd@kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Jul 18, 2023 at 3:39=E2=80=AFAM David Gow <davidgow@google.com> wro=
te:
>
> On Sat, 8 Jul 2023 at 05:10, Rae Moar <rmoar@google.com> wrote:
> >
> > Add filtering of test attributes. Users can filter tests using the
> > module_param called "filter".
> >
> > Filters are imputed in the format: <attribute_name><operation><value>
> >
> > Example: kunit.filter=3D"speed>slow"
> >
> > Operations include: >, <, >=3D, <=3D, !=3D, and =3D. These operations w=
ill act the
> > same for attributes of the same type but may not between types.
> >
> > Note multiple filters can be inputted by separating them with a comma.
> > Example: kunit.filter=3D"speed=3Dslow, module!=3Dexample"
> >
> > Since both suites and test cases can have attributes, there may be
> > conflicts. The process of filtering follows these rules:
> > - Filtering always operates at a per-test level.
> > - If a test has an attribute set, then the test's value is filtered on.
> > - Otherwise, the value falls back to the suite's value.
> > - If neither are set, the attribute has a global "default" value, which
> >   is used.
> >
> > Filtered tests will not be run or show in output. The tests can instead=
 be
> > skipped using the configurable option "kunit.filter_action=3Dskip".
> >
> > Note the default settings for running tests remains unfiltered.
> >
> > Finally, add "filter" methods for the speed and module attributes to pa=
rse
> > and compare attribute values.
> >
> > Note this filtering functionality will be added to kunit.py in the next
> > patch.
> >
> > Signed-off-by: Rae Moar <rmoar@google.com>
> > ---
> >
> > Changes since v1:
> > - Change method for inputting filters to allow for spaces in filtering
> >   values
> > - Add option to skip filtered tests instead of not run or show them wit=
h
> >   the --filter_skip flag
> >
> >  include/kunit/attributes.h |  31 +++++
> >  lib/kunit/attributes.c     | 256 +++++++++++++++++++++++++++++++++++++
> >  lib/kunit/executor.c       |  94 +++++++++++---
> >  lib/kunit/executor_test.c  |  12 +-
> >  lib/kunit/test.c           |  10 +-
> >  5 files changed, 375 insertions(+), 28 deletions(-)
> >
> > diff --git a/include/kunit/attributes.h b/include/kunit/attributes.h
> > index 9fcd184cce36..bc76a0b786d2 100644
> > --- a/include/kunit/attributes.h
> > +++ b/include/kunit/attributes.h
> > @@ -9,6 +9,20 @@
> >  #ifndef _KUNIT_ATTRIBUTES_H
> >  #define _KUNIT_ATTRIBUTES_H
> >
> > +/*
> > + * struct kunit_attr_filter - representation of attributes filter with=
 the
> > + * attribute object and string input
> > + */
> > +struct kunit_attr_filter {
> > +       struct kunit_attr *attr;
> > +       char *input;
> > +};
> > +
> > +/*
> > + * Returns the name of the filter's attribute.
> > + */
> > +const char *kunit_attr_filter_name(struct kunit_attr_filter filter);
> > +
> >  /*
> >   * Print all test attributes for a test case or suite.
> >   * Output format for test cases: "# <test_name>.<attribute>: <value>"
> > @@ -16,4 +30,21 @@
> >   */
> >  void kunit_print_attr(void *test_or_suite, bool is_test, unsigned int =
test_level);
> >
> > +/*
> > + * Returns the number of fitlers in input.
> > + */
> > +int kunit_get_filter_count(char *input);
> > +
> > +/*
> > + * Parse attributes filter input and return an objects containing the
> > + * attribute object and the string input of the next filter.
> > + */
> > +struct kunit_attr_filter kunit_next_attr_filter(char **filters, int *e=
rr);
> > +
> > +/*
> > + * Returns a copy of the suite containing only tests that pass the fil=
ter.
> > + */
> > +struct kunit_suite *kunit_filter_attr_tests(const struct kunit_suite *=
const suite,
> > +               struct kunit_attr_filter filter, char *action, int *err=
);
> > +
> >  #endif /* _KUNIT_ATTRIBUTES_H */
> > diff --git a/lib/kunit/attributes.c b/lib/kunit/attributes.c
> > index 43dcb5de8b8f..91cbcacafba9 100644
> > --- a/lib/kunit/attributes.c
> > +++ b/lib/kunit/attributes.c
> > @@ -67,6 +67,104 @@ static const char *attr_string_to_string(void *attr=
, bool *to_free)
> >         return (char *) attr;
> >  }
> >
> > +/* Filter Methods */
> > +
> > +static const char op_list[] =3D "<>!=3D";
> > +
> > +/*
> > + * Returns whether the inputted integer value matches the filter given
> > + * by the operation string and inputted integer.
> > + */
> > +static int int_filter(long val, const char *op, int input, int *err)
> > +{
> > +       if (!strncmp(op, "<=3D", 2))
> > +               return (val <=3D input);
> > +       else if (!strncmp(op, ">=3D", 2))
> > +               return (val >=3D input);
> > +       else if (!strncmp(op, "!=3D", 2))
> > +               return (val !=3D input);
> > +       else if (!strncmp(op, ">", 1))
> > +               return (val > input);
> > +       else if (!strncmp(op, "<", 1))
> > +               return (val < input);
> > +       else if (!strncmp(op, "=3D", 1))
> > +               return (val =3D=3D input);
> > +       *err =3D -EINVAL;
> > +       pr_err("kunit executor: invalid filter operation: %s\n", op);
>
> More a nitpick for the kunit.py patch, but I'd love to have this shown
> to the user as an error when run under kunit.py. It's very annoying to
> miss this and only get a "no tests run" error (or worse, unfiltered
> results) back.
>

I agree. I would love to change this. I am currently looking into how
easy this is to change.

It would be nice to at least see a general KUnit executor error in the
output instead of the "no tests run!" error.

>
> > +       return false;
> > +}