Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-10-07 Thread shuah

On 10/7/19 8:40 AM, Steven Rostedt wrote:

On Sun, 6 Oct 2019 10:18:11 -0700
Linus Torvalds  wrote:


On Sun, Oct 6, 2019 at 9:55 AM Theodore Y. Ts'o  wrote:


Well, one thing we *can* do is if (a) if we can create a kselftest
branch which we know is stable and won't change, and (b) we can get
assurances that Linus *will* accept that branch during the next merge
window, those subsystems which want to use kself test can simply pull
it into their tree.


Yes.

At the same time, I don't think it needs to be even that fancy. Even
if it's not a stable branch that gets shared between different
developers, it would be good to just have people do a "let's try this"
throw-away branch to use the kunit functionality and verify that
"yeah, this is fairly convenient for ext4".

It doesn't have to be merged in that form, but just confirmation that
the infrastructure is helpful before it gets merged would be good.


Can't you just create an ext4 branch that has the kselftest-next branch
in it, that you build upon. And push that after the kunit test is
merged?

In the past I've had to rely on other branches in next, and would just
hold two branches myself. One with everything not dependent on the other
developer's branch, and one with the work that was. At the merge
window, I would either merge the two or just send two pull requests
with the two branches.



I do something similar when I am working on top of a branch that isn't
already in the mainline. In any case, repeating myself

Let's work on top of - it is rebased to 5.4-rc1 and ready for use.

https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=test

Let's use that for kunit work for 5.5. I won't add any kselftest patches
to it and keep it dedicated for kunit work. When tests are ready for
upstream, I can keep adding them to this branch.

thanks,
-- Shuah
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-10-07 Thread shuah

On 10/7/19 2:40 AM, Brendan Higgins wrote:

On Sun, Oct 6, 2019 at 10:18 AM Linus Torvalds
 wrote:


On Sun, Oct 6, 2019 at 9:55 AM Theodore Y. Ts'o  wrote:


Well, one thing we *can* do is if (a) if we can create a kselftest
branch which we know is stable and won't change, and (b) we can get
assurances that Linus *will* accept that branch during the next merge
window, those subsystems which want to use kself test can simply pull
it into their tree.


Yes.

At the same time, I don't think it needs to be even that fancy. Even
if it's not a stable branch that gets shared between different
developers, it would be good to just have people do a "let's try this"
throw-away branch to use the kunit functionality and verify that
"yeah, this is fairly convenient for ext4".

It doesn't have to be merged in that form, but just confirmation that
the infrastructure is helpful before it gets merged would be good.


I thought we already had done this satisfactorily.


Adding a couple more tests will only help in the long run. The idea is
to see can this help


We have one proof-of-concept test in the branch in the kselftest repo
(proc sysctl test) that went out in the pull request, and we also had
some other tests that were not in the pull request (there is the ext4
timestamp stuff mentioned above, and we also had one against the list
data structure), which we were planning on sending out for review once
Shuah's pull request was accepted. I know the apparmor people also
wrote some tests that they said were useful; however, I have not
coordinated with them on upstreaming their tests. I know of some other
people who are using it, but I don't think the tests are as far along
for upstreaming.



Maybe that is a good start. To get the tests that are already in use
and get them in shape for upstream.


The point is: I thought we had plenty of signal that KUnit would be
useful to have merged into the mainline kernel. I thought the only
reason it was rejected for 5.4 was due to the directory name issue
combined with bad timing.



That is probably the initial thought. However, it makes perfect sense
to add a couple of tests in. We have a few weeks anyway and it gives
us more confidence on kunit.

I already have a branch that is in linux-next and it just has kunit in
it and I will rebase it to 5.4-rc1.

https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/log/?h=test

Let's use that for kunit work for 5.5. I won't add any kselftest patches
to it and keep it dedicated for kunit work. When tests are ready for
upstream, I can keep adding them to this branch.

thanks,
-- Shuah



___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-10-04 Thread shuah

On 10/4/19 6:33 PM, Brendan Higgins wrote:

On Fri, Oct 4, 2019 at 4:57 PM shuah  wrote:


On 10/4/19 5:52 PM, Brendan Higgins wrote:

On Fri, Oct 4, 2019 at 4:30 PM Theodore Y. Ts'o  wrote:


On Fri, Oct 04, 2019 at 04:47:09PM -0600, shuah wrote:

However, if I encourage arbitrary tests/improvements into my KUnit
branch, it further diverges away from torvalds/master, and is more
likely that there will be a merge conflict or issue that is not related
to the core KUnit changes that will cause the whole thing to be
rejected again in v5.5.


The idea is that the new development will happen based on kunit in
linux-kselftest next. It will work just fine. As we accepts patches,
they will go on top of kunit that is in linux-next now.


I don't see how this would work.  If I add unit tests to ext4, they
would be in fs/ext4.  And to the extent that I need to add test mocks
to allow the unit tests to work, they will involve changes to existing
files in fs/ext4.  I can't put them in the ext4.git tree without
pulling in the kunit changes into the ext4 git tree.  And if they ext4
unit tests land in the kunit tree, it would be a receipe for large
numbers of merge conflicts.


That's where I was originally coming from.

So here's a dumb idea: what if we merged KUnit through the ext4 tree?
I imagine that could potentially get very confusing when we go back to
sending changes in through the kselftest tree, but at least it means
that ext4 can use it in the meantime, which means that it at least
gets to be useful to one group of people. Also, since Ted seems pretty
keen on using this, I imagine it is much more likely to produce real
world, immediately useful tests not written by me (I'm not being lazy,
I just think it is better to get other people's experiences other than
my own).



That doesn't make sense does it? The tests might not be limited to
fs/ext4. We might have other sub-systems that add tests.


Well, I have some small isolated examples that I think would probably
work no matter what, so we can get some usage outside of ext4. Also,
if we want to limit the number of tests, then we don't expect to get
much usage outside of ext4 before v5.5 anyway. I just figure, it's
better to make it work for one person than no one, right?

In any case, I admit it is not a great idea. I just thought it had
some interesting advantages over going in through linux-kselftest that
were worth exploring.


So, we will have to work to make linux-next as the base for new
development and limit the number of tests to where it will be
easier work in this mode for 5.5. We can stage the pull requests
so that kunit lands first followed by tests.


So we are going to encourage maintainers to allow tests in their tree
based on KUnit on the assumption that KUnit will get merged before
there changes? That sounds like a huge burden, not just on us, but on
other maintainers and users.

I think if we are going to allow tests before KUnit is merged, we
should have the tests come in through the same tree as KUnit.


We have a similar situation with kselftest as well. Sub-systems
send tests that depend on their tress separately.


Well it is different if the maintainer wants to send the test in
through their tree, right? Otherwise, it won't matter what the state
of linux-next is and it won't matter when linux-kselftest gets merged.
Or am I not understanding you?



Let's talk about current state. Right now kunit is in linux-next and
we want to add a few more tests. We will have to coordinate the effort.
Once kunit get into mainline, then the need for this coordination goes
down.

Let's focus on the next few weeks first so we can get this into mainline
in 5.5.

The two of us can chat next week and come up with a plan.

thanks,
-- Shuah
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-10-04 Thread shuah

On 10/4/19 5:52 PM, Brendan Higgins wrote:

On Fri, Oct 4, 2019 at 4:30 PM Theodore Y. Ts'o  wrote:


On Fri, Oct 04, 2019 at 04:47:09PM -0600, shuah wrote:

However, if I encourage arbitrary tests/improvements into my KUnit
branch, it further diverges away from torvalds/master, and is more
likely that there will be a merge conflict or issue that is not related
to the core KUnit changes that will cause the whole thing to be
rejected again in v5.5.


The idea is that the new development will happen based on kunit in
linux-kselftest next. It will work just fine. As we accepts patches,
they will go on top of kunit that is in linux-next now.


I don't see how this would work.  If I add unit tests to ext4, they
would be in fs/ext4.  And to the extent that I need to add test mocks
to allow the unit tests to work, they will involve changes to existing
files in fs/ext4.  I can't put them in the ext4.git tree without
pulling in the kunit changes into the ext4 git tree.  And if they ext4
unit tests land in the kunit tree, it would be a receipe for large
numbers of merge conflicts.


That's where I was originally coming from.

So here's a dumb idea: what if we merged KUnit through the ext4 tree?
I imagine that could potentially get very confusing when we go back to
sending changes in through the kselftest tree, but at least it means
that ext4 can use it in the meantime, which means that it at least
gets to be useful to one group of people. Also, since Ted seems pretty
keen on using this, I imagine it is much more likely to produce real
world, immediately useful tests not written by me (I'm not being lazy,
I just think it is better to get other people's experiences other than
my own).



That doesn't make sense does it? The tests might not be limited to
fs/ext4. We might have other sub-systems that add tests.

So, we will have to work to make linux-next as the base for new
development and limit the number of tests to where it will be
easier work in this mode for 5.5. We can stage the pull requests
so that kunit lands first followed by tests.

We have a similar situation with kselftest as well. Sub-systems
send tests that depend on their tress separately.

thanks,
-- Shuah
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-10-04 Thread shuah

On 10/4/19 5:10 PM, Brendan Higgins wrote:

On Fri, Oct 4, 2019 at 3:47 PM shuah  wrote:


On 10/4/19 4:27 PM, Brendan Higgins wrote:

On Fri, Oct 04, 2019 at 03:59:10PM -0600, shuah wrote:

On 10/4/19 3:42 PM, Linus Torvalds wrote:

On Fri, Oct 4, 2019 at 2:39 PM Theodore Y. Ts'o  wrote:


This question is primarily directed at Shuah and Linus

What's the current status of the kunit series now that Brendan has
moved it out of the top-level kunit directory as Linus has requested?




The move happened smack in the middle of merge window and landed in
linux-next towards the end of the merge window.


We seemed to decide to just wait for 5.5, but there is nothing that
looks to block that. And I encouraged Shuah to find more kunit cases
for when it _does_ get merged.



Right. I communicated that to Brendan that we could work on adding more
kunit based tests which would help get more mileage on the kunit.


So if the kunit branch is stable, and people want to start using it
for their unit tests, then I think that would be a good idea, and then
during the 5.5 merge window we'll not just get the infrastructure,
we'll get a few more users too and not just examples.


I was planning on holding off on accepting more tests/changes until
KUnit is in torvalds/master. As much as I would like to go around
promoting it, I don't really want to promote too much complexity in a
non-upstream branch before getting it upstream because I don't want to
risk adding something that might cause it to get rejected again.

