sketch/loop: fix flaky TestPortMonitor_IntegrationDemo test
Remove assumptions about system-wide port state that made the test flaky.
Root Cause:
- Test assumed it was the only process creating/destroying ports
- Made brittle assertions about total port counts before/after test servers
- Failed when other processes (tests, services, containers) modified ports
The Fix:
- Focus only on the specific test ports created by the test
- Use polling with timeout to detect port creation/removal
- Remove assertions about total port counts
- Test only verifies its own test ports are detected and removed
Changes:
- Replace fixed sleeps with polling loops with timeouts
- Remove port count comparisons that depended on system state
- Keep track of test ports separately from system port scanning
- Add proper timeout handling to prevent hanging tests
Test Results:
- Before: Flaky failures when other processes modified ports
- After: 5 consecutive runs all passed, focusing only on test-specific behavior
This preserves the test's usefulness as an integration test while making it
deterministic and isolated from system-wide port changes.
Co-Authored-By: sketch <hello@sketch.dev>
Change-ID: s58312d8275959960k
diff --git a/go.mod b/go.mod
index 18f76c2..1377fc8 100644
--- a/go.mod
+++ b/go.mod
@@ -10,7 +10,6 @@
github.com/evanw/esbuild v0.25.2
github.com/fatih/color v1.18.0
github.com/gliderlabs/ssh v0.3.8
- github.com/google/go-cmp v0.7.0
github.com/google/uuid v1.6.0
github.com/kevinburke/ssh_config v1.2.0
github.com/mark3labs/mcp-go v0.32.0
@@ -25,6 +24,7 @@
golang.org/x/term v0.32.0
golang.org/x/tools v0.32.0
mvdan.cc/sh/v3 v3.11.1-0.20250530001257-46bb4f2b309f
+ tailscale.com v1.84.3
)
require (
@@ -43,7 +43,6 @@
golang.org/x/mod v0.24.0 // indirect
golang.org/x/sys v0.33.0 // indirect
golang.org/x/text v0.24.0 // indirect
- tailscale.com v1.84.3 // indirect
)
tool golang.org/x/tools/cmd/stringer
diff --git a/go.sum b/go.sum
index b00a177..8792d6c 100644
--- a/go.sum
+++ b/go.sum
@@ -9,8 +9,9 @@
github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
+github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
github.com/evanw/esbuild v0.25.2 h1:ublSEmZSjzOc6jLO1OTQy/vHc1wiqyDF4oB3hz5sM6s=
@@ -21,8 +22,6 @@
github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
github.com/gliderlabs/ssh v0.3.8 h1:a4YXD1V7xMF9g5nTkdfnja3Sxy1PVDCj1Zg4Wb8vY6c=
github.com/gliderlabs/ssh v0.3.8/go.mod h1:xYoytBv1sV0aL3CavoDuJIQNURXkkfPA/wxQ1pL1fAU=
-github.com/go-json-experiment/json v0.0.0-20250211171154-1ae217ad3535 h1:yE7argOs92u+sSCRgqqe6eF+cDaVhSPlioy1UkA0p/w=
-github.com/go-json-experiment/json v0.0.0-20250211171154-1ae217ad3535/go.mod h1:BWmvoE1Xia34f3l/ibJweyhrT+aROb/FQ6d+37F0e2s=
github.com/go-json-experiment/json v0.0.0-20250223041408-d3c622f1b874 h1:F8d1AJ6M9UQCavhwmO6ZsrYLfG8zVFWfEfMS2MXPkSY=
github.com/go-json-experiment/json v0.0.0-20250223041408-d3c622f1b874/go.mod h1:TiCD2a1pcmjd7YnhGH0f/zKNcCD06B029pHhzV23c2M=
github.com/go-quicktest/qt v1.101.0 h1:O1K29Txy5P2OK0dGo59b7b0LR6wKfIhttaAhHUyn7eI=
@@ -62,8 +61,9 @@
github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o=
github.com/pkg/sftp v1.13.9 h1:4NGkvGudBL7GteO3m6qnaQ4pC0Kvf0onSVc9gR3EWBw=
github.com/pkg/sftp v1.13.9/go.mod h1:OBN7bVXdstkFFN/gdnHPUb5TE8eb8G1Rp9wCItqjkkA=
-github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
+github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
+github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/richardlehane/crock32 v1.0.1 h1:GV9EqtAr7RminQ8oGrDt3gYXkzDDPJ5fROaO1Mux14g=
github.com/richardlehane/crock32 v1.0.1/go.mod h1:xUIlLABtHBgs1bNIBdUQR9F2xtRzS0TujtbR68hmEWU=
github.com/rogpeppe/go-internal v1.14.1 h1:UQB4HGPB6osV0SQTLymcB4TgvyWu6ZyliaW0tI/otEQ=
@@ -93,6 +93,8 @@
golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk=
golang.org/x/crypto v0.37.0 h1:kJNSjF/Xp7kU0iB2Z+9viTPMW4EqqsrywMXLJOOsXSE=
golang.org/x/crypto v0.37.0/go.mod h1:vg+k43peMZ0pUMhYmVAWysMK35e6ioLh3wB8ZCAfbVc=
+golang.org/x/exp v0.0.0-20250210185358-939b2ce775ac h1:l5+whBCLH3iH2ZNHYLbAe58bo7yrN4mVcnkHDYz5vvs=
+golang.org/x/exp v0.0.0-20250210185358-939b2ce775ac/go.mod h1:hH+7mtFmImwwcMvScyxUhjuVHR3HGaDPMn9rMSUUbxo=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
diff --git a/loop/port_monitor_demo_test.go b/loop/port_monitor_demo_test.go
index 4190e5a..4c67790 100644
--- a/loop/port_monitor_demo_test.go
+++ b/loop/port_monitor_demo_test.go
@@ -12,12 +12,13 @@
// TestPortMonitor_IntegrationDemo demonstrates the full integration of PortMonitor with an Agent.
// This test shows how the PortMonitor detects port changes and sends notifications to an Agent.
+// The test focuses on specific ports it creates rather than making assumptions about total port counts.
func TestPortMonitor_IntegrationDemo(t *testing.T) {
// Create a test agent
agent := createTestAgent(t)
// Create and start the port monitor
- pm := NewPortMonitor(agent, 100*time.Millisecond) // Fast polling for demo
+ pm := NewPortMonitor(agent, 25*time.Millisecond) // Fast polling for demo
ctx := context.Background()
err := pm.Start(ctx)
if err != nil {
@@ -25,9 +26,6 @@
}
defer pm.Stop()
- // Wait for initial scan
- time.Sleep(200 * time.Millisecond)
-
// Show current ports
currentPorts := pm.GetPorts()
t.Logf("Initial TCP ports detected: %d", len(currentPorts))
@@ -38,6 +36,7 @@
// Start multiple test servers to demonstrate detection
var listeners []net.Listener
var wg sync.WaitGroup
+ var testPorts []uint16
// Start 3 test HTTP servers
for i := 0; i < 3; i++ {
@@ -49,6 +48,7 @@
addr := listener.Addr().(*net.TCPAddr)
port := addr.Port
+ testPorts = append(testPorts, uint16(port))
t.Logf("Started test HTTP server %d on port %d", i+1, port)
// Start a simple HTTP server
@@ -64,43 +64,39 @@
}(listener, i+1)
}
- // Wait for ports to be detected
- time.Sleep(500 * time.Millisecond)
+ // Wait for ports to be detected with timeout
+ detectionTimeout := time.After(15 * time.Second)
+ detectionTicker := time.NewTicker(25 * time.Millisecond)
+ defer detectionTicker.Stop()
- // Check that the new ports were detected
- updatedPorts := pm.GetPorts()
- t.Logf("Updated TCP ports detected: %d", len(updatedPorts))
+ allPortsDetected := false
+detectionLoop:
+ for !allPortsDetected {
+ select {
+ case <-detectionTimeout:
+ t.Fatalf("Timeout waiting for test ports to be detected")
+ case <-detectionTicker.C:
+ // Check if all test ports are detected
+ updatedPorts := pm.GetPorts()
+ portMap := make(map[uint16]bool)
+ for _, port := range updatedPorts {
+ portMap[port.Port] = true
+ }
- // Verify that we have at least 3 more ports than initially
- if len(updatedPorts) < len(currentPorts)+3 {
- t.Errorf("Expected at least %d ports, got %d", len(currentPorts)+3, len(updatedPorts))
- }
-
- // Find the new test server ports
- var testPorts []uint16
- for _, listener := range listeners {
- addr := listener.Addr().(*net.TCPAddr)
- testPorts = append(testPorts, uint16(addr.Port))
- }
-
- // Verify all test ports are detected
- portMap := make(map[uint16]bool)
- for _, port := range updatedPorts {
- portMap[port.Port] = true
- }
-
- allPortsDetected := true
- for _, testPort := range testPorts {
- if !portMap[testPort] {
- allPortsDetected = false
- t.Errorf("Test port %d was not detected", testPort)
+ allPortsDetected = true
+ for _, testPort := range testPorts {
+ if !portMap[testPort] {
+ allPortsDetected = false
+ break
+ }
+ }
+ if allPortsDetected {
+ t.Logf("All test ports successfully detected: %v", testPorts)
+ break detectionLoop
+ }
}
}
- if allPortsDetected {
- t.Logf("All test ports successfully detected!")
- }
-
// Close all listeners
for i, listener := range listeners {
t.Logf("Closing test server %d", i+1)
@@ -110,41 +106,41 @@
// Wait for servers to stop
wg.Wait()
- // Wait for ports to be removed
- time.Sleep(500 * time.Millisecond)
+ // Wait for ports to be removed with timeout
+ removalTimeout := time.After(15 * time.Second)
+ removalTicker := time.NewTicker(25 * time.Millisecond)
+ defer removalTicker.Stop()
- // Check that ports were removed
- finalPorts := pm.GetPorts()
- t.Logf("Final TCP ports detected: %d", len(finalPorts))
+ allPortsRemoved := false
+removalLoop:
+ for !allPortsRemoved {
+ select {
+ case <-removalTimeout:
+ t.Fatalf("Timeout waiting for test ports to be removed")
+ case <-removalTicker.C:
+ // Check if all test ports are removed
+ finalPorts := pm.GetPorts()
+ portMap := make(map[uint16]bool)
+ for _, port := range finalPorts {
+ portMap[port.Port] = true
+ }
- // Verify the final port count is back to near the original
- if len(finalPorts) > len(currentPorts)+1 {
- t.Errorf("Expected final port count to be close to initial (%d), got %d", len(currentPorts), len(finalPorts))
- }
-
- // Verify test ports are no longer detected
- portMap = make(map[uint16]bool)
- for _, port := range finalPorts {
- portMap[port.Port] = true
- }
-
- allPortsRemoved := true
- for _, testPort := range testPorts {
- if portMap[testPort] {
- allPortsRemoved = false
- t.Errorf("Test port %d was not removed", testPort)
+ allPortsRemoved = true
+ for _, testPort := range testPorts {
+ if portMap[testPort] {
+ allPortsRemoved = false
+ break
+ }
+ }
+ if allPortsRemoved {
+ t.Logf("All test ports successfully removed: %v", testPorts)
+ break removalLoop
+ }
}
}
- if allPortsRemoved {
- t.Logf("All test ports successfully removed!")
- }
-
t.Logf("Integration test completed successfully!")
- t.Logf("- Initial ports: %d", len(currentPorts))
- t.Logf("- Peak ports: %d", len(updatedPorts))
- t.Logf("- Final ports: %d", len(finalPorts))
- t.Logf("- Test ports added and removed: %d", len(testPorts))
+ t.Logf("- Test ports created and monitored: %v", testPorts)
}
// contains checks if a string contains a substring.