[Concept,15/18] buildman: Add unit tests for _check_output_for_loop()

Message ID 20260109183116.3262115-16-sjg@u-boot.org
State New
Headers
Series buildman: Improve test coverage for builder.py |

Commit Message

Simon Glass Jan. 9, 2026, 6:31 p.m. UTC
  From: Simon Glass <simon.glass@canonical.com>

Extract the check_output() inner function from make() as a separate
method _check_output_for_loop() so it can be unit-tested. This function
detects Kconfig restart loops caused by missing defaults.

Add unit tests covering:
- Normal output (no restart message)
- Restart message sets the flag
- Single NEW item after restart (no loop)
- Different NEW items (no loop)
- Duplicate items trigger loop detection
- Duplicates without restart flag (no loop)
- Multiple items with one duplicate

Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: Simon Glass <simon.glass@canonical.com>
---

 tools/buildman/builder.py      | 64 ++++++++++++++-------------
 tools/buildman/main.py         |  1 +
 tools/buildman/test_builder.py | 79 ++++++++++++++++++++++++++++++++++
 3 files changed, 114 insertions(+), 30 deletions(-)
  

Patch

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 307249b5e13..d24fad9a550 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -585,50 +585,54 @@  class Builder:
         if checkout and self.checkout:
             gitutil.checkout(commit.hash)
 
-    def make(self, _commit, _brd, _stage, cwd, *args, **kwargs):
-        """Run make
+    def _check_output_for_loop(self, data):
+        """Check output for config restart loops
+
+        This detects when Kconfig enters a restart loop due to missing
+        defaults. It looks for 'Restart config' followed by multiple
+        occurrences of the same Kconfig item with no default.
 
         Args:
-            cwd (str): Directory where make should be run
-            args: Arguments to pass to make
-            kwargs: Arguments to pass to command.run_one()
+            data (bytes): Output data to check
 
         Returns:
-            CommandResult: Result of the make operation
+            bool: True to terminate the command, False to continue
         """
+        if b'Restart config' in data:
+            self._restarting_config = True
 
-        def check_output(_stream, data):
-            """Check output for config restart loops
+        # If we see 'Restart config' followed by multiple errors
+        if self._restarting_config:
+            matches = RE_NO_DEFAULT.findall(data)
 
-            Args:
-                data (bytes): Output data to check
-
-            Returns:
-                bool: True to terminate the command, False to continue
-            """
-            if b'Restart config' in data:
-                self._restarting_config = True
+            # Number of occurrences of each Kconfig item
+            multiple = [matches.count(val) for val in set(matches)]
 
-            # If we see 'Restart config' following by multiple errors
-            if self._restarting_config:
-                m = RE_NO_DEFAULT.findall(data)
+            # If any of them occur more than once, we have a loop
+            if [val for val in multiple if val > 1]:
+                self._terminated = True
+                return True
+        return False
 
-                # Number of occurences of each Kconfig item
-                multiple = [m.count(val) for val in set(m)]
+    def make(self, _commit, _brd, _stage, cwd, *args, **kwargs):
+        """Run make
 
-                # If any of them occur more than once, we have a loop
-                if [val for val in multiple if val > 1]:
-                    self._terminated = True
-                    return True
-            return False
+        Args:
+            cwd (str): Directory where make should be run
+            args: Arguments to pass to make
+            kwargs: Arguments to pass to command.run_one()
 
