TestResults: introduce extend() method

This notably allows removing results argument from many functions in
Controller. One motivation of this change is to satisfy pylint which
emits warnings when there are more than 5 arguments on a function.

A testcase is also introduced to cover all method of TestResults class.

Author: rezib

lib/rift/Controller.py

@@ -481,9 +481,10 @@ def build_pkg(config, args, pkg, arch):
 
     mock.clean()
 
-def test_one_pkg(config, args, pkg, vm, arch, repos, results):
+def test_one_pkg(config, args, pkg, vm, arch, repos):
     """
-    Launch tests on a given package on a specific VM and a set of repositories.
+    Launch tests on a given package on a specific VM and a set of repositories
+    and return results.
     """
     message(f"Preparing {arch} test environment")
     _vm_start(vm)
@@ -495,7 +496,7 @@ def test_one_pkg(config, args, pkg, vm, arch, repos, results):
 
     banner(f"Starting tests of package {pkg.name} on architecture {arch}")
 
-    rc = 0
+    results = TestResults()
 
     tests = list(pkg.tests())
     if not args.noauto:
@@ -509,7 +510,6 @@ def test_one_pkg(config, args, pkg, vm, arch, repos, results):
             results.add_success(case, time.time() - now, out=proc.out, err=proc.err)
             message(f"Test '{case.fullname}' on architecture {arch}: OK")
         else:
-            rc = 1
             results.add_failure(case, time.time() - now, out=proc.out, err=proc.err)
             message(f"Test '{case.fullname}' on architecture {arch}: ERROR")
 
@@ -519,10 +519,10 @@ def test_one_pkg(config, args, pkg, vm, arch, repos, results):
         time.sleep(5)
         vm.stop()
 
-    return rc
+    return results
 
-def test_pkgs(config, args, results, pkgs, arch, extra_repos=None):
-    """Test a list of packages on a specific architecture."""
+def test_pkgs(config, args, pkgs, arch, extra_repos=None):
+    """Test a list of packages on a specific architecture and return results."""
 
     if extra_repos is None:
         extra_repos = []
@@ -533,7 +533,7 @@ def test_pkgs(config, args, results, pkgs, arch, extra_repos=None):
     if vm.running():
         raise RiftError('VM is already running')
 
-    rc = 0
+    results = TestResults()
 
     for pkg in pkgs:
 
@@ -559,27 +559,30 @@ def test_pkgs(config, args, results, pkgs, arch, extra_repos=None):
             continue
 
         pkg.load()
-        rc += test_one_pkg(config, args, pkg, vm, arch, repos, results)
+        results.extend(test_one_pkg(config, args, pkg, vm, arch, repos))
 
     if getattr(args, 'noquit', False):
         message("Not stopping the VM. Use: rift vm connect")
 
-    return rc
+    return results
+
 
-def validate_pkgs(config, args, results, pkgs, arch):
+def validate_pkgs(config, args, pkgs, arch):
     """
-    Validate packages on a specific architecture:
+    Validate packages on a specific architecture and return results:
         - rpmlint on specfile
         - check file patterns
         - build it
-        - lauch tests
+        - launch tests
     """
 
     repos = ProjectArchRepositories(config, arch)
 
     if args.publish and not repos.can_publish():
         raise RiftError("Cannot publish if 'working_repo' is undefined")
 
+    results = TestResults()
+
     for pkg in pkgs:
 
         case = TestCase('build', pkg.name, arch)
@@ -638,20 +641,20 @@ def validate_pkgs(config, args, results, pkgs, arch):
         mock.publish(staging)
         staging.update()
 
-        rc = 0
         if args.test:
