test : Cover the socket handlers and fix the five crashes that surfaced (PyTest - 5) by Manik-Khajuria-5 · Pull Request #1673 · fossasia/visdom

Manik-Khajuria-5 · 2026-08-01T13:00:55Z

Description

socket_handlers.py had no test coverage. This adds 122 integration tests (test_socket_lifecycle.py, test_socket_commands.py, plus a testutils/sockets.py helper that drives the real handlers without a WebSocket) and fixes what writing them turned up:

broadcast_layouts was a raise ValueError stub on the base class, so a source socket sending save_layouts crashed.
Pane close popped the pane twice, so sources always saw pane_data: None.
A non-integer plot_history frame raised TypeError — the range check ran before any type check.
pop_embeddings_pane raised on an exhausted history; now logs and drops.
heatmap(update="remove") stopped removing the plot: it posts {"data": [], "delete": True} with no name, and not [] is True, so the opts-only shortcut returned before the delete branch.

Fixes : #1695

Integration note

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

How Has This Been Tested?

pytest py/tests — 716 passed under Python 3.12.13 (594 pred branch, unchanged).
Fix 5 end to end against a running server with the real client: t 1 → 0 with the fix, 1 → 1 without.
Each socket fix has a test that fails without it. black --check

Screenshots (if appropriate):

n/a the two plot_surface_remove* screenshots go back to matchin

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing funected)
Code refactor or cleanup (changes to existing code for improved readability or performance)

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 socket and window lifecycle test coverage, harden plotting and embeddings updates, and improve persistence durability and configuration for tests and packaging.

Bug Fixes:

Prevent source sockets from crashing when saving layouts and ensure save_layouts broadcasts to all subscribers.
Fix pane close handling so deleted panes are only popped once, preserving pane data for close events and undo.
Make plot history frame selection robust by enforcing integer, in-range indices to avoid TypeError in socket handlers.
Guard embeddings pane history popping against empty or missing old_content to avoid IndexError/KeyError and keep the socket usable.
Ensure heatmap removal and partial trace updates correctly handle empty data lists and unnamed deletes without skipping the delete branch.
Assign unique window indices based on the maximum existing index to avoid index reuse after deletes.
Treat categorical axis values correctly in missing-point detection so updates on string x values no longer raise TypeError.

Enhancements:

Introduce atomic environment and undo file writes via temporary files and renames to prevent data loss on interrupted saves.
Extend UpdateHandler with plot history caps and generalized trace update logic to bound in-memory growth for text, images, embeddings, and plot histories.
Add LazyEnvData mapping behavior tests and deep-copy semantics to ensure environment forking and lazy loading behave predictably.
Refine storage wiring tests using shared testutils fakes and payload builders to assert that all persistence routes go through JSONStore.
Strengthen socket command handling with explicit HTTP errors for missing envs/windows and clearer failure responses.
Centralize shared test helpers (HTTP base class, socket doubles, payload builders, fakes) and register pytest markers for unit/integration/slow/server scopes.
Exclude tests from the distributed package via find_packages(exclude=...) and adjust pytest pythonpath to import testutils without shipping a tests package.

Build:

Update pyproject pytest configuration to include py/tests on PYTHONPATH, register unit/integration/slow/server markers, and clarify server marker semantics.
Adjust setup.py packaging to exclude tests from the visdom distribution while keeping py/ as the package root.

Documentation:

Document the Python testing layout, fixtures, HTTP test patterns, and pytest markers in .agents/context/testing.md.
Add example/manual README and a visual_check.py script describing manual visual UI checks outside the pytest suite.

Tests:

Add extensive integration tests for socket commands (close, undo, delete_env, save/save_all, layout edits, comments, embeddings drilldown, forward_to_vis, echo, unknown commands).
Add HTTP integration tests covering plot updates (heatmaps, trace updates, marker updates, layout/opts-only updates, categorical axes, empty data, unsupported updates).
Add unit tests for memory growth caps across text, embeddings undo history, image history, and plot history, plus handler wiring of caps.
Add unit tests for shared_utils helpers (NaN sanitization, NanSafeEncoder, warn_once, missing value detection, path/id helpers, slider index coercion).
Add unit tests for window building and update logic to verify pane construction, opts/layout merging, versioning, and legend renaming.
Add integration tests for socket lifecycle (open, register, close, readonly behavior, failure reasons) and window lifecycle CRUD over HTTP.
Add integration tests for environment lifecycle (implicit creation, fork, save/delete, reload) and static asset build/serving verification.
Add unit tests for t-SNE backend selection and normalization, plus edge-case HTTP tests for env id escaping and malformed requests.
Restructure existing tests into unit/integration subpackages and update imports to use testutils fakes and helpers.

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.

Both halves now run off VisdomHTTPTestCase instead of their own fixture block.

The three cap tables collapse onto parametrize; the file was untracked until now.

Guard the missing-point check behind a numeric test, and stop indexing data and traces past their length when an update supplies fewer entries than the plot.

It was the one history pane that grew without bound.

Removing a heatmap names no trace and posts no data, so the shortcut for opts-only updates returned before the delete branch and left the plot up.

sourcery-ai

Sorry @Manik-Khajuria-5, your pull request is larger than the review limit of 150000 diff characters

sourcery-ai · 2026-08-01T13:01:31Z

Reviewer's Guide

Adds comprehensive unit and integration tests around socket handlers, window/environment lifecycle, server utilities, and data model durability, and fixes several crashes and edge cases in plot updates, embeddings drill-down, layout broadcasting, environment forking, and atomic persistence, while refactoring test structure and tightening memory caps and defaults.

Sequence diagram for updated plot update and heatmap delete handling

sequenceDiagram
    actor Client
    participant UpdateHandler as UpdateHandler

    Client->>UpdateHandler: update_packet(p, args, max_text_lines, max_old_content, max_image_history, max_plot_history)
    Note over Client,UpdateHandler: args may include name, data, delete, update="remove" (heatmap)

    UpdateHandler->>UpdateHandler: update(p, args, max_text_lines, max_old_content, max_image_history, max_plot_history)
    alt utype == plot_history
        UpdateHandler->>UpdateHandler: append frame to p["content"]
        UpdateHandler->>UpdateHandler: trim to max_plot_history
        UpdateHandler->>UpdateHandler: set p["selected"]
    else utype == plot_update
        UpdateHandler->>UpdateHandler: update_window(p, args)
        UpdateHandler->>UpdateHandler: read delete, name, data
        alt delete is falsy and name is None and not data
            UpdateHandler-->>Client: return p (opts/layout-only update)
        else delete is truthy
            UpdateHandler->>UpdateHandler: remove selected traces from p["content"]["data"]
            UpdateHandler-->>Client: return p (heatmap trace removed)
        end
    end
    UpdateHandler-->>Client: p with contentID set

File-Level Changes

