Skip to content

test : HTTP window and environment lifecycle, and fix two request-handling bugs (PyTest - 2) - #1669

Open
Manik-Khajuria-5 wants to merge 16 commits into
fossasia:devfrom
Manik-Khajuria-5:TestingP2
Open

test : HTTP window and environment lifecycle, and fix two request-handling bugs (PyTest - 2)#1669
Manik-Khajuria-5 wants to merge 16 commits into
fossasia:devfrom
Manik-Khajuria-5:TestingP2

Conversation

@Manik-Khajuria-5

@Manik-Khajuria-5 Manik-Khajuria-5 commented Jul 31, 2026

Copy link
Copy Markdown
Member

Description

Adds 70 HTTP integration tests across four files in py/tests/integration/, covering the window and environment lifecycle end to end, and fixes two bugs they exposed.

  • test_window_lifecycle.py — create, exists, read, replace, close, ordering
  • test_window_types.py — every pane type through POST /events
  • test_environment_lifecycle.py — implicit creation, fork, save, delete, reload
  • test_edge_cases.py — eid escaping, error routes, malformed requests, awkward content

Each subclasses VisdomHTTPTestCase from PR-1 and carries pytestmark = pytest.mark.integration. Three helpers (update, close_window,save) were added to the base class.

Fixes : #1695

Integration note

This PR should be merged after #1668 , as it depends on the lower-layer changes introduced there.

Motivation and Context

Two defects had no test and are fixed here, each with a regression test:

Window indices could collide. register_window took the index from len(env), so closing any window but the last handed the next one an index alr pane order became undefined after a reload. Now max(i) + 1, which is what the undo path at socket_handlers.py:120 already did.

Three validation checks used a bare assert. An invalid reques
500 instead of a 400, and under python -O the checks vanished entirely forking an unknown env then raised KeyError, and a named trace update read p given. They now raise HTTPError(400) like the other checks in the file.

One existing assertion, assertIn(resp.code, (200, 500)), could not fail; it is replaced with the actual contract.

How Has This Been Tested?

  • pytest -m "not server" 371 passed, on a clean checkout, P
  • black --check ./py (23.1.0) clean.
  • Green under python -O too, which is the point of the second fix
  • All five regression tests fail when their fix is reverted, so they are load-bearing.
  • python example/demo.py against a live server: 70 panes, contigu no server errors and no visible difference from a clean branch.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I adapted the version number under py/visdom/VERSION according to Semantic Versioning
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Summary by Sourcery

Add comprehensive HTTP integration and unit tests for window and environment lifecycle, strengthen request validation, and ensure window indexing and storage wiring behave correctly.

New Features:

  • Introduce VisdomHTTPTestCase-based integration tests covering window CRUD, pane types, environment lifecycle, edge cases, and error handling.
  • Add shared test utilities (payload builders, fake handlers/sockets/stores, HTTP base class) and pytest fixtures to support hermetic testing.
  • Define unit test suites for server utilities and pane construction helpers to verify pure logic without a running server.

Bug Fixes:

  • Prevent window index collisions by assigning new pane indices based on the current maximum instead of environment size.
  • Replace bare asserts in HTTP request handlers with HTTP 400 errors for invalid named trace updates, missing fork sources, and unknown windows, including under optimized Python runs.

Enhancements:

  • Refine storage wiring tests to use shared fakes and payload builders, and assert JSON response bodies.
  • Document the Python testing strategy, directory layout, markers, and HTTP test patterns in the testing context file.
  • Extend pytest configuration with test markers and pythonpath entries for testutils and tighten package discovery to exclude tests from distribution.
  • Add utility tests to ensure environment ID escaping, hashing, stringify ordering, and related helpers behave consistently.

Build:

  • Exclude test packages from the distributed Python package via setup.py find_packages configuration.

Documentation:

  • Update testing documentation to describe unit vs integration layout, shared fixtures, HTTP test base class usage, and registered pytest markers.

Tests:

  • Add new integration suites for window lifecycle, environment lifecycle, pane types, and edge-case HTTP behavior.
  • Add unit tests for window builder logic and server utility functions, increasing coverage of pure helpers.
  • Introduce reusable test utilities and fixtures to standardize payloads, fakes, and HTTP interactions across the suite.

Chores:

  • Reorganize existing storage wiring tests under an integration subdirectory and align them with new test infrastructure.

Adds conftest.py, a testutils package, and unit tests for window().
AsyncHTTPTestCase is a unittest.TestCase, and pytest will not inject
fixtures into those. Run the Application on a background event loop
instead and talk to it with requests, which is already a runtime
dependency, so HTTP tests can be plain functions.
Lands the window lifecycle file as plain pytest functions on the
visdom_server fixture, dropping the per-file setUp block and the
temp directory it never cleaned up.
Collapses the repeated plot-trace and opts assertions into parametrized
tables, so each case reports separately instead of hiding behind the
first failure.
The reload case now builds its second Application from the app_factory
fixture, which shares env_path with visdom_server, instead of hand-rolling
one against a leaked temp directory.
The update-into-a-missing-env case asserted 'status is 200 or 500',
which could not fail. It now pins the actual behaviour: 200 with
'win does not exist'.
register_window took the index from len(env), so closing any window but
the last produced a duplicate index and the pane order went undefined
after a reload. Use max(i) + 1, which is what the undo path already does.
Three validation checks used a bare assert, so an invalid request came
back as a 500 and, under python -O, was skipped entirely: forking an
unknown env then raised KeyError and a named trace update read past the
data it was given. Raise HTTPError(400) like the other checks do.
Places the two test files dev added into the new layout, and points the
new storage-wiring test at env_payload: it called the local _env helper
this branch had already replaced, which merged cleanly and then failed.
Brings in dev along with the layout placement and the env_payload fix
made on the PR-1 branch. D1 and D2 both survive the auto-merge of
server_utils.py and web_handlers.py.
Replaces the background event loop and requests session with the
AsyncHTTPTestCase base class from PR-1, which already runs the app
in-process on an ephemeral port. The four integration files become
TestCase subclasses; pytest still collects and marks them.
first/second read as two windows; they are the id returned by each call,
which is the same id both times. Also assert only one pane exists.
The guidance still said VisdomHTTPTestCase was scheduled for replacement
by a background-loop fixture. Record the opposite, and the consequences
for anyone writing an HTTP test: no fixtures, no parametrize, share via
a base class.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @Manik-Khajuria-5, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Adds a structured HTTP integration test harness and unit tests for window/env utilities, replaces ad-hoc fakes with reusable test helpers, tightens test configuration/docs, and fixes two HTTP request-handling bugs (window index allocation and bare-assert validation) in server code.

