Improve the npm API's error handling and test coverage

betty/_npm.py

@@ -8,20 +8,21 @@
 
 import logging
 import sys
+from subprocess import CalledProcessError
 from typing import Sequence, TYPE_CHECKING, Self, final
+
 from typing_extensions import override
 
+from betty import subprocess
 from betty.error import UserFacingError
-from betty.locale.localizer import DEFAULT_LOCALIZER
 from betty.locale.localizable import _, Localizable
+from betty.locale.localizer import DEFAULT_LOCALIZER
 from betty.requirement import Requirement
-from betty.subprocess import run_process
 
 if TYPE_CHECKING:
     from pathlib import Path
     from asyncio import subprocess as aiosubprocess
 
-
 _NPM_SUMMARY_AVAILABLE = _("`npm` is available")
 _NPM_SUMMARY_UNAVAILABLE = _("`npm` is not available")
 _NPM_DETAILS = _(
@@ -42,7 +43,7 @@ async def npm(
     Run an npm command.
     """
     try:
-        return await run_process(
+        return await subprocess.run_process(
             ["npm", *arguments],
             cwd=cwd,
             # Use a shell on Windows so subprocess can find the executables it needs (see
@@ -69,6 +70,10 @@ async def new(cls) -> Self:
             )
             logging.getLogger(__name__).debug(_NPM_DETAILS.localize(DEFAULT_LOCALIZER))
             return cls(False)
+        except CalledProcessError as error:
+            logging.getLogger(__name__).exception(error)
+            logging.getLogger(__name__).debug(_NPM_DETAILS.localize(DEFAULT_LOCALIZER))
+            return cls(False)
         else:
             return cls(True)
 

betty/tests/test__npm.py

@@ -1,15 +1,41 @@
+from pathlib import Path
+from subprocess import CalledProcessError
+
+import pytest
 from pytest_mock import MockerFixture
 
-from betty._npm import NpmRequirement, NpmUnavailable
+from betty._npm import NpmRequirement, NpmUnavailable, npm
+
+
+class TestNpm:
+    async def test(self) -> None:
+        await npm(["--version"])
+
+    async def test_command_not_found(self, mocker: MockerFixture) -> None:
+        mocker.patch("betty.subprocess.run_process", side_effect=FileNotFoundError)
+        with pytest.raises(NpmUnavailable):
+            await npm(["--version"])
+
+    async def test_command_error(self, mocker: MockerFixture, tmp_path: Path) -> None:
+        mocker.patch(
+            "betty.subprocess.run_process",
+            side_effect=CalledProcessError(1, "", "", ""),
+        )
+        with pytest.raises(CalledProcessError):
+            await npm(["--version"])
 
 
 class TestNpmRequirement:
-    async def test_check_met(self) -> None:
+    async def test_is_met(self) -> None:
         sut = await NpmRequirement.new()
         assert sut.is_met()
 
-    async def test_check_unmet(self, mocker: MockerFixture) -> None:
-        m_npm = mocker.patch("betty._npm.npm")
-        m_npm.side_effect = NpmUnavailable()
+    async def test_is_met_with_command_not_found(self, mocker: MockerFixture) -> None:
+        mocker.patch("betty._npm.npm", side_effect=NpmUnavailable)
+        sut = await NpmRequirement.new()
+        assert not sut.is_met()
+
+    async def test_is_met_with_command_error(self, mocker: MockerFixture) -> None:
+        mocker.patch("betty._npm.npm", side_effect=CalledProcessError(1, "", "", ""))
         sut = await NpmRequirement.new()
         assert not sut.is_met()