refactor: migrate integration tests to Go, split monolith, clean up seed data#701
Merged
zhouzhuojie merged 40 commits intoJun 10, 2026
Merged
Conversation
… shell fallback - Split integration_test.go (1034 lines) into 4 focused files: - integration_server.go: TestMain, lifecycle, seed runner - integration_client.go: HTTP helpers - integration_benchmark_test.go: benchmarks - integration_test.go: test functions only - Replace init() side effect with explicit initFlagDefs() - Flatten opGroup nested machinery to single []entry literal - Delete test.sh and all test-shell references - Always build fresh to temp dir (no stale-binary footgun) - Add testing section to docs/home.md - Update sidebar, AGENTS.md, plan doc to reflect current state - gitignore integration.test + integration.test.log
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #701 +/- ##
==========================================
- Coverage 81.19% 76.87% -4.33%
==========================================
Files 28 35 +7
Lines 2271 2266 -5
==========================================
- Hits 1844 1742 -102
- Misses 337 422 +85
- Partials 90 102 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…nt), remove separate up step from CI
Docker compose bind mounts create directories when the host path doesn't exist, which caused go test -c to fail (trying to write a binary file into a directory). Also, stale containers from previous runs kept the old directory mount. Fix: remove the bind mount from docker-compose.yml, copy the binary into the running container via 'docker compose cp' after up.
… fix test-instance paths
… binary Replace the alpine integration-runner (which needed the test binary injected via bind-mount/docker-cp/cat-pipe) with a golang:1.26-alpine container that compiles and runs 'go test' directly against each flagr instance. Changes: - integration-runner: alpine → golang:1.26-alpine, source mounted at /src - Remove build-integration-test Makefile target entirely - Remove integration.test binary creation, cleanup, and gitignore entry - CI integration_test job no longer needs setup-go (Go is in container) - test/test-instance/bench-instance all use 'go test' via docker exec
The 821551instance variable from the Makefile for loop was not visible inside 'docker compose exec' (separate shell). Fixed by passing the URL via -e FLAGR_SERVER_URL=http://821551instance:18000. Also restructured CI integration_test job to use a GitHub Actions matrix with max-parallel:1, making each DB instance visible as its own CI entry. test-instance target now properly fails on test failure.
…unner, revert CI matrix Instead of fighting with docker compose exec env var passing, TestMain now accepts FLAGR_SERVER_URLS as a comma-separated list and runs the full test suite against each instance sequentially. - integration_server.go: TestMain loops over urls, resets seedFlagIDs/seedFlagKeys per instance, tracks exit code - docker-compose.yml: Remove integration-runner (no longer needed) - Makefile: FLAGR_SERVER_URLS passed directly to go test - CI: Revert to single job with setup-go, no matrix
… container The Docker compose service names (flagr_with_sqlite:18000) are only resolvable from within the Docker network. Running go test on the host runner couldn't resolve them. Fix: add back the golang integration-runner container, mount source code, run go test via 'docker compose exec -e FLAGR_SERVER_URLS' which inherits the env var from the host shell.
…working_dir to /src/integration_tests
…uction URLs in FLAGR_SERVER_URLS already include :18000, but TestMain was adding another :18000, producing http://flagr_with_sqlite:18000:18000 which is an invalid URL. This caused all tests to fail with 'unsupported protocol scheme empty' since baseURL was set to the invalid URL but http.NewRequest stripped it. Fix: just prepend 'http://' since the URLs already have ports. Also simplify: env var is now baked into docker-compose.yml, Makefile test target is just 'docker compose exec go test'.
…erride The env var approach (docker-compose.yml environment, docker compose exec -e, docker compose run, sh -c) all failed to pass FLAGR_SERVER_URLS into the container. The tests compiled and ran but baseURL was always empty. Fix: hardcode the 6 Docker instance URLs directly in the Go code. TestMain uses these by default. If FLAGR_SERVER_URLS or FLAGR_SERVER_URL is set, those override the hardcoded defaults. For local mode, startLocalServer still works as before.
The Docker container approach (integration-runner) was failing because source mounting and env var passing were unreliable across Docker Compose v2 versions. Tests compiled and ran but baseURL was always empty. Fix: add port mappings to each flagr service in docker-compose.yml (e.g., 18001:18000 for sqlite, 18002:18000 for mysql, etc.) and run 'go test' from the CI runner host. Tests connect via localhost:<port>. Changes: - docker-compose.yml: Add ports to flagr services, remove integration-runner - Makefile: Set FLAGR_SERVER_URLS with localhost ports, run go test from host - integration_server.go: Remove debug code, clean up TestMain
TestMain must be in a _test.go file for Go's test framework to detect and call it. The init() function was running but TestMain was silently ignored because the file wasn't a test file.
The root cause of ALL CI failures was that TestMain was defined in integration_server.go (not a _test.go file). Go's test framework only recognizes TestMain in _test.go files. After renaming to integration_server_test.go, TestMain is properly called. The env var FLAGR_SERVER_URLS is correctly passed from the Makefile shell prefix to the go test process. Tests connect to flagr instances via localhost port mappings (18001-18006).
Structural improvements: - Define typed API response structs (flagResponse, segmentResponse, variantResponse, constraintResponse, distributionResponse, tagResponse, evalResponse, batchEvalResponse) — eliminates all map[string]any and unsafe type assertions in tests - Move seedFlags into seed.go — makes seed module self-contained - Extract pollUntil helper — eliminates duplicate waitForServer/waitForEvalCache polling loops - Extract prepareServer() from TestMain — deduplicates the two branches - Delete doReqOrDie — seedFlags uses doReqAndDecode directly with testing.TB - Delete jsonInts — use json.Marshal(seedFlagIDs) instead - Table-drive TestIntegration_BatchEvalOperator — 80 lines → 25 - Remove version: 3.6 from docker-compose.yml (deprecated by Compose V2) - waitForEvalCache now fails on timeout instead of silently continuing - Fix server cleanup: Kill + WaitDelay instead of Interrupt, redirect server output to temp file to avoid 'I/O incomplete' errors - Drain response body in TestIntegration_Export to prevent broken pipe - Benchmarks use shared doReq helper with status code checking Files changed: - integration_client.go: response structs, pollUntil, cleaned HTTP helpers - seed.go: seedFlags moved in with testing.TB parameter - integration_server_test.go: prepareServer, cleaner TestMain - integration_test.go: typed structs, table-driven subtests - integration_benchmark_test.go: shared helpers, status checking - docker-compose.yml: removed deprecated version field
- Unify seedFlags/seedFlagsFromMain into single function taking errorf param (deletes ~70 lines of duplication) - Make pollUntil return error instead of relying on callback to terminate - Add doReqOK helper for non-decoding 2xx checks - Use doReq in waitForServer/waitForEvalCache instead of hand-rolled http.Get - Remove sleep 20 from Makefile, let test harness poll via waitForServer - Set EntityType default in initFlagDefs, simplify seedFlags condition - Replace raw JSON strings in benchmarks with structured map bodies - Clean dead imports (log, testing, io, net/http, encoding/json)
…ad check - Delete benchEvalRaw (dead code after structured body refactor) - Add assertion for entity_types response in FlagCRUD test - Add assertion for preload query results in FlagCRUD test - Change t.Log to t.Errorf in Preload test (unexpected segments is a bug, not a note)
- Use b.Loop() instead of b.N in benchmarks (Go 1.24+) - Safe type assertion for TCP address in startLocalServer - Document test ordering dependency at package level - Add missing return after decode error in doReqAndDecode
zhouzhuojie
force-pushed
the
zz/rewrite-integration-test
branch
from
26aa868 to
a2b032a
Compare
- Add bench and test-and-bench targets to integration_tests/Makefile - Extract shared SERVER_URLS variable and up target - CI integration_test job now uses test-and-bench - Benchmark results framed with clear headers for grep-ability in logs
prepareServer now checks if flags exist before seeding. This allows benchmarks to run after tests against the same servers without hitting UNIQUE constraint violations.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Replace the shell/shakedown integration tests (
integration_tests/test.sh) with a native Go test suite. The suite seeds ~50 realistic flags covering all 12 constraint operators as HTTP API fixtures, provides 12 test functions and 12 HTTP benchmarks, and supports three execution modes.File split
integration_test.gowas 1034 lines bundling 4 distinct responsibilities. Split into focused files:integration_server.goTestMain, server lifecycle,seedFlagsintegration_client.gointegration_benchmark_test.gointegration_test.goseed.goinitFlagDefs()with flat dataSeed data cleanup
init()side effect with explicitinitFlagDefs()callopGroupnested loop machinery to a single[]entryliteralmatchingEntity()method and bespokeitoahelperShell fallback removed
integration_tests/test.shand alltest-shellrefsCode quality fixes
BenchmarkEvalDisabledFlagnow targets the actual disabled flagserverCmd+deferwaitForEvalCachechecks for non-empty datastartLocalServeralways builds fresh to temp dirTestIntegration_Evaluationusest.RunsubtestsINSTANCE ?= flagr_with_sqlitedefault to Makefileintegration.testandintegration.test.loggitignoredMotivation and Context
The shell-based integration tests (
test.sh) were slow, fragile, invisible togo test, and produced no timing metrics. Porting to Go gives us nativego testintegration, structured assertions via testify, HTTP benchmarks viaBenchmarkXxx, and reliable seeding with polling instead ofsleep 5.Splitting the monolith was driven by a code quality review that flagged the file at 1034 lines (71 shy of the 1k threshold) bundling server lifecycle, HTTP helpers, test functions, and benchmarks into one file.
Closes: this replaces the old integration test approach entirely; no corresponding issue.
How Has This Been Tested?
go vet -tags=integration ./integration_tests/— cleanmake test-integration— auto-starts server, runs all 12 test functions against SQLitego test -tags=integration -bench=. -benchmem ./integration_tests/— all 12 benchmarks runmake test(unit tests) — unaffectedmake test-e2e— unaffectedintegration_testjobTypes of changes
Checklist:
close #700