fix(#2002): mock HTTP server in TestResolveLinuxBinary_Download - #2007
fix(#2002): mock HTTP server in TestResolveLinuxBinary_Download#2007fullsend-ai-coder[bot] wants to merge 1 commit into
Conversation
Replace live GitHub CDN calls in TestResolveLinuxBinary_Download with an httptest.Server serving mock checksums.txt and tarball data. This eliminates the external network dependency that caused merge queue ejections during GitHub CDN outages (e.g., the June 8 incident affecting PRs #1238, #1239, #1468, #1531, #1905). The refactored test follows the same pattern already used by TestDownloadReleaseBinary_ChecksumMatch and TestDownloadReleaseBinary_ChecksumMismatch in the same file: build a tar.gz with mock content, compute its SHA256, serve both via httptest, and override the package-level releaseBaseURL var. The ELF validation assertion was removed since the mock binary is not a real ELF — validateLinuxBinary is independently tested by TestValidateLinuxBinary_* tests elsewhere in the file. Note: Go tests could not run in the sandbox (requires Go 1.26.0, sandbox has Go 1.24.13). The change is syntactically valid (gofmt passes) and follows the exact pattern of adjacent passing tests. Manual verification of go test is required. Closes #2002
Site previewPreview: https://9289b6a5-site.fullsend-ai.workers.dev Commit: |
ReviewFindingsLow
Info
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| data, err := os.ReadFile(binPath) | ||
| require.NoError(t, err) | ||
| assert.Equal(t, "mock fullsend binary", string(data)) | ||
| } |
There was a problem hiding this comment.
[low] test-coverage-regression
The removal of the validateLinuxBinary assertion reduces integration-level validation that downloadReleaseBinary produces a valid ELF. This is a necessary trade-off for the mock strategy, since the mock binary is not a real ELF, and validateLinuxBinary is independently tested by TestValidateLinuxBinary_* tests (lines 454-475).
Suggested fix: Consider adding a comment explaining why validateLinuxBinary is not called (e.g., '// Mock binary is not a real ELF; validateLinuxBinary is tested separately in TestValidateLinuxBinary_*').
fullsend-ai-review
Bot
added
the
ready-for-merge
|
Closing as superseded. The functions this PR tests ( |
|
🤖 Finished Retro · ✅ Success · Started 6:49 AM UTC · Completed 7:01 AM UTC |
Retro: PR #2007 — mock HTTP server in TestResolveLinuxBinary_DownloadOverall: The fullsend pipeline worked efficiently end-to-end — retro agent filed issue #2002, triage labeled it What went wrong: The PR sat with Existing issues covering most findings:
One novel proposal filed below for the orphaned-issue lifecycle gap not covered by existing issues. Proposals filed
|
1 participant
Replace live GitHub CDN calls in TestResolveLinuxBinary_Download with an httptest.Server serving mock checksums.txt and tarball data. This eliminates the external network dependency that caused merge queue ejections during GitHub CDN outages (e.g., the June 8 incident affecting PRs #1238, #1239, #1468, #1531, #1905).
The refactored test follows the same pattern already used by TestDownloadReleaseBinary_ChecksumMatch and TestDownloadReleaseBinary_ChecksumMismatch in the same file: build a tar.gz with mock content, compute its SHA256, serve both via httptest, and override the package-level releaseBaseURL var.
The ELF validation assertion was removed since the mock binary is not a real ELF — validateLinuxBinary is independently tested by TestValidateLinuxBinary_* tests elsewhere in the file.
Note: Go tests could not run in the sandbox (requires Go 1.26.0, sandbox has Go 1.24.13). The change is syntactically valid (gofmt passes) and follows the exact pattern of adjacent passing tests. Manual verification of go test is required.
Closes #2002
Post-script verification
agent/2002-mock-download-test)be348d5073262d3ada97382950d093c71441dcf1..HEAD)