Skip to content

test(conformance): add ConformanceOptions to RunIOTests and fix EOF handling#347

Merged
funkyshu merged 3 commits into
mainfrom
feat/conformance-io-options
Jul 23, 2026
Merged

test(conformance): add ConformanceOptions to RunIOTests and fix EOF handling#347
funkyshu merged 3 commits into
mainfrom
feat/conformance-io-options

Conversation

@funkyshu

Copy link
Copy Markdown
Member

Summary

Prepares the canonical backend/testsuite conformance suite so it can be reused by backends (and a future Testcontainers module, see #294) instead of being duplicated. Two changes:

Added

  • RunIOTests now accepts optional ConformanceOptions (the same options type already used by RunConformanceTests), so a backend can opt out of IO sequences it genuinely cannot support. SkipFTPSpecificTests skips the Write, Seek, Write (partial write) cases — FTP streams writes directly to the server and cannot rewind an in-progress write.
  • The FTP TestIOConformance (vfsintegration) now passes SkipFTPSpecificTests: true; previously it had no way to, so those cases would fail against a real FTP server.

Fixed

  • ExecuteSequence EOF handling. A fixed-size read (R(n)) that returns io.EOF alongside the final bytes is a valid end-of-file condition on some backends. The sequence runner treated any error — including io.EOF — as fatal and aborted. It now recognizes io.EOF as a successful read and continues the sequence.

Notes

  • Reuses the existing ConformanceOptions struct rather than introducing a parallel IOOptions type, keeping one coherent options surface across the suite.
  • The RunIOTests signature change is backward compatible (variadic), so existing callers (mem, os, s3, gs, azure, sftp, dropbox contrib) are unaffected.
  • Skipped cases now use 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, so Write, Seek, Write runs 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

…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>
@c2fo-cibot c2fo-cibot Bot added the size/M Denotes a PR that changes 30-99 lines label Jul 23, 2026
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>
@c2fo-cibot c2fo-cibot Bot added size/L Denotes a PR that changes 100-499 lines and removed size/M Denotes a PR that changes 30-99 lines labels Jul 23, 2026
@funkyshu
funkyshu requested a review from Copilot July 23, 2026 02:18

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.

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 RunIOTests to accept optional ConformanceOptions, 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: true for 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.

Comment thread backend/testsuite/io_conformance.go
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>
@funkyshu
funkyshu merged commit 04ebe51 into main Jul 23, 2026
34 checks passed
@funkyshu
funkyshu deleted the feat/conformance-io-options branch July 23, 2026 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Denotes a PR that changes 100-499 lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants