[PATCH 1/1] of: unittest: print pass messages at same loglevel as fail

[PATCH 1/1] of: unittest: print pass messages at same loglevel as fail

[frowand.list]

From: Frank Rowand <frank.rowand@sony.com>

Printing the devicetree unittest pass message for each passed test
creates much console verbosity.  The existing pass messages are
printed at loglevel KERN_DEBUG so they will not print by default.

Change default to print the pass messages at the same loglevel as
the fail messages.

The test community expects either a pass or a fail message for each
test in a test suite.  The messages are typically post-processed to
report pass/fail results.

Suppressing printing the pass message for each individual test is
available via the kernel command line parameter unittest.hide_pass.

Signed-off-by: Frank Rowand <frank.rowand@sony.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  4 ++++
 drivers/of/unittest.c                           | 17 ++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f5a27f067db9..045455f9b7e1 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5997,6 +5997,10 @@
 			Note that genuine overcurrent events won't be
 			reported either.
 
+	unittest.hide_pass
+			Disable printing individual drivers/of/unittest test
+			pass messages.
+
 	unknown_nmi_panic
 			[X86] Cause panic on unknown NMI.
 
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 70992103c07d..2cfbdc6b29ac 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -12,6 +12,7 @@
 #include <linux/errno.h>
 #include <linux/hashtable.h>
 #include <linux/libfdt.h>
+#include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_fdt.h>
@@ -32,6 +33,19 @@
 
 #include "of_private.h"
 
+MODULE_LICENSE("GPL v2");
+static bool hide_pass;
+
+static int __init hide_pass_setup(char *str)
+{
+	hide_pass = true;
+	return 0;
+}
+
+early_param("hide_pass", hide_pass_setup);
+module_param(hide_pass, bool, 0);
+MODULE_PARM_DESC(hide_pass, "Disable printing individual of unittest pass messages");
+
 static struct unittest_results {
 	int passed;
 	int failed;
@@ -44,7 +58,8 @@ static struct unittest_results {
 		pr_err("FAIL %s():%i " fmt, __func__, __LINE__, ##__VA_ARGS__); \
 	} else { \
 		unittest_results.passed++; \
-		pr_debug("pass %s():%i\n", __func__, __LINE__); \
+		if (!hide_pass) \
+			pr_err("pass %s():%i\n", __func__, __LINE__); \
 	} \
 	failed; \
 })
-- 
Frank Rowand <frank.rowand@sony.com>

Re: [PATCH 1/1] of: unittest: print pass messages at same loglevel as fail

[Rob Herring]

On Thu, Feb 3, 2022 at 3:12 PM <frowand.list@gmail.com> wrote:
>
> From: Frank Rowand <frank.rowand@sony.com>
>
> Printing the devicetree unittest pass message for each passed test
> creates much console verbosity.  The existing pass messages are
> printed at loglevel KERN_DEBUG so they will not print by default.
>
> Change default to print the pass messages at the same loglevel as
> the fail messages.
>
> The test community expects either a pass or a fail message for each
> test in a test suite.  The messages are typically post-processed to
> report pass/fail results.
>
> Suppressing printing the pass message for each individual test is
> available via the kernel command line parameter unittest.hide_pass.
>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>  drivers/of/unittest.c                           | 17 ++++++++++++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index f5a27f067db9..045455f9b7e1 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5997,6 +5997,10 @@
>                         Note that genuine overcurrent events won't be
>                         reported either.
>
> +       unittest.hide_pass

Can we rename the module name to include 'dt' so we're not taking a
generic name.

