Skip to content

ci: restore a real integration gate; replace the escape hatch that never worked - #124

Merged
bfarmer67 merged 1 commit into
mainfrom
devs/bfarmer/restore-ci-integration-tier
Aug 26, 2026
Merged

bfarmer67 merged 1 commit into
mainfrom
devs/bfarmer/restore-ci-integration-tier

Conversation

@bfarmer67

@bfarmer67 bfarmer67 commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

Stacked on #123 (-> #122 -> #121). GitHub retargets as each merges. Review the fourth commit only.

The problem

Since 7ff3808, twelve days before 3.1.0, this repo has had zero integration coverage in CI and no working way to restore it. Both defects fixed earlier in this stack shipped through that hole.

That commit tagged every container-spinning class [TestCategory("LocalOnly")] for good reasons. But the CI job filtered negatively on TestCategory!=LocalOnly, so the filter then matched nothing, and dotnet test fails on a no-match. Instead of reconciling the filter with the tags, the job was disabled behind a repo variable.

Three things went wrong, all verified rather than inferred:

  1. The variable was never created. The repo has exactly one Actions variable, SOLUTION_NAME. The condition is false by absence.
  2. The documented way back does not work. Setting RUN_HEAVY_INTEGRATION=true un-skips a matrix that still carries TestCategory!=LocalOnly, so every cell matches zero tests and fails - the exact failure the gate was added to avoid. Measured:
    No test matches the given testcase filter
    `(FullyQualifiedName~OpenSearch)&TestCategory!=LocalOnly&TestCategory!=MultiNode`
    
  3. A negative filter cannot fail loudly. It reads the same whether the suite is passing, empty, or entirely untagged. That is why nobody noticed for three months.

What this delivers

115 integration tests gate every PR, ~2 minutes, selected positively on TestCategory=Gating:

Cell Tests Local GitHub runner
opensearch 88 1m02s verified
mongodb 10 13s verified
postgres 9 22s verified
aerospike 6 11s verified
multi-provider 2 4s verified

Verified green on GitHub-hosted runners across four separate dispatched runs, alongside unit tests on net8/9/10.

Every class was measured against explicit criteria rather than assumed heavy: shared assembly-fixture container only, no Docker image build, not multi-node, cell finishes in about a minute, observed green. No Docker image builds in the PR path, so the MCR/Azure-Front-Door throttle failure class is removed there rather than mitigated.

The zero-match trap cannot recur. A guard step runs --list-tests with the same filter before any container starts and fails by name if a cell matches nothing. Losing coverage now costs a red build instead of silence.

Every test is accounted for

136 integration tests, partitioned with nothing orphaned:

Tier Tests Runs
Gating 115 Every PR
LocalOnly 12 Manual dispatch
MultiNode 6 Existing manual workflow
Flaky 3 Local only - quarantined, see below

RUN_HEAVY_INTEGRATION is deleted, replaced by a Heavy Integration Tests workflow with working workflow_dispatch inputs for provider and framework. It selects positively on TestCategory=LocalOnly, so it runs exactly what the PR gate does not. Where a provider has nothing left in that category (opensearch, multi-provider - everything got promoted), the job skips with a notice rather than failing. That asymmetry is deliberate: an empty match in the PR gate means coverage vanished and must be loud; in the heavy suite it means coverage was promoted and is a success.

What I found by measuring, and what I backed out

I built this to also run post-merge on every push to main, on the reasoning that "run them locally" is a convention and conventions do not catch regressions. I removed that trigger before shipping, because measuring the suite first showed it is not dependably green:

  • AerospikeRunnerTest.Should_Succeed_WhenRunningUpTwice failed in a batch run (52s) and passed standalone (12s) minutes later. Intermittent; cause not isolated, plausibly container-teardown timing from the preceding provider.
  • Should_Fail_WhenMigrationHasLock in the Aerospike, MongoDB and Couchbase runner suites asserts on a race - it starts concurrent runner containers and requires one to observe lock contention, which does not happen on a fast host. Aerospike and MongoDB were measured failing with no defect present. Now quarantined as [TestCategory("Flaky")].

Notably, the Postgres equivalent of that test was already commented out by an earlier author. Same conclusion, reached silently. The quarantine makes it explicit and records the intended fix.

Enabling post-merge on a suite that is not dependably green produces a signal people learn to ignore, and an ignored signal is worse than an absent one - which is the same argument this PR's ADR makes for keeping the PR gate small and trustworthy. The trigger is three commented lines in the workflow, to be enabled once the intermittency is fixed. Tracked in ADR-0031's Consequences rather than dropped.

So this PR is honest but unfinished on that axis: the PR gate is real and verified; the heavy suite has a working trigger but still needs a human to pull it.

Docs

  • ADR-0031 - three tiers (unit / wire / integration), membership criteria, measured per-cell numbers, and the measured reasons the heavy suite is not yet automated. Amends ADR-0010, whose two-tier framing never named the wire tier and no longer described the running system.
  • CONTRIBUTING - tier table, criteria, the Flaky bar ("debt, not a category for tests that annoy me"), and "reach for the wire tier first".
  • docs/site/troubleshooting.md - symptom entries for both shipped defects. Anyone on 3.0.0/3.1.0 will search the literal string Index name is null for the given type; the MongoDB one has no error text at all, so it is indexed by its symptom.
  • docs/site/couchbase.md - the pinned ledger serializer.
  • docs/site/advanced.md - the two ledger wire-contract rules for custom record stores.
  • The Jekyll site is ASCII-only, matching every other page.