Change	Details	Files
Add extensive socket, window, environment, and HTTP integration/unit tests plus shared test utilities, reorganizing tests into unit/integration suites and documenting testing strategy.	Introduce integration tests that drive real Application via HTTP and socket handlers to cover window/env lifecycle, plot updates, text/media updates, socket commands, socket lifecycle, edge cases, and build verification. Add unit tests for data_model JSONStore durability and name collisions, server_utils helpers, window builder, shared_utils (NaN handling, warning dedup), image slider, t-SNE backend selection/normalization, and in-memory growth caps. Create testutils package (HTTP base class, fakes, socket helpers, payload builders) and conftest fixtures for stores, app, handlers, sockets, offline client, and warn_once reset; mark tests with unit/integration markers and update testing context docs. Move existing tests into unit/integration subdirectories and adjust imports/fixtures accordingly.	`py/tests/unit/test_data_model.py` `py/tests/integration/test_storage_wiring.py` `py/tests/unit/test_image_slider.py` `py/tests/unit/test_memory_caps.py` `py/tests/unit/test_shared_utils.py` `py/tests/unit/test_server_utils.py` `py/tests/unit/test_tsne.py` `py/tests/unit/test_window_builder.py` `py/tests/integration/test_socket_commands.py` `py/tests/integration/test_socket_lifecycle.py` `py/tests/integration/test_update_plots.py` `py/tests/integration/test_update_text_media.py` `py/tests/integration/test_window_types.py` `py/tests/integration/test_window_lifecycle.py` `py/tests/integration/test_environment_lifecycle.py` `py/tests/integration/test_edge_cases.py` `py/tests/integration/test_build_verification.py` `py/tests/testutils/__init__.py` `py/tests/testutils/http.py` `py/tests/testutils/fakes.py` `py/tests/testutils/sockets.py` `py/tests/testutils/payloads.py` `py/tests/conftest.py` `.agents/context/testing.md` `example/manual/visual_check.py` `example/manual/README.md`
Fix socket handler crashes and logic bugs for pane close, embeddings drill-down, plot history frame validation, heatmap removal, environment forking, and window existence errors, and centralize layout broadcasting.	Implement broadcast_layouts on the base socket handler so save_layouts sent by any socket type notifies subscribers and persists layouts to storage. Fix close command to pop pane data only once under the escaped eid, pushing deleted pane into undo stack and broadcasting correct pane_data to sources; ensure undo count is broadcast and undo restores panes with unique indices at the end. Harden plot_history frame handling (type check for int and non-bool, range validation) to avoid TypeError on bad frames and log/drop invalid updates. Guard pop_embeddings_pane against empty/absent old_content history, logging and preserving pane instead of raising IndexError/KeyError, and update content/has_previous/contentID correctly when history exists. Adjust update handler for plot traces: add max_plot_history cap and selection index maintenance; treat delete flag before opts-only shortcut; inject new traces when all have been deleted; iterate over idxs and new_data safely; refactor missing-value detection via _is_missing_value to avoid TypeError on categorical axes; validate named trace updates carry exactly one data entry and raise 400 otherwise. Change save command to raise HTTP 400 on missing prev_eid instead of assertion, and window retrieval for win_data to raise HTTP 400 with a generic reason when window is absent. Ensure readonly sockets short-circuit all commands before dispatch, as covered by tests.	`py/visdom/server/handlers/socket_handlers.py` `py/visdom/server/handlers/web_handlers.py` `py/visdom/utils/shared_utils.py`
Make JSONStore environment and undo persistence atomic and robust to interrupted writes and name collisions, including hash-based fallbacks for long IDs.	Introduce _atomic_write helper to write via .tmp and os.replace, mirroring save_undo semantics, and use it for serialize_env primary and hash fallback paths. Refactor save_undo to use atomic_write for both plain and hashed undo paths, ensuring interrupts leave previous stack intact and stranded .tmp files are not treated as envs. Add tests to verify interrupted saves keep previous env contents, env listing and existence remain correct, staging files are cleaned up on success and not mis-listed when stranded, rename failures leave previous env intact, long-name fallback files hash correctly and report real IDs, and undo saves are durable. Document HASHED_ENV_RE behavior where env file whose filename matches hash<64 hex> but lacks name field is dropped from list_envs while still loadable by id.	`py/visdom/data_model/json_store.py` `py/tests/unit/test_data_model.py`
Enforce and test in-memory caps for text, embeddings undo history, image history, and plot history to prevent unbounded growth, wiring caps through Application and handlers.	Add DEFAULT_MAX_PLOT_HISTORY in defaults and propagate to Application.max_plot_history and BaseHandler app attributes. Extend UpdateHandler.update and update_packet signature to accept max_plot_history, apply truncation for plot_history content list to keep newest frames within cap and ensure selected index stays valid. Add unit tests to verify caps for text lines, embeddings old_content, image_history frames, and plot_history frames, including truncation behavior and explicit selection semantics, and ensure handler copies caps from Application.	`py/visdom/server/defaults.py` `py/visdom/server/app.py` `py/visdom/server/handlers/base_handlers.py` `py/visdom/server/handlers/web_handlers.py` `py/tests/unit/test_image_slider.py` `py/tests/unit/test_memory_caps.py`
Adjust window registration index assignment to avoid index reuse after deletions, keeping indices unique and monotonic within an environment.	Modify register_window to compute new pane index as max(existing i) + 1 rather than len(env), preventing reuse when intermediate windows have been closed. Add tests that exercise window creation, recreation with same win id, closing and creating in churn scenarios to ensure indices remain unique and increasing.	`py/visdom/utils/server_utils.py` `py/tests/integration/test_window_lifecycle.py`
Tighten pytest configuration, package discovery, and testing documentation, making testutils importable and excluding tests from installed packages.	Update pyproject.toml pytest settings to include py/tests on pythonpath so testutils is importable, register markers unit/integration/slow/server with updated server description, and keep testpaths scoped to py/tests. Document test layout, marker usage, fixture availability, and HTTP vs unit test styles in testing context file, clarifying hermetic requirements and manual vs automated checks. Change setup.py find_packages to exclude tests and tests.* when packaging from py/, preventing tests module shipping to users.	`pyproject.toml` `.agents/context/testing.md` `setup.py`
Add manual visual-check scripts for UI verification without affecting automated test collection.	Introduce example/manual/visual_check.py that connects to a running visdom server, creates one window per visualization type, and prints a human checklist for browser inspection. Add README explaining manual checks location, purpose, and their separation from pytest/Playwright/Cypress suites.	`example/manual/visual_check.py` `example/manual/README.md`

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

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

Manik-Khajuria-5 added 29 commits July 31, 2026 17:01

Add shared test fixtures and split py/tests into unit and integration

57e5177

Adds conftest.py, a testutils package, and unit tests for window().

Test window create, read, close and ordering over HTTP

5c5cc9f

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.

Test every pane type created through /events

ddb31b2

Collapses the repeated plot-trace and opts assertions into parametrized tables, so each case reports separately instead of hiding behind the first failure.

Test env creation, fork, save, delete and reload over HTTP

4c74855

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.

Test env id escaping, error routes and awkward content

b726f59

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'.

Stop a closed window's index being handed to the next one

a7b61f2

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.

Merge branch 'dev' into TestingP1-1

4f12152

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.

Merge branch 'TestingP1-1' into TestingP2

0cadbbe

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.

Name the two ids in the window-replacement test

662202a

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.

Split the update-path tests into text/media and plot files

9e165ac

Both halves now run off VisdomHTTPTestCase instead of their own fixture block.

Land the memory cap tests as plain pytest functions

a7b0f92

The three cap tables collapse onto parametrize; the file was untracked until now.

Answer 200 instead of 500 on categorical x-values and short updates

fcf82c1

Guard the missing-point check behind a numeric test, and stop indexing data and traces past their length when an update supplies fewer entries than the plot.

Cap plot_history growth the way image history is capped

ed8c796

It was the one history pane that grew without bound.

Land the build verification tests under integration

bf2cec3

Rescue the t-SNE unit tests into the tracked suite

e363d1f

Cover the shared utility helpers

59e706b

Write environments atomically so an interrupted save keeps the old one

cad5892

Pin down LazyEnvData's mapping and deep-copy behaviour

0a36686

Move the manual visual check script out of the test suite

cf71eed

Cover socket open, registration and close

fbceede

Let a source socket save layouts instead of raising

0a3fc06

Send the closed pane data to sources instead of None

07c1958

Guard the embeddings pop and the plot history frame index

82ef6cf

Cover every socket command dispatched by on_message

c616548

Let an unnamed delete through the empty-data shortcut

c74bf4f

Removing a heatmap names no trace and posts no data, so the shortcut for opts-only updates returned before the delete branch and left the plot up.

sourcery-ai Bot reviewed Aug 1, 2026

View reviewed changes

Manik-Khajuria-5 requested review from Jayantparashar10, Saksham-Sirohi, marcoag, mariobehling, norbusan, rajnisht7, tonypzy and vedansh-5 August 1, 2026 13:04

Manik-Khajuria-5 added 2 commits August 2, 2026 15:12

Merge branch 'dev' into Testing-5

92d8b9a

test: drop redundant test_ prefix from files under py/tests

0c503f5

Manik-Khajuria-5 mentioned this pull request Aug 3, 2026

Fix & test : polling connect, constant-time login, readonly HTTP writes, and error-page disclosure (PyTest - 6) #1679

Open

8 tasks

Navigation Menu

Uh oh!

test : Cover the socket handlers and fix the five crashes that surfaced (PyTest - 5) - #1673

test : Cover the socket handlers and fix the five crashes that surfaced (PyTest - 5)#1673
Manik-Khajuria-5 wants to merge 31 commits into
fossasia:devfrom
Manik-Khajuria-5:Testing-5

Manik-Khajuria-5 commented Aug 1, 2026 •

edited

Loading

Uh oh!

sourcery-ai Bot left a comment

Uh oh!

sourcery-ai Bot commented Aug 1, 2026

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Manik-Khajuria-5 commented Aug 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Fixes : #1695

Integration note

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Summary by Sourcery

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai Bot commented Aug 1, 2026

Reviewer's Guide

Sequence diagram for updated plot update and heatmap delete handling

File-Level Changes

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Manik-Khajuria-5 commented Aug 1, 2026 •

edited

Loading