> +                       Disable printing individual drivers/of/unittest test
> +                       pass messages.
> +
>         unknown_nmi_panic
>                         [X86] Cause panic on unknown NMI.
>
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 70992103c07d..2cfbdc6b29ac 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -12,6 +12,7 @@
>  #include <linux/errno.h>
>  #include <linux/hashtable.h>
>  #include <linux/libfdt.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_fdt.h>
> @@ -32,6 +33,19 @@
>
>  #include "of_private.h"
>
> +MODULE_LICENSE("GPL v2");
> +static bool hide_pass;
> +
> +static int __init hide_pass_setup(char *str)
> +{
> +       hide_pass = true;
> +       return 0;
> +}
> +
> +early_param("hide_pass", hide_pass_setup);
> +module_param(hide_pass, bool, 0);
> +MODULE_PARM_DESC(hide_pass, "Disable printing individual of unittest pass messages");
> +
>  static struct unittest_results {
>         int passed;
>         int failed;
> @@ -44,7 +58,8 @@ static struct unittest_results {
>                 pr_err("FAIL %s():%i " fmt, __func__, __LINE__, ##__VA_ARGS__); \
>         } else { \
>                 unittest_results.passed++; \
> -               pr_debug("pass %s():%i\n", __func__, __LINE__); \
> +               if (!hide_pass) \
> +                       pr_err("pass %s():%i\n", __func__, __LINE__); \

Would 'PASS' be better here to align with FAIL?

If we make this info level, then you can filter with dmesg and also
seems to be aligned with what kunit does.


Rob

Re: [PATCH 1/1] of: unittest: print pass messages at same loglevel as fail

[Frank Rowand]

On 2/3/22 3:40 PM, Rob Herring wrote:
> On Thu, Feb 3, 2022 at 3:12 PM <frowand.list@gmail.com> wrote:
>>
>> From: Frank Rowand <frank.rowand@sony.com>
>>
>> Printing the devicetree unittest pass message for each passed test
>> creates much console verbosity.  The existing pass messages are
>> printed at loglevel KERN_DEBUG so they will not print by default.
>>
>> Change default to print the pass messages at the same loglevel as
>> the fail messages.
>>
>> The test community expects either a pass or a fail message for each
>> test in a test suite.  The messages are typically post-processed to
>> report pass/fail results.
>>
>> Suppressing printing the pass message for each individual test is
>> available via the kernel command line parameter unittest.hide_pass.
>>
>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>>  drivers/of/unittest.c                           | 17 ++++++++++++++++-
>>  2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index f5a27f067db9..045455f9b7e1 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -5997,6 +5997,10 @@
>>                         Note that genuine overcurrent events won't be
>>                         reported either.
>>
>> +       unittest.hide_pass
> 
> Can we rename the module name to include 'dt' so we're not taking a
> generic name.

I got most of the way through writing a reply to the various questions, then got to
the point where my answer to a specific question ended up being something to the
effect of: "this line of code (where a change was suggested) will end up being
replaced when I convert the unittest messages to KTAP format".

Then I got sidelined by going back and re-reading the KTAP specification email
thread from August, then discovering that there is also a patch submission email
thread from December where a KTAP specification is accepted into the kernel tree.

Being KTAP compliant does not allow for suppressing the individual test pass
messages, so I think I should just drop my desire to be able to do so.  That
would reduce this patch to a one line change to print the pass messages at the
same loglevel as the fail messages.  And I would prefer to not worry about
whether the pass message is 'pass' vs 'PASS' since that text will get replaced
by the KTAP syntax anyway.

Would you be ok with that one line patch?

-Frank

> 
>> +                       Disable printing individual drivers/of/unittest test
>> +                       pass messages.
>> +
>>         unknown_nmi_panic
>>                         [X86] Cause panic on unknown NMI.
>>
>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
>> index 70992103c07d..2cfbdc6b29ac 100644
>> --- a/drivers/of/unittest.c
>> +++ b/drivers/of/unittest.c
>> @@ -12,6 +12,7 @@
>>  #include <linux/errno.h>
>>  #include <linux/hashtable.h>
>>  #include <linux/libfdt.h>
>> +#include <linux/module.h>
>>  #include <linux/of.h>
>>  #include <linux/of_address.h>
>>  #include <linux/of_fdt.h>
>> @@ -32,6 +33,19 @@
>>
>>  #include "of_private.h"
>>
>> +MODULE_LICENSE("GPL v2");
>> +static bool hide_pass;
>> +
>> +static int __init hide_pass_setup(char *str)
>> +{
>> +       hide_pass = true;
>> +       return 0;
>> +}
>> +
>> +early_param("hide_pass", hide_pass_setup);
>> +module_param(hide_pass, bool, 0);
>> +MODULE_PARM_DESC(hide_pass, "Disable printing individual of unittest pass messages");
>> +
>>  static struct unittest_results {
>>         int passed;
>>         int failed;
>> @@ -44,7 +58,8 @@ static struct unittest_results {
>>                 pr_err("FAIL %s():%i " fmt, __func__, __LINE__, ##__VA_ARGS__); \
>>         } else { \
>>                 unittest_results.passed++; \
>> -               pr_debug("pass %s():%i\n", __func__, __LINE__); \
>> +               if (!hide_pass) \
>> +                       pr_err("pass %s():%i\n", __func__, __LINE__); \
> 
> Would 'PASS' be better here to align with FAIL?
> 
> If we make this info level, then you can filter with dmesg and also
> seems to be aligned with what kunit does.
> 
> 
> Rob
> 

Re: [PATCH 1/1] of: unittest: print pass messages at same loglevel as fail

[Rob Herring]

On Thu, Feb 03, 2022 at 09:41:45PM -0600, Frank Rowand wrote:
> On 2/3/22 3:40 PM, Rob Herring wrote:
> > On Thu, Feb 3, 2022 at 3:12 PM <frowand.list@gmail.com> wrote:
> >>
> >> From: Frank Rowand <frank.rowand@sony.com>
> >>
> >> Printing the devicetree unittest pass message for each passed test
> >> creates much console verbosity.  The existing pass messages are
> >> printed at loglevel KERN_DEBUG so they will not print by default.
> >>
> >> Change default to print the pass messages at the same loglevel as
> >> the fail messages.
> >>
> >> The test community expects either a pass or a fail message for each
> >> test in a test suite.  The messages are typically post-processed to
> >> report pass/fail results.
> >>
> >> Suppressing printing the pass message for each individual test is
> >> available via the kernel command line parameter unittest.hide_pass.
> >>
> >> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> >> ---
> >>  Documentation/admin-guide/kernel-parameters.txt |  4 ++++
> >>  drivers/of/unittest.c                           | 17 ++++++++++++++++-
> >>  2 files changed, 20 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> >> index f5a27f067db9..045455f9b7e1 100644
> >> --- a/Documentation/admin-guide/kernel-parameters.txt
> >> +++ b/Documentation/admin-guide/kernel-parameters.txt
> >> @@ -5997,6 +5997,10 @@
> >>                         Note that genuine overcurrent events won't be
> >>                         reported either.
> >>
> >> +       unittest.hide_pass
> > 
> > Can we rename the module name to include 'dt' so we're not taking a
> > generic name.
> 
> I got most of the way through writing a reply to the various questions, then got to
> the point where my answer to a specific question ended up being something to the
> effect of: "this line of code (where a change was suggested) will end up being
> replaced when I convert the unittest messages to KTAP format".
> 
> Then I got sidelined by going back and re-reading the KTAP specification email
> thread from August, then discovering that there is also a patch submission email
> thread from December where a KTAP specification is accepted into the kernel tree.
> 
> Being KTAP compliant does not allow for suppressing the individual test pass
> messages, so I think I should just drop my desire to be able to do so.  That
> would reduce this patch to a one line change to print the pass messages at the
> same loglevel as the fail messages.  And I would prefer to not worry about
> whether the pass message is 'pass' vs 'PASS' since that text will get replaced
> by the KTAP syntax anyway.
> 
> Would you be ok with that one line patch?

At info level, yes. If not, how soon until using ktap syntax?

Rob

Re: [PATCH 1/1] of: unittest: print pass messages at same loglevel as fail

[Frank Rowand]

On 2/9/22 6:34 PM, Rob Herring wrote:
> On Thu, Feb 03, 2022 at 09:41:45PM -0600, Frank Rowand wrote:
>> On 2/3/22 3:40 PM, Rob Herring wrote:
>>> On Thu, Feb 3, 2022 at 3:12 PM <frowand.list@gmail.com> wrote:
>>>>
>>>> From: Frank Rowand <frank.rowand@sony.com>
>>>>
>>>> Printing the devicetree unittest pass message for each passed test
>>>> creates much console verbosity.  The existing pass messages are
>>>> printed at loglevel KERN_DEBUG so they will not print by default.
>>>>
>>>> Change default to print the pass messages at the same loglevel as
>>>> the fail messages.
>>>>
>>>> The test community expects either a pass or a fail message for each
>>>> test in a test suite.  The messages are typically post-processed to
>>>> report pass/fail results.
>>>>
>>>> Suppressing printing the pass message for each individual test is
>>>> available via the kernel command line parameter unittest.hide_pass.
>>>>
>>>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>>>> ---
>>>>  Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>>>>  drivers/of/unittest.c                           | 17 ++++++++++++++++-
>>>>  2 files changed, 20 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>>> index f5a27f067db9..045455f9b7e1 100644
>>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>>> @@ -5997,6 +5997,10 @@
>>>>                         Note that genuine overcurrent events won't be
>>>>                         reported either.
>>>>
>>>> +       unittest.hide_pass
>>>
>>> Can we rename the module name to include 'dt' so we're not taking a
>>> generic name.
>>
>> I got most of the way through writing a reply to the various questions, then got to
>> the point where my answer to a specific question ended up being something to the
>> effect of: "this line of code (where a change was suggested) will end up being
>> replaced when I convert the unittest messages to KTAP format".
>>
>> Then I got sidelined by going back and re-reading the KTAP specification email
>> thread from August, then discovering that there is also a patch submission email
>> thread from December where a KTAP specification is accepted into the kernel tree.
>>
>> Being KTAP compliant does not allow for suppressing the individual test pass
>> messages, so I think I should just drop my desire to be able to do so.  That
>> would reduce this patch to a one line change to print the pass messages at the
>> same loglevel as the fail messages.  And I would prefer to not worry about
>> whether the pass message is 'pass' vs 'PASS' since that text will get replaced
>> by the KTAP syntax anyway.
>>
>> Would you be ok with that one line patch?
> 
> At info level, yes. If not, how soon until using ktap syntax?

OK, I'll redo the patch as a one liner, printing pass messages at the info
level.

The KTAP syntax is going to depend on KTAP specification version 2.  I have
a patch series against version 1 that is at version 4.  I think that series
is likely to be accepted with no further changes.

There was a big thread last summer about what to add to the KTAP
specification, but most of the suggestions (even with much agreement) did
not get into the KTAP version 1 specification that was added to the kernel
tree in December.  Once my KTAP version 1 specification clean up patches
are accepted, I will move on to pulling items from the August thread, one
item per patch series to focus discussion, to create KTAP version 2.  So
timing will be based on the review timing.  The involved parties have been
responsive, so I expect the process to move fairly quickly.

-Frank

> 
> Rob
> 

Re: [PATCH 1/1] of: unittest: print pass messages at same loglevel as fail

[Naresh Kamboju]

On Fri, 4 Feb 2022 at 02:42, <frowand.list@gmail.com> wrote:
>
> From: Frank Rowand <frank.rowand@sony.com>
>
> Printing the devicetree unittest pass message for each passed test
> creates much console verbosity.  The existing pass messages are
> printed at loglevel KERN_DEBUG so they will not print by default.
>
> Change default to print the pass messages at the same loglevel as
> the fail messages.
>
> The test community expects either a pass or a fail message for each
> test in a test suite.  The messages are typically post-processed to
> report pass/fail results.
>
> Suppressing printing the pass message for each individual test is
> available via the kernel command line parameter unittest.hide_pass.
>
> Signed-off-by: Frank Rowand <frank.rowand@sony.com>

Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>

After this patch applied I see the change in test output log
from the dmesg,

It would be great if you could add fail and skip cases.
When we find failures / regressions or when tests change from pass to fail
we will report those on the mailing list.

output:
-------
[    3.290336] ### dt-test ### start of unittest - you will see error messages
[    3.298352] ### dt-test ### EXPECT \ : Duplicate name in
testcase-data, renamed to \"duplicate-name#1\"
[    3.298535] Duplicate name in testcase-data, renamed to \"duplicate-name#1\"
[    3.318485] ### dt-test ### EXPECT / : Duplicate name in
testcase-data, renamed to \"duplicate-name#1\"
[    3.319418] ### dt-test ### pass of_unittest_check_tree_linkage():278
[    3.335123] ### dt-test ### pass of_unittest_check_tree_linkage():279
[    3.341662] ### dt-test ### pass of_unittest_check_phandles():387

Test job:
https://lkft.validation.linaro.org/scheduler/job/4473059#L1019

> ---
>  Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>  drivers/of/unittest.c                           | 17 ++++++++++++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index f5a27f067db9..045455f9b7e1 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5997,6 +5997,10 @@
>                         Note that genuine overcurrent events won't be
>                         reported either.
>
> +       unittest.hide_pass
> +                       Disable printing individual drivers/of/unittest test
> +                       pass messages.
> +
>         unknown_nmi_panic
>                         [X86] Cause panic on unknown NMI.
>
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 70992103c07d..2cfbdc6b29ac 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -12,6 +12,7 @@
>  #include <linux/errno.h>
>  #include <linux/hashtable.h>
>  #include <linux/libfdt.h>
> +#include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/of_fdt.h>
> @@ -32,6 +33,19 @@
>
>  #include "of_private.h"
>
> +MODULE_LICENSE("GPL v2");
> +static bool hide_pass;
> +
> +static int __init hide_pass_setup(char *str)
> +{
> +       hide_pass = true;
> +       return 0;
> +}
> +
> +early_param("hide_pass", hide_pass_setup);
> +module_param(hide_pass, bool, 0);
> +MODULE_PARM_DESC(hide_pass, "Disable printing individual of unittest pass messages");
> +
>  static struct unittest_results {
>         int passed;
>         int failed;
> @@ -44,7 +58,8 @@ static struct unittest_results {
>                 pr_err("FAIL %s():%i " fmt, __func__, __LINE__, ##__VA_ARGS__); \
>         } else { \
>                 unittest_results.passed++; \
> -               pr_debug("pass %s():%i\n", __func__, __LINE__); \
> +               if (!hide_pass) \
> +                       pr_err("pass %s():%i\n", __func__, __LINE__); \
>         } \
>         failed; \
>  })
> --
> Frank Rowand <frank.rowand@sony.com>


--
Linaro LKFT
https://lkft.linaro.org