To be clear, I can understand from your perspective why getting more
tests/usage before accepting it is a good thing. The more people that
play around with it, the more likely that someone will find an issue
with it, and more likely that what is accepted into torvalds/master is
of high quality.

However, if I encourage arbitrary tests/improvements into my KUnit
branch, it further diverges away from torvalds/master, and is more
likely that there will be a merge conflict or issue that is not related
to the core KUnit changes that will cause the whole thing to be
rejected again in v5.5.



The idea is that the new development will happen based on kunit in
linux-kselftest next. It will work just fine. As we accepts patches,
they will go on top of kunit that is in linux-next now.


But then wouldn't we want to limit what KUnit changes are going into
linux-kselftest next for v5.5? For example, we probably don't want to
do anymore feature development on it until it is in v5.5, since the
goal is to make it more stable, right?

I am guessing that it will probably be fine, but it still sounds like
we need to establish some ground rules, and play it *very* safe.



How about we identify a small number tests that can add value and focus
on them. I am thinking a number between 2 and 5. This way we get a feel
for the API, if it changes for the better great, if it doesn't have to,
then you know you already did a great job.

Does that sound reasonable to you?

thanks,
-- Shuah
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-10-04 Thread shuah

On 10/4/19 4:27 PM, Brendan Higgins wrote:

On Fri, Oct 04, 2019 at 03:59:10PM -0600, shuah wrote:

On 10/4/19 3:42 PM, Linus Torvalds wrote:

On Fri, Oct 4, 2019 at 2:39 PM Theodore Y. Ts'o  wrote:


This question is primarily directed at Shuah and Linus

What's the current status of the kunit series now that Brendan has
moved it out of the top-level kunit directory as Linus has requested?




The move happened smack in the middle of merge window and landed in
linux-next towards the end of the merge window.


We seemed to decide to just wait for 5.5, but there is nothing that
looks to block that. And I encouraged Shuah to find more kunit cases
for when it _does_ get merged.



Right. I communicated that to Brendan that we could work on adding more
kunit based tests which would help get more mileage on the kunit.


So if the kunit branch is stable, and people want to start using it
for their unit tests, then I think that would be a good idea, and then
during the 5.5 merge window we'll not just get the infrastructure,
we'll get a few more users too and not just examples.


I was planning on holding off on accepting more tests/changes until
KUnit is in torvalds/master. As much as I would like to go around
promoting it, I don't really want to promote too much complexity in a
non-upstream branch before getting it upstream because I don't want to
risk adding something that might cause it to get rejected again.

To be clear, I can understand from your perspective why getting more
tests/usage before accepting it is a good thing. The more people that
play around with it, the more likely that someone will find an issue
with it, and more likely that what is accepted into torvalds/master is
of high quality.

However, if I encourage arbitrary tests/improvements into my KUnit
branch, it further diverges away from torvalds/master, and is more
likely that there will be a merge conflict or issue that is not related
to the core KUnit changes that will cause the whole thing to be
rejected again in v5.5.



The idea is that the new development will happen based on kunit in
linux-kselftest next. It will work just fine. As we accepts patches,
they will go on top of kunit that is in linux-next now.

thanks,
-- Shuah
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-10-04 Thread shuah

On 10/4/19 3:42 PM, Linus Torvalds wrote:

On Fri, Oct 4, 2019 at 2:39 PM Theodore Y. Ts'o  wrote:


This question is primarily directed at Shuah and Linus

What's the current status of the kunit series now that Brendan has
moved it out of the top-level kunit directory as Linus has requested?




The move happened smack in the middle of merge window and landed in
linux-next towards the end of the merge window.


We seemed to decide to just wait for 5.5, but there is nothing that
looks to block that. And I encouraged Shuah to find more kunit cases
for when it _does_ get merged.



Right. I communicated that to Brendan that we could work on adding more
kunit based tests which would help get more mileage on the kunit.


So if the kunit branch is stable, and people want to start using it
for their unit tests, then I think that would be a good idea, and then
during the 5.5 merge window we'll not just get the infrastructure,
we'll get a few more users too and not just examples.


thanks,
-- Shuah
___
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org


Re: [PATCH v18 15/19] Documentation: kunit: add documentation for KUnit

2019-09-23 Thread shuah

On 9/23/19 1:49 PM, Randy Dunlap wrote:

On 9/23/19 11:06 AM, Brendan Higgins wrote:

On Mon, Sep 23, 2019 at 8:48 AM Randy Dunlap  wrote:


On 9/23/19 2:02 AM, Brendan Higgins wrote:

Add documentation for KUnit, the Linux kernel unit testing framework.
- Add intro and usage guide for KUnit
- Add API reference

Signed-off-by: Felix Guo 
Signed-off-by: Brendan Higgins 
Cc: Jonathan Corbet 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
  Documentation/dev-tools/index.rst   |   1 +
  Documentation/dev-tools/kunit/api/index.rst |  16 +
  Documentation/dev-tools/kunit/api/test.rst  |  11 +
  Documentation/dev-tools/kunit/faq.rst   |  62 +++
  Documentation/dev-tools/kunit/index.rst |  79 +++
  Documentation/dev-tools/kunit/start.rst | 180 ++
  Documentation/dev-tools/kunit/usage.rst | 576 
  7 files changed, 925 insertions(+)
  create mode 100644 Documentation/dev-tools/kunit/api/index.rst
  create mode 100644 Documentation/dev-tools/kunit/api/test.rst
  create mode 100644 Documentation/dev-tools/kunit/faq.rst
  create mode 100644 Documentation/dev-tools/kunit/index.rst
  create mode 100644 Documentation/dev-tools/kunit/start.rst
  create mode 100644 Documentation/dev-tools/kunit/usage.rst




diff --git a/Documentation/dev-tools/kunit/start.rst 
b/Documentation/dev-tools/kunit/start.rst
new file mode 100644
index ..6dc229e46bb3
--- /dev/null
+++ b/Documentation/dev-tools/kunit/start.rst
@@ -0,0 +1,180 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===
+Getting Started
+===
+
+Installing dependencies
+===
+KUnit has the same dependencies as the Linux kernel. As long as you can build
+the kernel, you can run KUnit.
+
+KUnit Wrapper
+=
+Included with KUnit is a simple Python wrapper that helps format the output to
+easily use and read KUnit output. It handles building and running the kernel, 
as
+well as formatting the output.
+
+The wrapper can be run with:
+
+.. code-block:: bash
+
+   ./tools/testing/kunit/kunit.py run
+
+Creating a kunitconfig
+==
+The Python script is a thin wrapper around Kbuild as such, it needs to be


around Kbuild. As such,


Thanks for pointing this out.




+configured with a ``kunitconfig`` file. This file essentially contains the
+regular Kernel config, with the specific test targets as well.
+
+.. code-block:: bash
+
+ git clone -b master https://kunit.googlesource.com/kunitconfig 
$PATH_TO_KUNITCONFIG_REPO
+ cd $PATH_TO_LINUX_REPO
+ ln -s $PATH_TO_KUNIT_CONFIG_REPO/kunitconfig kunitconfig
+
+You may want to add kunitconfig to your local gitignore.
+
+Verifying KUnit Works
+-
+
+To make sure that everything is set up correctly, simply invoke the Python
+wrapper from your kernel repo:
+
+.. code-block:: bash
+
+ ./tools/testing/kunit/kunit.py
+
+.. note::
+   You may want to run ``make mrproper`` first.


I normally use O=builddir when building kernels.
Does this support using O=builddir ?


Yep, it supports specifying a separate build directory.


+
+If everything worked correctly, you should see the following:
+
+.. code-block:: bash
+
+ Generating .config ...
+ Building KUnit Kernel ...
+ Starting KUnit Kernel ...
+
+followed by a list of tests that are run. All of them should be passing.
+
+.. note::
+   Because it is building a lot of sources for the first time, the ``Building
+   kunit kernel`` step may take a while.
+
+Writing your first test
+===


[snip]


diff --git a/Documentation/dev-tools/kunit/usage.rst 
b/Documentation/dev-tools/kunit/usage.rst
new file mode 100644
index ..c6e69634e274
--- /dev/null
+++ b/Documentation/dev-tools/kunit/usage.rst


TBD...


What did you mean by this comment?


I plan to review usage.rst soon... (To Be Done :)



I would like to apply the series very soon so it gets some soak time
after this move in linux-next and it can still make the rc1.

Since there changes can be addressed after rc1, I would like to not
require Brendan to do another version before I apply.

Hope you are okay with that Randy!

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v15 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-08-24 Thread shuah

On 8/23/19 7:34 PM, Brendan Higgins wrote:

## TL;DR

This revision addresses comments from Shuah by fixing a couple
checkpatch warnings and fixing some comment readability issues. No API
or major structual changes have been made since v13.

## Background

This patch set proposes KUnit, a lightweight unit testing and mocking
framework for the Linux kernel.

Unlike Autotest and kselftest, KUnit is a true unit testing framework;
it does not require installing the kernel on a test machine or in a VM
(however, KUnit still allows you to run tests on test machines or in VMs
if you want[1]) and does not require tests to be written in userspace
running on a host kernel. Additionally, KUnit is fast: From invocation
to completion KUnit can run several dozen tests in about a second.
Currently, the entire KUnit test suite for KUnit runs in under a second
from the initial invocation (build time excluded).

KUnit is heavily inspired by JUnit, Python's unittest.mock, and
Googletest/Googlemock for C++. KUnit provides facilities for defining
unit test cases, grouping related test cases into test suites, providing
common infrastructure for running tests, mocking, spying, and much more.

### What's so special about unit testing?

A unit test is supposed to test a single unit of code in isolation,
hence the name. There should be no dependencies outside the control of
the test; this means no external dependencies, which makes tests orders
of magnitudes faster. Likewise, since there are no external dependencies,
there are no hoops to jump through to run the tests. Additionally, this
makes unit tests deterministic: a failing unit test always indicates a
problem. Finally, because unit tests necessarily have finer granularity,
they are able to test all code paths easily solving the classic problem
of difficulty in exercising error handling code.

### Is KUnit trying to replace other testing frameworks for the kernel?

No. Most existing tests for the Linux kernel are end-to-end tests, which
have their place. A well tested system has lots of unit tests, a
reasonable number of integration tests, and some end-to-end tests. KUnit
is just trying to address the unit test space which is currently not
being addressed.

### More information on KUnit

There is a bunch of documentation near the end of this patch set that
describes how to use KUnit and best practices for writing unit tests.
For convenience I am hosting the compiled docs here[2].

Additionally for convenience, I have applied these patches to a
branch[3]. The repo may be cloned with:
git clone https://kunit.googlesource.com/linux
This patchset is on the kunit/rfc/v5.3/v15 branch.

