| Message ID | 20200130230812.142642-1-brendanhiggins@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | kunit: create a centralized executor to dispatch all KUnit tests | expand |
Quoting Brendan Higgins (2020-01-30 15:08:05) > ## TL;DR > > This patchset adds a centralized executor to dispatch tests rather than > relying on late_initcall to schedule each test suite separately along > with a couple of new features that depend on it. Is there any diff from v1 to v2? I don't know what changed, but I see that my Reviewed-by tag has been put on everything, so I guess everything I said was addressed or discussed in the previous round.
On Mon, Feb 3, 2020 at 11:19 PM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Brendan Higgins (2020-01-30 15:08:05) > > ## TL;DR > > > > This patchset adds a centralized executor to dispatch tests rather than > > relying on late_initcall to schedule each test suite separately along > > with a couple of new features that depend on it. > > Is there any diff from v1 to v2? I don't know what changed, but I see > that my Reviewed-by tag has been put on everything, so I guess > everything I said was addressed or discussed in the previous round. Oh yes, sorry about that. I have gotten a bit lazy in regard to changing logs. I noticed that a lot of people don't seem to care. I'll make a note that you do. Changes since last revision: - On patch 6/7, I flipped the include order and removed braces from the if statements. - On patch 7/7, I removed the periods from the short descriptions.
On 1/30/20 5:08 PM, Brendan Higgins wrote: > ## TL;DR > > This patchset adds a centralized executor to dispatch tests rather than > relying on late_initcall to schedule each test suite separately along > with a couple of new features that depend on it. And the "couple of new features" are .... ? > > ## What am I trying to do? > > Conceptually, I am trying to provide a mechanism by which test suites > can be grouped together so that they can be reasoned about collectively. > The last two of three patches in this series add features which depend > on this: > > PATCH 5/7 Prints out a test plan right before KUnit tests are run[1]; > this is valuable because it makes it possible for a test > harness to detect whether the number of tests run matches the > number of tests expected to be run, ensuring that no tests > silently failed. > > PATCH 6/7 Add a new kernel command-line option which allows the user to > specify that the kernel poweroff, halt, or reboot after > completing all KUnit tests; this is very handy for running > KUnit tests on UML or a VM so that the UML/VM process exits > cleanly immediately after running all tests without needing a > special initramfs. > > In addition, by dispatching tests from a single location, we can > guarantee that all KUnit tests run after late_init is complete, which That the tests will run after late init (and are guaranteed to do such) needs to be added to the documentation. -Frank > was a concern during the initial KUnit patchset review (this has not > been a problem in practice, but resolving with certainty is nevertheless > desirable). > > Other use cases for this exist, but the above features should provide an > idea of the value that this could provide. > > Alan Maguire (1): > kunit: test: create a single centralized executor for all tests > > Brendan Higgins (5): > vmlinux.lds.h: add linker section for KUnit test suites > arch: um: add linker section for KUnit test suites > init: main: add KUnit to kernel init > kunit: test: add test plan to KUnit TAP format > Documentation: Add kunit_shutdown to kernel-parameters.txt > > David Gow (1): > kunit: Add 'kunit_shutdown' option > > .../admin-guide/kernel-parameters.txt | 7 ++ > arch/um/include/asm/common.lds.S | 4 + > include/asm-generic/vmlinux.lds.h | 8 ++ > include/kunit/test.h | 82 ++++++++++++------- > init/main.c | 4 + > lib/kunit/Makefile | 3 +- > lib/kunit/executor.c | 71 ++++++++++++++++ > lib/kunit/test.c | 11 --- > tools/testing/kunit/kunit_kernel.py | 2 +- > tools/testing/kunit/kunit_parser.py | 76 ++++++++++++++--- > .../test_is_test_passed-all_passed.log | 1 + > .../test_data/test_is_test_passed-crash.log | 1 + > .../test_data/test_is_test_passed-failure.log | 1 + > 13 files changed, 217 insertions(+), 54 deletions(-) > create mode 100644 lib/kunit/executor.c >
On Tue, Feb 4, 2020 at 1:18 PM Frank Rowand <frowand.list@gmail.com> wrote: > > On 1/30/20 5:08 PM, Brendan Higgins wrote: > > ## TL;DR > > > > This patchset adds a centralized executor to dispatch tests rather than > > relying on late_initcall to schedule each test suite separately along > > with a couple of new features that depend on it. > > And the "couple of new features" are .... ? Sorry, I thought I addressed that in the next section, specifically by calling out: printing the test plan, and adding the new kernel command line option. I will call these out here in any future cover letters. > > ## What am I trying to do? > > > > Conceptually, I am trying to provide a mechanism by which test suites > > can be grouped together so that they can be reasoned about collectively. > > The last two of three patches in this series add features which depend > > on this: > > > > PATCH 5/7 Prints out a test plan right before KUnit tests are run[1]; > > this is valuable because it makes it possible for a test > > harness to detect whether the number of tests run matches the > > number of tests expected to be run, ensuring that no tests > > silently failed. > > > > PATCH 6/7 Add a new kernel command-line option which allows the user to > > specify that the kernel poweroff, halt, or reboot after > > completing all KUnit tests; this is very handy for running > > KUnit tests on UML or a VM so that the UML/VM process exits > > cleanly immediately after running all tests without needing a > > special initramfs. > > > > > In addition, by dispatching tests from a single location, we can > > guarantee that all KUnit tests run after late_init is complete, which > > That the tests will run after late init (and are guaranteed to do such) > needs to be added to the documentation. Yeah, that's reasonable. I am not sure where I should put this in the documentation, however. This seems kind of a technical detail, and all the pages I have now are more of how-tos, I think. Maybe I should send a patch which adds a page detailing how KUnit works? I would like to get some other people's thoughts on this. Such a technical guide wouldn't provide me a lot of value, at least not now, so I want to make sure that something like that would be valuable to others.
On 2/4/20 1:35 PM, Brendan Higgins wrote: > On Mon, Feb 3, 2020 at 11:19 PM Stephen Boyd <sboyd@kernel.org> wrote: >> >> Quoting Brendan Higgins (2020-01-30 15:08:05) >>> ## TL;DR >>> >>> This patchset adds a centralized executor to dispatch tests rather than >>> relying on late_initcall to schedule each test suite separately along >>> with a couple of new features that depend on it. >> >> Is there any diff from v1 to v2? I don't know what changed, but I see >> that my Reviewed-by tag has been put on everything, so I guess >> everything I said was addressed or discussed in the previous round. > > Oh yes, sorry about that. I have gotten a bit lazy in regard to > changing logs. I noticed that a lot of people don't seem to care. I'll > make a note that you do. Please ignore those who don't care. Just always include a change log. You may encounter bike shedding about where the log belongs (in patch 0, in the modified patches, in both locations). The color of my bike shed is simply that they exist somewhere, but my most favorite color is both places. > > Changes since last revision: > - On patch 6/7, I flipped the include order and removed braces from the if > statements. > - On patch 7/7, I removed the periods from the short descriptions. >
## TL;DR This patchset adds a centralized executor to dispatch tests rather than relying on late_initcall to schedule each test suite separately along with a couple of new features that depend on it. ## What am I trying to do? Conceptually, I am trying to provide a mechanism by which test suites can be grouped together so that they can be reasoned about collectively. The last two of three patches in this series add features which depend on this: PATCH 5/7 Prints out a test plan right before KUnit tests are run[1]; this is valuable because it makes it possible for a test harness to detect whether the number of tests run matches the number of tests expected to be run, ensuring that no tests silently failed. PATCH 6/7 Add a new kernel command-line option which allows the user to specify that the kernel poweroff, halt, or reboot after completing all KUnit tests; this is very handy for running KUnit tests on UML or a VM so that the UML/VM process exits cleanly immediately after running all tests without needing a special initramfs. In addition, by dispatching tests from a single location, we can guarantee that all KUnit tests run after late_init is complete, which was a concern during the initial KUnit patchset review (this has not been a problem in practice, but resolving with certainty is nevertheless desirable). Other use cases for this exist, but the above features should provide an idea of the value that this could provide. Alan Maguire (1): kunit: test: create a single centralized executor for all tests Brendan Higgins (5): vmlinux.lds.h: add linker section for KUnit test suites arch: um: add linker section for KUnit test suites init: main: add KUnit to kernel init kunit: test: add test plan to KUnit TAP format Documentation: Add kunit_shutdown to kernel-parameters.txt David Gow (1): kunit: Add 'kunit_shutdown' option .../admin-guide/kernel-parameters.txt | 7 ++ arch/um/include/asm/common.lds.S | 4 + include/asm-generic/vmlinux.lds.h | 8 ++ include/kunit/test.h | 82 ++++++++++++------- init/main.c | 4 + lib/kunit/Makefile | 3 +- lib/kunit/executor.c | 71 ++++++++++++++++ lib/kunit/test.c | 11 --- tools/testing/kunit/kunit_kernel.py | 2 +- tools/testing/kunit/kunit_parser.py | 76 ++++++++++++++--- .../test_is_test_passed-all_passed.log | 1 + .../test_data/test_is_test_passed-crash.log | 1 + .../test_data/test_is_test_passed-failure.log | 1 + 13 files changed, 217 insertions(+), 54 deletions(-) create mode 100644 lib/kunit/executor.c