Re: [PATCH v2 5/6] kunit: mptcp: adhear to KUNIT formatting standard
Hi Matt, > Like patch 1/6, I can apply it in MPTCP tree and send it later to > net-next with other patches. > Except if you guys prefer to apply it in KUnit tree and send it to > linux-next? Given 1/6 is going to net-next, it makes sense to send this out that way too, then, IMHO. The only slight concern I have is that the m68k test config patch in the series will get split from the others, but that should resolve itself when they pick up the last patch. At the very least, this shouldn't cause any conflicts with anything we're doing in the KUnit tree. Cheers, -- David
Re: [PATCH v6] lib: add basic KUnit test for lib/math
On Sat, Apr 17, 2021 at 2:04 AM Daniel Latypov wrote: > > Add basic test coverage for files that don't require any config options: > * part of math.h (what seem to be the most commonly used macros) > * gcd.c > * lcm.c > * int_sqrt.c > * reciprocal_div.c > (Ignored int_pow.c since it's a simple textbook algorithm.) > > These tests aren't particularly interesting, but they > * provide short and simple examples of parameterized tests > * provide a place to add tests for any new files in this dir > * are written so adding new test cases to cover edge cases should be easy > * looking at code coverage, we hit all the branches in the .c files > > Signed-off-by: Daniel Latypov > Reviewed-by: David Gow > --- Thanks: I've tested this version, and am happy with it. A part of me still kind-of would like there to be names for the parameters, but I definitely understand that it doesn't really work well for the lcm and gcd cases where we're doing both (a,b) and (b,a). So let's keep it as-is. Hopefully we can get these in for 5.13! Cheers, -- David
[PATCH v2] Documentation: kunit: Update kunit_tool page
The kunit_tool documentation page was pretty minimal, and a bit outdated. Update it and flesh it out a bit. In particular, - Mention that .kunitconfig is now in the build directory - Describe the use of --kunitconfig to specify a different config framgent - Mention the split functionality (i.e., commands other than 'run') - Describe --raw_output and kunit.py parse - Mention the globbing support - Provide a quick overview of other options, including --build_dir and --alltests Note that this does overlap a little with the new running_tips page. I don't think it's a problem having both: this page is supposed to be a bit more of a reference, rather than a list of useful tips, so the fact that they both describe the same features isn't a problem. Signed-off-by: David Gow Reviewed-by: Daniel Latypov --- Adopted the changes from Daniel. Changes since v1: https://lore.kernel.org/linux-kselftest/20210416034036.797727-1-david...@google.com/ - Mention that the default build directory is '.kunit' when discussing '.kunitconfig' files. - Reword the discussion of 'CONFIG_KUNIT_ALL_TESTS' under '--alltests' Documentation/dev-tools/kunit/kunit-tool.rst | 140 +-- 1 file changed, 132 insertions(+), 8 deletions(-) diff --git a/Documentation/dev-tools/kunit/kunit-tool.rst b/Documentation/dev-tools/kunit/kunit-tool.rst index 29ae2fee8123..4247b7420e3b 100644 --- a/Documentation/dev-tools/kunit/kunit-tool.rst +++ b/Documentation/dev-tools/kunit/kunit-tool.rst @@ -22,14 +22,19 @@ not require any virtualization support: it is just a regular program. What is a .kunitconfig? === -It's just a defconfig that kunit_tool looks for in the base directory. -kunit_tool uses it to generate a .config as you might expect. In addition, it -verifies that the generated .config contains the CONFIG options in the -.kunitconfig; the reason it does this is so that it is easy to be sure that a -CONFIG that enables a test actually ends up in the .config. +It's just a defconfig that kunit_tool looks for in the build directory +(``.kunit`` by default). kunit_tool uses it to generate a .config as you might +expect. In addition, it verifies that the generated .config contains the CONFIG +options in the .kunitconfig; the reason it does this is so that it is easy to +be sure that a CONFIG that enables a test actually ends up in the .config. -How do I use kunit_tool? - +It's also possible to pass a separate .kunitconfig fragment to kunit_tool, +which is useful if you have several different groups of tests you wish +to run independently, or if you want to use pre-defined test configs for +certain subsystems. + +Getting Started with kunit_tool +=== If a kunitconfig is present at the root directory, all you have to do is: @@ -48,10 +53,129 @@ However, you most likely want to use it with the following options: .. note:: This command will work even without a .kunitconfig file: if no -.kunitconfig is present, a default one will be used instead. + .kunitconfig is present, a default one will be used instead. + +If you wish to use a different .kunitconfig file (such as one provided for +testing a particular subsystem), you can pass it as an option. + +.. code-block:: bash + + ./tools/testing/kunit/kunit.py run --kunitconfig=fs/ext4/.kunitconfig For a list of all the flags supported by kunit_tool, you can run: .. code-block:: bash ./tools/testing/kunit/kunit.py run --help + +Configuring, Building, and Running Tests + + +It's also possible to run just parts of the KUnit build process independently, +which is useful if you want to make manual changes to part of the process. + +A .config can be generated from a .kunitconfig by using the ``config`` argument +when running kunit_tool: + +.. code-block:: bash + + ./tools/testing/kunit/kunit.py config + +Similarly, if you just want to build a KUnit kernel from the current .config, +you can use the ``build`` argument: + +.. code-block:: bash + + ./tools/testing/kunit/kunit.py build + +And, if you already have a built UML kernel with built-in KUnit tests, you can +run the kernel and display the test results with the ``exec`` argument: + +.. code-block:: bash + + ./tools/testing/kunit/kunit.py exec + +The ``run`` command which is discussed above is equivalent to running all three +of these in sequence. + +All of these commands accept a number of optional command-line arguments. The +``--help`` flag will give a complete list of these, or keep reading this page +for a guide to some of the more useful ones. + +Parsing Test Results + + +KUnit tests output their results in TAP (Test Anything Protocol) format. +kunit_tool will, when running tests, parse this output and print a summary +which is much more pleasant to read. If you wish to look at the raw test +results in TAP format, you can
[PATCH v8] fat: Add KUnit tests for checksums and timestamps
Add some basic sanity-check tests for the fat_checksum() function and the fat_time_unix2fat() and fat_time_fat2unix() functions. These unit tests verify these functions return correct output for a number of test inputs. These tests were inspored by -- and serve a similar purpose to -- the timestamp parsing KUnit tests in ext4[1]. Note that, unlike fat_time_unix2fat, fat_time_fat2unix wasn't previously exported, so this patch exports it as well. This is required for the case where we're building the fat and fat_test as modules. [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/inode-test.c Signed-off-by: David Gow Acked-by: OGAWA Hirofumi --- It's been a while, but this hopefully is a final version of the FAT KUnit patchset. It has a number of changes to keep it up-to-date with current KUnit standards, notably the use of parameterised tests and the addition of a '.kunitconfig' file to allow for easy testing. It also fixes an endianness tagging issue picked up by the kernel test robot under sparse on pa-risc. Cheers, -- David Changes since v7: https://lore.kernel.org/linux-kselftest/20201028064631.3774908-1-david...@google.com/ - Make the two timestamp tests parameterised: this means that the KUnit runtime and tooling are aware of the different testcases (and print a nice list of them to the TAP log when the test is run). - Fix some issues sparse picked up with __le32 tagged integers. - Add an fs/fat/.kunitconfig file which contains all the Kconfig entries needed to run the test. The test can now be run with: ./tools/testing/kunit/kunit.py run --kunitconfig fs/fat/.kunitconfig Changes since v6: https://lore.kernel.org/linux-kselftest/20201024060558.2556249-1-david...@google.com/ - Make CONFIG_FAT_DEFAULT_CODEPAGE depend on FAT_FS, rather than either VFAT_FS or MSDOS_FS. - This means that FAT_KUNIT_TEST can now also just depend of FAT_FS - Fix a few warnings that KUnit tool was eating: - KUnit's type checking needs a specific cast for the fat_checksum() expected results. - The time test cases shouldn't be 'const' - The fake superblock is now static, as otherwise it increased the stack size too much. Changes since v4/5: https://lore.kernel.org/linux-kselftest/20201024052047.2526780-1-david...@google.com/ - Fix a typo introduced in the Kconfig. It builds now. Changes since v3: https://lore.kernel.org/linux-kselftest/20201021061713.1545931-1-david...@google.com/ - Update the Kconfig entry to use "depends on" rather than "select", as discussed in [2]. - Depend on "MSDOS_FS || VFAT_FS", rather than "FAT_FS", as we need the CONFIG_FAT_DEFAULT_CODEPAGE symbol to be defined. Changes since v2: https://lore.kernel.org/linux-kselftest/20201020055856.1270482-1-david...@google.com/ - Comment that the export for fat_time_fat2unix() function is for KUnit tests. Changes since v1: https://lore.kernel.org/linux-kselftest/20201017064107.375174-1-david...@google.com/ - Now export fat_time_fat2unix() so that the test can access it when built as a module. [2]: https://lore.kernel.org/linux-ext4/52959e99-4105-3de9-730c-c46894b82...@infradead.org/T/#t fs/fat/.kunitconfig | 5 ++ fs/fat/Kconfig | 14 +++- fs/fat/Makefile | 2 + fs/fat/fat_test.c | 197 fs/fat/misc.c | 2 + 5 files changed, 219 insertions(+), 1 deletion(-) create mode 100644 fs/fat/.kunitconfig create mode 100644 fs/fat/fat_test.c diff --git a/fs/fat/.kunitconfig b/fs/fat/.kunitconfig new file mode 100644 index ..0a6971dbeccb --- /dev/null +++ b/fs/fat/.kunitconfig @@ -0,0 +1,5 @@ +CONFIG_KUNIT=y +CONFIG_FAT_FS=y +CONFIG_MSDOS_FS=y +CONFIG_VFAT_FS=y +CONFIG_FAT_KUNIT_TEST=y diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig index 66532a71e8fd..238cc55f84c4 100644 --- a/fs/fat/Kconfig +++ b/fs/fat/Kconfig @@ -77,7 +77,7 @@ config VFAT_FS config FAT_DEFAULT_CODEPAGE int "Default codepage for FAT" - depends on MSDOS_FS || VFAT_FS + depends on FAT_FS default 437 help This option should be set to the codepage of your FAT filesystems. @@ -115,3 +115,15 @@ config FAT_DEFAULT_UTF8 Say Y if you use UTF-8 encoding for file names, N otherwise. See for more information. + +config FAT_KUNIT_TEST + tristate "Unit Tests for FAT filesystems" if !KUNIT_ALL_TESTS + depends on KUNIT && FAT_FS + default KUNIT_ALL_TESTS + help + This builds the FAT KUnit tests + + For more information on KUnit and unit tests in general, please refer + to the KUnit documentation in Documentation/dev-tools/kunit + + If unsure, say N diff --git a/fs/fat/Makefile b/fs/fat/Makefile index 70645ce2f7fc..2b034112690d 100644 --- a/fs/fat/Makefile +++ b/fs/fat/Makefile @@ -10,3 +10,5 @@ obj-$(CONFIG_MSDOS_FS) += msdos.o fat-y := cache.o dir.o f
[PATCH] Documentation: kunit: Update kunit_tool page
The kunit_tool documentation page was pretty minimal, and a bit outdated. Update it and flesh it out a bit. In particular, - Mention that .kunitconfig is now in the build directory - Describe the use of --kunitconfig to specify a different config framgent - Mention the split functionality (i.e., commands other than 'run') - Describe --raw_output and kunit.py parse - Mention the globbing support - Provide a quick overview of other options, including --build_dir and --alltests Note that this does overlap a little with the new running_tips page. I don't think it's a problem having both: this page is supposed to be a bit more of a reference, rather than a list of useful tips, so the fact that they both describe the same features isn't a problem. Signed-off-by: David Gow --- Documentation/dev-tools/kunit/kunit-tool.rst | 132 ++- 1 file changed, 128 insertions(+), 4 deletions(-) diff --git a/Documentation/dev-tools/kunit/kunit-tool.rst b/Documentation/dev-tools/kunit/kunit-tool.rst index 29ae2fee8123..0b45affcd65c 100644 --- a/Documentation/dev-tools/kunit/kunit-tool.rst +++ b/Documentation/dev-tools/kunit/kunit-tool.rst @@ -22,14 +22,19 @@ not require any virtualization support: it is just a regular program. What is a .kunitconfig? === -It's just a defconfig that kunit_tool looks for in the base directory. +It's just a defconfig that kunit_tool looks for in the build directory. kunit_tool uses it to generate a .config as you might expect. In addition, it verifies that the generated .config contains the CONFIG options in the .kunitconfig; the reason it does this is so that it is easy to be sure that a CONFIG that enables a test actually ends up in the .config. -How do I use kunit_tool? - +It's also possible to pass a separate .kunitconfig fragment to kunit_tool, +which is useful if you have several different groups of tests you wish +to run independently, or if you want to use pre-defined test configs for +certain subsystems. + +Getting Started with kunit_tool +=== If a kunitconfig is present at the root directory, all you have to do is: @@ -48,10 +53,129 @@ However, you most likely want to use it with the following options: .. note:: This command will work even without a .kunitconfig file: if no -.kunitconfig is present, a default one will be used instead. + .kunitconfig is present, a default one will be used instead. + +If you wish to use a different .kunitconfig file (such as one provided for +testing a particular subsystem), you can pass it as an option. + +.. code-block:: bash + + ./tools/testing/kunit/kunit.py run --kunitconfig=fs/ext4/.kunitconfig For a list of all the flags supported by kunit_tool, you can run: .. code-block:: bash ./tools/testing/kunit/kunit.py run --help + +Configuring, Building, and Running Tests + + +It's also possible to run just parts of the KUnit build process independently, +which is useful if you want to make manual changes to part of the process. + +A .config can be generated from a .kunitconfig by using the ``config`` argument +when running kunit_tool: + +.. code-block:: bash + + ./tools/testing/kunit/kunit.py config + +Similarly, if you just want to build a KUnit kernel from the current .config, +you can use the ``build`` argument: + +.. code-block:: bash + + ./tools/testing/kunit/kunit.py build + +And, if you already have a built UML kernel with built-in KUnit tests, you can +run the kernel and display the test results with the ``exec`` argument: + +.. code-block:: bash + + ./tools/testing/kunit/kunit.py exec + +The ``run`` command which is discussed above is equivalent to running all three +of these in sequence. + +All of these commands accept a number of optional command-line arguments. The +``--help`` flag will give a complete list of these, or keep reading this page +for a guide to some of the more useful ones. + +Parsing Test Results + + +KUnit tests output their results in TAP (Test Anything Protocol) format. +kunit_tool will, when running tests, parse this output and print a summary +which is much more pleasant to read. If you wish to look at the raw test +results in TAP format, you can pass the ``--raw_output`` argument. + +.. code-block:: bash + + ./tools/testing/kunit/kunit.py run --raw_output + +.. note:: + The raw output from test runs may contain other, non-KUnit kernel log + lines. + +If you have KUnit results in their raw TAP format, you can parse them and print +the human-readable summary with the ``parse`` command for kunit_tool. This +accepts a filename for an argument, or will read from standard input. + +.. code-block:: bash + + # Reading from a file + ./tools/testing/kunit/kunit.py parse /var/log/dmesg + # Reading from stdin + dmesg | ./tools/testing/kunit/kunit.py parse
Re: [PATCH v2 5/6] kunit: mptcp: adhear to KUNIT formatting standard
Hi Nico, Matthieu, Thanks for going to the trouble of making these conform to the KUnit style guidelines. On Wed, Apr 14, 2021 at 5:25 PM Matthieu Baerts wrote: > > Hi Nico, > > On 14/04/2021 10:58, Nico Pache wrote: > > Drop 'S' from end of CONFIG_MPTCP_KUNIT_TESTS inorder to adhear to the > > KUNIT *_KUNIT_TEST config name format. [Super nitpicky comment for this series: It's 'adhere', not 'adhear'. It's not worth resending this out just to fix the spelling here, but if you do another version, it might be worth fiing.] > > For MPTCP, we have multiple KUnit tests: crypto and token. That's why we > wrote TESTS with a S. So (as this patch series attests), there are a few different config options which cover (or intend one day to cover) multiple suites, and hence end in KUNIT_TESTS. Personally, I'd still slightly prefer TEST here, just to have a common suffix for KUnit test options, and that's what I was going for when writing the style guide. That being said, it's also worth noting that there is an explicit exemption for existing tests, so if you (for example) have a bunch of automation which relies on this name and can't easily be changed, that's probably more important than a lone 'S'. > I'm fine without S if we need to stick with KUnit' standard. But maybe > the standard wants us to split the two tests and create > MPTCP_TOKEN_KUNIT_TEST and MPTCP_TOKEN_KUNIT_TEST config? > > In this case, we could eventually keep MPTCP_KUNIT_TESTS which will in > charge of selecting the two new ones. This is certainly an option if you want to do it, but personally I wouldn't bother unless it gives you some real advantage. One thing I would note, however, is that it's possible to have a per-subsystem '.kunitconfig' file, so if you want to run a particular group of tests (i.e., have a particular set of config options for testing), it'd be possible to have that rather than a meta-Kconfig entry. While there are some advantages to trying to have a 1:1 suite:config mapping, there's isn't actually anything that depends on it at the moment (or indeed, anything actively planned). So, in my view, there's no need for you to split the config entry unless you think there's a good reason you'd want to be able to build one set of tests but not the other. > Up to the KUnit maintainers to decide ;-) To summarise my view: personally, I'd prefer things the way this patch works: have everything end in _KUNIT_TEST, even if that enables a couple of suites. The extra 'S' on the end isn't a huge problem if you have a good reason to particularly want to keep it, though: as long as you don't have something like _K_UNIT_VERIFICATION or something equally silly that'd break grepping for '_KUNIT_TEST', it's fine be me. So, since it matches my prejudices, this patch is: Reviewed-by: David Gow Thanks, -- David
[PATCH v3] Documentation: dev-tools: Add Testing Overview
The kernel now has a number of testing and debugging tools, and we've seen a bit of confusion about what the differences between them are. Add a basic documentation outlining the testing tools, when to use each, and how they interact. This is a pretty quick overview rather than the idealised "kernel testing guide" that'd probably be optimal, but given the number of times questions like "When do you use KUnit and when do you use Kselftest?" are being asked, it seemed worth at least having something. Hopefully this can form the basis for more detailed documentation later. Signed-off-by: David Gow Reviewed-by: Marco Elver Reviewed-by: Daniel Latypov --- Thanks again. Assuming no-one has any objections, I think this is good to go. -- David Changes since v2: https://lore.kernel.org/linux-kselftest/20210414081428.337494-1-david...@google.com/ - A few typo fixes (Thanks Daniel) - Reworded description of dynamic analysis tools. - Updated dev-tools index page to not use ':doc:' syntax, but to provide a path instead. - Added Marco and Daniel's Reviewed-by tags. Changes since v1: https://lore.kernel.org/linux-kselftest/20210410070529.4113432-1-david...@google.com/ - Note KUnit's speed and that one should provide selftests for syscalls - Mention lockdep as a Dynamic Analysis Tool - Refer to "Dynamic Analysis Tools" instead of "Sanitizers" - A number of minor formatting tweaks and rewordings for clarity Documentation/dev-tools/index.rst| 4 + Documentation/dev-tools/testing-overview.rst | 117 +++ 2 files changed, 121 insertions(+) create mode 100644 Documentation/dev-tools/testing-overview.rst diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst index 1b1cf4f5c9d9..929d916ffd4c 100644 --- a/Documentation/dev-tools/index.rst +++ b/Documentation/dev-tools/index.rst @@ -7,6 +7,9 @@ be used to work on the kernel. For now, the documents have been pulled together without any significant effort to integrate them into a coherent whole; patches welcome! +A brief overview of testing-specific tools can be found in +Documentation/dev-tools/testing-overview.rst + .. class:: toc-title Table of contents @@ -14,6 +17,7 @@ whole; patches welcome! .. toctree:: :maxdepth: 2 + testing-overview coccinelle sparse kcov diff --git a/Documentation/dev-tools/testing-overview.rst b/Documentation/dev-tools/testing-overview.rst new file mode 100644 index ..b5b46709969c --- /dev/null +++ b/Documentation/dev-tools/testing-overview.rst @@ -0,0 +1,117 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +Kernel Testing Guide + + + +There are a number of different tools for testing the Linux kernel, so knowing +when to use each of them can be a challenge. This document provides a rough +overview of their differences, and how they fit together. + + +Writing and Running Tests += + +The bulk of kernel tests are written using either the kselftest or KUnit +frameworks. These both provide infrastructure to help make running tests and +groups of tests easier, as well as providing helpers to aid in writing new +tests. + +If you're looking to verify the behaviour of the Kernel — particularly specific +parts of the kernel — then you'll want to use KUnit or kselftest. + + +The Difference Between KUnit and kselftest +-- + +KUnit (Documentation/dev-tools/kunit/index.rst) is an entirely in-kernel system +for "white box" testing: because test code is part of the kernel, it can access +internal structures and functions which aren't exposed to userspace. + +KUnit tests therefore are best written against small, self-contained parts +of the kernel, which can be tested in isolation. This aligns well with the +concept of 'unit' testing. + +For example, a KUnit test might test an individual kernel function (or even a +single codepath through a function, such as an error handling case), rather +than a feature as a whole. + +This also makes KUnit tests very fast to build and run, allowing them to be +run frequently as part of the development process. + +There is a KUnit test style guide which may give further pointers in +Documentation/dev-tools/kunit/style.rst + + +kselftest (Documentation/dev-tools/kselftest.rst), on the other hand, is +largely implemented in userspace, and tests are normal userspace scripts or +programs. + +This makes it easier to write more complicated tests, or tests which need to +manipulate the overall system state more (e.g., spawning processes, etc.). +However, it's not possible to call kernel functions directly from kselftest. +This means that only kernel functionality which is exposed to userspace somehow +(e.g. by a syscall, device, filesystem, etc.) can be tested with kselftest. To +work around this, some tests include a companion kernel module which exposes +more i
Re: [PATCH v2] Documentation: dev-tools: Add Testing Overview
On Thu, Apr 15, 2021 at 12:30 AM Daniel Latypov wrote: > > On Wed, Apr 14, 2021 at 1:15 AM David Gow wrote: > > > > The kernel now has a number of testing and debugging tools, and we've > > seen a bit of confusion about what the differences between them are. > > > > Add a basic documentation outlining the testing tools, when to use each, > > and how they interact. > > > > This is a pretty quick overview rather than the idealised "kernel > > testing guide" that'd probably be optimal, but given the number of times > > questions like "When do you use KUnit and when do you use Kselftest?" > > are being asked, it seemed worth at least having something. Hopefully > > this can form the basis for more detailed documentation later. > > > > Signed-off-by: David Gow > > Reviewed-by: Daniel Latypov > > Looks good to me. Some minor typos and nits about wording here and there. > Thanks: I'll send out v3 with some fixes to your suggestions soon. Cheers, -- David > > --- > > Thanks, everyone, for the comments on the doc. I've made a few of the > > suggested changes. Please let me know what you think! > > > > -- David > > > > Changes since v1: > > https://lore.kernel.org/linux-kselftest/20210410070529.4113432-1-david...@google.com/ > > - Note KUnit's speed and that one should provide selftests for syscalls > > - Mention lockdep as a Dynamic Analysis Tool > > - Refer to "Dynamic Analysis Tools" instead of "Sanitizers" > > - A number of minor formatting tweaks and rewordings for clarity. > > > > Not changed: > > - I haven't included an exhaustive list of differences, advantages, etc, > > between KUnit and kselftest: for now, the doc continues to focus on > > the difference between 'in-kernel' and 'userspace' testing here. > > - Similarly, I'm not linking out to docs defining and describing "Unit" > > tests versus "End-to-end" tests. None of the existing documentation > > elsewhere quite matches what we do in the kernel perfectly, so it > > seems less confusing to focus on the 'in-kernel'/'userspace' > > distinction, and leave other definitions as a passing mention for > > those who are already familiar with the concepts. > > - I haven't linked to any talk videos here: a few of them are linked on > > (e.g.) the KUnit webpage, but I wanted to keep the Kernel documentation > > more self-contained for now. No objection to adding them in a follow-up > > patch if people feel strongly about it, though. > > - The link from index.rst to this doc is unchanged. I personally think > > that the link is prominent enough there: it's the first link, and > > shows up a few times. One possibility if people disagreed would be to > > merge this page with the index, but given not all dev-tools are going > > to be testing-related, it seemed a bit arrogant. :-) > > > > Documentation/dev-tools/index.rst| 3 + > > Documentation/dev-tools/testing-overview.rst | 117 +++ > > 2 files changed, 120 insertions(+) > > create mode 100644 Documentation/dev-tools/testing-overview.rst > > > > diff --git a/Documentation/dev-tools/index.rst > > b/Documentation/dev-tools/index.rst > > index 1b1cf4f5c9d9..f590e5860794 100644 > > --- a/Documentation/dev-tools/index.rst > > +++ b/Documentation/dev-tools/index.rst > > @@ -7,6 +7,8 @@ be used to work on the kernel. For now, the documents have > > been pulled > > together without any significant effort to integrate them into a coherent > > whole; patches welcome! > > > > +A brief overview of testing-specific tools can be found in > > :doc:`testing-overview`. > > + > > .. class:: toc-title > > > >Table of contents > > @@ -14,6 +16,7 @@ whole; patches welcome! > > .. toctree:: > > :maxdepth: 2 > > > > + testing-overview > > coccinelle > > sparse > > kcov > > diff --git a/Documentation/dev-tools/testing-overview.rst > > b/Documentation/dev-tools/testing-overview.rst > > new file mode 100644 > > index ..ce36a8cdf6b5 > > --- /dev/null > > +++ b/Documentation/dev-tools/testing-overview.rst > > @@ -0,0 +1,117 @@ > > +.. SPDX-License-Identifier: GPL-2.0 > > + > > + > > +Kernel Testing Guide > > + > > + > > + > > +There are a number of different tools for testing the Linux kernel, so > > knowing > > +when to use each of them can be a challenge
[PATCH v2] Documentation: dev-tools: Add Testing Overview
The kernel now has a number of testing and debugging tools, and we've seen a bit of confusion about what the differences between them are. Add a basic documentation outlining the testing tools, when to use each, and how they interact. This is a pretty quick overview rather than the idealised "kernel testing guide" that'd probably be optimal, but given the number of times questions like "When do you use KUnit and when do you use Kselftest?" are being asked, it seemed worth at least having something. Hopefully this can form the basis for more detailed documentation later. Signed-off-by: David Gow --- Thanks, everyone, for the comments on the doc. I've made a few of the suggested changes. Please let me know what you think! -- David Changes since v1: https://lore.kernel.org/linux-kselftest/20210410070529.4113432-1-david...@google.com/ - Note KUnit's speed and that one should provide selftests for syscalls - Mention lockdep as a Dynamic Analysis Tool - Refer to "Dynamic Analysis Tools" instead of "Sanitizers" - A number of minor formatting tweaks and rewordings for clarity. Not changed: - I haven't included an exhaustive list of differences, advantages, etc, between KUnit and kselftest: for now, the doc continues to focus on the difference between 'in-kernel' and 'userspace' testing here. - Similarly, I'm not linking out to docs defining and describing "Unit" tests versus "End-to-end" tests. None of the existing documentation elsewhere quite matches what we do in the kernel perfectly, so it seems less confusing to focus on the 'in-kernel'/'userspace' distinction, and leave other definitions as a passing mention for those who are already familiar with the concepts. - I haven't linked to any talk videos here: a few of them are linked on (e.g.) the KUnit webpage, but I wanted to keep the Kernel documentation more self-contained for now. No objection to adding them in a follow-up patch if people feel strongly about it, though. - The link from index.rst to this doc is unchanged. I personally think that the link is prominent enough there: it's the first link, and shows up a few times. One possibility if people disagreed would be to merge this page with the index, but given not all dev-tools are going to be testing-related, it seemed a bit arrogant. :-) Documentation/dev-tools/index.rst| 3 + Documentation/dev-tools/testing-overview.rst | 117 +++ 2 files changed, 120 insertions(+) create mode 100644 Documentation/dev-tools/testing-overview.rst diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst index 1b1cf4f5c9d9..f590e5860794 100644 --- a/Documentation/dev-tools/index.rst +++ b/Documentation/dev-tools/index.rst @@ -7,6 +7,8 @@ be used to work on the kernel. For now, the documents have been pulled together without any significant effort to integrate them into a coherent whole; patches welcome! +A brief overview of testing-specific tools can be found in :doc:`testing-overview`. + .. class:: toc-title Table of contents @@ -14,6 +16,7 @@ whole; patches welcome! .. toctree:: :maxdepth: 2 + testing-overview coccinelle sparse kcov diff --git a/Documentation/dev-tools/testing-overview.rst b/Documentation/dev-tools/testing-overview.rst new file mode 100644 index ..ce36a8cdf6b5 --- /dev/null +++ b/Documentation/dev-tools/testing-overview.rst @@ -0,0 +1,117 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +Kernel Testing Guide + + + +There are a number of different tools for testing the Linux kernel, so knowing +when to use each of them can be a challenge. This document provides a rough +overview of their differences, and how they fit together. + + +Writing and Running Tests += + +The bulk of kernel tests are written using either the kselftest or KUnit +frameworks. These both provide infrastructure to help make running tests and +groups of tests easier, as well as providing helpers to aid in writing new +tests. + +If you're looking to verify the behaviour of the Kernel — particularly specific +parts of the kernel — then you'll want to use KUnit or kselftest. + + +The Difference Between KUnit and kselftest +-- + +KUnit (Documentation/dev-tools/kunit/index.rst) is an entirely in-kernel system +for "white box" testing: because test code is part of the kernel, it can access +internal structures and functions which aren't exposed to userspace. + +KUnit tests therefore are best written against small, self-contained parts +of the kernel, which can be tested in isolation. This aligns well with the +concept of 'unit' testing. + +For example, a KUnit test might test an individual kernel function (or even a +single codepath through a function, such as an error handling case), rather +than a feature as a whole. +
Re: [PATCH v3] Documentation: kunit: add tips for running KUnit
On Wed, Apr 14, 2021 at 8:45 AM Daniel Latypov wrote: > > This is long overdue. > > There are several things that aren't nailed down (in-tree > .kunitconfig's), or partially broken (GCOV on UML), but having them > documented, warts and all, is better than having nothing. > > This covers a bunch of the more recent features > * kunit_filter_glob > * kunit.py run --kunitconfig > * slightly more detail on building tests as modules > * CONFIG_KUNIT_DEBUGFS > > By my count, the only headline features now not mentioned are the KASAN > integration and KernelCI json output support (kunit.py run --json). > > And then it also discusses how to get code coverage reports under UML > and non-UML since this is a question people have repeatedly asked. > > Non-UML coverage collection is no different from normal, but we should > probably explicitly call this out. > > As for UML, I was able to get it working again with two small hacks.* > E.g. with CONFIG_KUNIT=y && CONFIG_KUNIT_ALL_TESTS=y > Overall coverage rate: > lines..: 15.1% (18294 of 120776 lines) > functions..: 16.8% (1860 of 11050 functions) > > Note: this doesn't document --alltests since this is not stable yet. > Hopefully being run more frequently as part of KernelCI will help... > > *Using gcc/gcov-6 and not using uml_abort() in os_dump_core(). > I've documented these hacks in "Notes" but left TODOs for > brendanhigg...@google.com who tracked down the runtime issue in GCC. > To be clear: these are not issues specific to KUnit, but rather to UML. > > Signed-off-by: Daniel Latypov > --- I'm very happy with this now: all my issues with the previous versions are addressed. I'm particularly excited to have code coverage documented somewhere. Assuming Brendan's happy with the TODOs being there, I think this is ready to go. I also built this with Sphinx and gave it a quick look, and it all looks good there as well. Therefore, this is: Reviewed-by: David Gow Cheers, -- David
Re: [PATCH v5] lib: add basic KUnit test for lib/math
On Tue, Apr 13, 2021 at 3:07 AM Daniel Latypov wrote:
>
> Add basic test coverage for files that don't require any config options:
> * part of math.h (what seem to be the most commonly used macros)
> * gcd.c
> * lcm.c
> * int_sqrt.c
> * reciprocal_div.c
> (Ignored int_pow.c since it's a simple textbook algorithm.)
>
> These tests aren't particularly interesting, but they
> * provide short and simple examples of parameterized tests
> * provide a place to add tests for any new files in this dir
> * are written so adding new test cases to cover edge cases should be easy
> * looking at code coverage, we hit all the branches in the .c files
>
> Signed-off-by: Daniel Latypov
This looks good to me. A few comments/observations below, but nothing
that I think should actually block this.
Reviewed-by: David Gow
-- David
> ---
> Changes since v4:
> * add in test cases for some math.h macros (abs, round_up/round_down,
> div_round_down/closest)
> * use parameterized testing less to keep things terser
>
> Changes since v3:
> * fix `checkpatch.pl --strict` warnings
> * add test cases for gcd(0,0) and lcm(0,0)
> * minor: don't test both gcd(a,b) and gcd(b,a) when a == b
>
> Changes since v2: mv math_test.c => math_kunit.c
>
> Changes since v1:
> * Rebase and rewrite to use the new parameterized testing support.
> * misc: fix overflow in literal and inline int_sqrt format string.
> * related: commit 1f0e943df68a ("Documentation: kunit: provide guidance
> for testing many inputs") was merged explaining the patterns shown here.
> * there's an in-flight patch to update it for parameterized testing.
>
> v1: https://lore.kernel.org/lkml/20201019224556.3536790-1-dlaty...@google.com/
> ---
> lib/math/Kconfig | 5 +
> lib/math/Makefile | 2 +
> lib/math/math_kunit.c | 264 ++
> 3 files changed, 271 insertions(+)
> create mode 100644 lib/math/math_kunit.c
>
> diff --git a/lib/math/Kconfig b/lib/math/Kconfig
> index f19bc9734fa7..6ba8680439c1 100644
> --- a/lib/math/Kconfig
> +++ b/lib/math/Kconfig
> @@ -15,3 +15,8 @@ config PRIME_NUMBERS
>
> config RATIONAL
> bool
> +
> +config MATH_KUNIT_TEST
> + tristate "KUnit test for lib/math" if !KUNIT_ALL_TESTS
> + default KUNIT_ALL_TESTS
> + depends on KUNIT
This could have a description of the test and KUnit here, as mentioned
in the style guide doc:
https://www.kernel.org/doc/html/latest/dev-tools/kunit/style.html#test-kconfig-entries
(I think it's sufficiently self explanatory that it's not essential,
but it could be nice to have a more detailed description of the things
being tested than just "lib/math".)
> diff --git a/lib/math/Makefile b/lib/math/Makefile
> index be6909e943bd..30abb7a8d564 100644
> --- a/lib/math/Makefile
> +++ b/lib/math/Makefile
> @@ -4,3 +4,5 @@ obj-y += div64.o gcd.o lcm.o int_pow.o int_sqrt.o
> reciprocal_div.o
> obj-$(CONFIG_CORDIC) += cordic.o
> obj-$(CONFIG_PRIME_NUMBERS)+= prime_numbers.o
> obj-$(CONFIG_RATIONAL) += rational.o
> +
> +obj-$(CONFIG_MATH_KUNIT_TEST) += math_kunit.o
> diff --git a/lib/math/math_kunit.c b/lib/math/math_kunit.c
> new file mode 100644
> index ..80a087a32884
> --- /dev/null
> +++ b/lib/math/math_kunit.c
> @@ -0,0 +1,264 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Simple KUnit suite for math helper funcs that are always enabled.
> + *
> + * Copyright (C) 2020, Google LLC.
> + * Author: Daniel Latypov
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static void abs_test(struct kunit *test)
> +{
There's something weird about taking the absolute values of char
literals. I'm not sure if it's better to case integer literals (like
with 'short' below), or keep it as-is.
> + KUNIT_EXPECT_EQ(test, abs('\0'), '\0');
> + KUNIT_EXPECT_EQ(test, abs('a'), 'a');
> + KUNIT_EXPECT_EQ(test, abs(-'a'), 'a');
> +
> + /* The expression in the macro is actually promoted to an int. */
> + KUNIT_EXPECT_EQ(test, abs((short)0), 0);
> + KUNIT_EXPECT_EQ(test, abs((short)42), 42);
> + KUNIT_EXPECT_EQ(test, abs((short)-42), 42);
> +
> + KUNIT_EXPECT_EQ(test, abs(0), 0);
> + KUNIT_EXPECT_EQ(test, abs(42), 42);
> + KUNIT_EXPECT_EQ(test, abs(-42), 42);
> +
> + KUNIT_EXPECT_EQ(test, abs(0L), 0L);
> + KUNIT_EXPECT_EQ(test, abs(42L), 42L);
> + KUNIT_EXPECT_EQ(test, abs(-42L), 42L);
> +
> + KUNIT_EXPECT_EQ(test, abs(0LL), 0LL);
> + KUNIT_EXPECT_EQ(test, abs(42LL), 42LL);
> + KUNIT_EXPECT_EQ(test, abs(-42LL), 42LL);
> +
> +
Re: [PATCH] kunit: add unit test for filtering suites by names
On Tue, Apr 13, 2021 at 8:08 AM Daniel Latypov wrote:
>
> This adds unit tests for kunit_filter_subsuite() and
> kunit_filter_suites().
>
> Note: what the executor means by "subsuite" is the array of suites
> corresponding to each test file.
>
> This patch lightly refactors executor.c to avoid the use of global
> variables to make it testable.
> It also includes a clever `kfree_at_end()` helper that makes this test
> easier to write than it otherwise would have been.
>
> Tested by running just the new tests using itself
> $ ./tools/testing/kunit/kunit.py run '*exec*'
>
> Signed-off-by: Daniel Latypov
I really like this test, thanks.
A few small notes below, including what I think is a missing
kfree_at_end() call.
Assuming that one issue is fixed (or I'm mistaken):
Reviewed-by: David Gow
-- David
> ---
> lib/kunit/executor.c | 26
> lib/kunit/executor_test.c | 132 ++
> 2 files changed, 147 insertions(+), 11 deletions(-)
> create mode 100644 lib/kunit/executor_test.c
>
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 15832ed44668..96a4ae983786 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -19,7 +19,7 @@ MODULE_PARM_DESC(filter_glob,
> "Filter which KUnit test suites run at boot-time, e.g.
> list*");
>
> static struct kunit_suite * const *
> -kunit_filter_subsuite(struct kunit_suite * const * const subsuite)
> +kunit_filter_subsuite(struct kunit_suite * const * const subsuite, const
> char *filter_glob)
> {
> int i, n = 0;
> struct kunit_suite **filtered;
> @@ -52,19 +52,14 @@ struct suite_set {
> struct kunit_suite * const * const *end;
> };
>
> -static struct suite_set kunit_filter_suites(void)
> +static struct suite_set kunit_filter_suites(const struct suite_set
> *suite_set,
> + const char *filter_glob)
> {
> int i;
> struct kunit_suite * const **copy, * const *filtered_subsuite;
> struct suite_set filtered;
>
> - const size_t max = __kunit_suites_end - __kunit_suites_start;
> -
> - if (!filter_glob) {
> - filtered.start = __kunit_suites_start;
> - filtered.end = __kunit_suites_end;
> - return filtered;
> - }
> + const size_t max = suite_set->end - suite_set->start;
>
> copy = kmalloc_array(max, sizeof(*filtered.start), GFP_KERNEL);
> filtered.start = copy;
> @@ -74,7 +69,7 @@ static struct suite_set kunit_filter_suites(void)
> }
>
> for (i = 0; i < max; ++i) {
> - filtered_subsuite =
> kunit_filter_subsuite(__kunit_suites_start[i]);
> + filtered_subsuite =
> kunit_filter_subsuite(suite_set->start[i], filter_glob);
> if (filtered_subsuite)
> *copy++ = filtered_subsuite;
> }
> @@ -98,8 +93,13 @@ static void kunit_print_tap_header(struct suite_set
> *suite_set)
> int kunit_run_all_tests(void)
> {
> struct kunit_suite * const * const *suites;
> + struct suite_set suite_set = {
> + .start = __kunit_suites_start,
> + .end = __kunit_suites_end,
> + };
>
> - struct suite_set suite_set = kunit_filter_suites();
> + if (filter_glob)
> + suite_set = kunit_filter_suites(_set, filter_glob);
>
> kunit_print_tap_header(_set);
>
> @@ -115,4 +115,8 @@ int kunit_run_all_tests(void)
> return 0;
> }
>
> +#if IS_BUILTIN(CONFIG_KUNIT_TEST)
> +#include "executor_test.c"
> +#endif
> +
> #endif /* IS_BUILTIN(CONFIG_KUNIT) */
> diff --git a/lib/kunit/executor_test.c b/lib/kunit/executor_test.c
> new file mode 100644
> index ..8e925395beeb
> --- /dev/null
> +++ b/lib/kunit/executor_test.c
> @@ -0,0 +1,132 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test for the KUnit executor.
> + *
> + * Copyright (C) 2021, Google LLC.
> + * Author: Daniel Latypov
> + */
> +
> +#include
> +
> +static void kfree_at_end(struct kunit *test, const void *to_free);
> +static struct kunit_suite *alloc_fake_suite(struct kunit *test,
> + const char *suite_name);
> +
> +static void filter_subsuite_test(struct kunit *test)
> +{
> + struct kunit_suite *subsuite[3] = {NULL, NULL, NULL};
> + struct kunit_suite * const *filtered;
> +
> + subsuite[0] = alloc_fake_suite(test, "suite1");
> + subsuite[1] = alloc_fake_suite(test, "suite2");
> +
> + /* Wan
[PATCH] Documentation: dev-tools: Add Testing Overview
The kernel now has a number of testing and debugging tools, and we've seen a bit of confusion about what the differences between them are. Add a basic documentation outlining the testing tools, when to use each, and how they interact. This is a pretty quick overview rather than the idealised "kernel testing guide" that'd probably be optimal, but given the number of times questions like "When do you use KUnit and when do you use Kselftest?" are being asked, it seemed worth at least having something. Hopefully this can form the basis for more detailed documentation later. Signed-off-by: David Gow --- Documentation/dev-tools/index.rst| 3 + Documentation/dev-tools/testing-overview.rst | 102 +++ 2 files changed, 105 insertions(+) create mode 100644 Documentation/dev-tools/testing-overview.rst diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst index 1b1cf4f5c9d9..f590e5860794 100644 --- a/Documentation/dev-tools/index.rst +++ b/Documentation/dev-tools/index.rst @@ -7,6 +7,8 @@ be used to work on the kernel. For now, the documents have been pulled together without any significant effort to integrate them into a coherent whole; patches welcome! +A brief overview of testing-specific tools can be found in :doc:`testing-overview`. + .. class:: toc-title Table of contents @@ -14,6 +16,7 @@ whole; patches welcome! .. toctree:: :maxdepth: 2 + testing-overview coccinelle sparse kcov diff --git a/Documentation/dev-tools/testing-overview.rst b/Documentation/dev-tools/testing-overview.rst new file mode 100644 index ..8452adcb8608 --- /dev/null +++ b/Documentation/dev-tools/testing-overview.rst @@ -0,0 +1,102 @@ +.. SPDX-License-Identifier: GPL-2.0 + + +Kernel Testing Guide + + + +There are a number of different tools for testing the Linux kernel, so knowing +when to use each of them can be a challenge. This document provides a rough +overview of their differences, and how they fit together. + + +Writing and Running Tests += + +The bulk of kernel tests are written using either the :doc:`kselftest +` or :doc:`KUnit ` frameworks. These both provide +infrastructure to help make running tests and groups of tests easier, as well +as providing helpers to aid in writing new tests. + +If you're looking to verify the behaviour of the Kernel — particularly specific +parts of the kernel — then you'll want to use `KUnit` or `kselftest`. + + +The Difference Between KUnit and kselftest +-- + +:doc:`KUnit ` is an entirely in-kernel system for "white box" +testing: because test code is part of the kernel, it can access internal +structures and functions which aren't exposed to userspace. + +`KUnit` tests therefore are best written against small, self-contained parts +of the kernel, which can be tested in isolation. This aligns well with the +concept of Unit testing. + +For example, a KUnit test might test an individual kernel function (or even a +single codepath through a function, such as an error handling case), rather +than a feature as a whole. + +There is a KUnit test style guide which may give further pointers + + +:doc:`kselftest `, on the other hand, is largely implemented in +userspace, and tests are normal userspace scripts or programs. + +This makes it easier to write more complicated tests, or tests which need to +manipulate the overall system state more (e.g., spawning processes, etc.). +However, it's not possible to call kernel functions directly unless they're +exposed to userspace (by a syscall, device, filesystem, etc.) Some tests to +also provide a kernel module which is loaded by the test, though for tests +which run mostly or entirely within the kernel, `KUnit` may be the better tool. + +`kselftest` is therefore suited well to tests of whole features, as these will +expose an interface to userspace, which can be tested, but not implementation +details. This aligns well with 'system' or 'end-to-end' testing. + + +Code Coverage Tools +=== + +The Linux Kernel supports two different code coverage mesurement tools. These +can be used to verify that a test is executing particular functions or lines +of code. This is useful for determining how much of the kernel is being tested, +and for finding corner-cases which are not covered by the appropriate test. + +:doc:`kcov` is a feature which can be built in to the kernel to allow +capturing coverage on a per-task level. It's therefore useful for fuzzing and +other situations where information about code executed during, for example, a +single syscall is useful. + +:doc:`gcov` is GCC's coverage testing tool, which can be used with the kernel +to get global or per-module coverage. Unlike KCOV, it does not record per-task +coverage. Coverage data can be read from debugfs, and interpreted using the +usual
Re: [PATCH] Documentation: kunit: add tips for running KUnit
Thanks for writing this: it's good to have these things documented at last!
There are definitely a few things this document points out which still
need deciding, which does make this document lean a bit into "design
discussion" territory in a few of the notes. This doesn't bother me --
it's an accurate description of the state of things -- but I wouldn't
want this documentation held up too long because of these sorts of
TODOs (and can definitely see how having too many of them might
discourage KUnit use a bit). Particularly things like the
".kunitconfig" fragment file feature stuff: I feel that's something
better discussed on patches adding/using the feature than in the
documentation / reviews of the documentation, so I'd rather drop or
simplify those '..note:'s than bokeshed about it here (something I'm a
little guilty of below).
Otherwise, a few minor comments and nitpicks:
-- David
On Sat, Apr 10, 2021 at 2:01 AM Daniel Latypov wrote:
>
> This is long overdue.
>
> There are several things that aren't nailed down (in-tree
> .kunitconfig's), or partially broken (GCOV on UML), but having them
> documented, warts and all, is better than having nothing.
>
> This covers a bunch of the more recent features
> * kunit_filter_glob
> * kunit.py run --kunitconfig
> * kunit.py run --alltests
> * slightly more detail on building tests as modules
> * CONFIG_KUNIT_DEBUGFS
>
> By my count, the only headline features now not mentioned are the KASAN
> integration and KernelCI json output support (kunit.py run --json).
>
> And then it also discusses how to get code coverage reports under UML
> and non-UML since this is a question people have repeatedly asked.
>
> Non-UML coverage collection is no differnt from normal, but we should
> probably explicitly call thsi out.
Nit: typos in 'different' and 'this'.
>
> As for UML, I was able to get it working again with two small hacks.*
> E.g. with CONFIG_KUNIT=y && CONFIG_KUNIT_ALL_TESTS=y
> Overall coverage rate:
> lines..: 15.1% (18294 of 120776 lines)
> functions..: 16.8% (1860 of 11050 functions)
>
> *Switching to use gcc/gcov-6 and not using uml_abort().
> I've documented these hacks in "Notes" but left TODOs for
> brendanhigg...@google.com who tracked down the runtime issue in GCC.
> To be clear: these are not issues specific to KUnit, but rather to UML.
(We should probably note where uml_abort() needs to be replaced if
we're mentioning this, though doing so below in the more detailed
section may be more useful.)
>
> Signed-off-by: Daniel Latypov
> ---
> Documentation/dev-tools/kunit/index.rst | 1 +
> .../dev-tools/kunit/running_tips.rst | 278 ++
> Documentation/dev-tools/kunit/start.rst | 2 +
> 3 files changed, 281 insertions(+)
> create mode 100644 Documentation/dev-tools/kunit/running_tips.rst
>
> diff --git a/Documentation/dev-tools/kunit/index.rst
> b/Documentation/dev-tools/kunit/index.rst
> index 848478838347..7f7cf8d2ab20 100644
> --- a/Documentation/dev-tools/kunit/index.rst
> +++ b/Documentation/dev-tools/kunit/index.rst
> @@ -14,6 +14,7 @@ KUnit - Unit Testing for the Linux Kernel
> style
> faq
> tips
> + running_tips
>
> What is KUnit?
> ==
> diff --git a/Documentation/dev-tools/kunit/running_tips.rst
> b/Documentation/dev-tools/kunit/running_tips.rst
> new file mode 100644
> index ..d38e665e530f
> --- /dev/null
> +++ b/Documentation/dev-tools/kunit/running_tips.rst
> @@ -0,0 +1,278 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +
> +Tips For Running KUnit Tests
> +
> +
> +Using ``kunit.py run`` ("kunit tool")
> +=
> +
> +Running from any directory
> +--
> +
> +It can be handy to create a bash function like:
> +
> +.. code-block:: bash
> +
> + function run_kunit() {
> + ( cd "$(git rev-parse --show-toplevel)" &&
> ./tools/testing/kunit/kunit.py run $@ )
> + }
> +
> +.. note::
> + Early versions of ``kunit.py`` (before 5.6) didn't work unless run
> from
> + the kernel root, hence the use of a subshell and ``cd``.
> +
> +Running a subset of tests
> +-
> +
> +``kunit.py run`` accepts an optional glob argument to filter tests. Currently
> +this only matches against suite names, but this may change in the future.
> +
> +Say that we wanted to run the sysctl tests, we could do so via:
> +
> +.. code-block:: bash
> +
> + $ echo -e 'CONFIG_KUNIT=y\nCONFIG_KUNIT_ALL_TESTS=y' >
> .kunit/.kunitconfig
> + $ ./tools/testing/kunit/kunit.py run 'sysctl*'
> +
> +We're paying the cost of building more tests than we need this way, but it's
> +easier than fiddling with ``.kunitconfig`` files or commenting out
> +``kunit_suite``'s.
> +
> +However, if we wanted to define a set of tests in a less ad hoc way, the next
> +tip is useful.
> +
> +Defining a set of tests
>
[PATCH] kunit: tool: Fix a python tuple typing error
The first argument to namedtuple() should match the name of the type,
which wasn't the case for KconfigEntryBase.
Fixing this is enough to make mypy show no python typing errors again.
Fixes 97752c39bd ("kunit: kunit_tool: Allow .kunitconfig to disable config
items")
Signed-off-by: David Gow
---
tools/testing/kunit/kunit_config.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/kunit/kunit_config.py
b/tools/testing/kunit/kunit_config.py
index 0b550cbd667d..1e2683dcc0e7 100644
--- a/tools/testing/kunit/kunit_config.py
+++ b/tools/testing/kunit/kunit_config.py
@@ -13,7 +13,7 @@ from typing import List, Set
CONFIG_IS_NOT_SET_PATTERN = r'^# CONFIG_(\w+) is not set$'
CONFIG_PATTERN = r'^CONFIG_(\w+)=(\S+|".*")$'
-KconfigEntryBase = collections.namedtuple('KconfigEntry', ['name', 'value'])
+KconfigEntryBase = collections.namedtuple('KconfigEntryBase', ['name',
'value'])
class KconfigEntry(KconfigEntryBase):
--
2.30.0.617.g56c4b15f3c-goog
Re: [PATCH] kunit: tool: make --kunitconfig accept dirs, add lib/kunit fragment
On Tue, Feb 23, 2021 at 6:52 AM Daniel Latypov wrote:
>
> TL;DR
> $ ./tools/testing/kunit/kunit.py run --kunitconfig=lib/kunit
>
> Per suggestion from Ted [1], we can reduce the amount of typing by
> assuming a convention that these files are named '.kunitconfig'.
>
> In the case of [1], we now have
> $ ./tools/testing/kunit/kunit.py run --kunitconfig=fs/ext4
>
> Also add in such a fragment for kunit itself so we can give that as an
> example more close to home (and thus less likely to be accidentally
> broken).
>
> [1] https://lore.kernel.org/linux-ext4/ycnf4yp1db97z...@mit.edu/
>
> Signed-off-by: Daniel Latypov
> ---
Thanks! I really like this.
I'd assumed we'd check if the path exists, and fall back to appending
".kunitconfig", but checking if it's a directory is better.
I tried this out with all the different combinations I could think of,
and it works well.
Reviewed-by: David Gow
Cheers,
-- David
> lib/kunit/.kunitconfig | 3 +++
> tools/testing/kunit/kunit.py | 4 +++-
> tools/testing/kunit/kunit_kernel.py| 2 ++
> tools/testing/kunit/kunit_tool_test.py | 6 ++
> 4 files changed, 14 insertions(+), 1 deletion(-)
> create mode 100644 lib/kunit/.kunitconfig
>
> diff --git a/lib/kunit/.kunitconfig b/lib/kunit/.kunitconfig
> new file mode 100644
> index ..9235b7d42d38
> --- /dev/null
> +++ b/lib/kunit/.kunitconfig
> @@ -0,0 +1,3 @@
> +CONFIG_KUNIT=y
> +CONFIG_KUNIT_TEST=y
> +CONFIG_KUNIT_EXAMPLE_TEST=y
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index d5144fcb03ac..5da8fb3762f9 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -184,7 +184,9 @@ def add_common_opts(parser) -> None:
> help='Run all KUnit tests through allyesconfig',
> action='store_true')
> parser.add_argument('--kunitconfig',
> -help='Path to Kconfig fragment that enables
> KUnit tests',
> +help='Path to Kconfig fragment that enables
> KUnit tests.'
> +' If given a directory, (e.g. lib/kunit),
> "/.kunitconfig" '
> +'will get automatically appended.',
> metavar='kunitconfig')
>
> def add_build_opts(parser) -> None:
> diff --git a/tools/testing/kunit/kunit_kernel.py
> b/tools/testing/kunit/kunit_kernel.py
> index f309a33256cd..89a7d4024e87 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -132,6 +132,8 @@ class LinuxSourceTree(object):
> return
>
> if kunitconfig_path:
> + if os.path.isdir(kunitconfig_path):
> + kunitconfig_path =
> os.path.join(kunitconfig_path, KUNITCONFIG_PATH)
> if not os.path.exists(kunitconfig_path):
> raise ConfigError(f'Specified kunitconfig
> ({kunitconfig_path}) does not exist')
> else:
> diff --git a/tools/testing/kunit/kunit_tool_test.py
> b/tools/testing/kunit/kunit_tool_test.py
> index 1ad3049e9069..2e809dd956a7 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -251,6 +251,12 @@ class LinuxSourceTreeTest(unittest.TestCase):
> with tempfile.NamedTemporaryFile('wt') as kunitconfig:
> tree = kunit_kernel.LinuxSourceTree('',
> kunitconfig_path=kunitconfig.name)
>
> + def test_dir_kunitconfig(self):
> + with tempfile.TemporaryDirectory('') as dir:
> + with open(os.path.join(dir, '.kunitconfig'), 'w') as
> f:
> + pass
> + tree = kunit_kernel.LinuxSourceTree('',
> kunitconfig_path=dir)
> +
> # TODO: add more test cases.
>
>
>
> base-commit: b12b47249688915e987a9a2a393b522f86f6b7ab
> --
> 2.30.0.617.g56c4b15f3c-goog
>
Re: [PATCH v3 1/2] kunit: support failure from dynamic analysis tools
On Wed, Feb 10, 2021 at 6:14 AM Daniel Latypov wrote:
>
> From: Uriel Guajardo
>
> Add a kunit_fail_current_test() function to fail the currently running
> test, if any, with an error message.
>
> This is largely intended for dynamic analysis tools like UBSAN and for
> fakes.
> E.g. say I had a fake ops struct for testing and I wanted my `free`
> function to complain if it was called with an invalid argument, or
> caught a double-free. Most return void and have no normal means of
> signalling failure (e.g. super_operations, iommu_ops, etc.).
>
> Key points:
> * Always update current->kunit_test so anyone can use it.
> * commit 83c4e7a0363b ("KUnit: KASAN Integration") only updated it for
> CONFIG_KASAN=y
>
> * Create a new header so non-test code doesn't have
> to include all of (e.g. lib/ubsan.c)
>
> * Forward the file and line number to make it easier to track down
> failures
>
> * Declare the helper function for nice __printf() warnings about mismatched
> format strings even when KUnit is not enabled.
>
> Example output from kunit_fail_current_test("message"):
> [15:19:34] [FAILED] example_simple_test
> [15:19:34] # example_simple_test: initializing
> [15:19:34] # example_simple_test: lib/kunit/kunit-example-test.c:24:
> message
> [15:19:34] not ok 1 - example_simple_test
>
> Co-developed-by: Daniel Latypov
> Signed-off-by: Uriel Guajardo
> Signed-off-by: Daniel Latypov
> ---
> include/kunit/test-bug.h | 30 ++
> lib/kunit/test.c | 37 +
> 2 files changed, 63 insertions(+), 4 deletions(-)
> create mode 100644 include/kunit/test-bug.h
>
> diff --git a/include/kunit/test-bug.h b/include/kunit/test-bug.h
> new file mode 100644
> index ..18b1034ec43a
> --- /dev/null
> +++ b/include/kunit/test-bug.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * KUnit API allowing dynamic analysis tools to interact with KUnit tests
> + *
> + * Copyright (C) 2020, Google LLC.
> + * Author: Uriel Guajardo
> + */
> +
> +#ifndef _KUNIT_TEST_BUG_H
> +#define _KUNIT_TEST_BUG_H
> +
> +#define kunit_fail_current_test(fmt, ...) \
> + __kunit_fail_current_test(__FILE__, __LINE__, fmt, ##__VA_ARGS__)
> +
> +#if IS_ENABLED(CONFIG_KUNIT)
As the kernel test robot has pointed out on the second patch, this
probably should be IS_BUILTIN(), otherwise this won't build if KUnit
is a module, and the code calling it isn't.
This does mean that things like UBSAN integration won't work if KUnit
is a module, which is a shame.
(It's worth noting that the KASAN integration worked around this by
only calling inline functions, which would therefore be built-in even
if the rest of KUnit was built as a module. I don't think it's quite
as convenient to do that here, though.)
> +
> +extern __printf(3, 4) void __kunit_fail_current_test(const char *file, int
> line,
> + const char *fmt, ...);
> +
> +#else
> +
> +static __printf(3, 4) void __kunit_fail_current_test(const char *file, int
> line,
> + const char *fmt, ...)
> +{
> +}
> +
> +#endif
> +
> +
> +#endif /* _KUNIT_TEST_BUG_H */
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index ec9494e914ef..5794059505cf 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -7,6 +7,7 @@
> */
>
> #include
> +#include
> #include
> #include
> #include
> @@ -16,6 +17,38 @@
> #include "string-stream.h"
> #include "try-catch-impl.h"
>
> +/*
> + * Fail the current test and print an error message to the log.
> + */
> +void __kunit_fail_current_test(const char *file, int line, const char *fmt,
> ...)
> +{
> + va_list args;
> + int len;
> + char *buffer;
> +
> + if (!current->kunit_test)
> + return;
> +
> + kunit_set_failure(current->kunit_test);
> +
> + /* kunit_err() only accepts literals, so evaluate the args first. */
> + va_start(args, fmt);
> + len = vsnprintf(NULL, 0, fmt, args) + 1;
> + va_end(args);
> +
> + buffer = kunit_kmalloc(current->kunit_test, len, GFP_KERNEL);
> + if (!buffer)
> + return;
> +
> + va_start(args, fmt);
> + vsnprintf(buffer, len, fmt, args);
> + va_end(args);
> +
> + kunit_err(current->kunit_test, "%s:%d: %s", file, line, buffer);
> + kunit_kfree(current->kunit_test, buffer);
> +}
> +EXPORT_SYMBOL_GPL(__kunit_fail_current_test);
> +
> /*
> * Append formatted message to log, size of which is limited to
> * KUNIT_LOG_SIZE bytes (including null terminating byte).
> @@ -273,9 +306,7 @@ static void kunit_try_run_case(void *data)
> struct kunit_suite *suite = ctx->suite;
> struct kunit_case *test_case = ctx->test_case;
>
> -#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
> current->kunit_test = test;
> -#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT) */
>
[PATCH] kunit: tool: Disable PAGE_POISONING under --alltests
kunit_tool maintains a list of config options which are broken under
UML, which we exclude from an otherwise 'make ARCH=um allyesconfig'
build used to run all tests with the --alltests option.
Something in UML allyesconfig is causing segfaults when page poisining
is enabled (and is poisoning with a non-zero value). Previously, this
didn't occur, as allyesconfig enabled the CONFIG_PAGE_POISONING_ZERO
option, which worked around the problem by zeroing memory. This option
has since been removed, and memory is now poisoned with 0xAA, which
triggers segfaults in many different codepaths, preventing UML from
booting.
Note that we have to disable both CONFIG_PAGE_POISONING and
CONFIG_DEBUG_PAGEALLOC, as the latter will 'select' the former on
architectures (such as UML) which don't implement __kernel_map_pages().
Ideally, we'd fix this properly by tracking down the real root cause,
but since this is breaking KUnit's --alltests feature, it's worth
disabling there in the meantime so the kernel can boot to the point
where tests can actually run.
Fixes: f289041ed4 ("mm, page_poison: remove CONFIG_PAGE_POISONING_ZERO")
Signed-off-by: David Gow
---
As described above, 'make ARCH=um allyesconfig' is broken. KUnit has
been maintaining a list of configs to force-disable for this in
tools/testing/kunit/configs/broken_on_uml.config. The kernels we've
built with this have broken since CONFIG_PAGE_POISONING_ZERO was
removed, panic-ing on startup with:
<0>[0.10][ T11] Kernel panic - not syncing: Segfault with no mm
<4>[0.10][ T11] CPU: 0 PID: 11 Comm: kdevtmpfs Not tainted
5.11.0-rc7-3-g63381dc6f5f1-dirty #4
<4>[0.10][ T11] Stack:
<4>[0.10][ T11] 677d3d40 677d3f10 000e 600c0bc0
<4>[0.10][ T11] 677d3d90 603c99be 677d3d90 62529b93
<4>[0.10][ T11] 603c9ac0 677d3f10 62529b00 603c98a0
<4>[0.10][ T11] Call Trace:
<4>[0.10][ T11] [<600c0bc0>] ? set_signals+0x0/0x60
<4>[0.10][ T11] [<603c99be>] lookup_mnt+0x11e/0x220
<4>[0.10][ T11] [<62529b93>] ? down_write+0x93/0x180
<4>[0.10][ T11] [<603c9ac0>] ? lock_mount+0x0/0x160
<4>[0.10][ T11] [<62529b00>] ? down_write+0x0/0x180
<4>[0.10][ T11] [<603c98a0>] ? lookup_mnt+0x0/0x220
<4>[0.10][ T11] [<603c8160>] ? namespace_unlock+0x0/0x1a0
<4>[0.10][ T11] [<603c9b25>] lock_mount+0x65/0x160
<4>[0.10][ T11] [<6012f360>] ? up_write+0x0/0x40
<4>[0.10][ T11] [<603cbbd2>] do_new_mount_fc+0xd2/0x220
<4>[0.10][ T11] [<603eb560>] ? vfs_parse_fs_string+0x0/0xa0
<4>[0.10][ T11] [<603cbf04>] do_new_mount+0x1e4/0x260
<4>[0.10][ T11] [<603ccae9>] path_mount+0x1c9/0x6e0
<4>[0.10][ T11] [<603a9f4f>] ? getname_kernel+0xaf/0x1a0
<4>[0.10][ T11] [<603ab280>] ? kern_path+0x0/0x60
<4>[0.10][ T11] [<600238ee>] 0x600238ee
<4>[0.10][ T11] [<62523baa>] devtmpfsd+0x52/0xb8
<4>[0.10][ T11] [<62523b58>] ? devtmpfsd+0x0/0xb8
<4>[0.10][ T11] [<600fffd8>] kthread+0x1d8/0x200
<4>[0.10][ T11] [<600a4ea6>] new_thread_handler+0x86/0xc0
Disabling PAGE_POISONING fixes this. The issue can't be repoduced with
just PAGE_POISONING, there's clearly something (or several things) also
enabled by allyesconfig which contribute. Ideally, we'd track these down
and fix this at its root cause, but in the meantime it'd be nice to
disable PAGE_POISONING so we can at least get the kernel to boot. One
way would be to add a 'depends on !UML' or similar, but since
PAGE_POISONING does seem to work in the non-allyesconfig case, adding it
to our list of broken configs seemed the better choice.
Thoughts?
(Note that to reproduce this, you'll want to run
./tools/testing/kunit/kunit.py run --alltests --raw_output
It also depends on a couple of other fixes which are not upstream yet:
https://www.spinics.net/lists/linux-rtc/msg08294.html
https://lore.kernel.org/linux-i3c/20210127040636.1535722-1-david...@google.com/
Cheers,
-- David
tools/testing/kunit/configs/broken_on_uml.config | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/testing/kunit/configs/broken_on_uml.config
b/tools/testing/kunit/configs/broken_on_uml.config
index a7f0603d33f6..690870043ac0 100644
--- a/tools/testing/kunit/configs/broken_on_uml.config
+++ b/tools/testing/kunit/configs/broken_on_uml.config
@@ -40,3 +40,5 @@
# CONFIG_RESET_BRCMSTB_RESCAL is not set
# CONFIG_RESET_INTEL_GW is not set
# CONFIG_ADI_AXI_ADC is not set
+# CONFIG_DEBUG_PAGEALLOC is not set
+# CONFIG_PAGE_POISONING is not set
--
2.30.0.478.g8a0d178c01-goog
Re: [PATCH] kunit: make kunit_tool accept optional path to .kunitconfig fragment
On Sat, Jan 23, 2021 at 8:17 AM Daniel Latypov wrote:
>
> Currently running tests via KUnit tool means tweaking a .kunitconfig
> file, which you'd keep around locally and never commit.
> This changes makes it so users can pass in a path to a kunitconfig.
>
> One of the imagined use cases is having kunitconfig fragments in-tree
> to formalize interesting sets of tests for features/subsystems, e.g.
> $ ./tools/testing/kunit/kunit.py run fs/ext4/kunitconfig
>
> For now, this hypothetical fs/ext4/kunitconfig would contain
> CONFIG_KUNIT=y
> CONFIG_EXT4_FS=y
> CONFIG_EXT4_KUNIT_TESTS=y
>
> At the moment, it's not hard to manually whip up this file, but as more
> and more tests get added, this will get tedious.
>
> It also opens the door to documenting how to run all the tests relevant
> to a specific subsystem or feature as a simple one-liner.
>
> This can be seen as an analogue to tools/testing/selftests/*/config
> But in the case of KUnit, the tests live in the same directory as the
> code-under-test, so it feels more natural to allow the kunitconfig
> fragments to live anywhere. (Though, people could create a separate
> directory if wanted; this patch imposes no restrictions on the path).
>
> Signed-off-by: Daniel Latypov
> ---
Really glad this is finally happening. I tried it out, and it seemed
to work pretty well.
I was wondering whether a positional argument like this was best, or
whether it'd be better to have an explicitly named argument
(--kunitconfig=path). Thinking about it though, I'm quite happy with
having this as-is: the only real other contender for a coveted
positional argument spot would've been the name of a test or test
suite (e.g., kunit.py run ext4_inode_test), and that's not really
possible with the kunit_tool architecture as-is.
One other comment below (should this work for kunit.py config?),
otherwise it looks good.
-- David
> tools/testing/kunit/kunit.py | 9 ++---
> tools/testing/kunit/kunit_kernel.py| 12
> tools/testing/kunit/kunit_tool_test.py | 25 +
> 3 files changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
> index e808a47c839b..3204a23bd16e 100755
> --- a/tools/testing/kunit/kunit.py
> +++ b/tools/testing/kunit/kunit.py
> @@ -188,6 +188,9 @@ def add_build_opts(parser) -> None:
> help='As in the make command, "Specifies the
> number of '
> 'jobs (commands) to run simultaneously."',
> type=int, default=8, metavar='jobs')
> + parser.add_argument('kunitconfig',
> +help='Path to Kconfig fragment that enables
> KUnit tests',
> +type=str, nargs='?', metavar='kunitconfig')
>
Should this maybe be in add_common_opts()? I'd assume that we want
kunit.py config to accept this custom kunitconfig path as well.
> def add_exec_opts(parser) -> None:
> parser.add_argument('--timeout',
> @@ -256,7 +259,7 @@ def main(argv, linux=None):
> os.mkdir(cli_args.build_dir)
>
> if not linux:
> - linux =
> kunit_kernel.LinuxSourceTree(cli_args.build_dir)
> + linux =
> kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> kunitconfig_path=cli_args.kunitconfig)
>
> request = KunitRequest(cli_args.raw_output,
>cli_args.timeout,
> @@ -274,7 +277,7 @@ def main(argv, linux=None):
> os.mkdir(cli_args.build_dir)
>
> if not linux:
> - linux =
> kunit_kernel.LinuxSourceTree(cli_args.build_dir)
> + linux =
> kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> kunitconfig_path=cli_args.kunitconfig)
>
> request = KunitConfigRequest(cli_args.build_dir,
> cli_args.make_options)
> @@ -286,7 +289,7 @@ def main(argv, linux=None):
> sys.exit(1)
> elif cli_args.subcommand == 'build':
> if not linux:
> - linux =
> kunit_kernel.LinuxSourceTree(cli_args.build_dir)
> + linux =
> kunit_kernel.LinuxSourceTree(cli_args.build_dir,
> kunitconfig_path=cli_args.kunitconfig)
>
> request = KunitBuildRequest(cli_args.jobs,
> cli_args.build_dir,
> diff --git a/tools/testing/kunit/kunit_kernel.py
> b/tools/testing/kunit/kunit_kernel.py
> index 2076a5a2d060..0b461663e7d9 100644
> --- a/tools/testing/kunit/kunit_kernel.py
> +++ b/tools/testing/kunit/kunit_kernel.py
> @@ -123,7 +123,7 @@ def get_outfile_path(build_dir) -> str:
> class LinuxSourceTree(object):
> """Represents a Linux kernel source tree with KUnit tests."""
>
> - def __init__(self, build_dir: str, load_config=True,
>
Re: [PATCH] kunit: don't show `1 == 1` in failed assertion messages
On Fri, Jan 29, 2021 at 10:26 AM Daniel Latypov wrote: > > Currently, given something (fairly dystopian) like > > KUNIT_EXPECT_EQ(test, 2 + 2, 5) > > KUnit will prints a failure message like this. > > Expected 2 + 2 == 5, but > > 2 + 2 == 4 > > 5 == 5 > > With this patch, the output just becomes > > Expected 2 + 2 == 5, but > > 2 + 2 == 4 > > This patch is slightly hacky, but it's quite common* to compare an > expression to a literal integer value, so this can make KUnit less > chatty in many cases. (This patch also fixes variants like > KUNIT_EXPECT_GT, LE, et al.). > > It also allocates an additional string briefly, but given this only > happens on test failures, it doesn't seem too bad a tradeoff. > Also, in most cases it'll realize the lengths are unequal and bail out > before the allocation. > > We could save the result of the formatted string to avoid wasting this > extra work, but it felt cleaner to leave it as-is. > > Edge case: for something silly and unrealistic like > > KUNIT_EXPECT_EQ(test, 4, 5); > > It'll generate this message with a trailing "but" > > Expected 2 + 2 == 5, but > > I assume this is supposed to say "Expected 4 == 5" here. (I tested it to make sure, and that's what it did here.) Personally, I'd ideally like to get rid of the ", but", or even add a "but 4 != 5" style second line. Particularly in case the next line in the output might be confused for the rest of a sentence. That being said, this is a pretty silly edge case: I'd be worried if we ever saw that case in an actual submitted test. People might see it a bit while debugging, though: particularly if they're using KUNIT_EXPECT_EQ(test, 1, 2) as a way of forcing a test to fail. (I've done this while testing tooling, for instance.) > > It didn't feel worth adding a check up-front to see if both sides are > literals to handle this better. > > *A quick grep suggests 100+ comparisons to an integer literal as the > right hand side. > > Signed-off-by: Daniel Latypov > --- I tested this, and it works well: the results are definitely more human readable. I could see it making things slightly more complicated for people who wanted to automatically parse assertion errors, but no-one is doing that, and the extra complexity is pretty minimal anyway. One thing which might be worth doing is expanding this to KUNIT_EXPECT_STREQ() and/or KUNIT_EXPECT_PTR_EQ(). These have slightly more complicated formatting (quotes, leading 0s, etc), though. Comparing pointer literals is pretty unlikely to show up, though, so I don't think it's as important. (I thought that maybe the KASAN shadow memory tests might use them, but a quick look didn't reveal any.) For the record, this is what STREQ()/PTR_EQ()/ failures with literals look like: # example_simple_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:31 Expected "abc" == "abd", but "abc" == abc "abd" == abd # example_simple_test: EXPECTATION FAILED at lib/kunit/kunit-example-test.c:33 Expected 0x124 == 0x1234, but 0x124 == 0124 0x1234 == 1234 Either way, though, this is: Tested-by: David Gow Cheers, -- David
Re: [PATCH] soc: litex: Properly depend on HAS_IOMEM
On Wed, Jan 27, 2021 at 8:27 PM Stafford Horne wrote:
>
> On Tue, Jan 26, 2021 at 07:36:04PM -0800, David Gow wrote:
> > The LiteX SOC controller driver makes use of IOMEM functions like
> > devm_platform_ioremap_resource(), which are only available if
> > CONFIG_HAS_IOMEM is defined.
> >
> > This causes the driver not to be enable under make ARCH=um allyesconfig,
> > even though it won't build.
>
> Is this wording correct? I suspect it causes to driver TO BE enabled.
>
Whoops! Yes: that should be "causes the driver to be enabled" -- sorry
about that.
Let me know if you want me to re-send it with the fixed wording, or if
you just want to fix that yourself.
> >
> > By adding a dependency on HAS_IOMEM, the driver will not be enabled on
> > architectures which don't support it.
> >
> > Fixes: 22447a99c97e ("drivers/soc/litex: add LiteX SoC Controller driver")
> > Signed-off-by: David Gow a
>
> This looks ok to me. I can queue it for 5.11 fixes, if you can help with the
> wording above.
Thanks: that'd be great!
Cheers,
-- David
>
> -Stafford
>
> > ---
> > drivers/soc/litex/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/soc/litex/Kconfig b/drivers/soc/litex/Kconfig
> > index 7c6b009b6f6c..7a7c38282e11 100644
> > --- a/drivers/soc/litex/Kconfig
> > +++ b/drivers/soc/litex/Kconfig
> > @@ -8,6 +8,7 @@ config LITEX
> > config LITEX_SOC_CONTROLLER
> > tristate "Enable LiteX SoC Controller driver"
> > depends on OF || COMPILE_TEST
> > + depends on HAS_IOMEM
> > select LITEX
> > help
> > This option enables the SoC Controller Driver which verifies
> > --
> > 2.30.0.280.ga3ce27912f-goog
> >
Re: [PATCH 2/2] kcsan: Switch to KUNIT_CASE_PARAM for parameterized tests
On Thu, Jan 14, 2021 at 12:06 AM Marco Elver wrote: > > Since KUnit now support parameterized tests via KUNIT_CASE_PARAM, update > KCSAN's test to switch to it for parameterized tests. This simplifies > parameterized tests and gets rid of the "parameters in case name" > workaround (hack). > > At the same time, we can increase the maximum number of threads used, > because on systems with too few CPUs, KUnit allows us to now stop at the > maximum useful threads and not unnecessarily execute redundant test > cases with (the same) limited threads as had been the case before. > > Cc: David Gow > Signed-off-by: Marco Elver > --- Thanks! This looks great from the KUnit point of view: I'm particularly excited to see a use of the parameterised test generator that's not just reading from an array. I tested this as well, and it all seemed to work fine for me. Reviewed-by: David Gow Cheers, -- David
[PATCH] drivers: rtc: Make xilinx zynqmp driver depend on HAS_IOMEM
The Xilinx zynqmp RTC driver makes use of IOMEM functions like
devm_platform_ioremap_resource(), which are only available if
CONFIG_HAS_IOMEM is defined.
This causes the driver not to be enable under make ARCH=um allyesconfig,
even though it won't build.
By adding a dependency on HAS_IOMEM, the driver will not be enabled on
architectures which don't support it.
Fixes: 09ef18bcd5ac ("rtc: use devm_platform_ioremap_resource() to simplify
code")
Signed-off-by: David Gow
---
drivers/rtc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 6123f9f4fbc9..474d95184f20 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1300,7 +1300,7 @@ config RTC_DRV_OPAL
config RTC_DRV_ZYNQMP
tristate "Xilinx Zynq Ultrascale+ MPSoC RTC"
- depends on OF
+ depends on OF && HAS_IOMEM
help
If you say yes here you get support for the RTC controller found on
Xilinx Zynq Ultrascale+ MPSoC.
--
2.30.0.280.ga3ce27912f-goog
[PATCH] i3c/master/mipi-i3c-hci: Specify HAS_IOMEM dependency
The MIPI i3c HCI driver makes use of IOMEM functions like
devm_platform_ioremap_resource(), which are only available if
CONFIG_HAS_IOMEM is defined.
This causes the driver to be enabled under make ARCH=um allyesconfig,
even though it won't build.
By adding a dependency on HAS_IOMEM, the driver will not be enabled on
architectures which don't support it.
Fixes: 9ad9a52cce28 ("i3c/master: introduce the mipi-i3c-hci driver")
Signed-off-by: David Gow
---
drivers/i3c/master/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
index e68f15f4b4d0..afff0e2320f7 100644
--- a/drivers/i3c/master/Kconfig
+++ b/drivers/i3c/master/Kconfig
@@ -25,6 +25,7 @@ config DW_I3C_MASTER
config MIPI_I3C_HCI
tristate "MIPI I3C Host Controller Interface driver (EXPERIMENTAL)"
depends on I3C
+ depends on HAS_IOMEM
help
Support for hardware following the MIPI Aliance's I3C Host Controller
Interface specification.
--
2.30.0.280.ga3ce27912f-goog
[PATCH] soc: litex: Properly depend on HAS_IOMEM
The LiteX SOC controller driver makes use of IOMEM functions like
devm_platform_ioremap_resource(), which are only available if
CONFIG_HAS_IOMEM is defined.
This causes the driver not to be enable under make ARCH=um allyesconfig,
even though it won't build.
By adding a dependency on HAS_IOMEM, the driver will not be enabled on
architectures which don't support it.
Fixes: 22447a99c97e ("drivers/soc/litex: add LiteX SoC Controller driver")
Signed-off-by: David Gow
---
drivers/soc/litex/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/soc/litex/Kconfig b/drivers/soc/litex/Kconfig
index 7c6b009b6f6c..7a7c38282e11 100644
--- a/drivers/soc/litex/Kconfig
+++ b/drivers/soc/litex/Kconfig
@@ -8,6 +8,7 @@ config LITEX
config LITEX_SOC_CONTROLLER
tristate "Enable LiteX SoC Controller driver"
depends on OF || COMPILE_TEST
+ depends on HAS_IOMEM
select LITEX
help
This option enables the SoC Controller Driver which verifies
--
2.30.0.280.ga3ce27912f-goog
Re: [PATCH 1/2] kcsan: Make test follow KUnit style recommendations
On Thu, Jan 14, 2021 at 12:06 AM Marco Elver wrote: > > Per recently added KUnit style recommendations at > Documentation/dev-tools/kunit/style.rst, make the following changes to > the KCSAN test: > > 1. Rename 'kcsan-test.c' to 'kcsan_test.c'. > > 2. Rename suite name 'kcsan-test' to 'kcsan'. > > 3. Rename CONFIG_KCSAN_TEST to CONFIG_KCSAN_KUNIT_TEST and >default to KUNIT_ALL_TESTS. > > Cc: David Gow > Signed-off-by: Marco Elver Thanks very much -- it's great to see the naming guidelines starting to be picked up. I also tested the KUNIT_ALL_TESTS config option w/ KCSAN enabled, and it worked a treat. My only note is that we've had some problems[1] with mm-related changes which rename files getting corrupted at some point before reaching Linus, so it's probably worth keeping a close eye on this change to make sure nothing goes wrong. Reviewed-by: David Gow -- David [1]: https://www.spinics.net/lists/linux-mm/msg239149.html
Re: [PATCH] kunit: tool: Force the use of the 'tty' console for UML
On Wed, Jan 6, 2021 at 3:52 AM Shuah Khan wrote: > > On 1/5/21 11:57 AM, Andy Shevchenko wrote: > > On Tue, Jan 05, 2021 at 09:34:33AM -0700, Shuah Khan wrote: > >> On 1/5/21 9:21 AM, Petr Mladek wrote: > >>> On Mon 2021-01-04 09:23:57, Shuah Khan wrote: > >>>> On 12/22/20 4:11 AM, Andy Shevchenko wrote: > >>>>> On Mon, Dec 21, 2020 at 11:39:00PM -0800, David Gow wrote: > >>>>>> kunit_tool relies on the UML console outputting printk() output to the > >>>>>> tty in order to get results. Since the default console driver could > >>>>>> change, pass 'console=tty' to the kernel. > >>>>>> > >>>>>> This is triggered by a change[1] to use ttynull as a fallback console > >>>>>> driver which -- by chance or by design -- seems to have changed the > >>>>>> default console output on UML, breaking kunit_tool. While this may be > >>>>>> fixed, we should be less fragile to such changes in the default. > >>>>>> > >>>>>> [1]: > >>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=757055ae8dedf5333af17b3b5b4b70ba9bc9da4e > >>>>> > >>>>> Reported-by: Andy Shevchenko > >>>>> Tested-by: Andy Shevchenko > >>>>> > >>>> > >>>> Thank you all. Now in linux-kselftest kunit-fixes branch. > >>>> > >>>> Will send this up for rc3. > >>>> > >>>> Sorry for the delay - have been away from the keyboard for a > >>>> bit. > >>> > >>> JFYI, I am not sure that this is the right solution. I am > >>> looking into it, see > >>> https://lore.kernel.org/linux-kselftest/X%2FSRA1P8t+ONZFKb@alley/ > >>> for more details. > >>> > >> > >> Thanks Petr. I will hold off on sending the patch up to Linus and > >> let you find a the right solution. > > > > Please. leave it in Linux Next at least. Otherwise kunit will be broken for > > a > > long time which is not good. > > > > > > Yes. That is the plan. It will be in there until real fix comes in. > Thanks, Shuah. Personally, I think that this patch makes some sense to keep even if the underlying issue with ttynull is resolved. Given that kunit.py requires the console output, explicitly stating we want console=tty set is probably worth doing rather than relying on it being the default. That being said, I definitely agree that this patch doesn't fix the underlying issue with UML/ttynull: it just makes the kunit.py script less sensitive to such changes (which, while unlikely, could potentially occur legitimately down the track). Cheers, -- David
[PATCH] kunit: tool: Force the use of the 'tty' console for UML
kunit_tool relies on the UML console outputting printk() output to the
tty in order to get results. Since the default console driver could
change, pass 'console=tty' to the kernel.
This is triggered by a change[1] to use ttynull as a fallback console
driver which -- by chance or by design -- seems to have changed the
default console output on UML, breaking kunit_tool. While this may be
fixed, we should be less fragile to such changes in the default.
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=757055ae8dedf5333af17b3b5b4b70ba9bc9da4e
Signed-off-by: David Gow
Fixes: 757055ae8ded ("init/console: Use ttynull as a fallback when there is no
console")
---
tools/testing/kunit/kunit_kernel.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/kunit/kunit_kernel.py
b/tools/testing/kunit/kunit_kernel.py
index 57c1724b7e5d..698358c9c0d6 100644
--- a/tools/testing/kunit/kunit_kernel.py
+++ b/tools/testing/kunit/kunit_kernel.py
@@ -198,7 +198,7 @@ class LinuxSourceTree(object):
return self.validate_config(build_dir)
def run_kernel(self, args=[], build_dir='', timeout=None):
- args.extend(['mem=1G'])
+ args.extend(['mem=1G', 'console=tty'])
self._ops.linux_bin(args, timeout, build_dir)
outfile = get_outfile_path(build_dir)
subprocess.call(['stty', 'sane'])
--
2.29.2.729.g45daf8777d-goog
Re: [RFC PATCH] bpf: preload: Fix build error when O= is set
On Wed, Dec 16, 2020 at 10:53 PM Quentin Monnet wrote:
>
> 2020-11-21 17:48 UTC+0800 ~ David Gow
> > On Sat, Nov 21, 2020 at 3:38 PM Andrii Nakryiko
> > wrote:
> >>
> >> On Thu, Nov 19, 2020 at 12:51 AM David Gow wrote:
> >>>
> >>> If BPF_PRELOAD is enabled, and an out-of-tree build is requested with
> >>> make O=, compilation seems to fail with:
> >>>
> >>> tools/scripts/Makefile.include:4: *** O=.kunit does not exist. Stop.
> >>> make[4]: *** [../kernel/bpf/preload/Makefile:8:
> >>> kernel/bpf/preload/libbpf.a] Error 2
> >>> make[3]: *** [../scripts/Makefile.build:500: kernel/bpf/preload] Error 2
> >>> make[2]: *** [../scripts/Makefile.build:500: kernel/bpf] Error 2
> >>> make[2]: *** Waiting for unfinished jobs
> >>> make[1]: *** [.../Makefile:1799: kernel] Error 2
> >>> make[1]: *** Waiting for unfinished jobs
> >>> make: *** [Makefile:185: __sub-make] Error 2
> >>>
> >>> By the looks of things, this is because the (relative path) O= passed on
> >>> the command line is being passed to the libbpf Makefile, which then
> >>> can't find the directory. Given OUTPUT= is being passed anyway, we can
> >>> work around this by explicitly setting an empty O=, which will be
> >>> ignored in favour of OUTPUT= in tools/scripts/Makefile.include.
> >>
> >> Strange, but I can't repro it. I use make O= all the
> >> time with no issues. I just tried specifically with a make O=.build,
> >> where .build is inside Linux repo, and it still worked fine. See also
> >> be40920fbf10 ("tools: Let O= makes handle a relative path with -C
> >> option") which was supposed to address such an issue. So I'm wondering
> >> what exactly is causing this problem.
> >>
> > [+ linux-um list]
> >
> > Hmm... From a quick check, I can't reproduce this on x86, so it's
> > possibly a UML-specific issue.
> >
> > The problem here seems to be that $PWD is, for whatever reason, equal
> > to the srcdir on x86, but not on UML. In general, $PWD behaves pretty
> > weirdly -- I don't fully understand it -- but if I add a tactical "PWD
> > := $(shell pwd)" or use $(CURDIR) instead, the issue shows up on x86
> > as well. I guess this is because PWD only gets updated when set by a
> > shell or something, and UML does this somewhere?
> >
> > Thoughts?
> >
> > Cheers,
> > -- David
>
> Hi David, Andrii,
>
> David, did you use a different command for building for UML and x86? I'm
> asking because I reproduce on x86, but only for some targets, in
> particular when I tried bindeb-pkg.
I just ran "make ARCH={x86,um} O=.bpftest", with defconfig + enabling
BPF_PRELOAD and its dependencies. UML fails, x86 works. (Though I can
reproduce the failure if I make bindeb-pkg on x86).
(It also shows up when building UML with the allyesconfig-based KUnit
alltests option by running "./tools/testing/kunit/kunit.py run
--alltests", though this understandably takes a long time and is less
obvious)
>
> With "make O=.build vmlinux", I have:
> - $(O) for "dummy" check in tools/scripts/Makefile.include set to
> /linux/.build
> - $(PWD) for same check set to /linux/tools
> - Since $(O) is an absolute path, the "dummy" check passes
>
> With "make O=.build bindeb-pkg", I have instead:
> - $(O) set to .build (relative path)
> - $(PWD) set to /linux/.build
> - "dummy" check changes to /linux/.build and searches for .build in it,
> which fails and aborts the build
>
> (tools/scripts/Makefile.include is included from libbpf's Makefile,
> called from kernel/bpf/preload/Makefile.)
>
> I'm not sure how exactly the bindeb-pkg target ends up passing these values.
Yeah: I haven't been able to find where uml is changing them either:
I'm assuming there's something which changes directory and/or spawns a
shell/recursive make to change $(PWD) or something.
> For what it's worth, I have been solving this (before finding this
> thread) with a fix close to yours, I pass "O=$(abspath .)" on the
> command line for building libbpf in kernel/bpf/preload/Makefile. It
> looked consistent to me with the "tools/:" target from the main
> Makefile, where "O=$(abspath $(objtree))" is passed (and $(objtree) is ".").
Given that there are several targets being broken here, it's probably
worth having a fix like this which overrides O= rather than trying to
hunt down every target which could change $(PWD). I don't particularly
mind whether we use O= or O=$(abspath .), both are working in the UML
usecase as well.
Does anyone object to basically accepting either this patch as-is, or
using O=$(abspath .)?
Cheers,
-- David
Re: [PATCH] Documentation: kunit: include example of a parameterized test
On Wed, Dec 16, 2020 at 8:23 AM Daniel Latypov wrote:
>
> Commit fadb08e7c750 ("kunit: Support for Parameterized Testing")
> introduced support but lacks documentation for how to use it.
>
> This patch builds on commit 1f0e943df68a ("Documentation: kunit: provide
> guidance for testing many inputs") to show a minimal example of the new
> feature.
>
> Signed-off-by: Daniel Latypov
> ---
This looks good to me. Thanks!
Reviewed-by: David Gow
-- David
smime.p7s
Description: S/MIME Cryptographic Signature
[PATCH] kunit: tool: Fix spelling of "diagnostic" in kunit_parser
Various helper functions were misspelling "diagnostic" in their names. It finally got annoying, so fix it. Signed-off-by: David Gow --- tools/testing/kunit/kunit_parser.py | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 6614ec4d0898..1a1e1d13f1d3 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -97,11 +97,11 @@ def print_log(log): TAP_ENTRIES = re.compile(r'^(TAP|[\s]*ok|[\s]*not ok|[\s]*[0-9]+\.\.[0-9]+|[\s]*#).*$') -def consume_non_diagnositic(lines: List[str]) -> None: +def consume_non_diagnostic(lines: List[str]) -> None: while lines and not TAP_ENTRIES.match(lines[0]): lines.pop(0) -def save_non_diagnositic(lines: List[str], test_case: TestCase) -> None: +def save_non_diagnostic(lines: List[str], test_case: TestCase) -> None: while lines and not TAP_ENTRIES.match(lines[0]): test_case.log.append(lines[0]) lines.pop(0) @@ -113,7 +113,7 @@ OK_NOT_OK_SUBTEST = re.compile(r'^[\s]+(ok|not ok) [0-9]+ - (.*)$') OK_NOT_OK_MODULE = re.compile(r'^(ok|not ok) ([0-9]+) - (.*)$') def parse_ok_not_ok_test_case(lines: List[str], test_case: TestCase) -> bool: - save_non_diagnositic(lines, test_case) + save_non_diagnostic(lines, test_case) if not lines: test_case.status = TestStatus.TEST_CRASHED return True @@ -139,7 +139,7 @@ SUBTEST_DIAGNOSTIC = re.compile(r'^[\s]+# (.*)$') DIAGNOSTIC_CRASH_MESSAGE = re.compile(r'^[\s]+# .*?: kunit test case crashed!$') def parse_diagnostic(lines: List[str], test_case: TestCase) -> bool: - save_non_diagnositic(lines, test_case) + save_non_diagnostic(lines, test_case) if not lines: return False line = lines[0] @@ -155,7 +155,7 @@ def parse_diagnostic(lines: List[str], test_case: TestCase) -> bool: def parse_test_case(lines: List[str]) -> Optional[TestCase]: test_case = TestCase() - save_non_diagnositic(lines, test_case) + save_non_diagnostic(lines, test_case) while parse_diagnostic(lines, test_case): pass if parse_ok_not_ok_test_case(lines, test_case): @@ -166,7 +166,7 @@ def parse_test_case(lines: List[str]) -> Optional[TestCase]: SUBTEST_HEADER = re.compile(r'^[\s]+# Subtest: (.*)$') def parse_subtest_header(lines: List[str]) -> Optional[str]: - consume_non_diagnositic(lines) + consume_non_diagnostic(lines) if not lines: return None match = SUBTEST_HEADER.match(lines[0]) @@ -179,7 +179,7 @@ def parse_subtest_header(lines: List[str]) -> Optional[str]: SUBTEST_PLAN = re.compile(r'[\s]+[0-9]+\.\.([0-9]+)') def parse_subtest_plan(lines: List[str]) -> Optional[int]: - consume_non_diagnositic(lines) + consume_non_diagnostic(lines) match = SUBTEST_PLAN.match(lines[0]) if match: lines.pop(0) @@ -202,7 +202,7 @@ def max_status(left: TestStatus, right: TestStatus) -> TestStatus: def parse_ok_not_ok_test_suite(lines: List[str], test_suite: TestSuite, expected_suite_index: int) -> bool: - consume_non_diagnositic(lines) + consume_non_diagnostic(lines) if not lines: test_suite.status = TestStatus.TEST_CRASHED return False @@ -235,7 +235,7 @@ def bubble_up_test_case_errors(test_suite: TestSuite) -> TestStatus: def parse_test_suite(lines: List[str], expected_suite_index: int) -> Optional[TestSuite]: if not lines: return None - consume_non_diagnositic(lines) + consume_non_diagnostic(lines) test_suite = TestSuite() test_suite.status = TestStatus.SUCCESS name = parse_subtest_header(lines) @@ -264,7 +264,7 @@ def parse_test_suite(lines: List[str], expected_suite_index: int) -> Optional[Te TAP_HEADER = re.compile(r'^TAP version 14$') def parse_tap_header(lines: List[str]) -> bool: - consume_non_diagnositic(lines) + consume_non_diagnostic(lines) if TAP_HEADER.match(lines[0]): lines.pop(0) return True @@ -274,7 +274,7 @@ def parse_tap_header(lines: List[str]) -> bool: TEST_PLAN = re.compile(r'[0-9]+\.\.([0-9]+)') def parse_test_plan(lines: List[str]) -> Optional[int]: - consume_non_diagnositic(lines) + consume_non_diagnostic(lines) match = TEST_PLAN.match(lines[0]) if match: lines.pop(0) @@ -286,7 +286,7 @@ def bubble_up_suite_errors(test_suite_list: List[TestSuite]) -> TestStatus: return bubble_up_errors(lambda x: x.status, test_suite_list) def parse_test_result(lines: List[str]) -> TestResult: - consume_non_diagnositic(lines) +
[PATCH] kunit: Print test statistics on failure
When a number of tests fail, it can be useful to get higher-level
statistics of how many tests are failing (or how many parameters are
failing in parameterised tests), and in what cases or suites. This is
already done by some non-KUnit tests, so add support for automatically
generating these for KUnit tests.
This change adds a 'kunit_stats_enabled' switch which has three values:
- 0: No stats are printed (current behaviour)
- 1: Stats are printed only for tests/suites with more than one
subtests, and at least one failure (new default)
- 2: Always print test statistics
For parameterised tests, the summary line looks as follows:
"# inode_test_xtimestamp_decoding: 0 / 16 test parameters failed"
For test suites, it looks like this:
"# ext4_inode_test: (0 / 1) tests failed (0 / 16 test parameters)"
kunit_tool is also updated to correctly ignore diagnostic lines, so that
these statistics do not prevent the result from parsing.
Signed-off-by: David Gow
---
This is largely a follow-up to the discussion here:
https://lore.kernel.org/linux-kselftest/CABVgOSmy4n_LGwDS7yWfoLftcQzxv6S+iXx9Y=opcgg2gu0...@mail.gmail.com/T/#t
Does this seem like a sensible addition?
Cheers,
-- David
lib/kunit/test.c| 71 +
tools/testing/kunit/kunit_parser.py | 2 +-
2 files changed, 72 insertions(+), 1 deletion(-)
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index ec9494e914ef..711e269366a7 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
#include
#include
@@ -16,6 +17,40 @@
#include "string-stream.h"
#include "try-catch-impl.h"
+/*
+ * KUnit statistic mode:
+ * 0 - disabled
+ * 1 - only when there is at least one failure, and more than one subtest
+ * 2 - enabled
+ */
+static int kunit_stats_enabled = 1;
+core_param(kunit_stats_enabled, kunit_stats_enabled, int, 0644);
+
+static bool kunit_should_print_stats(int num_failures, int num_subtests)
+{
+ if (kunit_stats_enabled == 0)
+ return false;
+
+ if (kunit_stats_enabled == 2)
+ return true;
+
+ return (num_failures > 0 && num_subtests > 1);
+}
+
+static void kunit_print_test_stats(struct kunit *test,
+ size_t num_failures, size_t num_subtests)
+{
+ if (!kunit_should_print_stats(num_failures, num_subtests))
+ return;
+
+ kunit_log(KERN_INFO, test,
+ KUNIT_SUBTEST_INDENT
+ "# %s: %lu / %lu test parameters failed",
+ test->name,
+ num_failures,
+ num_subtests);
+}
+
/*
* Append formatted message to log, size of which is limited to
* KUNIT_LOG_SIZE bytes (including null terminating byte).
@@ -346,15 +381,37 @@ static void kunit_run_case_catch_errors(struct
kunit_suite *suite,
test_case->success = test->success;
}
+static void kunit_print_suite_stats(struct kunit_suite *suite,
+ size_t num_failures,
+ size_t total_param_failures,
+ size_t total_params)
+{
+ size_t num_cases = kunit_suite_num_test_cases(suite);
+
+ if (!kunit_should_print_stats(num_failures, num_cases))
+ return;
+
+ kunit_log(KERN_INFO, suite,
+ "# %s: (%lu / %lu) tests failed (%lu / %lu test parameters)",
+ suite->name,
+ num_failures,
+ num_cases,
+ total_param_failures,
+ total_params);
+}
+
int kunit_run_tests(struct kunit_suite *suite)
{
char param_desc[KUNIT_PARAM_DESC_SIZE];
struct kunit_case *test_case;
+ size_t num_suite_failures = 0;
+ size_t total_param_failures = 0, total_params = 0;
kunit_print_subtest_start(suite);
kunit_suite_for_each_test_case(suite, test_case) {
struct kunit test = { .param_value = NULL, .param_index = 0 };
+ size_t num_params = 0, num_failures = 0;
bool test_success = true;
if (test_case->generate_params) {
@@ -385,13 +442,27 @@ int kunit_run_tests(struct kunit_suite *suite)
test.param_value =
test_case->generate_params(test.param_value, param_desc);
test.param_index++;
}
+
+ if (!test.success)
+ num_failures++;
+ num_params++;
+
} while (test.param_value);
+ kunit_print_test_stats(, num_failures, num_params);
+
kunit_print_ok_not_ok(, true, test_success,
kunit_test_case_num(suite, test_case),
test_case->name);
+
+
Re: [PATCH] kunit: tool: simplify kconfig is_subset_of() logic
On Wed, Dec 9, 2020 at 7:21 AM Daniel Latypov wrote: > > Don't use an O(nm) algorithm* and make it more readable by using a dict. > > *Most obviously, it does a nested for-loop over the entire other config. > A bit more subtle, it calls .entries(), which constructs a set from the > list for _every_ outer iteration. > > Signed-off-by: Daniel Latypov > --- Thanks! This works great here: I didn't time it to see how much faster it is, but it's clearly an improvement. Reviewed-by: David Gow Cheers, -- David
Re: [PATCH 3/3] kunit: tool: move kunitconfig parsing into __init__
On Sat, Dec 5, 2020 at 2:18 AM Daniel Latypov wrote:
>
> On Thu, Dec 3, 2020 at 7:57 PM David Gow wrote:
> >
> > On Fri, Dec 4, 2020 at 3:41 AM Daniel Latypov wrote:
> > >
> > > LinuxSourceTree will unceremoniously crash if the user doesn't call
> > > read_kunitconfig() first in a number of functions.
> >
> > This patch seems to partly be reverting the changes here, right:
> > https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/tools/testing/kunit?h=kunit=fcdb0bc08ced274078f371e1e0fe6421a97fa9f2
> > (That patch moved the reading of kunitconfig out of __init__)
>
> Yes.
>
> >
> > My overall concern is that, really, there are some operations that
> > shouldn't need a kunitconfig (even if they do at the moment), so we'd
> > ideally want at least some of the operations currently under
> > LinuxSourceTree to be able to be run without first reading a
> > kunitconfig. Most notably, it'd be nice if kunit.py exec (and hence
> > LinuxSourceTree::run_kernel()) didn't need a kunitconfig, as the
> > kernel ought to already be built at this point.
> >
> > Now, this is all a little bit hypothetical, as we haven't bothered to
> > make kunit.py exec work without a kunitconfig thus far, but I'm a
> > touch hesitant to make it harder to bypass the kunitconfig reading
> > anyway.
>
> Fair point.
>
> So one alternative to this to make type-checkers happy is to declare
> _config instead of sneakily setting it in some random later method.
> Then in all the places that rely on _config, we'd need to add in
> checks that it's in fact set to give a better error message (so it's
> clear to the user that it's an internal tool bug and has nothing to do
> with them).
This seems plausible, if a bit verbose.
>
> The copy-paste of create+read_kunitconfig() is annoying, which is why
> I went with this.
Personally, the duplication of calls to {create,read}_kunitconfig()
doesn't bother me, but I definitely can see the advantage of having
the type system pick up when we've missed one.
> How about __init__ takes an optional argument that can disable this parsing?
This would be okay: I'm starting to feel that really, the ultimate
solution is either to split LinuxSourceTree up (and have separate
things for configuring, building, and running the kernel), or to pass
the kconfig stuff into just the functions that require it. But that is
a much more serious refactor, which I haven't fully thought through,
and I don't want to let the perfect be the enemy of the good here.
>
> E.g.
>
> def __init__(kconfig = None):
>if kconfig is not None:
> self._config = kconfig
>else:
> // create and read
>
What would the kconfig argument here be? Just an empty Kconfig()?
I'm not a huge fan of passing a "None" kconfig object when we want to
load a config, and a non-None one when we want an empty one: that
seems confusingly backwards.
Maybe it'd be possible to move the loading of the kunitconfig outside
LinuxSourceTree, and pass that (or an empty one) as needed?
> Or if we don't like the idea of requiring users who don't want a
> kconfig to pass in a dummy,
>
> def __init__(load_kconfig=True):
>if not load_kconfig:
> self._config = None
>...
>
I slightly prefer this, for the reasons above: True/False makes more
sense than None/Kconfig().
> >
> > >
> > > Adn currently every place we create an instance, the caller also calls
> > > create_kunitconfig() and read_kunitconfig().
> > >
> > > Move these instead into __init__() so they can't be forgotten and to
> > > reduce copy-paste.
> >
> > This seems to now be missing the create_kunitconfig() stuff (see below).
>
> Ah good catch. Completely unintentional.
> I'm sure I had the create_kunitconfig() stuff in __init__() at some
> point but must have inadvertently removed it somehow later on.
>
> > >
> > > The https://github.com/google/pytype type-checker complained that
> > > _config wasn't initialized. With this, kunit_tool now type checks
> > > under both pytype and mypy.
> > >
> > > Signed-off-by: Daniel Latypov
> > > ---
Okay, so it looks like there are a few options with _kconfig:
1. Check for None everywhere (after explicitly setting it in the
constructor). Pros: Nicer error messages, doesn't require other
changes, Cons: verbose, still somewhat prone to error (could forget
{create,read}_kunitconfig())
2. Pass a Kconfig object into the constructor. Pros: a kconfig must
exist, so less error prone, Cons: if we allow passing None to load it,
that's confusing.
3. Pass a bool into the constructor. Pros: similarly less error prone.
Cons: True/False
Re: [PATCH 1/3] kunit: tool: surface and address more typing issues
On Fri, Dec 4, 2020 at 3:41 AM Daniel Latypov wrote: > > The authors of this tool were more familiar with a different > type-checker, https://github.com/google/pytype. > > That's open source, but mypy seems more prevalent (and runs faster). > And unlike pytype, mypy doesn't try to infer types so it doesn't check > unanotated functions. > > So annotate ~all functions in kunit tool to increase type-checking > coverage. > Note: per https://www.python.org/dev/peps/pep-0484/, `__init__()` should > be annotated as `-> None`. > > Doing so makes mypy discover a number of new violations. > Exclude main() since we reuse `request` for the different types of > requests, which mypy isn't happy about. > > This commit fixes all but one error, where `TestSuite.status` might be > None. > > Signed-off-by: Daniel Latypov > --- This looks good to me: I gave it some quick testing, and reading through it, all of the changes seem sensible. I wasn't able to get pytype running here, but mypy worked fine. Reviewed-by: David Gow -- David
Re: [PATCH 2/3] kunit: tool: fix minor typing issue with None status
On Fri, Dec 4, 2020 at 3:41 AM Daniel Latypov wrote:
>
This seems good to me, but I have a few questions, particularly around
the description.
> The code to handle aggregating statuses didn't check that the status
> actually got set.
I don't understand what you mean here. Does this refer to
Test{Case,Suite}::status potentially being None, and that not being
supported properly in bubble_up_{suite_,test_case_,}errors(), or
something else? Either way, I can't see any additional code to "check"
that the status has been set. As far as I can tell everything except
the default to SUCCESS is a no-op, or am I missing something?
> Default the value to SUCCESS.
I'm a little iffy about defaulting this to success, but I think it's
okay for now: the skip test support will eventually change this to a
SKIPPED value.
>
> This sorta follows the precedent in commit 3fc48259d525 ("kunit: Don't
> fail test suites if one of them is empty").
>
> Also slightly simplify the code and add type annotations.
>
> Signed-off-by: Daniel Latypov
> ---
Otherwise, the actual code changes all seem sensible, and it worked
fine when I tested it, so:
Reviewed-by: David Gow
-- David
Re: [PATCH 3/3] kunit: tool: move kunitconfig parsing into __init__
On Fri, Dec 4, 2020 at 3:41 AM Daniel Latypov wrote: > > LinuxSourceTree will unceremoniously crash if the user doesn't call > read_kunitconfig() first in a number of functions. This patch seems to partly be reverting the changes here, right: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/tools/testing/kunit?h=kunit=fcdb0bc08ced274078f371e1e0fe6421a97fa9f2 (That patch moved the reading of kunitconfig out of __init__) My overall concern is that, really, there are some operations that shouldn't need a kunitconfig (even if they do at the moment), so we'd ideally want at least some of the operations currently under LinuxSourceTree to be able to be run without first reading a kunitconfig. Most notably, it'd be nice if kunit.py exec (and hence LinuxSourceTree::run_kernel()) didn't need a kunitconfig, as the kernel ought to already be built at this point. Now, this is all a little bit hypothetical, as we haven't bothered to make kunit.py exec work without a kunitconfig thus far, but I'm a touch hesitant to make it harder to bypass the kunitconfig reading anyway. > > Adn currently every place we create an instance, the caller also calls > create_kunitconfig() and read_kunitconfig(). > > Move these instead into __init__() so they can't be forgotten and to > reduce copy-paste. This seems to now be missing the create_kunitconfig() stuff (see below). > > The https://github.com/google/pytype type-checker complained that > _config wasn't initialized. With this, kunit_tool now type checks > under both pytype and mypy. > > Signed-off-by: Daniel Latypov > --- > tools/testing/kunit/kunit.py| 20 > tools/testing/kunit/kunit_kernel.py | 19 +++ > 2 files changed, 11 insertions(+), 28 deletions(-) > > diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py > index 08951a114654..b58fb3733cfa 100755 > --- a/tools/testing/kunit/kunit.py > +++ b/tools/testing/kunit/kunit.py > @@ -256,10 +256,7 @@ def main(argv, linux=None): > os.mkdir(cli_args.build_dir) > > if not linux: > - linux = kunit_kernel.LinuxSourceTree() > - > - linux.create_kunitconfig(cli_args.build_dir) > - linux.read_kunitconfig(cli_args.build_dir) > + linux = > kunit_kernel.LinuxSourceTree(cli_args.build_dir) > > request = KunitRequest(cli_args.raw_output, >cli_args.timeout, > @@ -277,10 +274,7 @@ def main(argv, linux=None): > os.mkdir(cli_args.build_dir) > > if not linux: > - linux = kunit_kernel.LinuxSourceTree() > - > - linux.create_kunitconfig(cli_args.build_dir) > - linux.read_kunitconfig(cli_args.build_dir) > + linux = > kunit_kernel.LinuxSourceTree(cli_args.build_dir) > > request = KunitConfigRequest(cli_args.build_dir, > cli_args.make_options) > @@ -292,10 +286,7 @@ def main(argv, linux=None): > sys.exit(1) > elif cli_args.subcommand == 'build': > if not linux: > - linux = kunit_kernel.LinuxSourceTree() > - > - linux.create_kunitconfig(cli_args.build_dir) > - linux.read_kunitconfig(cli_args.build_dir) > + linux = > kunit_kernel.LinuxSourceTree(cli_args.build_dir) > > request = KunitBuildRequest(cli_args.jobs, > cli_args.build_dir, > @@ -309,10 +300,7 @@ def main(argv, linux=None): > sys.exit(1) > elif cli_args.subcommand == 'exec': > if not linux: > - linux = kunit_kernel.LinuxSourceTree() > - > - linux.create_kunitconfig(cli_args.build_dir) > - linux.read_kunitconfig(cli_args.build_dir) > + linux = > kunit_kernel.LinuxSourceTree(cli_args.build_dir) > > exec_request = KunitExecRequest(cli_args.timeout, > cli_args.build_dir, > diff --git a/tools/testing/kunit/kunit_kernel.py > b/tools/testing/kunit/kunit_kernel.py > index bda7c4fd4d3e..79793031d2c4 100644 > --- a/tools/testing/kunit/kunit_kernel.py > +++ b/tools/testing/kunit/kunit_kernel.py > @@ -129,10 +129,15 @@ def get_outfile_path(build_dir) -> str: > class LinuxSourceTree(object): > """Represents a Linux kernel source tree with KUnit tests.""" > > - def __init__(self) -> None: > - self._ops = LinuxSourceTreeOperations() > + def __init__(self, build_dir: str, > defconfig=DEFAULT_KUNITCONFIG_PATH) -> None: > signal.signal(signal.SIGINT, self.signal_handler) > > + self._ops = LinuxSourceTreeOperations() > + > + kunitconfig_path
Re: [PATCH v2 4/4] minor: kunit: tool: fix unit test so it can run from non-root dir
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov wrote:
>
> Also take this time to rename get_absolute_path() to test_data_path().
>
> 1. the name is currently a lie. It gives relative paths, e.g. if I run
> from the same dir as the test file, it gives './test_data/'
>
> See https://docs.python.org/3/reference/import.html#__file__, which
> doesn't stipulate that implementations provide absolute paths.
>
> 2. it's only used for generating paths to tools/testing/kunit/test_data/
> So we can tersen things by making it less general.
>
> Cache the absolute path to the test data files per suggestion from [1].
> Using relative paths, the tests break because of this code in kunit.py
> if get_kernel_root_path():
> os.chdir(get_kernel_root_path())
>
> [1]
> https://lore.kernel.org/linux-kselftest/cabvgosnh0gz7z5jhrcgyg1wg0zddbtlosucob-gwmexlgvt...@mail.gmail.com/
>
> Fixes: 5578d008d9e0 ("kunit: tool: fix running kunit_tool from outside kernel
> tree")
> Signed-off-by: Daniel Latypov
> ---
Thanks: I much prefer this to v1. Having it work the same way as
test_tmpdir is a bonus.
Reviewed-by: David Gow
Cheers,
-- David
Re: [PATCH v2 3/4] kunit: tool: use `with open()` in unit test
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov wrote: > > The use of manual open() and .close() calls seems to be an attempt to > keep the contents in scope. > But Python doesn't restrict variables like that, so we can introduce new > variables inside of a `with` and use them outside. > > Do so to make the code more Pythonic. > > Signed-off-by: Daniel Latypov > --- Reviewed-by: David Gow Cheers, -- David
Re: [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov wrote: > > * Stop leaking file objects. > * Use self.addCleanup() to ensure we call cleanup functions even if > setUp() fails. > * use mock.patch.stopall instead of more error-prone manual approach > > Signed-off-by: Daniel Latypov > --- This patch hasn't changed since v1, right? It's still: Reviewed-by: David Gow Cheers, -- David
Re: [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov wrote: > > Use self.assertEqual/assertNotEqual() instead. > Besides being more appropriate in a unit test, it'll also give a better > error message by show the unexpected values. > > Also > * Delete redundant check of exception types. self.assertRaises does this. > * s/kall/call. There's no reason to name it this way. > * This is probably a misunderstanding from the docs which uses it > since `mock.call` is in scope as `call`. > > Signed-off-by: Daniel Latypov > --- Looks good, thanks! Reviewed-by: David Gow -- David
Re: [PATCH v3] lib: Convert test_hexdump.c to KUnit
On Wed, Dec 2, 2020 at 8:21 PM Andy Shevchenko wrote: > > On Wed, Dec 2, 2020 at 1:57 PM David Gow wrote: > > On Wed, Dec 2, 2020 at 6:06 PM Andy Shevchenko > > wrote: > > > On Wed, Dec 02, 2020 at 09:51:19AM +0530, Arpitha Raghunandan wrote: > > ... > > > > What I;m talking about is the output. How it will be implemented (using > > > the > > > same variable or differently) is up to you. So the point is I want to see > > > the > > > statistics of success/total at the end. > > > > > > I think this should be done in KUNIT rather than in the individual test > > > cases. > > > > I tend to agree here that this really is something for KUnit. At the > > moment, the tools/testing/kunit/kunit.py script will parse the kernel > > log and generate these sorts of statistics. I know that needing to run > > it through a script might seem like a step backwards, but there's no > > formal place for statistics in the KTAP specification[1] being worked > > on to standardise kselftest/kunit output formats. > > Then it sucks. Fix specification (in a long term) and does it have a > comment style of messages that we can have this statistics printed > (but maybe not parsed)? I should clarify: there's nothing in the spec which explicitly defines a place for such statistics (nor anything which requires them). There are "diagnostic" lines which are not parsed, and so it'd be possible to output statistics there. KUnit itself doesn't at present, but allows individual tests to log diagnostic lines which could be such statistics, particularly in cases like this where the full structure of the tests aren't quite exposed to the framework. > > Note that there are > > other parsers for TAP-like formats which are being used with KUnit > > results, so systems like LAVA could also sum up these statistics. It's > > also possible, as Arpitha alluded to, to have the test dump them out > > as a comment. > > Fine to me. > > > This won't actually work for this test as-is, though, as the KUnit > > version is running as a single giant test case (so KUnit believes that > > 1/1 tests have passed, rather than having any more-detailed > > statistics). It looks like there are a few ways to split it up a bit > > which would make it neater (a test each for the for() loops in > > test_hexdump_init() seems sensible to me), but at the moment, there's > > not really a way of programmatically generating test cases which KUnit > > then counts > > Fix it, please. We rely on this statistics pretty much. The hope is that the Parameterised Test feature will make this possible (though, as mentioned, there are a few other issues around then making those statistics available, but we should be able to work through those). It may be a silly question, but what is it that makes these statistics useful in this test? Maybe I'm misunderstanding something, but I'd've thought that the important things were whether or not _all_ tests had passed, and -- if not --- _which_ ones had failed. Is the count of failing cases within a test like this really that useful for debugging, or is it more for comparing against different versions? Either way, we'll try to make sure they're available. > > The "Parameterised Tests"[2] work Arpitha has been working on ought to > > go some way to helping here, though it won't solve this completely in > > this initial version. The problem there is that parameterised tests > > are not reported individually in a way the kunit.py parser can report > > cleanly, yet, so it'll still only be counted as one test until that's > > changed (though, at least, that shouldn't require any test-specific > > work). > > > > My suggestion for the ultimate state of the test would be: > > - Split up the test into separate KUnit tests for the different > > "categories" of tests: (e.g., test_hexdump_set, > > test_hexdump_overflow_set_ascii, etc) > > - Replace the for loops in test_hexdump_init() with parameters, so > > that KUnit is aware of the original runs. > > - Once KUnit and the tooling supports it, these will be reported as > > subtests. (In the meantime, the results will be listed individually, > > commented out) > > I'm fine as long as we have this information printed to the user. Okay -- Arpitha: does this seem like a sensible approach to you? If printing it to the kernel log is really essential, I'll have a look into how we can do that in KUnit. > > Of course, it'll take a while before all of those KUnit pieces are in > > place. I personally think that a good compromise would be to just do > > the first of these for now, which would make
Re: [PATCH v3] lib: Convert test_hexdump.c to KUnit
On Wed, Dec 2, 2020 at 6:06 PM Andy Shevchenko
wrote:
>
> On Wed, Dec 02, 2020 at 09:51:19AM +0530, Arpitha Raghunandan wrote:
> > On 01/12/20 4:36 pm, Andy Shevchenko wrote:
> > > On Tue, Dec 1, 2020 at 9:21 AM Arpitha Raghunandan <98.a...@gmail.com>
> > > wrote:
>
> ...
>
> > >> I ran both the original and converted tests as is to produce the
> > >> output for success of the test in the two cases. I also ran these
> > >> tests with a small modification to show the difference in the output
> > >> for failure of the test in both cases. The modification I made is:
> > >> static const char * const test_data_4_le[] __initconst = {
> > >> - "7bdb32be", "b293180a", "24c4ba70", "9b34837d",
> > >> + "7bdb32be", "b293180a", "24c4ba70", "9b3483d",
> > >>
> > >> The difference in outputs can be seen here:
> > >> https://gist.github.com/arpi-r/38f53a3c65692bf684a6bf3453884b6e
> > >
> > > Looks pretty much good, what I'm sad to see is the absence of the test
> > > statistics. Any ideas if we can get it back?
> > >
> >
> > I could again include variable total_tests as was in the original test.
> > Would that be fine?
>
> What I;m talking about is the output. How it will be implemented (using the
> same variable or differently) is up to you. So the point is I want to see the
> statistics of success/total at the end.
>
> I think this should be done in KUNIT rather than in the individual test cases.
I tend to agree here that this really is something for KUnit. At the
moment, the tools/testing/kunit/kunit.py script will parse the kernel
log and generate these sorts of statistics. I know that needing to run
it through a script might seem like a step backwards, but there's no
formal place for statistics in the KTAP specification[1] being worked
on to standardise kselftest/kunit output formats. Note that there are
other parsers for TAP-like formats which are being used with KUnit
results, so systems like LAVA could also sum up these statistics. It's
also possible, as Arpitha alluded to, to have the test dump them out
as a comment.
This won't actually work for this test as-is, though, as the KUnit
version is running as a single giant test case (so KUnit believes that
1/1 tests have passed, rather than having any more-detailed
statistics). It looks like there are a few ways to split it up a bit
which would make it neater (a test each for the for() loops in
test_hexdump_init() seems sensible to me), but at the moment, there's
not really a way of programmatically generating test cases which KUnit
then counts
The "Parameterised Tests"[2] work Arpitha has been working on ought to
go some way to helping here, though it won't solve this completely in
this initial version. The problem there is that parameterised tests
are not reported individually in a way the kunit.py parser can report
cleanly, yet, so it'll still only be counted as one test until that's
changed (though, at least, that shouldn't require any test-specific
work).
My suggestion for the ultimate state of the test would be:
- Split up the test into separate KUnit tests for the different
"categories" of tests: (e.g., test_hexdump_set,
test_hexdump_overflow_set_ascii, etc)
- Replace the for loops in test_hexdump_init() with parameters, so
that KUnit is aware of the original runs.
- Once KUnit and the tooling supports it, these will be reported as
subtests. (In the meantime, the results will be listed individually,
commented out)
Of course, it'll take a while before all of those KUnit pieces are in
place. I personally think that a good compromise would be to just do
the first of these for now, which would make kunit_tool give at least
a 4/4 rather than 1/1 result. Then, once the parameterised testing
work is merged (and perhaps the tooling fixes are finished), the tests
could be updated to take advantage of that.
Cheres,
-- David
[1]:
https://lore.kernel.org/linux-kselftest/cy4pr13mb1175b804e31e502221bc8163fd...@cy4pr13mb1175.namprd13.prod.outlook.com/T/
[2]:
https://lore.kernel.org/linux-kselftest/20201116054035.211498-1-98.a...@gmail.com/
Re: [PATCH 2/5] kunit: tool: fix unit test so it can run from non-root dir
On Wed, Dec 2, 2020 at 3:00 AM Daniel Latypov wrote:
>
> On Mon, Nov 30, 2020 at 11:33 PM David Gow wrote:
> >
> > On Tue, Dec 1, 2020 at 7:33 AM Daniel Latypov wrote:
> > >
> > > get_absolute_path() makes an attempt to allow for this.
> > > But that doesn't work as soon as os.chdir() gets called.
> >
> > Can we explain why this doesn't work? It's because the test_data/
> > files are accessed with relative paths, so chdir breaks access to
> > them, right?
>
> Correct.
> Because it actually returns a relative path.
>
> (I forgot that I called out that get_absolute_path() gives relative
> paths in the last patch, and not this one. I can update the commit
> desc if we want a v2 of this)
>
> >
> > >
> > > So make it so that os.chdir() does nothing to avoid this.
> > >
> > > Note: mock.patch.object() doesn't seem to work in setUpModule(), hence
> > > the introduction of a new base class instead.
> > >
> > > Fixes: 5578d008d9e0 ("kunit: tool: fix running kunit_tool from outside
> > > kernel tree")
> > > Signed-off-by: Daniel Latypov
> > > ---
> >
> > I don't like this: disabling a real system call seems like overkill to
> > work around a path issue like this.
> >
> > Wouldn't it be better to either:
> > (a) stop kunit_tool from needing to chdir entirely; or
>
> a) is doable, but would require plumbing cwd=get_kernel_root_path() to
> all the subprocess calls to make, etc.
> I'm not sure fixing a internal test-only issue necessarily justifies
> taking that path instead of the easier "just add a chdir" we opted for
> in 5578d008d9e0 ("kunit: tool: fix running kunit_tool from outside
> kernel tree").
>
> > (b) have get_absolute_path() / test_data_path() produce an absolute path.
> >
> > The latter really seems like the most sensible approach: have the
> > script read its own path and read files relative to that.
>
> b) is not that simple, sadly.
>
> Say I invoke
> $ python3 kunit_tool_test.py
> then __file__ = kunit_tool_test.py.
>
> So __file__ is a relative path, but the code assumed it was an
> absolute one and any change of directory breaks things.
> Working around that would require something like caching the result of
> os.path.abspath(__file__) somewhere.
So, to clarify, __file__ is a relative path based on the cwd when the
script is initially run, right?
In any case, caching the result of os.path.abspath(__file__) seems
like the most sensible solution to me. There's global state anyway
(the current directory), we might as well have it in an explicit
variable, IMHO.
>
> I can see the point about not mocking out something like os.chdir().
> But on the other hand, do we have any other legitimate reason to call
> it besides that one place in kunit.py?
> If we do have something that relies on doing an os.chdir(), it should
> ideally notice that it didn't work and manifest in a unit test failure
> somehow.
Certainly there doesn't seem to be any other need to chdir() in
kunit_tool at the moment, but I could see us doing so when adding
other features. More to the point, if both kunit.py and
kunit_tool_test.py rely on or manipulate the current directory as part
of their state, that seems like it's asking for some trouble down the
line.
If we use an absolute path for the test data, that's something that
seems unlikely to ever need further changes or cause issues.
>
> Alternatively, we could make get_kernel_root_path() return ""/None to
> avoid the chdir call.
> kunit.py: if get_kernel_root_path():
> kunit.py: os.chdir(get_kernel_root_path())
>
> There'd be no adverse affects atm for stubbing that out, and I don't forsee
> any.
> But if we want to be even safer, then perhaps we have
>
> def chdir_to_kernel_root():
>...
>
> and mock out just that func in the unit test?
I'd be okay with this, even if I'd prefer us to use an absolute path
for the test_data as well. Having something like this might even give
us the opportunity to verify that we're actually trying to change to
the kernel directory in cases where we need to, but that seems like
it's out-of-scope for a simple fix.
-- David
Re: [PATCH 5/5] minor: kunit: tool: s/get_absolute_path/test_data_path in unit test
On Tue, Dec 1, 2020 at 7:33 AM Daniel Latypov wrote: > > 1. the name is a lie. It gives relative paths, e.g. if I run from the > same dir as the test file, it gives './test_data/' > > 2. it's only used for generating paths to tools/testing/kunit/test_data/ > So we can tersen things by making it less general. > > Signed-off-by: Daniel Latypov > --- This is an excellent and overdue rename/replacement. My only note is re: the concerns I have in patch 2, where I think we probably ought to make this function actually return an absolute path. It seems from the code (and the function name) that that was the intent, so if we can fix it, that'd be ideal. Personally, though, I'd still prefer the new test_data_path(), just have it be an actually absolute path. -- David
Re: [PATCH 4/5] kunit: tool: use `with open()` in unit test
On Tue, Dec 1, 2020 at 7:33 AM Daniel Latypov wrote: > > The use of manual open() and .close() calls seems to be an attempt to > keep the contents in scope. > But Python doesn't restrict variables like that, so we can introduce new > variables inside of a `with` and use them outside. > > Do so to make the code more Pythonic. > > Signed-off-by: Daniel Latypov > --- I'm fine with this, and it clearly works fine for me. Out of curiosity, though, is there any difference here other than it being more usual Python style? We've struggled a bit in the past toeing a line between trying to follow "normal" Python style versus adapting it a bit to be more "kernel-y". Experience thus far has actually been that going out on our own has caused more problems than it solves, so I'm all for this change, but I do admit that my brain does understand the older code a touch more easily. In any case, Reviewed-by: David Gow -- David
Re: [PATCH 2/5] kunit: tool: fix unit test so it can run from non-root dir
On Tue, Dec 1, 2020 at 7:33 AM Daniel Latypov wrote:
>
> get_absolute_path() makes an attempt to allow for this.
> But that doesn't work as soon as os.chdir() gets called.
Can we explain why this doesn't work? It's because the test_data/
files are accessed with relative paths, so chdir breaks access to
them, right?
>
> So make it so that os.chdir() does nothing to avoid this.
>
> Note: mock.patch.object() doesn't seem to work in setUpModule(), hence
> the introduction of a new base class instead.
>
> Fixes: 5578d008d9e0 ("kunit: tool: fix running kunit_tool from outside kernel
> tree")
> Signed-off-by: Daniel Latypov
> ---
I don't like this: disabling a real system call seems like overkill to
work around a path issue like this.
Wouldn't it be better to either:
(a) stop kunit_tool from needing to chdir entirely; or
(b) have get_absolute_path() / test_data_path() produce an absolute path.
The latter really seems like the most sensible approach: have the
script read its own path and read files relative to that.
Cheers,
-- David
> tools/testing/kunit/kunit_tool_test.py | 15 +++
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit_tool_test.py
> b/tools/testing/kunit/kunit_tool_test.py
> index 3fbe1acd531a..9f1f1e1b772a 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -32,7 +32,13 @@ def tearDownModule():
> def get_absolute_path(path):
> return os.path.join(os.path.dirname(__file__), path)
>
> -class KconfigTest(unittest.TestCase):
> +class KUnitTest(unittest.TestCase):
> + """Contains common setup, like stopping main() from calling chdir."""
> + def setUp(self):
> + mock.patch.object(os, 'chdir').start()
> + self.addCleanup(mock.patch.stopall)
> +
> +class KconfigTest(KUnitTest):
>
> def test_is_subset_of(self):
> kconfig0 = kunit_config.Kconfig()
> @@ -88,7 +94,7 @@ class KconfigTest(unittest.TestCase):
> self.assertEqual(actual_kconfig.entries(),
> expected_kconfig.entries())
>
> -class KUnitParserTest(unittest.TestCase):
> +class KUnitParserTest(KUnitTest):
>
> def assertContains(self, needle, haystack):
> for line in haystack:
> @@ -250,7 +256,7 @@ class KUnitParserTest(unittest.TestCase):
> result.status)
> self.assertEqual('kunit-resource-test',
> result.suites[0].name)
>
> -class KUnitJsonTest(unittest.TestCase):
> +class KUnitJsonTest(KUnitTest):
>
> def _json_for(self, log_file):
> with(open(get_absolute_path(log_file))) as file:
> @@ -285,8 +291,9 @@ class StrContains(str):
> def __eq__(self, other):
> return self in other
>
> -class KUnitMainTest(unittest.TestCase):
> +class KUnitMainTest(KUnitTest):
> def setUp(self):
> + super().setUp()
> path =
> get_absolute_path('test_data/test_is_test_passed-all_passed.log')
> with open(path) as file:
> all_passed_log = file.readlines()
> --
> 2.29.2.454.gaff20da3a2-goog
>
Re: [PATCH 3/5] kunit: tool: stop using bare asserts in unit test
On Tue, Dec 1, 2020 at 7:33 AM Daniel Latypov wrote: > > Use self.assertEqual/assertNotEqual() instead. > Besides being more appropriate in a unit test, it'll also give a better > error message by show the unexpected values. > > Also > * Delete redundant check of exception types. self.assertRaises does this. > * s/kall/call. There's no reason to name it this way. > * This is probably a misunderstanding from the docs which uses it > since `mock.call` is in scope as `call`. > > Signed-off-by: Daniel Latypov > --- This works for me, and seems pretty sensible from my rudimentary python knowledge. Reviewed-by: David Gow Cheers, -- David
Re: [PATCH 1/5] kunit: tool: fix unit test cleanup handling
On Tue, Dec 1, 2020 at 7:32 AM Daniel Latypov wrote:
>
> * Stop leaking file objects.
> * Use self.addCleanup() to ensure we call cleanup functions even if
> setUp() fails.
> * use mock.patch.stopall instead of more error-prone manual approach
>
> Signed-off-by: Daniel Latypov
> ---
I won't pretend to be an expert on Python, but this seems good to me.
I tested it on my machine and it works fine.
So,
Reviewed-by: David Gow
-- Davkd
> tools/testing/kunit/kunit_tool_test.py | 14 ++
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit_tool_test.py
> b/tools/testing/kunit/kunit_tool_test.py
> index 497ab51bc170..3fbe1acd531a 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -288,19 +288,17 @@ class StrContains(str):
> class KUnitMainTest(unittest.TestCase):
> def setUp(self):
> path =
> get_absolute_path('test_data/test_is_test_passed-all_passed.log')
> - file = open(path)
> - all_passed_log = file.readlines()
> - self.print_patch = mock.patch('builtins.print')
> - self.print_mock = self.print_patch.start()
> + with open(path) as file:
> + all_passed_log = file.readlines()
> +
> + self.print_mock = mock.patch('builtins.print').start()
> + self.addCleanup(mock.patch.stopall)
> +
> self.linux_source_mock = mock.Mock()
> self.linux_source_mock.build_reconfig =
> mock.Mock(return_value=True)
> self.linux_source_mock.build_um_kernel =
> mock.Mock(return_value=True)
> self.linux_source_mock.run_kernel =
> mock.Mock(return_value=all_passed_log)
>
> - def tearDown(self):
> - self.print_patch.stop()
> - pass
> -
> def test_config_passes_args_pass(self):
> kunit.main(['config', '--build_dir=.kunit'],
> self.linux_source_mock)
> assert self.linux_source_mock.build_reconfig.call_count == 1
>
> base-commit: b65054597872ce3aefbc6a666385eabdf9e288da
> --
> 2.29.2.454.gaff20da3a2-goog
>
Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
On Mon, Nov 23, 2020 at 9:08 PM Marco Elver wrote: > > On Tue, 17 Nov 2020 at 08:21, David Gow wrote: > > On Mon, Nov 16, 2020 at 1:41 PM Arpitha Raghunandan <98.a...@gmail.com> > > wrote: > > > > > > Implementation of support for parameterized testing in KUnit. This > > > approach requires the creation of a test case using the > > > KUNIT_CASE_PARAM() macro that accepts a generator function as input. > > > > > > This generator function should return the next parameter given the > > > previous parameter in parameterized tests. It also provides a macro to > > > generate common-case generators based on arrays. Generators may also > > > optionally provide a human-readable description of parameters, which is > > > displayed where available. > > > > > > Note, currently the result of each parameter run is displayed in > > > diagnostic lines, and only the overall test case output summarizes > > > TAP-compliant success or failure of all parameter runs. In future, when > > > supported by kunit-tool, these can be turned into subsubtest outputs. > > > > > > Signed-off-by: Arpitha Raghunandan <98.a...@gmail.com> > > > Co-developed-by: Marco Elver > > > Signed-off-by: Marco Elver > > > --- > > [Resending this because my email client re-defaulted to HTML! Aarrgh!] > > > > This looks good to me! I tested it in UML and x86-64 w/ KASAN, and > > both worked fine. > > > > Reviewed-by: David Gow > > Tested-by: David Gow > > Thank you! > > > Thanks for sticking with this! > > Will these patches be landing in 5.11 or 5.12? > I can't think of any reason not to have these in 5.11. We haven't started staging things in the kselftest/kunit branch for 5.11 yet, though. Patch 2 will probably need to be acked by Ted for ext4 first. Brendan, Shuah: can you make sure this doesn't get lost in patchwork? Cheers, -- David > > -- David > > Thanks, > -- Marco
Re: [RFC PATCH] bpf: preload: Fix build error when O= is set
On Sat, Nov 21, 2020 at 3:38 PM Andrii Nakryiko
wrote:
>
> On Thu, Nov 19, 2020 at 12:51 AM David Gow wrote:
> >
> > If BPF_PRELOAD is enabled, and an out-of-tree build is requested with
> > make O=, compilation seems to fail with:
> >
> > tools/scripts/Makefile.include:4: *** O=.kunit does not exist. Stop.
> > make[4]: *** [../kernel/bpf/preload/Makefile:8:
> > kernel/bpf/preload/libbpf.a] Error 2
> > make[3]: *** [../scripts/Makefile.build:500: kernel/bpf/preload] Error 2
> > make[2]: *** [../scripts/Makefile.build:500: kernel/bpf] Error 2
> > make[2]: *** Waiting for unfinished jobs
> > make[1]: *** [.../Makefile:1799: kernel] Error 2
> > make[1]: *** Waiting for unfinished jobs
> > make: *** [Makefile:185: __sub-make] Error 2
> >
> > By the looks of things, this is because the (relative path) O= passed on
> > the command line is being passed to the libbpf Makefile, which then
> > can't find the directory. Given OUTPUT= is being passed anyway, we can
> > work around this by explicitly setting an empty O=, which will be
> > ignored in favour of OUTPUT= in tools/scripts/Makefile.include.
>
> Strange, but I can't repro it. I use make O= all the
> time with no issues. I just tried specifically with a make O=.build,
> where .build is inside Linux repo, and it still worked fine. See also
> be40920fbf10 ("tools: Let O= makes handle a relative path with -C
> option") which was supposed to address such an issue. So I'm wondering
> what exactly is causing this problem.
>
[+ linux-um list]
Hmm... From a quick check, I can't reproduce this on x86, so it's
possibly a UML-specific issue.
The problem here seems to be that $PWD is, for whatever reason, equal
to the srcdir on x86, but not on UML. In general, $PWD behaves pretty
weirdly -- I don't fully understand it -- but if I add a tactical "PWD
:= $(shell pwd)" or use $(CURDIR) instead, the issue shows up on x86
as well. I guess this is because PWD only gets updated when set by a
shell or something, and UML does this somewhere?
Thoughts?
Cheers,
-- David
> >
> > Signed-off-by: David Gow
> > ---
> >
> > Hi all,
> >
> > I'm not 100% sure this is the correct fix here -- it seems to work for
> > me, and makes some sense, but let me know if there's a better way.
> >
> > One other thing worth noting is that I've been hitting this with
> > make allyesconfig on ARCH=um, but there's a comment in the Kconfig
> > suggesting that, because BPF_PRELOAD depends on !COMPILE_TEST, that
> > maybe it shouldn't be being built at all. I figured that it was worth
> > trying to fix this anyway.
> >
> > Cheers,
> > -- David
> >
> >
> > kernel/bpf/preload/Makefile | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/bpf/preload/Makefile b/kernel/bpf/preload/Makefile
> > index 23ee310b6eb4..39848d296097 100644
> > --- a/kernel/bpf/preload/Makefile
> > +++ b/kernel/bpf/preload/Makefile
> > @@ -5,7 +5,7 @@ LIBBPF_A = $(obj)/libbpf.a
> > LIBBPF_OUT = $(abspath $(obj))
> >
> > $(LIBBPF_A):
> > - $(Q)$(MAKE) -C $(LIBBPF_SRCS) OUTPUT=$(LIBBPF_OUT)/
> > $(LIBBPF_OUT)/libbpf.a
> > + $(Q)$(MAKE) -C $(LIBBPF_SRCS) O= OUTPUT=$(LIBBPF_OUT)/
> > $(LIBBPF_OUT)/libbpf.a
> >
> > userccflags += -I $(srctree)/tools/include/ -I
> > $(srctree)/tools/include/uapi \
> > -I $(srctree)/tools/lib/ -Wno-unused-result
> > --
> > 2.29.2.454.gaff20da3a2-goog
> >
[PATCH v2] drivers: fpga: Specify HAS_IOMEM dependency for FPGA_DFL
Because dfl.c uses the 'devm_ioremap', 'devm_iounmap',
'devm_ioremap_resource', and 'devm_platform_ioremap_resource'
functions, it should depend on HAS_IOMEM.
This fixes make allyesconfig under UML (ARCH=um), which doesn't provide
HAS_IOMEM.
Fixes: 89eb35e810a8 ("fpga: dfl: map feature mmio resources in their own
feature drivers")
Signed-off-by: David Gow
---
Changes since v1:
(
https://lore.kernel.org/linux-fpga/20201119082209.3598354-1-david...@google.com/
)
- Add Fixes tag
drivers/fpga/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index 7cd5a29fc437..5645226ca3ce 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -142,6 +142,7 @@ config FPGA_DFL
tristate "FPGA Device Feature List (DFL) support"
select FPGA_BRIDGE
select FPGA_REGION
+ depends on HAS_IOMEM
help
Device Feature List (DFL) defines a feature list structure that
creates a linked list of feature headers within the MMIO space
--
2.29.2.454.gaff20da3a2-goog
Re: [PATCH] drivers: fpga: Specify HAS_IOMEM dependency for FPGA_DFL
On Fri, Nov 20, 2020 at 2:27 PM Moritz Fischer wrote:
>
> Hi David,
>
> On Thu, Nov 19, 2020 at 12:22:09AM -0800, David Gow wrote:
> > Because dfl.c uses the 'devm_ioremap', 'devm_iounmap',
> > 'devm_ioremap_resource', and 'devm_platform_ioremap_resource'
> > functions, it should depend on HAS_IOMEM.
> >
> > This fixes make allyesconfig under UML (ARCH=um), which doesn't provide
> > HAS_IOMEM.
> >
> > Signed-off-by: David Gow
> > ---
> > drivers/fpga/Kconfig | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
> > index 7cd5a29fc437..5645226ca3ce 100644
> > --- a/drivers/fpga/Kconfig
> > +++ b/drivers/fpga/Kconfig
> > @@ -142,6 +142,7 @@ config FPGA_DFL
> > tristate "FPGA Device Feature List (DFL) support"
> > select FPGA_BRIDGE
> > select FPGA_REGION
> > + depends on HAS_IOMEM
> > help
> > Device Feature List (DFL) defines a feature list structure that
> > creates a linked list of feature headers within the MMIO space
> > --
> > 2.29.2.454.gaff20da3a2-goog
> >
> Do you think we can add a Fixes: tag for this?
Sure. I think it should be:
Fixes: 543be3d ("fpga: add device feature list support")
Let me know if you want me to re-send the patch with that included.
Cheers,
-- David
[RFC PATCH] bpf: preload: Fix build error when O= is set
If BPF_PRELOAD is enabled, and an out-of-tree build is requested with make O=, compilation seems to fail with: tools/scripts/Makefile.include:4: *** O=.kunit does not exist. Stop. make[4]: *** [../kernel/bpf/preload/Makefile:8: kernel/bpf/preload/libbpf.a] Error 2 make[3]: *** [../scripts/Makefile.build:500: kernel/bpf/preload] Error 2 make[2]: *** [../scripts/Makefile.build:500: kernel/bpf] Error 2 make[2]: *** Waiting for unfinished jobs make[1]: *** [.../Makefile:1799: kernel] Error 2 make[1]: *** Waiting for unfinished jobs make: *** [Makefile:185: __sub-make] Error 2 By the looks of things, this is because the (relative path) O= passed on the command line is being passed to the libbpf Makefile, which then can't find the directory. Given OUTPUT= is being passed anyway, we can work around this by explicitly setting an empty O=, which will be ignored in favour of OUTPUT= in tools/scripts/Makefile.include. Signed-off-by: David Gow --- Hi all, I'm not 100% sure this is the correct fix here -- it seems to work for me, and makes some sense, but let me know if there's a better way. One other thing worth noting is that I've been hitting this with make allyesconfig on ARCH=um, but there's a comment in the Kconfig suggesting that, because BPF_PRELOAD depends on !COMPILE_TEST, that maybe it shouldn't be being built at all. I figured that it was worth trying to fix this anyway. Cheers, -- David kernel/bpf/preload/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/bpf/preload/Makefile b/kernel/bpf/preload/Makefile index 23ee310b6eb4..39848d296097 100644 --- a/kernel/bpf/preload/Makefile +++ b/kernel/bpf/preload/Makefile @@ -5,7 +5,7 @@ LIBBPF_A = $(obj)/libbpf.a LIBBPF_OUT = $(abspath $(obj)) $(LIBBPF_A): - $(Q)$(MAKE) -C $(LIBBPF_SRCS) OUTPUT=$(LIBBPF_OUT)/ $(LIBBPF_OUT)/libbpf.a + $(Q)$(MAKE) -C $(LIBBPF_SRCS) O= OUTPUT=$(LIBBPF_OUT)/ $(LIBBPF_OUT)/libbpf.a userccflags += -I $(srctree)/tools/include/ -I $(srctree)/tools/include/uapi \ -I $(srctree)/tools/lib/ -Wno-unused-result -- 2.29.2.454.gaff20da3a2-goog
[PATCH] staging: hikey9xx: Specify HAS_IOMEM dependency for MFD_HI6421_SPMI
MFD_CORE is selected by MFD_HI6421_SPMI, and MFD_CORE depends on HAS_IOMEM. If HAS_IOMEM is not set, this can cause a conflict in Kconfig resolution, yielding the following error: WARNING: unmet direct dependencies detected for MFD_CORE Depends on [n]: HAS_IOMEM [=n] Selected by [y]: - MFD_HI6421_SPMI [=y] && STAGING [=y] && OF [=y] && SPMI [=y] By specifying HAS_IOMEM as a dependency for MFD_HI6421_SPMI (as SPMI_HISI3670 already dows), this issue is resolved, and no such warning appears when building on architectures without HAS_IOMEM. Signed-off-by: David Gow --- drivers/staging/hikey9xx/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/hikey9xx/Kconfig b/drivers/staging/hikey9xx/Kconfig index b29f5d5df134..2e48ded92a7e 100644 --- a/drivers/staging/hikey9xx/Kconfig +++ b/drivers/staging/hikey9xx/Kconfig @@ -25,6 +25,7 @@ config SPMI_HISI3670 # to be placed at drivers/mfd config MFD_HI6421_SPMI tristate "HiSilicon Hi6421v600 SPMI PMU/Codec IC" + depends on HAS_IOMEM depends on OF depends on SPMI select MFD_CORE -- 2.29.2.454.gaff20da3a2-goog
[PATCH] drivers: fpga: Specify HAS_IOMEM dependency for FPGA_DFL
Because dfl.c uses the 'devm_ioremap', 'devm_iounmap', 'devm_ioremap_resource', and 'devm_platform_ioremap_resource' functions, it should depend on HAS_IOMEM. This fixes make allyesconfig under UML (ARCH=um), which doesn't provide HAS_IOMEM. Signed-off-by: David Gow --- drivers/fpga/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig index 7cd5a29fc437..5645226ca3ce 100644 --- a/drivers/fpga/Kconfig +++ b/drivers/fpga/Kconfig @@ -142,6 +142,7 @@ config FPGA_DFL tristate "FPGA Device Feature List (DFL) support" select FPGA_BRIDGE select FPGA_REGION + depends on HAS_IOMEM help Device Feature List (DFL) defines a feature list structure that creates a linked list of feature headers within the MMIO space -- 2.29.2.454.gaff20da3a2-goog
Re: [PATCH v9 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature
On Mon, Nov 16, 2020 at 1:42 PM Arpitha Raghunandan <98.a...@gmail.com> wrote: > > Modify fs/ext4/inode-test.c to use the parameterized testing > feature of KUnit. > > Signed-off-by: Arpitha Raghunandan <98.a...@gmail.com> > Signed-off-by: Marco Elver > --- [Resending this because the HTML-email demon struck again! Sorry for the mess!] Thanks: this is working well over here. The only (minor) issue I've noticed is that the diagnostic output is too big for the default log buffer if debugfs output is used, causing some of the messages to be dropped from the debugfs results file. But that's clearly not an issue with this patch, and the actual combined result is still present (and the complete results should show up in dmesg anyway). Tested-by: David Gow Reviewed-by: David Gow Thanks! -- David
Re: [PATCH v9 1/2] kunit: Support for Parameterized Testing
On Mon, Nov 16, 2020 at 1:41 PM Arpitha Raghunandan <98.a...@gmail.com> wrote: > > Implementation of support for parameterized testing in KUnit. This > approach requires the creation of a test case using the > KUNIT_CASE_PARAM() macro that accepts a generator function as input. > > This generator function should return the next parameter given the > previous parameter in parameterized tests. It also provides a macro to > generate common-case generators based on arrays. Generators may also > optionally provide a human-readable description of parameters, which is > displayed where available. > > Note, currently the result of each parameter run is displayed in > diagnostic lines, and only the overall test case output summarizes > TAP-compliant success or failure of all parameter runs. In future, when > supported by kunit-tool, these can be turned into subsubtest outputs. > > Signed-off-by: Arpitha Raghunandan <98.a...@gmail.com> > Co-developed-by: Marco Elver > Signed-off-by: Marco Elver > --- [Resending this because my email client re-defaulted to HTML! Aarrgh!] This looks good to me! I tested it in UML and x86-64 w/ KASAN, and both worked fine. Reviewed-by: David Gow Tested-by: David Gow Thanks for sticking with this! -- David
Re: [PATCH v6 1/2] kunit: Support for Parameterized Testing
On Sat, Nov 14, 2020 at 9:38 AM Arpitha Raghunandan <98.a...@gmail.com> wrote: > > On 14/11/20 5:44 am, Marco Elver wrote: > > > > Arpitha: Do you want to send v7, but with the following modifications > > from what I proposed? Assuming nobody objects. > > > > 1. Remove the num_params counter and don't print the number of params > > anymore, nor do validation that generators are deterministic. > > 2. Remove the []. > > [ I'm happy to send as well, just let me know what you prefer. ] > > > > Thanks, > > -- Marco > > > > If no objections I will send the v7 with the above changes. > Thanks! This sounds good to me! Cheers, -- David
Re: [PATCH v6 1/2] kunit: Support for Parameterized Testing
On Fri, Nov 13, 2020 at 6:31 PM Marco Elver wrote: > > On Fri, Nov 13, 2020 at 01:17PM +0800, David Gow wrote: > > On Thu, Nov 12, 2020 at 8:37 PM Marco Elver wrote: > [...] > > > > (It also might be a little tricky with the current implementation to > > > > produce the test plan, as the parameters come from a generator, and I > > > > don't think there's a way of getting the number of parameters ahead of > > > > time. That's a problem with the sub-subtest model, too, though at > > > > least there it's a little more isolated from other tests.) > > > > > > The whole point of generators, as I envisage it, is to also provide the > > > ability for varying parameters dependent on e.g. environment, > > > configuration, number of CPUs, etc. The current array-based generator is > > > the simplest possible use-case. > > > > > > However, we *can* require generators generate a deterministic number of > > > parameters when called multiple times on the same system. > > > > I think this is a reasonable compromise, though it's not actually > > essential. As I understand the TAP spec, the test plan is actually > > optional (and/or can be at the end of the sequence of tests), though > > kunit_tool currently only supports having it at the beginning (which > > is strongly preferred by the spec anyway). I think we could get away > > with having it at the bottom of the subtest results though, which > > would save having to run the generator twice, when subtest support is > > added to kunit_tool. > > I can't find this in the TAP spec, where should I look? Perhaps we > shouldn't venture too far off the beaten path, given we might not be the > only ones that want to parse this output. > It's in the "Test Lines and the Plan" section: "The plan is optional but if there is a plan before the test points it must be the first non-diagnostic line output by the test file. In certain instances a test file may not know how many test points it will ultimately be running. In this case the plan can be the last non-diagnostic line in the output. The plan cannot appear in the middle of the output, nor can it appear more than once." My only concern with running through the generator multiple times to get the count is that it might be slow and/or more difficult if someone uses a more complicated generator. I can't think of anything specific yet, though, so we can always do it for now and change it later if a problematic case occurs. > > > To that end, I propose a v7 (below) that takes care of getting number of > > > parameters (and also displays descriptions for each parameter where > > > available). > > > > > > Now it is up to you how you want to turn the output from diagnostic > > > lines into something TAP compliant, because now we have the number of > > > parameters and can turn it into a subsubtest. But I think kunit-tool > > > doesn't understand subsubtests yet, so I suggest we take these patches, > > > and then somebody can prepare kunit-tool. > > > > > > > This sounds good to me. The only thing I'm not sure about is the > > format of the parameter description: thus far test names be valid C > > identifier names, due to the fact they're named after the test > > function. I don't think there's a fundamental reason parameters (and > > hence, potentially, subsubtests) need to follow that convention as > > well, but it does look a bit odd. Equally, the square brackets around > > the description shouldn't be necessary according to the TAP spec, but > > do seem to make things a little more readable, particuarly with the > > names in the ext4 inode test. I'm not too worried about either of > > those, though: I'm sure it'll look fine once I've got used to it. > > The parameter description doesn't need to be a C identifier. At least > that's what I could immediately glean from TAP v13 spec (I'm looking > here: https://testanything.org/tap-version-13-specification.html and see > e.g. "ok 1 - Input file opened" ...). > Yeah: it looked a bit weird for everything else to be an identifier (given that KUnit does require it for tests), but these parameter descriptions not to be. It's not a problem, though, so let's go ahead with it. > [...] > > > > In any case, I'm happy to leave the final decision here to Arpitha and > > > > Marco, so long as we don't actually violate the TAP/KTAP spec and > > > > kunit_tool is able to read at least the top-level result. My > > > > preference is still to go either with the "# [test_case->name]: > > > > [ok|not ok] [index] - param-
Re: [PATCH v6 1/2] kunit: Support for Parameterized Testing
On Thu, Nov 12, 2020 at 8:37 PM Marco Elver wrote: > > On Thu, Nov 12, 2020 at 04:18PM +0800, David Gow wrote: > > On Thu, Nov 12, 2020 at 12:55 AM Bird, Tim wrote: > [...] > > > > kunit_tool has a bug when parsing the comments / diagnostic lines, > > > > which requires a ": " to be present. This is a bug, which is being > > > > fixed[1], so while it's _nice_ to not trigger it, it's not really an > > > > important long-term goal of the format. In any case, this kunit_tool > > > > issue only affects the comment lines: if the per-parameter result line > > > > is an actual result, rather than just a diagnostic, this shouldn't be > > > > a problem. > > > > > > > > In any case, I still prefer my proposed option for now -- noting that > > > > these per-parameter results are not actually supposed to be parsed -- > > > > with then the possibility of expanding them to actual nested results > > > > later should we wish. But if the idea of having TAP-like lines in > > > > diagnostics seems too dangerous (e.g. because people will try to parse > > > > them anyway), then I think the options we have are to stick to the > > > > output format given in the current version of this patch (which > > > > doesn't resemble a TAP result), make each parameterised version its > > > > own test (without a "container test", which would require a bit of > > > > extra work while computing test plans), or to hold this whole feature > > > > back until we can support arbitrary test hierarchies in KUnit. > > > It seems like you're missing a 4th option, which is just tack the > > > parameter > > > name on, without using a colon, and have these testcases treated > > > as unique within the context of the super-test. Is there some reason > > > these can't be expressed as individual testcases in the parent test? > > > > > > > No: there's no fundamental reason why we couldn't do that, though > > there are some things which make it suboptiomal, IMHO. > > > > Firstly, there could be a lot of parameters, and that by not grouping > > them together it could make dealing with the results a little > > unwieldy. The other side of that is that it'll result in tests being > > split up and renamed as they're ported to use this, whereas if the > > whole test shows up once (with subtests or without), the old test name > > can still be there, with a single PASS/FAIL for the whole test. > > Agree, it's suboptimal and having the parameterized not be absorbed by > the whole suite would be strongly preferred. > > > (It also might be a little tricky with the current implementation to > > produce the test plan, as the parameters come from a generator, and I > > don't think there's a way of getting the number of parameters ahead of > > time. That's a problem with the sub-subtest model, too, though at > > least there it's a little more isolated from other tests.) > > The whole point of generators, as I envisage it, is to also provide the > ability for varying parameters dependent on e.g. environment, > configuration, number of CPUs, etc. The current array-based generator is > the simplest possible use-case. > > However, we *can* require generators generate a deterministic number of > parameters when called multiple times on the same system. I think this is a reasonable compromise, though it's not actually essential. As I understand the TAP spec, the test plan is actually optional (and/or can be at the end of the sequence of tests), though kunit_tool currently only supports having it at the beginning (which is strongly preferred by the spec anyway). I think we could get away with having it at the bottom of the subtest results though, which would save having to run the generator twice, when subtest support is added to kunit_tool. > To that end, I propose a v7 (below) that takes care of getting number of > parameters (and also displays descriptions for each parameter where > available). > > Now it is up to you how you want to turn the output from diagnostic > lines into something TAP compliant, because now we have the number of > parameters and can turn it into a subsubtest. But I think kunit-tool > doesn't understand subsubtests yet, so I suggest we take these patches, > and then somebody can prepare kunit-tool. > This sounds good to me. The only thing I'm not sure about is the format of the parameter description: thus far test names be valid C identifier names, due to the fact they're named after the test function. I don't think there's a fundamental reason parameters (and hence, potentially, subs
Re: [PATCH v6 1/2] kunit: Support for Parameterized Testing
On Thu, Nov 12, 2020 at 12:55 AM Bird, Tim wrote: > > > > > -Original Message- > > From: David Gow > > > > On Wed, Nov 11, 2020 at 1:02 AM Bird, Tim wrote: > > > > > > > -Original Message- > > > > From: David Gow > > > > > > > > On Mon, Nov 9, 2020 at 2:49 PM Arpitha Raghunandan <98.a...@gmail.com> > > > > wrote: > > > > > > > > > > On 07/11/20 3:36 pm, Marco Elver wrote: > > > > > > On Sat, 7 Nov 2020 at 05:58, David Gow wrote: > > > > > >> On Sat, Nov 7, 2020 at 3:22 AM Arpitha Raghunandan > > > > > >> <98.a...@gmail.com> wrote: > > > > > >>> > > > > > >>> Implementation of support for parameterized testing in KUnit. > > > > > >>> This approach requires the creation of a test case using the > > > > > >>> KUNIT_CASE_PARAM macro that accepts a generator function as input. > > > > > >>> This generator function should return the next parameter given the > > > > > >>> previous parameter in parameterized tests. It also provides > > > > > >>> a macro to generate common-case generators. > > > > > >>> > > > > > >>> Signed-off-by: Arpitha Raghunandan <98.a...@gmail.com> > > > > > >>> Co-developed-by: Marco Elver > > > > > >>> Signed-off-by: Marco Elver > > > > > >>> --- > > > > > >> > > > > > >> This looks good to me! A couple of minor thoughts about the output > > > > > >> format below, but I'm quite happy to have this as-is regardless. > > > > > >> > > > > > >> Reviewed-by: David Gow > > > > > >> > > > > > >> Cheers, > > > > > >> -- David > > > > > >> > > > > > >>> Changes v5->v6: > > > > > >>> - Fix alignment to maintain consistency > > > > > >>> Changes v4->v5: > > > > > >>> - Update kernel-doc comments. > > > > > >>> - Use const void* for generator return and prev value types. > > > > > >>> - Add kernel-doc comment for KUNIT_ARRAY_PARAM. > > > > > >>> - Rework parameterized test case execution strategy: each > > > > > >>> parameter is executed > > > > > >>> as if it was its own test case, with its own test > > > > > >>> initialization and cleanup > > > > > >>> (init and exit are called, etc.). However, we cannot add new > > > > > >>> test cases per TAP > > > > > >>> protocol once we have already started execution. Instead, log > > > > > >>> the result of > > > > > >>> each parameter run as a diagnostic comment. > > > > > >>> Changes v3->v4: > > > > > >>> - Rename kunit variables > > > > > >>> - Rename generator function helper macro > > > > > >>> - Add documentation for generator approach > > > > > >>> - Display test case name in case of failure along with param index > > > > > >>> Changes v2->v3: > > > > > >>> - Modifictaion of generator macro and method > > > > > >>> Changes v1->v2: > > > > > >>> - Use of a generator method to access test case parameters > > > > > >>> > > > > > >>> include/kunit/test.h | 36 ++ > > > > > >>> lib/kunit/test.c | 46 > > > > > >>> +++- > > > > > >>> 2 files changed, 69 insertions(+), 13 deletions(-) > > > > > >>> > > > > > >>> diff --git a/include/kunit/test.h b/include/kunit/test.h > > > > > >>> index db1b0ae666c4..16616d3974f9 100644 > > > > > >>> --- a/include/kunit/test.h > > > > > >>> +++ b/include/kunit/test.h > > > > > >>> @@ -107,6 +107,7 @@ struct kunit; > > > > > > [...] > > > > > >>> - kunit_suite_for_each_test_case(suite, test_case) > > > > > >>> - kunit
Re: [PATCH v6 1/2] kunit: Support for Parameterized Testing
On Wed, Nov 11, 2020 at 1:02 AM Bird, Tim wrote:
>
>
>
> > -Original Message-
> > From: David Gow
> >
> > On Mon, Nov 9, 2020 at 2:49 PM Arpitha Raghunandan <98.a...@gmail.com>
> > wrote:
> > >
> > > On 07/11/20 3:36 pm, Marco Elver wrote:
> > > > On Sat, 7 Nov 2020 at 05:58, David Gow wrote:
> > > >> On Sat, Nov 7, 2020 at 3:22 AM Arpitha Raghunandan <98.a...@gmail.com>
> > > >> wrote:
> > > >>>
> > > >>> Implementation of support for parameterized testing in KUnit.
> > > >>> This approach requires the creation of a test case using the
> > > >>> KUNIT_CASE_PARAM macro that accepts a generator function as input.
> > > >>> This generator function should return the next parameter given the
> > > >>> previous parameter in parameterized tests. It also provides
> > > >>> a macro to generate common-case generators.
> > > >>>
> > > >>> Signed-off-by: Arpitha Raghunandan <98.a...@gmail.com>
> > > >>> Co-developed-by: Marco Elver
> > > >>> Signed-off-by: Marco Elver
> > > >>> ---
> > > >>
> > > >> This looks good to me! A couple of minor thoughts about the output
> > > >> format below, but I'm quite happy to have this as-is regardless.
> > > >>
> > > >> Reviewed-by: David Gow
> > > >>
> > > >> Cheers,
> > > >> -- David
> > > >>
> > > >>> Changes v5->v6:
> > > >>> - Fix alignment to maintain consistency
> > > >>> Changes v4->v5:
> > > >>> - Update kernel-doc comments.
> > > >>> - Use const void* for generator return and prev value types.
> > > >>> - Add kernel-doc comment for KUNIT_ARRAY_PARAM.
> > > >>> - Rework parameterized test case execution strategy: each parameter
> > > >>> is executed
> > > >>> as if it was its own test case, with its own test initialization
> > > >>> and cleanup
> > > >>> (init and exit are called, etc.). However, we cannot add new test
> > > >>> cases per TAP
> > > >>> protocol once we have already started execution. Instead, log the
> > > >>> result of
> > > >>> each parameter run as a diagnostic comment.
> > > >>> Changes v3->v4:
> > > >>> - Rename kunit variables
> > > >>> - Rename generator function helper macro
> > > >>> - Add documentation for generator approach
> > > >>> - Display test case name in case of failure along with param index
> > > >>> Changes v2->v3:
> > > >>> - Modifictaion of generator macro and method
> > > >>> Changes v1->v2:
> > > >>> - Use of a generator method to access test case parameters
> > > >>>
> > > >>> include/kunit/test.h | 36 ++
> > > >>> lib/kunit/test.c | 46
> > > >>> +++-
> > > >>> 2 files changed, 69 insertions(+), 13 deletions(-)
> > > >>>
> > > >>> diff --git a/include/kunit/test.h b/include/kunit/test.h
> > > >>> index db1b0ae666c4..16616d3974f9 100644
> > > >>> --- a/include/kunit/test.h
> > > >>> +++ b/include/kunit/test.h
> > > >>> @@ -107,6 +107,7 @@ struct kunit;
> > > > [...]
> > > >>> - kunit_suite_for_each_test_case(suite, test_case)
> > > >>> - kunit_run_case_catch_errors(suite, test_case);
> > > >>> + kunit_suite_for_each_test_case(suite, test_case) {
> > > >>> + struct kunit test = { .param_value = NULL,
> > > >>> .param_index = 0 };
> > > >>> + bool test_success = true;
> > > >>> +
> > > >>> + if (test_case->generate_params)
> > > >>> + test.param_value =
> > > >>> test_case->generate_params(NULL);
> > > >>> +
> > > >>> + do {
> > > >>> + kunit_run_case_catch_errors(suite, test_case,
> > > >>> );
> > > >>> +
[PATCH] kunit: kunit_tool: Correctly parse diagnostic messages
Currently, kunit_tool expects all diagnostic lines in test results to contain ": " somewhere, as both the subtest header and the crash report do. Fix this to accept any line starting with (minus indent) "# " as being a valid diagnostic line. This matches what the TAP spec[1] and the draft KTAP spec[2] are expecting. [1]: http://testanything.org/tap-specification.html [2]: https://lore.kernel.org/linux-kselftest/cy4pr13mb1175b804e31e502221bc8163fd...@cy4pr13mb1175.namprd13.prod.outlook.com/T/ Signed-off-by: David Gow --- tools/testing/kunit/kunit_parser.py | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 84a1af2581f5..dab4cfa05b74 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -134,8 +134,8 @@ def parse_ok_not_ok_test_case(lines: List[str], test_case: TestCase) -> bool: else: return False -SUBTEST_DIAGNOSTIC = re.compile(r'^[\s]+# .*?: (.*)$') -DIAGNOSTIC_CRASH_MESSAGE = 'kunit test case crashed!' +SUBTEST_DIAGNOSTIC = re.compile(r'^[\s]+# (.*)$') +DIAGNOSTIC_CRASH_MESSAGE = re.compile(r'^[\s]+# .*?: kunit test case crashed!$') def parse_diagnostic(lines: List[str], test_case: TestCase) -> bool: save_non_diagnositic(lines, test_case) @@ -145,7 +145,8 @@ def parse_diagnostic(lines: List[str], test_case: TestCase) -> bool: match = SUBTEST_DIAGNOSTIC.match(line) if match: test_case.log.append(lines.pop(0)) - if match.group(1) == DIAGNOSTIC_CRASH_MESSAGE: + crash_match = DIAGNOSTIC_CRASH_MESSAGE.match(line) + if crash_match: test_case.status = TestStatus.TEST_CRASHED return True else: -- 2.29.2.222.g5d2a92d10f8-goog
Re: [PATCH v6 1/2] kunit: Support for Parameterized Testing
On Mon, Nov 9, 2020 at 2:49 PM Arpitha Raghunandan <98.a...@gmail.com> wrote:
>
> On 07/11/20 3:36 pm, Marco Elver wrote:
> > On Sat, 7 Nov 2020 at 05:58, David Gow wrote:
> >> On Sat, Nov 7, 2020 at 3:22 AM Arpitha Raghunandan <98.a...@gmail.com>
> >> wrote:
> >>>
> >>> Implementation of support for parameterized testing in KUnit.
> >>> This approach requires the creation of a test case using the
> >>> KUNIT_CASE_PARAM macro that accepts a generator function as input.
> >>> This generator function should return the next parameter given the
> >>> previous parameter in parameterized tests. It also provides
> >>> a macro to generate common-case generators.
> >>>
> >>> Signed-off-by: Arpitha Raghunandan <98.a...@gmail.com>
> >>> Co-developed-by: Marco Elver
> >>> Signed-off-by: Marco Elver
> >>> ---
> >>
> >> This looks good to me! A couple of minor thoughts about the output
> >> format below, but I'm quite happy to have this as-is regardless.
> >>
> >> Reviewed-by: David Gow
> >>
> >> Cheers,
> >> -- David
> >>
> >>> Changes v5->v6:
> >>> - Fix alignment to maintain consistency
> >>> Changes v4->v5:
> >>> - Update kernel-doc comments.
> >>> - Use const void* for generator return and prev value types.
> >>> - Add kernel-doc comment for KUNIT_ARRAY_PARAM.
> >>> - Rework parameterized test case execution strategy: each parameter is
> >>> executed
> >>> as if it was its own test case, with its own test initialization and
> >>> cleanup
> >>> (init and exit are called, etc.). However, we cannot add new test cases
> >>> per TAP
> >>> protocol once we have already started execution. Instead, log the
> >>> result of
> >>> each parameter run as a diagnostic comment.
> >>> Changes v3->v4:
> >>> - Rename kunit variables
> >>> - Rename generator function helper macro
> >>> - Add documentation for generator approach
> >>> - Display test case name in case of failure along with param index
> >>> Changes v2->v3:
> >>> - Modifictaion of generator macro and method
> >>> Changes v1->v2:
> >>> - Use of a generator method to access test case parameters
> >>>
> >>> include/kunit/test.h | 36 ++
> >>> lib/kunit/test.c | 46 +++-
> >>> 2 files changed, 69 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/include/kunit/test.h b/include/kunit/test.h
> >>> index db1b0ae666c4..16616d3974f9 100644
> >>> --- a/include/kunit/test.h
> >>> +++ b/include/kunit/test.h
> >>> @@ -107,6 +107,7 @@ struct kunit;
> > [...]
> >>> - kunit_suite_for_each_test_case(suite, test_case)
> >>> - kunit_run_case_catch_errors(suite, test_case);
> >>> + kunit_suite_for_each_test_case(suite, test_case) {
> >>> + struct kunit test = { .param_value = NULL, .param_index =
> >>> 0 };
> >>> + bool test_success = true;
> >>> +
> >>> + if (test_case->generate_params)
> >>> + test.param_value =
> >>> test_case->generate_params(NULL);
> >>> +
> >>> + do {
> >>> + kunit_run_case_catch_errors(suite, test_case,
> >>> );
> >>> + test_success &= test_case->success;
> >>> +
> >>> + if (test_case->generate_params) {
> >>> + kunit_log(KERN_INFO, ,
> >>> + KUNIT_SUBTEST_INDENT
> >>> + "# %s: param-%d %s",
> >>
> >> Would it make sense to have this imitate the TAP format a bit more?
> >> So, have "# [ok|not ok] - [name]" as the format? [name] could be
> >> something like "[test_case->name]:param-[index]" or similar.
> >> If we keep it commented out and don't indent it further, it won't
> >> formally be a nested test (though if we wanted to support those later,
> >> it'd be easy to add), but I think it would be nicer to be consistent
> >> here.
> >
>
Re: [PATCH v6 2/2] fs: ext4: Modify inode-test.c to use KUnit parameterized testing feature
On Sat, Nov 7, 2020 at 3:23 AM Arpitha Raghunandan <98.a...@gmail.com> wrote: > > Modify fs/ext4/inode-test.c to use the parameterized testing > feature of KUnit. > > Signed-off-by: Arpitha Raghunandan <98.a...@gmail.com> > --- This looks good to me. Thanks! Reviewed-by: David Gow -- David
Re: [PATCH v6 1/2] kunit: Support for Parameterized Testing
On Sat, Nov 7, 2020 at 3:22 AM Arpitha Raghunandan <98.a...@gmail.com> wrote:
>
> Implementation of support for parameterized testing in KUnit.
> This approach requires the creation of a test case using the
> KUNIT_CASE_PARAM macro that accepts a generator function as input.
> This generator function should return the next parameter given the
> previous parameter in parameterized tests. It also provides
> a macro to generate common-case generators.
>
> Signed-off-by: Arpitha Raghunandan <98.a...@gmail.com>
> Co-developed-by: Marco Elver
> Signed-off-by: Marco Elver
> ---
This looks good to me! A couple of minor thoughts about the output
format below, but I'm quite happy to have this as-is regardless.
Reviewed-by: David Gow
Cheers,
-- David
> Changes v5->v6:
> - Fix alignment to maintain consistency
> Changes v4->v5:
> - Update kernel-doc comments.
> - Use const void* for generator return and prev value types.
> - Add kernel-doc comment for KUNIT_ARRAY_PARAM.
> - Rework parameterized test case execution strategy: each parameter is
> executed
> as if it was its own test case, with its own test initialization and cleanup
> (init and exit are called, etc.). However, we cannot add new test cases per
> TAP
> protocol once we have already started execution. Instead, log the result of
> each parameter run as a diagnostic comment.
> Changes v3->v4:
> - Rename kunit variables
> - Rename generator function helper macro
> - Add documentation for generator approach
> - Display test case name in case of failure along with param index
> Changes v2->v3:
> - Modifictaion of generator macro and method
> Changes v1->v2:
> - Use of a generator method to access test case parameters
>
> include/kunit/test.h | 36 ++
> lib/kunit/test.c | 46 +++-
> 2 files changed, 69 insertions(+), 13 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index db1b0ae666c4..16616d3974f9 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -107,6 +107,7 @@ struct kunit;
> *
> * @run_case: the function representing the actual test case.
> * @name: the name of the test case.
> + * @generate_params: the generator function for parameterized tests.
> *
> * A test case is a function with the signature,
> * ``void (*)(struct kunit *)``
> @@ -141,6 +142,7 @@ struct kunit;
> struct kunit_case {
> void (*run_case)(struct kunit *test);
> const char *name;
> + const void* (*generate_params)(const void *prev);
>
> /* private: internal use only. */
> bool success;
> @@ -163,6 +165,22 @@ static inline char *kunit_status_to_string(bool status)
> */
> #define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
>
> +/**
> + * KUNIT_CASE_PARAM - A helper for creation a parameterized
> kunit_case
> + *
> + * @test_name: a reference to a test case function.
> + * @gen_params: a reference to a parameter generator function.
> + *
> + * The generator function ``const void* gen_params(const void *prev)`` is
> used
> + * to lazily generate a series of arbitrarily typed values that fit into a
> + * void*. The argument @prev is the previously returned value, which should
> be
> + * used to derive the next value; @prev is set to NULL on the initial
> generator
> + * call. When no more values are available, the generator must return NULL.
> + */
> +#define KUNIT_CASE_PARAM(test_name, gen_params)\
> + { .run_case = test_name, .name = #test_name,\
> + .generate_params = gen_params }
> +
> /**
> * struct kunit_suite - describes a related collection of kunit_case
> *
> @@ -208,6 +226,10 @@ struct kunit {
> const char *name; /* Read only after initialization! */
> char *log; /* Points at case log after initialization */
> struct kunit_try_catch try_catch;
> + /* param_value is the current parameter value for a test case. */
> + const void *param_value;
> + /* param_index stores the index of the parameter in parameterized
> tests. */
> + int param_index;
> /*
> * success starts as true, and may only be set to false during a
> * test case; thus, it is safe to update this across multiple
> @@ -1742,4 +1764,18 @@ do {
> \
> fmt,
> \
> #
Re: [PATCH] Documentation: kunit: provide guidance for testing many inputs
On Tue, Nov 3, 2020 at 5:37 AM Daniel Latypov wrote:
>
> usage.rst goes into a detailed about faking out classes, but currently
Nit: a detailed what?
> lacks wording about how one might idiomatically test a range of inputs.
>
> Give an example of how one might test a hash function via macros/helper
> funcs and a table-driven test and very briefly discuss pros and cons.
>
> Also highlight the KUNIT_EXPECT_*_MSG() variants (that aren't mentioned
> elsewhere [1]) which are particularly useful in these situations.
>
> It is also criminally underused at the moment, only appearing in 2
> tests (both written by people involved in KUnit).
>
> [1] not even on
> https://www.kernel.org/doc/html/latest/dev-tools/kunit/api/test.html
I suspect we'll eventually want to document the _MSG variants here as
well, though it will bloat the page somewhat. In any case, it can be
left to a separate patch.
>
> Signed-off-by: Daniel Latypov
> ---
Thanks for writing this -- it's definitely a common test pattern which
it'd be nice to encourage and explain a bit better.
Cheers,
-- David
> Documentation/dev-tools/kunit/usage.rst | 66 +
> 1 file changed, 66 insertions(+)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst
> b/Documentation/dev-tools/kunit/usage.rst
> index 62142a47488c..317390df2b96 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -451,6 +451,72 @@ We can now use it to test ``struct eeprom_buffer``:
> destroy_eeprom_buffer(ctx->eeprom_buffer);
> }
>
> +Testing various inputs
> +--
Nit: "various" isn't hugely descriptive here. Maybe something like
"Testing against multiple inputs" would be better?
> +
> +Testing just a few inputs might not be enough to have confidence that the
> code
> +works correctly, e.g. for a hash function.
> +
> +In such cases, it can be helpful to have a helper macro or function, e.g.
> this
> +fictitious example for ``md5sum(1)``
> +
> +.. code-block:: c
> +
> + /* Note: the cast is to satisfy overly strict type-checking. */
> + #define TEST_MD5(in, want) \
> + md5sum(in, out); \
> + KUNIT_EXPECT_STREQ_MSG(test, (char *)out, want, "md5sum(%s)",
> in);
> +
> + char out[16];
> + TEST_MD5("hello world", "5eb63bbbe01eeed093cb22bb8f5acdc3");
> + TEST_MD5("hello world!", "fc3ff98e8c6a0d3087d515c0473f8677");
> +
> +Note the use of ``KUNIT_EXPECT_STREQ_MSG`` to give more context when it fails
> +and make it easier to track down. (Yes, in this example, ``want`` is likely
> +going to be unique enough on its own).
> +
> +The ``_MSG`` variants are even more useful when the same expectation is
> called
> +multiple times (in a loop or helper function) and thus the line number isn't
> +enough to identify what failed, like below.
> +
> +In some cases, it can be helpful to write a *table-driven test* instead, e.g.
> +
> +.. code-block:: c
> +
> + int i;
> + char out[16];
> +
> + struct md5_test_case {
> + const char *str;
> + const char *md5;
> + };
> +
> + struct md5_test_case cases[] = {
> + {
> + .str = "hello world",
> + .md5 = "5eb63bbbe01eeed093cb22bb8f5acdc3",
> + },
> + {
> + .str = "hello world!",
> + .md5 = "fc3ff98e8c6a0d3087d515c0473f8677",
> + },
> + };
> + for (i = 0; i < ARRAY_SIZE(cases); ++i) {
> + md5sum(cases[i].str, out);
> + KUNIT_EXPECT_STREQ_MSG(test, (char *)out, cases[i].md5,
> + "md5sum(%s)", cases[i].str);
> + }
> +
> +
> +There's more boilerplate involved, but it can:
> +
> +* be more readable when there are multiple inputs/outputs thanks to field
> names,
> +
> + * E.g. see ``fs/ext4/inode-test.c`` for an example of both.
> +* reduce duplication if test cases can be shared across multiple tests.
> +
> + * E.g. if we had a magical ``undo_md5sum`` function, we could reuse
> ``cases``.
> +
This is a bit of a nitpick, but I don't think this is quite conveying
the usefulness of table-based testing. Maybe it's that a hypothetical
"undo_md5sum" is too unrealistic an example? Maybe, instead of having
both the macro-based and table-driven examples based around md5sum(),
the table-based one could use something more obviously invertible /
reusable, and include both in the example code. E.g, something akin to
toupper() and tolower() or some other conversion function. I think
having a better example here is probably more useful than having both
the table- and macro- driven examples test the same thing.
> .. _kunit-on-non-uml:
>
> KUnit on non-UML architectures
>
> base-commit: 77c8473edf7f7664137f555cfcdc8c460bbd947d
> --
> 2.29.1.341.ge80a0c044ae-goog
>
Re: [PATCH v1] kunit: tool: unmark test_data as binary blobs
On Fri, Nov 6, 2020 at 7:24 AM Brendan Higgins
wrote:
>
> The tools/testing/kunit/test_data/ directory was marked as binary
> because some of the test_data files cause checkpatch warnings. Fix this
> by dropping the .gitattributes file.
>
> Fixes: afc63da64f1e ("kunit: kunit_parser: make parser more robust")
> Signed-off-by: Brendan Higgins
> ---
Reviewed-by: David Gow
Thanks. I wasn't able to find any issues which required those files to
be binary.
For the record, a link to the original issue, which appeared to be
with whitespace (spaces before tabs) in git apply:
https://lkml.org/lkml/2020/3/13/920
Cheers,
-- David
Re: [PATCH] kunit: tool: fix extra trailing \n in raw + parsed test output
On Sat, Oct 31, 2020 at 6:39 AM Daniel Latypov wrote: > > For simplcity, strip all trailing whitespace from parsed output. > I imagine no one is printing out meaningful trailing whitespace via > KUNIT_FAIL() or similar, and that if they are, they really shouldn't. > > `isolate_kunit_output()` yielded liens with trailing \n, which results > in artifacty output like this: > > $ ./tools/testing/kunit/kunit.py run > [16:16:46] [FAILED] example_simple_test > [16:16:46] # example_simple_test: EXPECTATION FAILED at > lib/kunit/kunit-example-test.c:29 > > [16:16:46] Expected 1 + 1 == 3, but > > [16:16:46] 1 + 1 == 2 > > [16:16:46] 3 == 3 > > [16:16:46] not ok 1 - example_simple_test > > [16:16:46] > > After this change: > [16:16:46] # example_simple_test: EXPECTATION FAILED at > lib/kunit/kunit-example-test.c:29 > [16:16:46] Expected 1 + 1 == 3, but > [16:16:46] 1 + 1 == 2 > [16:16:46] 3 == 3 > [16:16:46] not ok 1 - example_simple_test > [16:16:46] > > We should *not* be expecting lines to end with \n in kunit_tool_test.py > for this reason. > > Do the same for `raw_output()` as well which suffers from the same > issue. > > This is a followup to [1], but rebased onto kunit-fixes to pick up the > other raw_output() fix and fixes for kunit_tool_test.py. > > [1] > https://lore.kernel.org/linux-kselftest/20201020233219.4146059-1-dlaty...@google.com/ > > Signed-off-by: Daniel Latypov > --- Thanks! I tried this out against everything I could (including the nastier --alltests option), and didn't hit any problems, so it looks good to go to me! Reviewed-by: David Gow Tested-by: David Gow Cheers, -- David
Re: [PATCH] kunit: tool: fix extra trailing \n in parsed test output
On Fri, Oct 30, 2020 at 1:41 PM Daniel Latypov wrote:
>
> On Thu, Oct 29, 2020 at 7:34 PM David Gow wrote:
> >
> > On Wed, Oct 21, 2020 at 7:32 AM Daniel Latypov wrote:
> > >
> > > For simplcity, strip all trailing whitespace from parsed output.
> > > I imagine no one is printing out meaningful trailing whitespace via
> > > KUNIT_FAIL() or similar, and that if they are, they really shouldn't.
> > >
> > > At some point, the lines from `isolate_kunit_output()` started having
> > > trailing \n, which results in artifacty output like this:
> > >
> > > $ ./tools/testing/kunit/kunit.py run
> > > [16:16:46] [FAILED] example_simple_test
> > > [16:16:46] # example_simple_test: EXPECTATION FAILED at
> > > lib/kunit/kunit-example-test.c:29
> > >
> > > [16:16:46] Expected 1 + 1 == 3, but
> > >
> > > [16:16:46] 1 + 1 == 2
> > >
> > > [16:16:46] 3 == 3
> > >
> > > [16:16:46] not ok 1 - example_simple_test
> > >
> > > [16:16:46]
> > >
> > > After this change:
> > > [16:16:46] # example_simple_test: EXPECTATION FAILED at
> > > lib/kunit/kunit-example-test.c:29
> > > [16:16:46] Expected 1 + 1 == 3, but
> > > [16:16:46] 1 + 1 == 2
> > > [16:16:46] 3 == 3
> > > [16:16:46] not ok 1 - example_simple_test
> > > [16:16:46]
> > >
> > > Signed-off-by: Daniel Latypov
> > > ---
> >
> > Thanks! This is a long-overdue fix, and it worked well for me.
> >
> > Tested-by: David Gow
> >
> > One comment below:
> >
> > > tools/testing/kunit/kunit_parser.py | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/testing/kunit/kunit_parser.py
> > > b/tools/testing/kunit/kunit_parser.py
> > > index 8019e3dd4c32..e68b1c66a73f 100644
> > > --- a/tools/testing/kunit/kunit_parser.py
> > > +++ b/tools/testing/kunit/kunit_parser.py
> > > @@ -342,7 +342,8 @@ def parse_run_tests(kernel_output) -> TestResult:
> > > total_tests = 0
> > > failed_tests = 0
> > > crashed_tests = 0
> > > - test_result =
> > > parse_test_result(list(isolate_kunit_output(kernel_output)))
> > > + test_result = parse_test_result(list(
> > > +l.rstrip() for l in isolate_kunit_output(kernel_output)))
> >
> > Could we do this inside isolate_kunit_output() instead? That seems
> > like it'd be a more logical place for it (removing the newline is a
> > sort of isolating the output), and it'd avoid making this line quite
> > as horrifyingly nested.
>
> Good point.
> We could either do it on each yield (messy), or before, i.e.
>
> diff --git a/tools/testing/kunit/kunit_parser.py
> b/tools/testing/kunit/kunit_parser.py
> index 8019e3dd4c32..14d35deb96cd 100644
> --- a/tools/testing/kunit/kunit_parser.py
> +++ b/tools/testing/kunit/kunit_parser.py
> @@ -54,6 +54,7 @@ kunit_end_re = re.compile('(List of all partitions:|'
> def isolate_kunit_output(kernel_output):
> started = False
> for line in kernel_output:
> + line = line.rstrip() # line always has a trailing \n
> if kunit_start_re.search(line):
> prefix_len = len(line.split('TAP version')[0])
> started = True
>
> I had some vague concerns about this as
> kunit_start_re = re.compile(r'TAP version [0-9]+$')
> has that anchor at the end.
>
> This could ostensibly make it match more things than before.
> Since I'm using rstrip() out of laziness, that means strings like
> 'TAP version 42\t\n'
> will now also match.
>
> I don't really think that's an issue, but I'd sent this as a more
> conservative change initially.
> I can send the diff above as a replacement for this patch.
I prefer this if it works. From my cursory testing, it does (though
the kunt_tool_tests.py tests will need updating). At the very least,
I'm able to get it to work with --alltests / allyesconfig (with a few
options tactically disabled), which was the main reason we needed
isolate_kunit_output() in the first place.
So, unless anyone can find a real-world case where this breaks
something, let's go with this.
Cheers,
-- David
>
> >
> > > if test_result.status == TestStatus.NO_TESTS:
> > > print(red('[ERROR] ') + yellow('no tests run!'))
> > > elif test_result.status == TestStatus.FAILURE_TO_PARSE_TESTS:
> > >
> > > base-commit: c4d6fe7311762f2e03b3c27ad38df7c40c80cc93
> > > --
> > > 2.29.0.rc1.297.gfa9743e501-goog
> > >
Re: [PATCH] kunit: tool: fix --raw_output to actually show output
On Fri, Oct 30, 2020 at 2:17 PM Daniel Latypov wrote: > > Currently --raw_output means nothing gets shown. > Why? > Because `raw_output()` has a `yield` and therefore is a generator, which > means it only executes when you ask it for a value. > > Given no one actually is using it as a generator (checked via the added > type annotation), drop the yield so we actually print the output. > > Also strip off the trailing \n (and any other whitespace) to avoid > [<601d6d3a>] ? printk+0x0/0x9b > > [<601e5058>] ? kernel_init+0x23/0x14b > > [<600170d2>] ? new_thread_handler+0x82/0xc0 > making the output unreadable. > > Signed-off-by: Daniel Latypov > --- The bug where --raw_output doesn't show anything is already fixed[1], but it does still show the extra newlines. Maybe it's worth making just the newline fix, and rolling it into the other patch[2] handling newlines? Cheers, -- David [1]: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes=3023d8ff3fc60e5d32dc1d05f99ad6ffa12b0033 [2]: https://lore.kernel.org/linux-kselftest/20201020233219.4146059-1-dlaty...@google.com/
Re: [PATCH] kunit: tool: fix pre-existing python type annotation errors
On Thu, Oct 22, 2020 at 6:08 AM Daniel Latypov wrote: > > The code uses annotations, but they aren't accurate. > Note that type checking in python is a separate process, running > `kunit.py run` will not check and complain about invalid types at > runtime. > > Fix pre-existing issues found by running a type checker > $ mypy *.py > > All but one of these were returning `None` without denoting this > properly (via `Optional[Type]`). > > Signed-off-by: Daniel Latypov > --- I'm not going to pretend to really understand python annotations completely, but this all seems correct from what I know of the code, and I was able to install mypy and verify the issues were fixed. Clearly, if we're going to have type annotations here, we should be verifying the code against them. Is there a way we could get python itself to verify this code when the script runs, rather than have to use mypy as a tool to verify it separately? Otherwise, maybe we can run it automatically from the kunit_tool_test.py unit tests or something similar? Regardless, this is Reviewed-by: David Gow Cheers, -- David
Re: [PATCH] kunit: tool: print out stderr from make (like build warnings)
On Fri, Oct 30, 2020 at 6:10 AM Daniel Latypov wrote: > > Currently the tool redirects make stdout + stderr, and only shows them > if the make command fails. > This means build warnings aren't shown to the user. > > This change prints the contents of stderr even if make succeeds, under > the assumption these are only build warnings or other messages the user > likely wants to see. > > We drop stdout from the raised exception since we can no longer easily > collate stdout and stderr and just showing the stderr seems fine. > > Example with a warning: > > [14:56:35] Building KUnit Kernel ... > ../lib/kunit/kunit-test.c: In function ‘kunit_test_successful_try’: > ../lib/kunit/kunit-test.c:19:6: warning: unused variable ‘unused’ > [-Wunused-variable] >19 | int unused; > | ^~ > > [14:56:40] Starting KUnit Kernel ... > > Note the stderr has a trailing \n, and since we use print, we add > another, but it helps separate make and kunit.py output. > > Example with a build error: > > [15:02:45] Building KUnit Kernel ... > ERROR:root:../lib/kunit/kunit-test.c: In function ‘kunit_test_successful_try’: > ../lib/kunit/kunit-test.c:19:2: error: unknown type name ‘invalid_type’ >19 | invalid_type *test = data; > | ^~~~ > ... > > Signed-off-by: Daniel Latypov > --- Thanks a lot -- this was really bugging me, and works great. Reviewed-by: David Gow
Re: [PATCH] kunit: tool: fix extra trailing \n in parsed test output
On Wed, Oct 21, 2020 at 7:32 AM Daniel Latypov wrote:
>
> For simplcity, strip all trailing whitespace from parsed output.
> I imagine no one is printing out meaningful trailing whitespace via
> KUNIT_FAIL() or similar, and that if they are, they really shouldn't.
>
> At some point, the lines from `isolate_kunit_output()` started having
> trailing \n, which results in artifacty output like this:
>
> $ ./tools/testing/kunit/kunit.py run
> [16:16:46] [FAILED] example_simple_test
> [16:16:46] # example_simple_test: EXPECTATION FAILED at
> lib/kunit/kunit-example-test.c:29
>
> [16:16:46] Expected 1 + 1 == 3, but
>
> [16:16:46] 1 + 1 == 2
>
> [16:16:46] 3 == 3
>
> [16:16:46] not ok 1 - example_simple_test
>
> [16:16:46]
>
> After this change:
> [16:16:46] # example_simple_test: EXPECTATION FAILED at
> lib/kunit/kunit-example-test.c:29
> [16:16:46] Expected 1 + 1 == 3, but
> [16:16:46] 1 + 1 == 2
> [16:16:46] 3 == 3
> [16:16:46] not ok 1 - example_simple_test
> [16:16:46]
>
> Signed-off-by: Daniel Latypov
> ---
Thanks! This is a long-overdue fix, and it worked well for me.
Tested-by: David Gow
One comment below:
> tools/testing/kunit/kunit_parser.py | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/kunit/kunit_parser.py
> b/tools/testing/kunit/kunit_parser.py
> index 8019e3dd4c32..e68b1c66a73f 100644
> --- a/tools/testing/kunit/kunit_parser.py
> +++ b/tools/testing/kunit/kunit_parser.py
> @@ -342,7 +342,8 @@ def parse_run_tests(kernel_output) -> TestResult:
> total_tests = 0
> failed_tests = 0
> crashed_tests = 0
> - test_result =
> parse_test_result(list(isolate_kunit_output(kernel_output)))
> + test_result = parse_test_result(list(
> +l.rstrip() for l in isolate_kunit_output(kernel_output)))
Could we do this inside isolate_kunit_output() instead? That seems
like it'd be a more logical place for it (removing the newline is a
sort of isolating the output), and it'd avoid making this line quite
as horrifyingly nested.
> if test_result.status == TestStatus.NO_TESTS:
> print(red('[ERROR] ') + yellow('no tests run!'))
> elif test_result.status == TestStatus.FAILURE_TO_PARSE_TESTS:
>
> base-commit: c4d6fe7311762f2e03b3c27ad38df7c40c80cc93
> --
> 2.29.0.rc1.297.gfa9743e501-goog
>
Re: [PATCH v2] KUnit: Docs: style: fix some Kconfig example issues
On Thu, Oct 29, 2020 at 1:42 AM Randy Dunlap wrote:
>
> Fix the Kconfig example to be closer to Kconfig coding style.
>
> Also add punctuation and a trailing slash ('/') to a sub-directory
> name -- this is how the text mostly appears in other Kconfig files.
>
> Signed-off-by: Randy Dunlap
> Cc: David Gow
> Cc: linux-kselft...@vger.kernel.org
> Cc: kunit-...@googlegroups.com
> Cc: Shuah Khan
> Cc: Shuah Khan
> Cc: Brendan Higgins
> Reviewed-by: David Gow
> ---
> v2: covert spaces indentation to tabs in Kconfig example
>
Thanks! Sphinx is still converting the tabs to spaces for its HTML
output, but this is nevertheless an improvement.
Cheers,
-- David
Re: [PATCH] KUnit: Docs: usage: wording fixes
On Thu, Oct 29, 2020 at 1:43 AM Randy Dunlap wrote: > > Fix minor grammar and punctutation glitches. > Hyphenate "architecture-specific" instances. > > Signed-off-by: Randy Dunlap > Cc: David Gow > Cc: linux-kselft...@vger.kernel.org > Cc: kunit-...@googlegroups.com > Cc: Shuah Khan > Cc: Shuah Khan > Cc: Brendan Higgins > --- These all look sensible: thanks for catching them! Reviewed-by: David Gow Cheers, -- David
Re: [PATCH] KUnit: Docs: style: fix some Kconfig example issues
On Wed, Oct 28, 2020 at 1:32 PM Randy Dunlap wrote:
>
> On 10/27/20 8:00 PM, David Gow wrote:
> > On Wed, Oct 28, 2020 at 2:49 AM Randy Dunlap wrote:
> >>
> >> Fix the Kconfig example to be closer to Kconfig coding style.
> >> (Except that it still uses spaces instead of tabs for indentation;
> >> I guess that Sphinx wants it that way.)
> >>
> >> Also add punctuation and a trailing slash ('/') to a sub-directory
> >> name -- this is how the text mostly appears in other Kconfig files.
> >>
> >> Signed-off-by: Randy Dunlap
> >> Cc: David Gow
> >> Cc: linux-kselft...@vger.kernel.org
> >> Cc: kunit-...@googlegroups.com
> >> Cc: Shuah Khan
> >> Cc: Shuah Khan
> >> Cc: Brendan Higgins
> >> ---
> >
> > Thanks for fixing this!
> >
> > For what it's worth, I _think_ we could get away with tabs for
> > indentation in the file without Sphinx actually complaining, but it
> > does annoy some of the editors, and as far as I can tell, Sphinx
> > converts them back to spaces in its output. I'm far from an expert,
> > though...
> >
> > Regardless, this is:
> >
> > Reviewed-by: David Gow
>
> I tested with tabs for indentation and it's no problem with Sphinx.
> Some editors care? I am surprised. and don't much care.
Neat!
As for editors, I think it was just some aggressively set per-filetype
defaults, so I'm not particularly concerned either.
> I would be happy to submit a v2 using tabs for indentation.
That sounds good to me if it works, thanks!
Cheers,
-- David
Re: [PATCH] KUnit: Docs: fix a wording typo
On Wed, Oct 28, 2020 at 2:49 AM Randy Dunlap wrote: > > Fix a wording typo (keyboard glitch). > > Signed-off-by: Randy Dunlap > Cc: David Gow > Cc: linux-kselft...@vger.kernel.org > Cc: kunit-...@googlegroups.com > Cc: Shuah Khan > Cc: Shuah Khan > Cc: Brendan Higgins > --- Whoops! Thanks for catching this! Reviewed-by: David Gow Cheers, -- David
[PATCH v7] fat: Add KUnit tests for checksums and timestamps
Add some basic sanity-check tests for the fat_checksum() function and
the fat_time_unix2fat() and fat_time_fat2unix() functions. These unit
tests verify these functions return correct output for a number of test
inputs.
These tests were inspored by -- and serve a similar purpose to -- the
timestamp parsing KUnit tests in ext4[1].
Note that, unlike fat_time_unix2fat, fat_time_fat2unix wasn't previously
exported, so this patch exports it as well. This is required for the
case where we're building the fat and fat_test as modules.
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/inode-test.c
Signed-off-by: David Gow
Acked-by: OGAWA Hirofumi
---
Changes since v6:
https://lore.kernel.org/linux-kselftest/20201024060558.2556249-1-david...@google.com/
- Make CONFIG_FAT_DEFAULT_CODEPAGE depend on FAT_FS, rather than either
VFAT_FS or MSDOS_FS.
- This means that FAT_KUNIT_TEST can now also just depend of FAT_FS
- Fix a few warnings that KUnit tool was eating:
- KUnit's type checking needs a specific cast for the fat_checksum()
expected results.
- The time test cases shouldn't be 'const'
- The fake superblock is now static, as otherwise it increased the
stack size too much.
Changes since v4/5:
https://lore.kernel.org/linux-kselftest/20201024052047.2526780-1-david...@google.com/
- Fix a typo introduced in the Kconfig. It builds now.
Changes since v3:
https://lore.kernel.org/linux-kselftest/20201021061713.1545931-1-david...@google.com/
- Update the Kconfig entry to use "depends on" rather than "select", as
discussed in [2].
- Depend on "MSDOS_FS || VFAT_FS", rather than "FAT_FS", as we need the
CONFIG_FAT_DEFAULT_CODEPAGE symbol to be defined.
Changes since v2:
https://lore.kernel.org/linux-kselftest/20201020055856.1270482-1-david...@google.com/
- Comment that the export for fat_time_fat2unix() function is for KUnit
tests.
Changes since v1:
https://lore.kernel.org/linux-kselftest/20201017064107.375174-1-david...@google.com/
- Now export fat_time_fat2unix() so that the test can access it when
built as a module.
[2]:
https://lore.kernel.org/linux-ext4/52959e99-4105-3de9-730c-c46894b82...@infradead.org/T/#t
fs/fat/Kconfig| 14 +++-
fs/fat/Makefile | 2 +
fs/fat/fat_test.c | 196 ++
fs/fat/misc.c | 2 +
4 files changed, 213 insertions(+), 1 deletion(-)
create mode 100644 fs/fat/fat_test.c
diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index 66532a71e8fd..238cc55f84c4 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
@@ -77,7 +77,7 @@ config VFAT_FS
config FAT_DEFAULT_CODEPAGE
int "Default codepage for FAT"
- depends on MSDOS_FS || VFAT_FS
+ depends on FAT_FS
default 437
help
This option should be set to the codepage of your FAT filesystems.
@@ -115,3 +115,15 @@ config FAT_DEFAULT_UTF8
Say Y if you use UTF-8 encoding for file names, N otherwise.
See for more information.
+
+config FAT_KUNIT_TEST
+ tristate "Unit Tests for FAT filesystems" if !KUNIT_ALL_TESTS
+ depends on KUNIT && FAT_FS
+ default KUNIT_ALL_TESTS
+ help
+ This builds the FAT KUnit tests
+
+ For more information on KUnit and unit tests in general, please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit
+
+ If unsure, say N
diff --git a/fs/fat/Makefile b/fs/fat/Makefile
index 70645ce2f7fc..2b034112690d 100644
--- a/fs/fat/Makefile
+++ b/fs/fat/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_MSDOS_FS) += msdos.o
fat-y := cache.o dir.o fatent.o file.o inode.o misc.o nfs.o
vfat-y := namei_vfat.o
msdos-y := namei_msdos.o
+
+obj-$(CONFIG_FAT_KUNIT_TEST) += fat_test.o
diff --git a/fs/fat/fat_test.c b/fs/fat/fat_test.c
new file mode 100644
index ..7d3fe928fbe6
--- /dev/null
+++ b/fs/fat/fat_test.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for FAT filesystems.
+ *
+ * Copyright (C) 2020 Google LLC.
+ * Author: David Gow
+ */
+
+#include
+
+#include "fat.h"
+
+static void fat_checksum_test(struct kunit *test)
+{
+ /* With no extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("VMLINUX"), (u8)44);
+ /* With 3-letter extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("README TXT"), (u8)115);
+ /* With short (1-letter) extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("ABCDEFGHA "), (u8)98);
+}
+
+
+struct fat_timestamp_testcase {
+ const char *name;
+ struct timespec64 ts;
+ __le16 time;
+ __le16 date;
+ u8 cs;
+ int time_offset;
+};
+
+static struct fat_timestamp_testcase time_test_cases[] = {
+ {
+ .name = "Earliest possible UTC (1980-01-01 00:00:00)",
+ .ts = {.tv_sec = 315532800LL, .tv_nsec =
Re: [PATCH] KUnit: Docs: style: fix some Kconfig example issues
On Wed, Oct 28, 2020 at 2:49 AM Randy Dunlap wrote:
>
> Fix the Kconfig example to be closer to Kconfig coding style.
> (Except that it still uses spaces instead of tabs for indentation;
> I guess that Sphinx wants it that way.)
>
> Also add punctuation and a trailing slash ('/') to a sub-directory
> name -- this is how the text mostly appears in other Kconfig files.
>
> Signed-off-by: Randy Dunlap
> Cc: David Gow
> Cc: linux-kselft...@vger.kernel.org
> Cc: kunit-...@googlegroups.com
> Cc: Shuah Khan
> Cc: Shuah Khan
> Cc: Brendan Higgins
> ---
Thanks for fixing this!
For what it's worth, I _think_ we could get away with tabs for
indentation in the file without Sphinx actually complaining, but it
does annoy some of the editors, and as far as I can tell, Sphinx
converts them back to spaces in its output. I'm far from an expert,
though...
Regardless, this is:
Reviewed-by: David Gow
-- David
[PATCH v6] fat: Add KUnit tests for checksums and timestamps
Add some basic sanity-check tests for the fat_checksum() function and
the fat_time_unix2fat() and fat_time_fat2unix() functions. These unit
tests verify these functions return correct output for a number of test
inputs.
These tests were inspored by -- and serve a similar purpose to -- the
timestamp parsing KUnit tests in ext4[1].
Note that, unlike fat_time_unix2fat, fat_time_fat2unix wasn't previously
exported, so this patch exports it as well. This is required for the
case where we're building the fat and fat_test as modules.
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/inode-test.c
Signed-off-by: David Gow
Acked-by: OGAWA Hirofumi
---
I am _so_ sorry: I sent out the wrong version again! It's clearly not my
day! This version actually has the fix!
-- David
Changes since v4/v5:
https://lore.kernel.org/linux-kselftest/20201024052047.2526780-1-david...@google.com/
- Fix a typo introduced in the Kconfig. It builds now.
Changes since v3:
https://lore.kernel.org/linux-kselftest/20201021061713.1545931-1-david...@google.com/
- Update the Kconfig entry to use "depends on" rather than "select", as
discussed in [2].
- Depend on "MSDOS_FS || VFAT_FS", rather than "FAT_FS", as we need the
CONFIG_FAT_DEFAULT_CODEPAGE symbol to be defined.
Changes since v2:
https://lore.kernel.org/linux-kselftest/20201020055856.1270482-1-david...@google.com/
- Comment that the export for fat_time_fat2unix() function is for KUnit
tests.
Changes since v1:
https://lore.kernel.org/linux-kselftest/20201017064107.375174-1-david...@google.com/
- Now export fat_time_fat2unix() so that the test can access it when
built as a module.
[2]:
https://lore.kernel.org/linux-ext4/52959e99-4105-3de9-730c-c46894b82...@infradead.org/T/#t
fs/fat/Kconfig| 12 +++
fs/fat/Makefile | 2 +
fs/fat/fat_test.c | 196 ++
fs/fat/misc.c | 2 +
4 files changed, 212 insertions(+)
create mode 100644 fs/fat/fat_test.c
diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index 66532a71e8fd..4e66f7e8defc 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
@@ -115,3 +115,15 @@ config FAT_DEFAULT_UTF8
Say Y if you use UTF-8 encoding for file names, N otherwise.
See for more information.
+
+config FAT_KUNIT_TEST
+ tristate "Unit Tests for FAT filesystems" if !KUNIT_ALL_TESTS
+ depends on KUNIT && (MSDOS_FS || VFAT_FS)
+ default KUNIT_ALL_TESTS
+ help
+ This builds the FAT KUnit tests
+
+ For more information on KUnit and unit tests in general, please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit
+
+ If unsure, say N
diff --git a/fs/fat/Makefile b/fs/fat/Makefile
index 70645ce2f7fc..2b034112690d 100644
--- a/fs/fat/Makefile
+++ b/fs/fat/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_MSDOS_FS) += msdos.o
fat-y := cache.o dir.o fatent.o file.o inode.o misc.o nfs.o
vfat-y := namei_vfat.o
msdos-y := namei_msdos.o
+
+obj-$(CONFIG_FAT_KUNIT_TEST) += fat_test.o
diff --git a/fs/fat/fat_test.c b/fs/fat/fat_test.c
new file mode 100644
index ..c99bfbdf89bb
--- /dev/null
+++ b/fs/fat/fat_test.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for FAT filesystems.
+ *
+ * Copyright (C) 2020 Google LLC.
+ * Author: David Gow
+ */
+
+#include
+
+#include "fat.h"
+
+static void fat_checksum_test(struct kunit *test)
+{
+ /* With no extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("VMLINUX"), 44);
+ /* With 3-letter extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("README TXT"), 115);
+ /* With short (1-letter) extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("ABCDEFGHA "), 98);
+}
+
+
+struct fat_timestamp_testcase {
+ const char *name;
+ struct timespec64 ts;
+ __le16 time;
+ __le16 date;
+ u8 cs;
+ int time_offset;
+};
+
+const static struct fat_timestamp_testcase time_test_cases[] = {
+ {
+ .name = "Earliest possible UTC (1980-01-01 00:00:00)",
+ .ts = {.tv_sec = 315532800LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Latest possible UTC (2107-12-31 23:59:58)",
+ .ts = {.tv_sec = 4354819198LL, .tv_nsec = 0L},
+ .time = 49021,
+ .date = 65439,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Earliest possible (UTC-11) (== 1979-12-31 13:00:00
UTC)",
+ .ts = {.tv_sec = 315493200LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 11 * 60,
+ },
+ {
+
[PATCH v5] fat: Add KUnit tests for checksums and timestamps
Add some basic sanity-check tests for the fat_checksum() function and
the fat_time_unix2fat() and fat_time_fat2unix() functions. These unit
tests verify these functions return correct output for a number of test
inputs.
These tests were inspored by -- and serve a similar purpose to -- the
timestamp parsing KUnit tests in ext4[1].
Note that, unlike fat_time_unix2fat, fat_time_fat2unix wasn't previously
exported, so this patch exports it as well. This is required for the
case where we're building the fat and fat_test as modules.
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/inode-test.c
Signed-off-by: David Gow
Acked-by: OGAWA Hirofumi
---
Whoops! Sent out a broken early version of this as v4 instead.
Sorry about that!
-- David
Changes since v4:
https://lore.kernel.org/linux-kselftest/20201024052047.2526780-1-david...@google.com/
- Fix a typo introduced in the Kconfig. It builds now.
Changes since v3:
https://lore.kernel.org/linux-kselftest/20201021061713.1545931-1-david...@google.com/
- Update the Kconfig entry to use "depends on" rather than "select", as
discussed in [2].
- Depend on "MSDOS_FS || VFAT_FS", rather than "FAT_FS", as we need the
CONFIG_FAT_DEFAULT_CODEPAGE symbol to be defined.
Changes since v2:
https://lore.kernel.org/linux-kselftest/20201020055856.1270482-1-david...@google.com/
- Comment that the export for fat_time_fat2unix() function is for KUnit
tests.
Changes since v1:
https://lore.kernel.org/linux-kselftest/20201017064107.375174-1-david...@google.com/
- Now export fat_time_fat2unix() so that the test can access it when
built as a module.
[2]:
https://lore.kernel.org/linux-ext4/52959e99-4105-3de9-730c-c46894b82...@infradead.org/T/#t
fs/fat/Kconfig| 12 +++
fs/fat/Makefile | 2 +
fs/fat/fat_test.c | 196 ++
fs/fat/misc.c | 2 +
4 files changed, 212 insertions(+)
create mode 100644 fs/fat/fat_test.c
diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index 66532a71e8fd..5f44bf94224e 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
@@ -115,3 +115,15 @@ config FAT_DEFAULT_UTF8
Say Y if you use UTF-8 encoding for file names, N otherwise.
See for more information.
+
+config FAT_KUNIT_TEST
+ tristate "Unit Tests for FAT filesystems" if !KUNIT_ALL_TESTS
+ depends on KUNIT & (MSDOS_FS || VFAT_FS)
+ default KUNIT_ALL_TESTS
+ help
+ This builds the FAT KUnit tests
+
+ For more information on KUnit and unit tests in general, please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit
+
+ If unsure, say N
diff --git a/fs/fat/Makefile b/fs/fat/Makefile
index 70645ce2f7fc..2b034112690d 100644
--- a/fs/fat/Makefile
+++ b/fs/fat/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_MSDOS_FS) += msdos.o
fat-y := cache.o dir.o fatent.o file.o inode.o misc.o nfs.o
vfat-y := namei_vfat.o
msdos-y := namei_msdos.o
+
+obj-$(CONFIG_FAT_KUNIT_TEST) += fat_test.o
diff --git a/fs/fat/fat_test.c b/fs/fat/fat_test.c
new file mode 100644
index ..c99bfbdf89bb
--- /dev/null
+++ b/fs/fat/fat_test.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for FAT filesystems.
+ *
+ * Copyright (C) 2020 Google LLC.
+ * Author: David Gow
+ */
+
+#include
+
+#include "fat.h"
+
+static void fat_checksum_test(struct kunit *test)
+{
+ /* With no extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("VMLINUX"), 44);
+ /* With 3-letter extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("README TXT"), 115);
+ /* With short (1-letter) extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("ABCDEFGHA "), 98);
+}
+
+
+struct fat_timestamp_testcase {
+ const char *name;
+ struct timespec64 ts;
+ __le16 time;
+ __le16 date;
+ u8 cs;
+ int time_offset;
+};
+
+const static struct fat_timestamp_testcase time_test_cases[] = {
+ {
+ .name = "Earliest possible UTC (1980-01-01 00:00:00)",
+ .ts = {.tv_sec = 315532800LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Latest possible UTC (2107-12-31 23:59:58)",
+ .ts = {.tv_sec = 4354819198LL, .tv_nsec = 0L},
+ .time = 49021,
+ .date = 65439,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Earliest possible (UTC-11) (== 1979-12-31 13:00:00
UTC)",
+ .ts = {.tv_sec = 315493200LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 11 * 60,
+ },
+ {
+ .name = "Latest possible (UTC+11) (== 2108-01-01 10
Re: [PATCH] ext: EXT4_KUNIT_TESTS should depend on EXT4_FS instead of selecting it
On Fri, Oct 23, 2020 at 10:07 PM Theodore Y. Ts'o wrote: > > On Thu, Oct 22, 2020 at 04:52:52PM -0700, Brendan Higgins wrote: > > So you, me, Luis, David, and a whole bunch of other people have been > > thinking about this problem for a while. What if we just put > > kunitconfig fragments in directories along side the test files they > > enable? > > > > For example, we could add a file to fs/ext4/kunitconfig which contains: > > > > CONFIG_EXT4_FS=y > > CONFIG_EXT4_KUNIT_TESTS=y > > > > We could do something similar in fs/jdb2, etc. > > > > Obviously some logically separate KUnit tests (different maintainers, > > different Kconfig symbols, etc) reside in the same directory, for > > these we could name the kunitconfig file something like > > lib/list-test.kunitconfig (not a great example because lists are > > always built into Linux), but you get the idea. > > > > Then like Ted suggested, if you call kunit.py run foo/bar, then > > > > if bar is a directory, then kunit.py will look for foo/bar/kunitconfig > > > > if bar is a file ending with .kunitconfig like foo/bar.kunitconfig, > > then it will use that kunitconfig > > > > if bar is '...' (foo/...) then kunit.py will look for all kunitconfigs > > underneath foo. > > > > Once all the kunitconfigs have been resolved, they will be merged into > > the .kunitconfig. If they can be successfully merged together, the new > > .kunitconfig will then continue to function as it currently does. > > I was thinking along a similar set of lines this morning. One thing > I'd add in addition to your suggestion to that is to change how > .kunitconfig is interpreted such that > > CONFIG_KUNIT=y > > is always implied, so it doesn't have to be specified explicitly, and > that if a line like: > > fs/ext4 > > or > > mm > > etc. occurs, that will cause a include of the Kunitconfig (I'd using a > capitalized version of the filename like Kconfig, so that it's easier > to see in a directory listing) in the named directory. > > That way, .kunitconfig is backwards compatible, but it also allows > people to put a one-liner into .kunitconfig to enable the unit tests > for that particular directory. > > What do folks think? > I quite like the idea of supporting includes, as it'd make it easier to have test hierarchies as well: fs/Kunitconfig could include ext4/Kunitconfig and fat/Kunitconfig, for instance. If we're adding something more complicated to the Kunitconfig files than just raw config entries, there are other things we could do, too (personally, I'd quite like to be able to list KUnit test modules to be loaded someday, though that's a bit outside the scope of this discussion). There are some issues with exactly how we format these that'll need to be resolved: there are cases where there are multiple distinct "areas" that need testing which share a subdirectory (something like lib/), so just using directory paths with one Kunitconfig file per directory has some limitations. At the same time, it's definitely nicer to be able to just specify a directory where that works. Here's what I propose (noting that, realistically, it's unlikely we'll have time to build the perfect system straight away): 1. We've agreed that 'select' isn't the solution we want, so accept this patch to get rid of it, and avoid using it elsewhere (I've done this for the fat test[1]). Do we know if changing this now will seriously break anyone's workflow (particularly if there's something automated that'd break?) 2. Add support to kunit.py for loading an arbitrary kunitconfig, rather than using the default one in the root or build dir. (e.g., kunit.py run [path_to_kunitconfig]). This'd let us create per-subsystem/feature/etc kunitconfigs and use them straight away. This'd still require people to pass in the kunitconfig path each time they run the tests, but'd at least give maintainers a single command they can use and recommend — they'd just need to have a Kunitconfig file somewhere in the tree with the tests they want people to run. Maybe if you pass a directory path, it looks for "Kunitconfig" in that directory, but otherwise it can be a file like "lib/data-structres.Kunitconfig" or something which doesn't correspond to a directory. 3. Add the include support, etc, to kunitconfig, so people working on multiple subsystems can more easily run groups of tests. This may get a little complicated if we care about handling things with conflicting dependencies, so I don't want to hold up the first two on that. How does that sound? -- David [1]: https://lore.kernel.org/linux-kselftest/20201024052047.2526780-1-david...@google.com/
[PATCH v4] fat: Add KUnit tests for checksums and timestamps
Add some basic sanity-check tests for the fat_checksum() function and
the fat_time_unix2fat() and fat_time_fat2unix() functions. These unit
tests verify these functions return correct output for a number of test
inputs.
These tests were inspored by -- and serve a similar purpose to -- the
timestamp parsing KUnit tests in ext4[1].
Note that, unlike fat_time_unix2fat, fat_time_fat2unix wasn't previously
exported, so this patch exports it as well. This is required for the
case where we're building the fat and fat_test as modules.
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/inode-test.c
Signed-off-by: David Gow
Acked-by: OGAWA Hirofumi
---
Changes since v3:
https://lore.kernel.org/linux-kselftest/20201021061713.1545931-1-david...@google.com/
- Update the Kconfig entry to use "depends on" rather than "select", as
discussed in [2].
- Depend on "MSDOS_FS || VFAT_FS", rather than "FAT_FS", as we need the
CONFIG_FAT_DEFAULT_CODEPAGE symbol to be defined.
Changes since v2:
https://lore.kernel.org/linux-kselftest/20201020055856.1270482-1-david...@google.com/
- Comment that the export for fat_time_fat2unix() function is for KUnit
tests.
Changes since v1:
https://lore.kernel.org/linux-kselftest/20201017064107.375174-1-david...@google.com/
- Now export fat_time_fat2unix() so that the test can access it when
built as a module.
[2]:
https://lore.kernel.org/linux-ext4/52959e99-4105-3de9-730c-c46894b82...@infradead.org/T/#t
fs/fat/Kconfig| 12 +++
fs/fat/Makefile | 2 +
fs/fat/fat_test.c | 196 ++
fs/fat/misc.c | 2 +
4 files changed, 212 insertions(+)
create mode 100644 fs/fat/fat_test.c
diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index 66532a71e8fd..5f44bf94224e 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
@@ -115,3 +115,15 @@ config FAT_DEFAULT_UTF8
Say Y if you use UTF-8 encoding for file names, N otherwise.
See for more information.
+
+config FAT_KUNIT_TEST
+ tristate "Unit Tests for FAT filesystems" if !KUNIT_ALL_TESTS
+ depends on KUNIT & (MSDOS_FS || VFAT_FS)
+ default KUNIT_ALL_TESTS
+ help
+ This builds the FAT KUnit tests
+
+ For more information on KUnit and unit tests in general, please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit
+
+ If unsure, say N
diff --git a/fs/fat/Makefile b/fs/fat/Makefile
index 70645ce2f7fc..2b034112690d 100644
--- a/fs/fat/Makefile
+++ b/fs/fat/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_MSDOS_FS) += msdos.o
fat-y := cache.o dir.o fatent.o file.o inode.o misc.o nfs.o
vfat-y := namei_vfat.o
msdos-y := namei_msdos.o
+
+obj-$(CONFIG_FAT_KUNIT_TEST) += fat_test.o
diff --git a/fs/fat/fat_test.c b/fs/fat/fat_test.c
new file mode 100644
index ..c99bfbdf89bb
--- /dev/null
+++ b/fs/fat/fat_test.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for FAT filesystems.
+ *
+ * Copyright (C) 2020 Google LLC.
+ * Author: David Gow
+ */
+
+#include
+
+#include "fat.h"
+
+static void fat_checksum_test(struct kunit *test)
+{
+ /* With no extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("VMLINUX"), 44);
+ /* With 3-letter extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("README TXT"), 115);
+ /* With short (1-letter) extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("ABCDEFGHA "), 98);
+}
+
+
+struct fat_timestamp_testcase {
+ const char *name;
+ struct timespec64 ts;
+ __le16 time;
+ __le16 date;
+ u8 cs;
+ int time_offset;
+};
+
+const static struct fat_timestamp_testcase time_test_cases[] = {
+ {
+ .name = "Earliest possible UTC (1980-01-01 00:00:00)",
+ .ts = {.tv_sec = 315532800LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Latest possible UTC (2107-12-31 23:59:58)",
+ .ts = {.tv_sec = 4354819198LL, .tv_nsec = 0L},
+ .time = 49021,
+ .date = 65439,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Earliest possible (UTC-11) (== 1979-12-31 13:00:00
UTC)",
+ .ts = {.tv_sec = 315493200LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 11 * 60,
+ },
+ {
+ .name = "Latest possible (UTC+11) (== 2108-01-01 10:59:58 UTC)",
+ .ts = {.tv_sec = 4354858798LL, .tv_nsec = 0L},
+ .time = 49021,
+ .date = 65439,
+ .cs = 0,
+ .time_offset = -11 * 60,
+ },
+ {
+ .n
[PATCH] kunit: Fix kunit.py --raw_output option
Due to the raw_output() function on kunit_parser.py actually being a
generator, it only runs if something reads the lines it returns. Since
we no-longer do that (parsing doesn't actually happen if raw_output is
enabled), it was not printing anything.
Fixes: 45ba7a893ad89114e773b3dc32f6431354c465d6 ("kunit: kunit_tool: Separate
out config/build/exec/parse")
Signed-off-by: David Gow
---
tools/testing/kunit/kunit_parser.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/tools/testing/kunit/kunit_parser.py
b/tools/testing/kunit/kunit_parser.py
index 8019e3dd4c32..744ee9cb0073 100644
--- a/tools/testing/kunit/kunit_parser.py
+++ b/tools/testing/kunit/kunit_parser.py
@@ -66,7 +66,6 @@ def isolate_kunit_output(kernel_output):
def raw_output(kernel_output):
for line in kernel_output:
print(line)
- yield line
DIVIDER = '=' * 60
--
2.29.0.rc1.297.gfa9743e501-goog
Re: [PATCH v1] kunit: tools: fix kunit_tool tests for parsing test plans
On Thu, Oct 22, 2020 at 4:39 AM Brendan Higgins
wrote:
>
> Some tests logs for kunit_tool tests are missing their test plans
> causing their tests to fail; fix this by adding the test plans.
>
> Fixes: 45dcbb6f5ef7 ("kunit: test: add test plan to KUnit TAP format")
> Signed-off-by: Brendan Higgins
Excellent: this fixes all of the failing tests in kunit_tool_test.py for me.
Reviewed-by: David Gow
-- David
> ---
> tools/testing/kunit/kunit_tool_test.py| 32 ++
> .../test_data/test_config_printk_time.log | Bin 1584 -> 1605 bytes
> .../test_data/test_interrupted_tap_output.log | Bin 1982 -> 2003 bytes
> .../test_data/test_kernel_panic_interrupt.log | Bin 1321 -> 1342 bytes
> .../test_data/test_multiple_prefixes.log | Bin 1832 -> 1861 bytes
> .../kunit/test_data/test_pound_no_prefix.log | Bin 1193 -> 1200 bytes
> .../kunit/test_data/test_pound_sign.log | Bin 1656 -> 1676 bytes
> 7 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/kunit/kunit_tool_test.py
> b/tools/testing/kunit/kunit_tool_test.py
> index 99c3c5671ea48..0b60855fb8198 100755
> --- a/tools/testing/kunit/kunit_tool_test.py
> +++ b/tools/testing/kunit/kunit_tool_test.py
> @@ -179,7 +179,7 @@ class KUnitParserTest(unittest.TestCase):
> print_mock = mock.patch('builtins.print').start()
> result = kunit_parser.parse_run_tests(
> kunit_parser.isolate_kunit_output(file.readlines()))
> - print_mock.assert_any_call(StrContains("no kunit output
> detected"))
> + print_mock.assert_any_call(StrContains('no tests run!'))
> print_mock.stop()
> file.close()
>
> @@ -198,39 +198,57 @@ class KUnitParserTest(unittest.TestCase):
> 'test_data/test_config_printk_time.log')
> with open(prefix_log) as file:
> result =
> kunit_parser.parse_run_tests(file.readlines())
> - self.assertEqual('kunit-resource-test', result.suites[0].name)
> + self.assertEqual(
> + kunit_parser.TestStatus.SUCCESS,
> + result.status)
> + self.assertEqual('kunit-resource-test',
> result.suites[0].name)
>
> def test_ignores_multiple_prefixes(self):
> prefix_log = get_absolute_path(
> 'test_data/test_multiple_prefixes.log')
> with open(prefix_log) as file:
> result =
> kunit_parser.parse_run_tests(file.readlines())
> - self.assertEqual('kunit-resource-test', result.suites[0].name)
> + self.assertEqual(
> + kunit_parser.TestStatus.SUCCESS,
> + result.status)
> + self.assertEqual('kunit-resource-test',
> result.suites[0].name)
>
> def test_prefix_mixed_kernel_output(self):
> mixed_prefix_log = get_absolute_path(
> 'test_data/test_interrupted_tap_output.log')
> with open(mixed_prefix_log) as file:
> result =
> kunit_parser.parse_run_tests(file.readlines())
> - self.assertEqual('kunit-resource-test', result.suites[0].name)
> + self.assertEqual(
> + kunit_parser.TestStatus.SUCCESS,
> + result.status)
> + self.assertEqual('kunit-resource-test',
> result.suites[0].name)
>
> def test_prefix_poundsign(self):
> pound_log = get_absolute_path('test_data/test_pound_sign.log')
> with open(pound_log) as file:
> result =
> kunit_parser.parse_run_tests(file.readlines())
> - self.assertEqual('kunit-resource-test', result.suites[0].name)
> + self.assertEqual(
> + kunit_parser.TestStatus.SUCCESS,
> + result.status)
> + self.assertEqual('kunit-resource-test',
> result.suites[0].name)
>
> def test_kernel_panic_end(self):
> panic_log =
> get_absolute_path('test_data/test_kernel_panic_interrupt.log')
> with open(panic_log) as file:
> result =
> kunit_parser.parse_run_tests(file.readlines())
> - self.assertEqual('kunit-resource-test', result.suites[0].name)
> + self.assertEqual(
> + kunit_parser.TestStatus.TEST_CRASHED,
>
Re: [PATCH] ext: EXT4_KUNIT_TESTS should depend on EXT4_FS instead of selecting it
On Thu, Oct 22, 2020 at 5:15 AM Brendan Higgins
wrote:
>
> On Tue, Oct 20, 2020 at 12:37 AM Geert Uytterhoeven
> wrote:
> >
> > EXT4_KUNIT_TESTS selects EXT4_FS, thus enabling an optional feature the
> > user may not want to enable. Fix this by making the test depend on
> > EXT4_FS instead.
> >
> > Fixes: 1cbeab1b242d16fd ("ext4: add kunit test for decoding extended
> > timestamps")
> > Signed-off-by: Geert Uytterhoeven
>
> If I remember correctly, having EXT4_KUNIT_TESTS select EXT4_FS was
> something that Ted specifically requested, but I don't have any strong
> feelings on it either way.
For what it's worth, the upcoming FAT filesystem tests[1] are also
select-ing FAT_FS at the moment, so if this changes here, I'll likely
update it there as well.
-- David
[1]:
https://lore.kernel.org/linux-kselftest/20201021061713.1545931-1-david...@google.com/T/#u
[PATCH] kunit: Fix kunit.py parse subcommand (use null build_dir)
When JSON support was added in [1], the KunitParseRequest tuple was
updated to contain a 'build_dir' field, but kunit.py parse doesn't
accept --build_dir as an option. The code nevertheless tried to access
it, resulting in this error:
AttributeError: 'Namespace' object has no attribute 'build_dir'
Given that the parser only uses the build_dir variable to set the
'build_environment' json field, we set it to None (which gives the JSON
'null') for now. Ultimately, we probably do want to be able to set this,
but since it's new functionality which (for the parse subcommand) never
worked, this is the quickest way of getting it back up and running.
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git/commit/?h=kunit-fixes=21a6d1780d5bbfca0ce9b8104ca6233502fcbf86
Fixes: 21a6d1780d5bbfca0ce9b8104ca6233502fcbf86 ("kunit: tool: allow generating
test results in JSON")
Signed-off-by: David Gow
---
This is a quick fix because kunit.py parse is completely broken: it
appears it was introduced in the rebase of the JSON parser after the
separation of concerns patch.
tools/testing/kunit/kunit.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/kunit/kunit.py b/tools/testing/kunit/kunit.py
index ebf5f5763dee..a6d5f219f714 100755
--- a/tools/testing/kunit/kunit.py
+++ b/tools/testing/kunit/kunit.py
@@ -337,7 +337,7 @@ def main(argv, linux=None):
kunit_output = f.read().splitlines()
request = KunitParseRequest(cli_args.raw_output,
kunit_output,
- cli_args.build_dir,
+ None,
cli_args.json)
result = parse_tests(request)
if result.status != KunitStatus.SUCCESS:
base-commit: c4d6fe7311762f2e03b3c27ad38df7c40c80cc93
--
2.29.0.rc1.297.gfa9743e501-goog
[PATCH v3] fat: Add KUnit tests for checksums and timestamps
Add some basic sanity-check tests for the fat_checksum() function and
the fat_time_unix2fat() and fat_time_fat2unix() functions. These unit
tests verify these functions return correct output for a number of test
inputs.
These tests were inspored by -- and serve a similar purpose to -- the
timestamp parsing KUnit tests in ext4[1].
Note that, unlike fat_time_unix2fat, fat_time_fat2unix wasn't previously
exported, so this patch exports it as well. This is required for the
case where we're building the fat and fat_test as modules.
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/inode-test.c
Signed-off-by: David Gow
Acked-by: OGAWA Hirofumi
---
Changes since v2:
https://lore.kernel.org/linux-kselftest/20201020055856.1270482-1-david...@google.com/
- Comment that the export for fat_time_fat2unix() function is for KUnit
tests.
Changes since v1:
https://lore.kernel.org/linux-kselftest/20201017064107.375174-1-david...@google.com/
- Now export fat_time_fat2unix() so that the test can access it when
built as a module.
Cheers,
-- David
fs/fat/Kconfig| 13 +++
fs/fat/Makefile | 2 +
fs/fat/fat_test.c | 196 ++
fs/fat/misc.c | 2 +
4 files changed, 213 insertions(+)
create mode 100644 fs/fat/fat_test.c
diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index 66532a71e8fd..fdef03b79c69 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
@@ -115,3 +115,16 @@ config FAT_DEFAULT_UTF8
Say Y if you use UTF-8 encoding for file names, N otherwise.
See for more information.
+
+config FAT_KUNIT_TEST
+ tristate "Unit Tests for FAT filesystems" if !KUNIT_ALL_TESTS
+ select FAT_FS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ This builds the FAT KUnit tests
+
+ For more information on KUnit and unit tests in general, please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit
+
+ If unsure, say N
diff --git a/fs/fat/Makefile b/fs/fat/Makefile
index 70645ce2f7fc..2b034112690d 100644
--- a/fs/fat/Makefile
+++ b/fs/fat/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_MSDOS_FS) += msdos.o
fat-y := cache.o dir.o fatent.o file.o inode.o misc.o nfs.o
vfat-y := namei_vfat.o
msdos-y := namei_msdos.o
+
+obj-$(CONFIG_FAT_KUNIT_TEST) += fat_test.o
diff --git a/fs/fat/fat_test.c b/fs/fat/fat_test.c
new file mode 100644
index ..c99bfbdf89bb
--- /dev/null
+++ b/fs/fat/fat_test.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for FAT filesystems.
+ *
+ * Copyright (C) 2020 Google LLC.
+ * Author: David Gow
+ */
+
+#include
+
+#include "fat.h"
+
+static void fat_checksum_test(struct kunit *test)
+{
+ /* With no extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("VMLINUX"), 44);
+ /* With 3-letter extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("README TXT"), 115);
+ /* With short (1-letter) extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("ABCDEFGHA "), 98);
+}
+
+
+struct fat_timestamp_testcase {
+ const char *name;
+ struct timespec64 ts;
+ __le16 time;
+ __le16 date;
+ u8 cs;
+ int time_offset;
+};
+
+const static struct fat_timestamp_testcase time_test_cases[] = {
+ {
+ .name = "Earliest possible UTC (1980-01-01 00:00:00)",
+ .ts = {.tv_sec = 315532800LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Latest possible UTC (2107-12-31 23:59:58)",
+ .ts = {.tv_sec = 4354819198LL, .tv_nsec = 0L},
+ .time = 49021,
+ .date = 65439,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Earliest possible (UTC-11) (== 1979-12-31 13:00:00
UTC)",
+ .ts = {.tv_sec = 315493200LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 11 * 60,
+ },
+ {
+ .name = "Latest possible (UTC+11) (== 2108-01-01 10:59:58 UTC)",
+ .ts = {.tv_sec = 4354858798LL, .tv_nsec = 0L},
+ .time = 49021,
+ .date = 65439,
+ .cs = 0,
+ .time_offset = -11 * 60,
+ },
+ {
+ .name = "Leap Day / Year (1996-02-29 00:00:00)",
+ .ts = {.tv_sec = 825552000LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 8285,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Year 2000 is leap year (2000-02-29 00:00:00)",
+ .ts = {.tv_sec = 951782400LL, .tv_nsec = 0L},
+ .time = 0,
+ .date
Re: [PATCH v2] Documentation: kunit: Update Kconfig parts for KUNIT's module support
On Tue, Oct 13, 2020 at 2:38 PM 'SeongJae Park' via KUnit Development
wrote:
>
> From: SeongJae Park
>
> If 'CONFIG_KUNIT=m', letting kunit tests that do not support loadable
> module build depends on 'KUNIT' instead of 'KUNIT=y' result in compile
> errors. This commit updates the document for this.
>
> Fixes: 9fe124bf1b77 ("kunit: allow kunit to be loaded as a module")
> Signed-off-by: SeongJae Park
Sorry for the delay in looking at this. Apart from another minuscule
typo below, this looks good to me.
Reviewed-by: David Gow
Cheers,
-- David
> ---
>
> Changes from v1
> (https://lore.kernel.org/linux-kselftest/20201012105420.5945-1-sjp...@amazon.com/):
> - Fix a typo (Marco Elver)
>
> ---
> Documentation/dev-tools/kunit/start.rst | 2 +-
> Documentation/dev-tools/kunit/usage.rst | 5 +
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/dev-tools/kunit/start.rst
> b/Documentation/dev-tools/kunit/start.rst
> index d23385e3e159..454f307813ea 100644
> --- a/Documentation/dev-tools/kunit/start.rst
> +++ b/Documentation/dev-tools/kunit/start.rst
> @@ -197,7 +197,7 @@ Now add the following to ``drivers/misc/Kconfig``:
>
> config MISC_EXAMPLE_TEST
> bool "Test for my example"
> - depends on MISC_EXAMPLE && KUNIT
> + depends on MISC_EXAMPLE && KUNIT=y
>
> and the following to ``drivers/misc/Makefile``:
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst
> b/Documentation/dev-tools/kunit/usage.rst
> index 3c3fe8b5fecc..b331f5a5b0b9 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -556,6 +556,11 @@ Once the kernel is built and installed, a simple
>
> ...will run the tests.
>
> +.. note::
> + Note that you should make your test depends on ``KUNIT=y`` in Kconfig if
> the
nit: Grammatically, this should technically be either "depend" (2nd
person), or something like "make sure [that] your test depends".
> + test does not support module build. Otherwise, it will trigger compile
> + errors if ``CONFIG_KUNIT`` is ``m``.
> +
Someday it'd be nice to better discuss the reasons a test suite might
not be compilable as a module. It's probably outside the scope of this
commit to do it properly, though.
> Writing new tests for other architectures
> -
>
> --
> 2.17.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to kunit-dev+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/kunit-dev/20201013063743.32179-1-sjpark%40amazon.com.
Re: [PATCH v2] fat: Add KUnit tests for checksums and timestamps
On Tue, Oct 20, 2020 at 2:51 PM OGAWA Hirofumi wrote: > > David Gow writes: > > > diff --git a/fs/fat/misc.c b/fs/fat/misc.c > > index f1b2a1fc2a6a..445ad3542e74 100644 > > --- a/fs/fat/misc.c > > +++ b/fs/fat/misc.c > > @@ -229,6 +229,7 @@ void fat_time_fat2unix(struct msdos_sb_info *sbi, > > struct timespec64 *ts, > > ts->tv_nsec = 0; > > } > > } > > +EXPORT_SYMBOL_GPL(fat_time_fat2unix); > > Hm, can this export only if FAT_KUNIT_TEST is builtin or module (maybe > #if IS_ENABLED(...))? And #if will also be worked as the comment too. > That's possible, but I'd prefer to export it unconditionally for two reasons: 1. It'd make it possible to build the fat_test module without having to rebuild the whole kernel/fat. 2. It'd be consistent with fat_time_unix2fat(), which is exported for use in vfat/msdos anyway. Neither of those are dealbreakers, though, so if you'd still prefer this to be behind an ifdef, I'll change it. -- David
Re: [PATCH] lib: add basic KUnit test for lib/math
On Tue, Oct 20, 2020 at 6:46 AM Daniel Latypov wrote:
>
> Add basic test coverage for files that don't require any config options:
> * gcd.c
> * lcm.c
> * int_sqrt.c
> * reciprocal_div.c
> (Ignored int_pow.c since it's a simple textbook algorithm.)
>
I don't see a particular reason why int_pow.c being a simple algorithm
means it shouldn't be tested. I'm not saying it has to be tested by
this particular change -- and I doubt the test would be
earth-shatteringly interesting -- but there's no real reason against
testing it.
> These tests aren't particularly interesting, but
> * they're chosen as easy to understand examples of how to write tests
> * provides a place to add tests for any new files in this dir
> * written so adding new test cases to cover edge cases should be easy
I think these tests can stand on their own merits, rather than just as
examples (though I do think they do make good additional examples for
how to test these sorts of functions).
So, I'd treat this as an actual test of the maths functions (and
you've got what seems to me a decent set of test cases for that,
though there are a couple of comments below) first, and any use it
gains as an example is sort-of secondary to that (anything that makes
it a better example is likely to make it a better test anyway).
In any case, modulo the comments below, this seems good to me.
-- David
> Signed-off-by: Daniel Latypov
> ---
> lib/math/Kconfig | 5 ++
> lib/math/Makefile| 2 +
> lib/math/math_test.c | 197 +++
> 3 files changed, 204 insertions(+)
> create mode 100644 lib/math/math_test.c
>
> diff --git a/lib/math/Kconfig b/lib/math/Kconfig
> index f19bc9734fa7..6ba8680439c1 100644
> --- a/lib/math/Kconfig
> +++ b/lib/math/Kconfig
> @@ -15,3 +15,8 @@ config PRIME_NUMBERS
>
> config RATIONAL
> bool
> +
> +config MATH_KUNIT_TEST
> + tristate "KUnit test for lib/math" if !KUNIT_ALL_TESTS
> + default KUNIT_ALL_TESTS
> + depends on KUNIT
> diff --git a/lib/math/Makefile b/lib/math/Makefile
> index be6909e943bd..fba6fe90f50b 100644
> --- a/lib/math/Makefile
> +++ b/lib/math/Makefile
> @@ -4,3 +4,5 @@ obj-y += div64.o gcd.o lcm.o int_pow.o int_sqrt.o
> reciprocal_div.o
> obj-$(CONFIG_CORDIC) += cordic.o
> obj-$(CONFIG_PRIME_NUMBERS)+= prime_numbers.o
> obj-$(CONFIG_RATIONAL) += rational.o
> +
> +obj-$(CONFIG_MATH_KUNIT_TEST) += math_test.o
> diff --git a/lib/math/math_test.c b/lib/math/math_test.c
> new file mode 100644
> index ..6f4681ea7c72
> --- /dev/null
> +++ b/lib/math/math_test.c
> @@ -0,0 +1,197 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Simple KUnit suite for math helper funcs that are always enabled.
> + *
> + * Copyright (C) 2020, Google LLC.
> + * Author: Daniel Latypov
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/* Generic test case for unsigned long inputs. */
> +struct test_case {
> + unsigned long a, b;
> + unsigned long result;
> +};
> +
> +static void gcd_test(struct kunit *test)
> +{
> + const char *message_fmt = "gcd(%lu, %lu)";
> + int i;
> +
> + struct test_case test_cases[] = {
> + {
> + .a = 0, .b = 1,
> + .result = 1,
> + },
> + {
> + .a = 2, .b = 2,
> + .result = 2,
> + },
> + {
> + .a = 2, .b = 4,
> + .result = 2,
> + },
> + {
> + .a = 3, .b = 5,
> + .result = 1,
> + },
> + {
> + .a = 3*9, .b = 3*5,
> + .result = 3,
> + },
> + {
> + .a = 3*5*7, .b = 3*5*11,
> + .result = 15,
> + },
> + {
> + .a = (1 << 21) - 1,
> + .b = (1 << 22) - 1,
It might be worth noting the factors of these (7^2*127*337 and
3*23*89*683) in a comment.
They aren't mersenne primes, if that's what you were going for, though
they are coprime.
> + .result = 1,
> + },
> + };
> +
> + for (i = 0; i < ARRAY_SIZE(test_cases); ++i) {
> + KUNIT_EXPECT_EQ_MSG(test, test_cases[i].result,
> + gcd(test_cases[i].a, test_cases[i].b),
> + message_fmt, test_cases[i].a,
> + test_cases[i].b);
> +
> + /* gcd(a,b) == gcd(b,a) */
> + KUNIT_EXPECT_EQ_MSG(test, test_cases[i].result,
> + gcd(test_cases[i].b, test_cases[i].a),
> + message_fmt, test_cases[i].b,
> + test_cases[i].a);
> + }
> +}
> +
>
[PATCH v2] fat: Add KUnit tests for checksums and timestamps
Add some basic sanity-check tests for the fat_checksum() function and
the fat_time_unix2fat() and fat_time_fat2unix() functions. These unit
tests verify these functions return correct output for a number of test
inputs.
These tests were inspored by -- and serve a similar purpose to -- the
timestamp parsing KUnit tests in ext4[1].
Note that, unlike fat_time_unix2fat, fat_time_fat2unix wasn't previously
exported, so this patch exports it as well. This is required for the
case where we're building the fat and fat_test as modules.
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/inode-test.c
Signed-off-by: David Gow
Acked-by: OGAWA Hirofumi
---
Changes since v1:
https://lore.kernel.org/linux-kselftest/20201017064107.375174-1-david...@google.com/
- Now export fat_time_fat2unix() so that the test can access it when
built as a module.
fs/fat/Kconfig| 13 +++
fs/fat/Makefile | 2 +
fs/fat/fat_test.c | 197 ++
fs/fat/misc.c | 1 +
4 files changed, 213 insertions(+)
create mode 100644 fs/fat/fat_test.c
diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index 66532a71e8fd..fdef03b79c69 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
@@ -115,3 +115,16 @@ config FAT_DEFAULT_UTF8
Say Y if you use UTF-8 encoding for file names, N otherwise.
See for more information.
+
+config FAT_KUNIT_TEST
+ tristate "Unit Tests for FAT filesystems" if !KUNIT_ALL_TESTS
+ select FAT_FS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ This builds the FAT KUnit tests
+
+ For more information on KUnit and unit tests in general, please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit
+
+ If unsure, say N
diff --git a/fs/fat/Makefile b/fs/fat/Makefile
index 70645ce2f7fc..2b034112690d 100644
--- a/fs/fat/Makefile
+++ b/fs/fat/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_MSDOS_FS) += msdos.o
fat-y := cache.o dir.o fatent.o file.o inode.o misc.o nfs.o
vfat-y := namei_vfat.o
msdos-y := namei_msdos.o
+
+obj-$(CONFIG_FAT_KUNIT_TEST) += fat_test.o
diff --git a/fs/fat/fat_test.c b/fs/fat/fat_test.c
new file mode 100644
index ..c1b4348b9b3b
--- /dev/null
+++ b/fs/fat/fat_test.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for FAT filesystems.
+ *
+ * Copyright (C) 2020 Google LLC.
+ * Author: David Gow
+ */
+
+#include
+
+#include "fat.h"
+
+static void fat_checksum_test(struct kunit *test)
+{
+ /* With no extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("VMLINUX"), 44);
+ /* With 3-letter extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("README TXT"), 115);
+ /* With short (1-letter) extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("ABCDEFGHA "), 98);
+}
+
+
+struct fat_timestamp_testcase {
+ const char *name;
+ struct timespec64 ts;
+ __le16 time;
+ __le16 date;
+ u8 cs;
+ int time_offset;
+};
+
+const static struct fat_timestamp_testcase time_test_cases[] = {
+ {
+ .name = "Earliest possible UTC (1980-01-01 00:00:00)",
+ .ts = {.tv_sec = 315532800LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Latest possible UTC (2107-12-31 23:59:58)",
+ .ts = {.tv_sec = 4354819198LL, .tv_nsec = 0L},
+ .time = 49021,
+ .date = 65439,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Earliest possible (UTC-11) (== 1979-12-31 13:00:00
UTC)",
+ .ts = {.tv_sec = 315493200LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 11 * 60,
+ },
+ {
+ .name = "Latest possible (UTC+11) (== 2108-01-01 10:59:58 UTC)",
+ .ts = {.tv_sec = 4354858798LL, .tv_nsec = 0L},
+ .time = 49021,
+ .date = 65439,
+ .cs = 0,
+ .time_offset = -11 * 60,
+ },
+ {
+ .name = "Leap Day / Year (1996-02-29 00:00:00)",
+ .ts = {.tv_sec = 825552000LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 8285,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Year 2000 is leap year (2000-02-29 00:00:00)",
+ .ts = {.tv_sec = 951782400LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 10333,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Year 2100 not leap year (2100-03-01 00:00:00)",
+ .ts = {.
Re: [PATCH] kasan: adopt KUNIT tests to SW_TAGS mode
On Sat, Oct 17, 2020 at 3:33 AM Andrey Konovalov wrote: > > Now that we have KASAN-KUNIT tests integration, it's easy to see that > some KASAN tests are not adopted to the SW_TAGS mode and are failing. > > Adjust the allocation size for kasan_memchr() and kasan_memcmp() by > roung it up to OOB_TAG_OFF so the bad access ends up in a separate > memory granule. > > Add new kmalloc_uaf_16() and kasan_bitops_uaf() tests that rely on UAFs, > as it's hard to adopt the existing kmalloc_oob_16() and kasan_bitops_oob() > (rename from kasan_bitops()) without losing the precision. > > Disable kasan_global_oob() and kasan_alloca_oob_left/right() as SW_TAGS > mode doesn't instrument globals nor dynamic allocas. > > Signed-off-by: Andrey Konovalov This looks good to me. Though, as you mention, writing to freed memory might not bode well for system stability after the test runs. I don't think that needs to be a goal for these tests, though. One thing which we're hoping to add to KUnit soon is support for skipping tests: once that's in place, we can use it to mark tests as explicitly skipped if they rely on the GENERIC mode. That'll take a little while to get upstream though, so I wouldn't want to hold this up for it. Otherwise, from the KUnit side, this looks great. I also tested it against the GENERIC mode on x86_64 (which is all I have set up here at the moment), and nothing obviously had broken. So: Tested-by: David Gow Cheers, -- David
[PATCH] fat: Add KUnit tests for checksums and timestamps
Add some basic sanity-check tests for the fat_checksum() function and
the fat_time_unix2fat() and fat_time_fat2unix() functions. These unit
tests verify these functions return correct output for a number of test
inputs.
These tests were inspored by -- and serve a similar purpose to -- the
timestamp parsing KUnit tests in ext4[1].
[1]:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/ext4/inode-test.c
Signed-off-by: David Gow
---
fs/fat/Kconfig| 13 +++
fs/fat/Makefile | 2 +
fs/fat/fat_test.c | 197 ++
3 files changed, 212 insertions(+)
create mode 100644 fs/fat/fat_test.c
diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index 66532a71e8fd..fdef03b79c69 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
@@ -115,3 +115,16 @@ config FAT_DEFAULT_UTF8
Say Y if you use UTF-8 encoding for file names, N otherwise.
See for more information.
+
+config FAT_KUNIT_TEST
+ tristate "Unit Tests for FAT filesystems" if !KUNIT_ALL_TESTS
+ select FAT_FS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ This builds the FAT KUnit tests
+
+ For more information on KUnit and unit tests in general, please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit
+
+ If unsure, say N
diff --git a/fs/fat/Makefile b/fs/fat/Makefile
index 70645ce2f7fc..2b034112690d 100644
--- a/fs/fat/Makefile
+++ b/fs/fat/Makefile
@@ -10,3 +10,5 @@ obj-$(CONFIG_MSDOS_FS) += msdos.o
fat-y := cache.o dir.o fatent.o file.o inode.o misc.o nfs.o
vfat-y := namei_vfat.o
msdos-y := namei_msdos.o
+
+obj-$(CONFIG_FAT_KUNIT_TEST) += fat_test.o
diff --git a/fs/fat/fat_test.c b/fs/fat/fat_test.c
new file mode 100644
index ..c1b4348b9b3b
--- /dev/null
+++ b/fs/fat/fat_test.c
@@ -0,0 +1,197 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for FAT filesystems.
+ *
+ * Copyright (C) 2020 Google LLC.
+ * Author: David Gow
+ */
+
+#include
+
+#include "fat.h"
+
+static void fat_checksum_test(struct kunit *test)
+{
+ /* With no extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("VMLINUX"), 44);
+ /* With 3-letter extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("README TXT"), 115);
+ /* With short (1-letter) extension. */
+ KUNIT_EXPECT_EQ(test, fat_checksum("ABCDEFGHA "), 98);
+}
+
+
+struct fat_timestamp_testcase {
+ const char *name;
+ struct timespec64 ts;
+ __le16 time;
+ __le16 date;
+ u8 cs;
+ int time_offset;
+};
+
+const static struct fat_timestamp_testcase time_test_cases[] = {
+ {
+ .name = "Earliest possible UTC (1980-01-01 00:00:00)",
+ .ts = {.tv_sec = 315532800LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Latest possible UTC (2107-12-31 23:59:58)",
+ .ts = {.tv_sec = 4354819198LL, .tv_nsec = 0L},
+ .time = 49021,
+ .date = 65439,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Earliest possible (UTC-11) (== 1979-12-31 13:00:00
UTC)",
+ .ts = {.tv_sec = 315493200LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 33,
+ .cs = 0,
+ .time_offset = 11 * 60,
+ },
+ {
+ .name = "Latest possible (UTC+11) (== 2108-01-01 10:59:58 UTC)",
+ .ts = {.tv_sec = 4354858798LL, .tv_nsec = 0L},
+ .time = 49021,
+ .date = 65439,
+ .cs = 0,
+ .time_offset = -11 * 60,
+ },
+ {
+ .name = "Leap Day / Year (1996-02-29 00:00:00)",
+ .ts = {.tv_sec = 825552000LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 8285,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Year 2000 is leap year (2000-02-29 00:00:00)",
+ .ts = {.tv_sec = 951782400LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 10333,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Year 2100 not leap year (2100-03-01 00:00:00)",
+ .ts = {.tv_sec = 4107542400LL, .tv_nsec = 0L},
+ .time = 0,
+ .date = 61537,
+ .cs = 0,
+ .time_offset = 0,
+ },
+ {
+ .name = "Leap year + timezone UTC+1 (== 2004-02-29 00:30:00
UTC)",
+ .ts = {.tv_sec = 1078014600LL, .tv_nsec = 0L},
+ .time = 48064,
+ .date = 12380,
+ .cs = 0,
+ .time_offset =
Re: linux-next: build warning after merge of the akpm-current tree
[+kasan-dev, +kunit-dev]
On Mon, Sep 14, 2020 at 3:01 PM Stephen Rothwell wrote:
>
> Hi all,
>
> After merging the akpm-current tree, today's linux-next build (x86_64
> allmodconfig) produced this warning:
>
> In file included from lib/test_kasan_module.c:16:
> lib/../mm/kasan/kasan.h:232:6: warning: conflicting types for built-in
> function '__asan_register_globals'; expected 'void(void *, long int)'
> [-Wbuiltin-declaration-mismatch]
> 232 | void __asan_register_globals(struct kasan_global *globals, size_t
> size);
> | ^~~
> lib/../mm/kasan/kasan.h:233:6: warning: conflicting types for built-in
> function '__asan_unregister_globals'; expected 'void(void *, long int)'
> [-Wbuiltin-declaration-mismatch]
> 233 | void __asan_unregister_globals(struct kasan_global *globals, size_t
> size);
> | ^
> lib/../mm/kasan/kasan.h:235:6: warning: conflicting types for built-in
> function '__asan_alloca_poison'; expected 'void(void *, long int)'
> [-Wbuiltin-declaration-mismatch]
> 235 | void __asan_alloca_poison(unsigned long addr, size_t size);
> | ^~~~
> lib/../mm/kasan/kasan.h:236:6: warning: conflicting types for built-in
> function '__asan_allocas_unpoison'; expected 'void(void *, long int)'
> [-Wbuiltin-declaration-mismatch]
> 236 | void __asan_allocas_unpoison(const void *stack_top, const void
> *stack_bottom);
> | ^~~
> lib/../mm/kasan/kasan.h:238:6: warning: conflicting types for built-in
> function '__asan_load1'; expected 'void(void *)'
> [-Wbuiltin-declaration-mismatch]
> 238 | void __asan_load1(unsigned long addr);
> | ^~~~
[...some more similar warnings truncated...]
Whoops -- these are an issue with the patch: the test_kasan_module.c
file should be built with -fno-builtin. I've out a new version of the
series which fixes this:
https://lore.kernel.org/linux-mm/20200915035828.570483-1-david...@google.com/T/#t
Basically, the fix is just:
diff --git a/lib/Makefile b/lib/Makefile
index 8c94cad26db7..d4af75136c54 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o
CFLAGS_test_kasan.o += -fno-builtin
CFLAGS_test_kasan.o += $(call cc-disable-warning, vla)
obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o
+CFLAGS_test_kasan_module.o += -fno-builtin
obj-$(CONFIG_TEST_UBSAN) += test_ubsan.o
CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
UBSAN_SANITIZE_test_ubsan.o := y
--
2.28.0.618.gf4bc123cb7-goog
> drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c: In function
> 'common_nfc_set_geometry':
> drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c:514:3: warning: initialization
> discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
> 514 | nanddev_get_ecc_requirements(>base);
> | ^~~~
>
I was unable to reproduce this warning: it looks unrelated, so I'm
assuming it was attributed.
> Introduced by commit
>
> 77e7d1c8c356 ("KASAN: Port KASAN Tests to KUnit")
>
> --
> Cheers,
> Stephen Rothwell
Sorry for the mess,
-- David
[PATCH v14 2/5] KUnit: KASAN Integration
From: Patricia Alfonso
Integrate KASAN into KUnit testing framework.
- Fail tests when KASAN reports an error that is not expected
- Use KUNIT_EXPECT_KASAN_FAIL to expect a KASAN error in KASAN
tests
- Expected KASAN reports pass tests and are still printed when run
without kunit_tool (kunit_tool still bypasses the report due to the
test passing)
- KUnit struct in current task used to keep track of the current
test from KASAN code
Make use of "[PATCH v3 kunit-next 1/2] kunit: generalize
kunit_resource API beyond allocated resources" and "[PATCH v3
kunit-next 2/2] kunit: add support for named resources" from Alan
Maguire [1]
- A named resource is added to a test when a KASAN report is
expected
- This resource contains a struct for kasan_data containing
booleans representing if a KASAN report is expected and if a
KASAN report is found
[1]
(https://lore.kernel.org/linux-kselftest/1583251361-12748-1-git-send-email-alan.magu...@oracle.com/T/#t)
Signed-off-by: Patricia Alfonso
Signed-off-by: David Gow
Reviewed-by: Andrey Konovalov
Reviewed-by: Dmitry Vyukov
Acked-by: Brendan Higgins
Tested-by: Andrey Konovalov
---
include/kunit/test.h | 5 +
include/linux/kasan.h | 6 ++
lib/kunit/test.c | 13 +++-
lib/test_kasan.c | 47 +--
mm/kasan/report.c | 32 +
5 files changed, 96 insertions(+), 7 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 59f3144f009a..3391f38389f8 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -224,6 +224,11 @@ struct kunit {
struct list_head resources; /* Protected by lock. */
};
+static inline void kunit_set_failure(struct kunit *test)
+{
+ WRITE_ONCE(test->success, false);
+}
+
void kunit_init_test(struct kunit *test, const char *name, char *log);
int kunit_run_tests(struct kunit_suite *suite);
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 087fba34b209..30d343b4a40a 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -14,6 +14,12 @@ struct task_struct;
#include
#include
+/* kasan_data struct is used in KUnit tests for KASAN expected failures */
+struct kunit_kasan_expectation {
+ bool report_expected;
+ bool report_found;
+};
+
extern unsigned char kasan_early_shadow_page[PAGE_SIZE];
extern pte_t kasan_early_shadow_pte[PTRS_PER_PTE];
extern pmd_t kasan_early_shadow_pmd[PTRS_PER_PMD];
diff --git a/lib/kunit/test.c b/lib/kunit/test.c
index c36037200310..dcc35fd30d95 100644
--- a/lib/kunit/test.c
+++ b/lib/kunit/test.c
@@ -10,16 +10,12 @@
#include
#include
#include
+#include
#include "debugfs.h"
#include "string-stream.h"
#include "try-catch-impl.h"
-static void kunit_set_failure(struct kunit *test)
-{
- WRITE_ONCE(test->success, false);
-}
-
static void kunit_print_tap_version(void)
{
static bool kunit_has_printed_tap_version;
@@ -288,6 +284,10 @@ static void kunit_try_run_case(void *data)
struct kunit_suite *suite = ctx->suite;
struct kunit_case *test_case = ctx->test_case;
+#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
+ current->kunit_test = test;
+#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT) */
+
/*
* kunit_run_case_internal may encounter a fatal error; if it does,
* abort will be called, this thread will exit, and finally the parent
@@ -602,6 +602,9 @@ void kunit_cleanup(struct kunit *test)
spin_unlock(>lock);
kunit_remove_resource(test, res);
}
+#if (IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT))
+ current->kunit_test = NULL;
+#endif /* IS_ENABLED(CONFIG_KASAN) && IS_ENABLED(CONFIG_KUNIT)*/
}
EXPORT_SYMBOL_GPL(kunit_cleanup);
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 53e953bb1d1d..58bffadd8367 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -23,6 +23,8 @@
#include
+#include
+
#include "../mm/kasan/kasan.h"
#define OOB_TAG_OFF (IS_ENABLED(CONFIG_KASAN_GENERIC) ? 0 :
KASAN_SHADOW_SCALE_SIZE)
@@ -32,14 +34,55 @@
* are not eliminated as dead code.
*/
-int kasan_int_result;
void *kasan_ptr_result;
+int kasan_int_result;
+
+static struct kunit_resource resource;
+static struct kunit_kasan_expectation fail_data;
+static bool multishot;
+
+static int kasan_test_init(struct kunit *test)
+{
+ /*
+* Temporarily enable multi-shot mode and set panic_on_warn=0.
+* Otherwise, we'd only get a report for the first case.
+*/
+ multishot = kasan_save_enable_multi_shot();
+
+ return 0;
+}
+
+static void kasan_test_exit(struct kunit *test)
+{
+ kasan_restore_multi_shot(multishot);
+}
+
+/**
+ * KUNIT_EXPECT_KASA
[PATCH v14 3/5] KASAN: Port KASAN Tests to KUnit
From: Patricia Alfonso
Transfer all previous tests for KASAN to KUnit so they can be run
more easily. Using kunit_tool, developers can run these tests with their
other KUnit tests and see "pass" or "fail" with the appropriate KASAN
report instead of needing to parse each KASAN report to test KASAN
functionalities. All KASAN reports are still printed to dmesg.
Stack tests do not work properly when KASAN_STACK is enabled so
those tests use a check for "if IS_ENABLED(CONFIG_KASAN_STACK)" so they
only run if stack instrumentation is enabled. If KASAN_STACK is not
enabled, KUnit will print a statement to let the user know this test
was not run with KASAN_STACK enabled.
copy_user_test and kasan_rcu_uaf cannot be run in KUnit so there is a
separate test file for those tests, which can be run as before as a
module.
Signed-off-by: Patricia Alfonso
Signed-off-by: David Gow
Reviewed-by: Brendan Higgins
Reviewed-by: Andrey Konovalov
Reviewed-by: Dmitry Vyukov
Tested-by: Andrey Konovalov
---
lib/Kconfig.kasan | 22 +-
lib/Makefile| 4 +-
lib/test_kasan.c| 687 +++-
lib/test_kasan_module.c | 111 +++
4 files changed, 386 insertions(+), 438 deletions(-)
create mode 100644 lib/test_kasan_module.c
diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
index 047b53dbfd58..9a237887e52e 100644
--- a/lib/Kconfig.kasan
+++ b/lib/Kconfig.kasan
@@ -167,12 +167,24 @@ config KASAN_VMALLOC
for KASAN to detect more sorts of errors (and to support vmapped
stacks), but at the cost of higher memory usage.
-config TEST_KASAN
- tristate "Module for testing KASAN for bug detection"
- depends on m
+config KASAN_KUNIT_TEST
+ tristate "KUnit-compatible tests of KASAN bug detection capabilities"
if !KUNIT_ALL_TESTS
+ depends on KASAN && KUNIT
+ default KUNIT_ALL_TESTS
help
- This is a test module doing various nasty things like
- out of bounds accesses, use after free. It is useful for testing
+ This is a KUnit test suite doing various nasty things like
+ out of bounds and use after free accesses. It is useful for testing
kernel debugging features like KASAN.
+ For more information on KUnit and unit tests in general, please refer
+ to the KUnit documentation in Documentation/dev-tools/kunit
+
+config TEST_KASAN_MODULE
+ tristate "KUnit-incompatible tests of KASAN bug detection capabilities"
+ depends on m && KASAN
+ help
+ This is a part of the KASAN test suite that is incompatible with
+ KUnit. Currently includes tests that do bad copy_from/to_user
+ accesses.
+
endif # KASAN
diff --git a/lib/Makefile b/lib/Makefile
index a4a4c6864f51..d4af75136c54 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -65,9 +65,11 @@ CFLAGS_test_bitops.o += -Werror
obj-$(CONFIG_TEST_SYSCTL) += test_sysctl.o
obj-$(CONFIG_TEST_HASH) += test_hash.o test_siphash.o
obj-$(CONFIG_TEST_IDA) += test_ida.o
-obj-$(CONFIG_TEST_KASAN) += test_kasan.o
+obj-$(CONFIG_KASAN_KUNIT_TEST) += test_kasan.o
CFLAGS_test_kasan.o += -fno-builtin
CFLAGS_test_kasan.o += $(call cc-disable-warning, vla)
+obj-$(CONFIG_TEST_KASAN_MODULE) += test_kasan_module.o
+CFLAGS_test_kasan_module.o += -fno-builtin
obj-$(CONFIG_TEST_UBSAN) += test_ubsan.o
CFLAGS_test_ubsan.o += $(call cc-disable-warning, vla)
UBSAN_SANITIZE_test_ubsan.o := y
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 58bffadd8367..63c26171a791 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -5,8 +5,6 @@
* Author: Andrey Ryabinin
*/
-#define pr_fmt(fmt) "kasan test: %s " fmt, __func__
-
#include
#include
#include
@@ -77,416 +75,327 @@ static void kasan_test_exit(struct kunit *test)
fail_data.report_found); \
} while (0)
-
-
-/*
- * Note: test functions are marked noinline so that their names appear in
- * reports.
- */
-static noinline void __init kmalloc_oob_right(void)
+static void kmalloc_oob_right(struct kunit *test)
{
char *ptr;
size_t size = 123;
- pr_info("out-of-bounds to right\n");
ptr = kmalloc(size, GFP_KERNEL);
- if (!ptr) {
- pr_err("Allocation failed\n");
- return;
- }
-
- ptr[size + OOB_TAG_OFF] = 'x';
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
+ KUNIT_EXPECT_KASAN_FAIL(test, ptr[size + OOB_TAG_OFF] = 'x');
kfree(ptr);
}
-static noinline void __init kmalloc_oob_left(void)
+static void kmalloc_oob_left(struct kunit *test)
{
char *ptr;
size_t size = 15;
- pr_info("out-of-bounds to left\n");
ptr = kmalloc(size, GFP_KERNEL);
- if (!ptr) {
- pr_err("Allocation failed\n");
- return;
- }
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test,
[PATCH v14 0/5] KASAN-KUnit Integration
l.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=adb72ae1915db28f934e9e02c18bfcea2f3ed3b7 [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=47227d27e2fcb01a9e8f5958d8997cf47a820afc [5] https://bugzilla.kernel.org/show_bug.cgi?id=206337 [6] https://lore.kernel.org/linux-kselftest/20200620054944.167330-1-david...@google.com/ [7] https://lkml.org/lkml/2020/7/31/571 [8] https://lore.kernel.org/linux-kselftest/8d43e88e-1356-cd63-9152-209b81b16...@linuxfoundation.org/T/#u [9] https://www.spinics.net/lists/kernel/msg3660451.html David Gow (1): mm: kasan: Do not panic if both panic_on_warn and kasan_multishot set Patricia Alfonso (4): Add KUnit Struct to Current Task KUnit: KASAN Integration KASAN: Port KASAN Tests to KUnit KASAN: Testing Documentation Documentation/dev-tools/kasan.rst | 70 +++ include/kunit/test.h | 5 + include/linux/kasan.h | 6 + include/linux/sched.h | 4 + lib/Kconfig.kasan | 22 +- lib/Makefile | 4 +- lib/kunit/test.c | 13 +- lib/test_kasan.c | 728 -- lib/test_kasan_module.c | 111 + mm/kasan/report.c | 34 +- 10 files changed, 554 insertions(+), 443 deletions(-) create mode 100644 lib/test_kasan_module.c -- 2.28.0.618.gf4bc123cb7-goog
