Skip to content

Fix TestFullDIFCConfigFromJSON timeout waiting for backend connections#3960

Merged
lpcox merged 1 commit intomainfrom
fix/difc-integration-test-timeout
Apr 16, 2026
Merged

Fix TestFullDIFCConfigFromJSON timeout waiting for backend connections#3960
lpcox merged 1 commit intomainfrom
fix/difc-integration-test-timeout

Conversation

@lpcox
Copy link
Copy Markdown
Collaborator

@lpcox lpcox commented Apr 16, 2026

Problem

TestFullDIFCConfigFromJSON fails with a 5s timeout waiting for "Starting MCPG" in stderr:

Error: Should be true
Messages: timeout waiting for gateway stderr to contain "Starting MCPG" within 5s

Root Cause

"Starting MCPG in ROUTED/UNIFIED mode" only prints after NewUnified() returns. NewUnified() blocks on registerAllTools() which tries to connect to all backends — including a Docker container (test/server1:latest) with a 30s launch timeout. The test's 5s deadline expires while waiting for Docker to fail.

Fix

The test's assertions only verify DIFC guard registration output, which appears before the blocking backend connections. Changed the wait string from "Starting MCPG" to "[DIFC] Registered guard", added a 300ms flush delay, and removed the health check (which requires full startup).

Changes

File Change
test/integration/difc_config_test.go Wait for guard registration instead of full startup; remove health check and unused encoding/json import

make agent-finished

The test waited 5s for 'Starting MCPG' in stderr, but that message only
prints after NewUnified() returns. NewUnified blocks on registerAllTools()
which tries to connect backends (Docker container launch has a 30s
timeout). The test's assertions only need DIFC guard registration output.

Fix: wait for '[DIFC] Registered guard' instead, which appears before
the blocking backend connections. Remove the health check (requires full
startup) and the now-unused encoding/json import.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 16, 2026 14:59
@lpcox lpcox merged commit faabe08 into main Apr 16, 2026
14 checks passed
@lpcox lpcox deleted the fix/difc-integration-test-timeout branch April 16, 2026 15:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 DIFC integration test to avoid timing out while the gateway blocks on slow backend/tool registration (notably Docker-backed servers), by waiting for earlier DIFC guard-registration logs instead of full startup completion.

Changes:

  • Updated TestFullDIFCConfigFromJSON to wait for DIFC guard registration output rather than "Starting MCPG".
  • Removed the /health check (which requires full gateway startup) and dropped the now-unused encoding/json import.
  • Added a short post-detection delay to allow additional stderr output to flush before terminating the subprocess.
Show a summary per file
File Description
test/integration/difc_config_test.go Makes the test synchronize on DIFC guard-registration logs and removes the health check to avoid backend-connection startup delays.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment on lines +418 to +422
ok := waitForStderr(&stderr, "[DIFC] Registered guard", 5*time.Second)
require.Truef(t, ok, "timeout waiting for DIFC guard registration within %s; stderr:\n%s", 5*time.Second, stderr.String())

var health map[string]interface{}
json.NewDecoder(resp.Body).Decode(&health)
t.Logf("Health response: %+v", health)
}
// Brief pause to let remaining sequential guard registrations flush to stderr
time.Sleep(300 * time.Millisecond)
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test now waits for the first occurrence of "[DIFC] Registered guard" and then sleeps 300ms before killing the gateway. Since launcher.ServerIDs() returns IDs in map iteration order (non-deterministic), you can kill the process after only one guard registration, before the second backend's guard (and related log lines like server1/server2) is emitted—making the assertions below flaky. Prefer waiting for both expected registrations explicitly (e.g., wait for "Registered guard ... server 'server1'" and "... server 'server2'", or wait for both server IDs to appear) and then remove the fixed sleep.

See below for a potential fix:

	// Wait for both configured servers to appear in stderr before stopping the
	// process. Guard registration happens before the blocking registerAllTools
	// backend connections that may take 30+ seconds for Docker containers, but
	// the registrations are sequential and server iteration order is
	// non-deterministic. Waiting for both server-specific log entries avoids
	// flakiness from killing the process after only the first registration.
	ok := waitForStderr(&stderr, "server1", 5*time.Second)
	require.Truef(t, ok, "timeout waiting for server1 DIFC configuration within %s; stderr:\n%s", 5*time.Second, stderr.String())

	ok = waitForStderr(&stderr, "server2", 5*time.Second)
	require.Truef(t, ok, "timeout waiting for server2 DIFC configuration within %s; stderr:\n%s", 5*time.Second, stderr.String())

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants