From 8f7b4c78efcef7f6aa505a477b8e55af1421c7d2 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 26 Aug 2026 17:21:23 -0500 Subject: [PATCH] feat(connscale): emit the band-less diagnostic fields on every run (BACKLOG #1366) The ratio says THAT something moved; these say WHICH. #1211 limb one made `empty_claims_per_msg` survive a passing run via `$GITHUB_STEP_SUMMARY`. The readings that explain a movement were not in that scope, and they die with the job: `tests/test_connscale_smoke.py` runs inside the `test` job, and THAT JOB UPLOADS NO ARTIFACTS -- ci.yml's three `upload-artifact` steps are in `load-test` (:2162), `load-test-sqlserver` (:2331) and `windows-service-smoke` (:2521). Measured absent from the emission on origin/main, all five zero: drain_seconds, reload_seconds, fd_probe_ticks, fd_probe_degraded_ticks, cpu_util_cores_mean. WHY THESE FIVE AND NOT "EVERYTHING ON THE RECORD": they are the readings that separate the competing explanations. Drain-tail and reload-probe separate ONLY on drain_seconds against reload_seconds. Contention and probe-cost separate ONLY on the FD probe's tick counts -- `fd_probe_degraded_ticks` non-zero means the walk could not measure, zero means it measured cleanly and any wrong reading is a wrong SUBJECT rather than a failed sample. That exact distinction decided a live investigation into the FD gauge today, and it was unavailable from CI. A SECOND RENDERER RATHER THAN A PARAMETER ON THE FIRST, and the constraint decides it. ONLY TWO metrics have a monotonic SLO -- empty_claims_monotonic and fd_count_monotonic. Every field here has NO BAND. `render_readings_markdown` emits prior / band floor / margin; for a band-less field those would be a threshold computed from whichever reading happened to precede it -- false precision manufactured by the renderer and, in a job summary, indistinguishable from a measured one. So this table carries no band, no threshold and no verdict column, and says so in its own preamble. `None` RENDERS AS A DASH AND NEVER AS `0`. "the probe did not measure" and "the probe measured zero" are different verdicts, and telling them apart is the entire purpose of fd_probe_degraded_ticks. Emitted from the FIXTURE, before any assertion, so a passing run is recorded as fully as a failing one -- and a test pins that the call is UNCONDITIONAL, because the renderer's own tests cannot see a fixture that gates it on failure. NOT IN SCOPE: artifact upload (the step-summary channel already reaches every run and needs no ci.yml change), and the empty_claims band -- that is #1211 limb two and it stays blocked until samples exist. TWO OF MY OWN TESTS WERE HOLLOW AND THE MUTATION RUN PROVED IT. Both looked fine: - the field-set test ITERATED `DIAGNOSTIC_FIELDS` and checked each label against the rendered text. That reads the same constant it is testing, so renaming a field out of the set SURVIVED -- the test looked for the new name and found it. Now pinned against a literal, so an add, a removal or a rename all fail deliberately. - the no-band test asserted the substring "no band", which the table's HEADING also satisfies, so deleting the explanatory sentence SURVIVED. A substring occurring twice cannot witness the presence of either occurrence. Now pinned to the sentence. A DUPLICATE TEST NAME SILENTLY DELETED #236's CAP TEST, and the count caught it. This file had 29 `def test_` and ran 28: my new cap test was given the name of the existing one, and Python replaced the earlier definition with no error, no warning and nothing collected. Renamed, and a guard now fails the file if any two tests share a name -- pytest does not warn about this and the only visible trace is a collected count one lower than the definitions, which nobody reads. Verified: ruff check + ruff format --check clean; mypy strict clean (9 files); 32 tests passing, all new ones confirmed present BY NAME rather than inferred from the total; definitions equal collected (32/32). Six mutants, ALL KILLED, EACH BY A DISTINCT RED SET: None as zero; the no-band declaration dropped; a field dropped from the set; the discriminates text dropped; a silent cap; the fixture gated on failure. Every mutant asserted a unique anchor, a changed hash, an unchanged NUL count AND no introduced control bytes, and every restore was byte-identical. Rebased onto f8597f4db. THE REBASE NEEDED A SEMANTIC RESOLUTION GIT COULD NOT SEE. This branch replaced the local `context = {...}` dict in _record_ratio_readings with a shared _run_context() helper, precisely so more than one emitter could read ONE definition. PR #599 (BACKLOG #1292) meanwhile added a second caller on main, `readings_payload(..., context=context)`, against the local dict this branch had already removed. Git merged both sides without a conflict and produced a NameError: the definition was gone and a use remained. Resolved the way the helper's own docstring dictates -- the new caller reads _run_context() too. Caught by running the connscale tests, not by the merge: 1 failed and 9 errors before, 82 passed after. Co-Authored-By: Claude Opus 5 --- docs/BACKLOG.md | 34 ++++ harness/load/connscale/report.py | 128 +++++++++++++ tests/test_connscale_empty_claims_per_msg.py | 189 +++++++++++++++++++ tests/test_connscale_smoke.py | 49 ++++- 4 files changed, 390 insertions(+), 10 deletions(-) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index d7bf9efb7..30a3336f3 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -16843,3 +16843,37 @@ four branches returning the same text would pass every other assertion in the bl **Driven against the PURE verdict function with injected hashes**, never by mutating the installed gate: that file is machine-global and every PreToolUse hook on this box reads it, so a test may not take it out from under a concurrent session. Same reasoning the existing negative control gives. + +## 1366. the four connscale fields that discriminate the surviving failure hypotheses never escape the test job + +> 🔢 **Filed 2026-08-26 (builder 2) - BUILT IN THIS COMMIT, not yet landed.** Successor in subject to [#1211](#1211-empty_claims_per_msg-is-not-contention-immune-the-ratio-form-excursions-past-its-own-slo-band-on-a-hosted-runner), whose limb one made the RATIO survive a passing run; this makes the readings that EXPLAIN the ratio survive one too. +> Verdict: build +> Closing-act: code + +**Cluster:** Developer Experience & CI. **Priority:** P2. **Verdict:** build. +**Severity:** no engine effect, no PHI axis, no deployment axis (sec. 0). The cost is diagnostic: a connscale failure on a hosted runner cannot be attributed from CI alone, so each occurrence burns a full cycle and ends in a re-run rather than a cause. + +**What:** `tests/test_connscale_smoke.py` runs inside the `test` job. **That job uploads no artifacts** -- ci.yml's three `upload-artifact` steps are in `load-test` (:2162), `load-test-sqlserver` (:2331) and `windows-service-smoke` (:2521). So every reading on a `ConnScaleRecord` that is not printed dies with the job. #1211 limb one made ONE metric survive, `empty_claims_per_msg`, via `$GITHUB_STEP_SUMMARY`. The fields that say WHICH explanation is right were not part of that scope. + +**The four the investigation actually turns on**, measured absent from the emission on `origin/main`: + +``` +drain_seconds 0 reload_seconds 0 +fd_probe_ticks 0 fd_probe_degraded_ticks 0 +cpu_util_cores_mean 0 +``` + +* **drain-tail vs reload-probe separate ONLY on `drain_seconds` against `reload_seconds`.** +* **contention vs probe-cost separate ONLY on the FD probe's tick counts** -- `fd_probe_degraded_ticks` non-zero means the walk could not measure; zero means it measured cleanly and a wrong reading is a wrong SUBJECT rather than a failed sample. That distinction is what a live investigation into the connscale FD gauge turned on, and it was unavailable from CI. + +**THE DESIGN CONSTRAINT THAT DECIDES THE SHAPE, and it is why this is a second renderer rather than a parameter on the first.** Only **two** metrics have a monotonic SLO -- `empty_claims_monotonic` and `fd_count_monotonic`. Every field above has **no band**. `render_readings_markdown` emits `prior` / `band floor` / `margin`, which for a band-less field would be **a threshold computed from whichever reading happened to precede it** -- false precision manufactured by the renderer and indistinguishable, in a job summary, from a measured one. So the table carries no band, no threshold and no verdict column, and says so in its own preamble. + +**Emitted from the FIXTURE, before any assertion**, for the same reason #1211's readings are: a field recorded only on failure cannot establish its own normal range. That is the selection bias #1211 exists to fix, one metric family over. + +**`None` renders as a dash and never as `0`.** "the probe did not measure" and "the probe measured zero" are different verdicts, and telling them apart is the whole purpose of `fd_probe_degraded_ticks`. + +**Not in scope, deliberately:** artifact upload (the step-summary channel already reaches every run and needs no ci.yml change), and any change to the `empty_claims_per_msg` band -- that is #1211 limb two and it stays blocked until samples exist. + +**Related:** #1211 (limb one shipped the channel this reuses; limb two blocked), #1101 (the per-message form these annotate), #320 (windows-2025 leg timing, one of the explanations these fields separate). + +**Source:** scoped by the Dispatcher, corrected by this lane's scouting -- the original scoping described artifact-upload work already done and a selection bias already fixed by #1211 limb one; what remained was the band-less fields and the constraint above. diff --git a/harness/load/connscale/report.py b/harness/load/connscale/report.py index 2f9d559d9..6de3f5e08 100644 --- a/harness/load/connscale/report.py +++ b/harness/load/connscale/report.py @@ -361,6 +361,61 @@ def monotonic_pairs( return pairs +@dataclass(frozen=True) +class DiagnosticField: + """One reading emitted for DIAGNOSIS, with no band and therefore no verdict. + + Deliberately separate from :class:`MonotonicPair`. A pair carries `prior`, `threshold` and an `ok` + flag because its metric HAS an SLO band; these fields do not. Rendering a floor for a band-less + field would print a threshold computed from an adjacent reading -- a number that looks measured and + is manufactured by the renderer. Two shapes, because there are two kinds of reading. + """ + + label: str + read: Callable[[ConnScaleRecord], object] + #: Why this field is here -- which competing explanation it separates. Emitted in the table's + #: preamble so a reader meeting it in a job summary knows what it is FOR, not just what it is. + discriminates: str + + +#: The band-less readings emitted on every run (BACKLOG #1366). +#: +#: THESE ARE EXACTLY THE FIELDS THAT SEPARATE THE SURVIVING EXPLANATIONS for a connscale failure, which +#: is why the set is small and named rather than "everything on the record". Without them a failure is +#: permanently undiagnosable: the ratio says THAT something moved, and nothing says WHICH. +#: +#: Single definition -- the emitter and its tests both read this tuple, so a field added here is +#: covered without editing a second list. +DIAGNOSTIC_FIELDS: tuple[DiagnosticField, ...] = ( + DiagnosticField( + "drain_seconds", + lambda r: r.drain_seconds, + "drain-tail vs reload-probe: these two separate ONLY on drain_seconds against reload_seconds", + ), + DiagnosticField( + "reload_seconds", + lambda r: r.reload_seconds, + "the other half of that pair; None means the reload probe did not measure this step", + ), + DiagnosticField( + "fd_probe_ticks", + lambda r: r.fd_probe_ticks, + "how many intervals the FD walk actually sampled -- a low count is a coarse gauge, not a fault", + ), + DiagnosticField( + "fd_probe_degraded_ticks", + lambda r: r.fd_probe_degraded_ticks, + "contention vs probe-cost: NON-ZERO means the walk could not measure, ZERO means it measured " + "cleanly and any wrong reading is a wrong SUBJECT rather than a failed sample", + ), + DiagnosticField( + "cpu_util_cores_mean", + lambda r: r.cpu_util_cores_mean, + "how loaded the box was across the hold, which is the input every contention story needs", + ), +) + + @dataclass(frozen=True) class ConnScaleReport: profile: str @@ -557,6 +612,79 @@ def readings_payload( "readings": readings, } + def render_diagnostics_markdown( + self, + fields: tuple[DiagnosticField, ...] = DIAGNOSTIC_FIELDS, + *, + context: dict[str, str] | None = None, + max_rows: int = _MAX_SUMMARY_ROWS, + ) -> str: + """The band-less diagnostic table, emitted on every run (BACKLOG #1366). + + NOT a variant of :meth:`render_readings_markdown`, and the separation is the point. That one + renders `prior`, `band floor` and `margin` because its metric has an SLO band. ONLY + ``empty_claims_monotonic`` and ``fd_count_monotonic`` have one; every field here has none, so + those columns would be a floor computed from whichever reading happened to precede it -- + false precision manufactured by the renderer rather than measured by anything. + + So this table carries NO verdict column and NO threshold. It says what was observed and what + each field is FOR, and leaves the judgement to a reader who has the competing explanations in + front of them. + + Pure: returns text, writes nothing. Row count capped for the same reason as the banded table -- + an oversized ``$GITHUB_STEP_SUMMARY`` write is dropped in full rather than trimmed. + """ + head = ["### connscale diagnostics (no band, no verdict)"] + ctx = { + "profile": self.profile, + "db_backend": self.db_backend or "sqlite", + **(context or {}), + } + head.append("") + head.append(" | ".join(f"{k}: {v}" for k, v in ctx.items())) + head.append("") + head.append( + "Recorded on every run, pass or fail. **None of these has an SLO band**, so there is no " + "threshold here and nothing below is a verdict -- they are the readings that separate " + "competing explanations for a failure (BACKLOG #1366)." + ) + head.append("") + for f in fields: + head.append(f"- `{f.label}` -- {f.discriminates}") + head.append("") + + if not self.records: + head.append("No record was produced by this run.") + return "\n".join(head) + "\n" + + head.append("| lane | N | " + " | ".join(f.label for f in fields) + " |") + head.append("|---|---|" + "---|" * len(fields)) + + rows: list[str] = [] + dropped = 0 + for r in sorted(self.records, key=lambda r: (r.sweep_mode, r.claim_mode, r.count)): + if len(rows) >= max_rows: + dropped += 1 + continue + cells = [] + for f in fields: + v = f.read(r) + # `None` renders as an explicit dash, NEVER as 0. "the probe did not measure" and + # "the probe measured zero" are different verdicts and the whole point of these + # fields is telling them apart. + cells.append("-" if v is None else (f"{v:.4g}" if isinstance(v, float) else str(v))) + rows.append( + f"| {lane_label(r.sweep_mode, r.claim_mode)} | {r.count} | " + + " | ".join(cells) + + " |" + ) + + out = head + rows + if dropped: + out.append("") + out.append(f"{dropped} further row(s) not shown: capped at {max_rows}.") + return "\n".join(out) + "\n" + def to_csv(self) -> str: """One row per (sweep_mode, N) step — for spreadsheet curve plotting.""" buf = io.StringIO() diff --git a/tests/test_connscale_empty_claims_per_msg.py b/tests/test_connscale_empty_claims_per_msg.py index 36812fd58..a08154a97 100644 --- a/tests/test_connscale_empty_claims_per_msg.py +++ b/tests/test_connscale_empty_claims_per_msg.py @@ -19,13 +19,16 @@ from __future__ import annotations +import dataclasses import json import pathlib +import re import warnings import pytest from harness.load.connscale.report import ( + DIAGNOSTIC_FIELDS, ConnScaleRecord, ConnScaleReport, NoLoss, @@ -37,6 +40,7 @@ _empty_claims_per_msg, _monotonic_slo, ) +from tests import test_connscale_smoke as _smoke_module from tests.test_connscale_smoke import ( _DEFAULT_READINGS_PATH, _LOCAL_READINGS_ENV, @@ -548,3 +552,188 @@ def test_a_failed_json_write_warns_rather_than_failing_the_run(tmp_path, monkeyp with pytest.warns(UserWarning, match="could not write connscale readings JSON"): _write_readings_json({"schema_version": 1}) + + +# -------------------------------------------------------------------------------------------------- +# BACKLOG #1366: the BAND-LESS diagnostic fields. +# +# The ratio says THAT something moved; these say WHICH. Drain-tail and reload-probe separate ONLY on +# drain_seconds against reload_seconds; contention separates from probe-cost ONLY on the FD probe's +# tick counts. Every one of these readings already existed on the record and never left the `test` +# job, which uploads no artifacts -- so a connscale failure was undiagnosable from CI alone. +# +# NONE OF THEM HAS AN SLO BAND, and that is the design constraint the whole shape follows from. Only +# empty_claims_monotonic and fd_count_monotonic have one. Rendering `prior` / `band floor` / `margin` +# for a band-less field would print a threshold computed from whichever reading happened to precede it +# -- false precision manufactured by the renderer. Hence a SECOND table rather than a reuse of the +# banded one. +# -------------------------------------------------------------------------------------------------- + + +def _diag(mode: str, count: int, **over: object) -> ConnScaleRecord: + """A record with the diagnostic fields set. Reuses `_rec` so the inert placeholders stay in one + place; `dataclasses.replace` works because ConnScaleRecord is frozen.""" + base = _rec(mode, count, per_msg=10.0) + return dataclasses.replace(base, **over) # type: ignore[arg-type] + + +def _diag_report(*records: ConnScaleRecord) -> ConnScaleReport: + return ConnScaleReport( + profile="smoke", + engine_url="http://127.0.0.1:0", + db_backend=None, + shim_installed=True, + records=list(records), + slos=[], + result_ok=True, + exit_code=0, + ) + + +#: The fields this item ships, written out INDEPENDENTLY of the constant under test. +#: +#: THE FIRST VERSION OF THE TEST BELOW ITERATED `DIAGNOSTIC_FIELDS` AND CHECKED EACH LABEL AGAINST THE +#: RENDERED TEXT. That is self-referential: rename a field in the set and the test looks for the NEW +#: name, finds it, and passes. A mutation run proved it -- renaming `cpu_util_cores_mean` out of the +#: shipped set SURVIVED. A test that reads the same constant it is testing measures nothing. +_EXPECTED_DIAGNOSTIC_LABELS = frozenset( + { + "drain_seconds", + "reload_seconds", + "fd_probe_ticks", + "fd_probe_degraded_ticks", + "cpu_util_cores_mean", + } +) + + +def test_the_shipped_field_set_is_exactly_what_this_item_promises() -> None: + """Pinned against a LITERAL, not against the constant itself, so an add, a removal or a rename all + fail here and have to be made deliberately.""" + shipped = {f.label for f in DIAGNOSTIC_FIELDS} + assert shipped == _EXPECTED_DIAGNOSTIC_LABELS, ( + f"the shipped diagnostic set changed. Added: {sorted(shipped - _EXPECTED_DIAGNOSTIC_LABELS)}; " + f"removed: {sorted(_EXPECTED_DIAGNOSTIC_LABELS - shipped)}. That is a scope change to BACKLOG " + f"#1366, not a refactor -- update this literal in the same commit and say why." + ) + + +def test_every_shipped_diagnostic_field_reaches_the_table() -> None: + """...and each one actually renders. Checked against the LITERAL for the same reason.""" + text = _diag_report(_diag("fixed_per_conn", 12)).render_diagnostics_markdown() + for label in sorted(_EXPECTED_DIAGNOSTIC_LABELS): + assert label in text, f"{label} never reached the table: {text}" + + +def test_each_field_says_what_it_DISCRIMINATES_not_just_what_it_is() -> None: + """A reader meeting these in a job summary needs to know which explanation each one separates. + A bare column header is a number with no question attached to it.""" + text = _diag_report(_diag("fixed_per_conn", 12)).render_diagnostics_markdown() + for field in DIAGNOSTIC_FIELDS: + assert field.discriminates in text, f"{field.label} emitted without its rationale" + + +def test_THE_DISTINCTION_THAT_MATTERS_none_renders_as_a_dash_never_as_zero() -> None: + """ "the probe did not measure" and "the probe measured zero" are DIFFERENT VERDICTS, and telling + them apart is the entire point of fd_probe_degraded_ticks. A None coerced to 0 would assert a + clean measurement that never happened.""" + text = _diag_report( + _diag("fixed_per_conn", 12, reload_seconds=None, fd_probe_degraded_ticks=0) + ).render_diagnostics_markdown() + row = next(line for line in text.splitlines() if line.startswith("| fixed_per_conn |")) + assert "| - |" in row, f"an unmeasured field must render as a dash: {row}" + assert "| 0 |" in row, f"a measured zero must still render as 0: {row}" + + +def test_the_table_carries_NO_band_and_says_so() -> None: + """The constraint that produced a second renderer. A threshold column here would be manufactured.""" + text = _diag_report(_diag("fixed_per_conn", 12)).render_diagnostics_markdown() + header = next(line for line in text.splitlines() if line.startswith("| lane | N |")) + for forbidden in ("prior", "band floor", "margin", "verdict"): + assert forbidden not in header, f"a band-less table must not carry a {forbidden!r} column" + # PINNED TO THE SENTENCE, NOT THE PHRASE. The first version asserted "no band" over the whole + # text, which the HEADING also satisfies -- so deleting the explanatory sentence SURVIVED a + # mutation. A substring occurring twice cannot witness the presence of either occurrence. + assert "None of these has an SLO band" in text, ( + "the preamble must state IN PROSE that these fields carry no band. The heading alone is not " + "enough -- a reader skimming to the table needs it beside the numbers." + ) + + +def test_it_emits_on_a_PASSING_report(): # noqa: ANN201 + """The selection-bias property, one metric family over from #1211. A field recorded only on + failure cannot establish its own normal range.""" + report = _diag_report(_diag("fixed_per_conn", 12), _diag("fixed_per_conn", 24)) + assert report.result_ok, "the test lost its own premise" + text = report.render_diagnostics_markdown() + assert text.count("| fixed_per_conn |") == 2, text + + +def test_the_lane_label_is_the_SAME_definition_the_banded_table_uses() -> None: + """Two tables from one run must name a lane identically, or a reader diffing them chases a + difference in the labelling rather than in the data.""" + recs = [_diag("fixed_per_conn", 12, claim_mode="pooled")] + text = _diag_report(*recs).render_diagnostics_markdown() + assert f"| {lane_label('fixed_per_conn', 'pooled')} |" in text + assert "| fixed_per_conn/pooled |" in text + + +def test_a_run_with_no_records_says_so_rather_than_rendering_an_empty_table() -> None: + text = _diag_report().render_diagnostics_markdown() + assert "No record was produced" in text + assert "| lane | N |" not in text + + +def test_a_capped_DIAGNOSTICS_table_states_what_it_dropped() -> None: + """Same reason as the banded table: an oversized step-summary write is dropped ENTIRELY, so a big + profile must lose rows -- and may not lose them silently. + + NAMED DISTINCTLY FROM THE BANDED TABLE'S CAP TEST ON PURPOSE. This function first shared that + name, and Python silently REPLACED the earlier definition -- pytest collected one, #236's cap test + stopped running, and nothing reported it. The guard below now fails the file rather than the run + going quietly one test lighter.""" + recs = [_diag("fixed_per_conn", n) for n in range(12, 30)] + text = _diag_report(*recs).render_diagnostics_markdown(max_rows=4) + assert "capped at 4" in text + assert "14 further row(s) not shown" in text + assert len([x for x in text.splitlines() if x.startswith("| fixed_per_conn |")]) == 4 + + +def test_the_fixture_emits_diagnostics_UNCONDITIONALLY_not_only_on_failure() -> None: + """The renderer emitting on a passing report is only half the property. The FIXTURE must call it + without a condition -- gating it on `result_ok` would restore exactly the selection bias #1211 + exists to remove, and the renderer's own tests could not see that. + + Scanned rather than driven: the fixture spawns engine subprocesses, so running it here would cost + the whole sweep to assert one call site. + """ + source = pathlib.Path(_smoke_module.__file__).read_text(encoding="utf-8") + body = source[source.index("async def smoke_report") :] + call = re.search(r"^\s*_record_diagnostics\(report\)\s*$", body, re.M) + assert call, f"the fixture must call _record_diagnostics; body was {body[:400]!r}" + line = body[: call.start()].count(chr(10)) + preceding = body.splitlines()[max(0, line - 3) : line] + assert not any(re.match(r"\s*(if|elif|else|try|except)\b", p) for p in preceding), ( + f"the call must be unconditional; the three lines before it were {preceding}" + ) + + +def test_no_two_tests_in_this_file_share_a_name() -> None: + """A duplicate `def test_...` is SILENT: the later definition replaces the earlier, pytest collects + one, and the lost test leaves no error, no warning and no failing assertion. The only visible trace + is a collected count one lower than the number of definitions -- which nobody reads. + + Caught here for real: a new cap test was given the name of an existing one, and #236's cap coverage + vanished. The file counted 29 definitions and ran 28. + """ + source = pathlib.Path(__file__).read_text(encoding="utf-8") + names = re.findall(r"^def (test_[A-Za-z0-9_]+)", source, re.M) + duplicates = sorted({n for n in names if names.count(n) > 1}) + assert not duplicates, ( + f"these test names are defined more than once, so the earlier definition is silently " + f"discarded: {duplicates}. Rename one; do not delete either." + ) + # Positive control: the scan must actually be finding tests, or the assertion above is vacuous. + assert len(names) > 20, ( + f"the name scan found only {len(names)} tests; it is not reading the file" + ) diff --git a/tests/test_connscale_smoke.py b/tests/test_connscale_smoke.py index dc47bfed9..d20537fd3 100644 --- a/tests/test_connscale_smoke.py +++ b/tests/test_connscale_smoke.py @@ -282,20 +282,12 @@ def _record_ratio_readings(report: ConnScaleReport) -> None: The tolerance is IMPORTED, not typed in. A second copy of 0.25 here would be a second definition of the band, and the emitted floor could then drift away from the one the SLO actually enforces. """ - context = { - # What a later reader needs to tell samples apart. #1211's whole question is whether the - # ratio moves with runner contention, so the core count is part of the reading, not trivia. - "runner_os": os.environ.get("RUNNER_OS", "local"), - "cpus": str(os.cpu_count()), - "run_id": os.environ.get("GITHUB_RUN_ID", "-"), - "sha": os.environ.get("GITHUB_SHA", "-")[:8] or "-", - } _append_step_summary( report.render_readings_markdown( "empty_claims_per_msg", lambda r: r.empty_claims_per_msg, tolerance=_MONOTONIC_TOLERANCE, - context=context, + context=_run_context(), ) ) _write_readings_json( @@ -303,7 +295,7 @@ def _record_ratio_readings(report: ConnScaleReport) -> None: "empty_claims_per_msg", lambda r: r.empty_claims_per_msg, tolerance=_MONOTONIC_TOLERANCE, - context=context, + context=_run_context(), ) ) @@ -331,6 +323,42 @@ def _write_readings_json(payload: dict[str, object]) -> None: warnings.warn(f"could not write connscale readings JSON: {exc}", stacklevel=2) +def _run_context() -> dict[str, str]: + """What a later reader needs to tell one run's samples from another's. + + ONE definition, read by BOTH emitters. Two copies would drift, and a reader diffing the two tables + a single run produces would then be chasing a difference in the context rather than in the data. + + #1211's question is whether the ratio moves with runner contention, so the core count is part of + the reading rather than trivia. + """ + return { + "runner_os": os.environ.get("RUNNER_OS", "local"), + "cpus": str(os.cpu_count()), + "run_id": os.environ.get("GITHUB_RUN_ID", "-"), + "sha": os.environ.get("GITHUB_SHA", "-")[:8] or "-", + } + + +def _record_diagnostics(report: ConnScaleReport) -> None: + """Persist the band-less diagnostic fields this run produced (BACKLOG #1366). + + THE RATIO SAYS THAT SOMETHING MOVED; THESE SAY WHICH. Without them a connscale failure is + permanently undiagnosable from CI alone -- drain-tail and reload-probe separate ONLY on + `drain_seconds` against `reload_seconds`, and contention separates from probe-cost ONLY on the FD + probe's tick counts. Those readings existed on every record already and never left the `test` job, + which uploads no artifacts. + + Emitted from the FIXTURE for the same reason #1211's readings are: it runs before any assertion, so + a passing run is recorded as fully as a failing one. A field that appears only on failure cannot + establish what its normal range is, which is the defect #1211 exists to fix one metric over. + + NO BAND AND NO VERDICT -- see `render_diagnostics_markdown`. None of these has an SLO, so any + threshold printed beside them would be manufactured by the renderer. + """ + _append_step_summary(report.render_diagnostics_markdown(context=_run_context())) + + @pytest.fixture(scope="module") async def smoke_report() -> ConnScaleReport: """ONE ``run_connscale`` sweep for the whole module (BACKLOG #1331). @@ -376,6 +404,7 @@ async def smoke_report() -> ConnScaleReport: ) # In the FIXTURE, so the readings are recorded before any assertion can fail the module. _record_ratio_readings(report) + _record_diagnostics(report) return report