Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
d7ded1c
docs(tests): audit every test that does not run, assert, or exist
astubbs Aug 7, 2026
46327fc
docs(ledger): fix the two entries that hid the test-hardening audit
astubbs Aug 7, 2026
241c1de
fix(docs): correct eight counts an adversarial fact-check refuted
astubbs Aug 7, 2026
70aae90
docs(tests): correct the audit, and give all ten deleted stubs a disp…
astubbs Aug 7, 2026
cadf4c9
test: delete four tests that never ran, never passed, or never checke…
astubbs Aug 7, 2026
e67d8b8
test: delete dead commented-out alternatives, changing no live value
astubbs Aug 7, 2026
e21a802
test(offsets): check the five codecs OffsetEncodingTests was branchin…
astubbs Aug 7, 2026
e22dae1
docs(tests): qualify the issue references the gate flagged
astubbs Aug 7, 2026
0bc57bb
Merge remote-tracking branch 'origin/chore/audit-inactive-tests' into…
astubbs Aug 7, 2026
d950cd5
docs: qualify the remaining issue references the gate flagged
astubbs Aug 8, 2026
f5e72f4
Merge remote-tracking branch 'origin/master' into chore/audit-inactiv…
astubbs Aug 12, 2026
0e62586
Merge branch 'chore/audit-inactive-tests' into test/inactive-test-rem…
astubbs Aug 12, 2026
2d5c5b3
docs(inflight): record the state of the #263/#264 inactive-test stack
astubbs Aug 12, 2026
58520ec
docs(agents): put the inactive-test audit pointer where R19 asked for it
astubbs Aug 12, 2026
69a613e
docs(inflight): qualify the stack's own issue references
astubbs Aug 12, 2026
1e9cf91
Merge branch 'chore/audit-inactive-tests' into test/inactive-test-rem…
astubbs Aug 12, 2026
0a6e11f
docs(tests): record which commented-out lines were ever live, and whi…
astubbs Aug 12, 2026
26da0ed
docs(tests): record that LoadTest's `8` rung could never have run
astubbs Aug 12, 2026
37e0bb1
test(load): make the volumes LoadTest was run at by hand reachable again
astubbs Aug 12, 2026
6497b4a
test(transactions): make the concurrency ladder reachable, and its lo…
astubbs Aug 12, 2026
a462a82
test(volume): reach the 2M this test was meant to, and never did
astubbs Aug 12, 2026
138eede
test(multi-instance): the 10M was never too big - the deadline was to…
astubbs Aug 12, 2026
110f806
test(probe): automate the environment inspection JavaEnvTest did by hand
astubbs Aug 12, 2026
7ee836a
test(vertx): assert what a 5xx does, which nothing did before
astubbs Aug 12, 2026
66d8455
refactor(tests): one completion ceiling, and fix the anchor LoadTest …
astubbs Aug 12, 2026
cac1599
test: apply code review - unit-test the ceiling, isolate the racing t…
astubbs Aug 12, 2026
3e659fc
chore: take the rename tooling from master
astubbs Aug 12, 2026
3974fa6
chore: take the rename tooling from master
astubbs Aug 12, 2026
a17d96c
fix: point the KafkaSanityTests javadoc at the current BrokerPollSystem
astubbs Aug 12, 2026
5351788
test: point the KafkaSanityTests javadoc at the current BrokerPollSystem
astubbs Aug 12, 2026
dfd3e94
refactor: move io.confluent.* to bz.stub.* (pure rename, no content c…
astubbs Aug 12, 2026
e7a4f4d
refactor: rename io.confluent.* references to bz.stub.* (content only)
astubbs Aug 12, 2026
f434fcb
refactor: move io.confluent.* to bz.stub.* (pure rename, no content c…
astubbs Aug 12, 2026
4ee4d8a
refactor: rename io.confluent.* references to bz.stub.* (content only)
astubbs Aug 12, 2026
cc03aa8
Merge remote-tracking branch 'origin/master' into chore/audit-inactiv…
astubbs Aug 12, 2026
0b98a00
Merge remote-tracking branch 'origin/master' into test/inactive-test-…
astubbs Aug 12, 2026
ca8d4a3
test(probe): lock every autopsy test against the dump guard, not just…
astubbs Aug 12, 2026
1a178b8
Merge branch 'chore/audit-inactive-tests' into test/inactive-test-rem…
astubbs Aug 12, 2026
a69cd34
Address PR review feedback (#264)
astubbs Aug 12, 2026
9495f9d
docs: cut the three landed plans, salvage what outlived them, drop a …
astubbs Aug 12, 2026
b16c021
Merge remote-tracking branch 'origin/master' into test/inactive-test-…
astubbs Aug 12, 2026
dbbfd62
test(core): re-enable the two long-dark tests, and write the produce-…
astubbs Aug 13, 2026
e8c9bb1
fix(test): assert the commit frontier in processInKeyOrder, not the p…
astubbs Aug 13, 2026
2d18733
docs(inflight): shrink the stack note to what is still open, and rena…
astubbs Aug 13, 2026
689b7b5
test(load): measure the 40,000 rung before leaving it automatic, and …
astubbs Aug 13, 2026
1887362
fix(test): correct the key attribution in processInKeyOrder's contigu…
astubbs Aug 13, 2026
88b0b1e
docs: retire the flake astubbs#260 fixed, and sweep the strict call s…
astubbs Aug 13, 2026
8c01831
refactor(test): delete the helper this PR stranded, and stop hand-rol…
astubbs Aug 13, 2026
be8194d
fix(test): stop asserting the tick path in the sibling test, and clos…
astubbs Aug 13, 2026
970c521
docs(inflight): file the review follow-ups, and delete the note that …
astubbs Aug 13, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions CONCEPTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ parallel. This is how the project gets concurrency without adding partitions.
Records handed to the worker pool and not yet resolved as succeeded or failed. Distinct from records
merely fetched: in-flight work is what a commit must wait for, and what a shutdown must drain.

**Commit frontier**
The offset a partition would resume from if consumption restarted — the highest offset committed for
it. It is *exclusive*: it names the next record expected to be polled, not the last one completed.

The frontier advances only across a contiguous run of completed records, so a completed record sitting
above an in-flight one does not move it. A partition can therefore have most of its work done and still
be committing its starting offset. That property is also what makes the frontier the thing worth
asserting about a commit: which intermediate offsets a partition commits on the way there depends on
when the periodic commit happens to fire, but where it ends up does not.

## Transactional commit

**Produce lock**
Expand Down Expand Up @@ -83,6 +93,16 @@ A test that awaits a consequence whose precondition it cannot guarantee will occ
load-tightness flake: the margin is not too small, the awaited event may simply never happen in some
interleavings, so no timeout is long enough to make the test reliable.

**Tick-path assertion**
A test assertion that pins the intermediate observations a system passed through on its way to a settled
state, rather than pinning the settled state itself. Because those intermediates are only captured when
a periodic action happens to fire, the assertion's truth is a property of the machine's speed rather
than of the code under test — green on an idle box, red under load, with both readings correct.

Distinct from a load-tightness flake and an unforceable trigger: no deadline is too tight, and no
awaited event is missing. The awaited condition is one of several legitimate outcomes, and once a
different one has occurred it can never become true, so the wait always expires in full.

**Ambient probe**
The always-on recorder attached to broker integration tests, which annotates a failure with
consumer-group progress evidence so the contention-versus-product-bug question is answered before
Expand All @@ -96,3 +116,7 @@ test in question — a short, low-volume test cannot trip them, and a clean read
a real deadline missed under contention, and an unforceable trigger is an awaited event that never
occurred. All three present as the same expired await, and the whole diagnostic difficulty of this
area is telling them apart.
- **A tick-path assertion presents as that same expired await, and is the fourth member of the
confusion.** It is told apart by asking whether what the test actually saw is *also correct*: the
other three all mean the expected thing did not happen, while a tick-path assertion means something
equally valid happened instead.
11 changes: 6 additions & 5 deletions bin/todo-index.sh
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,13 @@ count_markers() {
}

generate() {
local count
count=$(count_markers)

cat <<EOF
# TODO index

**Generated file - do not edit by hand.** Regenerate with \`bin/todo-index.sh\`
(\`bin/todo-index.sh --check\` fails if this file is stale).

Every \`TODO\` / \`FIXME\` / \`XXX\` marker in the tracked tree, grouped by module. **$count marker(s)** at
the time of generation.
Every \`TODO\` / \`FIXME\` / \`XXX\` marker in the tracked tree, grouped by module.

## Prioritised? See the refactoring backlog

Expand All @@ -158,6 +154,11 @@ grep -rn "check legacy is recursive"
A line number would be wrong within a day and would drag this file into every unrelated diff. The
marker's own text is stable until someone edits the marker.

For the same reason this file carries no marker **count**: it would be a second, drifting statement
of something the list below already says exactly, and the two would disagree the moment either moved.
Count the entries if you need a number. \`bin/todo-index.sh\` prints one to the console when it runs,
where it cannot go stale.

## How to use this

Markers here are *not* a backlog - most are notes-to-self left next to the code they concern, and
Expand Down
12 changes: 9 additions & 3 deletions docs/inflight/branch-stale-and-diagnostic.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,15 @@ the only copy of a 455-line audit of disabled, kneecapped and weakened tests, co
triaged" - and neither this entry nor its `docs/refactoring.md` line mentioned it, so it sat on the
delete list with unique content on it. That audit is now absorbed into
[`docs/test-hardening/inactive-tests-audit-2026-08-08.md`](../test-hardening/inactive-tests-audit-2026-08-08.md),
with the two reasons its own git history refutes corrected. Its other commits (restore
`LargeVolumeInMemoryTests` to 1M, capture the OOM diagnostics) are still unsalvaged - take those
before deleting.
with the two reasons its own git history refutes corrected.

Of its other two commits, the **OOM diagnostics are now salvaged** to
[`docs/test-hardening/large-volume-in-memory-tests-oom-diagnostics-2026-04-22.md`](../test-hardening/large-volume-in-memory-tests-oom-diagnostics-2026-04-22.md) -
they are the only measured evidence that `LargeVolumeInMemoryTests` at 1M exhausts the heap in the
close path, so they had to survive the branch. The **restore-to-1M commit is still unsalvaged**, and
should not simply be cherry-picked: those same diagnostics show it OOMs as written. Scope for doing
it properly is in [`docs/refactoring.md`](../refactoring.md). Take that commit before deleting the
branch.

**Do not delete `master-confluent`** (pinned at pre-rebrand `7f290122`): it is ruleset-protected and
is the base of astubbs#29 and astubbs#31 - retarget those first. `dev-cc` is pinned at the same commit.
Expand Down
91 changes: 91 additions & 0 deletions docs/inflight/test-inactive-test-review-followups.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# Review follow-ups left open from the inactive-test work

A seven-reviewer pass over astubbs#264 found four P1s, all fixed there (two of them reproduced by *running*
the tests: a 3-in-10 tick-path flake and a 1-in-10 worker-scheduling race). These are the findings that were
deliberately **not** folded into that PR, because each is a change to shared machinery rather than a fix to
the work under review.

## The frontier assertion should live in the shared test base

`ParallelEoSStreamProcessorTest` has private `awaitFrontier(int partition, long expected)` and
`highestCommitFor(TopicPartition)`. Every sibling in that family - `assertCommits`, `assertCommitLists`,
`awaitForCommit`, `awaitForCommitExact(int)` - lives in `AbstractParallelEoSStreamProcessorTestBase`, and is
reused across modules (`VertxTest` calls straight into it).

The irony is recorded in the learning this work produced
([`assert-the-commit-frontier-not-the-tick-path`](../solutions/test-flakiness/assert-the-commit-frontier-not-the-tick-path.md)):
that doc's whole thesis is that astubbs#260's rule failed to generalise because it lived somewhere you only
read if you already held the helper - and the fix for it was then put somewhere only one test class can reach.

Move both to the base, next to `awaitForCommit`, building on the already-mode-agnostic `getCommitHistory()`
that `highestCommitFor` calls anyway. Two blind alleys, already checked so they are not re-explored:

- **`awaitForCommitExact(int, int)`** was the obvious existing home and was deleted in astubbs#264 as dead. It
verified `sendOffsetsToTransaction` on the producer spy, so it only ever worked under
`PERIODIC_TRANSACTIONAL_PRODUCER` - useless for a test parameterised over all three commit modes.
- **`hasCommittedToPartition(tp)`** (the truth-subject path used by `MultiTopicTest`, `MutinyPCTest`,
`TransactionTimeoutsTest`) reads the mock **consumer's** history, and under transactional mode commits go to
the *producer*. That is precisely why `getCommitHistory()` dispatches on commit mode.

Neither spans the three modes. The new helper does, which is why it exists rather than reusing them.

## The frontier's negative checkpoints sample rather than hold

`await().untilAsserted` returns on its first passing poll, so a checkpoint that asserts "this partition has
**not** advanced" proves only the instant it ran. The exact-history form it replaced constrained continuously,
and the `verify(producerSpy, after(verificationWaitDelay).never())` it replaced before that supplied a real
settle window.

Concretely: a commit landing strictly between two checkpoints - say partition 1 committing 5 or 6 while record
4 is still in flight - is invisible, because `max` at the later checkpoint is the same either way. Committing
past in-flight work is exactly what `processInKeyOrder` exists to catch.

The fix is a holding variant used only at the "must not advance" checkpoints, keeping `awaitFrontier` for the
rising ones:

```java
private void awaitFrontierAndHold(int partition, long expected) {
awaitFrontier(partition, expected);
var tp = new TopicPartition(INPUT_TOPIC, partition);
await().during(ofMillis(verificationWaitDelay)).atMost(defaultTimeout)
.untilAsserted(() -> assertThat(highestCommitFor(tp)).hasValue(expected));
}
```

`verificationWaitDelay` is two commit intervals - the same budget the deleted `never()` form had. This restores
the settle without reintroducing the tick-path sensitivity the frontier change removed. Do it with the hoist
above, not separately.

## `CompletionCeiling`'s overflow guard is stricter than `Duration` needs

The guard tests whether `units * ceilingNanos` overflows a long, but the value produced is
`units * ceiling / gatingUnits`, and `Duration.multipliedBy` is `BigDecimal`-based. So it rejects knob values
whose *result* is perfectly representable, and blames the operator: `-Dvolume.messages=100000000` against the
120s site throws `"size ... is too large to scale a PT2M ceiling - check the knob"` for a valid 12,000s
deadline. The documented rungs (400k, 2M, 10M) all sit well under it, so nothing configured today trips it -
roughly 15x headroom above the largest.

Guard the operation that can actually overflow (catch `ArithmeticException` around the `Duration` arithmetic
and rethrow with the same friendly message) rather than a nanosecond proxy. Also add a positive-`ceilingAtGating`
check: `Duration.ZERO` currently divides by zero *inside* the guard meant to prevent raw arithmetic errors.
`CompletionCeilingTest` pins only the far side of the boundary, so the false-rejection band is untested.

## `CompletionCeilingTest` does not mirror the one inverted call site

The test's own javadoc says it "mirrors the real call sites, so a change to any of their constants breaks here
first", and it pins three of four. The one it skips is `TransactionAndCommitModeTest.timeoutFor`, the only site
whose arguments are inverted - `completionCeiling(GATING_CONCURRENCY, threads, defaultTimeout)` - because more
threads must *shorten*, not lengthen. That inversion is exactly where an argument-order slip is invisible, and
it is the site whose hand-rolled predecessor was provably wrong. Add the fourth mirror, including a rung case.

## Inherit: the audit is a dated record and has drifted

`docs/test-hardening/inactive-tests-audit-2026-08-08.md` records
`grep -c "^- \[ \]" docs/quarantined-tests.md # 0`; that returns **3** today, because the registry gained
entries after 2026-08-08. Left as-is deliberately - a dated record's claims are not rewritten, only references
that no longer resolve (`docs/citations.md`). Re-run its reproduction commands rather than trusting the
numbers, and read the date first.

A reviewer also flagged that the corrections made to that audit were written **into** the dated file rather
than into a new dated audit, so it now carries later conclusions under an earlier date. Worth a decision if
that file is edited again.
46 changes: 46 additions & 0 deletions docs/inflight/test-required-perf-lane-scope.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# What belongs in the required Performance Tests lane

`bin/performance-test.sh` is the "Performance Tests" leg of `maven.yml` - **a required check on every PR**,
`ubuntu-latest`, `timeout-minutes: 60`, and no retry (`AGENTS.md`: a flake fails the build, deliberately).
Anything `@Tag("performance")` therefore blocks merges. Two questions about that scope are open.

## Decided: the 40,000-message rung stays automatic

`LoadTest.asyncConsumeAndProcessAtVolume` runs there at `HIGH_VOLUME_TOTAL` (40,000), ten times the gating
volume, on a class listed at **1/20, undiagnosed** in the load-tightness flake family
([`test-load-tightness-flakes.md`](test-load-tightness-flakes.md)) at its *gating* volume.

Measured before leaving it automatic, per the `AGENTS.md` rule to separate contention from a real
concurrency bug: on an uncontended broker it ran **5/5 green at ~52s against a derived 600s ceiling**. So the
code is fine at this volume and nowhere near its deadline. It then passed the real CI lane on first run.
Left automatic on that evidence.

## Open: the lane pulls in a Postgres container nothing there uses

`LoadTest extends DbTest`, and `DbTest` starts a `PostgreSQLContainer` in a **static initialiser**. Tagging
one method `performance` is enough to class-load it, so from astubbs#264 onward the required lane starts and
holds a database that no test in it reads or writes - `asyncConsumeAndProcess`'s only DB call is commented
out. `DbTest` is itself listed at **2/20** in the same flake family, for "postgres container start under
contention".

So the lane inherited a known-flaky container start, and its memory, to run a test that never touches a
database. Options:

1. **Move the volume case to a class extending `BrokerIntegrationTest`** rather than `DbTest`, leaving the
DB-shaped tests behind. Cleanest; no test in `LoadTest` appears to need Postgres.
2. **Make `DbTest`'s container lazy** (behind an accessor rather than a static initialiser) so class loading
alone does not pay for it. Helps every DB-shaped test, not just this one.
3. **Leave it** and watch, as with the volume decision.

Related, and worth deciding together: the lane declares no `forkCount`, so all four `@Tag("performance")`
classes run in **one fork**, sharing one heap and the static broker. They run *sequentially* - `-Pci` sets
`parallel-tests=false`, and failsafe's `<parallel>` element is not read by the JUnit Platform provider at
all - so this is a heap-and-wall-clock question, not a concurrency one. The gating integration lane already
forks (`-DforkCount=4 -DreuseForks=true`); this lane never got that treatment.

## Watch: `-Dload.total` moves the gating tests too

`LoadTest.total` and `asyncConsumeAndProcessAtVolume` read the **same** property. Reaching the documented top
rung with `-Dload.total=400000` also raises the two untagged gating tests to 400,000, each with a derived
100-minute ceiling against a 60-minute job cap. Fine when run deliberately outside CI; a trap if anyone puts
that flag in a lane.
17 changes: 5 additions & 12 deletions docs/inflight/test-untracked-ci-flakes.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,18 @@ and Unit lanes). 8 of 45 runs carried markers. None of these tests appear in any

The retry that hid them is gone - that half is done and written up in
[`docs/solutions/workflow-issues/ci-retries-hid-flakes-from-the-ledger-2026-08-07.md`](../solutions/workflow-issues/ci-retries-hid-flakes-from-the-ledger-2026-08-07.md),
which also has the scan method. What is open is the three tests themselves.
which also has the scan method. What is open is the tests themselves - two of the scan's three, plus
one met later. The scan's third,
`ParallelEoSStreamProcessorTest.queuedMessagesNotProcessedOrCommittedIfSubmittedDuringShutdown`
(3/45), is fixed and gone from this ledger: astubbs#260 established the extra commit was correct
product behaviour and the assertion was wrong, so no product change was needed.

| Test | Rate | Why it is worth attention |
|---|---|---|
| `OffsetEncodingBackPressureTest.backPressureShouldPreventTooManyMessagesBeingQueuedForProcessing` | 4/45 | The most frequent. UNDIAGNOSED but quarantined by explicit rule-1 exception - see below. Backpressure area - compare `vacuous-await-condition-brokerpoller-backpressure-2026-07-31.md`, a *different* class in the same area, so rule it in or out rather than assuming |
| `ParallelEoSStreamProcessorTest.queuedMessagesNotProcessedOrCommittedIfSubmittedDuringShutdown` | 3/45 | **A regression** - see below |
| `PCMetricsTest.metricsRegisterBinding` | 2 seen | Second sighting, mechanism known, quarantined (owner astubbs#265) - see below |
| `ProducerManagerTest.producedRecordsCantBeInTransactionWithoutItsOffsetDirect` | 1 seen (2026-08-12) | Not from the original scan - found while babysitting astubbs#287. Mechanism known and owned (astubbs#262), quarantined - see below |

**Start with the regression.** astubbs#101 fixed this exact test as "the shutdown-commit flake that
was aborting PIT", and it is back. It has the best starting position of the three: a known prior fix
to diff against, and a documented consequence -
`docs/plans/2026-08-03-001-investigate-transactional-commit-flake.md` §1 records that instability in
this test made the PIT mutation lane report *suite stability* rather than mutation coverage. While it
flakes, a green PIT lane means less than it appears to.

Failures surface as `AbstractParallelEoSStreamProcessorTestBase.assertCommits`, so the assertion
helper is where the message comes from, not necessarily where the cause is.

**Classify before touching any of them** - the same rule that governs the load-tightness family next
door, and for the same reason: two of that family turned out to be real product bugs, and the third
was neither tight nor a stall but a test that could not force its own trigger.
Expand Down
Loading
Loading