On Mon, Jul 8, 2019 at 11:08 AM Brendan Higgins brendanhiggins@google.com wrote:
On Fri, Jul 5, 2019 at 1:15 PM Luis Chamberlain mcgrof@kernel.org wrote:
On Wed, Jul 03, 2019 at 05:35:58PM -0700, Brendan Higgins wrote:
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
But a nitpick below, I think that can be fixed later with a follow up patch.
+/**
- struct kunit - represents a running instance of a test.
- @priv: for user to store arbitrary data. Commonly used to pass data created
- in the init function (see &struct kunit_suite).
- Used to store information about the current context under which the test is
- running. Most of this data is private and should only be accessed indirectly
- via public functions; the one exception is @priv which can be used by the
- test writer to store arbitrary data.
- A brief note on locking:
- First off, we need to lock because in certain cases a user may want to use an
- expectation in a thread other than the thread that the test case is running
- in.
This as a prefix to the struct without a lock seems odd. It would be clearer I think if you'd explain here what locking mechanism we decided to use and why it suffices today.
Whoops, sorry this should have been in the next patch. Will fix.
Err..no, this shouldn't be here at all. Sorry about that. I just need to rewrite the comment.
+/**
- suite_test() - used to register a &struct kunit_suite with KUnit.
You mean kunit_test_suite()?
Yep, sorry about that. Will fix.
- @suite: a statically allocated &struct kunit_suite.
- Registers @suite with the test framework. See &struct kunit_suite for more
- information.
- NOTE: Currently KUnit tests are all run as late_initcalls; this means that
- they cannot test anything where tests must run at a different init phase. One
- significant restriction resulting from this is that KUnit cannot reliably
- test anything that is initialize in the late_init phase.
initialize prior to the late init phase.That is, this is useless to test things running early.
Yeah, I can add that phrasing in.
- TODO(brendanhiggins@google.com): Don't run all KUnit tests as late_initcalls.
- I have some future work planned to dispatch all KUnit tests from the same
- place, and at the very least to do so after everything else is definitely
- initialized.
TODOs are odd to be adding to documentation, this is just not common place practice. The NOTE should suffice for you.
Because it is a kernel doc? Would you usually make a separate non-kernel doc comment for a TODO? I guess that makes sense.
Thanks!