From 49b90bf2aa56e6a81cafcc2ce4b7ea02d9055305 Mon Sep 17 00:00:00 2001 From: Thomas Schmelzer Date: Thu, 25 Jun 2026 20:52:39 +0400 Subject: [PATCH] Fix stop_server coverage gap, add return types, unify console output Tracked in the fork at tschm/pycharting#7, tschm/pycharting#8, tschm/pycharting#9. #7 Test-isolation/coverage gap (interface.py:285): the cleanup_globals fixture used `global _active_server`, which rebinds the test module's imported copy and never touches `interface._active_server`. The running server leaked across tests, leaving the no-server branch of stop_server() uncovered and `test_status_when_no_server` intermittently failing. The fixture now resets the live module global (stop + None); the no-server test forces the branch and asserts on the message. Coverage is back to 100% and deterministic across runs. #8 Add explicit return annotations: stop_server() -> None and _repr_html_() -> str, matching the rest of the public API surface. #9 Route all user-facing console output through a single _notify() helper instead of scattered bare print() calls. No bare print() remains in src/pycharting/; stdout behavior is preserved. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/pycharting/api/interface.py | 42 ++++++++++++++++++--------- tests/test_interface.py | 50 ++++++++++++++++++++------------- 2 files changed, 59 insertions(+), 33 deletions(-) diff --git a/src/pycharting/api/interface.py b/src/pycharting/api/interface.py index 35177a1..efa4283 100644 --- a/src/pycharting/api/interface.py +++ b/src/pycharting/api/interface.py @@ -32,6 +32,20 @@ _active_server: ChartServer | None = None +def _notify(message: str) -> None: + """Emit a user-facing console message. + + All direct, user-facing output (chart URLs, status banners) is funneled + through this single helper rather than scattered ``print`` calls, so the + output channel is consistent and can be changed in one place. Diagnostic + detail continues to flow through ``logger``. + + Args: + message (str): The message to display to the user. + """ + print(message) + + def plot( index: np.ndarray | pd.Series | list, open: np.ndarray | pd.Series | list | None = None, @@ -211,7 +225,7 @@ def plot( webbrowser.open(chart_url) except Exception as e: logger.warning(f"Could not open browser: {e}") - print(f"Please open this URL manually: {chart_url}") + _notify(f"Please open this URL manually: {chart_url}") result = { "status": "success", @@ -223,31 +237,31 @@ def plot( } # Print user-friendly message - print("\n✓ Chart created successfully!") - print(f" URL: {chart_url}") - print(f" Data points: {data_manager.length:,}") + _notify("\n✓ Chart created successfully!") + _notify(f" URL: {chart_url}") + _notify(f" Data points: {data_manager.length:,}") if not open_browser: - print(" Open the URL above in your browser to view the chart.") - print() + _notify(" Open the URL above in your browser to view the chart.") + _notify("") # Block until server shutdown if requested if block and _active_server: # pragma: no cover logger.info("Blocking until chart is closed (press Ctrl+C to force exit)...") - print("Keeping server alive until you close the browser page...") + _notify("Keeping server alive until you close the browser page...") try: # Wait with timeout so Ctrl+C can interrupt while not _active_server._shutdown_event.is_set(): _active_server._shutdown_event.wait(timeout=0.5) logger.info("Server shutdown detected, returning control") - print("\n✓ Chart closed, server stopped") + _notify("\n✓ Chart closed, server stopped") except KeyboardInterrupt: logger.info("Interrupted by user") - print("\n⚠️ Interrupted - stopping server...") + _notify("\n⚠️ Interrupted - stopping server...") _active_server.stop_server() except Exception as e: logger.error(f"Error creating chart: {e}", exc_info=True) - print(f"\n✗ Error creating chart: {e}\n") + _notify(f"\n✗ Error creating chart: {e}\n") return { "status": "error", @@ -258,7 +272,7 @@ def plot( return result -def stop_server(): +def stop_server() -> None: """Manually shut down the active chart server. While the server has an auto-shutdown feature (triggered after a timeout when no clients are connected), @@ -280,9 +294,9 @@ def stop_server(): if _active_server and _active_server.is_running: logger.info("Stopping ChartServer...") _active_server.stop_server() - print("✓ Chart server stopped") + _notify("✓ Chart server stopped") else: - print("ⓘ No active server to stop") + _notify("ⓘ No active server to stop") def get_server_status() -> dict[str, Any]: @@ -322,7 +336,7 @@ def get_server_status() -> dict[str, Any]: # Jupyter notebook support -def _repr_html_(): +def _repr_html_() -> str: """Jupyter notebook representation.""" status = get_server_status() if status["running"]: diff --git a/tests/test_interface.py b/tests/test_interface.py index 802d6ed..a60f680 100644 --- a/tests/test_interface.py +++ b/tests/test_interface.py @@ -5,28 +5,30 @@ import numpy as np import pytest -from pycharting.api.interface import _active_server, _repr_html_, get_server_status, plot, stop_server +from pycharting.api.interface import _repr_html_, get_server_status, plot, stop_server from pycharting.api.routes import _data_managers @pytest.fixture(autouse=True) def cleanup_globals(): - """Clean up global state between tests.""" - global _active_server - - # Stop any active server before test - if _active_server and _active_server.is_running: - _active_server.stop_server() - - # Clear data managers - _data_managers.clear() + """Clean up global state between tests. + + Operates on the live ``interface`` module global rather than a name + imported into this module — a plain ``global _active_server`` here would + rebind the test module's own copy and leave the running server (and the + real ``interface._active_server``) untouched, leaking state between tests. + """ + import pycharting.api.interface as iface + + def _reset() -> None: + if iface._active_server and iface._active_server.is_running: + iface._active_server.stop_server() + iface._active_server = None + _data_managers.clear() + _reset() yield - - # Clean up after test - if _active_server and _active_server.is_running: - _active_server.stop_server() - _data_managers.clear() + _reset() class TestPlotFunction: @@ -165,10 +167,20 @@ def test_stop_server_when_running(self): status = get_server_status() assert status["running"] is False - def test_stop_server_when_not_running(self): - """Test stopping when no server is active.""" - # Should not raise an error - stop_server() # Should print info message + def test_stop_server_when_not_running(self, capsys): + """Test stopping when no server is active reports the no-op message.""" + import pycharting.api.interface as iface + + # Force the module global to None so the "no active server" branch is + # exercised deterministically, regardless of test execution order / + # xdist worker state. + original = iface._active_server + iface._active_server = None + try: + stop_server() # Should not raise; should emit the info message + assert "No active server to stop" in capsys.readouterr().out + finally: + iface._active_server = original class TestGetServerStatus: