From f546ee068c26d106ab017af1c442e99eb70074e7 Mon Sep 17 00:00:00 2001 From: Devon Loehr Date: Tue, 14 Jan 2025 12:12:38 -0800 Subject: [PATCH] Fix argument mismatch in split_cl.py When adding tests, the LoadDescription function accidentally got called with the wrong arguments in the main function. This fixes that; a future CL (uploading shortly) adds more robust tests that would have caught this. Bug: 389568463, 389069356 Change-Id: Icaf5e83cd8caa9f3975173f8c8ee7d92ef44ee56 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/6170638 Commit-Queue: Andy Perelson Reviewed-by: Brian Ryner Reviewed-by: Andy Perelson Auto-Submit: Devon Loehr --- split_cl.py | 6 +++++- tests/split_cl_test.py | 16 +++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/split_cl.py b/split_cl.py index 23ee5a71a..fb521a9f0 100644 --- a/split_cl.py +++ b/split_cl.py @@ -218,8 +218,12 @@ def PrintClInfo(cl_index, num_cls, directories, file_paths, description, print() -def LoadDescription(description_file): +def LoadDescription(description_file, dry_run): if not description_file: + if not dry_run: + # Parser checks this as well, so should be impossible + raise ValueError( + "Must provide a description file except during dry runs") return ('Dummy description for dry run.\n' 'directory = $directory') diff --git a/tests/split_cl_test.py b/tests/split_cl_test.py index ab5cf9f46..923e22b2c 100755 --- a/tests/split_cl_test.py +++ b/tests/split_cl_test.py @@ -196,11 +196,21 @@ class SplitClTest(unittest.TestCase): @mock.patch("gclient_utils.FileRead", return_value="Description") def testLoadDescription(self, mock_file_read): # No description provided, use the dummy: - self.assertTrue(split_cl.LoadDescription(None).startswith("Dummy")) + self.assertTrue( + split_cl.LoadDescription(None, True).startswith("Dummy")) self.assertEqual(mock_file_read.call_count, 0) - # Description file provided, load it - self.assertEqual(split_cl.LoadDescription("SomeFile.txt"), + # No description provided during a real run + self.assertRaises(ValueError, split_cl.LoadDescription, None, False) + self.assertEqual(mock_file_read.call_count, 0) + + # Description file provided, load it regardless of dry run + self.assertEqual(split_cl.LoadDescription("SomeFile.txt", False), + "Description") + self.assertEqual(mock_file_read.call_count, 1) + + mock_file_read.reset_mock() + self.assertEqual(split_cl.LoadDescription("SomeFile.txt", True), "Description") self.assertEqual(mock_file_read.call_count, 1)