Code review feedback.

Author: tjprescott

.github/skills/create-api-review-pr/SKILL.md

@@ -18,6 +18,7 @@ If the user asks to create an API review PR for a new package, explain that new
 3. The latest Node.js LTS must be installed.
 4. API.md workflow Node dependencies must be installed (`npm ci` from `scripts/api_md_workflow`).
 5. `azpysdk` must be installed (`pip install -e ./eng/tools/azure-sdk-tools`).
+6. ApiView stub generator dependencies must be installed (`pip install -r ./eng/apiview_reqs.txt`).
 
 ## Information to Gather
 

eng/tools/azure-sdk-tools/azpysdk/apistub.py

@@ -112,7 +112,9 @@ def run(self, args: argparse.Namespace) -> int:
             )
             logger.info(f"Processing {package_name} for apistub check")
 
-            if getattr(args, "install_deps", False):
+            install_deps = getattr(args, "install_deps", False) or getattr(args, "isolate", False)
+
+            if install_deps:
                 # install dependencies
                 self.install_dev_reqs(executable, args, package_dir)
 
@@ -143,7 +145,7 @@ def run(self, args: argparse.Namespace) -> int:
                     python_executable=executable,
                 )
 
-            if getattr(args, "install_deps", False):
+            if install_deps:
                 self.pip_freeze(executable)
 
             pkg_path = get_package_wheel_path(package_dir)

eng/tools/azure-sdk-tools/tests/test_apistub.py

@@ -73,16 +73,56 @@ def test_no_prebuilt_dir_falls_back_to_pkg_root(self, mock_find_whl, mock_parsed
 class TestRunOutputDirectory:
     """Verify that dest_dir controls where the output token path ends up."""
 
-    def _make_args(self, dest_dir=None, generate_md=False):
+    def _make_args(self, dest_dir=None, generate_md=False, isolate=False, install_deps=False):
         return argparse.Namespace(
             target=".",
-            isolate=False,
+            isolate=isolate,
             command="apistub",
             service=None,
             dest_dir=dest_dir,
             generate_md=generate_md,
+            install_deps=install_deps,
         )
 
+    @patch(
+        "azpysdk.apistub.REPO_ROOT", os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "..", ".."))
+    )
+    @patch("azpysdk.apistub.get_cross_language_mapping_path", return_value=None)
+    @patch("azpysdk.apistub.get_package_wheel_path", return_value="/fake/pkg.whl")
+    @patch("azpysdk.apistub.create_package_and_install")
+    @patch("azpysdk.apistub.install_into_venv")
+    @patch("azpysdk.apistub.set_envvar_defaults")
+    def test_isolate_installs_dependencies(
+        self, _env, install_into_venv, _create, _get_whl, _get_mapping, tmp_path, monkeypatch
+    ):
+        """When --isolate is passed, apistub should install dependencies."""
+        monkeypatch.chdir(os.getcwd())
+        stub = apistub()
+        staging = str(tmp_path / "staging")
+        os.makedirs(staging, exist_ok=True)
+        fake_parsed = MagicMock()
+        fake_parsed.folder = str(tmp_path)
+        fake_parsed.name = "azure-core"
+
+        with patch.object(stub, "get_targeted_directories", return_value=[fake_parsed]), patch.object(
+            stub, "get_executable", return_value=(sys.executable, staging)
+        ), patch.object(stub, "install_dev_reqs") as install_dev_reqs, patch.object(
+            stub, "pip_freeze"
+        ) as pip_freeze, patch.object(
+            stub, "run_venv_command"
+        ):
+            args = self._make_args(isolate=True)
+            stub.run(args)
+
+        install_dev_reqs.assert_called_once_with(sys.executable, args, str(tmp_path))
+        install_into_venv.assert_called_once()
+        repo_root = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "..", ".."))
+        assert install_into_venv.call_args.args[1][0:2] == [
+            "-r",
+            os.path.join(repo_root, "eng", "apiview_reqs.txt"),
+        ]
+        pip_freeze.assert_called_once_with(sys.executable)
+
     @patch(
         "azpysdk.apistub.REPO_ROOT", os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..", "..", ".."))
     )