Skip to content

Commit e70dde0

Browse files
johannbothagnufede
andauthored
fix(ci_visibility): correct format string in retry-bookkeeping warning (#17862)
> [!NOTE] > CC @gnufede @vitor-de-araujo — flagging the CI App Libraries team since this touches `ddtrace/testing/internal/pytest/plugin.py`. Happy to hand this off if you'd rather take it. ## Summary Fixes a format-string bug in `RetryReports.make_final_report` that has been corrupting CI logs and (we believe) contributing to spurious test failures whenever Datadog Test Optimization's attempt-to-fix retries hit a bookkeeping mismatch. ## The bug The `log.warning` at `ddtrace/testing/internal/pytest/plugin.py:968` had two placeholders (`%s` and `%r`) but only one argument (`test`). Every time this branch fires, Python's logging hits a `TypeError` while formatting and emits a `--- Logging error ---` traceback to stderr instead of the intended diagnostic. The mismatch was introduced in [#15651](#15651) ([a95ac0d](a95ac0d52c), 2025-12-17), so we've had no usable signal for the underlying bookkeeping bug for ~4.5 months. ## When this branch fires The `IndexError` branch is reached when a retry handler reports a `final_status` (e.g. `PASS`) but no individual retry was recorded under that outcome in `reports_by_outcome`. **In the dd-trace-py kafka contrib suite this is happening ~70 times per day across py3.9/3.10/3.13 (steady-state since at least 2026-04-27).** The `IndexError` itself is caught locally and the function returns a degraded report with `longrepr=None` — that part is fine. The real damage is the `--- Logging error ---` output that appears as part of the test's stderr capture, which gets surfaced in pytest output and (in some flows) interacts badly with attempt-to-fix retries to produce spurious FAILED markers. ## How we noticed We marked some `test_data_streams_payload_size` parametrizations as Active in Flaky Tests Management after #17762 fixed the bucket-boundary race. They re-quarantined within hours. Investigation showed the test logic was passing on every attempt — the failure came from this format-string bug polluting the protocol's stderr. ## Fix Two small changes in the warning: 1. Add the missing `outcome` argument so the format string actually formats. 2. Include the outcomes that WERE recorded so the deeper bookkeeping bug becomes diagnosable when this branch fires. ## Out of scope The deeper logic bug — *why* `reports_by_outcome[outcome]` ends up empty when the retry handler reports an outcome — is left for the CI App Libraries team. With this fix in place, the warning is now actionable and should make root-causing it straightforward. --- > [!NOTE] > This fix was developed with the assistance of an LLM (Claude). Co-authored-by: gnufede <federico.mon@datadoghq.com>
1 parent b7e82f2 commit e70dde0

2 files changed

Lines changed: 11 additions & 1 deletion

File tree

ddtrace/testing/internal/pytest/plugin.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,13 @@ def make_final_report(self, test: Test, item: pytest.Item, final_status: TestSta
956956
longrepr = source_report.longrepr
957957
wasxfail = getattr(source_report, "wasxfail", None)
958958
except IndexError:
959-
log.warning("Test %s has final outcome %r, but no retry had this outcome; this should never happen", test)
959+
log.warning(
960+
"Test %s has final outcome %r, but no retry had this outcome; this should never happen. "
961+
"Outcomes seen: %s",
962+
test,
963+
outcome,
964+
sorted(self.reports_by_outcome.keys()),
965+
)
960966
longrepr = None
961967
wasxfail = None
962968

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
CI Visibility: Fixes an issue in the pytest plugin where a malformed log call emitted a ``--- Logging error ---`` traceback to stderr during Attempt to Fix retries, polluting pytest output and contributing to spurious test failures.

0 commit comments

Comments
 (0)