Integration Test Review

Integration test flakiness review

Integration tests in this repo run on real VMs against live ESS stacks, on hosts that vary in speed, distro, kernel, and concurrent load. The dominant failure mode is a test that passes locally and in the PR run, then flakes weeks later on a slower host, a different distro, or after an unrelated product change. This skill encodes the failure modes actually observed in a year of flake fixes (see references/flake-taxonomy.md for the evidence: every rule below cites real incidents).

Method

1. Read the diff. For each added or modified test function, also read enough surrounding file context to understand the test's setup, assertions, and cleanup — flakes usually live in the interaction, not the changed lines alone.
2. Run the Tier 1 mechanical scan over the changed files.
3. For each test, walk the Tier 2 semantic questions that match what the test does.
4. Check Tier 3 idioms: is the test hand-rolling something a shared helper does safely?
5. Report findings: file:line, category, why it will flake (concrete scenario on a slow/parallel/different-OS host), and the fix. Rank by likelihood × blast radius. A test that can leave an installed agent, a root-owned dir, or a mutated shared Fleet object behind poisons other tests — rank that above a self-contained flake.

Judge only realistic nondeterminism. Do not flag a wait as "racy" if the condition is polled with a generous budget; do flag budgets with no headroom over known product intervals (see Tier 2 §Timeouts).

Tier 1 — mechanical red flags (grep the diff first)

Each of these has caused real CI failures; they are close to always-wrong.

Tier 2 — semantic questions

Every Elasticsearch query the test asserts on

• Who generates the matching data, and is production guaranteed? A test that waits for ambient events (auditd, network traffic, host logs) it never causes will pass or fail with host activity. The test must trigger its own events — ideally inside the poll loop, so each retry tick regenerates the signal (rule activation, restarts, and slow pipelines are then covered automatically).
• Is the filter tight enough to match only that data? Broad exists: queries plus Hits[0] select an arbitrary ambient document. Filter on values the test controls: its own tag, namespace, process.name, generated message. If two documents are compared (e.g. process-runtime vs otel-runtime), both sides must provably be the same underlying event, and build-metadata / event-type-specific fields (agent.version, auditd.data.*) belong in the ignore list.
• Is a @timestamp >= watermark bound captured before the action that produces the document? Capturing time.Now() after Install/Enroll returns excludes events emitted during the call — the query can then never match. (This exact bug shipped twice.)

Every wait / Eventually

• Poll the value you assert, not a weaker proxy. File existence ≠ file has content; process exists ≠ upgrade progressing; Kibana 200 on a policy PUT ≠ agent applied it (gate on the applied policy revision instead).
• Read agent-local state locally. Fleet's agent document only advances on checkin, which the test cannot force. If the value exists in elastic-agent status / fixture.ExecStatus, poll that, never Kibana. This explicitly includes upgrade state: UpgradeDetails in Kibana's agent doc is a checkin-lagged mirror of agent-local state, not "Fleet-side state" — raising the timeout on the Kibana copy is the wrong fix when the local status has the value immediately.
• "Reset then assert absence" on async streams is a race. Draining a log buffer and requiring zero later occurrences fails when late writes land after the drain. Assert monotonic cumulative counts or post-marker occurrences instead.
• Drive state machines by observed condition, not magic counts. "Restart 3 times, sleep 10s" encodes internal timing; loop while the observable condition (reported version, state) still holds, under a bounded context.

Timeouts

• Compare every budget against the product interval that bounds it. Fleet checkin long-poll is 5 minutes: any wait for a Fleet-reported status transition must strictly exceed it (6m+). A budget equal to the interval has zero headroom and will flake on slow hosts.
• Nested budgets: an outer context must exceed the sum of the serial inner Eventually budgets it wraps.
• Big blind timeouts are a smell in both directions. For long operations (artifact downloads are ~450MB), don't just raise the number — poll the reported state (UpgradeDetails.State, DownloadPercent), fail fast on FAILED/ROLLBACK, detect stalls; then a generous ceiling is safe.

Health assertions

• Does the config intentionally break something? Unreachable output, invalid input, missing credentials — then global-HEALTHY is wrong by construction. Assert only the units the test controls (IsHealthyOrDegradedFromOutput, or per-unit checks), or give the test a real mock ES.
• Is the asserted field guaranteed populated? e.g. VersionInfo.Name is intermittently empty for some monitoring components; empty vs wrong must be distinguishable.

Environment & versions

• OS/distro/arch: kernel-coupled behavior (auditd, systemd, package managers) must pin the distros it was validated on in define.Requirements. Windows: file handles block deletion; slower hosts widen race windows.
• Runtime topology: don't assume beats run as separate processes (PID scraping, process-name regexes, process-only metric fields, one shared build hash). Pin _runtime_experimental explicitly when behavior depends on it; switch on VersionInfo.Name (beat-v2-client vs beats-receiver).
• Dynamically resolved versions: "latest snapshot" can equal the local build (rpm -U refuses; migration assertions become invalid) and "previous minor" may not exist for the platform (windows/arm64 < 9.3.0). Handle the equal-version case; gate on capability, not availability.
• Third-party fixtures: leave nothing security/protocol-relevant implicit (ssl_enabled => false if the client speaks plain TCP) — vendor defaults are computed from the host and change across versions.
• Exact product log strings in allow-lists/assertions silently break when the product rewords errors; prefer stable substrings or structured fields.

Shared state & cleanup (blast radius: other tests)

• Cluster-global ES objects (index templates, pipelines, policies) must be named per-test/namespace — a hardcoded shared name means the second test silently overwrites the first.
• Never mutate well-known shared Fleet objects (fleet-default-output) mid-suite on a shared stack; it bumps policy revisions for every enrolled agent. If unavoidable, do it before enrollment.
• Cleanup must survive the failure path: registered via t.Cleanup (LIFO), with credentials (uninstall tokens) re-fetched after the state changes they must survive. A failed cleanup that leaves an installed/tamper-protected agent or root-owned /tmp dir breaks unrelated later tests.
• Anything writing to stdout (progress bars, spinners) must be terminated — go test output is machine-parsed and pollution fails other tests.

Tier 3 — use the shared tooling

Hand-rolled equivalents of these have all caused flakes:

• Agent lifecycle: define.NewFixtureFromLocalBuild → Prepare/Configure → fixture.Install(...). Never PrepareAgentCommand + cmd.Start() + stdout scraping — it yields no diagnostics on failure and leaks agent processes.
• Config switching: fixture.Configure (reload) instead of teardown/restart.
• Logs: fixture.Exec(ctx, []string{"logs", ...}) instead of scraping process output.
• Health: fixture.IsHealthy only when everything is genuinely expected healthy; IsHealthyOrDegradedFromOutput when the output is intentionally absent.
• Policy propagation barrier: tools.IsMinPolicyRevision with the revision returned by UpdatePolicy.
• Placeholder ES: integration.StartMockES, not a dead localhost:9200 (and never status_reporting.enabled: false — it masks real product bugs).
• Doc comparisons across runtimes: reuse the established ignore-list machinery (assert_tools.go) rather than fresh map-diff code.

When in doubt about whether a pattern has bitten before, check references/flake-taxonomy.md — it maps every category to the commits/PRs that fixed real instances, useful for citing precedent in review comments.