diff --git a/docs/source/cli.rst b/docs/source/cli.rst index ebd2a91a..14230b9a 100644 --- a/docs/source/cli.rst +++ b/docs/source/cli.rst @@ -71,11 +71,19 @@ Build JavaScript application ---------------------------- The ``build`` subcommand invokes ``npm run build``, optionally accepting -a target directory. +a target directory. By default, it uses the current working directory, +where it expects a regular NPM ``package.json`` file. .. code-block:: shell - responder build [] + responder build + +When specifying a target directory, responder will change to that +directory beforehand. + +.. code-block:: shell + + responder build /path/to/project .. _helloworld.py: https://github.com/kennethreitz/responder/blob/main/examples/helloworld.py diff --git a/responder/util/cmd.py b/responder/util/cmd.py index a11a94f0..4a9644dc 100644 --- a/responder/util/cmd.py +++ b/responder/util/cmd.py @@ -2,8 +2,32 @@ import shutil import subprocess import threading +from pathlib import Path -RESPONDER_BIN = shutil.which("responder") + +class ResponderProgram: + """ + Provide full path to the `responder` program. + """ + + @staticmethod + def path(): + program = shutil.which("responder") + if program is None: + raise RuntimeError("Could not find 'responder' executable in PATH") + return program + + @classmethod + def build(cls, path: Path): + """ + Invoke `responder build`, and return stdout and stderr. + """ + command = [ + cls.path(), + "build", + str(path), + ] + return subprocess.call(command) class ResponderServer(threading.Thread): @@ -13,13 +37,28 @@ class ResponderServer(threading.Thread): def __init__(self, target: str, port: int = 5042, limit_max_requests: int = None): super().__init__() + + # Sanity checks, as suggested by @coderabbitai. Thanks. + if not target or not isinstance(target, str): + raise ValueError("Target must be a non-empty string") + if not isinstance(port, int) or port < 1: + raise ValueError("Port must be a positive integer") + if limit_max_requests is not None and ( + not isinstance(limit_max_requests, int) or limit_max_requests < 1 + ): + raise ValueError("limit_max_requests must be a positive integer if specified") + self.target = target self.port = port self.limit_max_requests = limit_max_requests + # Allow the thread to be terminated when the main program exits. + self.process = None + self.daemon = True + def run(self): command = [ - RESPONDER_BIN, + ResponderProgram.path(), "run", self.target, ] @@ -28,4 +67,21 @@ def run(self): env = {} if self.port is not None: env = {"PORT": str(self.port)} - subprocess.check_call(command, env=env) + + self.process = subprocess.Popen( + command, + env=env, + universal_newlines=True, + ) + self.process.wait() + + def stop(self): + """ + Gracefully stop the process. + """ + if self.process and self.process.poll() is None: + self.process.terminate() + try: + self.process.wait(timeout=5) # Wait up to 5 seconds for graceful shutdown + except subprocess.TimeoutExpired: + self.process.kill() # Force kill if not terminated diff --git a/responder/util/python.py b/responder/util/python.py index be1591a4..7c1984f9 100644 --- a/responder/util/python.py +++ b/responder/util/python.py @@ -9,6 +9,14 @@ def load_target(target: str, default_property: str = "api", method: str = "run") Load Python code from file or module. """ + # Sanity checks, as suggested by @coderabbitai. Thanks. + if not target or ":" in target and len(target.split(":")) != 2: + raise ValueError( + "Invalid target format. " + "Use '', ':', " + "'/path/to/app.py', or '/path/to/app.py:" + ) + # Decode launch target location address. # Module: acme.app:foo # Path: /path/to/acme/app.py:foo @@ -21,8 +29,9 @@ def load_target(target: str, default_property: str = "api", method: str = "run") # Import launch target. Treat input location either as a filesystem path # (/path/to/acme/app.py), or as a module address specification (acme.app). - if Path(target).exists(): - app = load_file_module(target) + path = Path(target) + if path.is_file(): + app = load_file_module(path) else: app = importlib.import_module(target) @@ -36,19 +45,22 @@ def load_target(target: str, default_property: str = "api", method: str = "run") ) if not hasattr(api, method): raise AttributeError( - f"{msg_prefix}: API instance '{prop}' has no method 'run'" + f"{msg_prefix}: API instance '{prop}' has no method '{method}'" ) return api except ImportError as ex: raise ImportError(f"{msg_prefix}: {ex}") from ex -def load_file_module(module: str): +def load_file_module(path: Path): """ Load Python file as Python module. """ - spec = importlib.util.spec_from_file_location("__app__", module) + name = f"__{path.stem}__" + spec = importlib.util.spec_from_file_location(name, path) + if spec is None: + raise ImportError(f"Failed loading module from file: {path}") app = importlib.util.module_from_spec(spec) - sys.modules["__app__"] = app + sys.modules[name] = app spec.loader.exec_module(app) return app diff --git a/tests/test_cli.py b/tests/test_cli.py index f1fd7f72..4c6ce8f7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,13 +1,15 @@ # ruff: noqa: S603, S607 import json import subprocess -import time +from pathlib import Path import pytest import requests +from _pytest.capture import CaptureFixture from responder.__version__ import __version__ -from responder.util.cmd import RESPONDER_BIN, ResponderServer +from responder.util.cmd import ResponderProgram, ResponderServer +from tests.util import random_port, wait_server pytest.importorskip("docopt", reason="docopt-ng package not installed") @@ -25,28 +27,56 @@ def test_cli_version(capfd): assert stdout == __version__ -def test_cli_build(capfd, tmp_path): +def responder_build(path: Path, capfd: CaptureFixture): + ResponderProgram.build(path=path) + output = capfd.readouterr() + + stdout = output.out.strip() + stderr = output.err.strip() + + return stdout, stderr + + +def test_cli_build_success(capfd, tmp_path): """ Verify that `responder build` works as expected. """ # Temporary surrogate `package.json` file. - package_json = {"scripts": {"build": "echo foobar"}} + package_json = {"scripts": {"build": "echo Hotzenplotz"}} package_json_file = tmp_path / "package.json" package_json_file.write_text(json.dumps(package_json)) # Invoke `responder build`. - command = [ - RESPONDER_BIN, - "build", - str(tmp_path), - ] - subprocess.check_call(command) + stdout, stderr = responder_build(tmp_path, capfd) + assert "Hotzenplotz" in stdout - output = capfd.readouterr() - stdout = output.out.strip() - assert "foobar" in stdout +def test_cli_build_missing_package_json(capfd, tmp_path): + """ + Verify `responder build`, while `package.json` file is missing. + """ + + # Invoke `responder build`. + stdout, stderr = responder_build(tmp_path, capfd) + assert "npm error code ENOENT" in stderr + assert "Could not read package.json" in stderr + assert "Error: ENOENT: no such file or directory" in stderr + + +def test_cli_build_invalid_package_json(capfd, tmp_path): + """ + Verify `responder build` using an invalid `package.json` file. + """ + + # Temporary surrogate `package.json` file. + package_json_file = tmp_path / "package.json" + package_json_file.write_text("foobar") + + # Invoke `responder build`. + stdout, stderr = responder_build(tmp_path, capfd) + assert "npm error code EJSONPARSE" in stderr + assert '"foobar" is not valid JSON' in stderr def test_cli_run(capfd): @@ -58,14 +88,15 @@ def test_cli_run(capfd): # Start a Responder service instance in the background, using its CLI. # Make it terminate itself after serving one HTTP request. server = ResponderServer( - target="examples/helloworld.py", port=9876, limit_max_requests=1 + target="examples/helloworld.py", port=random_port(), limit_max_requests=1 ) - server.start() - time.sleep(0.5) - - response = requests.get(f"http://127.0.0.1:{server.port}/hello", timeout=1) - assert "hello, world!" == response.text - server.join() + try: + server.start() + wait_server(server.port) + response = requests.get(f"http://127.0.0.1:{server.port}/hello", timeout=1) + assert "hello, world!" == response.text + finally: + server.join() output = capfd.readouterr() diff --git a/tests/util.py b/tests/util.py new file mode 100644 index 00000000..3fb866a9 --- /dev/null +++ b/tests/util.py @@ -0,0 +1,30 @@ +import time + +import requests + + +def random_port(): + """ + Return a random available port. + """ + import socket + + sock = socket.socket() + sock.bind(("", 0)) + port = sock.getsockname()[1] + sock.close() + return port + + +def wait_server(port: int): + """ + Wait for server to be ready. + """ + for _ in range(20): # 2 second timeout + try: + requests.get(f"http://127.0.0.1:{port}/", timeout=0.1) + break + except requests.exceptions.RequestException: + time.sleep(0.1) + else: + raise RuntimeError("Server failed to start")