test(conformance): add ConformanceOptions to RunIOTests and fix EOF handling#347
Merged
Conversation
…andling Adds optional ConformanceOptions to RunIOTests so backends can skip IO sequences they cannot support; SkipFTPSpecificTests skips the Write,Seek,Write (partial write) cases. Fixes ExecuteSequence to treat io.EOF from a fixed-size read as a valid end-of-file (continue) rather than aborting the sequence. Wires the FTP IO conformance test to skip partial writes. Prepares the canonical testsuite for reuse by a testcontainers module (see #294). Co-authored-by: Cursor <cursoragent@cursor.com>
Addresses review findings on PR B: - Add untagged tests (backend/testsuite/io_conformance_test.go) that run RunIOTests against the mem backend and cover ExecuteSequence's io.EOF continue path via a fake that returns io.EOF alongside a full read. Previously all callers were vfsintegration-gated, so this logic had no coverage in the default test run. - Replace the fragile description-prefix match for the FTP skip with a structured IOTestCase.RequiresSeekWithinWrite field, and lock the mapping with a test. Note left re: verifying the exact FTP-unsupported sequence set against vsftpd in PR C. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the backend/testsuite IO conformance suite to be more reusable across backends by (1) allowing IO tests to accept ConformanceOptions (consistent with RunConformanceTests) so backends can explicitly skip unsupported IO sequences, and (2) improving the sequence runner’s handling of io.EOF on fixed-size reads to avoid incorrectly failing valid EOF scenarios.
Changes:
- Extended
RunIOTeststo accept optionalConformanceOptions, enabling selective skipping of IO sequences (e.g., FTP partial-write patterns). - Added IO conformance unit tests (mem-backed) to cover both the new skip path and the updated EOF behavior.
- Updated the FTP integration conformance test to pass
SkipFTPSpecificTests: truefor unsupported seek-within-write sequences.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
CHANGELOG.md |
Documents the new IO conformance options support and the EOF-handling fix under Unreleased. |
backend/testsuite/io_conformance.go |
Adds opt-in skipping for seek-within-write sequences and adjusts EOF handling during fixed-size reads. |
backend/testsuite/io_conformance_test.go |
Adds unit tests for the new RunIOTests options behavior and EOF-on-full-read handling. |
backend/ftp/conformance_test.go |
Passes SkipFTPSpecificTests to IO conformance to avoid unsupported FTP sequences. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses Copilot review: ExecuteSequence discarded the byte count from file.Read and swallowed io.EOF unconditionally, which would also mask a genuine short read (e.g. (0, io.EOF)) as success. Now only forgive EOF when n == the requested byte count; anything less remains a failure. Adds a regression test (shortReadEOFFile) proving a short read paired with io.EOF is reported as an error rather than silently continuing. Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Prepares the canonical
backend/testsuiteconformance suite so it can be reused by backends (and a future Testcontainers module, see #294) instead of being duplicated. Two changes:Added
RunIOTestsnow accepts optionalConformanceOptions(the same options type already used byRunConformanceTests), so a backend can opt out of IO sequences it genuinely cannot support.SkipFTPSpecificTestsskips theWrite, Seek, Write(partial write) cases — FTP streams writes directly to the server and cannot rewind an in-progress write.TestIOConformance(vfsintegration) now passesSkipFTPSpecificTests: true; previously it had no way to, so those cases would fail against a real FTP server.Fixed
ExecuteSequenceEOF handling. A fixed-size read (R(n)) that returnsio.EOFalongside the final bytes is a valid end-of-file condition on some backends. The sequence runner treated any error — includingio.EOF— as fatal and aborted. It now recognizesio.EOFas a successful read and continues the sequence.Notes
ConformanceOptionsstruct rather than introducing a parallelIOOptionstype, keeping one coherent options surface across the suite.RunIOTestssignature change is backward compatible (variadic), so existing callers (mem,os,s3,gs,azure,sftp, dropbox contrib) are unaffected.t.Skip(visible as skipped) rather than a silent early return.Testing
go build ./...go vet -tags=vfsintegration ./backend/ftp/... ./backend/testsuite/...go test -tags=vfsintegration -run TestConformance -run TestIOConformance ./backend/mem/... ./backend/os/...— all IO sequences pass, including read-to-EOF cases (mem/os support partial writes, soWrite, Seek, Writeruns and passes there).golangci-lint run --build-tags=vfsintegration ./backend/testsuite/... ./backend/ftp/...— 0 issues.Part of a series extracted from #294 (this is "PR B"; the Azure/FTP backend fixes landed in #346).
Made with Cursor