2020-01-23 18:49:31

by Alan Maguire

[permalink] [raw]
Subject: [PATCH v2 kunit-next 0/3] kunit: add debugfs representation to show results/run tests

When kunit tests are run on native (i.e. non-UML) environments, the results
of test execution are often intermixed with dmesg output. This patch
series attempts to solve this by providing a debugfs representation
of the results of the last test run, available as

/sys/kernel/debug/kunit/<testsuite>/results

In addition, we provide a way to re-run the tests and show results via

/sys/kernel/debug/kunit/<testsuite>/run

Changes since v1:
- trimmed unneeded include files in lib/kunit/debugfs.c (Greg)
- renamed global debugfs functions to be prefixed with kunit_ (Greg)
- removed error checking for debugfs operations (Greg)

Alan Maguire (3):
kunit: add debugfs /sys/kernel/debug/kunit/<suite>/results display
kunit: add "run" debugfs file to run suites, display results
kunit: update documentation to describe debugfs representation

Documentation/dev-tools/kunit/usage.rst | 19 +++++
include/kunit/test.h | 21 +++--
lib/kunit/Makefile | 3 +-
lib/kunit/debugfs.c | 137 ++++++++++++++++++++++++++++++++
lib/kunit/debugfs.h | 16 ++++
lib/kunit/test.c | 85 +++++++++++++++-----
6 files changed, 254 insertions(+), 27 deletions(-)
create mode 100644 lib/kunit/debugfs.c
create mode 100644 lib/kunit/debugfs.h

--
1.8.3.1


2020-01-23 18:49:34

by Alan Maguire

[permalink] [raw]
Subject: [PATCH v2 kunit-next 2/3] kunit: add "run" debugfs file to run suites, display results

Add /sys/kernel/debug/kunit/<suite>/run file which will run the
specified suite and show results.

Signed-off-by: Alan Maguire <[email protected]>
---
lib/kunit/debugfs.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)

diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
index 578843c..1ea3fbc 100644
--- a/lib/kunit/debugfs.c
+++ b/lib/kunit/debugfs.c
@@ -13,6 +13,7 @@

#define KUNIT_DEBUGFS_ROOT "kunit"
#define KUNIT_DEBUGFS_RESULTS "results"
+#define KUNIT_DEBUGFS_RUN "run"

/*
* Create a debugfs representation of test suites:
@@ -20,6 +21,7 @@
* Path Semantics
* /sys/kernel/debug/kunit/<testsuite>/results Show results of last run for
* testsuite
+ * /sys/kernel/debug/kunit/<testsuite>/run Run testsuite and show results
*
*/

@@ -67,6 +69,18 @@ static int debugfs_print_results(struct seq_file *seq, void *v)
return 0;
}

+/*
+ * /sys/kernel/debug/kunit/<testsuite>/run (re)runs suite and shows all results.
+ */
+static int debugfs_run_print_results(struct seq_file *seq, void *v)
+{
+ struct kunit_suite *suite = (struct kunit_suite *)seq->private;
+
+ kunit_run_tests(suite);
+
+ return debugfs_print_results(seq, v);
+}
+
static int debugfs_release(struct inode *inode, struct file *file)
{
return single_release(inode, file);
@@ -88,6 +102,22 @@ static int debugfs_results_open(struct inode *inode, struct file *file)
.release = debugfs_release,
};

+static int debugfs_run_open(struct inode *inode, struct file *file)
+{
+ struct kunit_suite *suite;
+
+ suite = (struct kunit_suite *)inode->i_private;
+
+ return single_open(file, debugfs_run_print_results, suite);
+}
+
+static const struct file_operations debugfs_run_fops = {
+ .open = debugfs_run_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = debugfs_release,
+};
+
void kunit_debugfs_create_suite(struct kunit_suite *suite)
{
/* First add /sys/kernel/debug/kunit/<testsuite> */
@@ -96,6 +126,9 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite)
debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444,
suite->debugfs,
suite, &debugfs_results_fops);
+ debugfs_create_file(KUNIT_DEBUGFS_RUN, S_IFREG | 0444,
+ suite->debugfs,
+ suite, &debugfs_run_fops);
}

void kunit_debugfs_destroy_suite(struct kunit_suite *suite)
--
1.8.3.1

