test hygiene: track stray suites, cover shared utils, atomic env writes (PyTest - 4) by Manik-Khajuria-5 · Pull Request #1672 · fossasia/visdom

Manik-Khajuria-5 · 2026-08-01T12:03:08Z

Description

Test-suite hygiene plus one durability fix in JSONStore.

Moved two untracked suites into the collected tree: build verification (integration/test_build_verification.py) and t-SNE (unit/test_tsne.py).
Added unit/test_shared_utils.py; extended unit/test_data_model.py with LazyEnvData's mapping and deep-copy behaviour.
Fix: serialize_env wrote straight to the target file, so an interrupted save truncated the old environment and load_env reported it as empty. Writes now stage into <path>.tmp and os.replace; save_undo shares the same helper.
Moved visual_check.py to example/manual/ it needs a live server and a human eye, so it isn't collectable. Rule documented in .agents/context/testing.md.

Fixes : #1695

Integration note

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

Motivation and Context

Two test files existed but were never collected, so that code was untested in CI. The atomic write closes a real data-loss window on env save.

How Has This Been Tested?

pytest py/tests on Python 3.12: 593 passed (174 in the touched files). No new dependencies; all tests hermetic.

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 functionality to not work as expected)
Code refactor or cleanup (changes to existing code for improv)

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

Strengthen visdom’s persistence and update semantics while formalizing the Python test layout into unit and integration suites with shared test utilities and manual examples.

New Features:

Add a configurable in‑memory cap for plot history panes, wired through application defaults and handler attributes.

Bug Fixes:

Make JSONStore environment and undo saves atomic via staged temporary files and os.replace to avoid truncated or lost environments on interrupted writes.
Fix plot update handling for categorical axes and all‑missing updates using a shared missing‑value helper, preventing TypeError and ensuring gaps are skipped safely.
Ensure window indices are monotonically assigned based on the current maximum instead of len(env) so indices remain unique after deletions.
Replace assertion‑based failures in fork_env, window read, and named trace updates with HTTP 400 errors carrying clear reasons, and handle empty update payloads without index errors.

Enhancements:

Refactor update handlers to support capped plot history, robust trace injection when all traces have been deleted, and safer marker updates.
Introduce shared test utilities (HTTP base case, payload builders, fakes and spy storage) and reorganize tests into unit and integration subpackages with common fixtures.
Clarify pyproject pytest configuration with pythonpath support for testutils and registered markers for unit, integration, slow, and server tests.
Exclude tests packages from the distributed visdom wheel to avoid shipping internal test code to end users.
Document test layout, hermeticity rules, HTTP test patterns, and manual visual checks via updated testing context docs and example/manual assets.

Build:

Adjust setup.py package discovery to exclude tests packages from distribution.
Update pyproject.toml pytest configuration to add py/tests to pythonpath and register unit, integration, slow, and refined server markers.

Documentation:

Expand testing documentation to describe the py/tests layout, fixtures, markers, HTTP test usage, and the role of manual visual checks.
Add example/manual documentation and a visual_check script for human‑driven UI verification outside the automated pytest suite.

Tests:

Add extensive unit and integration coverage for window construction, update semantics across pane types, environment and window lifecycles, edge cases, and shared utilities including tsne helpers and warning behavior.
Introduce durability tests for JSONStore env and undo writes to validate atomic write behavior and long‑name hash fallback interactions.
Add build verification integration tests to assert that static frontend assets are present and correctly served by the Application.

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.

sourcery-ai

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

sourcery-ai · 2026-08-01T12:03:46Z

Reviewer's Guide

Refactors and expands the Python test suite structure (unit vs integration, shared testutils) while adding durability and edge-case coverage for JSONStore/env handling and UpdateHandler, plus introducing atomic env writes and new memory caps in server defaults and handlers.

Sequence diagram for atomic JSONStore env writes

sequenceDiagram
    actor Client
    participant ForkEnvHandler as ForkEnvHandler
    participant JSONStore as JSONStore
    participant OS as os

    Client->>ForkEnvHandler: POST /env/fork
    ForkEnvHandler->>ForkEnvHandler: copy.deepcopy(handler.state[prev_eid])
    ForkEnvHandler->>JSONStore: save_env(eid, handler.state[eid])
    JSONStore->>JSONStore: serialize_env(eid, env_data)
    JSONStore->>JSONStore: _hash_path(eid)
    JSONStore->>JSONStore: _atomic_write(path, payload)
    JSONStore->>OS: open(path + ".tmp", "w")
    OS-->>JSONStore: write(payload)
    JSONStore->>OS: os.replace(tmp, path)

File-Level Changes

