From: Simon Glass <simon.glass@canonical.com> Add a col parameter to Builder to allow passing in a terminal.Color object instead of creating one internally. This makes the dependency explicit and allows for easier testing. Update control.py to create the Color object and pass it to Builder. Update all test files to pass col to Builder. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 5 +++-- tools/buildman/control.py | 5 ++++- tools/buildman/test.py | 35 ++++++++++++++++++++-------------- tools/buildman/test_builder.py | 21 +++++++++++++------- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 00a3b2f165b..e52956cda0e 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -216,7 +216,7 @@ class Builder: """ def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, - gnu_make='make', checkout=True, step=1, + col, gnu_make='make', checkout=True, step=1, no_subdirs=False, full_path=False, verbose_build=False, mrproper=False, fallback_mrproper=False, per_board_out_dir=False, config_only=False, @@ -236,6 +236,7 @@ class Builder: git_dir: Git directory containing source repository num_threads: Number of builder threads to run num_jobs: Number of jobs to run at once (passed to make as -j) + col: terminal.Color object for coloured output gnu_make: the command name of GNU Make. checkout: True to check out source, False to skip that step. This is used for testing. @@ -335,7 +336,7 @@ class Builder: self._restarting_config = False self.warnings_as_errors = warnings_as_errors - self.col = terminal.Color() + self.col = col self.thread_exceptions = [] self.test_thread_exceptions = test_thread_exceptions diff --git a/tools/buildman/control.py b/tools/buildman/control.py index ddb8c2f145e..398441aa9ed 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -809,9 +809,12 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, if args.config_only and args.target: raise ValueError('Cannot use --config-only with --target') + # Create colour object for output + col = terminal.Color() + # Create a new builder with the selected args builder = Builder(toolchains, output_dir, git_dir, - args.threads, args.jobs, checkout=True, step=args.step, + args.threads, args.jobs, col=col, checkout=True, step=args.step, no_subdirs=args.no_subdirs, full_path=args.full_path, verbose_build=args.verbose_build, mrproper=args.mrproper, diff --git a/tools/buildman/test.py b/tools/buildman/test.py index cf37b821bf7..6702fd9aad8 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -264,7 +264,7 @@ class TestBuildOutput(TestBuildBase): Iterator containing the output lines, each a PrintLine() object """ build = builder.Builder(self.toolchains, self.base_dir, None, threads, - 2, checkout=False) + 2, self._col, checkout=False) build.do_make = self.make board_selected = self.brds.get_selected_dict() @@ -332,7 +332,7 @@ class TestBuildOutput(TestBuildBase): filter_dtb_warnings: Adjust the check for output produced with the --filter-dtb-warnings flag """ - col = terminal.Color() + col = self._col boards01234 = ('board0 board1 board2 board3 board4' if list_error_boards else '') boards1234 = 'board1 board2 board3 board4' if list_error_boards else '' @@ -642,7 +642,7 @@ class TestBuild(TestBuildBase): def test_output_dir(self): """Test output-directory naming for a commit""" build = builder.Builder(self.toolchains, BASE_DIR, None, 1, 2, - checkout=False) + self._col, checkout=False) build.commits = self.commits build.commit_count = len(self.commits) subject = self.commits[1].subject.translate(builder.trans_valid_chars) @@ -652,7 +652,7 @@ class TestBuild(TestBuildBase): def test_output_dir_current(self): """Test output-directory naming for current source""" build = builder.Builder(self.toolchains, BASE_DIR, None, 1, 2, - checkout=False) + self._col, checkout=False) build.commits = None build.commit_count = 0 self.check_dirs(build, '/current') @@ -660,7 +660,7 @@ class TestBuild(TestBuildBase): def test_output_dir_no_subdirs(self): """Test output-directory naming without subdirectories""" build = builder.Builder(self.toolchains, BASE_DIR, None, 1, 2, - checkout=False, no_subdirs=True) + self._col, checkout=False, no_subdirs=True) build.commits = None build.commit_count = 0 self.check_dirs(build, '') @@ -759,7 +759,8 @@ class TestBuild(TestBuildBase): for name in to_remove + to_leave: _touch(name) - build = builder.Builder(self.toolchains, base_dir, None, 1, 2) + build = builder.Builder(self.toolchains, base_dir, None, 1, 2, + self._col) build.commits = self.commits build.commit_count = len(COMMITS) # pylint: disable=protected-access @@ -1003,7 +1004,7 @@ class TestBuildMisc(TestBuildBase): # Check a missing tool with self.assertRaises(ValueError) as exc: builder.Builder(self.toolchains, self.base_dir, None, 0, 2, - dtc_skip=True) + self._col, dtc_skip=True) self.assertIn('Cannot find dtc', str(exc.exception)) # Create a fake tool to use @@ -1012,13 +1013,14 @@ class TestBuildMisc(TestBuildBase): os.chmod(dtc, 0o777) build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, - dtc_skip=True) + self._col, dtc_skip=True) tch = self.toolchains.select('arm') env = build.make_environment(tch) self.assertIn(b'DTC', env) # Try the normal case, i.e. not skipping the dtc build - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) tch = self.toolchains.select('arm') env = build.make_environment(tch) self.assertNotIn(b'DTC', env) @@ -1146,7 +1148,8 @@ class TestBuilderFuncs(TestBuildBase): def test_read_func_sizes(self): """Test read_func_sizes() function""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) # Create test data simulating 'nm' output # NM_SYMBOL_TYPES = 'tTdDbBr' - text, data, bss, rodata @@ -1175,7 +1178,8 @@ class TestBuilderFuncs(TestBuildBase): def test_read_func_sizes_static(self): """Test read_func_sizes() with static function symbols""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) # Test static functions (have . in name after first char) nm_output = '''00000100 t func.1234 @@ -1196,7 +1200,8 @@ class TestBuilderFuncs(TestBuildBase): def test_process_environment(self): """Test _process_environment() function""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) # Environment file uses null-terminated strings env_data = 'bootcmd=run bootm\x00bootdelay=3\x00console=ttyS0\x00' @@ -1215,14 +1220,16 @@ class TestBuilderFuncs(TestBuildBase): def test_process_environment_nonexistent(self): """Test _process_environment() with non-existent file""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) env = build._process_environment('/nonexistent/path/uboot.env') self.assertEqual({}, env) def test_process_environment_invalid_lines(self): """Test _process_environment() handles invalid lines gracefully""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) # Include lines without '=' which should be ignored env_data = 'valid=value\x00invalid_no_equals\x00another=good\x00' diff --git a/tools/buildman/test_builder.py b/tools/buildman/test_builder.py index b92e0be18be..928be84167d 100644 --- a/tools/buildman/test_builder.py +++ b/tools/buildman/test_builder.py @@ -24,9 +24,10 @@ class TestPrintFuncSizeDetail(unittest.TestCase): def setUp(self): """Set up test fixtures""" # Create a minimal Builder for testing + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp', git_dir=None, num_threads=0, - num_jobs=1) + num_jobs=1, col=self.col) terminal.set_print_test_mode() def tearDown(self): @@ -153,9 +154,10 @@ class TestPrepareThread(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) terminal.set_print_test_mode() def tearDown(self): @@ -267,9 +269,10 @@ class TestPrepareWorkingSpace(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) terminal.set_print_test_mode() def tearDown(self): @@ -477,9 +480,10 @@ class TestPrepareOutputSpace(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) terminal.set_print_test_mode() def tearDown(self): @@ -560,9 +564,10 @@ class TestCheckOutputForLoop(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) # Reset state before each test self.builder._restarting_config = False self.builder._terminated = False @@ -639,9 +644,10 @@ class TestMake(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) @mock.patch('buildman.builder.command.run_one') def test_make_basic(self, mock_run_one): @@ -731,9 +737,10 @@ class TestPrintBuildSummary(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) # Set a start time in the past (less than 1 second ago to avoid # duration output) self.builder._start_time = datetime.now() -- 2.43.0