[OE-Core][PATCH 0/2] Add a display limit for regression report generation

[OE-Core][PATCH 0/2] Add a display limit for regression report generation

[Alexis Lothoré]

It has been observed that useful information in regression report can be
drowned in huge regression lists which are often false-positives (for
example, a whole set of tests has been temporarily disabled).

This series brings a default limit to how many changes are displayed per
base/target comparison. This default can still be overriden on commandline,
for example to have a better look at the whole regression list when trying
to debug an issue (i.e. by disabling the limit)

First commit implement the limit, its default value and the corresponding
commandline option in resulttool.
Second commit allow yocto_testresults_query.py to drive this value.

As a result, one can for example do the following:
- yocto_testresults_query 4.3_M1 4.3_M2
  -> will display at most 50 regressions per test
- yocto_testresults_query -l 10 4.3_M1 4.3_M2
  -> override the display limit and reduce it to 10 regressions per pair.
- yocto_testresults_query -l 0 4.3_M1 4.3_M2
  -> disable the display limit, print all regressions

An example of regression report with display limit can be found here:
https://pastebin.com/6QbfGstR

Alexis Lothoré (2):
  scripts/resulttool: limit the number of changes displayed per test
  scripts/yocto_testresults_query: add option to change display limit

 scripts/lib/resulttool/regression.py | 23 +++++++++++++++++++----
 scripts/yocto_testresults_query.py   | 13 ++++++++++---
 2 files changed, 29 insertions(+), 7 deletions(-)

-- 
2.42.0

[OE-Core][PATCH 1/2] scripts/resulttool: limit the number of changes displayed per test

[Alexis Lothoré]

From: Alexis Lothoré <alexis.lothore@bootlin.com>

Most of the changes list generated in regression reports fall in one
of the two following categories:
- there is only a few (<10) changes listed and the info is
  valuable/relevant
- the list is huge (> 100 ? 1000 ?) and basically tells us that the whole
  tests category suffers the same status (test missing, test failing, test
  skipped, etc)

Prevent those huge, worthless lists by limiting the output for each test
result pair:
- current default limit is arbitrarily set to 50
- limit can still be overriden with a new "-l"/"--limit" flag, either with
  custom value, or with 0 to print the whole lists of changes

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 scripts/lib/resulttool/regression.py | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/scripts/lib/resulttool/regression.py b/scripts/lib/resulttool/regression.py
index 3d64b8f4af7c..5c5ed6e6a670 100644
--- a/scripts/lib/resulttool/regression.py
+++ b/scripts/lib/resulttool/regression.py
@@ -78,6 +78,8 @@ STATUS_STRINGS = {
     "None": "No matching test result"
 }
 
+REGRESSIONS_DISPLAY_LIMIT=50
+
 def test_has_at_least_one_matching_tag(test, tag_list):
     return "oetags" in test and any(oetag in tag_list for oetag in test["oetags"])
 
@@ -181,11 +183,12 @@ def get_status_str(raw_status):
     raw_status_lower = raw_status.lower() if raw_status else "None"
     return STATUS_STRINGS.get(raw_status_lower, raw_status)
 
-def compare_result(logger, base_name, target_name, base_result, target_result):
+def compare_result(logger, base_name, target_name, base_result, target_result, display_limit):
     base_result = base_result.get('result')
     target_result = target_result.get('result')
     result = {}
     new_tests = 0
+    regressions_count = 0
 
     if base_result and target_result:
         for k in base_result:
@@ -212,7 +215,14 @@ def compare_result(logger, base_name, target_name, base_result, target_result):
             resultstring = "Regression:  %s\n             %s\n" % (base_name, target_name)
             for k in sorted(result):
                 if not result[k]['target'] or not result[k]['target'].startswith("PASS"):
-                    resultstring += '    %s: %s -> %s\n' % (k, get_status_str(result[k]['base']), get_status_str(result[k]['target']))
+                    # Count regressions only if we have to limit the number of
+                    # displayed regressions
+                    if display_limit > 0:
+                        regressions_count = regressions_count + 1
+                    if regressions_count <= display_limit:
+                        resultstring += '    %s: %s -> %s\n' % (k, get_status_str(result[k]['base']), get_status_str(result[k]['target']))
+            if regressions_count > display_limit:
+                resultstring += f'    [...]\n    (In total, {regressions_count} regressions/status changes detected)\n'
             if new_pass_count > 0:
                 resultstring += f'    Additionally, {new_pass_count} previously failing test(s) is/are now passing\n'
         else:
