Skip to content

Commit 3c0d4a9

Browse files
authored
Merge pull request #134 from morluto/codex/optimize-go-test-suite
test: speed up Go test workflow
2 parents 6195dd0 + ca78ad6 commit 3c0d4a9

33 files changed

Lines changed: 587 additions & 371 deletions

.github/workflows/ci.yml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ jobs:
2828
with:
2929
go-version-file: go.mod
3030
cache: true
31-
- run: go test -short -parallel=4 ./...
31+
cache-dependency-path: go.sum
32+
- run: go test -short -p=8 -parallel=8 ./...
3233

3334
validate-agents-md:
3435
name: Validate AGENTS.md
@@ -48,6 +49,7 @@ jobs:
4849
with:
4950
go-version-file: go.mod
5051
cache: true
52+
cache-dependency-path: go.sum
5153
- name: Check formatting
5254
run: make fmt-check
5355

@@ -63,6 +65,7 @@ jobs:
6365
with:
6466
go-version-file: go.mod
6567
cache: true
68+
cache-dependency-path: go.sum
6669
- name: Resolve lint comparison base
6770
id: lint-base
6871
shell: bash
@@ -138,6 +141,7 @@ jobs:
138141
with:
139142
go-version-file: go.mod
140143
cache: true
144+
cache-dependency-path: go.sum
141145
- name: Run tests with coverage and timing
142146
run: make cover-check
143147
- name: Upload coverage artifact
@@ -157,6 +161,7 @@ jobs:
157161
with:
158162
go-version-file: go.mod
159163
cache: true
164+
cache-dependency-path: go.sum
160165
- run: make test-race
161166

162167
tidy:
@@ -169,6 +174,7 @@ jobs:
169174
with:
170175
go-version-file: go.mod
171176
cache: true
177+
cache-dependency-path: go.sum
172178
- name: Check go.mod is tidy
173179
run: make tidy-check
174180

@@ -182,6 +188,7 @@ jobs:
182188
with:
183189
go-version-file: go.mod
184190
cache: true
191+
cache-dependency-path: go.sum
185192
- run: make generate-check
186193

187194
security:
@@ -197,6 +204,7 @@ jobs:
197204
with:
198205
go-version-file: go.mod
199206
cache: true
207+
cache-dependency-path: go.sum
200208
- name: Initialize CodeQL
201209
uses: github/codeql-action/init@v3
202210
with:
@@ -219,6 +227,7 @@ jobs:
219227
with:
220228
go-version-file: go.mod
221229
cache: true
230+
cache-dependency-path: go.sum
222231
- run: npm run test:npm
223232
- run: npm run test:e2e
224233
- run: npm pack --dry-run

.github/workflows/release.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ jobs:
1919
with:
2020
go-version-file: go.mod
2121
cache: true
22+
cache-dependency-path: go.sum
2223
- uses: actions/setup-node@v6
2324
with:
2425
node-version: 22
@@ -47,6 +48,7 @@ jobs:
4748
with:
4849
go-version-file: go.mod
4950
cache: true
51+
cache-dependency-path: go.sum
5052
- name: Build native binary
5153
shell: bash
5254
env:
@@ -80,6 +82,7 @@ jobs:
8082
with:
8183
go-version-file: go.mod
8284
cache: true
85+
cache-dependency-path: go.sum
8386
- name: Run GoReleaser
8487
uses: goreleaser/goreleaser-action@v7
8588
with:

CONTRIBUTING.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,11 @@ make test
3838
go test ./internal/app -run '^TestName$'
3939
```
4040

41+
The repository test suite uses eight package workers and eight in-process
42+
parallel test slots by default because its isolated SQLite and filesystem
43+
tests are I/O-heavy. Override either value for a constrained machine, for
44+
example `make test TEST_PACKAGE_PARALLELISM=4 TEST_PARALLELISM=4`.
45+
4146
Run the fast local checks before pushing and the complete validation before a
4247
pull request:
4348

Makefile

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,15 @@ GOFMT ?= gofmt
1010
GOLANGCI_LINT ?= $(shell command -v golangci-lint 2>/dev/null || printf '%s/bin/golangci-lint' "$$($(GO) env GOPATH)")
1111
GOLANGCI_LINT_VERSION ?= v2.12.2
1212
GOLANGCI_LINT_BIN ?= $(shell $(GO) env GOPATH)/bin
13+
# Most repository tests are I/O-heavy and opt into t.Parallel. Keep this
14+
# overridable for constrained machines while allowing local and CI runs to
15+
# use more than the historical four-test cap.
16+
TEST_PARALLELISM ?= 8
17+
TEST_PACKAGE_PARALLELISM ?= 8
1318

1419
help:
1520
@echo "Common targets:"
16-
@echo " make test cached local test suite"
21+
@echo " make test cached local test suite (packages=$(TEST_PACKAGE_PARALLELISM), tests=$(TEST_PARALLELISM))"
1722
@echo " make check fast formatting, test, and changed-code lint checks"
1823
@echo " make verify complete uncached local validation"
1924
@echo " make test-race focused race tests for stateful packages"
@@ -60,22 +65,26 @@ install-tools:
6065
sh "$$installer" -b "$(GOLANGCI_LINT_BIN)" "$(GOLANGCI_LINT_VERSION)"
6166

6267
test:
63-
$(GO) test -short -parallel=4 -timeout 120s ./...
68+
$(GO) test -short -p=$(TEST_PACKAGE_PARALLELISM) -parallel=$(TEST_PARALLELISM) -timeout 120s ./...
6469

6570
test-uncached:
66-
$(GO) test -short -parallel=4 -count=1 -timeout 120s ./...
71+
$(GO) test -short -p=$(TEST_PACKAGE_PARALLELISM) -parallel=$(TEST_PARALLELISM) -count=1 -timeout 120s ./...
6772

6873
test-race:
69-
$(GO) test -short -race -parallel=2 -timeout 300s ./internal/app ./internal/corpus ./internal/workspace
74+
# Keep package-level overlap for cross-package race coverage while bounding
75+
# in-process test concurrency for the CPU-heavy SQLite tests.
76+
$(GO) test -short -race -p=4 -parallel=2 -timeout 600s ./internal/app ./internal/corpus ./internal/workspace
7077

7178
test-race-full:
72-
$(GO) test -race -parallel=4 -count=1 -timeout 300s ./...
79+
# Keep package-level overlap for cross-package race coverage while bounding
80+
# in-process test concurrency for the CPU-heavy SQLite tests.
81+
$(GO) test -race -p=4 -parallel=2 -count=1 -timeout 900s ./...
7382

7483
test-verbose:
75-
$(GO) test -short -v -parallel=4 -timeout 120s ./...
84+
$(GO) test -short -v -p=$(TEST_PACKAGE_PARALLELISM) -parallel=$(TEST_PARALLELISM) -timeout 120s ./...
7685

7786
test-cover:
78-
$(GO) test -short -parallel=4 \
87+
$(GO) test -short -p=$(TEST_PACKAGE_PARALLELISM) -parallel=$(TEST_PARALLELISM) \
7988
-coverprofile=coverage.out \
8089
-covermode=set \
8190
-coverpkg=./internal/... \

internal/app/app_parallel_test.go

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
package app
2+
3+
import (
4+
"context"
5+
"errors"
6+
"strings"
7+
"testing"
8+
"time"
9+
10+
"github.com/morluto/gitcontribute/internal/cli"
11+
"github.com/morluto/gitcontribute/internal/config"
12+
"github.com/morluto/gitcontribute/internal/github"
13+
)
14+
15+
func TestEndToEndSyncSearchDossier(t *testing.T) {
16+
t.Parallel()
17+
ctx := context.Background()
18+
srv := newTestServer("octocat", "test")
19+
defer srv.Close()
20+
21+
svc := newTestService(t, srv)
22+
defer func() { _ = svc.Close() }()
23+
24+
syncRes, err := svc.Sync(ctx, cli.RepoRef{Owner: "octocat", Repo: "test"})
25+
if err != nil {
26+
t.Fatalf("sync: %v", err)
27+
}
28+
if syncRes.Updated != 2 {
29+
t.Fatalf("updated = %d, want 2", syncRes.Updated)
30+
}
31+
32+
searchRes, err := svc.Search(ctx, "searchable", cli.SearchOptions{Kind: "issues", Limit: 10})
33+
if err != nil {
34+
t.Fatalf("search: %v", err)
35+
}
36+
if searchRes.Total != 1 || len(searchRes.Matches) != 1 {
37+
t.Fatalf("search results = %+v", searchRes)
38+
}
39+
if searchRes.Matches[0].Number != 1 {
40+
t.Fatalf("unexpected match: %+v", searchRes.Matches[0])
41+
}
42+
43+
dossierRes, err := svc.Dossier(ctx, cli.RepoRef{Owner: "octocat", Repo: "test"})
44+
if err != nil {
45+
t.Fatalf("dossier: %v", err)
46+
}
47+
if dossierRes.Stars != 42 {
48+
t.Fatalf("stars = %d, want 42", dossierRes.Stars)
49+
}
50+
if dossierRes.OpenIssues != 1 {
51+
t.Fatalf("open issues = %d, want 1", dossierRes.OpenIssues)
52+
}
53+
if dossierRes.Summary != "A test repository" {
54+
t.Fatalf("summary = %q", dossierRes.Summary)
55+
}
56+
}
57+
58+
func TestTailSourceRunsOneIdempotentIteration(t *testing.T) {
59+
t.Parallel()
60+
ctx := context.Background()
61+
srv := newTestServer("octocat", "tail")
62+
defer srv.Close()
63+
svc := newTestService(t, srv)
64+
defer func() { _ = svc.Close() }()
65+
66+
if _, err := svc.AddRepoSource(ctx, "explicit", []cli.RepoRef{{Owner: "octocat", Repo: "tail"}}); err != nil {
67+
t.Fatalf("add source: %v", err)
68+
}
69+
result, err := svc.TailSource(ctx, "explicit", cli.TailOptions{
70+
Since: time.Hour, Budget: 1, Interval: time.Minute, Once: true,
71+
})
72+
if err != nil {
73+
t.Fatalf("tail source: %v", err)
74+
}
75+
if result.Iterations != 1 || result.Last == nil || result.Last.Repositories != 1 {
76+
t.Fatalf("tail result = %+v", result)
77+
}
78+
}
79+
80+
func TestDiscoveryCrawlDoesNotAdvanceCheckpointWhenBudgetExhausted(t *testing.T) {
81+
t.Parallel()
82+
ctx := context.Background()
83+
srv := newTestServer("octocat", "discovered")
84+
defer srv.Close()
85+
svc := newTestService(t, srv)
86+
defer func() { _ = svc.Close() }()
87+
if _, err := svc.AddSearchSource(ctx, "bounded", "language:go"); err != nil {
88+
t.Fatal(err)
89+
}
90+
if _, err := svc.Crawl(ctx, "bounded", cli.CrawlOptions{Since: time.Hour, Budget: 1}); err == nil || !strings.Contains(err.Error(), "budget") {
91+
t.Fatalf("crawl error = %v, want budget exhaustion", err)
92+
}
93+
c, err := svc.openCorpus(ctx)
94+
if err != nil {
95+
t.Fatal(err)
96+
}
97+
if checkpoint, exists, err := c.GetTime(ctx, "source:bounded"); err != nil || exists {
98+
t.Fatalf("checkpoint = %v exists=%v err=%v", checkpoint, exists, err)
99+
}
100+
}
101+
102+
func TestAddSearchSourceRejectsUnstableName(t *testing.T) {
103+
t.Parallel()
104+
ctx := context.Background()
105+
srv := newTestServer("octocat", "test")
106+
defer srv.Close()
107+
svc := newTestService(t, srv)
108+
defer func() { _ = svc.Close() }()
109+
if _, err := svc.AddSearchSource(ctx, "contains spaces", "language:go"); err == nil {
110+
t.Fatal("expected invalid source name error")
111+
}
112+
}
113+
114+
func TestSearchReportsDefaultLimit(t *testing.T) {
115+
t.Parallel()
116+
ctx := context.Background()
117+
srv := newTestServer("octocat", "test")
118+
defer srv.Close()
119+
svc := newTestService(t, srv)
120+
defer func() { _ = svc.Close() }()
121+
if _, err := svc.Sync(ctx, cli.RepoRef{Owner: "octocat", Repo: "test"}); err != nil {
122+
t.Fatal(err)
123+
}
124+
result, err := svc.Search(ctx, "searchable", cli.SearchOptions{})
125+
if err != nil {
126+
t.Fatal(err)
127+
}
128+
if result.Limit != 20 {
129+
t.Fatalf("reported limit = %d, want 20", result.Limit)
130+
}
131+
}
132+
133+
func TestLocalInitializationDoesNotResolveKeyringAuth(t *testing.T) {
134+
t.Parallel()
135+
ctx := context.Background()
136+
paths := config.NewPaths(&config.Env{Home: t.TempDir()})
137+
configPath, err := paths.ConfigFile()
138+
if err != nil {
139+
t.Fatal(err)
140+
}
141+
cfg := config.Default()
142+
cfg.TokenSource.Method = "keyring"
143+
cfg.TokenSource.Key = "test-account"
144+
if err := config.ApplyDefaults(cfg, paths); err != nil {
145+
t.Fatal(err)
146+
}
147+
if err := config.Save(configPath, cfg); err != nil {
148+
t.Fatal(err)
149+
}
150+
svc, err := New(paths, "test", nil)
151+
if err != nil {
152+
t.Fatalf("local service construction resolved GitHub auth: %v", err)
153+
}
154+
defer func() { _ = svc.Close() }()
155+
if _, err := svc.Init(ctx); err != nil {
156+
t.Fatalf("local init resolved GitHub auth: %v", err)
157+
}
158+
}
159+
160+
func TestConfiguredAuthenticationIsRequired(t *testing.T) {
161+
t.Setenv("GITCONTRIBUTE_TEST_MISSING_TOKEN", "")
162+
cfg := config.Default()
163+
cfg.TokenSource = config.TokenSource{
164+
Method: "env",
165+
Key: "GITCONTRIBUTE_TEST_MISSING_TOKEN",
166+
}
167+
168+
_, err := tokenSource(cfg).Token(context.Background())
169+
if !errors.Is(err, github.ErrRequiredToken) {
170+
t.Fatalf("configured auth error = %v, want ErrRequiredToken", err)
171+
}
172+
}
173+
174+
func TestNewRejectsInvalidConfiguredTokenSource(t *testing.T) {
175+
t.Parallel()
176+
paths := config.NewPaths(&config.Env{Home: t.TempDir()})
177+
configPath, err := paths.ConfigFile()
178+
if err != nil {
179+
t.Fatal(err)
180+
}
181+
cfg := config.Default()
182+
cfg.TokenSource.Method = "keyrign"
183+
if err := config.ApplyDefaults(cfg, paths); err != nil {
184+
t.Fatal(err)
185+
}
186+
if err := config.Save(configPath, cfg); err != nil {
187+
t.Fatal(err)
188+
}
189+
190+
_, err = New(paths, "test", nil)
191+
if err == nil || !strings.Contains(err.Error(), "invalid token_source method") {
192+
t.Fatalf("New error = %v, want invalid token source", err)
193+
}
194+
}

0 commit comments

Comments
 (0)