-            rc = test_pkgs(
-                config,
-                args,
-                results,
-                [pkg],
-                arch,
-                [staging.consumables[arch]]
+            results.extend(
+                test_pkgs(
+                    config,
+                    args,
+                    [pkg],
+                    arch,
+                    [staging.consumables[arch]]
+                )
             )
 
         # Also publish on working repo if requested
         # XXX: All RPMs should be published when all of them have been validated
-        if rc == 0 and args.publish:
+        if results.global_result and args.publish:
             message("Publishing RPMS...")
             mock.publish(repos.working)
 
@@ -666,6 +669,8 @@ def validate_pkgs(config, args, results, pkgs, arch):
 
     banner(f"All packages checked on architecture {arch}")
 
+    return results
+
 def vm_build(vm, args, config):
     """Build VM image."""
     if not args.deploy and args.output is None:
@@ -742,8 +747,12 @@ def action_vm(args, config):
         ret = vm_build(vm, args, config)
     return ret
 
-def build_pkgs(config, args, pkgs, arch, results):
-    """Build a list of packages on a given architecture and report results."""
+def build_pkgs(config, args, pkgs, arch):
+    """
+    Build a list of packages on a given architecture and return results.
+    """
+    results = TestResults()
+
     for pkg in pkgs:
         case = TestCase('build', pkg.name, arch)
         now = time.time()
@@ -774,6 +783,8 @@ def build_pkgs(config, args, pkgs, arch, results):
         else:
             results.add_success(case, time.time() - now)
 
+    return results
+
 def action_build(args, config):
     """Action for 'build' command."""
 
@@ -794,7 +805,7 @@ def action_build(args, config):
     for arch in config.get('arch'):
 
         pkgs = Package.list(config, staff, modules, args.packages)
-        build_pkgs(config, args, pkgs, arch, results)
+        results.extend(build_pkgs(config, args, pkgs, arch))
 
         if getattr(args, 'junit', False):
             logging.info('Writing test results in %s', args.junit)
@@ -825,12 +836,13 @@ def action_test(args, config):
     results = TestResults('test')
     # Test package on all project supported architectures
     for arch in config.get('arch'):
-        test_pkgs(
-            config,
-            args,
-            results,
-            Package.list(config, staff, modules, args.packages),
-            arch
+        results.extend(
+            test_pkgs(
+                config,
+                args,
+                Package.list(config, staff, modules, args.packages),
+                arch
+            )
         )
     if getattr(args, 'junit', False):
         logging.info('Writing test results in %s', args.junit)
@@ -856,12 +868,13 @@ def action_validate(args, config):
     results = TestResults('validate')
     # Validate packages on all project supported architectures
     for arch in config.get('arch'):
-        validate_pkgs(
-            config,
-            args,
-            results,
-            Package.list(config, staff, modules, args.packages),
-            arch
+        results.extend(
+            validate_pkgs(
+                config,
+                args,
+                Package.list(config, staff, modules, args.packages),
+                arch
+            )
         )
     banner('All packages checked on all architectures')
 
@@ -894,7 +907,7 @@ def action_validdiff(args, config):
     # Re-validate all updated packages for all architectures supported by the
     # project.
     for arch in config.get('arch'):
-        validate_pkgs(config, args, results, updated.values(), arch)
+        results.extend(validate_pkgs(config, args, updated.values(), arch))
 
 
     if getattr(args, 'junit', False):

lib/rift/TestResults.py

@@ -137,6 +137,15 @@ def _add_result(self, result):
         """
         self.results.append(result)
 
+    def extend(self, other):
+        """
+        Extend TestResults with all TestResult from the other TestResults.
+        """
+        for result in other.results:
+            self.results.append(result)
+            if result.value == 'Failure':
+                self.global_result = False
+
     def junit(self, filename):
         """
         Generate a junit xml file containing all tests results.

tests/TestResults.py

@@ -0,0 +1,91 @@
+#
+# Copyright (C) 2025 CEA
+#
+import textwrap
+import xml.etree.ElementTree as ET
+from io import BytesIO
+
+from TestUtils import RiftTestCase
+from rift.TestResults import TestResults, TestCase
+
+class TestResultsTest(RiftTestCase):
+    """
+    Tests class for TestResults
+    """
+
+    def test_init(self):
+        """ TestResults instance """
+        results = TestResults()
+
+        self.assertCountEqual(results.results, [])
+        self.assertTrue(results.global_result)
+        self.assertEqual(len(results), 0)
+
+    def test_add_success(self):
+        results = TestResults()
+        results.add_success(TestCase('test1', 'pkg', 'x86_64'), 1)
+        results.add_success(TestCase('test2', 'pkg', 'x86_64'), 1)
+        self.assertTrue(results.global_result)
+        self.assertEqual(len(results), 2)
+
+    def test_add_failure(self):
+        results = TestResults()
+        results.add_failure(TestCase('test1', 'pkg', 'x86_64'), 1)
+        results.add_failure(TestCase('test2', 'pkg', 'x86_64'), 1)
+        self.assertFalse(results.global_result)
+        self.assertEqual(len(results), 2)
+
+    def test_add_success_failure(self):
+        results = TestResults()
+        results.add_success(TestCase('test1', 'pkg', 'x86_64'), 1)
+        results.add_failure(TestCase('test2', 'pkg', 'x86_64'), 1)
+        self.assertFalse(results.global_result)
+        self.assertEqual(len(results), 2)
+
+    def test_extend(self):
+        results = TestResults()
+        results.add_success(TestCase('test1', 'pkg1', 'x86_64'), 1)
+        results.add_failure(TestCase('test2', 'pkg1', 'x86_64'), 1)
+        others = TestResults()
+        others.add_success(TestCase('test1', 'pkg2', 'x86_64'), 1)
+        others.add_failure(TestCase('test2', 'pkg2', 'x86_64'), 1)
+        results.extend(others)
+        self.assertFalse(results.global_result)
+        self.assertEqual(len(results), 4)
+
+    def test_junit(self):
+        results = TestResults()
+        results.add_success(TestCase('test1', 'pkg', 'x86_64'), 1, out="output test1")
+        results.add_failure(TestCase('test2', 'pkg', 'x86_64'), 1, out="output test2")
+        output = BytesIO()
+        results.junit(output)
+        root = ET.fromstring(output.getvalue().decode())
+        self.assertEqual(root.tag, 'testsuite')
+        self.assertEqual(root.attrib, { 'tests': '2'})
+        self.assertEqual(len(root.findall('*')), 2)
+        for element in root:
+            self.assertEqual(element.tag, 'testcase')
+            self.assertIn(element.attrib['name'], ['test1', 'test2'])
+            self.assertEqual(element.attrib['classname'], 'rift.pkg')
+            self.assertEqual(element.attrib['time'], '1.00')
+            if element.attrib['name'] == 'test1':
+                self.assertIsNone(element.find('failure'))
+                self.assertEqual(element.find('system-out').text, 'output test1')
+            else:
+                self.assertIsNotNone(element.find('failure'))
+                self.assertEqual(element.find('system-out').text, 'output test2')
+
+    def test_summary(self):
+        results = TestResults()
+        results.add_success(TestCase('test1', 'pkg', 'x86_64'), 1)
+        results.add_failure(TestCase('test2', 'pkg', 'x86_64'), 1)
+        self.assertEqual(
+            results.summary(),
+            textwrap.dedent(
+                """\
+                NAME      ARCH   DURATION RESULT
+                ----      ----   -------- ------
+                pkg.test1 x86_64       1s Success
+                pkg.test2 x86_64       1s FAILURE"""
+            )
+        )