Skip to content

Commit 025cb88

Browse files
adnaanclaude
adnaan
and
claude
authored
fix(e2e): add pre-test cleanup for stale Docker containers (#34)
* fix(e2e): add pre-test cleanup for stale Docker containers Add cleanup for lvt-test-* containers that can linger after test crashes or interruptions. Previously, only chrome-e2e-test-* containers were cleaned up, which could lead to port conflicts (e.g., port 8800 already allocated) when running tests after a crashed test run. Changes: - Add CleanupTestContainers() to remove stale app containers - Add CleanupAllTestContainers() convenience function - Update TestMain to clean both Chrome and app containers before/after tests - Clean up on SIGINT/SIGTERM to prevent orphaned containers Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address bot review comments - Move testContainerPrefix to package-level const block (Copilot, Claude) - Add documentation explaining why no grace period for test containers - Remove unused cleanupChromeContainers() function (Copilot) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent a5e8337 commit 025cb88

3 files changed

Lines changed: 36 additions & 8 deletions

File tree

‎e2e/shared_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,20 @@ func TestMain(m *testing.M) {
3737

3838
go func() {
3939
<-sigCh
40-
log.Println("🛑 Interrupted - cleaning up Chrome containers...")
40+
log.Println("🛑 Interrupted - cleaning up test containers...")
4141
chromePoolMu.Lock()
4242
if chromePool != nil {
4343
chromePool.Cleanup()
4444
}
4545
chromePoolMu.Unlock()
46-
cleanupChromeContainers()
46+
cleanupAllTestContainers()
4747
log.Println("✅ Cleanup complete")
4848
os.Exit(1)
4949
}()
5050

51-
// Cleanup any leftover containers from previous runs
51+
// Cleanup any leftover containers from previous runs (Chrome + app containers)
5252
// This is safe to run even if Docker is not available - it will just log a warning
53-
cleanupChromeContainers()
53+
cleanupAllTestContainers()
5454

5555
// Run tests
5656
code := m.Run()
@@ -63,7 +63,7 @@ func TestMain(m *testing.M) {
6363
chromePoolMu.Unlock()
6464

6565
// Final cleanup of any remaining containers
66-
cleanupChromeContainers()
66+
cleanupAllTestContainers()
6767

6868
os.Exit(code)
6969
}

‎e2e/test_main_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ package e2e
44

55
import e2etest "github.com/livetemplate/lvt/testing"
66

7-
func cleanupChromeContainers() {
8-
e2etest.CleanupChromeContainers()
7+
// cleanupAllTestContainers removes both Chrome and app test containers.
8+
// This should be called at the start of tests to ensure a clean environment.
9+
func cleanupAllTestContainers() {
10+
e2etest.CleanupAllTestContainers()
911
}

‎testing/chrome.go‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,13 @@ func GetClientLibraryJS() []byte {
2727
const (
2828
dockerImage = "chromedp/headless-shell:latest"
2929
chromeContainerPrefix = "chrome-e2e-test-"
30-
staleContainerGrace = 1 * time.Minute // Reduced from 10 min for faster cleanup
30+
// testContainerPrefix is used for app containers in integration tests.
31+
// These containers are created by runDockerContainer in e2e tests.
32+
// Unlike Chrome containers, they don't use --rm flag and can linger after crashes.
33+
// No grace period is applied because test containers should be ephemeral and
34+
// are only created/destroyed within a single test run.
35+
testContainerPrefix = "lvt-test-"
36+
staleContainerGrace = 1 * time.Minute // Reduced from 10 min for faster cleanup
3137
)
3238

3339
func removeContainersByFilter(filter string, shouldRemove func(string) (bool, error)) ([]string, error) {
@@ -110,6 +116,26 @@ func CleanupChromeContainers() {
110116
}
111117
}
112118

119+
// CleanupTestContainers removes any lingering test app containers (lvt-test-*).
120+
// These containers are created by runDockerContainer in e2e tests for integration testing.
121+
// Unlike Chrome containers which use --rm flag, these can linger if tests crash or are interrupted.
122+
func CleanupTestContainers() {
123+
// Remove all containers matching the prefix, regardless of state
124+
// We don't apply grace period for test containers as they should be ephemeral
125+
if removed, err := removeContainersByFilter(testContainerPrefix, nil); err != nil {
126+
fmt.Fprintf(os.Stderr, "warning: failed to clean test containers: %v\n", err)
127+
} else if len(removed) > 0 {
128+
fmt.Fprintf(os.Stderr, "Cleaned up %d stale test container(s): %s\n", len(removed), strings.Join(removed, ", "))
129+
}
130+
}
131+
132+
// CleanupAllTestContainers removes both Chrome containers and test app containers.
133+
// This is a convenience function to run all cleanup operations before tests start.
134+
func CleanupAllTestContainers() {
135+
CleanupChromeContainers()
136+
CleanupTestContainers()
137+
}
138+
113139
// GetFreePort asks the kernel for a free open port that is ready to use
114140
func GetFreePort() (port int, err error) {
115141
var a *net.TCPAddr

0 commit comments

Comments
 (0)