fix: increase integration test timeouts for Docker image pulls in CI#5118
Merged
fix: increase integration test timeouts for Docker image pulls in CI#5118
Conversation
TestBinaryInvocation_LogFileCreation and TestDIFCConfigWithGuards fail in CI because Docker images (alpine:latest, mcp/playwright:latest) are not pre-pulled and the pull time exceeds the short test timeouts (5s and 15s respectively). Increase timeouts to accommodate Docker pulls: - TestBinaryInvocation_LogFileCreation: 5s → 40s (context 10s → 45s) - TestBinaryInvocation_LogDirEnvironmentVariable: 5s → 40s (context 10s → 45s) - TestDIFCConfigWithGuards: 15s → 50s (context 30s → 60s) Fixes: https://github.com/github/gh-aw-mcpg/actions/runs/25325915594 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts a few slow-start integration tests so they remain reliable in CI when Docker images need to be pulled before the gateway becomes healthy. It fits the test suite by addressing startup timing assumptions rather than changing runtime behavior.
Changes:
- Increased subprocess context deadlines for two binary integration tests and one DIFC/guards integration test.
- Increased startup wait windows used by
/healthpolling or stderr-based startup detection. - Updated nearby test comments to document that the extra time is intended to absorb Docker image pull latency in CI.
Show a summary per file
| File | Description |
|---|---|
test/integration/difc_config_test.go |
Extends the guarded DIFC startup timeout and stderr wait window for slower CI container pulls. |
test/integration/binary_test.go |
Extends startup and process context timeouts for the log file integration tests that launch Docker-backed backends. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
This was referenced May 4, 2026
This was referenced May 4, 2026
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.
Problem
TestBinaryInvocation_LogFileCreationandTestDIFCConfigWithGuardsfail in CI because Docker images (alpine:latest,mcp/playwright:latest) are not pre-pulled, and the time to pull them exceeds the short test timeouts (5s and 15s respectively).See: https://github.com/github/gh-aw-mcpg/actions/runs/25325915594/job/74247185258
Root Cause
NewUnified()callsregisterAllTools()which launches Docker containers and waits for them to connect. The HTTP listener only starts AFTERNewUnifiedreturns, so/healthisn't available until all backends finish connecting (or timeout). In CI, Docker image pulls add 15-30s, exceeding the test timeouts.Fix
Increase timeouts to accommodate Docker pulls in CI:
TestBinaryInvocation_LogFileCreation: 5s → 40s (context 10s → 45s)TestBinaryInvocation_LogDirEnvironmentVariable: 5s → 40s (context 10s → 45s)TestDIFCConfigWithGuards: 15s → 50s (context 30s → 60s)These timeouts are generous enough for slow CI environments while still catching real startup failures.