From e5903b5f29dbc0d3b2a964f04863df310e3363d3 Mon Sep 17 00:00:00 2001 From: Bedram Tamang Date: Wed, 10 Jun 2026 16:35:46 -0700 Subject: [PATCH 1/2] fix(serve): coerce --reload CLI string to bool via env() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When Cleo passes --reload=False/True as a string, the old code forwarded the raw string to uvicorn, which treated any non-empty string as truthy. Now the string is coerced with _env(cli_reload, cli_reload) from fastapi_startkit.environment, which maps "False"/"false" → False and "True"/"true" → True before passing the value to uvicorn. Adds tests/fastapi/test_serve_command.py covering: - None option falls back to fastapi config value - "False" → False - "True" → True - "false" (lowercase) → False Co-Authored-By: Claude Sonnet 4.6 --- .../fastapi/commands/serve_command.py | 4 +- .../tests/fastapi/test_serve_command.py | 85 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 fastapi_startkit/tests/fastapi/test_serve_command.py diff --git a/fastapi_startkit/src/fastapi_startkit/fastapi/commands/serve_command.py b/fastapi_startkit/src/fastapi_startkit/fastapi/commands/serve_command.py index be268cd5..e4552368 100644 --- a/fastapi_startkit/src/fastapi_startkit/fastapi/commands/serve_command.py +++ b/fastapi_startkit/src/fastapi_startkit/fastapi/commands/serve_command.py @@ -1,4 +1,5 @@ from fastapi_startkit.console.command import Command +from fastapi_startkit.environment import env as _env from cleo.helpers import option @@ -51,7 +52,8 @@ def handle(self): host = self.option("host") or cfg_host port = int(self.option("port") or cfg_port) - reload = cfg_reload if self.option("reload") is None else self.option("reload") + cli_reload = self.option("reload") + reload = cfg_reload if cli_reload is None else _env(cli_reload, cli_reload) app = self.option("app") exist = self.is_app_exist() diff --git a/fastapi_startkit/tests/fastapi/test_serve_command.py b/fastapi_startkit/tests/fastapi/test_serve_command.py new file mode 100644 index 00000000..51225c0c --- /dev/null +++ b/fastapi_startkit/tests/fastapi/test_serve_command.py @@ -0,0 +1,85 @@ +"""Tests for ServeCommand --reload bool coercion.""" + +from unittest.mock import patch + +from fastapi_startkit.configuration.config import Config +from fastapi_startkit.fastapi.commands.serve_command import ServeCommand + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +_CONFIG_DEFAULTS = { + "fastapi.host": "127.0.0.1", + "fastapi.port": 8000, + "fastapi.reload": True, + "fastapi.reload_dirs": None, + "fastapi.reload_excludes": None, +} + + +def _make_options(reload_value, host=None, port=None, app="bootstrap.application:app"): + """Return a side_effect function for ServeCommand.option().""" + mapping = { + "reload": reload_value, + "host": host, + "port": port, + "app": app, + } + return lambda key: mapping[key] + + +def _run_handle(reload_option_value, cfg_reload=True): + """ + Instantiate ServeCommand, mock its dependencies, call handle(), and return + the kwargs dict that was passed to uvicorn.run. + + Config and uvicorn are both imported inside handle(), so we patch them at + their canonical module paths rather than on the serve_command module. + """ + cmd = ServeCommand.__new__(ServeCommand) + + config_map = {**_CONFIG_DEFAULTS, "fastapi.reload": cfg_reload} + + with ( + patch.object(Config, "get", side_effect=lambda key, default=None: config_map.get(key, default)), + patch("uvicorn.run") as mock_uvicorn_run, + patch.object(ServeCommand, "is_app_exist", return_value=True), + patch.object(ServeCommand, "line"), + patch.object(ServeCommand, "option", side_effect=_make_options(reload_option_value)), + ): + cmd.handle() + return mock_uvicorn_run.call_args[1] + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + + +class TestServeCommandReloadCoercion: + def test_reload_none_falls_back_to_cfg_reload_true(self): + """When --reload is not passed, reload inherits the fastapi config value.""" + kwargs = _run_handle(reload_option_value=None, cfg_reload=True) + assert kwargs["reload"] is True + + def test_reload_none_falls_back_to_cfg_reload_false(self): + """When --reload is not passed and cfg is False, reload is False.""" + kwargs = _run_handle(reload_option_value=None, cfg_reload=False) + assert kwargs["reload"] is False + + def test_reload_string_False_becomes_bool_false(self): + """--reload=False (string) must be coerced to bool False.""" + kwargs = _run_handle(reload_option_value="False") + assert kwargs["reload"] is False + + def test_reload_string_True_becomes_bool_true(self): + """--reload=True (string) must be coerced to bool True.""" + kwargs = _run_handle(reload_option_value="True") + assert kwargs["reload"] is True + + def test_reload_string_false_lowercase_becomes_bool_false(self): + """--reload=false (lowercase) must be coerced to bool False.""" + kwargs = _run_handle(reload_option_value="false") + assert kwargs["reload"] is False From c299af5fc41908ba387f0523af175d5c6e94b44c Mon Sep 17 00:00:00 2001 From: Bedram Tamang Date: Wed, 10 Jun 2026 16:50:02 -0700 Subject: [PATCH 2/2] test(serve): rewrite reload tests using Cleo CommandTester Replace hand-rolled unittest.mock approach (patching option() and calling handle() directly) with CommandTester.execute() so Cleo drives the full command path naturally. External dependencies (uvicorn.run, Config.get, is_app_exist) are still patched; option() is no longer patched. Co-Authored-By: Claude Sonnet 4.6 --- .../tests/fastapi/test_serve_command.py | 76 +++++++++---------- 1 file changed, 34 insertions(+), 42 deletions(-) diff --git a/fastapi_startkit/tests/fastapi/test_serve_command.py b/fastapi_startkit/tests/fastapi/test_serve_command.py index 51225c0c..84371549 100644 --- a/fastapi_startkit/tests/fastapi/test_serve_command.py +++ b/fastapi_startkit/tests/fastapi/test_serve_command.py @@ -1,7 +1,9 @@ -"""Tests for ServeCommand --reload bool coercion.""" +"""Tests for ServeCommand --reload bool coercion (using Cleo CommandTester).""" from unittest.mock import patch +from cleo.testers.command_tester import CommandTester + from fastapi_startkit.configuration.config import Config from fastapi_startkit.fastapi.commands.serve_command import ServeCommand @@ -19,38 +21,28 @@ } -def _make_options(reload_value, host=None, port=None, app="bootstrap.application:app"): - """Return a side_effect function for ServeCommand.option().""" - mapping = { - "reload": reload_value, - "host": host, - "port": port, - "app": app, - } - return lambda key: mapping[key] - - -def _run_handle(reload_option_value, cfg_reload=True): +def _run(args: str, cfg_reload: bool = True) -> dict: """ - Instantiate ServeCommand, mock its dependencies, call handle(), and return - the kwargs dict that was passed to uvicorn.run. + Execute ServeCommand via CommandTester and return the kwargs dict that + was passed to uvicorn.run(). - Config and uvicorn are both imported inside handle(), so we patch them at - their canonical module paths rather than on the serve_command module. + CommandTester drives the full handle() path — option() is resolved by + Cleo from the parsed *args* string, so we never patch option() here. + External I/O (uvicorn, Config, is_app_exist) is patched so the server + never actually starts. """ - cmd = ServeCommand.__new__(ServeCommand) + cfg = {**_CONFIG_DEFAULTS, "fastapi.reload": cfg_reload} - config_map = {**_CONFIG_DEFAULTS, "fastapi.reload": cfg_reload} + cmd = ServeCommand() + tester = CommandTester(cmd) with ( - patch.object(Config, "get", side_effect=lambda key, default=None: config_map.get(key, default)), - patch("uvicorn.run") as mock_uvicorn_run, + patch.object(Config, "get", side_effect=lambda key, default=None: cfg.get(key, default)), + patch("uvicorn.run") as mock_uvicorn, patch.object(ServeCommand, "is_app_exist", return_value=True), - patch.object(ServeCommand, "line"), - patch.object(ServeCommand, "option", side_effect=_make_options(reload_option_value)), ): - cmd.handle() - return mock_uvicorn_run.call_args[1] + tester.execute(args) + return mock_uvicorn.call_args[1] # --------------------------------------------------------------------------- @@ -59,27 +51,27 @@ def _run_handle(reload_option_value, cfg_reload=True): class TestServeCommandReloadCoercion: - def test_reload_none_falls_back_to_cfg_reload_true(self): - """When --reload is not passed, reload inherits the fastapi config value.""" - kwargs = _run_handle(reload_option_value=None, cfg_reload=True) - assert kwargs["reload"] is True - - def test_reload_none_falls_back_to_cfg_reload_false(self): - """When --reload is not passed and cfg is False, reload is False.""" - kwargs = _run_handle(reload_option_value=None, cfg_reload=False) + def test_reload_string_false_becomes_bool_false(self): + """--reload False (string) must be coerced to bool False.""" + kwargs = _run("--reload False") assert kwargs["reload"] is False - def test_reload_string_False_becomes_bool_false(self): - """--reload=False (string) must be coerced to bool False.""" - kwargs = _run_handle(reload_option_value="False") - assert kwargs["reload"] is False + def test_reload_string_true_becomes_bool_true(self): + """--reload True (string) must be coerced to bool True.""" + kwargs = _run("--reload True") + assert kwargs["reload"] is True - def test_reload_string_True_becomes_bool_true(self): - """--reload=True (string) must be coerced to bool True.""" - kwargs = _run_handle(reload_option_value="True") + def test_no_reload_flag_uses_cfg_default_true(self): + """No --reload flag: reload should inherit the fastapi config value (True).""" + kwargs = _run("", cfg_reload=True) assert kwargs["reload"] is True + def test_no_reload_flag_uses_cfg_default_false(self): + """No --reload flag with cfg=False: reload should be False.""" + kwargs = _run("", cfg_reload=False) + assert kwargs["reload"] is False + def test_reload_string_false_lowercase_becomes_bool_false(self): - """--reload=false (lowercase) must be coerced to bool False.""" - kwargs = _run_handle(reload_option_value="false") + """--reload false (lowercase) must also be coerced to bool False.""" + kwargs = _run("--reload false") assert kwargs["reload"] is False