From 19558f05722deb715204d3552e8281a6c6273bbc Mon Sep 17 00:00:00 2001 From: isaacyang Date: Fri, 4 Jul 2025 17:54:21 +0800 Subject: [PATCH 1/7] Fix: the unavailable test not print the log --- .../checkbox_support/scripts/fwts_test.py | 3 +- .../scripts/tests/test_fwts_test.py | 94 ++++++++++++++++++- 2 files changed, 94 insertions(+), 3 deletions(-) diff --git a/checkbox-support/checkbox_support/scripts/fwts_test.py b/checkbox-support/checkbox_support/scripts/fwts_test.py index e18686397b..12ff39823b 100644 --- a/checkbox-support/checkbox_support/scripts/fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/fwts_test.py @@ -679,7 +679,8 @@ def main(): print() print(" Please review the following log for more information:") print() - print_log(args.log) + if not unavailable: + print_log(args.log) if args.fail_level != "none": if fail_priority == fail_levels["FAILED_CRITICAL"]: diff --git a/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py b/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py index 6df037134f..53f4fd4e46 100644 --- a/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py @@ -17,13 +17,13 @@ # along with Checkbox. If not, see . import unittest +from unittest.mock import patch, MagicMock from tempfile import NamedTemporaryFile import os from io import StringIO -from checkbox_support.scripts.fwts_test import print_log -from unittest.mock import patch +from checkbox_support.scripts.fwts_test import print_log, main class LogPrinterTest(unittest.TestCase): @@ -48,3 +48,93 @@ def tearDown(self): os.unlink(self.logfile.name) except OSError: pass + + +class MainFunctionTest(unittest.TestCase): + def setUp(self): + self.logfile = NamedTemporaryFile(delete=False) + # Write some test content to the log file + with open(self.logfile.name, "w") as f: + f.write("Test log content\n") + + @patch("sys.argv", ["fwts_test.py", "--log", "test.log"]) + @patch("sys.stdout", new_callable=StringIO) + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_print_log_called_when_no_unavailable_tests( + self, mock_popen, mock_stdout + ): + """Test that print_log is called when no tests are unavailable.""" + # Mock Popen to return successful test results + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"PASSED: Test completed successfully", + None, + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + # Mock fwts --show-tests to return some available tests + mock_fwts_process = MagicMock() + mock_fwts_process.communicate.return_value = ( + b"acpitests\nversion\n", + None, + ) + mock_fwts_process.returncode = 0 + + # Configure Popen to return different results for different calls + mock_popen.side_effect = [mock_fwts_process, mock_process] + + # Run main with a test that should be available + with patch( + "sys.argv", + [ + "fwts_test.py", + "--test", + "acpitests", + "--log", + self.logfile.name, + ], + ): + main() + + # Verify that print_log was called (log content should be in stdout) + output = mock_stdout.getvalue() + self.assertIn("Test log content", output) + + @patch("sys.argv", ["fwts_test.py", "--log", "test.log"]) + @patch("sys.stdout", new_callable=StringIO) + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_print_log_not_called_when_unavailable_tests( + self, mock_popen, mock_stdout + ): + """Test that print_log is NOT called when tests are unavailable.""" + # Mock fwts --show-tests to return different available tests + mock_fwts_process = MagicMock() + mock_fwts_process.communicate.return_value = (b"version\n", None) + mock_fwts_process.returncode = 0 + mock_popen.return_value = mock_fwts_process + + # Run main with a test that should NOT be available + with patch( + "sys.argv", + [ + "fwts_test.py", + "--test", + "nonexistent_test", + "--log", + self.logfile.name, + ], + ): + main() + + # Verify that print_log was NOT called (log content not in stdout) + output = mock_stdout.getvalue() + self.assertNotIn("Test log content", output) + # But we should see the unavailable test message + self.assertIn("Unavailable Tests: 1", output) + + def tearDown(self): + try: + os.unlink(self.logfile.name) + except OSError: + pass From d7fba5b77400f5a4e1648445d5e9c9be6ee74dc5 Mon Sep 17 00:00:00 2001 From: isaacyang Date: Tue, 8 Jul 2025 14:19:34 +0800 Subject: [PATCH 2/7] Adopt Stanley's suggestion to bypass the unavailable tests --- .../checkbox_support/scripts/fwts_test.py | 2 +- .../scripts/tests/test_fwts_test.py | 56 +++++++++++++++++-- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/checkbox-support/checkbox_support/scripts/fwts_test.py b/checkbox-support/checkbox_support/scripts/fwts_test.py index 12ff39823b..41506b3e57 100644 --- a/checkbox-support/checkbox_support/scripts/fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/fwts_test.py @@ -679,7 +679,7 @@ def main(): print() print(" Please review the following log for more information:") print() - if not unavailable: + if tests: # Only print log if there were tests actually run print_log(args.log) if args.fail_level != "none": diff --git a/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py b/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py index 53f4fd4e46..227fe019c8 100644 --- a/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py @@ -60,10 +60,10 @@ def setUp(self): @patch("sys.argv", ["fwts_test.py", "--log", "test.log"]) @patch("sys.stdout", new_callable=StringIO) @patch("checkbox_support.scripts.fwts_test.Popen") - def test_print_log_called_when_no_unavailable_tests( + def test_print_log_called_when_tests_are_run( self, mock_popen, mock_stdout ): - """Test that print_log is called when no tests are unavailable.""" + """Test that print_log is called when tests are actually run.""" # Mock Popen to return successful test results mock_process = MagicMock() mock_process.communicate.return_value = ( @@ -104,10 +104,10 @@ def test_print_log_called_when_no_unavailable_tests( @patch("sys.argv", ["fwts_test.py", "--log", "test.log"]) @patch("sys.stdout", new_callable=StringIO) @patch("checkbox_support.scripts.fwts_test.Popen") - def test_print_log_not_called_when_unavailable_tests( + def test_print_log_not_called_when_no_tests_run( self, mock_popen, mock_stdout ): - """Test that print_log is NOT called when tests are unavailable.""" + """Test that print_log is NOT called when no tests are run.""" # Mock fwts --show-tests to return different available tests mock_fwts_process = MagicMock() mock_fwts_process.communicate.return_value = (b"version\n", None) @@ -133,6 +133,54 @@ def test_print_log_not_called_when_unavailable_tests( # But we should see the unavailable test message self.assertIn("Unavailable Tests: 1", output) + @patch("sys.argv", ["fwts_test.py", "--log", "test.log"]) + @patch("sys.stdout", new_callable=StringIO) + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_print_log_called_when_mixed_available_unavailable( + self, mock_popen, mock_stdout + ): + """Test that print_log is called when some tests run, some unavailable.""" + # Mock Popen to return successful test results + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"PASSED: Test completed successfully", + None, + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + # Mock fwts --show-tests to return some available tests + mock_fwts_process = MagicMock() + mock_fwts_process.communicate.return_value = ( + b"acpitests\nversion\n", + None, + ) + mock_fwts_process.returncode = 0 + + # Configure Popen to return different results for different calls + mock_popen.side_effect = [mock_fwts_process, mock_process] + + # Run main with mixed available and unavailable tests + with patch( + "sys.argv", + [ + "fwts_test.py", + "--test", + "acpitests", + "--test", + "nonexistent_test", + "--log", + self.logfile.name, + ], + ): + main() + + # Verify that print_log was called (log content should be in stdout) + output = mock_stdout.getvalue() + self.assertIn("Test log content", output) + # Also verify that unavailable test message appears + self.assertIn("Unavailable Tests: 1", output) + def tearDown(self): try: os.unlink(self.logfile.name) From 5a34f6c02d2e44aba764d1c12457c27c5ada74fa Mon Sep 17 00:00:00 2001 From: isaacyang Date: Tue, 8 Jul 2025 16:02:06 +0800 Subject: [PATCH 3/7] fwts_test: add the available test filter --- .../checkbox_support/scripts/fwts_test.py | 85 +++-- .../scripts/tests/test_fwts_test.py | 358 +++++++++++++++++- 2 files changed, 412 insertions(+), 31 deletions(-) diff --git a/checkbox-support/checkbox_support/scripts/fwts_test.py b/checkbox-support/checkbox_support/scripts/fwts_test.py index 41506b3e57..1017622a2a 100644 --- a/checkbox-support/checkbox_support/scripts/fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/fwts_test.py @@ -168,6 +168,35 @@ SLEEP_TIME_RE = re.compile(r"(Suspend|Resume):\s+([\d\.]+)\s+seconds.") +def get_available_fwts_tests(): + """ + Get a list of all available FWTS tests by running 'fwts --show-tests'. + + Returns: + list: A list of available test names that can be run with FWTS. + """ + cmd = "fwts --show-tests" + result = Popen(cmd, stdout=PIPE, stderr=PIPE, shell=True) + stdout, stderr = result.communicate() + + if result.returncode != 0: + raise RuntimeError(f"FWTS command failed: {stderr.decode()}") + + # Parse the output to extract test names + output_lines = stdout.decode().strip().split("\n") + available_tests = [] + + for line in output_lines: + # Skip empty lines and section headers (lines ending with ':') + if line.strip() and not line.endswith(":"): + # Extract the first word as the test name + test_name = line.lstrip().split()[0] + available_tests.append(test_name) + + # Remove duplicates while preserving order + return list(dict.fromkeys(available_tests)) + + def get_sleep_times(log, start_marker): suspend_time = "" resume_time = "" @@ -267,6 +296,23 @@ def print_log(logfile): print("WARNING: Found bad char in " + logfile) +def filter_available_tests(requested_tests): + """ + Given a list of requested tests, return a tuple: + (available_tests, unavailable_tests), where available_tests are those + present in the current system's available FWTS tests, and unavailable_tests + are those not present. + """ + available_tests_set = set(get_available_fwts_tests()) + available = [ + test for test in requested_tests if test in available_tests_set + ] + unavailable = [ + test for test in requested_tests if test not in available_tests_set + ] + return available, unavailable + + def main(): description_text = "Tests the system BIOS using the Firmware Test Suite" epilog_text = ( @@ -423,17 +469,22 @@ def main(): Popen("fwts -h", shell=True).communicate()[0] return 0 elif args.list: - print("\n".join(TESTS)) + available, unavailable = filter_available_tests(TESTS) + print("\n".join(available)) return 0 elif args.list_hwe: - print("\n".join(HWE_TESTS)) + available, unavailable = filter_available_tests(HWE_TESTS) + print("\n".join(available)) return 0 elif args.list_qa: - print("\n".join(QA_TESTS)) + available, unavailable = filter_available_tests(QA_TESTS) + print("\n".join(available)) return 0 elif args.list_server: + available, unavailable = filter_available_tests(SERVER_TESTS) print("Server Certification Tests:") - print(" * ", "\n * ".join(SERVER_TESTS)) + print(" * ", "\n * ".join(available)) + return 0 elif args.test: requested_tests.extend(args.test) elif args.hwe: @@ -532,31 +583,7 @@ def main(): # Because the list of available tests varies from arch to arch, we # need to validate our test selections and remove any unsupported # tests. - cmd = "fwts --show-tests" - fwts_test_list = ( - Popen(cmd, stdout=PIPE, shell=True) - .communicate()[0] - .strip() - .decode() - .split("\n") - ) - AVAILABLE_TESTS = list( - dict.fromkeys( - [ - item.lstrip().split()[0] - for item in fwts_test_list - if not item.endswith(":") and item != "" - ] - ) - ) - # Compare requested tests to AVAILABLE_TESTS, and if we've requested a - # test that isn't available, go ahead and mark it as skipped, otherwise - # add it to tests for execution - for test in requested_tests: - if test not in AVAILABLE_TESTS: - unavailable.append(test) - else: - tests.append(test) + tests, unavailable = filter_available_tests(requested_tests) if tests: for test in tests: diff --git a/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py b/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py index 227fe019c8..e40cad7a88 100644 --- a/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py @@ -18,12 +18,18 @@ import unittest from unittest.mock import patch, MagicMock +from subprocess import PIPE from tempfile import NamedTemporaryFile import os from io import StringIO -from checkbox_support.scripts.fwts_test import print_log, main +from checkbox_support.scripts.fwts_test import ( + print_log, + main, + filter_available_tests, + get_available_fwts_tests, +) class LogPrinterTest(unittest.TestCase): @@ -50,6 +56,120 @@ def tearDown(self): pass +class FilterAvailableTestsTest(unittest.TestCase): + """Test the filter_available_tests helper function.""" + + @patch("checkbox_support.scripts.fwts_test.get_available_fwts_tests") + def test_filter_available_tests_all_available(self, mock_get_available): + """Test when all requested tests are available.""" + mock_get_available.return_value = ["acpitests", "version", "mtrr"] + requested = ["acpitests", "version"] + + available, unavailable = filter_available_tests(requested) + + self.assertEqual(available, ["acpitests", "version"]) + self.assertEqual(unavailable, []) + + @patch("checkbox_support.scripts.fwts_test.get_available_fwts_tests") + def test_filter_available_tests_some_unavailable(self, mock_get_available): + """Test when some requested tests are unavailable.""" + mock_get_available.return_value = ["acpitests", "version"] + requested = ["acpitests", "nonexistent_test", "version"] + + available, unavailable = filter_available_tests(requested) + + self.assertEqual(available, ["acpitests", "version"]) + self.assertEqual(unavailable, ["nonexistent_test"]) + + @patch("checkbox_support.scripts.fwts_test.get_available_fwts_tests") + def test_filter_available_tests_none_available(self, mock_get_available): + """Test when none of the requested tests are available.""" + mock_get_available.return_value = ["acpitests", "version"] + requested = ["nonexistent_test1", "nonexistent_test2"] + + available, unavailable = filter_available_tests(requested) + + self.assertEqual(available, []) + self.assertEqual( + unavailable, ["nonexistent_test1", "nonexistent_test2"] + ) + + @patch("checkbox_support.scripts.fwts_test.get_available_fwts_tests") + def test_filter_available_tests_empty_requested(self, mock_get_available): + """Test with empty requested tests list.""" + mock_get_available.return_value = ["acpitests", "version"] + requested = [] + + available, unavailable = filter_available_tests(requested) + + self.assertEqual(available, []) + self.assertEqual(unavailable, []) + + +class ListOptionsTest(unittest.TestCase): + """Test the updated list options that use filter_available_tests.""" + + @patch("sys.stdout", new_callable=StringIO) + @patch("checkbox_support.scripts.fwts_test.filter_available_tests") + def test_list_option(self, mock_filter, mock_stdout): + """Test --list option.""" + mock_filter.return_value = (["acpitests", "version"], []) + + with patch("sys.argv", ["fwts_test.py", "--list"]): + result = main() + + self.assertEqual(result, 0) + output = mock_stdout.getvalue() + self.assertIn("acpitests", output) + self.assertIn("version", output) + + @patch("sys.stdout", new_callable=StringIO) + @patch("checkbox_support.scripts.fwts_test.filter_available_tests") + def test_list_hwe_option(self, mock_filter, mock_stdout): + """Test --list-hwe option.""" + mock_filter.return_value = (["mtrr", "virt"], ["apicedge"]) + + with patch("sys.argv", ["fwts_test.py", "--list-hwe"]): + result = main() + + self.assertEqual(result, 0) + output = mock_stdout.getvalue() + self.assertIn("mtrr", output) + self.assertIn("virt", output) + self.assertNotIn("apicedge", output) # Should not show unavailable + + @patch("sys.stdout", new_callable=StringIO) + @patch("checkbox_support.scripts.fwts_test.filter_available_tests") + def test_list_qa_option(self, mock_filter, mock_stdout): + """Test --list-qa option.""" + mock_filter.return_value = (["acpitests", "version"], ["nonexistent"]) + + with patch("sys.argv", ["fwts_test.py", "--list-qa"]): + result = main() + + self.assertEqual(result, 0) + output = mock_stdout.getvalue() + self.assertIn("acpitests", output) + self.assertIn("version", output) + self.assertNotIn("nonexistent", output) + + @patch("sys.stdout", new_callable=StringIO) + @patch("checkbox_support.scripts.fwts_test.filter_available_tests") + def test_list_server_option(self, mock_filter, mock_stdout): + """Test --list-server option.""" + mock_filter.return_value = (["acpitests", "version"], ["nonexistent"]) + + with patch("sys.argv", ["fwts_test.py", "--list-server"]): + result = main() + + self.assertEqual(result, 0) + output = mock_stdout.getvalue() + self.assertIn("Server Certification Tests:", output) + self.assertIn("acpitests", output) + self.assertIn("version", output) + self.assertNotIn("nonexistent", output) + + class MainFunctionTest(unittest.TestCase): def setUp(self): self.logfile = NamedTemporaryFile(delete=False) @@ -139,7 +259,7 @@ def test_print_log_not_called_when_no_tests_run( def test_print_log_called_when_mixed_available_unavailable( self, mock_popen, mock_stdout ): - """Test that print_log is called when some tests run, some unavailable.""" + """Test print_log with some tests run, some unavailable.""" # Mock Popen to return successful test results mock_process = MagicMock() mock_process.communicate.return_value = ( @@ -181,8 +301,242 @@ def test_print_log_called_when_mixed_available_unavailable( # Also verify that unavailable test message appears self.assertIn("Unavailable Tests: 1", output) + @patch("sys.stdout", new_callable=StringIO) + @patch("checkbox_support.scripts.fwts_test.filter_available_tests") + def test_hwe_option_extends_requested_tests( + self, mock_filter, mock_stdout + ): + """Test that --hwe option extends requested_tests with HWE_TESTS.""" + # Mock filter_available_tests to return some available tests + mock_filter.return_value = (["acpitests", "version"], []) + + # Run main with --hwe option + with patch( + "sys.argv", + [ + "fwts_test.py", + "--hwe", + "--log", + self.logfile.name, + ], + ): + main() + + # Verify that filter_available_tests was called with HWE_TESTS + # We can't directly check the args, but we can verify the function was called + mock_filter.assert_called_once() + + # Verify that print_log was called (log content should be in stdout) + output = mock_stdout.getvalue() + self.assertIn("Test log content", output) + + @patch("sys.stdout", new_callable=StringIO) + @patch("checkbox_support.scripts.fwts_test.filter_available_tests") + def test_qa_option_extends_requested_tests(self, mock_filter, mock_stdout): + """Test that --qa option extends requested_tests with QA_TESTS.""" + # Mock filter_available_tests to return some available tests + mock_filter.return_value = (["acpitests", "version"], []) + + # Run main with --qa option + with patch( + "sys.argv", + [ + "fwts_test.py", + "--qa", + "--log", + self.logfile.name, + ], + ): + main() + + # Verify that filter_available_tests was called + mock_filter.assert_called_once() + + # Verify that print_log was called (log content should be in stdout) + output = mock_stdout.getvalue() + self.assertIn("Test log content", output) + + @patch("sys.stdout", new_callable=StringIO) + @patch("checkbox_support.scripts.fwts_test.filter_available_tests") + def test_server_option_extends_requested_tests( + self, mock_filter, mock_stdout + ): + """Test that --server option extends requested_tests with SERVER_TESTS.""" + # Mock filter_available_tests to return some available tests + mock_filter.return_value = (["acpitests", "version"], []) + + # Run main with --server option + with patch( + "sys.argv", + [ + "fwts_test.py", + "--server", + "--log", + self.logfile.name, + ], + ): + main() + + # Verify that filter_available_tests was called + mock_filter.assert_called_once() + + # Verify that print_log was called (log content should be in stdout) + output = mock_stdout.getvalue() + self.assertIn("Test log content", output) + def tearDown(self): try: os.unlink(self.logfile.name) except OSError: pass + + +class GetAvailableFwtsTestsTest(unittest.TestCase): + """Test the get_available_fwts_tests function.""" + + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_get_available_fwts_tests_success(self, mock_popen): + """Test successful retrieval of available FWTS tests.""" + # Mock successful command execution + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"acpitests\nversion\nmtrr\nvirt\n", + b"", + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + result = get_available_fwts_tests() + + # Verify the command was called correctly + mock_popen.assert_called_once_with( + "fwts --show-tests", stdout=PIPE, stderr=PIPE, shell=True + ) + + # Verify the result + self.assertEqual(result, ["acpitests", "version", "mtrr", "virt"]) + + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_get_available_fwts_tests_with_section_headers(self, mock_popen): + """Test parsing output with section headers (lines ending with ':').""" + # Mock output with section headers that should be ignored + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"ACPI Tests:\nacpitests\nversion\n\nUEFI Tests:\nmtrr\nvirt\n", + b"", + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + result = get_available_fwts_tests() + + self.assertEqual(result, ["acpitests", "version", "mtrr", "virt"]) + + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_get_available_fwts_tests_with_empty_lines(self, mock_popen): + """Test parsing output with empty lines.""" + # Mock output with empty lines that should be ignored + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"acpitests\n\nversion\n\n\nmtrr\n", + b"", + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + result = get_available_fwts_tests() + + self.assertEqual(result, ["acpitests", "version", "mtrr"]) + + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_get_available_fwts_tests_with_whitespace(self, mock_popen): + """Test parsing output with leading/trailing whitespace.""" + # Mock output with whitespace that should be handled + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b" acpitests \n\tversion\n mtrr \n", + b"", + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + result = get_available_fwts_tests() + + self.assertEqual(result, ["acpitests", "version", "mtrr"]) + + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_get_available_fwts_tests_with_multiple_words(self, mock_popen): + """Test parsing output where lines have multiple words (takes first word).""" + # Mock output with multiple words per line + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"acpitests - ACPI tests\nversion - Version info\n" + b"mtrr - MTRR tests\n", + b"", + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + result = get_available_fwts_tests() + + self.assertEqual(result, ["acpitests", "version", "mtrr"]) + + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_get_available_fwts_tests_removes_duplicates(self, mock_popen): + """Test that duplicate test names are removed while preserving order.""" + # Mock output with duplicates + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"acpitests\nversion\nacpitests\nmtrr\nversion\n", + b"", + ) + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + result = get_available_fwts_tests() + + self.assertEqual(result, ["acpitests", "version", "mtrr"]) + + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_get_available_fwts_tests_command_failure(self, mock_popen): + """Test handling of command failure.""" + # Mock command failure + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"", + b"fwts: command not found", + ) + mock_process.returncode = 1 + mock_popen.return_value = mock_process + + with self.assertRaises(RuntimeError) as context: + get_available_fwts_tests() + + self.assertIn("FWTS command failed", str(context.exception)) + self.assertIn("fwts: command not found", str(context.exception)) + + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_get_available_fwts_tests_empty_output(self, mock_popen): + """Test handling of empty output.""" + # Mock empty output + mock_process = MagicMock() + mock_process.communicate.return_value = (b"", b"") + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + result = get_available_fwts_tests() + + self.assertEqual(result, []) + + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_get_available_fwts_tests_only_whitespace(self, mock_popen): + """Test handling of output with only whitespace.""" + # Mock output with only whitespace + mock_process = MagicMock() + mock_process.communicate.return_value = (b" \n\t\n \n", b"") + mock_process.returncode = 0 + mock_popen.return_value = mock_process + + result = get_available_fwts_tests() + + self.assertEqual(result, []) From 089433ea2723d52c07554db34ea7d40b70ee1b66 Mon Sep 17 00:00:00 2001 From: isaacyang Date: Tue, 8 Jul 2025 16:16:28 +0800 Subject: [PATCH 4/7] fwts_test: Fix test failures by mocking Popen return values for test options --- .../scripts/tests/test_fwts_test.py | 102 +++++++++++++----- 1 file changed, 74 insertions(+), 28 deletions(-) diff --git a/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py b/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py index e40cad7a88..85f1e3a3a6 100644 --- a/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py @@ -302,13 +302,36 @@ def test_print_log_called_when_mixed_available_unavailable( self.assertIn("Unavailable Tests: 1", output) @patch("sys.stdout", new_callable=StringIO) - @patch("checkbox_support.scripts.fwts_test.filter_available_tests") - def test_hwe_option_extends_requested_tests( - self, mock_filter, mock_stdout - ): + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_hwe_option_extends_requested_tests(self, mock_popen, mock_stdout): """Test that --hwe option extends requested_tests with HWE_TESTS.""" - # Mock filter_available_tests to return some available tests - mock_filter.return_value = (["acpitests", "version"], []) + # Mock fwts --show-tests to return some available tests + mock_fwts_process = MagicMock() + mock_fwts_process.communicate.return_value = ( + b"acpitests\nversion\nmtrr\nvirt\napicedge\nklog\noops\n", + None, + ) + mock_fwts_process.returncode = 0 + + # Mock Popen to return successful test results for each test + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"PASSED: Test completed successfully", + None, + ) + mock_process.returncode = 0 + + # Configure Popen to return different results for different calls + # First call is for fwts --show-tests, then one for each test in HWE_TESTS + mock_popen.side_effect = [ + mock_fwts_process, # fwts --show-tests + mock_process, # version + mock_process, # mtrr + mock_process, # virt + mock_process, # apicedge + mock_process, # klog + mock_process, # oops + ] # Run main with --hwe option with patch( @@ -322,22 +345,37 @@ def test_hwe_option_extends_requested_tests( ): main() - # Verify that filter_available_tests was called with HWE_TESTS - # We can't directly check the args, but we can verify the function was called - mock_filter.assert_called_once() - # Verify that print_log was called (log content should be in stdout) output = mock_stdout.getvalue() self.assertIn("Test log content", output) @patch("sys.stdout", new_callable=StringIO) - @patch("checkbox_support.scripts.fwts_test.filter_available_tests") - def test_qa_option_extends_requested_tests(self, mock_filter, mock_stdout): + @patch("checkbox_support.scripts.fwts_test.Popen") + def test_qa_option_extends_requested_tests(self, mock_popen, mock_stdout): """Test that --qa option extends requested_tests with QA_TESTS.""" - # Mock filter_available_tests to return some available tests - mock_filter.return_value = (["acpitests", "version"], []) + # Mock fwts --show-tests to return all QA_TESTS + from checkbox_support.scripts.fwts_test import QA_TESTS + + mock_fwts_process = MagicMock() + mock_fwts_process.communicate.return_value = ( + ("\n".join(QA_TESTS)).encode(), + None, + ) + mock_fwts_process.returncode = 0 + + # Mock Popen to return successful test results for each test + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"PASSED: Test completed successfully", + None, + ) + mock_process.returncode = 0 + + # Provide enough mocks: 1 for fwts --show-tests, then one for each QA_TESTS + mock_popen.side_effect = [mock_fwts_process] + [mock_process] * len( + QA_TESTS + ) - # Run main with --qa option with patch( "sys.argv", [ @@ -349,23 +387,35 @@ def test_qa_option_extends_requested_tests(self, mock_filter, mock_stdout): ): main() - # Verify that filter_available_tests was called - mock_filter.assert_called_once() - - # Verify that print_log was called (log content should be in stdout) output = mock_stdout.getvalue() self.assertIn("Test log content", output) @patch("sys.stdout", new_callable=StringIO) - @patch("checkbox_support.scripts.fwts_test.filter_available_tests") + @patch("checkbox_support.scripts.fwts_test.Popen") def test_server_option_extends_requested_tests( - self, mock_filter, mock_stdout + self, mock_popen, mock_stdout ): """Test that --server option extends requested_tests with SERVER_TESTS.""" - # Mock filter_available_tests to return some available tests - mock_filter.return_value = (["acpitests", "version"], []) + from checkbox_support.scripts.fwts_test import SERVER_TESTS + + mock_fwts_process = MagicMock() + mock_fwts_process.communicate.return_value = ( + ("\n".join(SERVER_TESTS)).encode(), + None, + ) + mock_fwts_process.returncode = 0 + + mock_process = MagicMock() + mock_process.communicate.return_value = ( + b"PASSED: Test completed successfully", + None, + ) + mock_process.returncode = 0 + + mock_popen.side_effect = [mock_fwts_process] + [mock_process] * len( + SERVER_TESTS + ) - # Run main with --server option with patch( "sys.argv", [ @@ -377,10 +427,6 @@ def test_server_option_extends_requested_tests( ): main() - # Verify that filter_available_tests was called - mock_filter.assert_called_once() - - # Verify that print_log was called (log content should be in stdout) output = mock_stdout.getvalue() self.assertIn("Test log content", output) From 46f01c25ab8a883d09960a320c2acaa0e098f697 Mon Sep 17 00:00:00 2001 From: isaacyang Date: Tue, 8 Jul 2025 16:22:52 +0800 Subject: [PATCH 5/7] fwts_test: fix tiny issue. --- checkbox-support/checkbox_support/scripts/fwts_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/checkbox-support/checkbox_support/scripts/fwts_test.py b/checkbox-support/checkbox_support/scripts/fwts_test.py index 1017622a2a..968fbe21e2 100644 --- a/checkbox-support/checkbox_support/scripts/fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/fwts_test.py @@ -180,7 +180,7 @@ def get_available_fwts_tests(): stdout, stderr = result.communicate() if result.returncode != 0: - raise RuntimeError(f"FWTS command failed: {stderr.decode()}") + raise RuntimeError("FWTS command failed: {}".format(stderr.decode())) # Parse the output to extract test names output_lines = stdout.decode().strip().split("\n") From 4100fef66d7509c444d04ea232065d7d213f4030 Mon Sep 17 00:00:00 2001 From: isaacyang Date: Tue, 8 Jul 2025 16:36:01 +0800 Subject: [PATCH 6/7] fwts_test:avoid the duplicated test in the available tests from fwts --show-tests --- checkbox-support/checkbox_support/scripts/fwts_test.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/checkbox-support/checkbox_support/scripts/fwts_test.py b/checkbox-support/checkbox_support/scripts/fwts_test.py index 968fbe21e2..05fc622297 100644 --- a/checkbox-support/checkbox_support/scripts/fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/fwts_test.py @@ -185,16 +185,18 @@ def get_available_fwts_tests(): # Parse the output to extract test names output_lines = stdout.decode().strip().split("\n") available_tests = [] + seen_tests = set() for line in output_lines: # Skip empty lines and section headers (lines ending with ':') if line.strip() and not line.endswith(":"): # Extract the first word as the test name test_name = line.lstrip().split()[0] - available_tests.append(test_name) + if test_name not in seen_tests: + available_tests.append(test_name) + seen_tests.add(test_name) - # Remove duplicates while preserving order - return list(dict.fromkeys(available_tests)) + return available_tests def get_sleep_times(log, start_marker): From f9b7b7ce0c594c39729f5b7ab040b0f66e4a7264 Mon Sep 17 00:00:00 2001 From: isaacyang Date: Wed, 9 Jul 2025 14:45:42 +0800 Subject: [PATCH 7/7] Polish it --- .../checkbox_support/scripts/fwts_test.py | 10 ++++------ .../scripts/tests/test_fwts_test.py | 16 ++++++++-------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/checkbox-support/checkbox_support/scripts/fwts_test.py b/checkbox-support/checkbox_support/scripts/fwts_test.py index 05fc622297..0cfc12b387 100644 --- a/checkbox-support/checkbox_support/scripts/fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/fwts_test.py @@ -184,17 +184,15 @@ def get_available_fwts_tests(): # Parse the output to extract test names output_lines = stdout.decode().strip().split("\n") - available_tests = [] - seen_tests = set() + available_tests = set() for line in output_lines: # Skip empty lines and section headers (lines ending with ':') if line.strip() and not line.endswith(":"): # Extract the first word as the test name test_name = line.lstrip().split()[0] - if test_name not in seen_tests: - available_tests.append(test_name) - seen_tests.add(test_name) + if test_name not in available_tests: + available_tests.add(test_name) return available_tests @@ -305,7 +303,7 @@ def filter_available_tests(requested_tests): present in the current system's available FWTS tests, and unavailable_tests are those not present. """ - available_tests_set = set(get_available_fwts_tests()) + available_tests_set = get_available_fwts_tests() available = [ test for test in requested_tests if test in available_tests_set ] diff --git a/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py b/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py index 85f1e3a3a6..113e50186d 100644 --- a/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py +++ b/checkbox-support/checkbox_support/scripts/tests/test_fwts_test.py @@ -460,7 +460,7 @@ def test_get_available_fwts_tests_success(self, mock_popen): ) # Verify the result - self.assertEqual(result, ["acpitests", "version", "mtrr", "virt"]) + self.assertSetEqual(result, {"acpitests", "version", "mtrr", "virt"}) @patch("checkbox_support.scripts.fwts_test.Popen") def test_get_available_fwts_tests_with_section_headers(self, mock_popen): @@ -476,7 +476,7 @@ def test_get_available_fwts_tests_with_section_headers(self, mock_popen): result = get_available_fwts_tests() - self.assertEqual(result, ["acpitests", "version", "mtrr", "virt"]) + self.assertSetEqual(result, {"acpitests", "version", "mtrr", "virt"}) @patch("checkbox_support.scripts.fwts_test.Popen") def test_get_available_fwts_tests_with_empty_lines(self, mock_popen): @@ -492,7 +492,7 @@ def test_get_available_fwts_tests_with_empty_lines(self, mock_popen): result = get_available_fwts_tests() - self.assertEqual(result, ["acpitests", "version", "mtrr"]) + self.assertSetEqual(result, {"acpitests", "version", "mtrr"}) @patch("checkbox_support.scripts.fwts_test.Popen") def test_get_available_fwts_tests_with_whitespace(self, mock_popen): @@ -508,7 +508,7 @@ def test_get_available_fwts_tests_with_whitespace(self, mock_popen): result = get_available_fwts_tests() - self.assertEqual(result, ["acpitests", "version", "mtrr"]) + self.assertSetEqual(result, {"acpitests", "version", "mtrr"}) @patch("checkbox_support.scripts.fwts_test.Popen") def test_get_available_fwts_tests_with_multiple_words(self, mock_popen): @@ -525,7 +525,7 @@ def test_get_available_fwts_tests_with_multiple_words(self, mock_popen): result = get_available_fwts_tests() - self.assertEqual(result, ["acpitests", "version", "mtrr"]) + self.assertSetEqual(result, {"acpitests", "version", "mtrr"}) @patch("checkbox_support.scripts.fwts_test.Popen") def test_get_available_fwts_tests_removes_duplicates(self, mock_popen): @@ -541,7 +541,7 @@ def test_get_available_fwts_tests_removes_duplicates(self, mock_popen): result = get_available_fwts_tests() - self.assertEqual(result, ["acpitests", "version", "mtrr"]) + self.assertSetEqual(result, {"acpitests", "version", "mtrr"}) @patch("checkbox_support.scripts.fwts_test.Popen") def test_get_available_fwts_tests_command_failure(self, mock_popen): @@ -572,7 +572,7 @@ def test_get_available_fwts_tests_empty_output(self, mock_popen): result = get_available_fwts_tests() - self.assertEqual(result, []) + self.assertEqual(result, set()) @patch("checkbox_support.scripts.fwts_test.Popen") def test_get_available_fwts_tests_only_whitespace(self, mock_popen): @@ -585,4 +585,4 @@ def test_get_available_fwts_tests_only_whitespace(self, mock_popen): result = get_available_fwts_tests() - self.assertEqual(result, []) + self.assertEqual(result, set())