test : Add shared test fixtures and split py/tests into unit and integration (PyTest - 1)

.agents/context/testing.md

@@ -11,8 +11,10 @@ pytest -m "not server"                 # skip tests that need a live server (CI
 ```
 
 Config lives in `pyproject.toml` (`[tool.pytest.ini_options]`): discovery is scoped to
-`py/tests/` and `pythonpath = ["py"]` makes `import visdom` work without an editable install.
-Experimental `test_*.py` scripts in the repo root (and `test/`) are intentionally out of scope.
+`py/tests/`, and `pythonpath = ["py", "py/tests"]` makes both `import visdom` and
+`import testutils` work without an editable install. Because discovery is scoped by `testpaths`,
+experimental `test_*.py` scripts in the repo root (and `test/`) stay out of scope; `testutils/` is
+excluded via `norecursedirs` so helpers are importable but never collected.
 
 ## Run E2E / Visual Tests (Cypress)
 
@@ -38,12 +40,60 @@ Always use port `8098` and `-env_path /tmp` for isolation.
 
 ## Writing Python Tests
 
-- Place in `py/tests/`, name files `test_*.py`, classes `Test*` (unittest `TestCase` or plain
-  pytest functions both work — pytest auto-discovers unittest).
-- Keep them hermetic (no running server). A test that genuinely needs a live server must be
-  marked `@pytest.mark.server` so CI can deselect it.
-- Start with simple hermetic tests (e.g., `test_smoke.py`) and add focused unit tests for
-  server/window/env lifecycle code as coverage grows.
+### Where a test goes
+
+```
+py/tests/
+  conftest.py        shared fixtures, auto-loaded by pytest
+  testutils/         importable helpers (fakes, payload builders, HTTP base class)
+  unit/              pure logic: no Application, no I/O beyond tmp_path
+  integration/       in-process Application, real HTTP, or handler dispatch
+```
+
+`py/tests/` has **no** `__init__.py` on purpose — `setup.py` runs `find_packages(where="py")`,
+so a package there would ship a top-level `tests` distribution to users. `testutils/` is a
+package and is reachable because `py/tests` is on `pythonpath`.
+
+- Name a file after what it covers — `unit/window_builder.py`, not `unit/test_window_builder.py`.
+  The `unit/` and `integration/` directories already say these are tests, so the filename does not
+  repeat it; `python_files = ["*.py"]` in `pyproject.toml` collects them. Test *functions* still
+  need the `test_` prefix. **Write plain `def test_*()` functions, not `unittest.TestCase`
+  subclasses.** pytest still runs `TestCase` (much of the suite predates this rule), but
+  **pytest cannot inject fixtures into `TestCase` methods** — `def test_x(self, app)` fails.
+  Only autouse fixtures reach them. A `TestCase` therefore cannot use anything in the table
+  below, and cannot use `@pytest.mark.parametrize` either.
+- Keep them hermetic. A test that needs an **externally launched** server must be marked
+  `@pytest.mark.server` so CI can deselect it; nothing in the tracked suite needs one today.
+
+### Shared fixtures (`py/tests/conftest.py`)
+
+| Fixture | Gives you |
+|---|---|
+| `env_path` | disposable environment directory |
+| `store` / `spy_store` | `JSONStore` / one that records backend calls |
+| `app` / `app_factory` | `Application` on a temp `env_path`; factory for reload assertions |
+| `handler` / `app_handler` | duck-typed handler, standalone or sharing an `Application`'s state |
+| `fake_socket` | records `write_message`; `.commands()` and `.last(cmd)` for assertions |
+| `offline_client` | `Visdom(send=False)` — never opens a connection |
+| `capture_send` | runs a client call and returns the payload it would have sent |
+
+`reset_warn_once` is autouse: `shared_utils.warn_once` dedupes against a module-level set, so
+without it a warning raised by one test silently suppresses the same warning in another.
+
+HTTP tests are the one exception to the plain-function rule today: subclass
+`testutils.VisdomHTTPTestCase`, which starts the app in-process on an ephemeral port, gives every
+test a fresh `env_path` that is cleaned up, and provides `post_json`, `create_window`,
+`create_text_window`, `get_envs`, `get_win_data`, `win_exists` and `panes`. Override the
+`app_kwargs` class attribute to vary server configuration.
+
+It is a `TestCase` only because `tornado.testing.AsyncHTTPTestCase` is one, so the fixtures above
+are unavailable inside it. It is scheduled to be replaced by an equivalent `visdom_server` fixture
+that runs the app on a background event loop and talks to it with `requests`, which removes the
+last reason for any `TestCase` in this suite.
+
+### Markers
+
+`unit`, `integration`, `slow`, and `server` are registered in `pyproject.toml`.
 
 ## CI
 

CONTRIBUTING.md

@@ -133,7 +133,7 @@ Blindly copy-pasting AI-generated code that you cannot explain or debug is not p
 We actively welcome your pull requests.
 
 1. Fork the repo and create your branch from `dev`.
-2. If you've added code that should be tested, add tests.
+2. If you've added code that should be tested, add tests. For Python, see [Python Tests](#python-tests) below.
 3. If you've changed APIs, update the documentation.
 4. Ensure the Lua and Python interfaces to Visdom are in sync.
 5. If you change `js/`, commit the React-compiled version of `main.js`. For details, please see `Contributing to the UI` below.
@@ -145,6 +145,56 @@ We actively welcome your pull requests.
 8. If you haven't already, complete the Contributor License Agreement ("CLA").
 
 
+## Python Tests
+The Python test suite uses [pytest](https://docs.pytest.org/) and lives under `py/tests/`. It is
+hermetic: no visdom server has to be running, and no test touches the network. Visdom itself
+requires Python 3.12 or newer, so run the tests on 3.12+.
+
+#### Running the tests
+```bash
+pip install -e .                       # install visdom
+pip install -r test-requirements.txt   # pytest and the test dependencies
+pytest                                 # the whole suite
+pytest -m "not server"                 # what CI runs
+```
+Discovery is configured in `pyproject.toml`, so `pytest` finds the suite from anywhere in the
+repository and you do not need to pass a path.
+
+While working on a change, narrow the run down:
+```bash
+pytest py/tests/unit                     # the fast subset: no HTTP
+pytest py/tests/unit/window_builder.py   # a single file
+pytest -k window_builder                 # tests matching a name
+pytest -x --lf                           # stop at the first failure, then re-run just that one
+```
+The `unit`, `integration`, `slow` and `server` markers are registered in `pyproject.toml` for
+selecting with `-m`; a run only picks up the files that carry the marker.
+
+Note that `-q` is already set in `addopts`; passing another `-q` hides the summary line. Use
+`-o addopts=""` if you want pytest's full default output.
+
+#### Adding a test
+```
+py/tests/
+  conftest.py    shared fixtures, loaded automatically
+  testutils/     importable helpers — never collected as tests
+  unit/          pure logic: no Application, no I/O beyond tmp_path
+  integration/   in-process Application, real HTTP, or handler dispatch
+```
+- Put the file in `unit/` or `integration/`, depending on what it needs.
+- Name the file after what it covers — `unit/window_builder.py`, not `unit/test_window_builder.py`.
+  The directory already says these are tests. Test **functions** still need the `test_` prefix, or
+  pytest will not run them.
+- Prefer plain `def test_*()` functions. pytest cannot pass fixtures into `unittest.TestCase`
+  methods, so a `TestCase` cannot use anything in `conftest.py` or `@pytest.mark.parametrize`.
+  Tests that need a real HTTP round trip are the exception: subclass
+  `testutils.VisdomHTTPTestCase`, which runs the app in-process on an ephemeral port.
+- A test that needs an externally launched server must be marked `@pytest.mark.server` so CI can
+  deselect it. Nothing in the suite needs one today.
+- Test files carry the same license header as the rest of the repository.
+
+`.agents/context/testing.md` has the longer version, including a reference for every shared fixture.
+
 ## Contributing to the UI
 The UI is built with [React](https://facebook.github.io/react/). For testing,
 this means that `js/` needs to be compiled. This can be done with `yarn` or

py/tests/conftest.py

@@ -0,0 +1,141 @@
+"""Fixtures shared by the whole suite.
+
+Everything here is hermetic: temporary directories only, no listening sockets,
+no network. Tests that genuinely need an externally launched visdom must carry
+``@pytest.mark.server`` so CI can deselect them.
+"""
+
+from unittest.mock import patch
+
+import pytest
+
+from visdom.data_model.json_store import JSONStore
+from visdom.server.app import Application
+from visdom.utils import shared_utils
+
+from testutils.fakes import FakeHandler, FakeSocket, SpyStore
+
+
+@pytest.fixture
+def env_path(tmp_path):
+    """Disposable environment directory."""
+    return str(tmp_path)
+
+
+@pytest.fixture
+def store(env_path):
+    """JSONStore backed by a temporary directory."""
+    return JSONStore(env_path)
+
+
+@pytest.fixture
+def spy_store(env_path):
+    """JSONStore that records which backend methods the server calls."""
+    return SpyStore(env_path)
+
+
+@pytest.fixture
+def app(env_path):
+    """Application instance with temporary persistence and no listener.
+
+    The port is only recorded on the instance; nothing binds it, so this is
+    safe to construct in parallel with other tests.
+    """
+    return Application(port=8097, env_path=env_path)
+
+
+@pytest.fixture
+def app_factory(env_path):
+    """Build Applications sharing one ``env_path``, for reload assertions."""
+
+    def build(**kwargs):
+        kwargs.setdefault("port", 8097)
+        kwargs.setdefault("env_path", env_path)
+        return Application(**kwargs)
+
+    return build
+
+
+@pytest.fixture
+def fake_socket():
+    return FakeSocket()
+
+
+@pytest.fixture
+def handler(env_path):
+    """Duck-typed handler with its own empty state and store."""
+    return FakeHandler(env_path=env_path)
+
+
+@pytest.fixture
+def app_handler(app):
+    """Handler sharing a real Application's state, storage and subscribers."""
+    return FakeHandler(
+        state=app.state,
+        storage=app.storage,
+        subs=app.subs,
+        sources=app.sources,
+        env_path=app.env_path,
+    )
+
+
+@pytest.fixture
+def offline_client():
+    """Visdom client that never opens a connection.
+
+    With ``send=False`` the client's ``_send`` returns the payload instead of
+    performing I/O, so plot methods can be asserted as pure functions.
+    """
+    import visdom
+
+    return visdom.Visdom(send=False, use_incoming_socket=False)
+
+
+@pytest.fixture
+def capture_send(offline_client):
+    """Run a client call and return the payload it would have transmitted.
+
+    Usage::
+
+        sent = capture_send(lambda v: v.line(Y=[1, 2, 3]))
+        assert sent["payload"]["data"][0]["type"] == "scatter"
+    """
+
+    def run(call, win_exists=None):
+        sent = {}
+
+        def capture(msg, endpoint="events", **_):
+            sent["payload"] = msg
+            sent["endpoint"] = endpoint
+            return "win_capture"
+
+        patches = [patch.object(offline_client, "_send", side_effect=capture)]
+        if win_exists is not None:
+            patches.append(
+                patch.object(offline_client, "win_exists", return_value=win_exists)
+            )
+        for p in patches:
+            p.start()
+        try:
+            call(offline_client)
+        finally:
+            for p in reversed(patches):
+                p.stop()
+        return sent
+
+    return run
+
+
+@pytest.fixture(autouse=True)
+def reset_warn_once():
+    """Isolate ``shared_utils.warn_once`` between tests.
+
+    It dedupes against a module-level set, so without this a warning raised by
+    an earlier test silently suppresses the same warning in a later one, and
+    assertions on it pass or fail depending on execution order.
+    """
+    previous = set(shared_utils._seen_warnings)
+    shared_utils._seen_warnings.clear()
+    yield
+    shared_utils._seen_warnings.clear()
+    shared_utils._seen_warnings.update(previous)