[yocto-autobuilder-helper][PATCH 0/8] fix regression reports generation on "master-next" branches

[yocto-autobuilder-helper][PATCH 0/8] fix regression reports generation on "master-next" branches

[alexis.lothore]

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

This series fixes regression report generation on "next" branches, as raised in
[1].

The first five patches are preparatory updates for the real fix, being either
refactoring, cleanup or unit tests addition to better understand how integration
branches are used in send-qa-email.
The proper fix is in 6th commit, followed by corresponding tests
Finally, the last commit add Alexandre's "next" branch as "fork" branches to
enable regression reports generation when testing patches, as suggested in [1]
too.

Since patch testing branches are force-pushed on a regular basis, it is quite
difficult to get a relevant testing scenario, so this series has been tested by
faking SHA1 in yocto_testresults_query to match some master-next results in
yocto-testresults at the time of testing this series. I would gladly take
feedback about this series running for real in a master-next branch

[1] https://lists.yoctoproject.org/g/yocto/message/59067

Alexis Lothoré (8):
  scripts/utils: add unit tests for getcomparisonbranch
  scripts/send-qa-email: remove unused variable
  scripts/send-qa-email: invert boolean logic for release check
  scripts/send-qa-email: protect is_release_version from None value
  scripts/send-qa-email: add tests for is_release_version
  scripts/send-qa-email: fix testing branches regression reporting
  scripts/test_send_qa_email.py: add tests for base/target pair guessing
  config: flag A. Belloni master-next branch as testing branch

 config.json                   |   2 +-
 scripts/send_qa_email.py      |  34 +++++++----
 scripts/test_send_qa_email.py |  31 ++++++++++
 scripts/test_utils.py         | 104 ++++++++++++++++++++++++++++++++++
 4 files changed, 158 insertions(+), 13 deletions(-)
 create mode 100755 scripts/test_utils.py

-- 
2.39.2

[yocto-autobuilder-helper][PATCH 1/8] scripts/utils: add unit tests for getcomparisonbranch

[alexis.lothore]

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

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 scripts/test_utils.py | 104 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100755 scripts/test_utils.py

