[update-checkout] refactor to use pathlib.Path instead of os.path

utils/update_checkout/tests/test_locked_repository.py

@@ -1,10 +1,13 @@
+from pathlib import Path
 import unittest
 from unittest.mock import patch
 
 from update_checkout.update_checkout import UpdateArguments, _is_any_repository_locked
 
+FAKE_PATH = Path("/fake_path")
 
-def _update_arguments_with_fake_path(repo_name: str, path: str) -> UpdateArguments:
+
+def _update_arguments_with_fake_path(repo_name: str, path: Path) -> UpdateArguments:
     return UpdateArguments(
         repo_name=repo_name,
         source_root=path,
@@ -23,87 +26,93 @@ def _update_arguments_with_fake_path(repo_name: str, path: str) -> UpdateArgumen
 
 
 class TestIsAnyRepositoryLocked(unittest.TestCase):
-    @patch("os.path.exists")
-    @patch("os.path.isdir")
-    @patch("os.listdir")
-    def test_repository_with_lock_file(self, mock_listdir, mock_isdir, mock_exists):
+    @patch("pathlib.Path.exists", autospec=True)
+    @patch("pathlib.Path.is_dir", autospec=True)
+    @patch("pathlib.Path.iterdir", autospec=True)
+    def test_repository_with_lock_file(self, mock_iterdir, mock_is_dir, mock_exists):
         pool_args = [
-            _update_arguments_with_fake_path("repo1", "/fake_path"),
-            _update_arguments_with_fake_path("repo2", "/fake_path"),
+            _update_arguments_with_fake_path("repo1", FAKE_PATH),
+            _update_arguments_with_fake_path("repo2", FAKE_PATH),
         ]
 
-        def listdir_side_effect(path):
-            if "repo1" in path:
-                return ["index.lock", "config"]
-            elif "repo2" in path:
-                return ["HEAD", "config"]
+        def iterdir_side_effect(path: Path):
+            if "repo1" in path.as_posix():
+                return [path.joinpath("index.lock"), path.joinpath("config")]
+            elif "repo2" in path.as_posix():
+                return [path.joinpath("HEAD"), path.joinpath("config")]
             return []
 
         mock_exists.return_value = True
-        mock_isdir.return_value = True
-        mock_listdir.side_effect = listdir_side_effect
+        mock_is_dir.return_value = True
+        mock_iterdir.side_effect = iterdir_side_effect
 
         result = _is_any_repository_locked(pool_args)
         self.assertEqual(result, {"repo1"})
 
-    @patch("os.path.exists")
-    @patch("os.path.isdir")
-    @patch("os.listdir")
-    def test_repository_without_git_dir(self, mock_listdir, mock_isdir, mock_exists):
+    @patch("pathlib.Path.exists")
+    @patch("pathlib.Path.is_dir")
+    @patch("pathlib.Path.iterdir")
+    def test_repository_without_git_dir(self, mock_iterdir, mock_is_dir, mock_exists):
         pool_args = [
-            _update_arguments_with_fake_path("repo1", "/fake_path"),
+            _update_arguments_with_fake_path("repo1", FAKE_PATH),
         ]
 
         mock_exists.return_value = False
-        mock_isdir.return_value = False
-        mock_listdir.return_value = []
+        mock_is_dir.return_value = False
+        mock_iterdir.return_value = []
 
         result = _is_any_repository_locked(pool_args)
         self.assertEqual(result, set())
 
-    @patch("os.path.exists")
-    @patch("os.path.isdir")
-    @patch("os.listdir")
-    def test_repository_with_git_file(self, mock_listdir, mock_isdir, mock_exists):
+    @patch("pathlib.Path.exists")
+    @patch("pathlib.Path.is_dir")
+    @patch("pathlib.Path.iterdir")
+    def test_repository_with_git_file(self, mock_iterdir, mock_is_dir, mock_exists):
         pool_args = [
-            _update_arguments_with_fake_path("repo1", "/fake_path"),
+            _update_arguments_with_fake_path("repo1", FAKE_PATH),
         ]
 
         mock_exists.return_value = True
-        mock_isdir.return_value = False
-        mock_listdir.return_value = []
+        mock_is_dir.return_value = False
+        mock_iterdir.return_value = []
 
         result = _is_any_repository_locked(pool_args)
         self.assertEqual(result, set())
 
-    @patch("os.path.exists")
-    @patch("os.path.isdir")
-    @patch("os.listdir")
+    @patch("pathlib.Path.exists")
+    @patch("pathlib.Path.is_dir")
+    @patch("pathlib.Path.iterdir")
     def test_repository_with_multiple_lock_files(
-        self, mock_listdir, mock_isdir, mock_exists
+        self, mock_iterdir, mock_is_dir, mock_exists
     ):
         pool_args = [
-            _update_arguments_with_fake_path("repo1", "/fake_path"),
+            _update_arguments_with_fake_path("repo1", FAKE_PATH),
         ]
 
         mock_exists.return_value = True
-        mock_isdir.return_value = True
-        mock_listdir.return_value = ["index.lock", "merge.lock", "HEAD"]
+        mock_is_dir.return_value = True
+        mock_iterdir.return_value = [
+            FAKE_PATH.joinpath(x) for x in ("index.lock", "merge.lock", "HEAD")
+        ]
 
         result = _is_any_repository_locked(pool_args)
         self.assertEqual(result, {"repo1"})
 
-    @patch("os.path.exists")
-    @patch("os.path.isdir")
-    @patch("os.listdir")
-    def test_repository_with_no_lock_files(self, mock_listdir, mock_isdir, mock_exists):
+    @patch("pathlib.Path.exists")
+    @patch("pathlib.Path.is_dir")
+    @patch("pathlib.Path.iterdir")
+    def test_repository_with_no_lock_files(
+        self, mock_iterdir, mock_is_dir, mock_exists
+    ):
         pool_args = [
-            _update_arguments_with_fake_path("repo1", "/fake_path"),
+            _update_arguments_with_fake_path("repo1", FAKE_PATH),
         ]
 
         mock_exists.return_value = True
-        mock_isdir.return_value = True
-        mock_listdir.return_value = ["HEAD", "config", "logs"]
+        mock_is_dir.return_value = True
+        mock_iterdir.return_value = [
+            FAKE_PATH.joinpath(x) for x in ("HEAD", "config", "logs")
+        ]
 
         result = _is_any_repository_locked(pool_args)
         self.assertEqual(result, set())

utils/update_checkout/update_checkout/cli_arguments.py

@@ -1,4 +1,5 @@
 import argparse
+from pathlib import Path
 from typing import List, Optional
 
 from build_swift.build_swift.constants import SWIFT_SOURCE_ROOT
@@ -22,7 +23,7 @@ class CliArguments(argparse.Namespace):
     tag: Optional[str]
     match_timestamp: bool
     n_processes: int
-    source_root: str
+    source_root: Path
     use_submodules: bool
     verbose: bool
 
@@ -139,6 +140,7 @@ def parse_args() -> "CliArguments":
             "--source-root",
             help="The root directory to checkout repositories",
             default=SWIFT_SOURCE_ROOT,
+            type=Path,
         )
         parser.add_argument(
             "--use-submodules",

utils/update_checkout/update_checkout/git_command.py

@@ -1,4 +1,5 @@
 import os
+from pathlib import Path
 import shlex
 import subprocess
 import sys
@@ -44,7 +45,7 @@ def __str__(self):
 class Git:
     @staticmethod
     def run(
-        repo_path: str,
+        repo_path: Path,
         args: List[str],
         echo: bool = False,
         env: Optional[Dict[str, Any]] = None,
@@ -77,9 +78,7 @@ def run(
                     f"command `{command}` terminated with a non-zero exit "
                     f"status {str(e.returncode)}, aborting"
                 )
-            raise GitException(
-                e.returncode, command, os.path.basename(repo_path), output
-            )
+            raise GitException(e.returncode, command, repo_path.name, output)
         except OSError as e:
             if fatal:
                 sys.exit(

utils/update_checkout/update_checkout/runner_arguments.py

@@ -1,4 +1,5 @@
 from dataclasses import dataclass
+from pathlib import Path
 from typing import Any, Dict, List, Optional
 
 from .cli_arguments import CliArguments
@@ -14,7 +15,7 @@ class RunnerArguments:
 
 @dataclass
 class UpdateArguments(RunnerArguments):
-    source_root: str
+    source_root: Path
     config: Dict[str, Any]
     scheme_map: Any
     tag: Optional[str]