Message-ID: <CA+GJov4=9T0io-7a=zO=gmabS+93=4EOtABYosz=Aq2Nj-riRw@mail.gmail.com>
Date:   Tue, 18 Jul 2023 16:40:19 -0400
From:   Rae Moar <rmoar@...gle.com>
To:     David Gow <davidgow@...gle.com>
Cc:     shuah@...nel.org, dlatypov@...gle.com, brendan.higgins@...ux.dev,
        linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com,
        linux-kernel@...r.kernel.org, keescook@...omium.org,
        linux-hardening@...r.kernel.org, jstultz@...gle.com,
        tglx@...utronix.de, sboyd@...nel.org
Subject: Re: [RFC v2 4/9] kunit: Add ability to filter attributes

On Tue, Jul 18, 2023 at 3:39 AM David Gow <davidgow@...gle.com> wrote:
>
> On Sat, 8 Jul 2023 at 05:10, Rae Moar <rmoar@...gle.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="speed>slow"
> >
> > Operations include: >, <, >=, <=, !=, and =. These operations will 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="speed=slow, module!=example"
> >
> > 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=skip".
> >
> > Note the default settings for running tests remains unfiltered.
> >
> > Finally, add "filter" methods for the speed and module attributes to parse
> > and compare attribute values.
> >
> > Note this filtering functionality will be added to kunit.py in the next
> > patch.
> >
> > Signed-off-by: Rae Moar <rmoar@...gle.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 with
> >   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 *err);
> > +
> > +/*
> > + * Returns a copy of the suite containing only tests that pass the filter.
> > + */
> > +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[] = "<>!=";
> > +
> > +/*
> > + * 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, "<=", 2))
> > +               return (val <= input);
> > +       else if (!strncmp(op, ">=", 2))
> > +               return (val >= input);
> > +       else if (!strncmp(op, "!=", 2))
> > +               return (val != input);
> > +       else if (!strncmp(op, ">", 1))
> > +               return (val > input);
> > +       else if (!strncmp(op, "<", 1))
> > +               return (val < input);
> > +       else if (!strncmp(op, "=", 1))
> > +               return (val == input);
> > +       *err = -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;
> > +}

Powered by blists - more mailing lists