diff --git a/scripts/test_utils.py b/scripts/test_utils.py
new file mode 100755
index 0000000..ab91e3b
--- /dev/null
+++ b/scripts/test_utils.py
@@ -0,0 +1,104 @@
+#!/usr/bin/env python3
+
+import os
+import unittest
+import utils
+
+
+class TestGetComparisonBranch(unittest.TestCase):
+    TEST_CONFIG = {
+        "BUILD_HISTORY_DIRECTPUSH": [
+            "poky:morty",
+            "poky:pyro",
+            "poky:rocko",
+            "poky:sumo",
+            "poky:thud",
+            "poky:warrior",
+            "poky:zeus",
+            "poky:dunfell",
+            "poky:gatesgarth",
+            "poky:hardknott",
+            "poky:honister",
+            "poky:kirkstone",
+            "poky:langdale",
+            "poky:master"
+        ], "BUILD_HISTORY_FORKPUSH": {
+            "poky-contrib:ross/mut": "poky:master",
+            "poky:master-next": "poky:master",
+            "poky-contrib:abelloni/master-next": "poky:master"
+        }
+    }
+
+    def test_release_master(self):
+        repo = "ssh://git@push.yoctoproject.org/poky"
+        branch = "master"
+        basebranch, comparebranch = utils.getcomparisonbranch(
+            self.TEST_CONFIG, repo, branch)
+        self.assertEqual(
+            basebranch, "master", msg="Repo/branch pair present in BUILD_HISTORY_DIRECTPUSH must return corresponding base branch")
+        self.assertEqual(
+            comparebranch, None, msg="Repo/branch pair present in BUILD_HISTORY_DIRECTPUSH must return corresponding compare branch")
+
+    def test_release_kirkstone(self):
+        repo = "ssh://git@push.yoctoproject.org/poky"
+        branch = "kirkstone"
+        basebranch, comparebranch = utils.getcomparisonbranch(
+            self.TEST_CONFIG, repo, branch)
+        self.assertEqual(basebranch, "kirkstone",
+                         msg="Repo/branch pair present in BUILD_HISTORY_DIRECTPUSH must return corresponding base branch")
+        self.assertEqual(
+            comparebranch, None, msg="Repo/branch pair present in BUILD_HISTORY_DIRECTPUSH must return corresponding compare branch")
+
+    def test_release_langdale(self):
+        repo = "ssh://git@push.yoctoproject.org/poky"
+        branch = "langdale"
+        basebranch, comparebranch = utils.getcomparisonbranch(
+            self.TEST_CONFIG, repo, branch)
+        self.assertEqual(basebranch, "langdale",
+                         msg="Repo/branch pair present in BUILD_HISTORY_DIRECTPUSH must return corresponding base branch")
+        self.assertEqual(
+            comparebranch, None, msg="Repo/branch pair present in BUILD_HISTORY_DIRECTPUSH must return corresponding compare branch")
+
+    def test_master_next(self):
+        repo = "ssh://git@push.yoctoproject.org/poky"
+        branch = "master-next"
+        basebranch, comparebranch = utils.getcomparisonbranch(
+            self.TEST_CONFIG, repo, branch)
+        self.assertEqual(basebranch, "master-next",
+                         msg="Repo/branch pair present in BUILD_HISTORY_FORKPUSH must return corresponding base branch")
+        self.assertEqual(comparebranch, "master",
+                         msg="Repo/branch pair present in BUILD_HISTORY_FORKPUSH must return corresponding compare branch")
+
+    def test_abelloni_master_next(self):
+        repo = "ssh://git@push.yoctoproject.org/poky-contrib"
+        branch = "abelloni/master-next"
+        basebranch, comparebranch = utils.getcomparisonbranch(
+            self.TEST_CONFIG, repo, branch)
+        self.assertEqual(basebranch, "abelloni/master-next",
+                         msg="Repo/branch pair present in BUILD_HISTORY_FORKPUSH must return corresponding base branch")
+        self.assertEqual(comparebranch, "master",
+                         msg="Repo/branch pair present in BUILD_HISTORY_FORKPUSH must return corresponding compare branch")
+
+    def test_ross_master_next(self):
+        repo = "ssh://git@push.yoctoproject.org/poky-contrib"
+        branch = "ross/mut"
+        basebranch, comparebranch = utils.getcomparisonbranch(
+            self.TEST_CONFIG, repo, branch)
+        self.assertEqual(basebranch, "ross/mut",
+                         msg="Repo/branch pair present in BUILD_HISTORY_FORKPUSH must return corresponding base branch")
+        self.assertEqual(comparebranch, "master",
+                         msg="Repo/branch pair present in BUILD_HISTORY_FORKPUSH must return corresponding compare branch")
+
+    def test_arbitrary_branch(self):
+        repo = "ssh://git@push.yoctoproject.org/poky-contrib"
+        branch = "akanavin/package-version-updates"
+        basebranch, comparebranch = utils.getcomparisonbranch(
+            self.TEST_CONFIG, repo, branch)
+        self.assertEqual(
+            basebranch, None, msg="Arbitrary repo/branch should not return any specific basebranch")
+        self.assertEqual(
+            comparebranch, None,  msg="Arbitrary repo/branch should not return any specific comparebranch")
+
+
+if __name__ == '__main__':
+    unittest.main()
-- 
2.39.2

[yocto-autobuilder-helper][PATCH 2/8] scripts/send-qa-email: remove unused variable

[alexis.lothore]

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

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 scripts/send_qa_email.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/send_qa_email.py b/scripts/send_qa_email.py
index 7999c1b..96225a8 100755
--- a/scripts/send_qa_email.py
+++ b/scripts/send_qa_email.py
@@ -83,7 +83,6 @@ def send_qa_email():
 
     args = parser.parse_args()
 
-    scriptsdir = os.path.dirname(os.path.realpath(__file__))
     ourconfig = utils.loadconfig()
 
     with open(args.repojson) as f:
-- 
2.39.2

[yocto-autobuilder-helper][PATCH 3/8] scripts/send-qa-email: invert boolean logic for release check

[alexis.lothore]

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

is_non_release_version has an inverted logic which makes its reuse quite
confusing

Transform it as is_release_version and let caller do the negation if needed

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

diff --git a/scripts/send_qa_email.py b/scripts/send_qa_email.py
index 96225a8..320ff24 100755
--- a/scripts/send_qa_email.py
+++ b/scripts/send_qa_email.py
@@ -14,15 +14,15 @@ import re
 
 import utils
 
-def is_non_release_version(version):
+def is_release_version(version):
     p = re.compile('\d{8}-\d+')
-    return p.match(version) is not None
+    return p.match(version) is None
 
 def get_previous_tag(targetrepodir, version):
     previousversion = None
     previousmilestone = None
     if version:
-        if is_non_release_version(version):
+        if not is_release_version(version):
             return subprocess.check_output(["git", "describe", "--abbrev=0"], cwd=targetrepodir).decode('utf-8').strip()
         compareversion, comparemilestone, _ = utils.get_version_from_string(version)
         compareversionminor = compareversion[-1]
-- 
2.39.2

[yocto-autobuilder-helper][PATCH 4/8] scripts/send-qa-email: protect is_release_version from None value

[alexis.lothore]

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

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 scripts/send_qa_email.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/send_qa_email.py b/scripts/send_qa_email.py
index 320ff24..540eb94 100755
--- a/scripts/send_qa_email.py
+++ b/scripts/send_qa_email.py
@@ -16,7 +16,7 @@ import utils
 
 def is_release_version(version):
     p = re.compile('\d{8}-\d+')
-    return p.match(version) is None
+    return version is not None and p.match(version) is None
 
 def get_previous_tag(targetrepodir, version):
     previousversion = None
-- 
2.39.2

[yocto-autobuilder-helper][PATCH 5/8] scripts/send-qa-email: add tests for is_release_version

[alexis.lothore]

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

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

diff --git a/scripts/test_send_qa_email.py b/scripts/test_send_qa_email.py
index c1347fb..48bca98 100755
--- a/scripts/test_send_qa_email.py
+++ b/scripts/test_send_qa_email.py
@@ -29,6 +29,12 @@ class TestVersion(unittest.TestCase):
         {"input": {"version": "4.1.rc4"}, "expected": "yocto-4.0"}
     ]
 
+    test_data_is_release_version = [
+        {"input": "yocto-4.2", "expected":True},
+        {"input": "20230313-15", "expected":False},
+        {"input": None, "expected":False}
+    ]
+
     def test_versions(self):
         for data in self.test_data_get_version:
             test_name = data["input"]["version"]
@@ -36,6 +42,10 @@ class TestVersion(unittest.TestCase):
                 self.assertEqual(send_qa_email.get_previous_tag(os.environ.get(
                     "POKY_PATH"), data["input"]["version"]), data["expected"])
 
+    def test_is_release_version(self):
+        for data in self.test_data_is_release_version:
+            with self.subTest(f"{data['input']}"):
+                self.assertEqual(send_qa_email.is_release_version(data['input']), data['expected'])
 
 if __name__ == '__main__':
     if os.environ.get("POKY_PATH") is None:
-- 
2.39.2

[yocto-autobuilder-helper][PATCH 6/8] scripts/send-qa-email: fix testing branches regression reporting

[alexis.lothore]

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

d6018b891a3b7c62c7a2883c7fb9ae55e66f1363 broke regression reporting for testing
branches (e.g: master-next in poky, ross/mut in poky-contrib) by ignoring the comparebranch returned by
utils.getcomparison branch