2020-01-23 18:49:42

by Alan Maguire

[permalink] [raw]
Subject: [PATCH v2 kunit-next 3/3] kunit: update documentation to describe debugfs representation

Documentation should describe debugfs layout and semantics.

Signed-off-by: Alan Maguire <[email protected]>
---
Documentation/dev-tools/kunit/usage.rst | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
index 7cd56a1..b05c843 100644
--- a/Documentation/dev-tools/kunit/usage.rst
+++ b/Documentation/dev-tools/kunit/usage.rst
@@ -590,3 +590,22 @@ able to run one test case per invocation.

.. TODO([email protected]): Add an actual example of an architecture
dependent KUnit test.
+
+KUnit debugfs representation
+============================
+When kunit test suites are initialized, they create an associated directory
+in /sys/kernel/debug/kunit/<test-suite>. The directory contains two files
+
+- results: "cat results" displays results of last test run
+- run: "cat run" runs the test suite and displays the results
+
+Thus to re-run all currently loaded suites and display results, we can do this:
+
+```
+$ cat /sys/kernel/debug/kunit/*/run
+```
+
+The debugfs representation is primarily of use when kunit test suites are
+run in a native environment, either as modules or builtin. Having a way
+to display results like this is valuable as otherwise results can be
+intermixed with other events in dmesg output.
--
1.8.3.1

2020-01-23 22:28:14

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH v2 kunit-next 0/3] kunit: add debugfs representation to show results/run tests

+Luis Chamberlain

On Thu, Jan 23, 2020 at 10:47 AM Alan Maguire <[email protected]> wrote:
>
> When kunit tests are run on native (i.e. non-UML) environments, the results
> of test execution are often intermixed with dmesg output. This patch
> series attempts to solve this by providing a debugfs representation
> of the results of the last test run, available as
>
> /sys/kernel/debug/kunit/<testsuite>/results
>
> In addition, we provide a way to re-run the tests and show results via
>
> /sys/kernel/debug/kunit/<testsuite>/run

Ooo, cool! I like this! I was actually thinking about doing something
similar after talking to either Shuah, or Luis, so this is great! I
think Luis will be interested in this regardless so I cc'ed him.

> Changes since v1:
> - trimmed unneeded include files in lib/kunit/debugfs.c (Greg)
> - renamed global debugfs functions to be prefixed with kunit_ (Greg)
> - removed error checking for debugfs operations (Greg)
>
> Alan Maguire (3):
> kunit: add debugfs /sys/kernel/debug/kunit/<suite>/results display
> kunit: add "run" debugfs file to run suites, display results
> kunit: update documentation to describe debugfs representation
>
> Documentation/dev-tools/kunit/usage.rst | 19 +++++
> include/kunit/test.h | 21 +++--
> lib/kunit/Makefile | 3 +-
> lib/kunit/debugfs.c | 137 ++++++++++++++++++++++++++++++++
> lib/kunit/debugfs.h | 16 ++++
> lib/kunit/test.c | 85 +++++++++++++++-----
> 6 files changed, 254 insertions(+), 27 deletions(-)
> create mode 100644 lib/kunit/debugfs.c
> create mode 100644 lib/kunit/debugfs.h
>
> --
> 1.8.3.1
>

2020-01-31 02:44:34

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH v2 kunit-next 2/3] kunit: add "run" debugfs file to run suites, display results

On Thu, Jan 23, 2020 at 10:47 AM Alan Maguire <[email protected]> wrote:
>
> Add /sys/kernel/debug/kunit/<suite>/run file which will run the
> specified suite and show results.
>
> Signed-off-by: Alan Maguire <[email protected]>

If you don't mind, I would like to see the device tree unit test from
Frank before we accept this patch. I definitely like your approach
here, but this would break with KUnit test cases which depend on
__init code and data. I just figure that it would be easier for us to
solve the __init problem now if we have a working example that uses it
rather than having someone who wants to write a test which depends on
__init having to fix this after the fact. Let me know if this is a
problem for you.

