From 91ee307c8f92eacbbdbba60ffee83db919136030 Mon Sep 17 00:00:00 2001 From: Ryan Moran Date: Sat, 13 Dec 2025 14:50:46 -0800 Subject: [PATCH 1/2] Fixes cleanup bug in git server --- internal/git/server.go | 9 ++------- main.go | 16 +++++++--------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/internal/git/server.go b/internal/git/server.go index d678c6a..3d0c643 100644 --- a/internal/git/server.go +++ b/internal/git/server.go @@ -107,13 +107,8 @@ func (s Server) Port() int { return s.port } -// Close stops the Git HTTP server and closes the TCP listener. +// Close stops the Git HTTP server which closes the TCP listener. // Returns an error if the server or listener cannot be closed cleanly. func (s Server) Close() error { - err := s.server.Close() - if err != nil { - return err - } - - return s.listener.Close() + return s.server.Close() } diff --git a/main.go b/main.go index 7e254c9..45ef036 100644 --- a/main.go +++ b/main.go @@ -27,16 +27,14 @@ func main() { } func run(args, env []string) error { - cleanupMgr := internal.NewCleanupManager() - defer cleanupMgr.Execute() + cleanup := internal.NewCleanupManager() + defer cleanup.Execute() config := internal.ParseConfig(args[1:], env) - // Create context with cancellation for proper goroutine cleanup ctx, cancel := context.WithCancel(context.Background()) - defer cancel() + cleanup.Add("cancel-context", func() error { cancel(); return nil }) - // Handle signals to cancel context and cleanup sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) go func() { @@ -57,13 +55,13 @@ func run(args, env []string) error { if err != nil { return fmt.Errorf("failed to start git server in directory %q: %w", workingDirectory, err) } - cleanupMgr.Add("git-server", remote.Close) + cleanup.Add("git-server", remote.Close) client, err := docker.NewDefaultClient() if err != nil { return fmt.Errorf("failed to create docker client: %w\nMake sure Docker is installed and running (try 'docker ps')", err) } - cleanupMgr.Add("docker-client", func() error { + cleanup.Add("docker-client", func() error { client.Close() return nil }) @@ -89,7 +87,7 @@ func run(args, env []string) error { if err != nil { return fmt.Errorf("failed to create container %q from image %q: %w", session.ID(), image.Name, err) } - cleanupMgr.Add("container", func() error { + cleanup.Add("container", func() error { return container.ForceRemove(ctx) }) @@ -104,7 +102,7 @@ func run(args, env []string) error { if err != nil { return fmt.Errorf("failed to create git archive from %q on branch %q: %w", workingDirectory, session.Branch(), err) } - cleanupMgr.Add("archive", archive.Close) + cleanup.Add("archive", archive.Close) err = container.CopyTo(ctx, archive, "/") if err != nil { From e938b9131b72edcfe1991a1d9f11e5befa2b827c Mon Sep 17 00:00:00 2001 From: Ryan Moran Date: Sun, 21 Dec 2025 15:26:18 -0800 Subject: [PATCH 2/2] Adds some better integration testing --- integration/main_test.go | 59 +++++++++ integration/some_test.go | 275 +++++++++++++++++++++++++++++++++++++++ integration_test.go | 245 ---------------------------------- main_test.go | 14 -- 4 files changed, 334 insertions(+), 259 deletions(-) create mode 100644 integration/main_test.go create mode 100644 integration/some_test.go delete mode 100644 integration_test.go delete mode 100644 main_test.go diff --git a/integration/main_test.go b/integration/main_test.go new file mode 100644 index 0000000..01c12bf --- /dev/null +++ b/integration/main_test.go @@ -0,0 +1,59 @@ +package integration_test + +import ( + "log" + "os" + "os/exec" + "testing" +) + +var settings struct { + Path string +} + +func TestMain(m *testing.M) { + err := BeforeSuite() + if err != nil { + log.Fatal(err) + } + + code := m.Run() + + err = AfterSuite() + if err != nil { + log.Fatal(err) + } + + os.Exit(code) +} + +func BeforeSuite() error { + file, err := os.CreateTemp("", "contagent-*") + if err != nil { + return err + } + + err = file.Close() + if err != nil { + return err + } + + settings.Path = file.Name() + + cmd := exec.Command("go", "build", "-o", settings.Path, "../.") + err = cmd.Run() + if err != nil { + return err + } + + return nil +} + +func AfterSuite() error { + err := os.RemoveAll(settings.Path) + if err != nil { + return err + } + + return nil +} diff --git a/integration/some_test.go b/integration/some_test.go new file mode 100644 index 0000000..349cefe --- /dev/null +++ b/integration/some_test.go @@ -0,0 +1,275 @@ +package integration_test + +import ( + "fmt" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/moby/moby/client" + "github.com/stretchr/testify/require" +) + +// TestWorkflow validates the complete end-to-end workflow: +// 1. Git HTTP server starts and serves repository +// 2. Docker image builds successfully +// 3. Container is created with proper configuration +// 4. Git archive is copied into container +// 5. Container executes command and exits +// 6. Cleanup removes all resources +func TestWorkflow(t *testing.T) { + type Identifiers struct { + RepositoryPath string + Dockerfile string + } + + setup := func(t *testing.T) Identifiers { + dir, err := os.MkdirTemp("", "repository-*") + require.NoError(t, err) + + file, err := os.CreateTemp(dir, "Dockerfile.*") + require.NoError(t, err) + defer file.Close() + + fmt.Fprintln(file, "FROM ubuntu:25.10") + + cmd := exec.Command("git", "init") + cmd.Dir = dir + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + cmd = exec.Command("git", "add", "-A", ".") + cmd.Dir = dir + output, err = cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + cmd = exec.Command("git", "commit", "-m", "Initial commit") + cmd.Dir = dir + output, err = cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + t.Cleanup(func() { + err := os.RemoveAll(file.Name()) + require.NoError(t, err) + + err = os.RemoveAll(dir) + require.NoError(t, err) + }) + + return Identifiers{ + RepositoryPath: dir, + Dockerfile: file.Name(), + } + } + + t.Run("runs a simple echo command", func(t *testing.T) { + identifiers := setup(t) + + cmd := exec.Command(settings.Path, + "--dockerfile", identifiers.Dockerfile, + "bash", "-c", "echo integration_test") + cmd.Dir = identifiers.RepositoryPath + cmd.Env = append(os.Environ(), + "TERM=xterm-256color", + "COLORTERM=truecolor", + "ANTHROPIC_API_KEY=", + ) + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + // TODO: assert on output + }) + + t.Run("container has access to git repository", func(t *testing.T) { + identifiers := setup(t) + + cmd := exec.Command(settings.Path, + "--dockerfile", identifiers.Dockerfile, + "bash", "-c", "cd /app && git rev-parse --is-inside-work-tree") + cmd.Dir = identifiers.RepositoryPath + cmd.Env = append(os.Environ(), + "TERM=xterm-256color", + "COLORTERM=truecolor", + "ANTHROPIC_API_KEY=", + ) + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + // TODO: assert on output + }) + + t.Run("container can access working directory", func(t *testing.T) { + identifiers := setup(t) + + cmd := exec.Command(settings.Path, + "--dockerfile", identifiers.Dockerfile, + "bash", "-c", "pwd | grep -q /app") + cmd.Dir = identifiers.RepositoryPath + cmd.Env = append(os.Environ(), + "TERM=xterm-256color", + "COLORTERM=truecolor", + "ANTHROPIC_API_KEY=", + ) + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + // TODO: assert on output + }) + + t.Run("environment variables are passed through", func(t *testing.T) { + identifiers := setup(t) + + cmd := exec.Command(settings.Path, + "--dockerfile", identifiers.Dockerfile, + "bash", "-c", "test \"$TERM\" = 'screen-256color'") + cmd.Dir = identifiers.RepositoryPath + cmd.Env = append(os.Environ(), + "TERM=screen-256color", + "COLORTERM=truecolor", + "ANTHROPIC_API_KEY=test-key-123", + ) + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + // TODO: assert on output + }) + + t.Run("container can execute commands with non-zero exit", func(t *testing.T) { + identifiers := setup(t) + + cmd := exec.Command(settings.Path, + "--dockerfile", identifiers.Dockerfile, + "bash", "-c", "exit 42") + cmd.Dir = identifiers.RepositoryPath + cmd.Env = append(os.Environ(), + "TERM=xterm-256color", + "COLORTERM=truecolor", + "ANTHROPIC_API_KEY=", + ) + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + // TODO: assert on output + }) + + t.Run("with volumes", func(t *testing.T) { + identifiers := setup(t) + + tmpDir, err := os.MkdirTemp("", "contagent-volume-*") + require.NoError(t, err) + + t.Cleanup(func() { + err := os.RemoveAll(tmpDir) + require.NoError(t, err) + }) + + testFile := filepath.Join(tmpDir, "test.txt") + testContent := "integration test content" + err = os.WriteFile(testFile, []byte(testContent), 0644) + require.NoError(t, err) + + cmd := exec.Command(settings.Path, + "--dockerfile", identifiers.Dockerfile, + "--volume", fmt.Sprintf("%s:/mnt/test", tmpDir), + "bash", "-c", "cat /mnt/test/test.txt | grep -q 'integration test'", + ) + cmd.Dir = identifiers.RepositoryPath + cmd.Env = append(os.Environ(), + "TERM=xterm-256color", + "COLORTERM=truecolor", + "ANTHROPIC_API_KEY=", + ) + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + // TODO: assert on output + }) + + t.Run("with env vars", func(t *testing.T) { + identifiers := setup(t) + + cmd := exec.Command(settings.Path, + "--dockerfile", identifiers.Dockerfile, + "--env", "CUSTOM_VAR=test123", + "bash", "-c", "test \"$CUSTOM_VAR\" = 'test123'", + ) + cmd.Dir = identifiers.RepositoryPath + cmd.Env = append(os.Environ(), + "TERM=xterm-256color", + "COLORTERM=truecolor", + "ANTHROPIC_API_KEY=", + ) + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + // TODO: assert on output + }) + + // TestWorkflowCleanup verifies that cleanup happens and containers are removed + t.Run("cleanup", func(t *testing.T) { + identifiers := setup(t) + + cli, err := client.New(client.FromEnv, client.WithAPIVersionNegotiation()) + require.NoError(t, err) + + t.Cleanup(func() { + cli.Close() + }) + + ctx := t.Context() + + cmd := exec.Command(settings.Path, + "--dockerfile", identifiers.Dockerfile, + "bash", "-c", "exit 42") + cmd.Dir = identifiers.RepositoryPath + cmd.Env = append(os.Environ(), + "TERM=xterm-256color", + "COLORTERM=truecolor", + "ANTHROPIC_API_KEY=", + ) + output, err := cmd.CombinedOutput() + require.NoError(t, err, string(output)) + + // Give cleanup a moment to complete + time.Sleep(500 * time.Millisecond) + + result, err := cli.ContainerList(ctx, client.ContainerListOptions{}) + require.NoError(t, err) + + for _, item := range result.Items { + for _, name := range item.Names { + require.NotContains(t, name, "contagent") + } + } + }) + + t.Run("failure cases", func(t *testing.T) { + t.Run("when not in a git repository", func(t *testing.T) { + identifiers := setup(t) + + tmpDir, err := os.MkdirTemp("", "contagent-test-*") + require.NoError(t, err) + + t.Cleanup(func() { + err := os.RemoveAll(tmpDir) + require.NoError(t, err) + }) + + cmd := exec.Command(settings.Path, + "--dockerfile", identifiers.Dockerfile, + "bash", "-c", "echo test", + ) + cmd.Dir = tmpDir + cmd.Env = append(os.Environ(), + "TERM=xterm-256color", + "COLORTERM=truecolor", + "ANTHROPIC_API_KEY=", + ) + output, err := cmd.CombinedOutput() + require.ErrorContains(t, err, "exit status 1") + require.Contains(t, string(output), "not a git repository") + }) + }) +} diff --git a/integration_test.go b/integration_test.go deleted file mode 100644 index 07a460f..0000000 --- a/integration_test.go +++ /dev/null @@ -1,245 +0,0 @@ -//go:build integration -// +build integration - -package main - -import ( - "context" - "fmt" - "os" - "path/filepath" - "testing" - "time" - - "github.com/ryanmoran/contagent/internal/docker" - "github.com/stretchr/testify/require" -) - -// TestFullWorkflow validates the complete end-to-end workflow: -// 1. Git HTTP server starts and serves repository -// 2. Docker image builds successfully -// 3. Container is created with proper configuration -// 4. Git archive is copied into container -// 5. Container executes command and exits -// 6. Cleanup removes all resources -func TestFullWorkflow(t *testing.T) { - // Skip if Docker is not available - if os.Getenv("SKIP_INTEGRATION") == "true" { - t.Skip("Integration tests skipped") - } - - // Verify Docker daemon is running - client, err := docker.NewDefaultClient() - require.NoError(t, err, "Docker daemon must be running for integration tests") - defer client.Close() - - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) - defer cancel() - - // Verify we can connect to Docker - _, err = client.Ping(ctx) - require.NoError(t, err, "Failed to ping Docker daemon") - - t.Run("successful workflow with echo command", func(t *testing.T) { - // Set up environment to pass through - env := []string{ - "TERM=xterm-256color", - "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=", - } - - // Test with a simple echo command - err := run([]string{"contagent", "bash", "-c", "echo 'integration test'"}, env) - require.NoError(t, err) - }) - - t.Run("container has access to git repository", func(t *testing.T) { - env := []string{ - "TERM=xterm-256color", - "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=", - } - - // Verify container has git repository with correct branch - err := run([]string{"contagent", "bash", "-c", "cd /app && git rev-parse --is-inside-work-tree"}, env) - require.NoError(t, err) - }) - - t.Run("container can access working directory", func(t *testing.T) { - env := []string{ - "TERM=xterm-256color", - "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=", - } - - // Verify container starts in /app directory - err := run([]string{"contagent", "bash", "-c", "pwd | grep -q /app"}, env) - require.NoError(t, err) - }) - - t.Run("environment variables are passed through", func(t *testing.T) { - env := []string{ - "TERM=screen-256color", - "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=test-key-123", - } - - // Verify environment variables are set correctly - err := run([]string{"contagent", "bash", "-c", "test \"$TERM\" = 'screen-256color'"}, env) - require.NoError(t, err) - }) - - t.Run("container can execute commands with non-zero exit", func(t *testing.T) { - env := []string{ - "TERM=xterm-256color", - "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=", - } - - // Verify non-zero exit codes don't cause run() to error - // (exit status is logged but not returned as error) - err := run([]string{"contagent", "bash", "-c", "exit 42"}, env) - require.NoError(t, err) - }) -} - -// TestWorkflowWithDockerNotRunning tests error handling when Docker daemon is unavailable -func TestWorkflowWithDockerNotRunning(t *testing.T) { - if os.Getenv("SKIP_INTEGRATION") == "true" { - t.Skip("Integration tests skipped") - } - - // This test verifies that the error message is helpful when Docker is not available - // In practice, this is hard to test without stopping Docker, so we skip it - // unless specifically requested - if os.Getenv("TEST_DOCKER_UNAVAILABLE") != "true" { - t.Skip("Skipping Docker unavailability test (requires Docker to be stopped)") - } - - env := []string{ - "TERM=xterm-256color", - "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=", - } - - err := run([]string{"contagent", "bash", "-c", "echo test"}, env) - require.Error(t, err) - require.Contains(t, err.Error(), "docker") -} - -// TestWorkflowWithInvalidGitRepo tests error handling when not in a git repository -func TestWorkflowWithInvalidGitRepo(t *testing.T) { - if os.Getenv("SKIP_INTEGRATION") == "true" { - t.Skip("Integration tests skipped") - } - - // Create a temporary directory that is not a git repository - tmpDir, err := os.MkdirTemp("", "contagent-test-*") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) - - // Change to temp directory - originalDir, err := os.Getwd() - require.NoError(t, err) - defer os.Chdir(originalDir) - - err = os.Chdir(tmpDir) - require.NoError(t, err) - - // Copy Dockerfile to temp directory so build can work - dockerfilePath := filepath.Join(originalDir, "Dockerfile") - dockerfileContent, err := os.ReadFile(dockerfilePath) - require.NoError(t, err) - err = os.WriteFile(filepath.Join(tmpDir, "Dockerfile"), dockerfileContent, 0644) - require.NoError(t, err) - - env := []string{ - "TERM=xterm-256color", - "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=", - } - - // Should fail because we're not in a git repository - err = run([]string{"contagent", "bash", "-c", "echo test"}, env) - require.Error(t, err) - require.Contains(t, err.Error(), "git") -} - -// TestWorkflowCleanup verifies that cleanup happens and containers are removed -func TestWorkflowCleanup(t *testing.T) { - if os.Getenv("SKIP_INTEGRATION") == "true" { - t.Skip("Integration tests skipped") - } - - client, err := docker.NewDefaultClient() - require.NoError(t, err) - defer client.Close() - - ctx := context.Background() - - env := []string{ - "TERM=xterm-256color", - "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=", - } - - // Run a command - err = run([]string{"contagent", "bash", "-c", "echo cleanup test"}, env) - require.NoError(t, err) - - // Give cleanup a moment to complete - time.Sleep(500 * time.Millisecond) - - // Verify no contagent-* containers are left behind - containers, err := client.ListContainers(ctx) - require.NoError(t, err) - - for _, containerID := range containers { - t.Logf("Found container: %s (should not be a contagent container)", containerID) - } -} - -// TestWorkflowWithVolumes tests volume mounting -func TestWorkflowWithVolumes(t *testing.T) { - if os.Getenv("SKIP_INTEGRATION") == "true" { - t.Skip("Integration tests skipped") - } - - // Create a temporary directory with a test file - tmpDir, err := os.MkdirTemp("", "contagent-volume-*") - require.NoError(t, err) - defer os.RemoveAll(tmpDir) - - testFile := filepath.Join(tmpDir, "test.txt") - testContent := "integration test content" - err = os.WriteFile(testFile, []byte(testContent), 0644) - require.NoError(t, err) - - env := []string{ - "TERM=xterm-256color", - "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=", - } - - // Test with volume mount - volumeMount := fmt.Sprintf("%s:/mnt/test", tmpDir) - err = run([]string{"contagent", "-volume", volumeMount, "bash", "-c", "cat /mnt/test/test.txt | grep -q 'integration test'"}, env) - require.NoError(t, err) -} - -// TestWorkflowWithCustomEnv tests custom environment variables -func TestWorkflowWithCustomEnv(t *testing.T) { - if os.Getenv("SKIP_INTEGRATION") == "true" { - t.Skip("Integration tests skipped") - } - - env := []string{ - "TERM=xterm-256color", - "COLORTERM=truecolor", - "ANTHROPIC_API_KEY=", - } - - // Test with custom environment variable - err := run([]string{"contagent", "-env", "CUSTOM_VAR=test123", "bash", "-c", "test \"$CUSTOM_VAR\" = 'test123'"}, env) - require.NoError(t, err) -} diff --git a/main_test.go b/main_test.go deleted file mode 100644 index 1f615d6..0000000 --- a/main_test.go +++ /dev/null @@ -1,14 +0,0 @@ -package main - -import ( - "testing" - - "github.com/stretchr/testify/require" -) - -func TestRun(t *testing.T) { - t.Run("returns error when no arguments provided", func(t *testing.T) { - err := run([]string{"contagent", "bash", "-c", "sleep 1 && env"}, nil) - require.NoError(t, err) - }) -}