Fix regression reporting for those branches by using comparebranch again. The
fix also refactor/add a intermediary step to guess base and target for
regression reporting, to isolate a bit the logic and make it easier later to add
multiple base/target couples

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 scripts/send_qa_email.py | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/scripts/send_qa_email.py b/scripts/send_qa_email.py
index 540eb94..78e051a 100755
--- a/scripts/send_qa_email.py
+++ b/scripts/send_qa_email.py
@@ -49,18 +49,28 @@ def get_previous_tag(targetrepodir, version):
     defaultbaseversion, _, _ = utils.get_version_from_string(subprocess.check_output(["git", "describe", "--abbrev=0"], cwd=targetrepodir).decode('utf-8').strip())
     return utils.get_tag_from_version(defaultbaseversion, None)
 
-def generate_regression_report(querytool, targetrepodir, basebranch, resultdir, outputdir, yoctoversion):
-    baseversion = get_previous_tag(targetrepodir, yoctoversion)
-    print(f"Comparing {basebranch} to {baseversion}")
+def get_regression_base_and_target(basebranch, comparebranch, release, targetrepodir):
+    if not basebranch:
+        # Basebranch/comparebranch is an arbitrary configuration (not defined in config.json): do not run regression reporting
+        return None, None
+
+    if is_release_version(release):
+        # We are on a release: ignore comparebranch (which is very likely None), regression reporting must be done against previous tag
+        return get_previous_tag(targetrepodir, release), basebranch
+    elif comparebranch:
+        # Basebranch/comparebranch is defined in config.json: regression reporting must be done against branches as defined in config.json
+        return comparebranch, basebranch
+
+def generate_regression_report(querytool, targetrepodir, base, target, resultdir, outputdir):
+    print(f"Comparing {target} to {base}")
 
     try:
-        regreport = subprocess.check_output([querytool, "regression-report", baseversion, basebranch, '-t', resultdir])
+        regreport = subprocess.check_output([querytool, "regression-report", base, target, '-t', resultdir])
         with open(outputdir + "/testresult-regressions-report.txt", "wb") as f:
            f.write(regreport)
     except subprocess.CalledProcessError as e:
         error = str(e)
-        print(f"Error while generating report between {basebranch} and {baseversion} : {error}")
-
+        print(f"Error while generating report between {target} and {base} : {error}")
 
 def send_qa_email():
     parser = utils.ArgParser(description='Process test results and optionally send an email about the build to prompt QA to begin testing.')
@@ -142,8 +152,9 @@ def send_qa_email():
                 subprocess.check_call(["git", "push", "--all"], cwd=tempdir)
                 subprocess.check_call(["git", "push", "--tags"], cwd=tempdir)
 
-            if basebranch:
-                generate_regression_report(querytool, targetrepodir, basebranch, tempdir, args.results_dir, args.release)
+            regression_base, regression_target = get_regression_base_and_target(basebranch, comparebranch, args.release, targetrepodir)
+            if regression_base and regression_target:
+                generate_regression_report(querytool, targetrepodir, regression_base, regression_target, tempdir, args.results_dir)
 
         finally:
             subprocess.check_call(["rm", "-rf",  tempdir])
-- 
2.39.2

[yocto-autobuilder-helper][PATCH 7/8] scripts/test_send_qa_email.py: add tests for base/target pair guessing

[alexis.lothore]

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

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

diff --git a/scripts/test_send_qa_email.py b/scripts/test_send_qa_email.py
index 48bca98..ccdcba6 100755
--- a/scripts/test_send_qa_email.py
+++ b/scripts/test_send_qa_email.py
@@ -35,6 +35,21 @@ class TestVersion(unittest.TestCase):
         {"input": None, "expected":False}
     ]
 
