fix azure/ftp bugs found in testcontainers pr - #346
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes behavioral inconsistencies/bugs in the Azure and FTP backends that were surfaced by testcontainers-related work, aligning Azure read/write semantics with other backends and correcting FTP root existence handling.
Changes:
backend/azure: Make Read/Seek on missing blobs returnos.ErrNotExist(instead of silently reading from an empty temp file), and ensure Write starts from an empty temp file without downloading existing blob contents first.backend/ftp: Treat the root location (/) as always existing (avoiding attempts to list a nonexistent parent).- Add/adjust unit tests and update
CHANGELOG.mdaccordingly.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| CHANGELOG.md | Documents the Azure/FTP bug fixes under ## [Unreleased]. |
| backend/ftp/location.go | Adds a special-case root (/) existence check in Location.Exists(). |
| backend/ftp/location_test.go | Adds coverage for root existence and updates non-root expectations. |
| backend/ftp/file_test.go | Fixes test setup for target location path in MoveToFile same-authority scenario. |
| backend/azure/file.go | Updates temp-file initialization logic to distinguish read vs write semantics. |
| backend/azure/file_test.go | Updates existing tests and adds new cases for missing-blob and write-no-download behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ot-exist API tests, clarify doc Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced Jul 23, 2026
funkyshu
added a commit
that referenced
this pull request
Jul 25, 2026
…persedes #294) (#348) * test(integration): add testcontainers-based backend conformance suite Adds a new test-only `testcontainers/` module that provisions real backend servers (minio, azurite, fake-gcs-server, atmoz/sftp, fauria/vsftpd) as disposable Docker containers and runs the shared backend/testsuite conformance and IO suites against them, plus a dedicated "Integration (testcontainers)" CI workflow. No credentials or manual setup required. Also fixes the conformance suite's special-character move assertion to compare File.Path() instead of File.URI() so sftp/ftp pass the same suite. Derived from the community contribution in #294 by Nathan Baulch, reworked and split into #346 (Azure/FTP bug fixes), #347 (conformance ConformanceOptions + EOF handling), and this module. Co-authored-by: Nathan Baulch <249604+NathanBaulch@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix(testcontainers): pin go directive to 1.25.11 per VFS go-version policy The module's go.mod declared `go 1.26`, which broke the CI matrix job running on Go 1.25 (VFS supports the latest Go minus one minor version). `go mod tidy` had resolved github.com/jlaffaye/ftp to the tagged v0.2.1 (which requires go 1.26) instead of the go-1.17 pseudo-version the core module pins. Pin jlaffaye/ftp to that same pseudo-version so the whole module builds on Go 1.25, and set the go directive to 1.25.11 to match the core and contrib modules. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(testcontainers): pin FTP control host to IPv4 loopback for CI On GitHub-hosted runners ctr.Host() returns "localhost", which resolves to the IPv6 loopback (::1) first. jlaffaye/ftp reuses the control-connection host for the passive data connection (discarding the private PASV IP the server advertises), so it dialed [::1]:21100. The passive ports are published on IPv4 only (0.0.0.0) via HostConfigModifier, so those data connections were refused ("dial tcp [::1]:21100: connect: connection refused"). Pin the control host to 127.0.0.1 (already used for PASV_ADDRESS) so the reused data-connection host stays on IPv4 and matches the published passive ports. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(copilot review): address PR #348 review findings - integration.yml: gate the Docker Hub login step on both DOCKERHUB_USERNAME *and* DOCKERHUB_TOKEN being non-empty, so a half-configured secret set does not run the login step and fail the workflow. - backend/testsuite: reword the two special-character move-assertion messages so they accurately describe what was checked. The move-back-to-source assertion had inherited a copy-pasted "destination file ... source string" message that mislabeled `newSrcSpaces` and the expected suffix; both messages now describe the operation ("moved-to-destination" vs "moved-back-to-source") and the expected suffix explicitly. The third review comment (loop-variable capture in the errgroup.SetupSuite goroutines) is a false positive under Go 1.22+ per-iteration loop scoping; the testcontainers module pins `go 1.25.11`, so `i` and `reg` are correctly captured per iteration and no code change is needed. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Nathan Baulch <249604+NathanBaulch@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
6 tasks
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.
No description provided.