## Changes Since Last Version

- Moved comment from inline in macro to kernel-doc to address checkpatch
   warning.
- Demoted BUG() to WARN_ON.
- Formatted some kernel-doc comments to make them more readible.

[1] 
https://google.github.io/kunit-docs/third_party/kernel/docs/usage.html#kunit-on-non-uml-architectures
[2] https://google.github.io/kunit-docs/third_party/kernel/docs/
[3] https://kunit.googlesource.com/linux/+/kunit/rfc/v5.3/v15



Hi Brendan,

Thanks for doing this work.

Thanks for accommodating my request to improve the document/comment
blocks in patch 01 and removing BUG() from patch 09. The comment block
reads well now.

Applied the series to linux-kselftest next for 5.4-rc1.

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core

2019-08-23 Thread shuah

On 8/23/19 1:20 PM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 12:04 PM shuah  wrote:


On 8/23/19 12:56 PM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 11:32 AM shuah  wrote:


On 8/23/19 11:54 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 10:34 AM shuah  wrote:


On 8/23/19 11:27 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 10:05 AM shuah  wrote:


On 8/23/19 10:48 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 8:33 AM shuah  wrote:


Hi Brendan,

On 8/20/19 5:20 PM, 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 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Luis Chamberlain 
Reviewed-by: Stephen Boyd 
---
   include/kunit/test.h | 179 
   kunit/Kconfig|  17 
   kunit/Makefile   |   1 +
   kunit/test.c | 191 +++
   4 files changed, 388 insertions(+)
   create mode 100644 include/kunit/test.h
   create mode 100644 kunit/Kconfig
   create mode 100644 kunit/Makefile
   create mode 100644 kunit/test.c

diff --git a/include/kunit/test.h b/include/kunit/test.h
new file mode 100644
index 0..e0b34acb9ee4e
--- /dev/null
+++ b/include/kunit/test.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Base unit test (KUnit) API.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_TEST_H
+#define _KUNIT_TEST_H
+
+#include 
+
+struct kunit;
+
+/**
+ * struct kunit_case - represents an individual test case.
+ * @run_case: the function representing the actual test case.
+ * @name: the name of the test case.
+ *
+ * A test case is a function with the signature, ``void (*)(struct kunit *)``
+ * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. 
Each
+ * test case is associated with a &struct kunit_suite and will be run after the
+ * suite's init function and followed by the suite's exit function.
+ *
+ * A test case should be static and should only be created with the 
KUNIT_CASE()
+ * macro; additionally, every array of test cases should be terminated with an
+ * empty test case.
+ *
+ * Example:


Can you fix these line continuations. It makes it very hard to read.
Sorry for this late comment. These comments lines are longer than 80
and wrap.


None of the lines in this commit are over 80 characters in column
width. Some are exactly 80 characters (like above).

My guess is that you are seeing the diff added text (+ ), which when
you add that to a line which is exactly 80 char in length ends up
being over 80 char in email. If you apply the patch you will see that
they are only 80 chars.



There are several comment lines in the file that are way too long.


Note that checkpatch also does not complain about any over 80 char
lines in this file.

Sorry if I am misunderstanding what you are trying to tell me. Please
confirm either way.



WARNING: Avoid unnecessary line continuations
#258: FILE: include/kunit/test.h:137:
+*/\

total: 0 errors, 2 warnings, 388 lines checked


Ah, okay so you don't like the warning about the line continuation.
That's not because it is over 80 char, but because there is a line
continuation after a comment. I don't really see a way to get rid of
it without removing the comment from inside the macro.

I put this TODO there in the first place a Luis' request, and I put it
in the body of the macro because this macro already had a kernel-doc
comment and I didn't think that an implementation detail TODO belonged
in the user documentation.


Go ahead fix these. It appears there are few lines that either longer
than 80. In general, I keep them around 75, so it is easier read.


Sorry, the above is the only checkpatch warning other than the
reminder to update the MAINTAINERS file.

Are you saying you want me to go through and make all the lines fit in
75 char column width? I hope not because that is going to be a pretty
substantial change to make.



There are two things with these comment lines. One is checkpatch
complaining and the other is general readability.


So for the checkpatch warning, do you want me to move the comment out
of the macro body into the kernel-doc comment? I don't really think it
is the right place for a comment of this nature, but I think it is
probably better than dropping it entirely (I don't see how else to do
it without just removing the commen

Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core

2019-08-23 Thread shuah

On 8/23/19 12:56 PM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 11:32 AM shuah  wrote:


On 8/23/19 11:54 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 10:34 AM shuah  wrote:


On 8/23/19 11:27 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 10:05 AM shuah  wrote:


On 8/23/19 10:48 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 8:33 AM shuah  wrote:


Hi Brendan,

On 8/20/19 5:20 PM, 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 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Luis Chamberlain 
Reviewed-by: Stephen Boyd 
---
  include/kunit/test.h | 179 
  kunit/Kconfig|  17 
  kunit/Makefile   |   1 +
  kunit/test.c | 191 +++
  4 files changed, 388 insertions(+)
  create mode 100644 include/kunit/test.h
  create mode 100644 kunit/Kconfig
  create mode 100644 kunit/Makefile
  create mode 100644 kunit/test.c

diff --git a/include/kunit/test.h b/include/kunit/test.h
new file mode 100644
index 0..e0b34acb9ee4e
--- /dev/null
+++ b/include/kunit/test.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Base unit test (KUnit) API.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_TEST_H
+#define _KUNIT_TEST_H
+
+#include 
+
+struct kunit;
+
+/**
+ * struct kunit_case - represents an individual test case.
+ * @run_case: the function representing the actual test case.
+ * @name: the name of the test case.
+ *
+ * A test case is a function with the signature, ``void (*)(struct kunit *)``
+ * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. 
Each
+ * test case is associated with a &struct kunit_suite and will be run after the
+ * suite's init function and followed by the suite's exit function.
+ *
+ * A test case should be static and should only be created with the 
KUNIT_CASE()
+ * macro; additionally, every array of test cases should be terminated with an
+ * empty test case.
+ *
+ * Example:


Can you fix these line continuations. It makes it very hard to read.
Sorry for this late comment. These comments lines are longer than 80
and wrap.


None of the lines in this commit are over 80 characters in column
width. Some are exactly 80 characters (like above).

My guess is that you are seeing the diff added text (+ ), which when
you add that to a line which is exactly 80 char in length ends up
being over 80 char in email. If you apply the patch you will see that
they are only 80 chars.



There are several comment lines in the file that are way too long.


Note that checkpatch also does not complain about any over 80 char
lines in this file.

Sorry if I am misunderstanding what you are trying to tell me. Please
confirm either way.



WARNING: Avoid unnecessary line continuations
#258: FILE: include/kunit/test.h:137:
+*/\

total: 0 errors, 2 warnings, 388 lines checked


Ah, okay so you don't like the warning about the line continuation.
That's not because it is over 80 char, but because there is a line
continuation after a comment. I don't really see a way to get rid of
it without removing the comment from inside the macro.

I put this TODO there in the first place a Luis' request, and I put it
in the body of the macro because this macro already had a kernel-doc
comment and I didn't think that an implementation detail TODO belonged
in the user documentation.


Go ahead fix these. It appears there are few lines that either longer
than 80. In general, I keep them around 75, so it is easier read.


Sorry, the above is the only checkpatch warning other than the
reminder to update the MAINTAINERS file.

Are you saying you want me to go through and make all the lines fit in
75 char column width? I hope not because that is going to be a pretty
substantial change to make.



There are two things with these comment lines. One is checkpatch
complaining and the other is general readability.


So for the checkpatch warning, do you want me to move the comment out
of the macro body into the kernel-doc comment? I don't really think it
is the right place for a comment of this nature, but I think it is
probably better than dropping it entirely (I don't see how else to do
it without just removing the comment entirely).



Don't drop the comments. It makes perfect sense to turn this into a
kernel-d

Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core

2019-08-23 Thread shuah

On 8/23/19 11:54 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 10:34 AM shuah  wrote:


On 8/23/19 11:27 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 10:05 AM shuah  wrote:


On 8/23/19 10:48 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 8:33 AM shuah  wrote:


Hi Brendan,

On 8/20/19 5:20 PM, 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 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Luis Chamberlain 
Reviewed-by: Stephen Boyd 
---
 include/kunit/test.h | 179 
 kunit/Kconfig|  17 
 kunit/Makefile   |   1 +
 kunit/test.c | 191 +++
 4 files changed, 388 insertions(+)
 create mode 100644 include/kunit/test.h
 create mode 100644 kunit/Kconfig
 create mode 100644 kunit/Makefile
 create mode 100644 kunit/test.c

diff --git a/include/kunit/test.h b/include/kunit/test.h
new file mode 100644
index 0..e0b34acb9ee4e
--- /dev/null
+++ b/include/kunit/test.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Base unit test (KUnit) API.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_TEST_H
+#define _KUNIT_TEST_H
+
+#include 
+
+struct kunit;
+
+/**
+ * struct kunit_case - represents an individual test case.
+ * @run_case: the function representing the actual test case.
+ * @name: the name of the test case.
+ *
+ * A test case is a function with the signature, ``void (*)(struct kunit *)``
+ * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. 
Each
+ * test case is associated with a &struct kunit_suite and will be run after the
+ * suite's init function and followed by the suite's exit function.
+ *
+ * A test case should be static and should only be created with the 
KUNIT_CASE()
+ * macro; additionally, every array of test cases should be terminated with an
+ * empty test case.
+ *
+ * Example:


Can you fix these line continuations. It makes it very hard to read.
Sorry for this late comment. These comments lines are longer than 80
and wrap.


None of the lines in this commit are over 80 characters in column
width. Some are exactly 80 characters (like above).

My guess is that you are seeing the diff added text (+ ), which when
you add that to a line which is exactly 80 char in length ends up
being over 80 char in email. If you apply the patch you will see that
they are only 80 chars.



There are several comment lines in the file that are way too long.


Note that checkpatch also does not complain about any over 80 char
lines in this file.

Sorry if I am misunderstanding what you are trying to tell me. Please
confirm either way.



WARNING: Avoid unnecessary line continuations
#258: FILE: include/kunit/test.h:137:
+*/\

total: 0 errors, 2 warnings, 388 lines checked


Ah, okay so you don't like the warning about the line continuation.
That's not because it is over 80 char, but because there is a line
continuation after a comment. I don't really see a way to get rid of
it without removing the comment from inside the macro.

I put this TODO there in the first place a Luis' request, and I put it
in the body of the macro because this macro already had a kernel-doc
comment and I didn't think that an implementation detail TODO belonged
in the user documentation.


Go ahead fix these. It appears there are few lines that either longer
than 80. In general, I keep them around 75, so it is easier read.


Sorry, the above is the only checkpatch warning other than the
reminder to update the MAINTAINERS file.

Are you saying you want me to go through and make all the lines fit in
75 char column width? I hope not because that is going to be a pretty
substantial change to make.



There are two things with these comment lines. One is checkpatch
complaining and the other is general readability.


So for the checkpatch warning, do you want me to move the comment out
of the macro body into the kernel-doc comment? I don't really think it
is the right place for a comment of this nature, but I think it is
probably better than dropping it entirely (I don't see how else to do
it without just removing the comment entirely).



Don't drop the comments. It makes perfect sense to turn this into a
kernel-doc comment.

We are going back forth on this a lot. I see several lines 81+ in
this file. I am at 5.3-rc5 

Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core

2019-08-23 Thread shuah

On 8/23/19 11:27 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 10:05 AM shuah  wrote:


On 8/23/19 10:48 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 8:33 AM shuah  wrote:


Hi Brendan,

On 8/20/19 5:20 PM, 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 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Luis Chamberlain 
Reviewed-by: Stephen Boyd 
---
include/kunit/test.h | 179 
kunit/Kconfig|  17 
kunit/Makefile   |   1 +
kunit/test.c | 191 +++
4 files changed, 388 insertions(+)
create mode 100644 include/kunit/test.h
create mode 100644 kunit/Kconfig
create mode 100644 kunit/Makefile
create mode 100644 kunit/test.c

diff --git a/include/kunit/test.h b/include/kunit/test.h
new file mode 100644
index 0..e0b34acb9ee4e
--- /dev/null
+++ b/include/kunit/test.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Base unit test (KUnit) API.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_TEST_H
+#define _KUNIT_TEST_H
+
+#include 
+
+struct kunit;
+
+/**
+ * struct kunit_case - represents an individual test case.
+ * @run_case: the function representing the actual test case.
+ * @name: the name of the test case.
+ *
+ * A test case is a function with the signature, ``void (*)(struct kunit *)``
+ * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. 
Each
+ * test case is associated with a &struct kunit_suite and will be run after the
+ * suite's init function and followed by the suite's exit function.
+ *
+ * A test case should be static and should only be created with the 
KUNIT_CASE()
+ * macro; additionally, every array of test cases should be terminated with an
+ * empty test case.
+ *
+ * Example:


Can you fix these line continuations. It makes it very hard to read.
Sorry for this late comment. These comments lines are longer than 80
and wrap.


None of the lines in this commit are over 80 characters in column
width. Some are exactly 80 characters (like above).

My guess is that you are seeing the diff added text (+ ), which when
you add that to a line which is exactly 80 char in length ends up
being over 80 char in email. If you apply the patch you will see that
they are only 80 chars.



There are several comment lines in the file that are way too long.


Note that checkpatch also does not complain about any over 80 char
lines in this file.

Sorry if I am misunderstanding what you are trying to tell me. Please
confirm either way.



WARNING: Avoid unnecessary line continuations
#258: FILE: include/kunit/test.h:137:
+*/\

total: 0 errors, 2 warnings, 388 lines checked


Ah, okay so you don't like the warning about the line continuation.
That's not because it is over 80 char, but because there is a line
continuation after a comment. I don't really see a way to get rid of
it without removing the comment from inside the macro.

I put this TODO there in the first place a Luis' request, and I put it
in the body of the macro because this macro already had a kernel-doc
comment and I didn't think that an implementation detail TODO belonged
in the user documentation.


Go ahead fix these. It appears there are few lines that either longer
than 80. In general, I keep them around 75, so it is easier read.


Sorry, the above is the only checkpatch warning other than the
reminder to update the MAINTAINERS file.

Are you saying you want me to go through and make all the lines fit in
75 char column width? I hope not because that is going to be a pretty
substantial change to make.



There are two things with these comment lines. One is checkpatch
complaining and the other is general readability.

Please go ahead and adjust them.

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v14 09/18] kunit: test: add support for test abort