+    # This data represent real data returned by utils.getcomparisonbranch
+    # and the release argument passed to send-qa-email script
+    regression_inputs = [
+        {"name": "Arbitrary branch", "input": {"basebranch": None,
+                                               "comparebranch": None, "release": None}, "expected": (None, None)},
+        {"name": "Master release", "input": {"basebranch": "master",
+                                             "comparebranch": None, "release": "yocto-4.2_M3.rc1"}, "expected": ("4.2_M2", "master")},
+        {"name": "Older release", "input": {"basebranch": "kirkstone",
+                                            "comparebranch": None, "release": "yocto-4.0.8.rc2"}, "expected": ("yocto-4.0.7", "kirkstone")},
+        {"name": "Master Next", "input": {"basebranch": "master-next",
+                                          "comparebranch": "master", "release": None}, "expected": ("master", "master-next")},
+        {"name": "Fork Master Next", "input": {"basebranch": "ross/mut",
+                                               "comparebranch": "master", "release": None}, "expected": ("master", "ross/mut")},
+    ]
+
     def test_versions(self):
         for data in self.test_data_get_version:
             test_name = data["input"]["version"]
@@ -47,6 +62,12 @@ class TestVersion(unittest.TestCase):
             with self.subTest(f"{data['input']}"):
                 self.assertEqual(send_qa_email.is_release_version(data['input']), data['expected'])
 
+    def test_get_regression_base_and_target(self):
+        for data in self.regression_inputs:
+            with self.subTest(data['name']):
+                self.assertEqual(send_qa_email.get_regression_base_and_target(
+                    data['input']['basebranch'], data['input']['comparebranch'], data['input']['release'], os.environ.get("POKY_PATH")), data['expected'])
+
 if __name__ == '__main__':
     if os.environ.get("POKY_PATH") is None:
         print("Please set POKY_PATH to proper poky clone location before running tests")
-- 
2.39.2

[yocto-autobuilder-helper][PATCH 8/8] config: flag A. Belloni master-next branch as testing branch

[alexis.lothore]

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

Add "abelloni/master-next" branch from poky-contrib in configuration so that
regression reports are generated when testing for patches

Signed-off-by: Alexis Lothoré <alexis.lothore@bootlin.com>
---
 config.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config.json b/config.json
index 687608d..fcd0588 100644
--- a/config.json
+++ b/config.json
@@ -6,7 +6,7 @@
     "BUILD_HISTORY_DIR" : "buildhistory",
     "BUILD_HISTORY_REPO" : "ssh://git@push.yoctoproject.org/poky-buildhistory",
     "BUILD_HISTORY_DIRECTPUSH" : ["poky:morty", "poky:pyro", "poky:rocko", "poky:sumo", "poky:thud", "poky:warrior", "poky:zeus", "poky:dunfell", "poky:gatesgarth", "poky:hardknott", "poky:honister", "poky:kirkstone", "poky:langdale", "poky:master"],
-    "BUILD_HISTORY_FORKPUSH" : {"poky-contrib:ross/mut" : "poky:master", "poky:master-next" : "poky:master"},
+    "BUILD_HISTORY_FORKPUSH" : {"poky-contrib:ross/mut" : "poky:master", "poky-contrib:abelloni/master-next": "poky/master", "poky:master-next" : "poky:master"},
 
     "BUILDTOOLS_URL_TEMPLOCAL" : "/srv/autobuilder/autobuilder.yocto.io/pub/non-release/20210214-8/buildtools/x86_64-buildtools-extended-nativesdk-standalone-3.2+snapshot-7d38cc8e749aedb8435ee71847e04b353cca541d.sh",
     "BUILDTOOLS_URL_TEMPLOCAL2" : "https://downloads.yoctoproject.org/releases/yocto/milestones/yocto-3.1_M3/buildtools/x86_64-buildtools-extended-nativesdk-standalone-3.0+snapshot-20200315.sh",
-- 
2.39.2

Re: [yocto] [yocto-autobuilder-helper][PATCH 0/8] fix regression reports generation on "master-next" branches

