Skip to content

unit and integration tests for filesystem pause/resume integrity#2252

Open
matthewlouisbrockman wants to merge 9 commits intozero-copy-fsfrom
zero-copy-fs-tests
Open

unit and integration tests for filesystem pause/resume integrity#2252
matthewlouisbrockman wants to merge 9 commits intozero-copy-fsfrom
zero-copy-fs-tests

Conversation

@matthewlouisbrockman
Copy link
Copy Markdown
Contributor

@matthewlouisbrockman matthewlouisbrockman commented Mar 29, 2026

Add unit and integration coverage for zero-copy-fs, covering Cache.ExportToDiff semantics and filesystem integrity across pause/resume for contiguous, scattered, and zeroed/truncate write patterns.

includes an opt-in stress test via env TESTS_FS_INTEGRITY_STRESS=1 for several gigabyte pause/resume validation (not automatically run on ci to avoid taking forever)

@cursor
Copy link
Copy Markdown

cursor bot commented Mar 29, 2026

PR Summary

Low Risk
Test-only changes; main risk is increased test runtime/flakiness due to new integration and optional stress coverage around pause/resume flows.

Overview
Adds new unit and integration tests to validate zero-copy filesystem snapshot behavior: Cache.ExportToDiff now has explicit coverage ensuring zero-filled dirty blocks are still exported as dirty payloads (and mapped to the snapshot build) and that non-contiguous dirty ranges preserve export ordering. Adds an integration suite that writes files with contiguous/scattered patterns plus zeroing+truncate, then repeatedly pause/resumes sandboxes and asserts file hash/size stability, with an opt-in large stress variant gated by TESTS_FS_INTEGRITY_STRESS.

Written by Cursor Bugbot for commit 1836ec5. This will update automatically on new commits. Configure here.

Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

LGTM — test-only PR with three minor nits already flagged inline.

Extended reasoning...

Overview

This PR adds unit tests to cache_test.go covering Cache.ExportToDiff semantics (zero dirty blocks, mixed dirty blocks, non-contiguous ranges) and a new integration test file for filesystem pause/resume integrity across contiguous, scattered, and zeroed/truncate write patterns.

Security risks

None. The changes are entirely test code with no modifications to production logic.

Level of scrutiny

Low. Test-only additions that exercise already-merged production code. The unit tests follow established patterns; the integration tests are gated behind the existing test infrastructure and an opt-in env var for the stress variant.

Other factors

Three nits were flagged inline — a missing resp.JSON201 nil check, a hardcoded path string that diverges from a Go constant, and a no-op for i := 0; i < 1 loop. All are in test code and none affect correctness of the tests under normal conditions. The overall coverage added is valuable and the structure is sound.

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

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.

3 participants