2019-08-23 Thread shuah

On 8/23/19 10:56 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 8:36 AM shuah  wrote:


Hi Brendan,

On 8/20/19 5:20 PM, Brendan Higgins wrote:

Add support for aborting/bailing out of test cases, which is needed for
implementing assertions.

An assertion is like an expectation, but bails out of the test case
early if the assertion is not met. The idea with assertions is that you
use them to state all the preconditions for your test. Logically
speaking, these are the premises of the test case, so if a premise isn't
true, there is no point in continuing the test case because there are no
conclusions that can be drawn without the premises. Whereas, the
expectation is the thing you are trying to prove.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Stephen Boyd 
---
   include/kunit/test.h  |   2 +
   include/kunit/try-catch.h |  75 +
   kunit/Makefile|   3 +-
   kunit/test.c  | 137 +-
   kunit/try-catch.c | 118 
   5 files changed, 319 insertions(+), 16 deletions(-)
   create mode 100644 include/kunit/try-catch.h
   create mode 100644 kunit/try-catch.c

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 6917b186b737a..390ce02f717b6 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -10,6 +10,7 @@
   #define _KUNIT_TEST_H

   #include 
+#include 
   #include 
   #include 
   #include 
@@ -167,6 +168,7 @@ struct kunit {

   /* private: internal use only. */
   const char *name; /* Read only after initialization! */
+ struct kunit_try_catch try_catch;
   /*
* success starts as true, and may only be set to false during a test
* case; thus, it is safe to update this across multiple threads using
diff --git a/include/kunit/try-catch.h b/include/kunit/try-catch.h
new file mode 100644
index 0..404f336cbdc85
--- /dev/null
+++ b/include/kunit/try-catch.h
@@ -0,0 +1,75 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * An API to allow a function, that may fail, to be executed, and recover in a
+ * controlled manner.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_TRY_CATCH_H
+#define _KUNIT_TRY_CATCH_H
+
+#include 
+
+typedef void (*kunit_try_catch_func_t)(void *);
+
+struct completion;
+struct kunit;
+
+/**
+ * struct kunit_try_catch - provides a generic way to run code which might 
fail.
+ * @test: The test case that is currently being executed.
+ * @try_completion: Completion that the control thread waits on while test 
runs.
+ * @try_result: Contains any errno obtained while running test case.
+ * @try: The function, the test case, to attempt to run.
+ * @catch: The function called if @try bails out.
+ * @context: used to pass user data to the try and catch functions.
+ *
+ * kunit_try_catch provides a generic, architecture independent way to execute
+ * an arbitrary function of type kunit_try_catch_func_t which may bail out by
+ * calling kunit_try_catch_throw(). If kunit_try_catch_throw() is called, @try
+ * is stopped at the site of invocation and @catch is called.
+ *
+ * struct kunit_try_catch provides a generic interface for the functionality
+ * needed to implement kunit->abort() which in turn is needed for implementing
+ * assertions. Assertions allow stating a precondition for a test simplifying
+ * how test cases are written and presented.
+ *
+ * Assertions are like expectations, except they abort (call
+ * kunit_try_catch_throw()) when the specified condition is not met. This is
+ * useful when you look at a test case as a logical statement about some piece
+ * of code, where assertions are the premises for the test case, and the
+ * conclusion is a set of predicates, rather expectations, that must all be
+ * true. If your premises are violated, it does not makes sense to continue.
+ */
+struct kunit_try_catch {
+ /* private: internal use only. */
+ struct kunit *test;
+ struct completion *try_completion;
+ int try_result;
+ kunit_try_catch_func_t try;
+ kunit_try_catch_func_t catch;
+ void *context;
+};
+
+void kunit_try_catch_init(struct kunit_try_catch *try_catch,
+   struct kunit *test,
+   kunit_try_catch_func_t try,
+   kunit_try_catch_func_t catch);
+
+void kunit_try_catch_run(struct kunit_try_catch *try_catch, void *context);
+
+void __noreturn kunit_try_catch_throw(struct kunit_try_catch *try_catch);
+
+static inline int kunit_try_catch_get_result(struct kunit_try_catch *try_catch)
+{
+ return try_catch->try_result;
+}
+
+/*
+ * Exposed for testing only.
+ */
+void kunit_generic_try_catch_init(struct kunit_try_catch *try_catch);
+
+#endif /* _KUNIT_TRY_CATCH_H */
diff --git a/kunit/Makefile b/kunit/Makefile
index 4e46450bcb3a8..c9176c9c578c6 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@

Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core

2019-08-23 Thread shuah

On 8/23/19 10:48 AM, Brendan Higgins wrote:

On Fri, Aug 23, 2019 at 8:33 AM shuah  wrote:


Hi Brendan,

On 8/20/19 5:20 PM, 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 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Luis Chamberlain 
Reviewed-by: Stephen Boyd 
---
   include/kunit/test.h | 179 
   kunit/Kconfig|  17 
   kunit/Makefile   |   1 +
   kunit/test.c | 191 +++
   4 files changed, 388 insertions(+)
   create mode 100644 include/kunit/test.h
   create mode 100644 kunit/Kconfig
   create mode 100644 kunit/Makefile
   create mode 100644 kunit/test.c

diff --git a/include/kunit/test.h b/include/kunit/test.h
new file mode 100644
index 0..e0b34acb9ee4e
--- /dev/null
+++ b/include/kunit/test.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Base unit test (KUnit) API.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_TEST_H
+#define _KUNIT_TEST_H
+
+#include 
+
+struct kunit;
+
+/**
+ * struct kunit_case - represents an individual test case.
+ * @run_case: the function representing the actual test case.
+ * @name: the name of the test case.
+ *
+ * A test case is a function with the signature, ``void (*)(struct kunit *)``
+ * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. 
Each
+ * test case is associated with a &struct kunit_suite and will be run after the
+ * suite's init function and followed by the suite's exit function.
+ *
+ * A test case should be static and should only be created with the 
KUNIT_CASE()
+ * macro; additionally, every array of test cases should be terminated with an
+ * empty test case.
+ *
+ * Example:


Can you fix these line continuations. It makes it very hard to read.
Sorry for this late comment. These comments lines are longer than 80
and wrap.


None of the lines in this commit are over 80 characters in column
width. Some are exactly 80 characters (like above).

My guess is that you are seeing the diff added text (+ ), which when
you add that to a line which is exactly 80 char in length ends up
being over 80 char in email. If you apply the patch you will see that
they are only 80 chars.



There are several comment lines in the file that are way too long.


Note that checkpatch also does not complain about any over 80 char
lines in this file.

Sorry if I am misunderstanding what you are trying to tell me. Please
confirm either way.



WARNING: Avoid unnecessary line continuations
#258: FILE: include/kunit/test.h:137:
+*/\

total: 0 errors, 2 warnings, 388 lines checked

Go ahead fix these. It appears there are few lines that either longer
than 80. In general, I keep them around 75, so it is easier read.

thanks,
-- Shuah


___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v14 09/18] kunit: test: add support for test abort

