Skip to content

files: cover buffered stream write lifecycle edge cases - #337043

Merged
Christof Marti (chrmarti) merged 1 commit into
mainfrom
chrmarti/test-buffered-stream-write-lifecycle
Sep 21, 2026
Merged

Christof Marti (chrmarti) merged 1 commit into
mainfrom
chrmarti/test-buffered-stream-write-lifecycle

Conversation

@chrmarti

Copy link
Copy Markdown
Collaborator

Summary

Tests-only follow-up to #337018, based on the latest main after that fix merged. Adds six deterministic file-service tests for the lifecycle and ordering pitfalls identified while reviewing the stream-write implementation's history:

  • Multiple buffered chunks emitted together remain serialized, including partial provider writes; verify exact file/buffer offsets, maximum concurrent writes, final bytes, and size.
  • A producer blocked by backpressure resumes across multiple chunks.
  • The peeked prefix is written before the remaining streamed chunks, with continuous file offsets.
  • A queued write to the same resource waits for pending work and handle closure, then succeeds after either a provider-write failure or a stream failure (two cases).
  • An empty stream closes its handle without issuing provider writes.

Uses the existing in-memory filesystem provider so assertions cover real content and handle lifecycle, while deferred writes and event-loop yields exercise the relevant ordering. No production code changes.

Validation

  • npm run gulp compile-client: passed with 0 errors.
  • File Service + InMemory File Service unit suites: 39 passing.
  • ESLint on the changed test file, git diff --check, and commit hygiene: passed.
  • Mutation check: temporarily restored immediate stream end/error completion in generated output only. The new partial-write test and both queue-recovery tests failed as expected (3 failing). Restored the generated output and reran both suites: 39 passing.

Local validation performed on Windows; cross-platform validation is left to PR CI.

Add regression coverage for buffered chunk serialization, partial writes, producer backpressure, peeked prefixes, empty streams, and queued writes following provider or stream failures. Follow up on #337018 without changing production code.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 21, 2026 14:23

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

The tests accurately cover the intended lifecycle scenarios without production-code changes or unresolved issues.

Review effort: Balanced
Findings: None

What changed in this PR

Adds deterministic regression coverage for buffered file-write lifecycle and ordering edge cases.

Changes:

  • Tests serialization, partial writes, backpressure, and peeked-prefix ordering.
  • Tests queue recovery after write/stream failures.
  • Verifies empty streams close without provider writes.
File Description
src/​vs/​platform/​files/​test/​browser/​fileService.test.ts Adds six buffered stream-write lifecycle tests and an in-memory buffered provider helper.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@chrmarti
Christof Marti (chrmarti) merged commit 3e1093f into main Sep 21, 2026
54 of 55 checks passed
@chrmarti
Christof Marti (chrmarti) deleted the chrmarti/test-buffered-stream-write-lifecycle branch September 21, 2026 16:32
@vs-code-engineering vs-code-engineering Bot added this to the 1.140.0 milestone Sep 21, 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.

3 participants