sync: Refactor to use _RunOneGC and fix config leakagev2.63

Extract _RunOneGC to handle GC on a single project. This refactoring
makes it easier to invoke GC from parallel worker tasks.

Also, avoid modifying the passed-in config dictionary in _RunOneGC by
creating a local copy, preventing unintended side effects on other
commands sharing the same config.

Bug: 498290329
Change-Id: I7b77ed6629b14b5ee3322870b9c6c8ce2bfd6ea2
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/574923
Reviewed-by: Becky Siegel <beckysiegel@google.com>
Tested-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>

1 files changed, 19 insertions, 18 deletions

diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py
index a6be0807f..b3726d2d1 100644
--- a/tests/test_subcmds_sync.py
+++ b/tests/test_subcmds_sync.py
@@ -552,35 +552,36 @@ class GCProjectsTest(unittest.TestCase):
     def test_GCProjects_skip_gc(self, mock_progress):
         """Test that it skips GC if opt.auto_gc is False."""
         self.opt.auto_gc = False
-        self.cmd._SetPreciousObjectsState = mock.Mock()
-        self.cmd._GCProjects([self.project], self.opt, None)
-        self.cmd._SetPreciousObjectsState.assert_called_once_with(
-            self.project, self.opt
-        )
+        with mock.patch.object(
+            sync.Sync, "_SetPreciousObjectsState"
+        ) as mock_set_state:
+            self.cmd._GCProjects([self.project], self.opt, None)
+            mock_set_state.assert_called_once_with(self.project, self.opt)
         self.assertFalse(self.project.bare_git.gc.called)
 
     @mock.patch("subcmds.sync.Progress")
     def test_GCProjects_sequential(self, mock_progress):
         """Test sequential GC (jobs < 2)."""
-        self.cmd._SetPreciousObjectsState = mock.Mock()
-        self.cmd._GCProjects([self.project], self.opt, None)
-        self.project.config.SetString.assert_called_once_with(
-            "gc.autoDetach", "false"
+        with mock.patch.object(sync.Sync, "_SetPreciousObjectsState"):
+            self.cmd._GCProjects([self.project], self.opt, None)
+        self.project.bare_git.gc.assert_called_once_with(
+            "--auto", config={"gc.autoDetach": "false"}
         )
-        self.project.bare_git.gc.assert_called_once_with("--auto")
+        # Verify that gc.autoDetach was not permanently set in config.
+        for call in self.project.config.SetString.call_args_list:
+            self.assertNotEqual(call.args[0], "gc.autoDetach")
 
     @mock.patch("subcmds.sync.Progress")
     def test_GCProjects_parallel(self, mock_progress):
         """Test parallel GC (jobs >= 2)."""
         self.opt.jobs = 2
-        self.cmd._SetPreciousObjectsState = mock.Mock()
-
-        with mock.patch("subcmds.sync._threading.Thread") as mock_thread:
-            mock_t = mock.MagicMock()
-            mock_thread.return_value = mock_t
-            err_event = mock.Mock()
-            err_event.is_set.return_value = False
-            self.cmd._GCProjects([self.project], self.opt, err_event)
+        with mock.patch.object(sync.Sync, "_SetPreciousObjectsState"):
+            with mock.patch("subcmds.sync._threading.Thread") as mock_thread:
+                mock_t = mock.MagicMock()
+                mock_thread.return_value = mock_t
+                err_event = mock.Mock()
+                err_event.is_set.return_value = False
+                self.cmd._GCProjects([self.project], self.opt, err_event)
 
         self.assertTrue(mock_thread.called)