gpt-5.2 ChatGPT: refactor tools code into cli.tools.vscode and add unit tests

* Move VS Code workspace logic (incl. guards) from cli.commands.tools into cli.tools.vscode
* Extract shared repo path resolution into cli.tools.paths and reuse for explore/terminal
* Simplify cli.commands.tools to pure orchestration via open_vscode_workspace
* Update existing tools command unit test to assert delegation instead of patching removed internals
* Add new unit tests for cli.tools.paths and cli.tools.vscode (workspace creation, reuse, guard errors)

https://chatgpt.com/share/69419a6a-c9e4-800f-9538-b6652b2da6b3

tests/unit/pkgmgr/cli/commands/test_tools.py

@@ -1,11 +1,9 @@
 from __future__ import annotations
 
-import json
-import os
-import tempfile
 import unittest
 from types import SimpleNamespace
 from typing import Any, Dict, List
+from unittest.mock import call, patch
 
 from pkgmgr.cli.commands.tools import handle_tools_command
 
@@ -26,36 +24,23 @@ class TestHandleToolsCommand(unittest.TestCase):
     Unit tests for pkgmgr.cli.commands.tools.handle_tools_command.
 
     We focus on:
-      - Correct path resolution for repositories that have a 'directory' key.
-      - Correct shell commands for 'explore' and 'terminal'.
-      - Proper workspace creation and invocation of 'code' for the 'code' command.
+      - Correct path resolution and shell commands for 'explore' and 'terminal'.
+      - For 'code': delegation to pkgmgr.cli.tools.open_vscode_workspace.
     """
 
     def setUp(self) -> None:
-        # Two fake repositories with explicit 'directory' entries so that
-        # _resolve_repository_path() does not need to call get_repo_dir().
         self.repos: List[Repository] = [
             {"alias": "repo1", "directory": "/tmp/repo1"},
             {"alias": "repo2", "directory": "/tmp/repo2"},
         ]
 
-        # Minimal CLI context; only attributes used in tools.py are provided.
         self.ctx = SimpleNamespace(
             config_merged={"directories": {"workspaces": "~/Workspaces"}},
             all_repositories=self.repos,
             repositories_base_dir="/base/dir",
         )
 
-    # ------------------------------------------------------------------ #
-    # Helper
-    # ------------------------------------------------------------------ #
-
     def _patch_run_command(self):
-        """
-        Convenience context manager for patching run_command in tools module.
-        """
-        from unittest.mock import patch
-
         return patch("pkgmgr.cli.commands.tools.run_command")
 
     # ------------------------------------------------------------------ #
@@ -63,12 +48,6 @@ class TestHandleToolsCommand(unittest.TestCase):
     # ------------------------------------------------------------------ #
 
     def test_explore_uses_directory_paths(self) -> None:
-        """
-        The 'explore' command should call Nautilus with the resolved
-        repository paths and use '& disown' as in the implementation.
-        """
-        from unittest.mock import call
-
         args = _Args(command="explore")
 
         with self._patch_run_command() as mock_run_command:
@@ -85,12 +64,6 @@ class TestHandleToolsCommand(unittest.TestCase):
     # ------------------------------------------------------------------ #
 
     def test_terminal_uses_directory_paths(self) -> None:
-        """
-        The 'terminal' command should open a GNOME Terminal tab with the
-        repository as its working directory.
-        """
-        from unittest.mock import call
-
         args = _Args(command="terminal")
 
         with self._patch_run_command() as mock_run_command:
@@ -106,63 +79,10 @@ class TestHandleToolsCommand(unittest.TestCase):
     # Tests for 'code'
     # ------------------------------------------------------------------ #
 
-    def test_code_creates_workspace_and_calls_code(self) -> None:
-        """
-        The 'code' command should:
-
-          - Build a workspace file name from sorted repository identifiers.
-          - Resolve the repository paths into VS Code 'folders'.
-          - Create the workspace file if it does not exist.
-          - Call 'code "<workspace_file>"' via run_command.
-        """
-        from unittest.mock import patch
-
+    def test_code_delegates_to_open_vscode_workspace(self) -> None:
         args = _Args(command="code")
 
-        with tempfile.TemporaryDirectory() as tmpdir:
-            # Patch expanduser so that the configured '~/Workspaces'
-            # resolves into our temporary directory.
-            with patch(
-                "pkgmgr.cli.commands.tools.os.path.expanduser"
-            ) as mock_expanduser:
-                mock_expanduser.return_value = tmpdir
+        with patch("pkgmgr.cli.commands.tools.open_vscode_workspace") as m:
+            handle_tools_command(args, self.ctx, self.repos)
 
-                # Patch get_repo_identifier so the resulting workspace file
-                # name is deterministic and easy to assert.
-                with patch(
-                    "pkgmgr.cli.commands.tools.get_repo_identifier"
-                ) as mock_get_identifier:
-                    mock_get_identifier.side_effect = ["repo-b", "repo-a"]
-
-                    with self._patch_run_command() as mock_run_command:
-                        handle_tools_command(args, self.ctx, self.repos)
-
-                    # The identifiers are ['repo-b', 'repo-a'], which are
-                    # sorted to ['repo-a', 'repo-b'] and joined with '_'.
-                    expected_workspace_name = "repo-a_repo-b.code-workspace"
-                    expected_workspace_file = os.path.join(
-                        tmpdir, expected_workspace_name
-                    )
-
-                    # Workspace file should have been created.
-                    self.assertTrue(
-                        os.path.exists(expected_workspace_file),
-                        "Workspace file was not created.",
-                    )
-
-                    # The content of the workspace must be valid JSON with
-                    # the expected folder paths.
-                    with open(expected_workspace_file, "r", encoding="utf-8") as f:
-                        data = json.load(f)
-
-                    self.assertIn("folders", data)
-                    folder_paths = {f["path"] for f in data["folders"]}
-                    self.assertEqual(
-                        folder_paths,
-                        {"/tmp/repo1", "/tmp/repo2"},
-                    )
-
-                    # And VS Code must have been invoked with that workspace.
-                    mock_run_command.assert_called_once_with(
-                        f'code "{expected_workspace_file}"'
-                    )
+        m.assert_called_once_with(self.ctx, self.repos)

tests/unit/pkgmgr/cli/tools/test_paths.py

@@ -0,0 +1,38 @@
+from __future__ import annotations
+
+import unittest
+from types import SimpleNamespace
+from unittest.mock import patch
+
+
+class TestResolveRepositoryPath(unittest.TestCase):
+    def test_explicit_directory_key_wins(self) -> None:
+        from pkgmgr.cli.tools.paths import resolve_repository_path
+
+        ctx = SimpleNamespace(repositories_base_dir="/base", repositories_dir="/base2")
+        repo = {"directory": "/explicit/repo"}
+
+        self.assertEqual(resolve_repository_path(repo, ctx), "/explicit/repo")
+
+    def test_fallback_uses_get_repo_dir_with_repositories_base_dir(self) -> None:
+        from pkgmgr.cli.tools.paths import resolve_repository_path
+
+        ctx = SimpleNamespace(repositories_base_dir="/base", repositories_dir="/base2")
+        repo = {"provider": "github.com", "account": "acme", "repository": "demo"}
+
+        with patch("pkgmgr.cli.tools.paths.get_repo_dir", return_value="/computed/repo") as m:
+            out = resolve_repository_path(repo, ctx)
+
+        self.assertEqual(out, "/computed/repo")
+        m.assert_called_once_with("/base", repo)
+
+    def test_raises_if_no_base_dir_in_context(self) -> None:
+        from pkgmgr.cli.tools.paths import resolve_repository_path
+
+        ctx = SimpleNamespace(repositories_base_dir=None, repositories_dir=None)
+        repo = {"provider": "github.com", "account": "acme", "repository": "demo"}
+
+        with self.assertRaises(RuntimeError) as cm:
+            resolve_repository_path(repo, ctx)
+
+        self.assertIn("Cannot resolve repositories base directory", str(cm.exception))

tests/unit/pkgmgr/cli/tools/test_vscode.py

@@ -0,0 +1,131 @@
+from __future__ import annotations
+
+import json
+import os
+import tempfile
+import unittest
+from types import SimpleNamespace
+from typing import Any, Dict, List
+from unittest.mock import patch
+
+Repository = Dict[str, Any]
+
+
+class TestOpenVSCodeWorkspace(unittest.TestCase):
+    def test_no_selected_repos_prints_message_and_returns(self) -> None:
+        from pkgmgr.cli.tools.vscode import open_vscode_workspace
+
+        ctx = SimpleNamespace(config_merged={}, all_repositories=[])
+
+        with patch("builtins.print") as p:
+            open_vscode_workspace(ctx, [])
+
+        p.assert_called_once()
+        self.assertIn("No repositories selected.", str(p.call_args[0][0]))
+
+    def test_raises_if_code_cli_missing(self) -> None:
+        from pkgmgr.cli.tools.vscode import open_vscode_workspace
+
+        ctx = SimpleNamespace(config_merged={}, all_repositories=[])
+        selected: List[Repository] = [{"provider": "github.com", "account": "x", "repository": "y"}]
+
+        with patch("pkgmgr.cli.tools.vscode.shutil.which", return_value=None):
+            with self.assertRaises(RuntimeError) as cm:
+                open_vscode_workspace(ctx, selected)
+
+        self.assertIn("VS Code CLI ('code') not found", str(cm.exception))
+
+    def test_raises_if_identifier_contains_slash(self) -> None:
+        from pkgmgr.cli.tools.vscode import open_vscode_workspace
+
+        ctx = SimpleNamespace(
+            config_merged={"directories": {"workspaces": "~/Workspaces"}},
+            all_repositories=[],
+        )
+        selected: List[Repository] = [{"provider": "github.com", "account": "x", "repository": "y"}]
+
+        with patch("pkgmgr.cli.tools.vscode.shutil.which", return_value="/usr/bin/code"), patch(
+            "pkgmgr.cli.tools.vscode.get_repo_identifier",
+            return_value="github.com/x/y",
+        ):
+            with self.assertRaises(RuntimeError) as cm:
+                open_vscode_workspace(ctx, selected)
+
+        msg = str(cm.exception)
+        self.assertIn("not yet identified", msg)
+        self.assertIn("identifier contains '/'", msg)
+
+    def test_creates_workspace_file_and_calls_code(self) -> None:
+        from pkgmgr.cli.tools.vscode import open_vscode_workspace
+
+        with tempfile.TemporaryDirectory() as tmp:
+            workspaces_dir = os.path.join(tmp, "Workspaces")
+            repo_path = os.path.join(tmp, "Repos", "dotlinker")
+
+            ctx = SimpleNamespace(
+                config_merged={"directories": {"workspaces": workspaces_dir}},
+                all_repositories=[],
+                repositories_base_dir=os.path.join(tmp, "Repos"),
+            )
+            selected: List[Repository] = [
+                {"provider": "github.com", "account": "kevin", "repository": "dotlinker"}
+            ]
+
+            with patch("pkgmgr.cli.tools.vscode.shutil.which", return_value="/usr/bin/code"), patch(
+                "pkgmgr.cli.tools.vscode.get_repo_identifier",
+                return_value="dotlinker",
+            ), patch(
+                "pkgmgr.cli.tools.vscode.resolve_repository_path",
+                return_value=repo_path,
+            ), patch(
+                "pkgmgr.cli.tools.vscode.run_command"
+            ) as run_cmd:
+                open_vscode_workspace(ctx, selected)
+
+            workspace_file = os.path.join(workspaces_dir, "dotlinker.code-workspace")
+            self.assertTrue(os.path.exists(workspace_file))
+
+            with open(workspace_file, "r", encoding="utf-8") as f:
+                data = json.load(f)
+
+            self.assertEqual(data["folders"], [{"path": repo_path}])
+            self.assertEqual(data["settings"], {})
+
+            run_cmd.assert_called_once_with(f'code "{workspace_file}"')
+
+    def test_uses_existing_workspace_file_without_overwriting(self) -> None:
+        from pkgmgr.cli.tools.vscode import open_vscode_workspace
+
+        with tempfile.TemporaryDirectory() as tmp:
+            workspaces_dir = os.path.join(tmp, "Workspaces")
+            os.makedirs(workspaces_dir, exist_ok=True)
+
+            workspace_file = os.path.join(workspaces_dir, "dotlinker.code-workspace")
+            original = {"folders": [{"path": "/original"}], "settings": {"x": 1}}
+            with open(workspace_file, "w", encoding="utf-8") as f:
+                json.dump(original, f)
+
+            ctx = SimpleNamespace(
+                config_merged={"directories": {"workspaces": workspaces_dir}},
+                all_repositories=[],
+            )
+            selected: List[Repository] = [
+                {"provider": "github.com", "account": "kevin", "repository": "dotlinker"}
+            ]
+
+            with patch("pkgmgr.cli.tools.vscode.shutil.which", return_value="/usr/bin/code"), patch(
+                "pkgmgr.cli.tools.vscode.get_repo_identifier",
+                return_value="dotlinker",
+            ), patch(
+                "pkgmgr.cli.tools.vscode.resolve_repository_path",
+                return_value="/new/path",
+            ), patch(
+                "pkgmgr.cli.tools.vscode.run_command"
+            ) as run_cmd:
+                open_vscode_workspace(ctx, selected)
+
+            with open(workspace_file, "r", encoding="utf-8") as f:
+                data = json.load(f)
+
+            self.assertEqual(data, original)
+            run_cmd.assert_called_once_with(f'code "{workspace_file}"')