2019-08-23 Thread shuah
 test.o \
string-stream.o \
-   assert.o
+   assert.o \
+   try-catch.o
  
  obj-$(CONFIG_KUNIT_TEST) +=		string-stream-test.o
  
diff --git a/kunit/test.c b/kunit/test.c

index 3cbceb34b3b36..ded9895143209 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -7,7 +7,9 @@
   */
  
  #include 

+#include 
  #include 
+#include 
  
  static void kunit_set_failure(struct kunit *test)

  {
@@ -162,6 +164,19 @@ static void kunit_fail(struct kunit *test, struct 
kunit_assert *assert)
WARN_ON(string_stream_destroy(stream));
  }
  
+static void __noreturn kunit_abort(struct kunit *test)

+{
+   kunit_try_catch_throw(&test->try_catch); /* Does not return. */
+
+   /*
+* Throw could not abort from test.
+*
+* XXX: we should never reach this line! As kunit_try_catch_throw is
+* marked __noreturn.
+*/
+   BUG();



I recall discussion on this. What's the point in keeping thie
BUG() around when it doesn't even reach? It can even be a
WARN_ON() in that case right?

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v14 01/18] kunit: test: add KUnit test runner core

2019-08-23 Thread shuah

Hi Brendan,

On 8/20/19 5:20 PM, 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 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
Reviewed-by: Luis Chamberlain 
Reviewed-by: Stephen Boyd 
---
  include/kunit/test.h | 179 
  kunit/Kconfig|  17 
  kunit/Makefile   |   1 +
  kunit/test.c | 191 +++
  4 files changed, 388 insertions(+)
  create mode 100644 include/kunit/test.h
  create mode 100644 kunit/Kconfig
  create mode 100644 kunit/Makefile
  create mode 100644 kunit/test.c

