Skip to content

Remove static state manipulation that tests use - breaks parallel testing - #126

Closed
Antony Stubbs (astubbs) wants to merge 1 commit into
confluentinc:masterfrom
astubbs:test-options
Closed

Remove static state manipulation that tests use - breaks parallel testing#126
Antony Stubbs (astubbs) wants to merge 1 commit into
confluentinc:masterfrom
astubbs:test-options

Conversation

@astubbs

Copy link
Copy Markdown
Contributor

No description provided.

Use test options class instead of public instance setters
@astubbs

Copy link
Copy Markdown
Contributor Author

Made redundant by #405

@astubbs
Antony Stubbs (astubbs) deleted the test-options branch October 20, 2022 17:45
Antony Stubbs (astubbs) referenced this pull request in astubbs/parallel-consumer Aug 1, 2026
… list

Correcting this PR's original shape. It shipped a docs/TODO_TRIAGE.md
that duplicated docs/refactoring.md - a 295-line backlog that already
had a release-gated 'Breaking changes queued for next major version'
section, a 'Remove static state (unblocks parallel test execution)'
section, and per-file entries for #233, the encoders and WorkContainer.
My triage re-derived all of that from scratch, with worse provenance:
refactoring.md carries the upstream issue numbers (confluentinc#405, #126, #143) and
design branches that I did not have.

The mistake was concluding 'no breaking-changes list exists' after
grepping only docs/inflight.md. Too narrow a search to support the
conclusion drawn from it.

So:
- delete docs/TODO_TRIAGE.md rather than maintain two taxonomies;
- fold into refactoring.md the two things it genuinely did NOT cover:
  the loop-cycle waits in ParallelEoSStreamProcessorTest (a known
  intermittent failure, where cycle-counting IS the flake mechanism),
  and the MockConsumer.groupMetadata() workaround duplicated across four
  example tests;
- point TODO_INDEX.md and docs/inflight.md at refactoring.md as the
  place triage happens, with an explicit 'do not start a parallel list'.

docs/TODO_INDEX.md still earns its place: refactoring.md's capture
convention is TODO(refactor):, of which there are 3, while the index
inventories all 91 plain markers - a population nothing was tracking.

AGENTS.md now says refactoring.md owns marker triage, and to read it
before concluding some category of deferred work is untracked, since it
is easy to miss by grepping only inflight.md. That note is the actual
fix for the mistake.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QqHpNSXC39ANv9kG1ZvUzn
Antony Stubbs (astubbs) referenced this pull request in astubbs/parallel-consumer Aug 3, 2026
…actoring backlog (#103)

91 TODO/FIXME/XXX markers were scattered across the tree with nothing
tracking them. This adds a generated inventory, and triages the useful
ones into the backlog that already exists rather than a new one.

docs/TODO_INDEX.md - generated inventory

bin/todo-index.sh walks the tracked tree and writes every marker,
grouped by module with file and line. --check exits non-zero when the
committed index is stale, so it can be wired into CI later.

This does not overlap docs/refactoring.md's existing // TODO(refactor):
convention - that captures 3 markers; this inventories all 91, a
population nothing was tracking.

Triage goes in docs/refactoring.md

An earlier revision of this PR added a separate docs/TODO_TRIAGE.md.
That was wrong and has been removed. docs/refactoring.md was already a
295-line backlog with a release-gated 'Breaking changes queued for next
major version' section (already listing the deprecated commitInterval
options I had 'discovered'), a 'Remove static state' section carrying
upstream refs (confluentinc#405, #126, #143) and design branches I did not have, and
per-file entries for #233, the encoders and WorkContainer. My triage
re-derived that taxonomy from scratch with worse provenance.

The root error: concluding 'no breaking-changes list exists' after
grepping only docs/inflight.md. Too narrow a search to support the
conclusion drawn from it.

Two findings genuinely absent from refactoring.md were folded into it:
the loop-cycle waits in ParallelEoSStreamProcessorTest (not a style
point - that class is on the known intermittent-failure list, and
cycle-counting IS the flake mechanism), and the MockConsumer
.groupMetadata() workaround copy-pasted across four example tests (one
Apache Kafka defect, four times; not ours to fix, but the duplication
is).

The guardrail is the durable part

AGENTS.md gains a 'Where things live' table - one row per document, what
it owns and what it is explicitly NOT for, since the boundaries are what
get guessed wrong. There was no docs index at all: pointers were
scattered across six sections, and docs/SELF_HOSTED_RUNNER.md was linked
from nowhere. That layout only works if you already know which section
to read, which is circular - and is how this PR went wrong.

An earlier guardrail attempt lived inside the 'Refactoring backlog'
section, so it only reached someone who had already found the file it
was warning them about. The warning is now at the top, before the
mistake rather than after it.

Why .adoc/.md are not scanned

Raised in review, and recorded as a comment in list_files() rather than
left as an unexplained omission. It was tried: scanning docs added 4
hits and not one was a marker. Three were docs/refactoring.md QUOTING
markers the index already lists from their .java source - so the same
work appeared twice and the count inflated - and the fourth was prose.

That is structural, not bad luck: refactoring.md is where marker triage
lives, and AGENTS.md / inflight.md / CHANGELOG.adoc describe this tool,
so scanning docs means indexing the index-of-work. The generated index
is the reductio - alone it accounts for ~95 self-referential hits. The
comment also says what to do instead: if a genuine marker lands in
prose, move it into the code it concerns, where the scan will find it.

Marker detection

101 on the first pass, 91 real. Excluded with reasons in the script:
shell variables (todo=(), ${todo[*]}), YAML field keys (upstream-map
.yaml uses todo: as a schema field), string literals, prose referencing
a marker elsewhere, and compound names where the word is part of a
filename (todo-index.sh). That last is live, not theoretical: merging
master brought in #102's workflow comment naming the script and the
scanner counted it as a 92nd marker until filtered - caught by --check,
a fair self-validation of the tool.

One self-inflicted bug found while building it: the anchored exclusions
were applied to grep -n output, so ^ never matched and they silently did
nothing. Noted in the script so it is not reintroduced.

Not wired into CI as a gate. --check exists for it, but making it gating
means every added or removed marker fails the build until regenerated -
worth deciding deliberately, not as a side effect here.

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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