Skip to content

Commit 81bfe6c

Browse files
committed
perf: speed up job integration tests
Amp-Thread-ID: https://ampcode.com/threads/T-019daa42-5cca-758d-b7b5-901134fb8691
1 parent 1d12fcf commit 81bfe6c

5 files changed

Lines changed: 210 additions & 19 deletions

File tree

internal/job/hook/wrapper.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ const (
3434
hookExitStatusEnv = "BUILDKITE_HOOK_EXIT_STATUS"
3535
hookWorkingDirEnv = "BUILDKITE_HOOK_WORKING_DIR"
3636
hookWrapperDir = "buildkite-agent-hook-wrapper"
37+
testJobID = "1111-1111-1111-1111"
3738

3839
batchWrapper = `@echo off
3940
SETLOCAL ENABLEDELAYEDEXPANSION
@@ -247,7 +248,7 @@ func NewWrapper(opts ...WrapperOpt) (*Wrapper, error) {
247248
return nil, fmt.Errorf("finding absolute path to %q: %w", wrap.hookPath, err)
248249
}
249250

250-
buildkiteAgent, err := os.Executable()
251+
buildkiteAgent, err := resolveBuildkiteAgentBinary()
251252
if err != nil {
252253
return nil, err
253254
}
@@ -274,6 +275,18 @@ func NewWrapper(opts ...WrapperOpt) (*Wrapper, error) {
274275
return wrap, nil
275276
}
276277

278+
func resolveBuildkiteAgentBinary() (string, error) {
279+
// Integration tests override self-execution to a bintest mock so hook wrappers
280+
// do not need to spin up the full test binary just to run `env dump`.
281+
if os.Getenv("BUILDKITE_JOB_ID") == testJobID {
282+
if overrideSelf := os.Getenv("BUILDKITE_OVERRIDE_SELF"); overrideSelf != "" {
283+
return overrideSelf, nil
284+
}
285+
}
286+
287+
return os.Executable()
288+
}
289+
277290
// WriteHookWrapper will write a hook wrapper script to a temporary file with the same extension as,
278291
// `hookWrapperName`. It will return the name of the temporary file. The file will be executable.
279292
// It will be created from the template specified by `templateType` with data from `input`.

internal/job/hook/wrapper_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"os"
88
"path/filepath"
99
"runtime"
10+
"strings"
1011
"testing"
1112

1213
"github.com/buildkite/agent/v3/env"
@@ -216,6 +217,22 @@ func TestScriptWrapperFailsOnHookWithInvalidShebang(t *testing.T) {
216217
assert.Error(t, err, `scriptwrapper tried to wrap hook with invalid shebang: "#!/usr/bin/env ruby"`)
217218
}
218219

220+
func TestWrapperUsesOverrideSelfForExecutorTester(t *testing.T) {
221+
t.Setenv("BUILDKITE_JOB_ID", "1111-1111-1111-1111")
222+
t.Setenv("BUILDKITE_OVERRIDE_SELF", "mock-buildkite-agent")
223+
224+
hookFilename := writeTestHook(t, "hook", "#!/bin/sh\ntrue\n")
225+
wrapper, err := hook.NewWrapper(
226+
hook.WithPath(hookFilename),
227+
hook.WithOS("linux"),
228+
)
229+
assert.NilError(t, err, "failed to create hook wrapper: %v", err)
230+
231+
wrapperContents, err := os.ReadFile(wrapper.Path())
232+
assert.NilError(t, err, "failed to read wrapper %q: %v", wrapper.Path(), err)
233+
assert.Assert(t, strings.Contains(string(wrapperContents), `"mock-buildkite-agent" env dump`))
234+
}
235+
219236
func writeTestHook(t *testing.T, fileName, content string) string {
220237
t.Helper()
221238

internal/job/integration/executor_tester.go

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,11 @@ type ExecutorTester struct {
4242
Repo *gitRepository
4343
Output string
4444

45-
cmd *exec.Cmd
46-
cmdLock sync.Mutex
47-
hookMock *bintest.Mock
48-
mocks []*bintest.Mock
45+
pendingLocalHooks bool
46+
cmd *exec.Cmd
47+
cmdLock sync.Mutex
48+
hookMock *bintest.Mock
49+
mocks []*bintest.Mock
4950
}
5051

5152
func NewExecutorTester(ctx context.Context) (*ExecutorTester, error) {
@@ -239,19 +240,30 @@ func (e *ExecutorTester) ExpectLocalHook(name string) *bintest.Expectation {
239240
panic(err)
240241
}
241242

242-
hookPath, err := e.writeHookScript(e.hookMock, name, hooksDir, "local", name)
243+
_, err := e.writeHookScript(e.hookMock, name, hooksDir, "local", name)
243244
if err != nil {
244245
panic(err)
245246
}
246247

247-
if err = e.Repo.Add(hookPath); err != nil {
248-
panic(err)
248+
e.pendingLocalHooks = true
249+
250+
return e.hookMock.Expect("local", name)
251+
}
252+
253+
func (e *ExecutorTester) flushPendingLocalHooks() error {
254+
if !e.pendingLocalHooks {
255+
return nil
249256
}
250-
if err = e.Repo.Commit("Added local hook file %s", name); err != nil {
251-
panic(err)
257+
258+
if err := e.Repo.Add(filepath.Join(".buildkite", "hooks")); err != nil {
259+
return fmt.Errorf("adding local hooks: %w", err)
260+
}
261+
if err := e.Repo.Commit("Added local hook files"); err != nil {
262+
return fmt.Errorf("committing local hooks: %w", err)
252263
}
253264

254-
return e.hookMock.Expect("local", name)
265+
e.pendingLocalHooks = false
266+
return nil
255267
}
256268

257269
// ExpectGlobalHook creates a mock object and a script in the global buildkite hooks dir
@@ -269,6 +281,10 @@ func (e *ExecutorTester) ExpectGlobalHook(name string) *bintest.Expectation {
269281
func (e *ExecutorTester) Run(t *testing.T, env ...string) error {
270282
t.Helper()
271283

284+
if err := e.flushPendingLocalHooks(); err != nil {
285+
return err
286+
}
287+
272288
// Mock out the meta-data calls to the agent after checkout
273289
if !e.HasMock("buildkite-agent") {
274290
agent := e.MockAgent(t)

internal/job/integration/git.go

Lines changed: 103 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,46 @@ package integration
22

33
import (
44
"fmt"
5+
"io"
56
"log"
67
"os"
78
"os/exec"
89
"path/filepath"
910
"strings"
11+
"sync"
1012
)
1113

1214
type gitRepository struct {
1315
Path string
1416
}
1517

18+
var testGitRepositoryTemplate = sync.OnceValues(func() (string, error) {
19+
repo, err := createSeedTestGitRepository()
20+
if err != nil {
21+
return "", err
22+
}
23+
return repo.Path, nil
24+
})
25+
1626
func createTestGitRespository() (*gitRepository, error) {
27+
templatePath, err := testGitRepositoryTemplate()
28+
if err != nil {
29+
return nil, err
30+
}
31+
32+
repoPath, err := newGitRepositoryPath()
33+
if err != nil {
34+
return nil, err
35+
}
36+
if err := copyDirectory(templatePath, repoPath); err != nil {
37+
_ = os.RemoveAll(repoPath)
38+
return nil, fmt.Errorf("copying git repo template: %w", err)
39+
}
40+
41+
return &gitRepository{Path: repoPath}, nil
42+
}
43+
44+
func createSeedTestGitRepository() (*gitRepository, error) {
1745
repo, err := newGitRepository()
1846
if err != nil {
1947
return nil, err
@@ -95,9 +123,25 @@ func createTestGitRespository() (*gitRepository, error) {
95123
}
96124

97125
func newGitRepository() (*gitRepository, error) {
126+
tempDir, err := newGitRepositoryPath()
127+
if err != nil {
128+
return nil, err
129+
}
130+
131+
gr := &gitRepository{Path: tempDir}
132+
gitErr := gr.ExecuteAll([][]string{
133+
{"init"},
134+
{"config", "user.email", "you@example.com"},
135+
{"config", "user.name", "Your Name"},
136+
})
137+
138+
return gr, gitErr
139+
}
140+
141+
func newGitRepositoryPath() (string, error) {
98142
tempDirRaw, err := os.MkdirTemp("", "git-repo")
99143
if err != nil {
100-
return nil, fmt.Errorf("Error creating temp dir: %w", err)
144+
return "", fmt.Errorf("Error creating temp dir: %w", err)
101145
}
102146

103147
// io.TempDir on Windows tilde-shortens (8.3 style?) long filenames in the path.
@@ -118,17 +162,68 @@ func newGitRepository() (*gitRepository, error) {
118162
// EvalSymlinks seems best? Maybe there's a better way?
119163
tempDir, err := filepath.EvalSymlinks(tempDirRaw)
120164
if err != nil {
121-
return nil, fmt.Errorf("EvalSymlinks for temp dir: %w", err)
165+
return "", fmt.Errorf("EvalSymlinks for temp dir: %w", err)
122166
}
167+
return tempDir, nil
168+
}
123169

124-
gr := &gitRepository{Path: tempDir}
125-
gitErr := gr.ExecuteAll([][]string{
126-
{"init"},
127-
{"config", "user.email", "you@example.com"},
128-
{"config", "user.name", "Your Name"},
170+
func copyDirectory(src, dst string) error {
171+
return filepath.WalkDir(src, func(path string, d os.DirEntry, walkErr error) error {
172+
if walkErr != nil {
173+
return walkErr
174+
}
175+
176+
relPath, err := filepath.Rel(src, path)
177+
if err != nil {
178+
return err
179+
}
180+
if relPath == "." {
181+
return nil
182+
}
183+
184+
targetPath := filepath.Join(dst, relPath)
185+
info, err := os.Lstat(path)
186+
if err != nil {
187+
return err
188+
}
189+
190+
switch mode := info.Mode(); {
191+
case mode.IsDir():
192+
return os.MkdirAll(targetPath, mode.Perm())
193+
case mode.IsRegular():
194+
return copyFile(path, targetPath, mode)
195+
case mode&os.ModeSymlink != 0:
196+
linkTarget, err := os.Readlink(path)
197+
if err != nil {
198+
return err
199+
}
200+
return os.Symlink(linkTarget, targetPath)
201+
default:
202+
return fmt.Errorf("unsupported file mode %s for %s", mode, path)
203+
}
129204
})
205+
}
130206

131-
return gr, gitErr
207+
func copyFile(src, dst string, mode os.FileMode) error {
208+
reader, err := os.Open(src)
209+
if err != nil {
210+
return err
211+
}
212+
defer reader.Close()
213+
214+
writer, err := os.OpenFile(dst, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, mode.Perm())
215+
if err != nil {
216+
return err
217+
}
218+
219+
if _, err := io.Copy(writer, reader); err != nil {
220+
writer.Close()
221+
return err
222+
}
223+
if err := writer.Close(); err != nil {
224+
return err
225+
}
226+
return os.Chmod(dst, mode.Perm())
132227
}
133228

134229
func (gr *gitRepository) Add(path string) error {
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package integration
2+
3+
import "testing"
4+
5+
func BenchmarkNewExecutorTester(b *testing.B) {
6+
b.ReportAllocs()
7+
8+
for b.Loop() {
9+
tester, err := NewExecutorTester(mainCtx)
10+
if err != nil {
11+
b.Fatalf("NewExecutorTester() error = %v", err)
12+
}
13+
if err := tester.Close(); err != nil {
14+
b.Fatalf("tester.Close() error = %v", err)
15+
}
16+
}
17+
}
18+
19+
func BenchmarkFlushPendingLocalHooks(b *testing.B) {
20+
b.ReportAllocs()
21+
22+
hooks := []string{
23+
"environment",
24+
"pre-checkout",
25+
"post-checkout",
26+
"pre-command",
27+
"post-command",
28+
"pre-artifact",
29+
"post-artifact",
30+
"pre-exit",
31+
}
32+
33+
for b.Loop() {
34+
tester, err := NewExecutorTester(mainCtx)
35+
if err != nil {
36+
b.Fatalf("NewExecutorTester() error = %v", err)
37+
}
38+
39+
for _, hook := range hooks {
40+
tester.ExpectLocalHook(hook).Once()
41+
}
42+
43+
if err := tester.flushPendingLocalHooks(); err != nil {
44+
b.Fatalf("tester.flushPendingLocalHooks() error = %v", err)
45+
}
46+
if err := tester.Close(); err != nil {
47+
b.Fatalf("tester.Close() error = %v", err)
48+
}
49+
}
50+
}

0 commit comments

Comments
 (0)