> ---
> lib/kunit/debugfs.c | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
> diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
> index 578843c..1ea3fbc 100644
> --- a/lib/kunit/debugfs.c
> +++ b/lib/kunit/debugfs.c
> @@ -13,6 +13,7 @@
>
> #define KUNIT_DEBUGFS_ROOT "kunit"
> #define KUNIT_DEBUGFS_RESULTS "results"
> +#define KUNIT_DEBUGFS_RUN "run"
>
> /*
> * Create a debugfs representation of test suites:
> @@ -20,6 +21,7 @@
> * Path Semantics
> * /sys/kernel/debug/kunit/<testsuite>/results Show results of last run for
> * testsuite
> + * /sys/kernel/debug/kunit/<testsuite>/run Run testsuite and show results
> *
> */
>
> @@ -67,6 +69,18 @@ static int debugfs_print_results(struct seq_file *seq, void *v)
> return 0;
> }
>
> +/*
> + * /sys/kernel/debug/kunit/<testsuite>/run (re)runs suite and shows all results.
> + */
> +static int debugfs_run_print_results(struct seq_file *seq, void *v)
> +{
> + struct kunit_suite *suite = (struct kunit_suite *)seq->private;
> +
> + kunit_run_tests(suite);
> +
> + return debugfs_print_results(seq, v);
> +}
> +
> static int debugfs_release(struct inode *inode, struct file *file)
> {
> return single_release(inode, file);
> @@ -88,6 +102,22 @@ static int debugfs_results_open(struct inode *inode, struct file *file)
> .release = debugfs_release,
> };
>
> +static int debugfs_run_open(struct inode *inode, struct file *file)
> +{
> + struct kunit_suite *suite;
> +
> + suite = (struct kunit_suite *)inode->i_private;
> +
> + return single_open(file, debugfs_run_print_results, suite);
> +}
> +
> +static const struct file_operations debugfs_run_fops = {
> + .open = debugfs_run_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = debugfs_release,
> +};
> +
> void kunit_debugfs_create_suite(struct kunit_suite *suite)
> {
> /* First add /sys/kernel/debug/kunit/<testsuite> */
> @@ -96,6 +126,9 @@ void kunit_debugfs_create_suite(struct kunit_suite *suite)
> debugfs_create_file(KUNIT_DEBUGFS_RESULTS, S_IFREG | 0444,
> suite->debugfs,
> suite, &debugfs_results_fops);
> + debugfs_create_file(KUNIT_DEBUGFS_RUN, S_IFREG | 0444,
> + suite->debugfs,
> + suite, &debugfs_run_fops);

Should anyone be able to read this? I think I agree since I am of the
opinion that people shouldn't build or load tests into a production
environment, but still I think it should be brought up.

I was actually talking to David the other day and we had the idea that
maybe KUnit should taint the kernel after tests run or after a
failure. Maybe that might communicate to a user that after running
tests the kernel shouldn't be used for production purposes.
(Obviously, I don't expect you to make that change here, the point of
anyone being able to cause tests to run just made me think of it.)
What do you think?

> }
>
> void kunit_debugfs_destroy_suite(struct kunit_suite *suite)
> --
> 1.8.3.1
>

2020-01-31 02:54:12

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH v2 kunit-next 3/3] kunit: update documentation to describe debugfs representation

On Thu, Jan 23, 2020 at 10:47 AM Alan Maguire <[email protected]> wrote:
>
> Documentation should describe debugfs layout and semantics.
>
> Signed-off-by: Alan Maguire <[email protected]>
> ---
> Documentation/dev-tools/kunit/usage.rst | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
> index 7cd56a1..b05c843 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -590,3 +590,22 @@ able to run one test case per invocation.
>
> .. TODO([email protected]): Add an actual example of an architecture
> dependent KUnit test.
> +
> +KUnit debugfs representation
> +============================
> +When kunit test suites are initialized, they create an associated directory
> +in /sys/kernel/debug/kunit/<test-suite>. The directory contains two files
> +
> +- results: "cat results" displays results of last test run
> +- run: "cat run" runs the test suite and displays the results
> +
> +Thus to re-run all currently loaded suites and display results, we can do this:
> +
> +```
> +$ cat /sys/kernel/debug/kunit/*/run
> +```

This should be in a ".. code-block:: bash", see above in this file for
an example.

> +
> +The debugfs representation is primarily of use when kunit test suites are
> +run in a native environment, either as modules or builtin. Having a way
> +to display results like this is valuable as otherwise results can be
> +intermixed with other events in dmesg output.
> --
> 1.8.3.1
>