[Richard Purdie]

On Mon, 2023-03-13 at 15:51 +0100, Alexis Lothoré via
lists.yoctoproject.org wrote:
> From: Alexis Lothoré <alexis.lothore@bootlin.com>
> 
> This series fixes regression report generation on "next" branches, as raised in
> [1].
> 
> The first five patches are preparatory updates for the real fix, being either
> refactoring, cleanup or unit tests addition to better understand how integration
> branches are used in send-qa-email.
> The proper fix is in 6th commit, followed by corresponding tests
> Finally, the last commit add Alexandre's "next" branch as "fork" branches to
> enable regression reports generation when testing patches, as suggested in [1]
> too.
> 
> Since patch testing branches are force-pushed on a regular basis, it is quite
> difficult to get a relevant testing scenario, so this series has been tested by
> faking SHA1 in yocto_testresults_query to match some master-next results in
> yocto-testresults at the time of testing this series. I would gladly take
> feedback about this series running for real in a master-next branch
> 
> [1] https://lists.yoctoproject.org/g/yocto/message/59067
> 
> Alexis Lothoré (8):
>   scripts/utils: add unit tests for getcomparisonbranch
>   scripts/send-qa-email: remove unused variable
>   scripts/send-qa-email: invert boolean logic for release check
>   scripts/send-qa-email: protect is_release_version from None value
>   scripts/send-qa-email: add tests for is_release_version
>   scripts/send-qa-email: fix testing branches regression reporting
>   scripts/test_send_qa_email.py: add tests for base/target pair guessing
>   config: flag A. Belloni master-next branch as testing branch

I think there is a regression somewhere:

https://autobuilder.yoctoproject.org/typhoon/#/builders/85/builds/2085/steps/29/logs/stdio

Cheers,

Richard

Re: [yocto] [yocto-autobuilder-helper][PATCH 0/8] fix regression reports generation on "master-next" branches

[Alexis Lothoré]

Hi Richard,
On 3/22/23 10:41, Richard Purdie wrote:
> On Mon, 2023-03-13 at 15:51 +0100, Alexis Lothoré via
> lists.yoctoproject.org wrote:
>> From: Alexis Lothoré <alexis.lothore@bootlin.com>
>>
>> This series fixes regression report generation on "next" branches, as raised in
>> [1].
>>
>> The first five patches are preparatory updates for the real fix, being either
>> refactoring, cleanup or unit tests addition to better understand how integration
>> branches are used in send-qa-email.
>> The proper fix is in 6th commit, followed by corresponding tests
>> Finally, the last commit add Alexandre's "next" branch as "fork" branches to
>> enable regression reports generation when testing patches, as suggested in [1]
>> too.
>>
>> Since patch testing branches are force-pushed on a regular basis, it is quite
>> difficult to get a relevant testing scenario, so this series has been tested by
>> faking SHA1 in yocto_testresults_query to match some master-next results in
>> yocto-testresults at the time of testing this series. I would gladly take
>> feedback about this series running for real in a master-next branch
>>
>> [1] https://lists.yoctoproject.org/g/yocto/message/59067
>>
>> Alexis Lothoré (8):
>>   scripts/utils: add unit tests for getcomparisonbranch
>>   scripts/send-qa-email: remove unused variable
>>   scripts/send-qa-email: invert boolean logic for release check
>>   scripts/send-qa-email: protect is_release_version from None value
>>   scripts/send-qa-email: add tests for is_release_version
>>   scripts/send-qa-email: fix testing branches regression reporting
>>   scripts/test_send_qa_email.py: add tests for base/target pair guessing
>>   config: flag A. Belloni master-next branch as testing branch
> 
> I think there is a regression somewhere:
> 
> https://autobuilder.yoctoproject.org/typhoon/#/builders/85/builds/2085/steps/29/logs/stdio
ACK, will take a look at it

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