Change	Details	Files
Strengthen JSONStore env/undo durability via atomic writes and add coverage for LazyEnvData mapping and deepcopy behaviour.	Introduce a shared _atomic_write helper that writes to .tmp then os.replace for both env and undo saves. Modify serialize_env to use _atomic_write for primary and hash-fallback files, ensuring a .tmp suffix for staging files. Refactor save_undo to delegate to _atomic_write instead of duplicating temp-file logic. Extend existing data_model tests with new durability scenarios (interrupted saves, stranded tmp files, failed rename, long-name hash fallback) and LazyEnvData mapping, error handling, deepcopy, and fork-persistence semantics. Add pytest unit marker and json_store_module alias import for monkeypatching open and os.replace in tests.	`py/visdom/data_model/json_store.py` `py/tests/unit/test_data_model.py`
Introduce unit and integration test infrastructure (markers, shared testutils, HTTP base class) and migrate existing tests into the new layout.	Split tests into py/tests/unit and py/tests/integration and move existing storage/wiring, window, edge-case, and shared-utils tests accordingly. Add pytest markers (unit, integration, slow, server) in pyproject.toml and ensure tests set pytestmark appropriately. Create testutils package with HTTP base class, payload builders, fake handler/socket/store implementations, and document usage in testing context docs. Wire Application defaults to include max_plot_history and expose caps via BaseHandler.initialize and handler attributes. Update docs in .agents/context/testing.md to describe test layout, hermetic requirements, markers, and HTTP testing strategy.	`py/tests/test_storage_wiring.py` `py/tests/integration/test_storage_wiring.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/unit/test_memory_caps.py` `py/tests/unit/test_shared_utils.py` `py/tests/unit/test_server_utils.py` `py/tests/unit/test_image_slider.py` `py/tests/unit/test_window_builder.py` `py/tests/unit/test_tsne.py` `py/tests/testutils/__init__.py` `py/tests/testutils/http.py` `py/tests/testutils/payloads.py` `py/tests/testutils/fakes.py` `.agents/context/testing.md` `pyproject.toml`
Extend UpdateHandler and related server defaults to support plot history caps, robust trace updates, categorical axes, and better HTTP error reporting.	Add DEFAULT_MAX_PLOT_HISTORY and wire it through Application, BaseHandler, and UpdateHandler.update/update_packet, as well as image_slider tests and new memory caps tests. Implement max_plot_history trimming logic for plot_history panes to keep newest frames and adjust selected index appropriately. Replace math.isnan/math.isinf checks with shared _is_missing_value helper to safely skip all-missing numeric updates without breaking categorical x-axes. Change named trace update validation from assert to HTTP 400 error when multiple data entries are provided with name and !delete. Allow injecting a new trace into an empty plot when all previous traces have been deleted, and handle partial data lists without IndexError. Tighten error handling for env fork and window read paths to raise HTTPError with stable, non-eid-specific reasons instead of bare asserts.	`py/visdom/server/handlers/web_handlers.py` `py/visdom/utils/shared_utils.py` `py/visdom/server/defaults.py` `py/visdom/server/app.py` `py/visdom/server/handlers/base_handlers.py` `py/tests/unit/test_image_slider.py` `py/tests/unit/test_memory_caps.py` `py/tests/integration/test_update_plots.py` `py/tests/integration/test_update_text_media.py`
Adjust window registration indexing to avoid reuse collisions after closes and add tests around window CRUD and ordering.	Change register_window to assign window indices as max(existing i) + 1 rather than len(env) so closing a non-last window doesn’t lead to index reuse. Add integration tests for window lifecycle over HTTP: create, exists, read, write, close, update missing windows, and index behaviour under churn. Ensure panes helper in HTTP test base class returns current env panes for ordering assertions.	`py/visdom/utils/server_utils.py` `py/tests/integration/test_window_lifecycle.py` `py/tests/testutils/http.py`
Clarify packaging and build behavior; exclude tests from distribution and add manual visual-check scripts for the UI.	Update setup.py find_packages to exclude tests and tests.* under py to avoid shipping test packages to users. Document test path and pythonpath behavior for py/tests (including testutils as a package) in testing context docs. Move visual_check.py into example/manual, add README describing manual checks, and ensure it is not collected by pytest. Add build verification tests to assert static assets exist and are served correctly, with conditional checks for downloaded frontend dependencies.	`setup.py` `.agents/context/testing.md` `example/manual/visual_check.py` `example/manual/README.md` `py/tests/integration/test_build_verification.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

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.

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.

Manik-Khajuria-5 added 23 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

sourcery-ai Bot reviewed Aug 1, 2026

View reviewed changes

Manik-Khajuria-5 requested review from marcoag, mariobehling, norbusan, tonypzy and vedansh-5 August 1, 2026 12:03

Manik-Khajuria-5 requested review from Jayantparashar10, Saksham-Sirohi and rajnisht7 August 1, 2026 12:03

Let an unnamed delete through the empty-data shortcut

c06dde8

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.

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

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

Open

8 tasks

Manik-Khajuria-5 added 2 commits August 2, 2026 14:51

Merge branch 'dev' into test-04-hygiene-store

e2d8ac5

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

93385ce

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 hygiene: track stray suites, cover shared utils, atomic env writes (PyTest - 4) - #1672

test hygiene: track stray suites, cover shared utils, atomic env writes (PyTest - 4)#1672
Manik-Khajuria-5 wants to merge 26 commits into
fossasia:devfrom
Manik-Khajuria-5:test-04-hygiene-store

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

Motivation and Context

How Has This Been Tested?

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 atomic JSONStore env writes

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