@@ -263,6 +273,10 @@ def regression_common(args, logger, base_results, target_results):
     if args.target_result_id:
         target_results = resultutils.filter_resultsdata(target_results, args.target_result_id)
 
+    display_limit=REGRESSIONS_DISPLAY_LIMIT
+    if args.limit:
+        display_limit=int(args.limit)
+
     fixup_ptest_names(base_results, logger)
     fixup_ptest_names(target_results, logger)
 
@@ -280,7 +294,7 @@ def regression_common(args, logger, base_results, target_results):
                 for b in target.copy():
                     if not can_be_compared(logger, base_results[a][c], target_results[a][b]):
                         continue
-                    res, resstr = compare_result(logger, c, b, base_results[a][c], target_results[a][b])
+                    res, resstr = compare_result(logger, c, b, base_results[a][c], target_results[a][b], display_limit)
                     if not res:
                         matches.append(resstr)
                         base.remove(c)
@@ -291,7 +305,7 @@ def regression_common(args, logger, base_results, target_results):
                 for b in target:
                     if not can_be_compared(logger, base_results[a][c], target_results[a][b]):
                         continue
-                    res, resstr = compare_result(logger, c, b, base_results[a][c], target_results[a][b])
+                    res, resstr = compare_result(logger, c, b, base_results[a][c], target_results[a][b], display_limit)
                     if res:
                         regressions.append(resstr)
         else:
@@ -403,4 +417,5 @@ def register_commands(subparsers):
     parser_build.add_argument('--commit-number', help="Revision number to search for, redundant if --commit is specified")
     parser_build.add_argument('--commit2', help="Revision to compare with")
     parser_build.add_argument('--commit-number2', help="Revision number to compare with, redundant if --commit2 is specified")
+    parser_build.add_argument('-l', '--limit', default=REGRESSIONS_DISPLAY_LIMIT, help="Maximum number of changes to display per test. Can be set to 0 to print all changes")
 
-- 
2.42.0

[OE-Core][PATCH 2/2] scripts/yocto_testresults_query: add option to change display limit

[Alexis Lothoré]

From: Alexis Lothoré <alexis.lothore@bootlin.com>

Add a "-l"/"--limit" option to allow changing the display limit in
resulttool.
- If no value is passed, resulttool uses its default value.
- If 0 is passed, the display limit is removed and every regression will be
  displayed
- If a custom value is passed, this value overrides the vlaue configured in
  resulttool

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 scripts/yocto_testresults_query.py | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/scripts/yocto_testresults_query.py b/scripts/yocto_testresults_query.py
index a5073736aab5..521ead8473ad 100755
--- a/scripts/yocto_testresults_query.py
+++ b/scripts/yocto_testresults_query.py
@@ -56,9 +56,12 @@ def fetch_testresults(workdir, sha1):
         subprocess.check_call(["git", "fetch", "--depth", "1", "origin", f"{rev}:{rev}"], cwd=workdir)
     return branch
 
-def compute_regression_report(workdir, basebranch, baserevision, targetbranch, targetrevision):
+def compute_regression_report(workdir, basebranch, baserevision, targetbranch, targetrevision, args):
     logger.info(f"Running resulttool regression between SHA1 {baserevision} and {targetrevision}")
-    report = subprocess.check_output([resulttool, "regression-git", "--branch", basebranch, "--commit", baserevision, "--branch2", targetbranch, "--commit2", targetrevision, workdir]).decode("utf-8")
+    command = [resulttool, "regression-git", "--branch", basebranch, "--commit", baserevision, "--branch2", targetbranch, "--commit2", targetrevision, workdir]
+    if args.limit:
+        command.extend(["-l", args.limit])
+    report = subprocess.check_output(command).decode("utf-8")
     return report
 
 def print_report_with_header(report, baseversion, baserevision, targetversion, targetrevision):