@bfarmer67
bfarmer67 force-pushed the devs/bfarmer/opensearch-connection-settings-hook branch from 80a9fdf to 2f23076 Compare August 26, 2026 17:31
@bfarmer67
bfarmer67 force-pushed the devs/bfarmer/restore-ci-integration-tier branch from 06a3bd7 to df354fa Compare August 26, 2026 17:32
@bfarmer67
bfarmer67 force-pushed the devs/bfarmer/opensearch-connection-settings-hook branch from 2f23076 to 83bc0ba Compare August 26, 2026 19:36
@bfarmer67
bfarmer67 force-pushed the devs/bfarmer/restore-ci-integration-tier branch 4 times, most recently from 025d43f to 1742886 Compare August 26, 2026 21:24
@bfarmer67
bfarmer67 force-pushed the devs/bfarmer/opensearch-connection-settings-hook branch from 83bc0ba to b32e1e5 Compare August 26, 2026 21:52
@bfarmer67
bfarmer67 force-pushed the devs/bfarmer/restore-ci-integration-tier branch from 1742886 to e202e0c Compare August 26, 2026 21:52
@bfarmer67
bfarmer67 force-pushed the devs/bfarmer/opensearch-connection-settings-hook branch from b32e1e5 to 524d587 Compare August 26, 2026 21:53
@bfarmer67
bfarmer67 force-pushed the devs/bfarmer/restore-ci-integration-tier branch from e202e0c to 3a8f357 Compare August 26, 2026 21:53
…ver worked

Since 7ff3808 (twelve days before 3.1.0) this repo has had zero integration
coverage in CI, and no working way to get it back. Both defects fixed
earlier in this stack shipped through that gap.

How it got there. That commit tagged every container-spinning class
[TestCategory("LocalOnly")] for sound reasons -- the heavy suite does not
gate the NuGet publish, and the recurring infra flakes (Couchbase GSI
indexer variance, Azure Front Door blocking MCR under ~36 simultaneous
base-image pulls) all live there. But the CI job filtered NEGATIVELY on
TestCategory!=LocalOnly, so the filter then matched nothing, and
`dotnet test` fails on a no-match. Rather than reconcile filter and
categories, the whole job was gated behind repo var RUN_HEAVY_INTEGRATION.

Three consequences, none intended:

  - The variable was never created. The repo has one Actions variable,
    SOLUTION_NAME. The condition is false by absence.
  - The documented way back does not work. Both the workflow comment and
    the commit message claim re-enabling needs "no code change". Measured:
    the job it re-enables still carries TestCategory!=LocalOnly, so setting
    the variable to true un-skips a matrix whose every cell matches zero
    tests and fails on the no-match -- exactly the failure the gate was
    added to avoid.
  - A negative filter cannot fail loudly. It reads identically whether the
    suite is passing, empty, or entirely untagged.

Every integration class was then measured against explicit criteria rather
than assumed heavy. Every integration test in the repository now runs
automatically:

  - 110 tests on every PR (~2 min), selected POSITIVELY on
    TestCategory=Gating: opensearch 83, mongodb 10, postgres 9,
    aerospike 6, multi-provider 2. All verified green locally and on
    GitHub-hosted runners. No Docker image builds in the PR path, so the
    MCR/AFD failure class is removed there rather than mitigated. One TFM:
    these assert behavior against a real server, which does not vary by
    target framework, and the unit + wire tiers already cover all three.

  - 27 tests on every push to main: the LocalOnly remainder -- 5 runner
    classes plus CliBinaryEndToEndTests (Docker image builds), 2 multi-node
    classes (3 JVMs), and 3 Couchbase squash classes. Couchbase is the only
    exclusion on speed: its six tests pass, repeatedly, but take 5-6.5
    minutes, five to six times every other cell combined, because
    IsolatedCouchbaseContainer waits out the GSI indexer's initial rebalance
    (the code allows up to 12 minutes for it).

A guard step runs --list-tests with the same filter before any container
starts and fails by name if a cell matches zero tests, so losing coverage
costs a red build instead of silence.

RUN_HEAVY_INTEGRATION is removed. heavy_integration_tests.yml runs the
LocalOnly suite post-merge and on demand, selecting positively on
TestCategory=LocalOnly so it runs exactly what the PR gate does not.
Post-merge rather than manual-only because "run it locally" is a convention,
and conventions do not catch regressions; tying it to a push detects a break
within one commit and attributes it to that commit. Not a nightly cron: a
scheduled run that fails on a day nobody changed anything produces no signal,
which is the mistake multi_node_tests.yml already corrected.

Docs updated alongside:

  - ADR-0031 records the three tiers (unit / wire / integration), the
    membership criteria, the measured per-cell numbers, and the three
    triggers. Amends ADR-0010's two-tier framing and names the wire tier
    it omitted.
  - CONTRIBUTING gains the tier table and "reach for the wire tier first".
  - docs/site/troubleshooting.md gains symptom entries for both shipped
    defects -- the OpenSearch "Index name is null" serialization failure and
    the silent MongoDB squash no-op -- since users on 3.0.0/3.1.0 will search
    for exactly those.
  - docs/site/couchbase.md documents the pinned ledger serializer.
  - docs/site/advanced.md gains the two ledger wire-contract rules for
    authors of custom record stores, plus the wire-test guidance.
  - The Jekyll site is ASCII-only, matching every other page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bfarmer67
bfarmer67 force-pushed the devs/bfarmer/restore-ci-integration-tier branch from 3a8f357 to 249092c Compare August 26, 2026 21:55
@bfarmer67
bfarmer67 changed the base branch from devs/bfarmer/opensearch-connection-settings-hook to main August 26, 2026 21:55
@bfarmer67
bfarmer67 merged commit 325e049 into main Aug 26, 2026
2 checks passed
@bfarmer67
bfarmer67 deleted the devs/bfarmer/restore-ci-integration-tier branch August 29, 2026 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant