Skip to content

Commit 7fd5211

Browse files
authored
refactor(test): consolidate testing infrastructure with Docker Compose (#41)
## Summary Migrate from manual server setup to Docker Compose-based hermetic testing. Consolidate tests under test/ directory with consistent patterns. Key changes: - New test structure: test/e2e/, test/integration/{gateway,orchestrator,extensions}/ - Container naming: sq-test-{context}-{unique-id} for parallel execution - Shared utilities in test/testutil/ (compose, docker, mysql, schema helpers) - Removed old {service}/integration_test/ directories and speculator service - Documentation: TESTING.md → doc/howto/, STRUCTURE.md → PROJECT_STRUCTURE.md - Makefile: alphabetical targets, new integration test targets, local-start/stop naming - Consolidate CI and create separate jobs for faster run and add bazel cache ## Test Plan - Unit Tested - Integration Tested - CI Validation <img width="586" height="543" alt="Screenshot 2026-02-22 at 7 45 39 PM" src="https://github.com/user-attachments/assets/939c5e08-23d1-4e99-bd84-bd34f0a21309" />
1 parent 3c25a2f commit 7fd5211

79 files changed

Lines changed: 2976 additions & 2617 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/actions/logs/action.yml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
name: Logs
2+
description: Display container logs on test failure
3+
4+
runs:
5+
using: composite
6+
steps:
7+
- name: Display container logs on failure
8+
if: failure()
9+
shell: bash
10+
run: |
11+
echo "=== Listing all Docker containers ==="
12+
docker ps -a --filter "name=sq-test-" || true
13+
echo ""
14+
echo "=== Dumping container logs ==="
15+
for container in $(docker ps -a --filter "name=sq-test-" --format "{{.Names}}"); do
16+
echo ">>> Logs for $container <<<"
17+
docker logs "$container" 2>&1 || true
18+
echo ""
19+
done
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
name: Run Bazel Test
2+
description: Run Bazel test and show logs on failure
3+
4+
inputs:
5+
target:
6+
description: Bazel test target pattern
7+
required: true
8+
9+
runs:
10+
using: composite
11+
steps:
12+
- name: Run Bazel test
13+
shell: bash
14+
run: ./tool/bazel test ${{ inputs.target }} --test_output=errors
15+
16+
- uses: ./.github/actions/logs
17+
if: always()

.github/actions/setup/action.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
name: Setup
2+
description: Setup Bazel cache for CI jobs
3+
4+
runs:
5+
using: composite
6+
steps:
7+
- name: Setup Bazel cache
8+
uses: actions/cache@v4
9+
with:
10+
path: |
11+
~/.cache/bazel
12+
~/.cache/bazelisk
13+
key: bazel-${{ runner.os }}-${{ hashFiles('MODULE.bazel', 'go.mod', '.bazelversion') }}
14+
restore-keys: |
15+
bazel-${{ runner.os }}-

.github/workflows/build_and_test.yml

Lines changed: 0 additions & 50 deletions
This file was deleted.

.github/workflows/ci.yml

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
name: CI
2+
3+
on:
4+
push:
5+
branches:
6+
- main
7+
pull_request:
8+
types:
9+
- opened
10+
- reopened
11+
- synchronize
12+
13+
permissions:
14+
contents: read
15+
pull-requests: read
16+
17+
jobs:
18+
# ---------------------------------------------------------------------------
19+
# BUILD AND UNIT TESTS (special case - Gazelle + build + unit tests)
20+
# ---------------------------------------------------------------------------
21+
build-and-unit-test:
22+
name: Build and Unit Test
23+
runs-on: ubuntu-latest
24+
steps:
25+
- uses: actions/checkout@v4
26+
- uses: ./.github/actions/setup
27+
28+
- name: Check BUILD files are up to date
29+
run: |
30+
echo "Running Gazelle to check BUILD files..." >&2
31+
make gazelle
32+
if ! git diff --quiet; then
33+
echo "BUILD files are out of date!" >&2
34+
echo "" >&2
35+
echo "The following files were modified by Gazelle:" >&2
36+
git diff --name-only >&2
37+
echo "" >&2
38+
echo "Please run 'make gazelle' locally and commit the changes." >&2
39+
exit 1
40+
fi
41+
echo "BUILD files are up to date" >&2
42+
43+
- name: Build project
44+
run: make build
45+
46+
- name: Run unit tests
47+
run: make test || echo "No unit tests found"
48+
49+
# ---------------------------------------------------------------------------
50+
# INTEGRATION TESTS (e2e, gateway, orchestrator)
51+
# ---------------------------------------------------------------------------
52+
e2e:
53+
name: E2E Integration Test
54+
runs-on: ubuntu-latest
55+
steps:
56+
- uses: actions/checkout@v4
57+
- uses: ./.github/actions/setup
58+
59+
- name: Run E2E tests
60+
run: make e2e-test
61+
62+
- uses: ./.github/actions/logs
63+
64+
gateway-integration-test:
65+
name: Gateway Integration Test
66+
runs-on: ubuntu-latest
67+
steps:
68+
- uses: actions/checkout@v4
69+
- uses: ./.github/actions/setup
70+
71+
- name: Run Gateway integration tests
72+
run: make integration-test-gateway
73+
74+
- uses: ./.github/actions/logs
75+
76+
orchestrator-integration-test:
77+
name: Orchestrator Integration Test
78+
runs-on: ubuntu-latest
79+
steps:
80+
- uses: actions/checkout@v4
81+
- uses: ./.github/actions/setup
82+
83+
- name: Run Orchestrator integration tests
84+
run: make integration-test-orchestrator
85+
86+
- uses: ./.github/actions/logs
87+
88+
# ---------------------------------------------------------------------------
89+
# EXTENSION TESTS
90+
# ---------------------------------------------------------------------------
91+
counter-integration-test:
92+
name: Counter Extension Test
93+
runs-on: ubuntu-latest
94+
steps:
95+
- uses: actions/checkout@v4
96+
- uses: ./.github/actions/setup
97+
- uses: ./.github/actions/run-bazel-test
98+
with:
99+
target: //test/integration/extension/counter/...
100+
101+
queue-integration-test:
102+
name: Queue Extension Test
103+
runs-on: ubuntu-latest
104+
steps:
105+
- uses: actions/checkout@v4
106+
- uses: ./.github/actions/setup
107+
- uses: ./.github/actions/run-bazel-test
108+
with:
109+
target: //test/integration/extension/queue/...
110+
111+
storage-integration-test:
112+
name: Storage Extension Test
113+
runs-on: ubuntu-latest
114+
steps:
115+
- uses: actions/checkout@v4
116+
- uses: ./.github/actions/setup
117+
- uses: ./.github/actions/run-bazel-test
118+
with:
119+
target: //test/integration/extension/storage/...
120+
121+
# ---------------------------------------------------------------------------
122+
# REQUIRED CHECKS GATE
123+
# ---------------------------------------------------------------------------
124+
required-checks:
125+
name: Required Checks
126+
runs-on: ubuntu-latest
127+
needs:
128+
- build-and-unit-test
129+
- e2e
130+
- gateway-integration-test
131+
- orchestrator-integration-test
132+
- counter-integration-test
133+
- queue-integration-test
134+
- storage-integration-test
135+
steps:
136+
- name: All required checks passed
137+
run: |
138+
echo "All required checks passed!" >&2

.gitignore

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,3 +10,7 @@ MODULE.bazel.lock
1010

1111
# Built binaries
1212
bin/
13+
.docker-bin/
14+
15+
# Make completion cache
16+
.make_targets_cache

BUILD.bazel

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,13 @@ load("@gazelle//:def.bzl", "gazelle")
77
# gazelle:resolve go github.com/uber/submitqueue/orchestrator/protopb //orchestrator/protopb
88
# gazelle:resolve go github.com/uber/submitqueue/speculator/protopb //speculator/protopb
99

10+
# Export marker files for test data dependencies (used by FindRepoRoot in tests)
11+
exports_files(
12+
[
13+
"MODULE.bazel",
14+
"go.mod",
15+
],
16+
visibility = ["//visibility:public"],
17+
)
18+
1019
gazelle(name = "gazelle")

CLAUDE.md

Lines changed: 58 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,7 @@ Three services, each following the same layout:
5353
│ ├── {step}.go # Step in workflow
5454
│ └── {step}_test.go
5555
├── proto/ # Proto definitions (.proto files)
56-
├── protopb/ # Generated proto code (committed to repo)
57-
└── integration_test/
56+
└── protopb/ # Generated proto code (committed to repo)
5857
```
5958

6059
### Controllers
@@ -141,27 +140,7 @@ extension/
141140

142141
### Directory Structure
143142

144-
```
145-
submitqueue/
146-
├── MODULE.bazel # Bzlmod dependencies
147-
├── go.mod # Go module dependencies
148-
├── BUILD.bazel # Root build configuration
149-
├── Makefile # Build automation
150-
├── .bazelversion # Pinned Bazel version
151-
├── .envrc # direnv configuration
152-
├── tool/bazel # Bazelisk wrapper
153-
├── gateway/ # Gateway service
154-
├── orchestrator/ # Orchestrator service
155-
├── speculator/ # Speculator service
156-
├── extension/ # Pluggable backend implementations
157-
├── entity/ # Domain entities
158-
├── example/ # Server and client examples
159-
│ ├── server/{service}/
160-
│ └── client/{service}/
161-
├── e2e_test/ # Cross-service hermetic tests (Testcontainers)
162-
├── doc/ # Documentation
163-
└── bin/ # Compiled binaries (gitignored)
164-
```
143+
See [doc/PROJECT_STRUCTURE.md](doc/PROJECT_STRUCTURE.md) for detailed project organization and architecture.
165144

166145
### Build System
167146

@@ -173,6 +152,9 @@ This repository uses **Bazel with Bzlmod** (NOT WORKSPACE) for dependency manage
173152
- **Bazel wrapper**: `./tool/bazel` (Bazelisk wrapper). With direnv (`.envrc`), use `bazel` directly.
174153
- **External dependencies**: Must be added to both `go.mod` AND `MODULE.bazel`
175154
- **BUILD files**: Every Go package must have a `BUILD.bazel` file
155+
- **Gazelle**: Run `make gazelle` after adding/removing Go files to update BUILD files
156+
- CI enforces BUILD files are in sync - will fail if `make gazelle` generates changes
157+
- Always run `make gazelle` before committing
176158

177159
### Proto Generation
178160

@@ -196,20 +178,49 @@ All generated proto files are **committed to the repository**. When modifying `.
196178
- Use **singular** names for directories (e.g., `mock/` not `mocks/`, `entity/` not `entities/`)
197179
- This applies to all folders including test mocks, extensions, entities, and service directories
198180

181+
### Makefile Convention
182+
183+
The `Makefile` follows strict conventions for maintainability:
184+
185+
**Alphabetical ordering:**
186+
- **Targets are alphabetically sorted** — makes it easy to find specific targets
187+
- **`.PHONY` declaration** — lists all targets in alphabetical order
188+
- **`help` target is always last** — exception to alphabetical ordering for discoverability
189+
- When adding new targets, insert them in alphabetical order (not at the end)
190+
191+
**Help text documentation:**
192+
- **Add `## Description` after each target** — enables auto-generated help and shell completion
193+
- Format: `target: ## Short description of what this target does`
194+
- Example: `build: ## Build all services and examples`
195+
- Run `make help` to see all documented targets with descriptions
196+
- Shell completion (zsh) shows these descriptions when you press `<TAB>`
197+
198+
**Example target with help text:**
199+
```makefile
200+
integration-test: build-all-linux ## Run all integration tests (auto-builds binaries)
201+
@echo "Running all integration tests..."
202+
@$(BAZEL) test //test/integration/... --test_output=errors
203+
```
204+
205+
This convention makes the Makefile self-documenting and enables powerful shell completion.
206+
199207
### Common Make Targets
200208

201209
```bash
202210
make build # Build all services
203211
make proto # Regenerate proto files
204212
make test # Run unit tests
205-
make integration-test # Run service integration tests
206-
make e2e-test # Run hermetic tests with Testcontainers
207-
make run-gateway # Run gateway (port 8081)
208-
make run-orchestrator # Run orchestrator (port 8082)
209-
make run-speculator # Run speculator (port 8083)
210-
make run-client-gateway # Run gateway client
213+
make integration-test # Run all integration tests (Docker-based)
214+
make integration-test-gateway # Test Gateway service
215+
make e2e-test # Run end-to-end tests
216+
make local-start # Start full stack with Docker Compose
217+
make local-ps # Show running containers and ports
218+
make local-logs # View logs from all services
219+
make local-stop # Stop all services
220+
make run-client-gateway # Test Gateway client (SERVER_ADDR, MESSAGE)
221+
make run-client-orchestrator # Test Orchestrator client
211222
make gazelle # Update BUILD.bazel files
212-
make clean # Remove binaries and Bazel cache
223+
make clean # Clean Bazel cache
213224
make clean-proto # Remove generated proto files
214225
```
215226

@@ -241,6 +252,23 @@ make clean-proto # Remove generated proto files
241252
2. **Avoid blocking operations for synchronization** — do not use `time.Sleep`. Design the tested routine to signal back (channels, callbacks, condition variables).
242253
3. **Use testify assertions** — use `stretchr/assert` or `require` instead of `t.Fatal()`.
243254

255+
**Integration Test Conventions:**
256+
257+
1. **Package naming** — use folder name as package name (NOT `*_test` suffix):
258+
- `test/integration/gateway/``package gateway`
259+
- `test/integration/extension/counter/mysql/``package mysql`
260+
- This matches Uber's go-code integration test pattern
261+
262+
2. **Bazel target naming** — use Gazelle-generated names and add `tags = ["integration"]`:
263+
- Target name matches folder: `name = "gateway_test"`, `name = "mysql_test"`
264+
- Always include `tags = ["integration"]` to exclude from unit tests
265+
- Include `data = [...]` for docker-compose and schema files
266+
267+
3. **Docker Compose-based** — all integration tests use Docker Compose:
268+
- Use `testutil.NewComposeStack()` for hermetic setup
269+
- Provide meaningful test context (e.g., "ext-storage-mysql", "svc-gateway")
270+
- Use `stack.ConnectMySQLService()` or `stack.MySQLServiceDSN()` for DB connections
271+
244272
### Code Style Guidelines
245273

246274
1. **Use SugaredLogger for structured logging** — always use `zap.SugaredLogger` with structured logging methods:

0 commit comments

Comments
 (0)