From: frowand.list at gmail.com (Frank Rowand) Subject: [RFC v3 18/19] of: unittest: split out a couple of test cases from unittest Date: Thu, 14 Feb 2019 18:05:47 -0800 [thread overview] Message-ID: <0e311e88-c4d4-e98d-1720-53a04bd526fc@gmail.com> (raw) In-Reply-To: <CAFd5g444f-FBq4x3U7BL-EY+bFxP0rsJhJ14=mjOi89PhMkURg@mail.gmail.com> On 2/14/19 4:56 PM, Brendan Higgins wrote: > On Thu, Feb 14, 2019 at 3:57 PM Frank Rowand <frowand.list at gmail.com> wrote: >> >> On 12/5/18 3:54 PM, Brendan Higgins wrote: >>> On Tue, Dec 4, 2018 at 2:58 AM Frank Rowand <frowand.list at gmail.com> wrote: >>>> >>>> Hi Brendan, >>>> >>>> On 11/28/18 11:36 AM, Brendan Higgins wrote: >>>>> Split out a couple of test cases that these features in base.c from the >>>>> unittest.c monolith. The intention is that we will eventually split out >>>>> all test cases and group them together based on what portion of device >>>>> tree they test. >>>> >>>> Why does splitting this file apart improve the implementation? >>> >>> This is in preparation for patch 19/19 and other hypothetical future >>> patches where test cases are split up and grouped together by what >>> portion of DT they test (for example the parsing tests and the >>> platform/device tests would probably go separate files as well). This >>> patch by itself does not do anything useful, but I figured it made >>> patch 19/19 (and, if you like what I am doing, subsequent patches) >>> easier to review. >> >> I do not see any value in splitting the devicetree tests into >> multiple files. >> >> Please help me understand what the benefits of such a split are. Note that my following comments are specific to the current devicetree unittests, and may not apply to the general case of unit tests in other subsystems. > Sorry, I thought it made sense in context of what I am doing in the > following patch. All I am trying to do is to provide an effective way > of grouping test cases. To be clear, the idea, assuming you agree, is Looking at _just_ the first few fragments of the following patch, the change is to break down a moderate size function of related tests, of_unittest_find_node_by_name(), into a lot of extremely small functions. Then to find the execution order of the many small functions requires finding the array of_test_find_node_by_name_cases[]. Then I have to chase off into the kunit test runner core, where I find that the set of tests in of_test_find_node_by_name_cases[] is processed by a late_initcall(). So now the order of the various test groupings, declared via module_test(), are subject to the fragile orderings of initcalls. There are ordering dependencies within the devicetree unittests. I do not like breaking the test cases down into such small atoms. I do not see any value __for devicetree unittests__ of having such small atoms. It makes it harder for me to read the source of the tests and understand the order they will execute. It also makes it harder for me to read through the actual tests (in this example the tests that are currently grouped in of_unittest_find_node_by_name()) because of all the extra function headers injected into the existing single function to break it apart into many smaller functions. Breaking the tests into separate chunks, each chunk invoked independently as the result of module_test() of each chunk, loses the summary result for the devicetree unittests of how many tests are run and how many passed. This is the only statistic that I need to determine whether the unittests have detected a new fault caused by a specific patch or commit. I don't need to look at any individual test result unless the overall result reports a failure. > that we would follow up with several other patches like this one and > the subsequent patch, one which would pull out a couple test > functions, as I have done here, and another that splits those > functions up into a bunch of proper test cases. > > I thought that having that many unrelated test cases in a single file > would just be a pain to sort through deal with, review, whatever. Having all the test cases in a single file makes it easier for me to read, understand, modify, and maintain the tests. > This is not something I feel particularly strongly about, it is just > pretty atypical from my experience to have so many unrelated test > cases in a single file. > > Maybe you would prefer that I break up the test cases first, and then > we split up the file as appropriate? I prefer that the test cases not be broken up arbitrarily. There _may_ be cases where the devicetree unittests are currently not well grouped and may benefit from change, but if so that should be handled independently of any transformation into a KUnit framework. > I just assumed that we would agree it would be way too much stuff for > a single file, so I went ahead and broke it up first, because I > thought it would make it easier to review in that order rather than > the other way around.
WARNING: multiple messages have this Message-ID (diff)
From: frowand.list@gmail.com (Frank Rowand) Subject: [RFC v3 18/19] of: unittest: split out a couple of test cases from unittest Date: Thu, 14 Feb 2019 18:05:47 -0800 [thread overview] Message-ID: <0e311e88-c4d4-e98d-1720-53a04bd526fc@gmail.com> (raw) Message-ID: <20190215020547.8t0lhuFsWTb6vPKI_LGeOvi0vRYBRpvDxGdsqAmr63Y@z> (raw) In-Reply-To: <CAFd5g444f-FBq4x3U7BL-EY+bFxP0rsJhJ14=mjOi89PhMkURg@mail.gmail.com> On 2/14/19 4:56 PM, Brendan Higgins wrote: > On Thu, Feb 14, 2019@3:57 PM Frank Rowand <frowand.list@gmail.com> wrote: >> >> On 12/5/18 3:54 PM, Brendan Higgins wrote: >>> On Tue, Dec 4, 2018@2:58 AM Frank Rowand <frowand.list@gmail.com> wrote: >>>> >>>> Hi Brendan, >>>> >>>> On 11/28/18 11:36 AM, Brendan Higgins wrote: >>>>> Split out a couple of test cases that these features in base.c from the >>>>> unittest.c monolith. The intention is that we will eventually split out >>>>> all test cases and group them together based on what portion of device >>>>> tree they test. >>>> >>>> Why does splitting this file apart improve the implementation? >>> >>> This is in preparation for patch 19/19 and other hypothetical future >>> patches where test cases are split up and grouped together by what >>> portion of DT they test (for example the parsing tests and the >>> platform/device tests would probably go separate files as well). This >>> patch by itself does not do anything useful, but I figured it made >>> patch 19/19 (and, if you like what I am doing, subsequent patches) >>> easier to review. >> >> I do not see any value in splitting the devicetree tests into >> multiple files. >> >> Please help me understand what the benefits of such a split are. Note that my following comments are specific to the current devicetree unittests, and may not apply to the general case of unit tests in other subsystems. > Sorry, I thought it made sense in context of what I am doing in the > following patch. All I am trying to do is to provide an effective way > of grouping test cases. To be clear, the idea, assuming you agree, is Looking at _just_ the first few fragments of the following patch, the change is to break down a moderate size function of related tests, of_unittest_find_node_by_name(), into a lot of extremely small functions. Then to find the execution order of the many small functions requires finding the array of_test_find_node_by_name_cases[]. Then I have to chase off into the kunit test runner core, where I find that the set of tests in of_test_find_node_by_name_cases[] is processed by a late_initcall(). So now the order of the various test groupings, declared via module_test(), are subject to the fragile orderings of initcalls. There are ordering dependencies within the devicetree unittests. I do not like breaking the test cases down into such small atoms. I do not see any value __for devicetree unittests__ of having such small atoms. It makes it harder for me to read the source of the tests and understand the order they will execute. It also makes it harder for me to read through the actual tests (in this example the tests that are currently grouped in of_unittest_find_node_by_name()) because of all the extra function headers injected into the existing single function to break it apart into many smaller functions. Breaking the tests into separate chunks, each chunk invoked independently as the result of module_test() of each chunk, loses the summary result for the devicetree unittests of how many tests are run and how many passed. This is the only statistic that I need to determine whether the unittests have detected a new fault caused by a specific patch or commit. I don't need to look at any individual test result unless the overall result reports a failure. > that we would follow up with several other patches like this one and > the subsequent patch, one which would pull out a couple test > functions, as I have done here, and another that splits those > functions up into a bunch of proper test cases. > > I thought that having that many unrelated test cases in a single file > would just be a pain to sort through deal with, review, whatever. Having all the test cases in a single file makes it easier for me to read, understand, modify, and maintain the tests. > This is not something I feel particularly strongly about, it is just > pretty atypical from my experience to have so many unrelated test > cases in a single file. > > Maybe you would prefer that I break up the test cases first, and then > we split up the file as appropriate? I prefer that the test cases not be broken up arbitrarily. There _may_ be cases where the devicetree unittests are currently not well grouped and may benefit from change, but if so that should be handled independently of any transformation into a KUnit framework. > I just assumed that we would agree it would be way too much stuff for > a single file, so I went ahead and broke it up first, because I > thought it would make it easier to review in that order rather than > the other way around.
next prev parent reply other threads:[~2019-02-15 2:05 UTC|newest] Thread overview: 232+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-28 19:36 [RFC v3 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 01/19] kunit: test: add KUnit test runner core brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-30 3:14 ` mcgrof 2018-11-30 3:14 ` Luis Chamberlain 2018-12-01 1:51 ` brendanhiggins 2018-12-01 1:51 ` Brendan Higgins 2018-12-01 2:57 ` mcgrof 2018-12-01 2:57 ` Luis Chamberlain 2018-12-05 13:15 ` anton.ivanov 2018-12-05 13:15 ` Anton Ivanov 2018-12-05 14:45 ` arnd 2018-12-05 14:45 ` Arnd Bergmann 2018-12-05 14:49 ` anton.ivanov 2018-12-05 14:49 ` Anton Ivanov 2018-11-30 3:28 ` mcgrof 2018-11-30 3:28 ` Luis Chamberlain 2018-12-01 2:08 ` brendanhiggins 2018-12-01 2:08 ` Brendan Higgins 2018-12-01 3:10 ` mcgrof 2018-12-01 3:10 ` Luis Chamberlain 2018-12-03 22:47 ` brendanhiggins 2018-12-03 22:47 ` Brendan Higgins 2018-12-01 3:02 ` mcgrof 2018-12-01 3:02 ` Luis Chamberlain 2018-11-28 19:36 ` [RFC v3 02/19] kunit: test: add test resource management API brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 03/19] kunit: test: add string_stream a std::stream like string builder brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-30 3:29 ` mcgrof 2018-11-30 3:29 ` Luis Chamberlain 2018-12-01 2:14 ` brendanhiggins 2018-12-01 2:14 ` Brendan Higgins 2018-12-01 3:12 ` mcgrof 2018-12-01 3:12 ` Luis Chamberlain 2018-12-03 10:55 ` pmladek 2018-12-03 10:55 ` Petr Mladek 2018-12-04 0:35 ` brendanhiggins 2018-12-04 0:35 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 04/19] kunit: test: add test_stream a std::stream like logger brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 05/19] kunit: test: add the concept of expectations brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 06/19] arch: um: enable running kunit from User Mode Linux brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 21:26 ` robh 2018-11-28 21:26 ` Rob Herring 2018-11-30 3:37 ` mcgrof 2018-11-30 3:37 ` Luis Chamberlain 2018-11-30 14:05 ` robh 2018-11-30 14:05 ` Rob Herring 2018-11-30 18:22 ` mcgrof 2018-11-30 18:22 ` Luis Chamberlain 2018-12-03 23:22 ` brendanhiggins 2018-12-03 23:22 ` Brendan Higgins 2018-11-30 3:30 ` mcgrof 2018-11-30 3:30 ` Luis Chamberlain 2018-11-28 19:36 ` [RFC v3 07/19] kunit: test: add initial tests brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-30 3:40 ` mcgrof 2018-11-30 3:40 ` Luis Chamberlain 2018-12-03 23:26 ` brendanhiggins 2018-12-03 23:26 ` Brendan Higgins 2018-12-03 23:43 ` mcgrof 2018-12-03 23:43 ` Luis Chamberlain 2018-11-28 19:36 ` [RFC v3 08/19] arch: um: add shim to trap to allow installing a fault catcher for tests brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-30 3:34 ` mcgrof 2018-11-30 3:34 ` Luis Chamberlain 2018-12-03 23:34 ` brendanhiggins 2018-12-03 23:34 ` Brendan Higgins 2018-12-03 23:46 ` mcgrof 2018-12-03 23:46 ` Luis Chamberlain 2018-12-04 0:44 ` brendanhiggins 2018-12-04 0:44 ` Brendan Higgins 2018-11-30 3:41 ` mcgrof 2018-11-30 3:41 ` Luis Chamberlain 2018-12-03 23:37 ` brendanhiggins 2018-12-03 23:37 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 09/19] kunit: test: add the concept of assertions brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 10/19] kunit: test: add test managed resource tests brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 11/19] kunit: add Python libraries for handing KUnit config and kernel brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-29 13:54 ` kieran.bingham 2018-11-29 13:54 ` Kieran Bingham 2018-12-03 23:48 ` brendanhiggins 2018-12-03 23:48 ` Brendan Higgins 2018-12-04 20:47 ` mcgrof 2018-12-04 20:47 ` Luis Chamberlain 2018-12-06 12:32 ` kieran.bingham 2018-12-06 12:32 ` Kieran Bingham 2018-12-06 15:37 ` willy 2018-12-06 15:37 ` Matthew Wilcox 2018-12-07 11:30 ` kieran.bingham 2018-12-07 11:30 ` Kieran Bingham 2018-12-11 14:09 ` pmladek 2018-12-11 14:09 ` Petr Mladek 2018-12-11 14:41 ` rostedt 2018-12-11 14:41 ` Steven Rostedt 2018-12-11 17:01 ` anton.ivanov 2018-12-11 17:01 ` Anton Ivanov 2019-02-09 0:40 ` brendanhiggins 2019-02-09 0:40 ` Brendan Higgins 2018-12-07 1:05 ` mcgrof 2018-12-07 1:05 ` Luis Chamberlain 2018-12-07 18:35 ` kent.overstreet 2018-12-07 18:35 ` Kent Overstreet 2018-11-30 3:44 ` mcgrof 2018-11-30 3:44 ` Luis Chamberlain 2018-12-03 23:50 ` brendanhiggins 2018-12-03 23:50 ` Brendan Higgins 2018-12-04 20:48 ` mcgrof 2018-12-04 20:48 ` Luis Chamberlain 2018-11-28 19:36 ` [RFC v3 12/19] kunit: add KUnit wrapper script and simple output parser brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 13/19] kunit: improve output from python wrapper brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 14/19] Documentation: kunit: add documentation for KUnit brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-29 13:56 ` kieran.bingham 2018-11-29 13:56 ` Kieran Bingham 2018-11-30 3:45 ` mcgrof 2018-11-30 3:45 ` Luis Chamberlain 2018-12-03 23:53 ` brendanhiggins 2018-12-03 23:53 ` Brendan Higgins 2018-12-06 12:16 ` kieran.bingham 2018-12-06 12:16 ` Kieran Bingham 2019-02-09 0:56 ` brendanhiggins 2019-02-09 0:56 ` Brendan Higgins 2019-02-11 12:16 ` kieran.bingham 2019-02-11 12:16 ` Kieran Bingham 2019-02-12 22:10 ` brendanhiggins 2019-02-12 22:10 ` Brendan Higgins 2019-02-13 21:55 ` kieran.bingham 2019-02-13 21:55 ` Kieran Bingham 2019-02-14 0:17 ` brendanhiggins 2019-02-14 0:17 ` Brendan Higgins 2019-02-14 17:26 ` mcgrof 2019-02-14 17:26 ` Luis Chamberlain 2019-02-14 22:07 ` brendanhiggins 2019-02-14 22:07 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 15/19] MAINTAINERS: add entry for KUnit the unit testing framework brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 16/19] arch: um: make UML unflatten device tree when testing brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-11-28 21:16 ` robh 2018-11-28 21:16 ` Rob Herring 2018-12-04 0:00 ` brendanhiggins 2018-12-04 0:00 ` Brendan Higgins 2018-11-30 3:46 ` mcgrof 2018-11-30 3:46 ` Luis Chamberlain 2018-12-04 0:02 ` brendanhiggins 2018-12-04 0:02 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 17/19] of: unittest: migrate tests to run on KUnit brendanhiggins 2018-11-28 19:36 ` Brendan Higgins [not found] ` <CAL_Jsq+09Kx7yMBC_Jw45QGmk6U_fp4N6HOZDwYrM4tWw+_dOA@mail.gmail.com> 2018-11-30 0:39 ` rdunlap 2018-11-30 0:39 ` Randy Dunlap 2018-12-04 0:13 ` brendanhiggins 2018-12-04 0:13 ` Brendan Higgins 2018-12-04 13:40 ` robh 2018-12-04 13:40 ` Rob Herring 2018-12-05 23:42 ` brendanhiggins 2018-12-05 23:42 ` Brendan Higgins 2018-12-07 0:41 ` robh 2018-12-07 0:41 ` Rob Herring 2018-12-04 0:08 ` brendanhiggins 2018-12-04 0:08 ` Brendan Higgins 2019-02-13 1:44 ` brendanhiggins 2019-02-13 1:44 ` Brendan Higgins 2019-02-14 20:10 ` robh 2019-02-14 20:10 ` Rob Herring 2019-02-14 21:52 ` brendanhiggins 2019-02-14 21:52 ` Brendan Higgins 2019-02-18 22:56 ` frowand.list 2019-02-18 22:56 ` Frank Rowand 2019-02-28 0:29 ` brendanhiggins 2019-02-28 0:29 ` Brendan Higgins 2018-12-04 10:56 ` frowand.list 2018-12-04 10:56 ` Frank Rowand 2018-11-28 19:36 ` [RFC v3 18/19] of: unittest: split out a couple of test cases from unittest brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-12-04 10:58 ` frowand.list 2018-12-04 10:58 ` Frank Rowand 2018-12-05 23:54 ` brendanhiggins 2018-12-05 23:54 ` Brendan Higgins 2019-02-14 23:57 ` frowand.list 2019-02-14 23:57 ` Frank Rowand 2019-02-15 0:56 ` brendanhiggins 2019-02-15 0:56 ` Brendan Higgins 2019-02-15 2:05 ` frowand.list [this message] 2019-02-15 2:05 ` Frank Rowand 2019-02-15 10:56 ` brendanhiggins 2019-02-15 10:56 ` Brendan Higgins 2019-02-18 22:25 ` frowand.list 2019-02-18 22:25 ` Frank Rowand 2019-02-20 20:44 ` frowand.list 2019-02-20 20:44 ` Frank Rowand 2019-02-20 20:47 ` frowand.list 2019-02-20 20:47 ` Frank Rowand 2019-02-28 3:52 ` brendanhiggins 2019-02-28 3:52 ` Brendan Higgins 2019-03-22 0:22 ` frowand.list 2019-03-22 0:22 ` Frank Rowand 2019-03-22 1:30 ` brendanhiggins 2019-03-22 1:30 ` Brendan Higgins 2019-03-22 1:47 ` frowand.list 2019-03-22 1:47 ` Frank Rowand 2019-03-25 22:15 ` brendanhiggins 2019-03-25 22:15 ` Brendan Higgins 2019-09-20 16:57 ` Rob Herring 2019-09-21 23:57 ` Frank Rowand 2019-03-22 1:34 ` frowand.list 2019-03-22 1:34 ` Frank Rowand 2019-03-25 22:18 ` brendanhiggins 2019-03-25 22:18 ` Brendan Higgins 2018-11-28 19:36 ` [RFC v3 19/19] of: unittest: split up some super large test cases brendanhiggins 2018-11-28 19:36 ` Brendan Higgins 2018-12-04 10:52 ` [RFC v3 00/19] kunit: introduce KUnit, the Linux kernel unit testing framework frowand.list 2018-12-04 10:52 ` Frank Rowand 2018-12-04 11:40 ` frowand.list 2018-12-04 11:40 ` Frank Rowand 2018-12-04 13:49 ` robh 2018-12-04 13:49 ` Rob Herring 2018-12-05 23:10 ` brendanhiggins 2018-12-05 23:10 ` Brendan Higgins 2019-03-22 0:27 ` frowand.list 2019-03-22 0:27 ` Frank Rowand 2019-03-25 22:04 ` brendanhiggins 2019-03-25 22:04 ` Brendan Higgins
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=0e311e88-c4d4-e98d-1720-53a04bd526fc@gmail.com \ --to=linux-kselftest@vger.kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).