Re: [PATCH v18 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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