patchtest: log errors and failures at end

From: Simone Weiß <simone.p.weiss@posteo.com>

At the moment, running patchtest locally will only print failures and errors
to the log when the not passing test case is executed. This might lead to
people overlooking issues with their patches, so print a log line at the
end if testcases showed issues. This should make it more easy to spot then
before.

Fixes [YOCTO #15389]

Signed-off-by: Simone Weiß <simone.p.weiss@posteo.com>
---
 scripts/patchtest | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

On 2024-02-13 15:52, Simone Weiß wrote:
> From: Simone Weiß <simone.p.weiss@posteo.com>
>
> At the moment, running patchtest locally will only print failures and errors
> to the log when the not passing test case is executed. This might lead to
> people overlooking issues with their patches, so print a log line at the
> end if testcases showed issues. This should make it more easy to spot then
> before.
>
> Fixes [YOCTO #15389]

This is a good idea, but it makes the log look messier than I expected 
(here's an example where I'm testing against a selftest you added recently):

Summary: There was 1 WARNING message.
FAIL: test CVE check ignore: CVE_CHECK_IGNORE is deprecated and should 
be replaced by CVE_STATUS (test_metadata.TestMetadata.test_cve_check_ignore)
PASS: test lic files chksum modified not mentioned 
(test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned)
SKIP: test lic files chksum presence: No added recipes, skipping test 
(test_metadata.TestMetadata.test_lic_files_chksum_presence)
SKIP: test license presence: No added recipes, skipping test 
(test_metadata.TestMetadata.test_license_presence)
PASS: test max line length (test_metadata.TestMetadata.test_max_line_length)
PASS: test src uri left files 
(test_metadata.TestMetadata.test_src_uri_left_files)
SKIP: test summary presence: No added recipes, skipping test 
(test_metadata.TestMetadata.test_summary_presence)
SKIP: test CVE tag format: No new CVE patches introduced 
(test_patch.TestPatch.test_cve_tag_format)
SKIP: test Signed-off-by presence: No new CVE patches introduced 
(test_patch.TestPatch.test_signed_off_by_presence)
SKIP: test Upstream-Status presence: No new CVE patches introduced 
(test_patch.TestPatch.test_upstream_status_presence_format)
SKIP: test pylint: No python related patches, skipping test 
(test_python_pylint.PyLint.test_pylint)
----------------------------------------------------------------------
Ran 21 tests in 20.416s

OK
----------------------------------------------------------------------

FAIL: patchtest: At least one patchtest caused a failure or an error - 
please check
----------------------------------------------------------------------


The use of "FAIL" also means that patchtest-send-results will pick it up 
and put it in the list of failures, even though it's a warning about 
failures.

I wonder if it'd be better here to either use "WARNING:" instead of 
"FAIL:", or just shorten it to "patchtest: At least one patchtest caused 
a failure or an error - please check" since it will only be for the 
local case?

>
> Signed-off-by: Simone Weiß <simone.p.weiss@posteo.com>
> ---
>   scripts/patchtest | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/patchtest b/scripts/patchtest
> index a1c824f7b7..7d4d4f51d3 100755
> --- a/scripts/patchtest
> +++ b/scripts/patchtest
> @@ -142,6 +142,8 @@ def _runner(resultklass, prefix=None):
>           logger.error(traceback.print_exc())
>           logger.error('patchtest: something went wrong')
>           return 1
> +    if result.test_failure or result.test_error:
> +        return 1
>   
>       return 0
>   
> @@ -158,9 +160,14 @@ def run(patch, logfile=None):
>       postmerge_resultklass = getResult(patch, True, logfile)
>       postmerge_result = _runner(postmerge_resultklass, 'test')
>   
> +    print('----------------------------------------------------------------------\n')
>       if premerge_result == 2 and postmerge_result == 2:
> -        logger.error('patchtest: any test cases found - did you specify the correct suite directory?')
> -
> +        logger.error('patchtest: Not any test cases found - did you specify the correct suite directory?')
> +    if premerge_result == 1 or postmerge_result == 1:
> +        logger.error('FAIL: patchtest: At least one patchtest caused a failure or an error - please check')
> +    else:
> +        logger.error('PASS: patchtest: All patchtests passed')
> +    print('----------------------------------------------------------------------\n')
>       return premerge_result or postmerge_result
>   
>   def main():
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#195428): https://lists.openembedded.org/g/openembedded-core/message/195428
> Mute This Topic: https://lists.openembedded.org/mt/104341021/7611679
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [tgamblin@baylibre.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>