Sequence diagram for HTTP env fork validation behavior

sequenceDiagram
    actor Client
    participant Application
    participant web_handlers
    participant handler

    Client->>Application: POST /events (fork_env)
    Application->>web_handlers: wrap_func(handler, args)
    web_handlers->>web_handlers: prev_eid = escape_eid(args.get("prev_eid"))
    web_handlers->>web_handlers: eid = escape_eid(args.get("eid"))
    alt prev_eid not in handler.state
        web_handlers-->>Client: HTTPError(400, reason=env to be forked doesn't exist)
    else prev_eid in handler.state
        web_handlers->>handler: handler.state[eid] = copy.deepcopy(handler.state[prev_eid])
        web_handlers->>handler: handler.storage.save_env(eid, handler.state[eid])
        handler-->>Client: 200 OK
    end
Loading

File-Level Changes

Change Details Files
Refactor storage wiring tests to use shared test doubles and payload builders.
  • Move storage wiring tests under py/tests/integration and update module docstring to rely on separate environment lifecycle coverage.
  • Replace ad-hoc SimpleNamespace handler and inline _SpyStore/_env helpers with shared FakeHandler, SpyStore and env_payload utilities.
  • Adjust expectations to assert through FakeHandler.json_body and SpyStore call tracking.
py/tests/test_storage_wiring.py
py/tests/testutils/fakes.py
py/tests/testutils/payloads.py
Clarify and extend testing documentation and pytest configuration to support shared helpers and markers.
  • Expand .agents/context/testing.md with structure for unit vs integration tests, shared fixtures, HTTP testing patterns, and marker usage.
  • Update pyproject.toml pytest configuration to add py/tests to pythonpath, register unit/integration/slow/server markers, and clarify marker semantics in comments.
.agents/context/testing.md
pyproject.toml
Fix HTTP request-handling bugs in server handlers and window registration logic.
  • Change named trace updates in web_handlers.update to raise HTTPError(400) when multiple data entries are provided without delete, instead of using a bare assert.
  • Update fork_env and win_data handlers to validate env/window existence via HTTPError(400) with stable, ASCII-safe reasons, replacing bare asserts that could leak non-ASCII or vanish under python -O.
  • Update register_window to allocate pane indices as max(existing_i)+1 rather than len(env) to avoid index reuse after closes, matching undo-path semantics.
py/visdom/server/handlers/web_handlers.py
py/visdom/utils/server_utils.py
Adjust packaging to exclude tests from installed distribution.
  • Update setup.py to call find_packages with exclude=["tests", "tests.*"], preventing test packages from being shipped to users.
setup.py
Add unit tests for server_utils helpers and pane construction/update logic.
  • Introduce unit test coverage for escape_eid, extract_eid, hash_password, stringify, and recursive_order, including edge cases and full login flow.
  • Add focused unit tests for window() and update_window() covering id generation, versioning, content/opts mapping, pane types, history panes, embeddings/network behavior, and legend/name update semantics.
py/tests/unit/test_server_utils.py
py/tests/unit/test_window_builder.py
py/tests/testutils/payloads.py
Introduce reusable HTTP test base class and integration tests for window and environment lifecycle, pane types, and edge cases.
  • Add VisdomHTTPTestCase that spins up an in-process Application with disposable env_path and HTTP helpers (post_json, create_window, update, close_window, win_exists, get_win_data, get_envs, save, panes).
  • Add integration tests for window CRUD and ordering, environment creation/fork/save/delete/reload, all pane types produced via POST /events, and edge/error cases including eid escaping, error routes, malformed requests, high-volume window creation, and awkward content.
  • Mark these tests with pytestmark = pytest.mark.integration and use shared testutils payload builders and HTTP helpers.
py/tests/testutils/http.py
py/tests/integration/test_window_lifecycle.py
py/tests/integration/test_environment_lifecycle.py
py/tests/integration/test_window_types.py
py/tests/integration/test_edge_cases.py
py/tests/testutils/__init__.py
Provide shared pytest fixtures and test doubles for hermetic tests.
  • Add conftest.py defining fixtures for env_path, JSONStore/SpyStore, Application instances, fake handler/socket, offline client, send-capture helper, and shared warn_once reset.
  • Implement FakeSocket, FakeHandler, and SpyStore doubles to drive web and socket handlers directly and capture JSON bodies and backend calls.
py/tests/conftest.py
py/tests/testutils/fakes.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@Manik-Khajuria-5 Manik-Khajuria-5 changed the title Test : HTTP window and environment lifecycle, and fix two request-handling bugs (PyTest - 2) test : HTTP window and environment lifecycle, and fix two request-handling bugs (PyTest - 2) Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a Python test suite (pytest) for the server and client

1 participant