@@ -85,7 +88,7 @@ def regression(args):
             sys.exit(1)
         basebranch = fetch_testresults(workdir, baserevision)
         targetbranch = fetch_testresults(workdir, targetrevision)
-        report = compute_regression_report(workdir, basebranch, baserevision, targetbranch, targetrevision)
+        report = compute_regression_report(workdir, basebranch, baserevision, targetbranch, targetrevision, args)
         print_report_with_header(report, args.base, baserevision, args.target, targetrevision)
     finally:
         if not args.testresultsdir:
@@ -109,6 +112,10 @@ def main():
         '-t',
         '--testresultsdir',
         help=f"An existing test results directory. {sys.argv[0]} will automatically clone it and use default branch if not provided")
+    parser_regression_report.add_argument(
+        '-l',
+        '--limit',
+        help=f"Maximum number of changes to display per test. Can be set to 0 to print all changes")
     parser_regression_report.set_defaults(func=regression)
 
     args = parser.parse_args()
-- 
2.42.0

Re: [OE-Core][PATCH 0/2] Add a display limit for regression report generation

[Alexandre Belloni]

Hello Alexis,

On 19/10/2023 11:53:50+0200, Alexis Lothor� via lists.openembedded.org wrote:
> It has been observed that useful information in regression report can be
> drowned in huge regression lists which are often false-positives (for
> example, a whole set of tests has been temporarily disabled).
> 
> This series brings a default limit to how many changes are displayed per
> base/target comparison. This default can still be overriden on commandline,
> for example to have a better look at the whole regression list when trying
> to debug an issue (i.e. by disabling the limit)
> 
> First commit implement the limit, its default value and the corresponding
> commandline option in resulttool.
> Second commit allow yocto_testresults_query.py to drive this value.
> 
> As a result, one can for example do the following:
> - yocto_testresults_query 4.3_M1 4.3_M2
>   -> will display at most 50 regressions per test
> - yocto_testresults_query -l 10 4.3_M1 4.3_M2
>   -> override the display limit and reduce it to 10 regressions per pair.
> - yocto_testresults_query -l 0 4.3_M1 4.3_M2
>   -> disable the display limit, print all regressions
> 
> An example of regression report with display limit can be found here:
> https://pastebin.com/6QbfGstR
> 
> Alexis Lothor� (2):
>   scripts/resulttool: limit the number of changes displayed per test
>   scripts/yocto_testresults_query: add option to change display limit
> 

This causes:

https://autobuilder.yoctoproject.org/typhoon/#/builders/80/builds/5886/steps/14/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/5935/steps/14/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/5952/steps/14/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/86/builds/5936/steps/14/logs/stdio
https://autobuilder.yoctoproject.org/typhoon/#/builders/127/builds/2296/steps/14/logs/stdio


2023-10-19 07:28:44,229 - oe-selftest - INFO - resulttooltests.ResultToolTests.test_can_match_non_static_ptest_names (subunit.RemotedTestCase)
2023-10-19 07:28:44,229 - oe-selftest - INFO -  ... ERROR
Stderr:
2023-10-19 07:05:57,080 - oe-selftest - INFO - Adding: "include selftest.inc" in /home/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-271681/conf/local.conf
2023-10-19 07:05:57,081 - oe-selftest - INFO - Adding: "include bblayers.inc" in bblayers.conf
2023-10-19 07:28:44,229 - oe-selftest - INFO - 6: 24/42 185/543 (0.01s) (0 failed) (resulttooltests.ResultToolTests.test_can_match_non_static_ptest_names)
2023-10-19 07:28:44,230 - oe-selftest - INFO - testtools.testresult.real._StringException: Traceback (most recent call last):
  File "/home/pokybuild/yocto-worker/oe-selftest-debian/build/meta/lib/oeqa/selftest/cases/resulttooltests.py", line 372, in test_can_match_non_static_ptest_names
    result, resultstring = regression.compare_result(
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: compare_result() missing 1 required positional argument: 'display_limit'


2023-10-19 07:28:44,311 - oe-selftest - INFO - resulttooltests.ResultToolTests.test_regression_can_get_regression_result (subunit.RemotedTestCase)
2023-10-19 07:28:44,311 - oe-selftest - INFO -  ... ERROR
Stderr:
2023-10-19 07:05:57,080 - oe-selftest - INFO - Adding: "include selftest.inc" in /home/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-271681/conf/local.conf
2023-10-19 07:05:57,081 - oe-selftest - INFO - Adding: "include bblayers.inc" in bblayers.conf
2023-10-19 07:28:44,312 - oe-selftest - INFO - 6: 31/42 192/543 (0.01s) (2 failed) (resulttooltests.ResultToolTests.test_regression_can_get_regression_result)
2023-10-19 07:28:44,312 - oe-selftest - INFO - testtools.testresult.real._StringException: Traceback (most recent call last):
  File "/home/pokybuild/yocto-worker/oe-selftest-debian/build/meta/lib/oeqa/selftest/cases/resulttooltests.py", line 85, in test_regression_can_get_regression_result
    result, text = regression.compare_result(self.logger, "BaseTestRunName", "TargetTestRunName", base_result_data, target_result_data)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: compare_result() missing 1 required positional argument: 'display_limit'


>  scripts/lib/resulttool/regression.py | 23 +++++++++++++++++++----
>  scripts/yocto_testresults_query.py   | 13 ++++++++++---
>  2 files changed, 29 insertions(+), 7 deletions(-)
> 
> -- 
> 2.42.0
> 

> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#189440): https://lists.openembedded.org/g/openembedded-core/message/189440
> Mute This Topic: https://lists.openembedded.org/mt/102057050/3617179
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [alexandre.belloni@bootlin.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [OE-Core][PATCH 0/2] Add a display limit for regression report generation

[Alexis Lothoré]

Hello Alexandre,

On 10/20/23 08:05, Alexandre Belloni via lists.openembedded.org wrote:
> Hello Alexis,> 
> https://autobuilder.yoctoproject.org/typhoon/#/builders/80/builds/5886/steps/14/logs/stdio
> https://autobuilder.yoctoproject.org/typhoon/#/builders/79/builds/5935/steps/14/logs/stdio
> https://autobuilder.yoctoproject.org/typhoon/#/builders/87/builds/5952/steps/14/logs/stdio
> https://autobuilder.yoctoproject.org/typhoon/#/builders/86/builds/5936/steps/14/logs/stdio
> https://autobuilder.yoctoproject.org/typhoon/#/builders/127/builds/2296/steps/14/logs/stdio
> 
> 
> 2023-10-19 07:28:44,229 - oe-selftest - INFO - resulttooltests.ResultToolTests.test_can_match_non_static_ptest_names (subunit.RemotedTestCase)
> 2023-10-19 07:28:44,229 - oe-selftest - INFO -  ... ERROR
> Stderr:
> 2023-10-19 07:05:57,080 - oe-selftest - INFO - Adding: "include selftest.inc" in /home/pokybuild/yocto-worker/oe-selftest-debian/build/build-st-271681/conf/local.conf
> 2023-10-19 07:05:57,081 - oe-selftest - INFO - Adding: "include bblayers.inc" in bblayers.conf
> 2023-10-19 07:28:44,229 - oe-selftest - INFO - 6: 24/42 185/543 (0.01s) (0 failed) (resulttooltests.ResultToolTests.test_can_match_non_static_ptest_names)
> 2023-10-19 07:28:44,230 - oe-selftest - INFO - testtools.testresult.real._StringException: Traceback (most recent call last):
>   File "/home/pokybuild/yocto-worker/oe-selftest-debian/build/meta/lib/oeqa/selftest/cases/resulttooltests.py", line 372, in test_can_match_non_static_ptest_names
>     result, resultstring = regression.compare_result(
>                            ^^^^^^^^^^^^^^^^^^^^^^^^^^
> TypeError: compare_result() missing 1 required positional argument: 'display_limit'

Looks like I forgot to update/run selftests locally before pushing... Sorry for
the mess and thanks for the notification.
Anyway, Ross and Richard gave some relevant feedback
(https://lore.kernel.org/yocto/90D7FE40-32EB-4D67-B099-9E0C764FC258@arm.com/) on
this so I will prepare a v2, which fill also fix this selftest.

Thanks,

Alexis
-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com