Skip to content

Integration testing with Testcontainers#294

Open
NathanBaulch wants to merge 7 commits into
C2FO:mainfrom
NathanBaulch:testcontainers
Open

Integration testing with Testcontainers#294
NathanBaulch wants to merge 7 commits into
C2FO:mainfrom
NathanBaulch:testcontainers

Conversation

@NathanBaulch

@NathanBaulch NathanBaulch commented Oct 13, 2025

Copy link
Copy Markdown
Contributor

This project would benefit greatly from using Testcontainers to reliably provision ephemeral backend servers for integration testing. This makes it quick and easy to run integration tests against real servers running locally in Docker without the hassle of creating cloud accounts and managing credentials, etc.

This PR duplicates the two existing vfsintegration test suites and modifies them to run their tests against the following local Docker containers:

I've created this sub-module in a folder called "testcontainers" but happy to rename/move.
I've included two backends for S3 since both are quite popular and it's easy enough to test both.
All containers are created using the latest image versions, but there's no reason why they couldn't be pinned to one or more specific versions if needed.

Currently 15 of 340 tests are failing due to bugs/inconsistencies in certain backends. These should be fixed before this PR is merged.

@c2fo-cibot c2fo-cibot Bot added the size/XXL Denotes a PR that changes 1000+ lines label Oct 13, 2025
@NathanBaulch NathanBaulch mentioned this pull request Oct 13, 2025
Merged
@NathanBaulch NathanBaulch changed the title Integration testing with testcontainers Integration testing with Testcontainers Oct 17, 2025
github-advanced-security[bot]
github-advanced-security AI found potential problems Jan 27, 2026
View reviewed changes
Comment thread testcontainers/io_conformance.go Fixed
@NathanBaulch

NathanBaulch commented Jan 27, 2026

Copy link
Copy Markdown
Contributor Author

I was happy to see some work has gone into compliance recently, an area I've also been tackling in this PR. I've merged these improvements and after a few iterations have managed to get all CI checks passing, including the new testcontainers package tests. 🎉

However this PR includes some temporary fixes and is not yet ready to merge in its current state...

  • ftp: Exists should return true for the root / location. Previously it would try searching the parent folder (which makes no sense) to see if / exists (impossible).
  • az: Unlike all other backends, writing without first seeking causes a partial write because the remote file is re-downloaded into tempfile first. I've fixed this behavior to match the other backends.
  • az: Seeking a file that doesn't exist should throw os.ErrNotExist like every other backend.
  • ftp/sftp: These backends URI encode the paths returned from the location/file String func, which causes problems in the "MoveToFile with spaces in path" tests. I've hacked a workaround for now but a more permanent decision should be made on whether the URI and/or String funcs should be URI encoded.
  • ftp: The FTP protocol does not support partial writes, so "Write, Seek, Write" tests cannot be supported. I've hacked in a fix that follows the new ConformanceOptions pattern.
  • ftp: Set the SkipTouchTimestampTest flag since the fauria/vsftpd server doesn't support MDTM or MFMT.
  • Some backends return EOF when reading up to the end of a file (and no further). ExecuteSequence was not handling this non-error state correctly.

@funkyshu funkyshu mentioned this pull request Jul 23, 2026
Merged
funkyshu added a commit that referenced this pull request Jul 23, 2026
@funkyshu @cursoragent
test(conformance): add ConformanceOptions to RunIOTests and fix EOF h…
04ebe51 
…andling (#347)

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

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>

* test(conformance): cover IO runner in CI and make FTP skip robust

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>

* fix(conformance): only forgive io.EOF on a fully-satisfied fixed read

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>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

size/XXL Denotes a PR that changes 1000+ lines

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@NathanBaulch @github-advanced-security