diff --git a/include/kunit/test.h b/include/kunit/test.h
new file mode 100644
index 0..e0b34acb9ee4e
--- /dev/null
+++ b/include/kunit/test.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Base unit test (KUnit) API.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#ifndef _KUNIT_TEST_H
+#define _KUNIT_TEST_H
+
+#include 
+
+struct kunit;
+
+/**
+ * struct kunit_case - represents an individual test case.
+ * @run_case: the function representing the actual test case.
+ * @name: the name of the test case.
+ *
+ * A test case is a function with the signature, ``void (*)(struct kunit *)``
+ * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. 
Each
+ * test case is associated with a &struct kunit_suite and will be run after the
+ * suite's init function and followed by the suite's exit function.
+ *
+ * A test case should be static and should only be created with the 
KUNIT_CASE()
+ * macro; additionally, every array of test cases should be terminated with an
+ * empty test case.
+ *
+ * Example:


Can you fix these line continuations. It makes it very hard to read.
Sorry for this late comment. These comments lines are longer than 80
and wrap.

There are several comment lines in the file that are way too long.

thanks,
-- Shuah

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v13 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-08-20 Thread shuah

On 8/20/19 5:23 PM, Brendan Higgins wrote:

On Tue, Aug 20, 2019 at 2:26 PM Brendan Higgins
 wrote:


On Tue, Aug 20, 2019 at 12:08 PM shuah  wrote:


On 8/20/19 12:24 PM, Brendan Higgins wrote:

On Tue, Aug 20, 2019 at 11:24:45AM -0600, shuah wrote:

On 8/13/19 11:50 PM, Brendan Higgins wrote:

## TL;DR

This revision addresses comments from Stephen and Bjorn Helgaas. Most
changes are pretty minor stuff that doesn't affect the API in anyway.
One significant change, however, is that I added support for freeing
kunit_resource managed resources before the test case is finished via
kunit_resource_destroy(). Additionally, Bjorn pointed out that I broke
KUnit on certain configurations (like the default one for x86, whoops).

Based on Stephen's feedback on the previous change, I think we are
pretty close. I am not expecting any significant changes from here on
out.



Hi Brendan,

I found checkpatch errors in one or two patches. Can you fix those and
send v14.


Hi Shuah,

Are you refering to the following errors?

ERROR: Macros with complex values should be enclosed in parentheses
#144: FILE: include/kunit/test.h:456:
+#define KUNIT_BINARY_CLASS \
+   kunit_binary_assert, KUNIT_INIT_BINARY_ASSERT_STRUCT

ERROR: Macros with complex values should be enclosed in parentheses
#146: FILE: include/kunit/test.h:458:
+#define KUNIT_BINARY_PTR_CLASS \
+   kunit_binary_ptr_assert, KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT

These values should *not* be in parentheses. I am guessing checkpatch is
getting confused and thinks that these are complex expressions, when
they are not.

I ignored the errors since I figured checkpatch was complaining
erroneously.

I could refactor the code to remove these macros entirely, but I think
the code is cleaner with them.



Please do. I am not veru sure what value these macros add.


Alright, I will have something for you later today.


I just sent a new revision with the fix.

Cheers



Thanks Brendan. I will get them in.

-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v13 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-08-20 Thread shuah

On 8/20/19 12:24 PM, Brendan Higgins wrote:

On Tue, Aug 20, 2019 at 11:24:45AM -0600, shuah wrote:

On 8/13/19 11:50 PM, Brendan Higgins wrote:

## TL;DR

This revision addresses comments from Stephen and Bjorn Helgaas. Most
changes are pretty minor stuff that doesn't affect the API in anyway.
One significant change, however, is that I added support for freeing
kunit_resource managed resources before the test case is finished via
kunit_resource_destroy(). Additionally, Bjorn pointed out that I broke
KUnit on certain configurations (like the default one for x86, whoops).

Based on Stephen's feedback on the previous change, I think we are
pretty close. I am not expecting any significant changes from here on
out.



Hi Brendan,

I found checkpatch errors in one or two patches. Can you fix those and
send v14.


Hi Shuah,

Are you refering to the following errors?

ERROR: Macros with complex values should be enclosed in parentheses
#144: FILE: include/kunit/test.h:456:
+#define KUNIT_BINARY_CLASS \
+   kunit_binary_assert, KUNIT_INIT_BINARY_ASSERT_STRUCT

ERROR: Macros with complex values should be enclosed in parentheses
#146: FILE: include/kunit/test.h:458:
+#define KUNIT_BINARY_PTR_CLASS \
+   kunit_binary_ptr_assert, KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT

These values should *not* be in parentheses. I am guessing checkpatch is
getting confused and thinks that these are complex expressions, when
they are not.

I ignored the errors since I figured checkpatch was complaining
erroneously.

I could refactor the code to remove these macros entirely, but I think
the code is cleaner with them.



Please do. I am not veru sure what value these macros add.

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v13 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-08-20 Thread shuah

On 8/13/19 11:50 PM, Brendan Higgins wrote:

## TL;DR

This revision addresses comments from Stephen and Bjorn Helgaas. Most
changes are pretty minor stuff that doesn't affect the API in anyway.
One significant change, however, is that I added support for freeing
kunit_resource managed resources before the test case is finished via
kunit_resource_destroy(). Additionally, Bjorn pointed out that I broke
KUnit on certain configurations (like the default one for x86, whoops).

Based on Stephen's feedback on the previous change, I think we are
pretty close. I am not expecting any significant changes from here on
out.



Hi Brendan,

I found checkpatch errors in one or two patches. Can you fix those and
send v14.

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v7 16/18] MAINTAINERS: add entry for KUnit the unit testing framework

2019-07-09 Thread shuah

On 7/9/19 12:30 AM, Brendan Higgins wrote:

Add myself as maintainer of KUnit, the Linux kernel's unit testing
framework.

Signed-off-by: Brendan Higgins 
Reviewed-by: Greg Kroah-Hartman 
Reviewed-by: Logan Gunthorpe 
---
  MAINTAINERS | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 677ef41cb012c..48d04d180a988 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8599,6 +8599,17 @@ S:   Maintained
  F:tools/testing/selftests/
  F:Documentation/dev-tools/kselftest*
  
+KERNEL UNIT TESTING FRAMEWORK (KUnit)

+M: Brendan Higgins 
+L: linux-kselft...@vger.kernel.org
+L: kunit-...@googlegroups.com
+W: https://google.github.io/kunit-docs/third_party/kernel/docs/
+S: Maintained
+F: Documentation/dev-tools/kunit/
+F: include/kunit/
+F: kunit/
+F: tools/testing/kunit/
+
  KERNEL USERMODE HELPER
  M:Luis Chamberlain 
  L:linux-ker...@vger.kernel.org



Thanks Brendan.

I am good with this. I can take KUnit patches through kselftest
with your Ack.

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v5 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-06-21 Thread shuah

On 6/21/19 12:13 PM, Theodore Ts'o wrote:

On Fri, Jun 21, 2019 at 08:59:48AM -0600, shuah wrote:

### But wait! Doesn't kselftest support in kernel testing?!




I think I commented on this before. I agree with the statement that
there is no overlap between Kselftest and KUnit. I would like see this
removed. Kselftest module support supports use-cases KUnit won't be able
to. I can build an kernel with Kselftest test modules and use it in the
filed to load and run tests if I need to debug a problem and get data
from a system. I can't do that with KUnit.

In my mind, I am not viewing this as which is better. Kselftest and
KUnit both have their place in the kernel development process. It isn't
productive and/or necessary to comparing Kselftest and KUnit without a
good understanding of the problem spaces for each of these.

I would strongly recommend not making reference to Kselftest and talk
about what KUnit offers.


Shuah,

Just to recall the history, this section of the FAQ was added to rebut
the ***very*** strong statements that Frank made that there was
overlap between Kselftest and Kunit, and that having too many ways for
kernel developers to do the identical thing was harmful (he said it
was too much of a burden on a kernel developer) --- and this was an
argument for not including Kunit in the upstream kernel.

If we're past that objection, then perhaps this section can be
dropped, but there's a very good reason why it was there.  I wouldn't
Brendan to be accused of ignoring feedback from those who reviewed his
patches.   :-)



Agreed. I understand that this FAQ probably was needed at one time and
Brendan added it to address the concerns.

I think at some point we do need to have a document that outlines when
to KUnit and when to use Kselftest modules. I think one concern people
have is that if KUnit is perceived as a  replacement for Ksefltest
module, Kselftest module will be ignored leaving users without the
ability to build and run with Kselftest modules and load them on a need
basis to gather data on a systems that aren't dedicated strictly for
testing.

I am trying to move the conversation forward from KUnit vs. Kselftest
modules discussion to which problem areas each one addresses keeping
in mind that it is not about which is better. Kselftest and KUnit both
have their place in the kernel development process. We just have to be
clear on usage as we write tests for each.

thanks,
-- Shuah


___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v5 00/18] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-06-21 Thread shuah
 use it using the following grep command:

grep -Hrn -e 'kselftest_module\.h'

and only got three results: lib/test_strscpy.c, lib/test_printf.c, and
lib/test_bitmap.c.




Again, unnecessary. KUnit can't replace Kselftest module in the way
Kselftest module support can be used for debugging and gathering
information on system that might be in active use and not dedicated
to test and development alone. I wouldn't hesitate loading a Kselftest
test module on my laptop and running tests, but I wouldn't use KUnit
the same way.

Again, this is not a competition between which is better. Kselftest
and KUnit serve different needs and problem spaces.

Please redo this documentation to reflect that.

thanks,
-- Shuah


___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-05-07 Thread shuah

On 5/7/19 2:01 AM, Greg KH wrote:

On Mon, May 06, 2019 at 08:14:12PM -0700, Frank Rowand wrote:

On 5/1/19 4:01 PM, Brendan Higgins wrote:

## TLDR

I rebased the last patchset on 5.1-rc7 in hopes that we can get this in
5.2.

Shuah, I think you, Greg KH, and myself talked off thread, and we agreed
we would merge through your tree when the time came? Am I remembering
correctly?

## Background

This patch set proposes KUnit, a lightweight unit testing and mocking
framework for the Linux kernel.

Unlike Autotest and kselftest, KUnit is a true unit testing framework;
it does not require installing the kernel on a test machine or in a VM
and does not require tests to be written in userspace running on a host
kernel. Additionally, KUnit is fast: From invocation to completion KUnit
can run several dozen tests in under a second. Currently, the entire
KUnit test suite for KUnit runs in under a second from the initial
invocation (build time excluded).

KUnit is heavily inspired by JUnit, Python's unittest.mock, and
Googletest/Googlemock for C++. KUnit provides facilities for defining
unit test cases, grouping related test cases into test suites, providing
common infrastructure for running tests, mocking, spying, and much more.


As a result of the emails replying to this patch thread, I am now
starting to look at kselftest.  My level of understanding is based
on some slide presentations, an LWN article, https://kselftest.wiki.kernel.org/
and a _tiny_ bit of looking at kselftest code.

tl;dr; I don't really understand kselftest yet.


(1) why KUnit exists


## What's so special about unit testing?

A unit test is supposed to test a single unit of code in isolation,
hence the name. There should be no dependencies outside the control of
the test; this means no external dependencies, which makes tests orders
of magnitudes faster. Likewise, since there are no external dependencies,
there are no hoops to jump through to run the tests. Additionally, this
makes unit tests deterministic: a failing unit test always indicates a
problem. Finally, because unit tests necessarily have finer granularity,
they are able to test all code paths easily solving the classic problem
of difficulty in exercising error handling code.


(2) KUnit is not meant to replace kselftest


## Is KUnit trying to replace other testing frameworks for the kernel?

No. Most existing tests for the Linux kernel are end-to-end tests, which
have their place. A well tested system has lots of unit tests, a
reasonable number of integration tests, and some end-to-end tests. KUnit
is just trying to address the unit test space which is currently not
being addressed.


My understanding is that the intent of KUnit is to avoid booting a kernel on
real hardware or in a virtual machine.  That seems to be a matter of semantics
to me because isn't invoking a UML Linux just running the Linux kernel in
a different form of virtualization?

So I do not understand why KUnit is an improvement over kselftest.


They are in two different categories. Kselftest falls into black box
regression test suite which is a collection of user-space tests with a
few kernel test modules back-ending the tests in some cases.

Kselftest can be used by both kernel developers and users and provides
a good way to regression test releases in test rings.

KUnit is a white box category and is a better fit as unit test framework
for development and provides a in-kernel testing. I wouldn't view them
one replacing the other. They just provide coverage for different areas
of testing.

I wouldn't view KUnit as something that would be easily run in test 
rings for example.


Brendan, does that sound about right?



It seems to me that KUnit is just another piece of infrastructure that I
am going to have to be familiar with as a kernel developer.  More overhead,
more information to stuff into my tiny little brain.

I would guess that some developers will focus on just one of the two test
environments (and some will focus on both), splitting the development
resources instead of pooling them on a common infrastructure.



What am I missing?


kselftest provides no in-kernel framework for testing kernel code
specifically.  That should be what kunit provides, an "easy" way to
write in-kernel tests for things.

Brendan, did I get it right?

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2 15/17] MAINTAINERS: add entry for KUnit the unit testing framework

2019-05-03 Thread shuah

On 5/1/19 5:01 PM, Brendan Higgins wrote:

Add myself as maintainer of KUnit, the Linux kernel's unit testing
framework.

Signed-off-by: Brendan Higgins 
---
  MAINTAINERS | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 5c38f21aee787..c78ae95c56b80 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8448,6 +8448,16 @@ S:   Maintained
  F:tools/testing/selftests/
  F:Documentation/dev-tools/kselftest*
  
+KERNEL UNIT TESTING FRAMEWORK (KUnit)

+M: Brendan Higgins 
+L: kunit-...@googlegroups.com
+W: https://google.github.io/kunit-docs/third_party/kernel/docs/
+S: Maintained
+F: Documentation/kunit/
+F: include/kunit/
+F: kunit/
+F: tools/testing/kunit/
+


Please add kselftest mailing list to this entry, based on our
conversation on taking these patches through kselftest tree.

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2 11/17] kunit: test: add test managed resource tests

2019-05-03 Thread shuah

On 5/1/19 5:01 PM, Brendan Higgins wrote:

From: Avinash Kondareddy 

Tests how tests interact with test managed resources in their lifetime.

Signed-off-by: Avinash Kondareddy 
Signed-off-by: Brendan Higgins 
---


I think this change log could use more details. It is vague on what it
does.

thanks,
-- Shuah

___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2 04/17] kunit: test: add kunit_stream a std::stream like logger

2019-05-02 Thread shuah
id kunit_stream_set_level(struct kunit_stream *this, const char *level)
+{
+   unsigned long flags;
+
+   spin_lock_irqsave(&this->lock, flags);
+   this->level = level;
+   spin_unlock_irqrestore(&this->lock, flags);
+}
+
+void kunit_stream_add(struct kunit_stream *this, const char *fmt, ...)
+{
+   va_list args;
+   struct string_stream *stream = this->internal_stream;
+
+   va_start(args, fmt);
+
+   if (string_stream_vadd(stream, fmt, args) < 0)
+   kunit_err(this->test, "Failed to allocate fragment: %s\n", fmt);
+
+   va_end(args);
+}
+
+void kunit_stream_append(struct kunit_stream *this,
+   struct kunit_stream *other)
+{
+   struct string_stream *other_stream = other->internal_stream;
+   const char *other_content;
+
+   other_content = string_stream_get_string(other_stream);
+
+   if (!other_content) {
+   kunit_err(this->test,
+ "Failed to get string from second argument for 
appending.\n");
+   return;
+   }
+
+   kunit_stream_add(this, other_content);
+}
+
+void kunit_stream_clear(struct kunit_stream *this)
+{
+   string_stream_clear(this->internal_stream);
+}
+
+void kunit_stream_commit(struct kunit_stream *this)
+{
+   struct string_stream *stream = this->internal_stream;
+   struct string_stream_fragment *fragment;
+   const char *level;
+   char *buf;
+
+   level = kunit_stream_get_level(this);
+   if (!level) {
+   kunit_err(this->test,
+ "Stream was committed without a specified log 
level.\n");
+   level = KERN_ERR;
+   kunit_stream_set_level(this, level);
+   }
+
+   buf = string_stream_get_string(stream);
+   if (!buf) {
+   kunit_err(this->test,
+"Could not allocate buffer, dumping stream:\n");
+   list_for_each_entry(fragment, &stream->fragments, node) {
+   kunit_err(this->test, fragment->fragment);
+   }
+   kunit_err(this->test, "\n");
+   goto cleanup;
+   }
+
+   kunit_printk(level, this->test, buf);
+   kfree(buf);
+
+cleanup:
+   kunit_stream_clear(this);
+}
+
+static int kunit_stream_init(struct kunit_resource *res, void *context)
+{
+   struct kunit *test = context;
+   struct kunit_stream *stream;
+
+   stream = kzalloc(sizeof(*stream), GFP_KERNEL);
+   if (!stream)
+   return -ENOMEM;
+   res->allocation = stream;
+   stream->test = test;
+   spin_lock_init(&stream->lock);
+   stream->internal_stream = new_string_stream();
+
+   if (!stream->internal_stream)
+   return -ENOMEM;


What happens to stream? Don't you want to free that?


+
+   return 0;
+}
+
+static void kunit_stream_free(struct kunit_resource *res)
+{
+   struct kunit_stream *stream = res->allocation;
+
+   if (!string_stream_is_empty(stream->internal_stream)) {
+   kunit_err(stream->test,
+"End of test case reached with uncommitted stream 
entries.\n");
+   kunit_stream_commit(stream);
+   }
+
+   destroy_string_stream(stream->internal_stream);
+   kfree(stream);
+}
+
+struct kunit_stream *kunit_new_stream(struct kunit *test)
+{
+   struct kunit_resource *res;
+
+   res = kunit_alloc_resource(test,
+  kunit_stream_init,
+  kunit_stream_free,
+  test);
+
+   if (res)
+   return res->allocation;
+   else
+   return NULL;
+}
diff --git a/kunit/test.c b/kunit/test.c
index 541f9adb1608c..f7575b127e2df 100644
--- a/kunit/test.c
+++ b/kunit/test.c
@@ -63,12 +63,20 @@ static void kunit_vprintk(const struct kunit *test,
  "kunit %s: %pV", test->name, vaf);
  }
  
+static void kunit_fail(struct kunit *test, struct kunit_stream *stream)

+{
+   kunit_set_success(test, false);
+   kunit_stream_set_level(stream, KERN_ERR);
+   kunit_stream_commit(stream);
+}
+
  int kunit_init_test(struct kunit *test, const char *name)
  {
spin_lock_init(&test->lock);
INIT_LIST_HEAD(&test->resources);
test->name = name;
test->vprintk = kunit_vprintk;
+   test->fail = kunit_fail;
  
  	return 0;

  }



thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2 03/17] kunit: test: add string_stream a std::stream like string builder

2019-05-02 Thread shuah
structure
name to begin with. :)


+   }
+   this->length = 0;
+   spin_unlock_irqrestore(&this->lock, flags);
+}
+
+char *string_stream_get_string(struct string_stream *this)
+{
+   struct string_stream_fragment *fragment;
+   size_t buf_len = this->length + 1; /* +1 for null byte. */
+   char *buf;
+   unsigned long flags;
+
+   buf = kzalloc(buf_len, GFP_KERNEL);
+   if (!buf)
+   return NULL;
+
+   spin_lock_irqsave(&this->lock, flags);
+   list_for_each_entry(fragment, &this->fragments, node)
+   strlcat(buf, fragment->fragment, buf_len);
+   spin_unlock_irqrestore(&this->lock, flags);
+
+   return buf;
+}
+
+bool string_stream_is_empty(struct string_stream *this)
+{
+   bool is_empty;
+   unsigned long flags;
+
+   spin_lock_irqsave(&this->lock, flags);
+   is_empty = list_empty(&this->fragments);
+   spin_unlock_irqrestore(&this->lock, flags);
+
+   return is_empty;
+}
+
+void destroy_string_stream(struct string_stream *stream)
+{
+   string_stream_clear(stream);
+   kfree(stream);
+}
+
+static void string_stream_destroy(struct kref *kref)
+{
+   struct string_stream *stream = container_of(kref,
+   struct string_stream,
+   refcount);
+   destroy_string_stream(stream);
+}
+
+struct string_stream *new_string_stream(void)
+{
+   struct string_stream *stream = kzalloc(sizeof(*stream), GFP_KERNEL);
+
+   if (!stream)
+   return NULL;
+
+   INIT_LIST_HEAD(&stream->fragments);
+   spin_lock_init(&stream->lock);
+   kref_init(&stream->refcount);
+   return stream;
+}
+
+void string_stream_get(struct string_stream *stream)
+{
+   kref_get(&stream->refcount);
+}
+
+int string_stream_put(struct string_stream *stream)
+{
+   return kref_put(&stream->refcount, &string_stream_destroy);
+}
+



thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [PATCH v2 07/17] kunit: test: add initial tests

2019-05-02 Thread shuah

On 5/1/19 5:01 PM, Brendan Higgins wrote:

Add a test for string stream along with a simpler example.

Signed-off-by: Brendan Higgins 
---
  kunit/Kconfig  | 12 ++
  kunit/Makefile |  4 ++
  kunit/example-test.c   | 88 ++
  kunit/string-stream-test.c | 61 ++
  4 files changed, 165 insertions(+)
  create mode 100644 kunit/example-test.c
  create mode 100644 kunit/string-stream-test.c

diff --git a/kunit/Kconfig b/kunit/Kconfig
index 64480092b2c24..5cb500355c873 100644
--- a/kunit/Kconfig
+++ b/kunit/Kconfig
@@ -13,4 +13,16 @@ config KUNIT
  special hardware. For more information, please see
  Documentation/kunit/
  
+config KUNIT_TEST

+   bool "KUnit test for KUnit"
+   depends on KUNIT
+   help
+ Enables KUnit test to test KUnit.
+


Please add a bit more information on what this config option
does. Why should user care to enable it?


+config KUNIT_EXAMPLE_TEST
+   bool "Example test for KUnit"
+   depends on KUNIT
+   help
+ Enables example KUnit test to demo features of KUnit.
+


Same here.


  endmenu
diff --git a/kunit/Makefile b/kunit/Makefile
index 6ddc622ee6b1c..60a9ea6cb4697 100644
--- a/kunit/Makefile
+++ b/kunit/Makefile
@@ -1,3 +1,7 @@
  obj-$(CONFIG_KUNIT) +=test.o \
string-stream.o \
kunit-stream.o
+
+obj-$(CONFIG_KUNIT_TEST) +=string-stream-test.o
+
+obj-$(CONFIG_KUNIT_EXAMPLE_TEST) +=example-test.o
diff --git a/kunit/example-test.c b/kunit/example-test.c
new file mode 100644
index 0..3947dd7c8f922
--- /dev/null
+++ b/kunit/example-test.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Example KUnit test to show how to use KUnit.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#include 
+
+/*
+ * This is the most fundamental element of KUnit, the test case. A test case
+ * makes a set EXPECTATIONs and ASSERTIONs about the behavior of some code; if
+ * any expectations or assertions are not met, the test fails; otherwise, the
+ * test passes.
+ *
+ * In KUnit, a test case is just a function with the signature
+ * `void (*)(struct kunit *)`. `struct kunit` is a context object that stores
+ * information about the current test.
+ */
+static void example_simple_test(struct kunit *test)
+{
+   /*
+* This is an EXPECTATION; it is how KUnit tests things. When you want
+* to test a piece of code, you set some expectations about what the
+* code should do. KUnit then runs the test and verifies that the code's
+* behavior matched what was expected.
+*/
+   KUNIT_EXPECT_EQ(test, 1 + 1, 2);
+}
+
+/*
+ * This is run once before each test case, see the comment on
+ * example_test_module for more information.
+ */
+static int example_test_init(struct kunit *test)
+{
+   kunit_info(test, "initializing\n");
+
+   return 0;
+}
+
+/*
+ * Here we make a list of all the test cases we want to add to the test module
+ * below.
+ */
+static struct kunit_case example_test_cases[] = {
+   /*
+* This is a helper to create a test case object from a test case
+* function; its exact function is not important to understand how to
+* use KUnit, just know that this is how you associate test cases with a
+* test module.
+*/
+   KUNIT_CASE(example_simple_test),
+   {},
+};
+
+/*
+ * This defines a suite or grouping of tests.
+ *
+ * Test cases are defined as belonging to the suite by adding them to
+ * `kunit_cases`.
+ *
+ * Often it is desirable to run some function which will set up things which
+ * will be used by every test; this is accomplished with an `init` function
+ * which runs before each test case is invoked. Similarly, an `exit` function
+ * may be specified which runs after every test case and can be used to for
+ * cleanup. For clarity, running tests in a test module would behave as 
follows:
+ *
+ * module.init(test);
+ * module.test_case[0](test);
+ * module.exit(test);
+ * module.init(test);
+ * module.test_case[1](test);
+ * module.exit(test);
+ * ...;
+ */
+static struct kunit_module example_test_module = {
+   .name = "example",
+   .init = example_test_init,
+   .test_cases = example_test_cases,
+};
+
+/*
+ * This registers the above test module telling KUnit that this is a suite of
+ * tests that need to be run.
+ */
+module_test(example_test_module);
diff --git a/kunit/string-stream-test.c b/kunit/string-stream-test.c
new file mode 100644
index 0..b2a98576797c9
--- /dev/null
+++ b/kunit/string-stream-test.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit test for struct string_stream.
+ *
+ * Copyright (C) 2019, Google LLC.
+ * Author: Brendan Higgins 
+ */
+
+#include 
+#include 
+#include 
+
+static void string_stream_test_get_stri

Re: [PATCH v2 00/17] kunit: introduce KUnit, the Linux kernel unit testing framework

2019-05-02 Thread shuah

On 5/2/19 4:50 AM, Greg KH wrote:

On Wed, May 01, 2019 at 04:01:09PM -0700, Brendan Higgins wrote:

## TLDR

I rebased the last patchset on 5.1-rc7 in hopes that we can get this in
5.2.


That might be rushing it, normally trees are already closed now for
5.2-rc1 if 5.1-final comes out this Sunday.


Shuah, I think you, Greg KH, and myself talked off thread, and we agreed
we would merge through your tree when the time came? Am I remembering
correctly?


No objection from me.



Yes. I can take these through kselftest tree when the time comes.
Agree with Greg that 5.2 might be rushing it. 5.3 would be a good
target.

thanks,
-- Shuah



___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [RFC v2 00/14] kunit: introduce KUnit, the Linux kernel unit testing framework

2018-11-28 Thread shuah

On 11/28/18 12:54 PM, Knut Omang wrote:

On Mon, 2018-11-26 at 17:41 -0800, Brendan Higgins wrote:

On Fri, Nov 23, 2018 at 9:15 PM Knut Omang  wrote:


On Tue, 2018-10-23 at 16:57 -0700, Brendan Higgins wrote:




Brendan, I regret you weren't at this year's testing and fuzzing workshop at
LPC last week so we could have continued our discussions from last year there!


Likewise! Unfortunately, I could not make it. So it goes.



I hope we can work on this for a while longer before anything gets merged.
Maybe it can be topic for a longer session in a future test related workshop?


I don't see why we cannot just discuss it here as we are already
doing.


Yes, as long as we are not wasting all the Cc:'ed people's valuable time.


Besides, you are mostly interested in out of tree testing,
right? I don't see how this precludes anything that you are trying to
do with KTF.


Both approaches provide assertion macros for running tests inside the kernel,
I doubt the kernel community would like to see yet two such sets of macros,
with differing syntax merged. One of the good reasons to have a *framework*
is that it is widely used and understood, so that people coming from one part 
of the
kernel can easily run, understand and relate to selected tests from another 
part.

The goal with KTF is to allow this for any kernel, old or new, not just kernels
built specifically for testing purposes. We felt that providing it as a 
separate git
module (still open source, for anyone to contribute to) is a more efficient 
approach
until we have more examples and experience with using it in different parts
of the kernel. We can definitely post the kernel side of KTF as a patch series 
fairly soon
if the community wants us to. Except for hybrid tests, the ktf.ko module works 
fine
independently of any user side support, just using the debugfs interface to run 
and
examine tests.



Having test framework in the kernel sources tree has benefits. It allows
us (kernel developers) to do co-development of kernel features and tests
for these features.

It encourages developers to write regression tests. More importantly,
kernel features and tests for these features are included in the same
release in most cases and/or allows us freedom to do so if test 
framework and tests are part of the kernel sources.


We have seen this with our experience with kselftest. It would not have
see the same level of attention and growth if it stayed outside the
kernel sources.

Most kernel developers would not want to include a externally maintained
module for testing. As a general rule, it has to be easy to run tests.


I think there are good uses cases for having the ability to maintain a
single source for tests that can be run against multiple kernels,
also distro kernels that the test framework cannot expect to be able to modify,
except from using the module interfaces.


Same reasons as above. Having the tests included in the kernel sources
makes it easier for distros to run those tests and include running them
during their qualification.



And there are good arguments for having (at least parts of)
the test framework easily available within the kernel under test.



When Kernel unit, functional, and regressions tests reside in the kernel
sources, they evolve quicker as kernel developers contribute tests as
part of their kernel work-flow. Maintaining tests and framework
separately will make it harder to maintain them and keep them updated
for us the kernel community.

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [RFC v2 01/14] kunit: test: add KUnit test runner core

2018-11-07 Thread Shuah Khan
On 11/06/2018 06:28 PM, Brendan Higgins wrote:
> On Fri, Nov 2, 2018 at 11:44 AM Shuah Khan  wrote:
>>
>> On 10/23/2018 05:57 PM, Brendan Higgins wrote:
> 
>>> + * Example:
>>> + *
>>> + * .. code-block:: c
>>> + *
>>> + *   void add_test_basic(struct test *test)
>>> + *   {
>>> + *   TEST_EXPECT_EQ(test, 1, add(1, 0));
>>> + *   TEST_EXPECT_EQ(test, 2, add(1, 1));
>>> + *   TEST_EXPECT_EQ(test, 0, add(-1, 1));
>>> + *   TEST_EXPECT_EQ(test, INT_MAX, add(0, INT_MAX));
>>> + *   TEST_EXPECT_EQ(test, -1, add(INT_MAX, INT_MIN));
>>> + *   }
>>> + *
>>> + *   static struct test_case example_test_cases[] = {
>>> + *   TEST_CASE(add_test_basic),
>>> + *   {},
>>> + *   };
>>> + *
>>> + */
>>> +struct test_case {
>>> + void (*run_case)(struct test *test);
>>> + const char name[256];
>>> +
>>> + /* private: internal use only. */
>>> + bool success;
>>> +};
>>> +
>>
>> Introducing a prefix kunit_* might be a good idea for the API.
>> This comment applies to the rest of patches as well.
> 
> What about kunit_* instead of test_* and kmock_* instead of mock_*?
> Does that seem reasonable?
> 

kunit_* would work well.

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [RFC v2 01/14] kunit: test: add KUnit test runner core

2018-11-02 Thread Shuah Khan
On 10/23/2018 05:57 PM, 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 
> ---
>  include/kunit/test.h | 165 ++
>  kunit/Kconfig|  17 +
>  kunit/Makefile   |   1 +
>  kunit/test.c | 168 +++
>  4 files changed, 351 insertions(+)
>  create mode 100644 include/kunit/test.h
>  create mode 100644 kunit/Kconfig
>  create mode 100644 kunit/Makefile
>  create mode 100644 kunit/test.c
> 
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> new file mode 100644
> index 0..e0b14b227ac44
> --- /dev/null
> +++ b/include/kunit/test.h
> @@ -0,0 +1,165 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Base unit test (KUnit) API.
> + *
> + * Copyright (C) 2018, Google LLC.
> + * Author: Brendan Higgins 
> + */
> +
> +#ifndef _KUNIT_TEST_H
> +#define _KUNIT_TEST_H
> +
> +#include 
> +#include 
> +
> +struct test;
> +
> +/**
> + * struct test_case - represents an individual test case.
> + * @run_case: the function representing the actual test case.
> + * @name: the name of the test case.
> + *
> + * A test case is a function with the signature, ``void (*)(struct test *)``
> + * that makes expectations (see TEST_EXPECT_TRUE()) about code under test. 
> Each
> + * test case is associated with a &struct test_module and will be run after 
> the
> + * module's init function and followed by the module's exit function.
> + *
> + * A test case should be static and should only be created with the 
> TEST_CASE()
> + * macro; additionally, every array of test cases should be terminated with 
> an
> + * empty test case.
> + *
> + * Example:
> + *
> + * .. code-block:: c
> + *
> + *   void add_test_basic(struct test *test)
> + *   {
> + *   TEST_EXPECT_EQ(test, 1, add(1, 0));
> + *   TEST_EXPECT_EQ(test, 2, add(1, 1));
> + *   TEST_EXPECT_EQ(test, 0, add(-1, 1));
> + *   TEST_EXPECT_EQ(test, INT_MAX, add(0, INT_MAX));
> + *   TEST_EXPECT_EQ(test, -1, add(INT_MAX, INT_MIN));
> + *   }
> + *
> + *   static struct test_case example_test_cases[] = {
> + *   TEST_CASE(add_test_basic),
> + *   {},
> + *   };
> + *
> + */
> +struct test_case {
> + void (*run_case)(struct test *test);
> + const char name[256];
> +
> + /* private: internal use only. */
> + bool success;
> +};
> +

Introducing a prefix kunit_* might be a good idea for the API.
This comment applies to the rest of patches as well.

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [RFC v2 00/14] kunit: introduce KUnit, the Linux kernel unit testing framework

2018-11-02 Thread Shuah Khan
Hi Brendan,


On 10/23/2018 05:57 PM, Brendan Higgins wrote:
> This patch set proposes KUnit, a lightweight unit testing and mocking
> framework for the Linux kernel.
> 
> Unlike Autotest and kselftest, KUnit is a true unit testing framework;
> it does not require installing the kernel on a test machine or in a VM
> and does not require tests to be written in userspace running on a host
> kernel. Additionally, KUnit is fast: From invocation to completion KUnit
> can run several dozen tests in under a second. Currently, the entire
> KUnit test suite for KUnit runs in under a second from the initial
> invocation (build time excluded).
> 
> KUnit is heavily inspired by JUnit, Python's unittest.mock, and
> Googletest/Googlemock for C++. KUnit provides facilities for defining
> unit test cases, grouping related test cases into test suites, providing
> common infrastructure for running tests, mocking, spying, and much more.
> 
> ## What's so special about unit testing?
> 
> A unit test is supposed to test a single unit of code in isolation,
> hence the name. There should be no dependencies outside the control of
> the test; this means no external dependencies, which makes tests orders
> of magnitudes faster. Likewise, since there are no external dependencies,
> there are no hoops to jump through to run the tests. Additionally, this
> makes unit tests deterministic: a failing unit test always indicates a
> problem. Finally, because unit tests necessarily have finer granularity,
> they are able to test all code paths easily solving the classic problem
> of difficulty in exercising error handling code.
> 
> ## Is KUnit trying to replace other testing frameworks for the kernel?
> 
> No. Most existing tests for the Linux kernel are end-to-end tests, which
> have their place. A well tested system has lots of unit tests, a
> reasonable number of integration tests, and some end-to-end tests. KUnit
> is just trying to address the unit test space which is currently not
> being addressed.
> 
> ## More information on KUnit
> 
> There is a bunch of documentation near the end of this patch set that
> describes how to use KUnit and best practices for writing unit tests.
> For convenience I am hosting the compiled docs here:
> https://google.github.io/kunit-docs/third_party/kernel/docs/
> 
> ## Changes Since Last Version
> 
>  - Updated patchset to apply cleanly on 4.19.
>  - Stripped down patchset to focus on just the core features (I dropped
>mocking, spying, and the MMIO stuff for now; you can find these
>patches here: https://kunit-review.googlesource.com/c/linux/+/1132),
>as suggested by Rob.
>  - Cleaned up some of the commit messages and tweaked commit order a
>bit based on suggestions.
>

Framework looks good. I think it would be helpful to include a real test
in the patch series to get a feel for how effective it is.

On one hand, KUnit stands on its own as its own and maybe it should be placed in
under tools/testing/KUnit, however I am wondering would it be beneficial for the
framework to under selftests.

I am a bit concerned about the number of test framework we have at the moment 
and
are we running the risk of fragmenting the landscape. I am concerned if this 
would
lead to developer confusion as to where to add tests.

That being said, I don't have a strong opinion one way or the other.

btw I started playing with kunit following the instructions and ran into 
problems:

./tools/testing/kunit/kunit.py 
usage: kunit.py [-h] {run,new} ...

Helps writing and running KUnit tests.

positional arguments:
  {run,new}
run   Runs KUnit tests.
new   Prints out boilerplate for writing new tests.

optional arguments:
  -h, --help  show this help message and exit

./tools/testing/kunit/kunit.py run
Regenerating .config ...
ERROR:root:Provided Kconfig is not contained in validated .config!

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm


Re: [RFC v2 00/14] kunit: introduce KUnit, the Linux kernel unit testing framework

2018-10-25 Thread Shuah Khan
On 10/23/2018 05:57 PM, Brendan Higgins wrote:
> This patch set proposes KUnit, a lightweight unit testing and mocking
> framework for the Linux kernel.
> 
> Unlike Autotest and kselftest, KUnit is a true unit testing framework;
> it does not require installing the kernel on a test machine or in a VM
> and does not require tests to be written in userspace running on a host
> kernel. Additionally, KUnit is fast: From invocation to completion KUnit
> can run several dozen tests in under a second. Currently, the entire
> KUnit test suite for KUnit runs in under a second from the initial
> invocation (build time excluded).
> 
> KUnit is heavily inspired by JUnit, Python's unittest.mock, and
> Googletest/Googlemock for C++. KUnit provides facilities for defining
> unit test cases, grouping related test cases into test suites, providing
> common infrastructure for running tests, mocking, spying, and much more.
> 
> ## What's so special about unit testing?
> 
> A unit test is supposed to test a single unit of code in isolation,
> hence the name. There should be no dependencies outside the control of
> the test; this means no external dependencies, which makes tests orders
> of magnitudes faster. Likewise, since there are no external dependencies,
> there are no hoops to jump through to run the tests. Additionally, this
> makes unit tests deterministic: a failing unit test always indicates a
> problem. Finally, because unit tests necessarily have finer granularity,
> they are able to test all code paths easily solving the classic problem
> of difficulty in exercising error handling code.
> 
> ## Is KUnit trying to replace other testing frameworks for the kernel?
> 
> No. Most existing tests for the Linux kernel are end-to-end tests, which
> have their place. A well tested system has lots of unit tests, a
> reasonable number of integration tests, and some end-to-end tests. KUnit
> is just trying to address the unit test space which is currently not
> being addressed.
> 
> ## More information on KUnit
> 
> There is a bunch of documentation near the end of this patch set that
> describes how to use KUnit and best practices for writing unit tests.
> For convenience I am hosting the compiled docs here:
> https://google.github.io/kunit-docs/third_party/kernel/docs/
> 
> ## Changes Since Last Version
> 
>  - Updated patchset to apply cleanly on 4.19.
>  - Stripped down patchset to focus on just the core features (I dropped
>mocking, spying, and the MMIO stuff for now; you can find these
>patches here: https://kunit-review.googlesource.com/c/linux/+/1132),
>as suggested by Rob.
>  - Cleaned up some of the commit messages and tweaked commit order a
>    bit based on suggestions.
> 

I am a bit behind on reviewing this patch series.

Just a quick note that I will start looking at these early next week.

thanks,
-- Shuah
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm