Skip to content

Commit 1310240

Browse files
test: document fixture-global counter limitation in flaky test seams
The PR review thread suggested keying flaky_by_index and flaky_instance_only by id(state) for per-instance counters. Tried that; broke fixture 021. Root cause: instance-level retry (021) constructs a fresh subgraph state on each retry, so id(state) resets per retry attempt — the per-instance counter starts over and retry exhausts. True per-instance semantics need an identifier stable across both node-level retry (state stable, id works) AND instance-level retry (state changes, id doesn't work). A state-field key would work but the field name is fixture-specific (item for 020, input for 021). Reverting to a fixture-global counter and documenting the limitation in the docstring + an inline comment, so a future fixture exercising the gap surfaces a real failure rather than silently miscounting. The existing fixtures (019 collect, 020 node-level retry, 021 instance-level retry) align with the global counter at runtime — not because the semantics are correct, but because the timing happens to land the failure on the right call.
1 parent 721aaad commit 1310240

1 file changed

Lines changed: 20 additions & 4 deletions

File tree

tests/conformance/adapter.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,17 @@ def _make_flaky_by_index_fn(
223223
success_compute = dict(cfg.get("success_compute", {}))
224224
fail_count_per_idx = cfg.get("fail_count_per_idx")
225225
fail_when_idx = cfg.get("fail_when_idx")
226+
# Fixture-global attempt counter. Truly per-instance semantics
227+
# would require an identifier stable across an instance's retries
228+
# AND distinct across fan-out instances. id(state) works for
229+
# NODE-LEVEL retry (state stable across retries) but not for
230+
# INSTANCE-LEVEL retry (each instance retry constructs a fresh
231+
# subgraph state). A state-field key would work in principle but
232+
# the field name is fixture-specific. The current fixtures
233+
# (020 node-level retry, 019 collect mode) pass with this global
234+
# counter because the timing aligns; documenting the limitation
235+
# here so a future fixture exercising the gap surfaces a real
236+
# failure rather than silently miscounting.
226237
attempt_counter = [0]
227238

228239
async def fn(state: Any) -> Mapping[str, Any]:
@@ -239,7 +250,8 @@ async def fn(state: Any) -> Mapping[str, Any]:
239250
message=f"flaky_by_index fail_when_idx={fail_when_idx}",
240251
category=category,
241252
)
242-
# fail_count_per_idx mode: fail the first N attempts.
253+
# fail_count_per_idx mode: fail the first N attempts (counted
254+
# globally across the fan-out).
243255
elif fail_count_per_idx is not None and idx < int(fail_count_per_idx):
244256
raise _CategorizedException(
245257
message=f"flaky_by_index failure at attempt {idx}",
@@ -257,9 +269,13 @@ def _make_flaky_instance_only_fn(
257269
cfg: Mapping[str, Any],
258270
trace: list[str],
259271
) -> Callable[[Any], Awaitable[Mapping[str, Any]]]:
260-
"""`flaky_instance_only` test seam — fails on first attempt of each
261-
fan-out instance, succeeds thereafter. Like flaky_by_index but the
262-
failure is keyed to the FIRST call only regardless of count."""
272+
"""`flaky_instance_only` test seam — fails on the first call,
273+
succeeds thereafter. Same fixture-global counter as
274+
flaky_by_index; see that function's note on the per-instance vs
275+
per-fixture keying tradeoff. The current fixtures (021
276+
instance-level retry) align with the global counter at runtime
277+
even though strict per-instance semantics would need a richer
278+
identifier."""
263279
fail_first_only = bool(cfg.get("fail_first_only", True))
264280
category = cfg.get("category", "provider_unavailable")
265281
success_compute = dict(cfg.get("success_compute", {}))

0 commit comments

Comments
 (0)