Messages in this thread | | | From | Brendan Higgins <> | Date | Mon, 15 Jul 2019 14:25:45 -0700 | Subject | Re: [PATCH v9 01/18] kunit: test: add KUnit test runner core |
| |
On Mon, Jul 15, 2019 at 1:10 PM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Brendan Higgins (2019-07-12 01:17:27) > > Add core facilities for defining unit tests; this provides a common way > > to define test cases, functions that execute code which is under test > > and determine whether the code under test behaves as expected; this also > > provides a way to group together related test cases in test suites (here > > we call them test_modules). > > > > Just define test cases and how to execute them for now; setting > > expectations on code will be defined later. > > > > Signed-off-by: Brendan Higgins <brendanhiggins@google.com> > > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Reviewed-by: Logan Gunthorpe <logang@deltatee.com> > > Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > > Reviewed-by: Stephen Boyd <sboyd@kernel.org> > > Minor nits below. > > > diff --git a/kunit/test.c b/kunit/test.c > > new file mode 100644 > > index 0000000000000..571e4c65deb5c > > --- /dev/null > > +++ b/kunit/test.c > > @@ -0,0 +1,189 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Base unit test (KUnit) API. > > + * > > + * Copyright (C) 2019, Google LLC. > > + * Author: Brendan Higgins <brendanhiggins@google.com> > > + */ > > + > > +#include <linux/kernel.h> > > +#include <kunit/test.h> > > + > > +static void kunit_set_failure(struct kunit *test) > > +{ > > + WRITE_ONCE(test->success, false); > > +} > > + > [...] > > + > > +void kunit_init_test(struct kunit *test, const char *name) > > +{ > > + test->name = name; > > + test->success = true; > > +} > > + > > +/* > > + * Performs all logic to run a test case. > > + */ > > +static void kunit_run_case(struct kunit_suite *suite, > > + struct kunit_case *test_case) > > +{ > > + struct kunit test; > > + int ret = 0; > > + > > + kunit_init_test(&test, test_case->name); > > + > > + if (suite->init) { > > + ret = suite->init(&test); > > Can you push the ret definition into this if scope? That way we can > avoid default initialize to 0 for it.
Sure! I would actually prefer that from a cosmetic standpoint. I just thought that mixing declarations and code was against the style guide.
> > + if (ret) { > > + kunit_err(&test, "failed to initialize: %d\n", ret); > > + kunit_set_failure(&test); > > Do we need to 'test_case->success = test.success' here too? Or is the > test failure extracted somewhere else?
Er, yes. That's kind of embarrassing. Good catch.
> > + return; > > + } > > + } > > + > > + test_case->run_case(&test); > > + > > + if (suite->exit) > > + suite->exit(&test); > > + > > + test_case->success = test.success;
Thanks!
| |