From 0f93307449f42a77da7535d70a01463687e67f86 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 30 Sep 2020 15:24:28 +0200 Subject: [PATCH 1/8] Add first high-level integration test for Executor --- internal/campaigns/executor_test.go | 160 ++++++++++++++++++ internal/campaigns/graphql/repository.go | 10 +- internal/campaigns/run_steps.go | 2 +- .../campaigns/testdata/dummydocker/docker | 53 ++++++ 4 files changed, 220 insertions(+), 5 deletions(-) create mode 100644 internal/campaigns/executor_test.go create mode 100755 internal/campaigns/testdata/dummydocker/docker diff --git a/internal/campaigns/executor_test.go b/internal/campaigns/executor_test.go new file mode 100644 index 0000000000..8c540a3a60 --- /dev/null +++ b/internal/campaigns/executor_test.go @@ -0,0 +1,160 @@ +package campaigns + +import ( + "archive/zip" + "bytes" + "context" + "fmt" + "io/ioutil" + "log" + "mime" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "testing" + "time" + + "github.com/sourcegraph/go-diff/diff" + "github.com/sourcegraph/src-cli/internal/api" + "github.com/sourcegraph/src-cli/internal/campaigns/graphql" +) + +func TestExecutor_Integration(t *testing.T) { + addToPath(t, "testdata/dummydocker") + + repo := &graphql.Repository{ + Name: "github.com/sourcegraph/src-cli", + DefaultBranch: &graphql.Branch{ + Name: "main", + Target: struct{ OID string }{OID: "d34db33f"}, + }, + } + + filesInRepo := map[string]string{ + "README.md": "# Welcome to the README\n", + "main.go": "package main\n\nfunc main() {\n\tfmt.Println( \"Hello World\")\n}\n", + } + + steps := []Step{ + {Run: `echo -e "foobar\n" >> README.md`, Container: "doesntmatter:13"}, + {Run: `go fmt main.go`, Container: "doesntmatter:13"}, + } + + h := newZipHandler(t, repo.Name, repo.DefaultBranch.Name, filesInRepo) + ts := httptest.NewServer(h) + defer ts.Close() + + var clientBuffer bytes.Buffer + client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + + testTempDir, err := ioutil.TempDir("", "executor-integration-test-*") + if err != nil { + t.Fatal(err) + } + + creator := &WorkspaceCreator{dir: testTempDir, client: client} + opts := ExecutorOpts{ + Cache: &ExecutionNoOpCache{}, + Creator: creator, + TempDir: testTempDir, + Parallelism: runtime.GOMAXPROCS(0), + Timeout: 1 * time.Second, + } + + called := false + updateFn := func(task *Task, ts TaskStatus) { called = true } + + executor := newExecutor(opts, client, updateFn) + + template := &ChangesetTemplate{} + executor.AddTask(repo, steps, template) + + executor.Start(context.Background()) + specs, err := executor.Wait() + if err != nil { + t.Fatal(err) + } + + if !called { + t.Fatalf("update was not called") + } + + if have, want := len(specs), 1; have != want { + t.Fatalf("wrong number of specs. want=%d, have=%d", want, have) + } + + if have, want := len(specs[0].Commits), 1; have != want { + t.Fatalf("wrong number of commits. want=%d, have=%d", want, have) + } + + fileDiffs, err := diff.ParseMultiFileDiff([]byte(specs[0].Commits[0].Diff)) + if err != nil { + t.Fatalf("failed to parse diff: %s", err) + } + + diffsByName := map[string]*diff.FileDiff{} + for _, fd := range fileDiffs { + diffsByName[fd.OrigName] = fd + } + + if have, want := len(diffsByName), 2; have != want { + t.Fatalf("wrong number of diffsByName. want=%d, have=%d", want, have) + } + + if _, ok := diffsByName["main.go"]; !ok { + t.Errorf("main.go was not changed") + } + if _, ok := diffsByName["README.md"]; !ok { + t.Errorf("README.md was not changed") + } +} + +func addToPath(t *testing.T, relPath string) { + t.Helper() + + oldPath := os.Getenv("PATH") + dummyDockerPath, err := filepath.Abs("testdata/dummydocker") + if err != nil { + t.Fatal(err) + } + os.Setenv("PATH", dummyDockerPath+":"+oldPath) +} + +func newZipHandler(t *testing.T, repo, branch string, files map[string]string) http.HandlerFunc { + wantPath := fmt.Sprintf("/%s@%s/-/raw", repo, branch) + + downloadName := filepath.Base(repo) + mediaType := mime.FormatMediaType("Attachment", map[string]string{ + "filename": downloadName, + }) + + return func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != wantPath { + t.Errorf("request has wrong path. want=%q, have=%q", wantPath, r.URL.Path) + w.WriteHeader(http.StatusNotFound) + return + } + + w.Header().Set("X-Content-Type-Options", "nosniff") + w.Header().Set("Content-Type", "application/zip") + w.Header().Set("Content-Disposition", mediaType) + + zipWriter := zip.NewWriter(w) + + for name, body := range files { + f, err := zipWriter.Create(name) + if err != nil { + log.Fatal(err) + } + if _, err := f.Write([]byte(body)); err != nil { + t.Errorf("failed to write body for %s to zip: %s", name, err) + } + } + + if err := zipWriter.Close(); err != nil { + t.Fatalf("closing zipWriter failed: %s", err) + } + } +} diff --git a/internal/campaigns/graphql/repository.go b/internal/campaigns/graphql/repository.go index 981c9191f6..584f3f0e66 100644 --- a/internal/campaigns/graphql/repository.go +++ b/internal/campaigns/graphql/repository.go @@ -19,15 +19,17 @@ fragment repositoryFields on Repository { } ` +type Branch struct { + Name string + Target struct{ OID string } +} + type Repository struct { ID string Name string URL string ExternalRepository struct{ ServiceType string } - DefaultBranch *struct { - Name string - Target struct{ OID string } - } + DefaultBranch *Branch } func (r *Repository) BaseRef() string { diff --git a/internal/campaigns/run_steps.go b/internal/campaigns/run_steps.go index 432c5aed4d..351c0bcb97 100644 --- a/internal/campaigns/run_steps.go +++ b/internal/campaigns/run_steps.go @@ -135,8 +135,8 @@ func runSteps(ctx context.Context, wc *WorkspaceCreator, repo *graphql.Repositor Stderr: strings.TrimSpace(stderrBuffer.String()), } } - logger.Logf("[Step %d] complete in %s", i+1, elapsed) + logger.Logf("[Step %d] complete in %s", i+1, elapsed) } if _, err := runGitCmd("add", "--all"); err != nil { diff --git a/internal/campaigns/testdata/dummydocker/docker b/internal/campaigns/testdata/dummydocker/docker new file mode 100755 index 0000000000..de7c71a07b --- /dev/null +++ b/internal/campaigns/testdata/dummydocker/docker @@ -0,0 +1,53 @@ +#!/bin/bash + +# This script is used by the executor integration test to simulate Docker. +# It gets put into $PATH as "docker" and accepts the "run" command. + +# Depending on the arguments to the "run" command it either acts like it +# created a tempfile, or it executes the script supplied as the last arg to +# "run". + +dummy_temp_file="DUMMYDOCKER-TEMP-FILE" + +if [[ "${1}" == "run" ]]; then + last_arg="${@: -1}" + + case "$last_arg" in + "mktemp") + # If the last arg is "mktemp" we're probing for a shell image and + # want to create a temp file in the container which we can put a script. + echo "${dummy_temp_file}" + exit 0 + ;; + + "${dummy_temp_file}") + # If the last arg is the temp file we "created" earlier, we now want to + # execute it. + # + # But the real script is in the matching host temp file, which should + # be mounted into the temp file inside the container. + # + # We need to find it in the args and then execute it. + + host_temp_file="" + for i in "$@"; + do + if [[ ${i} =~ ^type=bind,source=(.*),target=${dummy_temp_file},ro$ ]]; then + host_temp_file="${BASH_REMATCH[1]}" + fi + done + [ -z "$host_temp_file" ] && echo "host temp file not found in args" && exit 1; + + # Now that we have the path to the host temp file, we can execute it + source ${host_temp_file} + exit 0 + ;; + *) + echo "dummydocker doesn't know about this command: $last_arg" + exit 1 + ;; + esac +fi + +echo "dummydocker doesn't know about this command: $1" +exit 1 From fb30e07072f84dfd4c329fac887bfed3e7e2f225 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 30 Sep 2020 16:21:21 +0200 Subject: [PATCH 2/8] Exec bash process in dummydocker --- internal/campaigns/testdata/dummydocker/docker | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/campaigns/testdata/dummydocker/docker b/internal/campaigns/testdata/dummydocker/docker index de7c71a07b..08f7ebf445 100755 --- a/internal/campaigns/testdata/dummydocker/docker +++ b/internal/campaigns/testdata/dummydocker/docker @@ -39,8 +39,7 @@ if [[ "${1}" == "run" ]]; then [ -z "$host_temp_file" ] && echo "host temp file not found in args" && exit 1; # Now that we have the path to the host temp file, we can execute it - source ${host_temp_file} - exit 0 + exec bash ${host_temp_file} ;; *) echo "dummydocker doesn't know about this command: $last_arg" From 206aad2559f3e980be5c690c34983019651dbfa4 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 30 Sep 2020 16:21:49 +0200 Subject: [PATCH 3/8] Bump executor timeout to 5s in integration test --- internal/campaigns/executor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/campaigns/executor_test.go b/internal/campaigns/executor_test.go index 8c540a3a60..f24185b9f8 100644 --- a/internal/campaigns/executor_test.go +++ b/internal/campaigns/executor_test.go @@ -60,7 +60,7 @@ func TestExecutor_Integration(t *testing.T) { Creator: creator, TempDir: testTempDir, Parallelism: runtime.GOMAXPROCS(0), - Timeout: 1 * time.Second, + Timeout: 5 * time.Second, } called := false From b3dc44e7083c7bee3fc08fdbe0f632e74ca0fd55 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 30 Sep 2020 16:23:42 +0200 Subject: [PATCH 4/8] add docker.exec symlink for windows --- internal/campaigns/testdata/dummydocker/docker.exe | 1 + 1 file changed, 1 insertion(+) create mode 120000 internal/campaigns/testdata/dummydocker/docker.exe diff --git a/internal/campaigns/testdata/dummydocker/docker.exe b/internal/campaigns/testdata/dummydocker/docker.exe new file mode 120000 index 0000000000..12801ef6a5 --- /dev/null +++ b/internal/campaigns/testdata/dummydocker/docker.exe @@ -0,0 +1 @@ +testdata/dummydocker/docker \ No newline at end of file From 4cb89ec4074a7e534c1154a504699c5381639d63 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 30 Sep 2020 16:40:59 +0200 Subject: [PATCH 5/8] Use OS-specific path separator when setting PATH --- internal/campaigns/executor_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/campaigns/executor_test.go b/internal/campaigns/executor_test.go index f24185b9f8..369b1fcd2e 100644 --- a/internal/campaigns/executor_test.go +++ b/internal/campaigns/executor_test.go @@ -38,7 +38,7 @@ func TestExecutor_Integration(t *testing.T) { } steps := []Step{ - {Run: `echo -e "foobar\n" >> README.md`, Container: "doesntmatter:13"}, + {Run: `echo -e "foobar\n" >> README.md`, Container: "alpine:13"}, {Run: `go fmt main.go`, Container: "doesntmatter:13"}, } @@ -114,12 +114,11 @@ func TestExecutor_Integration(t *testing.T) { func addToPath(t *testing.T, relPath string) { t.Helper() - oldPath := os.Getenv("PATH") dummyDockerPath, err := filepath.Abs("testdata/dummydocker") if err != nil { t.Fatal(err) } - os.Setenv("PATH", dummyDockerPath+":"+oldPath) + os.Setenv("PATH", fmt.Sprintf("%s%c%s", dummyDockerPath, os.PathListSeparator, os.Getenv("PATH"))) } func newZipHandler(t *testing.T, repo, branch string, files map[string]string) http.HandlerFunc { From 4524a188fc22ecf5ae455f02cbc0f2e82d95bc9d Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 30 Sep 2020 16:43:23 +0200 Subject: [PATCH 6/8] .exe to .bat --- internal/campaigns/testdata/dummydocker/docker.bat | 1 + internal/campaigns/testdata/dummydocker/docker.exe | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 internal/campaigns/testdata/dummydocker/docker.bat delete mode 120000 internal/campaigns/testdata/dummydocker/docker.exe diff --git a/internal/campaigns/testdata/dummydocker/docker.bat b/internal/campaigns/testdata/dummydocker/docker.bat new file mode 100644 index 0000000000..7609ea4468 --- /dev/null +++ b/internal/campaigns/testdata/dummydocker/docker.bat @@ -0,0 +1 @@ +bash docker diff --git a/internal/campaigns/testdata/dummydocker/docker.exe b/internal/campaigns/testdata/dummydocker/docker.exe deleted file mode 120000 index 12801ef6a5..0000000000 --- a/internal/campaigns/testdata/dummydocker/docker.exe +++ /dev/null @@ -1 +0,0 @@ -testdata/dummydocker/docker \ No newline at end of file From a2427fc21cc26811808cf18e95105e172d6cfdf5 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 30 Sep 2020 16:50:50 +0200 Subject: [PATCH 7/8] Try cd'ing into bat file directory --- internal/campaigns/testdata/dummydocker/docker.bat | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/campaigns/testdata/dummydocker/docker.bat b/internal/campaigns/testdata/dummydocker/docker.bat index 7609ea4468..0720333ec0 100644 --- a/internal/campaigns/testdata/dummydocker/docker.bat +++ b/internal/campaigns/testdata/dummydocker/docker.bat @@ -1 +1,2 @@ +cd %~dp0 bash docker From f9b987f07ad7bc654816dcdbfe61d0db474746b7 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 30 Sep 2020 16:53:55 +0200 Subject: [PATCH 8/8] Skip integration test on windows --- internal/campaigns/executor_test.go | 4 ++++ internal/campaigns/testdata/dummydocker/docker.bat | 2 -- 2 files changed, 4 insertions(+), 2 deletions(-) delete mode 100644 internal/campaigns/testdata/dummydocker/docker.bat diff --git a/internal/campaigns/executor_test.go b/internal/campaigns/executor_test.go index 369b1fcd2e..1fa5f3f4d3 100644 --- a/internal/campaigns/executor_test.go +++ b/internal/campaigns/executor_test.go @@ -22,6 +22,10 @@ import ( ) func TestExecutor_Integration(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Test doesn't work on Windows because dummydocker is written in bash") + } + addToPath(t, "testdata/dummydocker") repo := &graphql.Repository{ diff --git a/internal/campaigns/testdata/dummydocker/docker.bat b/internal/campaigns/testdata/dummydocker/docker.bat deleted file mode 100644 index 0720333ec0..0000000000 --- a/internal/campaigns/testdata/dummydocker/docker.bat +++ /dev/null @@ -1,2 +0,0 @@ -cd %~dp0 -bash docker