+        Returns:
+            CommandResult: Result of the make operation
+        """
         self._restarting_config = False
         self._terminated = False
         cmd = [self.gnu_make] + list(args)
-        result = command.run_one(*cmd, capture=True, capture_stderr=True,
-                                 cwd=cwd, raise_on_error=False,
-                                 infile='/dev/null', output_func=check_output,
-                                 **kwargs)
+        result = command.run_one(
+            *cmd, capture=True, capture_stderr=True, cwd=cwd,
+            raise_on_error=False, infile='/dev/null',
+            output_func=lambda stream, data: self._check_output_for_loop(data),
+            **kwargs)
 
         if self._terminated:
             # Try to be helpful
diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index 18809d843c6..dadfd629506 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -58,6 +58,7 @@  def run_tests(skip_net_tests, debug, verbose, args):
          test_builder.TestPrepareWorkingSpace,
          test_builder.TestShowNotBuilt,
          test_builder.TestPrepareOutputSpace,
+         test_builder.TestCheckOutputForLoop,
          'buildman.toolchain'])
 
     return (0 if result.wasSuccessful() else 1)
diff --git a/tools/buildman/test_builder.py b/tools/buildman/test_builder.py
index 78c80aa6c43..09809a07706 100644
--- a/tools/buildman/test_builder.py
+++ b/tools/buildman/test_builder.py
@@ -548,5 +548,84 @@  class TestPrepareOutputSpace(unittest.TestCase):
         self.assertFalse(lines[0].newline)
 
 
+class TestCheckOutputForLoop(unittest.TestCase):
+    """Tests for Builder._check_output_for_loop()"""
+
+    def setUp(self):
+        """Set up test fixtures"""
+        self.builder = builder.Builder(
+            toolchains=None, base_dir='/tmp/test', git_dir='/src/repo',
+            num_threads=4, num_jobs=1)
+        # Reset state before each test
+        self.builder._restarting_config = False
+        self.builder._terminated = False
+
+    def test_no_restart_message(self):
+        """Test that normal output does not trigger termination"""
+        result = self.builder._check_output_for_loop(b'Building target...')
+
+        self.assertFalse(result)
+        self.assertFalse(self.builder._restarting_config)
+        self.assertFalse(self.builder._terminated)
+
+    def test_restart_message_sets_flag(self):
+        """Test that 'Restart config' sets the restarting flag"""
+        result = self.builder._check_output_for_loop(b'Restart config...')
+
+        self.assertFalse(result)  # No loop detected yet
+        self.assertTrue(self.builder._restarting_config)
+        self.assertFalse(self.builder._terminated)
+
+    def test_single_new_item_no_loop(self):
+        """Test that a single NEW item after restart is not a loop"""
+        self.builder._restarting_config = True
+
+        result = self.builder._check_output_for_loop(
+            b'(CONFIG_ITEM) [] (NEW)')
+
+        self.assertFalse(result)
+        self.assertFalse(self.builder._terminated)
+
+    def test_different_new_items_no_loop(self):
+        """Test that different NEW items do not trigger a loop"""
+        self.builder._restarting_config = True
+
+        result = self.builder._check_output_for_loop(
+            b'(CONFIG_A) [] (NEW)\n(CONFIG_B) [] (NEW)')
+
+        self.assertFalse(result)
+        self.assertFalse(self.builder._terminated)
+
+    def test_duplicate_items_triggers_loop(self):
+        """Test that duplicate NEW items trigger loop detection"""
+        self.builder._restarting_config = True
+
+        result = self.builder._check_output_for_loop(
+            b'(CONFIG_ITEM) [] (NEW)\n(CONFIG_ITEM) [] (NEW)')
+
+        self.assertTrue(result)
+        self.assertTrue(self.builder._terminated)
+
+    def test_no_loop_without_restart(self):
+        """Test that duplicates without restart flag do not trigger loop"""
+        # _restarting_config is False by default
+
+        result = self.builder._check_output_for_loop(
+            b'(CONFIG_ITEM) [] (NEW)\n(CONFIG_ITEM) [] (NEW)')
+
+        self.assertFalse(result)
+        self.assertFalse(self.builder._terminated)
+
+    def test_multiple_items_one_duplicate(self):
+        """Test loop detection with multiple items, one duplicated"""
+        self.builder._restarting_config = True
+
+        result = self.builder._check_output_for_loop(
+            b'(CONFIG_A) [] (NEW)\n(CONFIG_B) [] (NEW)\n(CONFIG_A) [] (NEW)')
+
+        self.assertTrue(result)
+        self.assertTrue(self.builder._terminated)
+
+
 if __name__ == '__main__':
     unittest.main()