Using GH actions provided Chrome for test speedup (#149)
OMG. This had wild behavior. Sometimes you'd log into tmate, and it
would work. Something about running Chrome manually would tickle some
dbus thing into working. The error would sometimes show up, but most
often it would be swallowed.
The punchline? We had a context timeout of 5 or 10 seconds in the tests
that would trigger silently, and we'd see nothing. Everything else
(including maybe the dbus stuff) was a red herring: the test executor
machine is swamped, and shit's slow, and the various timeouts needed
tweaking. I bet there are a few more timeouts to loosen eventually, but
it seems to be passing now.
Error message:
failed to start browser (please apt get chromium or equivalent): chrome failed to start:
diff --git a/.github/workflows/go_test.yml b/.github/workflows/go_test.yml
index e10dbb3..95049cd 100644
--- a/.github/workflows/go_test.yml
+++ b/.github/workflows/go_test.yml
@@ -44,9 +44,20 @@
go install mvdan.cc/gofumpt@latest
go install golang.org/x/tools/cmd/goimports@latest
# Empirically (by logging into runners with tmate), there are versions of Chromium/Chrome in /opt/google/chrome
- # and /usr/local/share/chromium/chrome-linux. However, adding them to the PATH
- # hasn't worked yet. I don't know why.
- sudo apt-get update && sudo apt-get -y install chromium
+ # and /usr/local/share/chromium/chrome-linux, and /usr/bin/chromium links to the former. Therefore, we
+ # don't need to apt-get it separately.
+
+ # If you wish to do some interactive debugging, the following is handy. It will print
+ # an SSH command that you can use to connect to the runner instance.
+ - name: tmate debugging (disabled)
+ if: false
+ run: |
+ sudo apt-get update && sudo apt-get install -y tmate
+ set -x
+ tmate -S /tmp/tmate.sock.${GITHUB_RUN_ID} new-session -d
+ tmate -S /tmp/tmate.sock.${GITHUB_RUN_ID} wait tmate-ready
+ tmate -S /tmp/tmate.sock.${GITHUB_RUN_ID} display -p '#{tmate_ssh}'
+ sleep 1800
- name: Go generate
run: |
diff --git a/claudetool/browse/browse.go b/claudetool/browse/browse.go
index 9ab1ae3..bebf88c 100644
--- a/claudetool/browse/browse.go
+++ b/claudetool/browse/browse.go
@@ -75,10 +75,15 @@
// when running in a container, even when we aren't root (which is largely
// the case for tests).
opts = append(opts, chromedp.NoSandbox)
+ // Setting 'DBUS_SESSION_BUS_ADDRESS=""' or this flag allows tests to pass
+ // in GitHub runner contexts. It's a mystery why the failure isn't clear when this fails.
+ opts = append(opts, chromedp.Flag("--disable-dbus", true))
+ // This can be pretty slow in tests
+ opts = append(opts, chromedp.WSURLReadTimeout(30*time.Second))
allocCtx, _ := chromedp.NewExecAllocator(b.ctx, opts...)
browserCtx, browserCancel := chromedp.NewContext(
allocCtx,
- chromedp.WithLogf(log.Printf),
+ chromedp.WithLogf(log.Printf), chromedp.WithErrorf(log.Printf), chromedp.WithBrowserOption(chromedp.WithDialTimeout(30*time.Second)),
)
b.browserCtx = browserCtx
diff --git a/claudetool/browse/browse_test.go b/claudetool/browse/browse_test.go
index 6a0a31d..d4c6b1e 100644
--- a/claudetool/browse/browse_test.go
+++ b/claudetool/browse/browse_test.go
@@ -18,6 +18,9 @@
func TestToolCreation(t *testing.T) {
// Create browser tools instance
tools := NewBrowseTools(context.Background())
+ t.Cleanup(func() {
+ tools.Close()
+ })
// Test each tool has correct name and description
toolTests := []struct {
@@ -68,6 +71,9 @@
func TestGetTools(t *testing.T) {
// Create browser tools instance
tools := NewBrowseTools(context.Background())
+ t.Cleanup(func() {
+ tools.Close()
+ })
// Test with screenshot tools included
t.Run("with screenshots", func(t *testing.T) {
@@ -102,10 +108,13 @@
}
// Create browser tools instance
- ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
tools := NewBrowseTools(ctx)
+ t.Cleanup(func() {
+ tools.Close()
+ })
// Initialize the browser
err := tools.Initialize()
@@ -118,9 +127,6 @@
}
}
- // Clean up
- defer tools.Close()
-
// Get browser context to verify it's working
browserCtx, err := tools.GetBrowserContext()
if err != nil {
@@ -148,11 +154,13 @@
}
// Create browser tools instance
- ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
tools := NewBrowseTools(ctx)
- defer tools.Close()
+ t.Cleanup(func() {
+ tools.Close()
+ })
// Check if browser initialization works
if err := tools.Initialize(); err != nil {
@@ -222,6 +230,9 @@
// Create browser tools instance
ctx := context.Background()
tools := NewBrowseTools(ctx)
+ t.Cleanup(func() {
+ tools.Close()
+ })
// Test SaveScreenshot function directly
testData := []byte("test image data")
@@ -256,6 +267,9 @@
// Create a test BrowseTools instance
ctx := context.Background()
browseTools := NewBrowseTools(ctx)
+ t.Cleanup(func() {
+ browseTools.Close()
+ })
// Create a test image
testDir := t.TempDir()
@@ -308,7 +322,7 @@
// TestDefaultViewportSize verifies that the browser starts with the correct default viewport size
func TestDefaultViewportSize(t *testing.T) {
- ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
// Skip if CI or headless testing environment
@@ -317,7 +331,9 @@
}
tools := NewBrowseTools(ctx)
- defer tools.Close()
+ t.Cleanup(func() {
+ tools.Close()
+ })
// Initialize browser (which should set default viewport to 1280x720)
err := tools.Initialize()
@@ -375,7 +391,7 @@
// TestResizeTool tests the browser resize functionality
func TestResizeTool(t *testing.T) {
- ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+ ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
// Skip if CI or headless testing environment
@@ -385,7 +401,9 @@
t.Run("ResizeWindow", func(t *testing.T) {
tools := NewBrowseTools(ctx)
- defer tools.Close()
+ t.Cleanup(func() {
+ tools.Close()
+ })
// Resize to mobile dimensions
resizeTool := tools.NewResizeTool()