From 42b1ac0a3e554ea7691e1fdbe254e26ac3598e2c Mon Sep 17 00:00:00 2001 From: xping-admin Date: Mon, 7 Sep 2026 21:40:40 +0200 Subject: [PATCH] fix(cli): count the tests a metric could not measure, not only the candidates the report dropped (#185) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ExcludedLowEvidence` was incremented in exactly one place — the coordinator's own reporting floor — so it counted only candidates a provider had already offered. Every decline inside a provider was invisible, and that is where most of them happen. A test whose runs all recorded a zero median normalises nothing, so `DurationProvider` could compute neither of its statistics and declined both; the test then landed silently inside `healthy`, which tells a reader it was looked at and is fine. It was not looked at. `ProviderReport` gains a per-kind `NotMeasured` tally beside its family size, and every provider fills it. `Examination.NotPosed` is deleted in favour of `AwaitingRuns` and `Unreadable`, which turned all six call sites into compile errors and forced each to say which it was — the two are different news and only the first is answered by waiting. Six baseline runs where the comparison needs seven empties as the store fills; a zero median never will. Per kind and never totalled. Summing counts one test once per question its data could not answer, intersecting collapses to nothing because pass and fail are always readable, and either total would move with `--kind` — which is the one thing a count of what could not be measured must not do. `DurationProvider.Unstable` is where the acceptance criterion lives: `RobustDispersion.Of` answers zero both for a sample too short to have a spread and for one whose median is not positive, so the dispersion gate was reading an unmeasured test as a perfectly steady one. The preconditions are now computed before it. `healthy` is deliberately unchanged. Subtracting the unmeasured would make a headline count move with `--kind`; `SummaryDto.Healthy` now documents that it means "no finding was raised" rather than "checked and fine". `SharedFailure` and `BrokenFixture` publish nothing rather than zero — they are counted in signature groups, not tests. The three retry kinds share one figure, taken at the precondition they all read, because the chain that picks between them never asks whether a later kind was measurable. Rendered as one line above the fence naming the three largest kinds, and published per kind as `summary.notMeasured`. Schema 1.14 -> 1.15. `docs/internals/finding-populations.md` records the per-kind decisions, per that file's own rule. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TMQcZLBGFTd8wz3mcfn9vc --- docs/cli/command-reference.md | 15 +- docs/internals/finding-populations.md | 89 ++++++++ .../Report/Contract/EnvelopeBuilder.cs | 34 +++ .../Report/Contract/ReportEnvelope.cs | 41 +++- src/Xping.Cli/Report/FindingCoordinator.cs | 51 ++++- .../Report/Providers/DurationProvider.cs | 78 ++++++- .../Report/Providers/FailureModeProvider.cs | 76 ++++--- .../Report/Providers/IFindingProvider.cs | 163 +++++++++++++-- .../Providers/ParallelSensitiveProvider.cs | 33 ++- .../Report/Providers/RetryProvider.cs | 50 ++++- .../Report/Providers/TimeSensitiveProvider.cs | 62 ++++-- .../Report/Providers/VanishedProvider.cs | 40 +++- .../Report/Rendering/TextReportRenderer.cs | 83 ++++++++ .../Commands/CliSurfaceTests.cs | 4 +- .../Report/DurationProviderTests.cs | 77 +++++++ .../Report/FailureModeProviderTests.cs | 109 ++++++++++ .../Report/FindingCoordinatorTests.cs | 105 +++++++++- .../Report/ParallelSensitiveProviderTests.cs | 42 ++++ .../Report/ReportEnvelopeTests.cs | 7 +- .../Report/RetryProviderTests.cs | 69 +++++++ .../Report/ShareableOutputTests.cs | 193 +++++++++++++++++- .../Report/TimeSensitiveProviderTests.cs | 40 ++++ .../Report/VanishedProviderTests.cs | 44 ++++ 23 files changed, 1404 insertions(+), 101 deletions(-) diff --git a/docs/cli/command-reference.md b/docs/cli/command-reference.md index fbe023d..831752f 100644 --- a/docs/cli/command-reference.md +++ b/docs/cli/command-reference.md @@ -337,11 +337,18 @@ For scripts and agents. Emits a versioned envelope and nothing else — no rende xping report --all --format json > findings.json ``` -Every finding carries a `headline` — the same sentence the rendered report prints — plus `metrics`, the labelled pairs behind it, and the raw `evidence` the two were resolved from. It also carries `population`, which is one of `allExecutions`, `excludesEnvironmental` or `excludesEnvironmentalAndClustered` and says which executions the counts inside `evidence` were taken over, and `evidenceSessions`, the number `evidenceLevel` was banded from: +Every finding carries a `headline` — the same sentence the rendered report prints — plus `metrics`, the labelled pairs behind it, and the raw `evidence` the two were resolved from. It also carries `population`, which is one of `allExecutions`, `excludesEnvironmental` or `excludesEnvironmentalAndClustered` and says which executions the counts inside `evidence` were taken over, and `evidenceSessions`, the number `evidenceLevel` was banded from. + +`summary.notMeasured` says, per kind, how many tests that metric could not be computed for at all — +split into the ones waiting for more runs and the ones whose recorded data cannot answer the question +however long you wait. It is deliberately not a total: adding the entries counts a test once per +question it could not answer, and a kind absent from the object keeps no such tally. Read one entry to +ask "how much of my suite could this metric read". Tests counted here are inside `healthy`, which +means "no finding was raised" rather than "checked and fine": ```json { - "schemaVersion": "1.14", + "schemaVersion": "1.15", "window": { "sessionCount": 20, "resolution": "default", "currentSliceSize": 3 }, "context": { "sha": "a3f9c2e", "branch": "main", "assembly": "Checkout.Tests" }, "summary": { @@ -351,6 +358,10 @@ Every finding carries a `headline` — the same sentence the rendered report pri "healthy": 409, "excludedLowEvidence": 41, "excludedNotSignificant": 6, + "notMeasured": { + "DurationRegression": { "awaitingRuns": 63, "unreadable": 32 }, + "ParallelSensitive": { "awaitingRuns": 0, "unreadable": 108 } + }, "partialSessions": 0 }, "findings": [ diff --git a/docs/internals/finding-populations.md b/docs/internals/finding-populations.md index a490ae6..489f9f7 100644 --- a/docs/internals/finding-populations.md +++ b/docs/internals/finding-populations.md @@ -122,6 +122,95 @@ silently dropped by another with nothing on screen to explain it. So a claim res a test with twenty runs of history is still printed. It is printed saying `evidence low`, which is what the level is for. +## What each kind could not measure + +A finding's population says which executions its counts were taken over. This section says something +prior to that: which **tests the kind could not be computed for at all**, and why. Until it was +recorded, a provider that declined for want of data left no trace anywhere — the summary's excluded +tally counts only the candidates the coordinator itself dropped at the reporting floor — so a test no +statistic could be taken of fell through into `healthy` and was reported to a reader as fine. + +The tally reaches the JSON as `summary.notMeasured`, keyed by kind, and the rendered report as one +line above the fence naming the three largest with `+n more`. + +### The two reasons, and the question that separates them + +| reason | meaning | what a reader does | +|---|---|---| +| `awaitingRuns` | the window does not hold enough of this test yet | wait; the store fills | +| `unreadable` | nothing this kind reads was recorded | change what is recorded, or accept it | + +The question at every decline site is whether **another run of the same shape as the ones already +recorded** would fix it. Nothing subtler is needed: the two differ in what the reader should do next, +not in how the gate was written. Six baseline runs where the comparison needs seven is `awaitingRuns`; +a test whose every run recorded a zero median normalises nothing, and the eighth such run normalises +nothing either, so it is `unreadable`. + +**This is not the reporting floor.** That floor asks whether a test has been around long enough to be +judged and is applied centrally, to a candidate a provider already computed. This counts tests for +which no candidate was ever computed. A candidate that reaches the coordinator has been measured by +definition, so the two can never both fire on one claim. + +### The matrix + +| kind | `awaitingRuns` | `unreadable` | +|---|---|---| +| `DurationRegression` | either arm under 7 / 3 comparable runs | an arm that held runs and normalised none of them | +| `DurationUnstable` | one normalised reading | nothing normalisable, or no run with a usable median | +| `ParallelSensitive` | structurally none — this kind has no session floor | fewer than two distinct concurrency levels | +| `TimeSensitive` | fewer than two arms' worth of runs on a clock | no clock at all, two time zones, or no split the runs admit | +| `Vanished` | baseline under `VanishedMinBaselineSessions`, or no baseline slice | structurally none — an appearance is always readable | +| `RetryMasked`, `RetryDeepening`, `RetryExhausted` | every run of the test was discounted as environmental | structurally none — an attempt number is always recorded | +| `Flaky`, `AlwaysFailing`, `TimingOut` | every execution of the test was discounted | structurally none — an outcome is always readable | +| `SharedFailure`, `BrokenFixture` | **absent** | **absent** | + +A published zero and an absence are different statements. A zero means the kind was offered tests and +could read every one; an absence means the kind keeps no such tally. + +### Why five kinds publish nothing + +**`SharedFailure` and `BrokenFixture` are counted in signature groups, not tests.** Every other entry +in the table is a count of fingerprints, and a count of groups published under the same field is not +a number a reader can compare with the one beside it. + +**The three retry kinds share one figure rather than each publishing their own.** They read one +reduction of a test's runs, and the chain that picks between them — exhausted, then deepening, then +masked — stops at the first that fires. Whether a later kind could have been measured on a test an +earlier one claimed is a question the algorithm never asks, and answering it for the tally alone would +mean running all three on every fingerprint to fill in a number nobody reads. So the count is taken at +the one precondition the three share: a test whose every run was an outage. + +### Two skips that are charged to nothing + +A fingerprint the index holds but cannot resolve to a test is an inconsistency inside the index, not a +measurement the data declined; every provider passes over it silently. + +A test absent from the recent slice is passed over by `DurationProvider` for the reason the population +matrix already gives — its absence belongs to `Vanished` — and counting it here would state one +disappearance twice, in a line whose whole purpose is to name questions whose answers are missing. + +### Why it is per kind and never a total + +Summing across kinds counts one test once per question its data could not answer. Intersecting them +collapses to nothing, because a test's pass and fail are always readable and so nearly every test is +measured by something. And either total would move with `--kind`, which is the one thing a count of +what could not be measured must not do: a reader comparing a full report with a narrowed one would +read the difference as the suite improving. Per kind it cannot move, because the six providers own +disjoint kind sets — a kind's figure comes from its own provider or the kind is absent from the map. + +### What the terms do not sum to + +For a kind that publishes both figures: + +``` +hypothesesTested[k] + notMeasured[k].AwaitingRuns + notMeasured[k].Unreadable + = the fingerprints that kind was offered +``` + +and the tests it measured and had nothing to say about are inside `hypothesesTested`, not published +separately. That term is the common case — it is very nearly the suite — and publishing it would make +the number the test count. + ## Why the exceptions are exceptions **`SharedFailure` and `BrokenFixture` keep environmental sessions.** An environmental session *is* a diff --git a/src/Xping.Cli/Report/Contract/EnvelopeBuilder.cs b/src/Xping.Cli/Report/Contract/EnvelopeBuilder.cs index d0374f4..d8e0013 100644 --- a/src/Xping.Cli/Report/Contract/EnvelopeBuilder.cs +++ b/src/Xping.Cli/Report/Contract/EnvelopeBuilder.cs @@ -7,6 +7,7 @@ using System.Text.Json; using System.Text.Json.Nodes; using Xping.Cli.Report.Model; +using Xping.Cli.Report.Providers; using Xping.Cli.Report.Windowing; namespace Xping.Cli.Report.Contract; @@ -82,6 +83,7 @@ public static ReportEnvelope Build( Math.Max(0, tests - flagged.Count), result.ExcludedLowEvidence, result.ExcludedNotSignificant, + NotMeasured(result), context.EnvironmentalSessionCount, context.PartialSessionCount, @@ -92,6 +94,38 @@ [.. shown.Select(BuildFinding)], new TruncationDto(shown.Count, result.Findings.Count, DrillDown.ForFullReport())); } + /// + /// Projects the per-kind not-measured tally into the envelope's spelling. + /// + /// What the providers produced. + /// One entry per kind that keeps such a tally, in order. + /// + /// Enumerated over the enum rather than over the dictionary, because a dictionary's order is + /// its insertion order and that is provider execution order — which the coordinator already + /// sorts, but only by provider name. Two runs over an unchanged store have to serialise + /// byte-identically, and taking the order from the declaration is what makes that a property of + /// the code rather than of a hash. + /// + /// Keyed by nameof rather than by the enum. The registered converter camel-cases enum + /// values while JsonSerializerOptions.DictionaryKeyPolicy is unset, so a + /// key would serialise as durationRegression and disagree with + /// the DurationRegression that every finding's kind and every --kind + /// argument spell. + /// + /// + private static Dictionary NotMeasured(AnalysisResult result) + { + var published = new Dictionary(StringComparer.Ordinal); + + foreach (FindingKind kind in Enum.GetValues()) + { + if (result.NotMeasured.TryGetValue(kind, out NotMeasuredCount count)) + published[kind.ToString()] = new NotMeasuredDto(count.AwaitingRuns, count.Unreadable); + } + + return published; + } + private static WindowDto BuildWindow(AnalysisWindow window) => new( window.From, diff --git a/src/Xping.Cli/Report/Contract/ReportEnvelope.cs b/src/Xping.Cli/Report/Contract/ReportEnvelope.cs index 1673e3d..85122d6 100644 --- a/src/Xping.Cli/Report/Contract/ReportEnvelope.cs +++ b/src/Xping.Cli/Report/Contract/ReportEnvelope.cs @@ -43,7 +43,7 @@ internal sealed record ReportEnvelope( /// evidenceSessions — the denominator evidenceLevel is banded from, which is the /// runs the finding was computed over rather than the runs its test appeared in. /// - public const string CurrentSchemaVersion = "1.14"; + public const string CurrentSchemaVersion = "1.15"; } /// @@ -84,13 +84,35 @@ internal sealed record ContextDto(string? Sha, string? Branch, string? Assembly) /// Distinct tests seen in the window. /// Findings produced, before truncation. /// Those findings broken down by severity. -/// Tests no finding was raised about. +/// +/// Tests no finding was raised about. Not "tests that were checked and are fine": a test a metric +/// could not be computed for is in here too, and is what says so. +/// The unmeasured are deliberately not subtracted — which of them there are depends on which kinds +/// were asked for, and a headline count that moves with --kind would be worse than one that +/// needs a second number read beside it. +/// /// Candidates dropped for resting on too little data. /// /// Candidates dropped because their kind's comparison, charged for every fingerprint it ran on, /// no longer said anything. A large suite over a short window silences most of what it tests, and a /// reader given only an empty block cannot tell that from a suite with nothing to report. /// +/// +/// Per kind, the tests that kind could not be measured on at all, keyed by the kind as +/// findings[].kind and --kind spell it. +/// +/// Not a total and cannot be made into one. Adding the values counts a test once per question its +/// data could not answer; a reader wanting "how much of my suite could this metric read" has to +/// read one entry. The four kinds absent from the map keep no such tally — SharedFailure and +/// BrokenFixture are counted in signature groups rather than tests, and the retry kinds are +/// decided by a chain in which a later kind is never attempted once an earlier one fires. A present +/// zero is a real statement: the kind was offered tests and read every one. +/// +/// +/// Distinct from , which counts candidates a provider offered +/// and the report then withheld. These are tests no candidate ever existed for. +/// +/// /// Sessions discounted as environment failures. /// /// Sessions that covered only part of the suite — a dotnet test --filter run, or anything @@ -108,12 +130,27 @@ internal sealed record SummaryDto( int Healthy, int ExcludedLowEvidence, int ExcludedNotSignificant, + IReadOnlyDictionary NotMeasured, int EnvironmentalSessions, int PartialSessions, int IncompleteSessions, int UnreadableSessions, IReadOnlyList FailedProviders); +/// +/// Why one kind could not be measured on some of the tests it was offered. +/// +/// +/// Two numbers because only one of them is answered by waiting. A test the comparison needs seven +/// runs of and has four will have seven; a test whose every run recorded a zero median normalises +/// nothing, and another such run normalises nothing either. A script deciding whether to keep +/// collecting reads the first; a developer deciding whether their adapter is recording what the +/// metric needs reads the second. +/// +/// Tests the kind needs more runs of. +/// Tests whose recorded data cannot answer this kind's question. +internal sealed record NotMeasuredDto(int AwaitingRuns, int Unreadable); + /// /// One finding, with every value already at its published precision. /// diff --git a/src/Xping.Cli/Report/FindingCoordinator.cs b/src/Xping.Cli/Report/FindingCoordinator.cs index 234029e..93360cd 100644 --- a/src/Xping.Cli/Report/FindingCoordinator.cs +++ b/src/Xping.Cli/Report/FindingCoordinator.cs @@ -3,6 +3,8 @@ * License: [MIT] */ +using System.Collections.ObjectModel; + using Xping.Cli.Report.Model; using Xping.Cli.Report.Providers; using Xping.Cli.Report.Scoring; @@ -21,14 +23,34 @@ namespace Xping.Cli.Report; /// eleven candidates and one that had nothing to discard are the same empty block, and a reader who /// cannot tell them apart learns the wrong thing from silence. /// +/// +/// Per kind, the fingerprints that kind could not be measured on at all, split by whether waiting +/// fixes it. +/// +/// A different quantity from the two counts above, and deliberately not added to them. Those count +/// candidates a provider offered and this pass then dropped; this counts fingerprints no candidate +/// was ever offered for, because the provider could compute nothing about them. Only the first kind +/// of drop is a judgement the report made. +/// +/// +/// Per kind, and never totalled. Summing across kinds would count one test as many times as +/// there are questions its data could not answer; intersecting them collapses to nothing, because +/// a test's pass and fail are always readable and so nearly every test is measured by something. +/// Worse, either total would move with --kind, which is the one thing a count of what could +/// not be measured must not do. Per kind it cannot: the six providers own disjoint kind sets, so a +/// kind's figure comes from its own provider or the kind is absent from the map. +/// +/// internal sealed record AnalysisResult( IReadOnlyList Findings, IReadOnlyList FailedProviders, int ExcludedLowEvidence, - int ExcludedNotSignificant) + int ExcludedNotSignificant, + IReadOnlyDictionary NotMeasured) { /// Gets an empty result. - public static AnalysisResult Empty { get; } = new([], [], 0, 0); + public static AnalysisResult Empty { get; } = + new([], [], 0, 0, ReadOnlyDictionary.Empty); } /// @@ -47,6 +69,14 @@ internal sealed record AnalysisResult( /// more to the point — can it judge its own p-values against the number of times it produced one. /// It reports the size of the family it tested and this decides what that family may claim. /// +/// +/// What a provider may report about itself, on the other side of that line, is what it observed and +/// what it could not — never what either is worth. is a +/// description of coverage and not a gate on emission: no candidate appears or disappears because +/// of it, exactly as no finding's severity moves because of the denominator a provider publishes +/// beside it. The rule the division protects is that a test flagged by one metric is never silently +/// dropped by another, and a count of unanswered questions cannot drop anything. +/// /// internal sealed class FindingCoordinator(IEnumerable providers) { @@ -68,6 +98,7 @@ public AnalysisResult Run( // provider has run. var surviving = new List(); var tested = new Dictionary(); + var notMeasured = new Dictionary(); int lowEvidence = 0; @@ -102,6 +133,20 @@ public AnalysisResult Run( tested[kind] = tested.GetValueOrDefault(kind) + count; } + // The same filter, deliberately. `--kind` has to narrow a kind's family, its candidates + // and the tally of what it could not measure together, or the report answers "how much + // of the suite could this metric read" differently according to what else was asked for + // in the same invocation. + // + // The addition never actually adds across providers, because each kind is owned by + // exactly one of them. It is written as an addition anyway so that a provider split in + // two later cannot silently overwrite half its own answer. + foreach ((FindingKind kind, NotMeasuredCount count) in report.NotMeasured) + { + if (kinds == null || kinds.Contains(kind)) + notMeasured[kind] = notMeasured.GetValueOrDefault(kind) + count; + } + foreach (FindingCandidate candidate in report.Candidates) { if (kinds != null && !kinds.Contains(candidate.Kind)) @@ -162,7 +207,7 @@ public AnalysisResult Run( findings.Sort(FindingOrder.Instance); failed.Sort(StringComparer.Ordinal); - return new AnalysisResult(findings, failed, lowEvidence, notSignificant); + return new AnalysisResult(findings, failed, lowEvidence, notSignificant, notMeasured); } /// diff --git a/src/Xping.Cli/Report/Providers/DurationProvider.cs b/src/Xping.Cli/Report/Providers/DurationProvider.cs index b1f18b0..5f3459f 100644 --- a/src/Xping.Cli/Report/Providers/DurationProvider.cs +++ b/src/Xping.Cli/Report/Providers/DurationProvider.cs @@ -353,6 +353,12 @@ public ProviderReport Analyze(AnalysisContext context) var candidates = new List(); int tested = 0; + // One tally per kind, because the two are declined by different gates over different + // samples: a test can be comparable and not dispersible, or the reverse. A single figure + // would be true of neither. + NotMeasuredCount regressionNotMeasured = default; + NotMeasuredCount unstableNotMeasured = default; + // Computed once for the whole window and shared by every test in a run, which is what makes // the normalisation a property of the run rather than something each test re-derives. Dictionary medians = SessionMedians(context); @@ -362,6 +368,9 @@ public ProviderReport Analyze(AnalysisContext context) foreach (string fingerprint in context.Tests.Fingerprints) { + // Counted against neither kind. The fingerprint and the reference come out of the same + // index, so this is an inconsistency inside it rather than a measurement the data + // declined — and there is no test to put a number against in any case. TestReference? test = context.Tests.ReferenceFor(fingerprint); if (test == null) continue; @@ -399,6 +408,12 @@ public ProviderReport Analyze(AnalysisContext context) // A test that has stopped running is not a duration finding. Its absence is what is // interesting about it, and that belongs to `Vanished` — claiming it here as well would // report one disappearance twice under two names. + // + // Counted against neither kind, for the same reason. "Duration could not measure this + // test" is a true sentence about a test that has gone, and printing it would state the + // disappearance a second time in the summary, in a line whose whole purpose is to name + // questions whose answers are missing rather than questions that have been answered + // elsewhere. if (current.Count == 0) continue; @@ -428,8 +443,10 @@ public ProviderReport Analyze(AnalysisContext context) if (regression.Tested) tested++; + else + regressionNotMeasured += regression.NotMeasured; - FindingCandidate? unstable = Unstable( + Examination instability = Unstable( context, test, all, @@ -438,6 +455,10 @@ public ProviderReport Analyze(AnalysisContext context) referenceMs, currentDiscounted + baselineDiscounted); + unstableNotMeasured += instability.NotMeasured; + + FindingCandidate? unstable = instability.Candidate; + // A regression suppresses the instability finding for the same test, and the two now // overlap more than they used to: a test whose baseline swings and whose recent runs // then step clear of all of it earns both, where the retired stability gate used to @@ -465,7 +486,12 @@ public ProviderReport Analyze(AnalysisContext context) // against. return new ProviderReport( candidates, - new Dictionary { [FindingKind.DurationRegression] = tested }); + new Dictionary { [FindingKind.DurationRegression] = tested }, + new Dictionary + { + [FindingKind.DurationRegression] = regressionNotMeasured, + [FindingKind.DurationUnstable] = unstableNotMeasured + }); } /// @@ -495,10 +521,22 @@ private static Examination Regression( // disagree with the claim. A run whose own median was not positive normalises nothing, and a // run where the test itself took no measurable time contributes a reading no ratio can be // taken against; either way the arm is thinner than its session count says. + // An arm that held runs and produced no reading from any of them. A run whose own median was + // not positive is no divisor, so a test whose recent or earlier runs are all of that shape + // contributes nothing to compare however many of them there are — and the next run of the + // same shape contributes nothing either. Separated from the floor below because the two are + // different news: this one is not answered by waiting, and telling a reader it is sends them + // back in a fortnight to be told the same thing. + if ((baselineProfile.Sessions > 0 && baselineProfile.Compared.Count == 0) || + (currentProfile.Sessions > 0 && currentProfile.Compared.Count == 0)) + { + return Examination.Unreadable; + } + if (baselineProfile.Compared.Count < MinimumBaselineSessions || currentProfile.Compared.Count < MinimumCurrentSessions) { - return Examination.NotPosed; + return Examination.AwaitingRuns; } // Everything past this line is a test the provider ran on this fingerprint, and counts @@ -613,14 +651,15 @@ private static double Probability(double value) => Math.Round(value, 6, MidpointRounding.AwayFromZero); /// - /// Attempts the instability finding, returning when a gate declines it. + /// Attempts the instability finding, saying both whether it could be measured and what + /// survived. /// /// /// Measured over the whole window rather than either slice. Instability is a standing property /// of a test, not a change between two halves of its history, and splitting the window would /// only halve the evidence behind it. /// - private static FindingCandidate? Unstable( + private static Examination Unstable( AnalysisContext context, TestReference test, IReadOnlyList all, @@ -629,9 +668,22 @@ private static double Probability(double value) => double referenceMs, int discountedEnvironmental) { + // Nothing to take a dispersion of. `RobustDispersion.Of` answers zero both for a sample too + // short to have a spread and for one whose median is not positive, so reading the gate below + // without asking this first reports a test whose every run recorded a zero median as + // perfectly steady — which is #185's own example, and the reason it was filed. An unmeasured + // test and a steady one are opposite pieces of news and they used to print the same way, + // which is to say they used to print as nothing at all. + if (whole.Normalised.Count == 0 || whole.NormalisedP50 <= 0) + return Examination.Unreadable; + + // One normalised reading is a point, not a spread. More runs give it one. + if (whole.Normalised.Count < 2) + return Examination.AwaitingRuns; + double dispersion = RobustDispersion.Of(whole.Normalised); if (dispersion < LocalAnalysisConstants.DurationUnstableDispersionMin) - return null; + return Examination.Of(null); // The test's own prior behaviour where it has any, and the window where it does not. A test // first seen this week has no baseline, and refusing to measure it at all would make its @@ -645,11 +697,19 @@ private static double Probability(double value) => // labouring at a fifth of the window's speed is. double floor = against.NormalisedP50 * referenceMs; + // No run this test appeared in had a usable median, so there is no scale to express the + // floor in and the product below is zero for a reason that has nothing to do with the test + // being quick. Implied by the guard above — both readings come from the same missing + // medians — but computed over `all` rather than over `whole`, and a floor that declines + // every millisecond gate is worth saying out loud where it happens. + if (referenceMs <= 0) + return Examination.Unreadable; + // Below a few tens of milliseconds the dispersion is measuring the scheduler, not the test. if (floor < LocalAnalysisConstants.DurationTrivialMs) - return null; + return Examination.Of(null); - return new FindingCandidate( + return Examination.Of(new FindingCandidate( FindingKind.DurationUnstable, new FindingSubject.SingleTest(test), new DurationUnstableEvidence( @@ -677,7 +737,7 @@ private static double Probability(double value) => // The runs behind the normalised readings, which is what the dispersion was computed // over. Two normalisable readings of 1 and 10 clear the dispersion floor on their own, // and a test present in five runs and normalisable in two holds two runs of evidence. - EvidenceSessions: whole.NormalisedSessions); + EvidenceSessions: whole.NormalisedSessions)); } /// diff --git a/src/Xping.Cli/Report/Providers/FailureModeProvider.cs b/src/Xping.Cli/Report/Providers/FailureModeProvider.cs index 5df38fb..486cbb0 100644 --- a/src/Xping.Cli/Report/Providers/FailureModeProvider.cs +++ b/src/Xping.Cli/Report/Providers/FailureModeProvider.cs @@ -351,32 +351,50 @@ internal sealed class FailureModeProvider : IFindingProvider /// Every kind here is a count of failures that happened, so no family is reported and nothing /// downstream corrects them for multiplicity. /// - public ProviderReport Analyze(AnalysisContext context) => - ProviderReport.Observations([.. Observed(context)]); - - /// - /// Walks the window, yielding what it observed. - /// - /// The window, sessions and shared indexes. - /// Candidate findings, in any order. - private static IEnumerable Observed(AnalysisContext context) + public ProviderReport Analyze(AnalysisContext context) { ArgumentNullException.ThrowIfNull(context); + var candidates = new List(); + int awaitingRuns = 0; + IReadOnlyList clusters = FindClusters(context); var clustered = new HashSet( clusters.Select(c => c.Signature.Hash), StringComparer.Ordinal); foreach (SignatureGroup cluster in clusters) - yield return SharedFailure(context, cluster); + candidates.Add(SharedFailure(context, cluster)); foreach (string fingerprint in context.Tests.Fingerprints) { - FindingCandidate? candidate = Individual(context, fingerprint, clustered); - if (candidate != null) - yield return candidate; + Examination examination = Individual(context, fingerprint, clustered); + + awaitingRuns += examination.NotMeasured.AwaitingRuns; + + if (examination.Candidate is { } candidate) + candidates.Add(candidate); } + + // Three kinds and one number, because the three read one thing: whether this test failed on + // its own account, and how. A test whose outcomes could not be read could not be read for + // any of them. + // + // `SharedFailure` and `BrokenFixture` are absent rather than zero. They are enumerated over + // signature groups, and a count of groups published under a field every other kind counts + // tests in would be a number a reader cannot compare with the one beside it. Absence says + // this kind keeps no such tally; the published zeros elsewhere say the kind was offered + // tests and read every one. + var notMeasured = new NotMeasuredCount(awaitingRuns, 0); + + return ProviderReport.Observations( + candidates, + new Dictionary + { + [FindingKind.Flaky] = notMeasured, + [FindingKind.AlwaysFailing] = notMeasured, + [FindingKind.TimingOut] = notMeasured + }); } /// @@ -592,7 +610,7 @@ private static SharedFailureEvidence BuildSharedEvidence( /// /// Classifies one test's own failures, once the shared and environmental ones are set aside. /// - private static FindingCandidate? Individual( + private static Examination Individual( AnalysisContext context, string fingerprint, HashSet clustered) { IReadOnlyList all = context.Tests.ExecutionsOf(fingerprint); @@ -629,15 +647,27 @@ private static SharedFailureEvidence BuildSharedEvidence( } if (considered.Count == 0) - return null; + { + // Nothing of this test's own behaviour was observed. Counted only where no failure of + // its was absorbed into a cluster: a test whose every failure was reported as part of a + // shared cause has not gone unmeasured, it has been measured and reported under a group, + // and naming it here would state one finding twice. + // + // Awaiting runs rather than unreadable. What is missing is a run of this test that was + // not an outage, and the next ordinary run supplies one — unlike a duration that cannot + // be normalised, which no number of further runs of the same shape repairs. + return clusteredOut == 0 ? Examination.AwaitingRuns : Examination.Of(null); + } List failures = [.. considered.Where(e => e.Failed)]; if (failures.Count == 0) - return null; + return Examination.Of(null); + // Charged to nothing: an index that holds the fingerprint but not the reference is + // inconsistent with itself, which is not a measurement the data declined. TestReference? test = context.Tests.ReferenceFor(fingerprint); if (test == null) - return null; + return Examination.Of(null); double failureRate = (double)failures.Count / considered.Count; @@ -678,7 +708,7 @@ private static SharedFailureEvidence BuildSharedEvidence( if (timeouts.Count > 0 && (double)timeouts.Count / failures.Count >= LocalAnalysisConstants.TimingOutShareMin) { - return TimingOut( + return Examination.Of(TimingOut( context, test, considered, @@ -687,7 +717,7 @@ private static SharedFailureEvidence BuildSharedEvidence( sessionsConsidered, environmental, clusteredOut, - occasions.Count); + occasions.Count)); } // Modal rather than sole. Failure modes are compared by exact hash over the exception type, @@ -709,7 +739,7 @@ private static SharedFailureEvidence BuildSharedEvidence( failureRate >= LocalAnalysisConstants.AlwaysFailingRate && modalShare >= LocalAnalysisConstants.AlwaysFailingModalShareMin) { - return new FindingCandidate( + return Examination.Of(new FindingCandidate( FindingKind.AlwaysFailing, new FindingSubject.SingleTest(test), new AlwaysFailingEvidence( @@ -733,13 +763,13 @@ private static SharedFailureEvidence BuildSharedEvidence( LastOccurrenceIn: lastFailureIn, DrillDown.ForTest(FindingKind.AlwaysFailing, test), - EvidenceSessions: occasions.Count); + EvidenceSessions: occasions.Count)); } // Everything else that failed at all. Either the failure mode varies between runs, or one // mode occurs inconsistently — two observations that a developer investigates the same way // and that the specification names alike. - return new FindingCandidate( + return Examination.Of(new FindingCandidate( FindingKind.Flaky, new FindingSubject.SingleTest(test), new FlakyEvidence( @@ -776,7 +806,7 @@ private static SharedFailureEvidence BuildSharedEvidence( LastOccurrenceIn: lastFailureIn, DrillDown.ForTest(FindingKind.Flaky, test), - EvidenceSessions: occasions.Count); + EvidenceSessions: occasions.Count)); } /// diff --git a/src/Xping.Cli/Report/Providers/IFindingProvider.cs b/src/Xping.Cli/Report/Providers/IFindingProvider.cs index 36721c7..31b53ca 100644 --- a/src/Xping.Cli/Report/Providers/IFindingProvider.cs +++ b/src/Xping.Cli/Report/Providers/IFindingProvider.cs @@ -154,18 +154,75 @@ internal interface IFindingProvider } /// -/// What one provider observed in a window, and how many questions it asked to observe it. +/// How many fingerprints a kind could not be measured on, and which of the two reasons applies. /// /// /// -/// The second half is the reason this is a record rather than a list. A p-value on its own cannot -/// be judged: the same 0.02 is strong evidence from one comparison and the commonest thing three -/// hundred comparisons produce. The coordinator applies +/// Two numbers rather than one, because the two are not the same news and only one of them is +/// answered by waiting. A window that holds four runs of a test the comparison needs seven of will +/// hold seven eventually; a test whose every run recorded a zero median normalises nothing, and the +/// eighth such run normalises nothing either. Publishing one figure over both would repeat, one +/// level down, exactly the conflation #185 was filed about — a reader told "awaiting more runs" +/// about a test that will never be measurable learns the wrong thing and waits. +/// +/// +/// The question that decides between them at a decline site is whether another run of the same +/// shape as the ones already recorded would fix it. Nothing subtler is needed, and nothing subtler +/// is defensible: the two reasons differ in what the reader should do next, not in how the gate +/// was written. +/// +/// +/// +/// Fingerprints the kind could not be measured on for want of runs. Shrinks as a store fills. +/// +/// +/// Fingerprints whose recorded data cannot answer this kind's question at all — no reading the +/// statistic can be taken over, no second concurrency level, no clock, no signature. More runs of +/// the same shape do not help. +/// +internal readonly record struct NotMeasuredCount(int AwaitingRuns, int Unreadable) +{ + /// Gets the fingerprints not measured, for whichever reason. + public int Total => AwaitingRuns + Unreadable; + + /// Gets whether this kind measured every fingerprint it was offered. + public bool IsEmpty => AwaitingRuns == 0 && Unreadable == 0; + + /// Adds two counts. + /// One count. + /// The other. + /// Their sum, reason by reason. + public static NotMeasuredCount operator +(NotMeasuredCount left, NotMeasuredCount right) => + new(left.AwaitingRuns + right.AwaitingRuns, left.Unreadable + right.Unreadable); + + /// Adds two counts. + /// One count. + /// The other. + /// Their sum, reason by reason. + public static NotMeasuredCount Add(NotMeasuredCount left, NotMeasuredCount right) => + left + right; +} + +/// +/// What one provider observed in a window, how many questions it asked, and how many it could not. +/// +/// +/// +/// The second component is the reason this is a record rather than a list. A p-value on its own +/// cannot be judged: the same 0.02 is strong evidence from one comparison and the commonest thing +/// three hundred comparisons produce. The coordinator applies /// once per kind and needs the denominator, and only the /// provider knows it — the count includes every fingerprint whose answer never became a candidate, /// which by definition is not in . /// /// +/// The third exists for the opposite reason. A provider that declines for want of data used to leave +/// no trace at all: the summary's excluded tally counts only the candidates the coordinator itself +/// dropped at the reporting floor, so a test the provider could compute nothing about landed +/// silently inside "healthy" and was reported to a reader as fine. It is not fine — it is a test +/// nothing was concluded about, and #185 is that distinction. +/// +/// /// Eager rather than an iterator, unlike the shape this replaced. A family size is not known until /// the last fingerprint has been examined, so a provider that streamed its candidates would have to /// publish the count before it had it. Nothing is lost: the coordinator materialised every @@ -181,46 +238,124 @@ internal interface IFindingProvider /// after the gates instead would report a family in which every member is a discovery, and correct /// for nothing. /// +/// +/// Per kind, the fingerprints that kind's question could not be asked of, split by whether waiting +/// fixes it. +/// +/// The unit is a fingerprint, so a kind whose subject is not a test does not appear here at all: +/// SharedFailure and BrokenFixture are enumerated over signature groups, and a count +/// of groups published under a field every other kind counts tests in is the confusion +/// docs/internals/finding-populations.md exists to prevent. +/// +/// +/// Absence and a published zero are different statements. Absent means this kind keeps no such +/// tally; a zero means the kind was offered fingerprints and could read every one of them. Both are +/// worth saying and only one of them can be said by a number. +/// +/// internal sealed record ProviderReport( IReadOnlyList Candidates, - IReadOnlyDictionary HypothesesTested) + IReadOnlyDictionary HypothesesTested, + IReadOnlyDictionary NotMeasured) { + /// + /// A report from a provider that tested hypotheses and measured every fingerprint it was + /// offered. + /// + /// What it is claiming. + /// The family behind each kind. + public ProviderReport( + IReadOnlyList candidates, + IReadOnlyDictionary hypothesesTested) + : this(candidates, hypothesesTested, ReadOnlyDictionary.Empty) + { + } + /// /// A report from a provider that counted things rather than testing anything. /// /// What it counted. + /// What it could not count, per kind. /// A report claiming no family. /// /// Every kind the retry and failure-mode providers emit is an observation of something that /// demonstrably happened — a retry that masked a failure, a signature that knocked over four /// tests at once. There is no null hypothesis to reject and so nothing to correct for, and the /// empty family is what carries all eight of them past the multiplicity pass untouched. + /// Reporting no family says nothing about whether the observation could be made, which is why + /// the second argument is still asked for. /// - public static ProviderReport Observations(IReadOnlyList candidates) => - new(candidates, ReadOnlyDictionary.Empty); + public static ProviderReport Observations( + IReadOnlyList candidates, + IReadOnlyDictionary notMeasured) => + new(candidates, ReadOnlyDictionary.Empty, notMeasured); +} + +/// +/// Why one fingerprint produced no candidate, or that it produced one. +/// +/// +/// Both declining states yield no candidate and neither is charged to the multiplicity correction, +/// so for the family size alone one value would do. They are kept apart because the summary reads +/// them too, and a reader acts differently on the two: one empties as the store fills and the other +/// never will. +/// +internal enum Examined +{ + /// The window does not hold enough of this test yet. More runs fix it. + AwaitingRuns, + + /// Nothing this kind reads was recorded. More runs of the same shape do not. + Unreadable, + + /// The question was asked, whatever the gates after it then said. + Measured } /// /// What examining one fingerprint produced. /// /// -/// The distinction the pair exists to draw is between a fingerprint the question could not be asked -/// of and one it was asked of and answered no. Both yield no candidate, and only the second is a +/// +/// The distinction this exists to draw is between a fingerprint the question could not be asked of +/// and one it was asked of and answered no. Both yield no candidate, and only the second is a /// hypothesis test that the multiplicity correction has to be charged for. A provider returning a /// bare FindingCandidate? conflates them, and undercounting the family is the direction that /// invents findings. +/// +/// +/// Every value is now read twice: once as a family the correction is or is not charged for, and once +/// as a measurement the summary says was or was not taken. That second reading is why the declining +/// side is two values rather than one, and why kinds that test no hypothesis — DurationUnstable +/// among them — return this type at all. For those, is nobody's denominator and +/// is simply the honest word for what happened. +/// /// -/// Whether the kind's hypothesis test was computed on this fingerprint. +/// Whether the question was asked, and why not where it was not. /// What survived the gates after it, if anything did. -internal readonly record struct Examination(bool Tested, FindingCandidate? Candidate) +internal readonly record struct Examination(Examined Outcome, FindingCandidate? Candidate) { - /// Gets the result for a fingerprint the question could not be asked of. - public static Examination NotPosed { get; } + /// Gets the result for a fingerprint the window does not yet hold enough of. + public static Examination AwaitingRuns { get; } = new(Examined.AwaitingRuns, null); + + /// Gets the result for a fingerprint whose data cannot answer the question. + public static Examination Unreadable { get; } = new(Examined.Unreadable, null); + + /// Gets whether the kind's question was asked of this fingerprint. + public bool Tested => Outcome == Examined.Measured; + + /// Gets what this examination contributes to the kind's not-measured tally. + public NotMeasuredCount NotMeasured => Outcome switch + { + Examined.AwaitingRuns => new NotMeasuredCount(1, 0), + Examined.Unreadable => new NotMeasuredCount(0, 1), + _ => default + }; /// /// The result for a fingerprint the test was computed on, whatever the gates then said. /// /// The candidate, or if a gate declined it. /// An examination that counts towards the family. - public static Examination Of(FindingCandidate? candidate) => new(true, candidate); + public static Examination Of(FindingCandidate? candidate) => new(Examined.Measured, candidate); } diff --git a/src/Xping.Cli/Report/Providers/ParallelSensitiveProvider.cs b/src/Xping.Cli/Report/Providers/ParallelSensitiveProvider.cs index ff90b5b..c0ab11a 100644 --- a/src/Xping.Cli/Report/Providers/ParallelSensitiveProvider.cs +++ b/src/Xping.Cli/Report/Providers/ParallelSensitiveProvider.cs @@ -255,6 +255,7 @@ public ProviderReport Analyze(AnalysisContext context) var candidates = new List(); int tested = 0; + NotMeasuredCount notMeasured = default; // Fingerprints are ordinal-sorted by the index, so findings come out in the same sequence on // every run whatever order the sessions were read in. @@ -264,14 +265,25 @@ public ProviderReport Analyze(AnalysisContext context) if (examination.Tested) tested++; + else + notMeasured += examination.NotMeasured; if (examination.Candidate is { } candidate) candidates.Add(candidate); } + // The awaiting half is structurally zero and published as such. This kind has no session + // floor — the paragraph on `Examine` says why — so nothing here is ever declined for want of + // runs of the shape already recorded. What it declines, it declines because the suite never + // varied its concurrency, and a hundred more single-threaded runs answer that no better than + // the twenty already read. return new ProviderReport( candidates, - new Dictionary { [FindingKind.ParallelSensitive] = tested }); + new Dictionary { [FindingKind.ParallelSensitive] = tested }, + new Dictionary + { + [FindingKind.ParallelSensitive] = notMeasured + }); } /// @@ -299,11 +311,17 @@ private static Examination Examine(AnalysisContext context, string fingerprint) List considered = population.Considered; ConcurrencyRange range = Range(considered); - // A test whose concurrency never varied. There is no trend to test for and so nothing to - // charge the correction with: a fingerprint that never ran at two levels is not a - // comparison this provider made and lost, it is one it could not make. + // A test whose concurrency never varied, or whose executions carried no concurrency at all. + // There is no trend to test for and so nothing to charge the correction with: a fingerprint + // that never ran at two levels is not a comparison this provider made and lost, it is one it + // could not make. + // + // Unreadable rather than awaiting runs, and the distinction is the whole of what a reader + // does with the number. More runs of a suite that always executes at one level produce more + // readings at one level; what this needs is a run at a different one, or an adapter that + // records the level at all. if (range.DistinctLevels < 2) - return Examination.NotPosed; + return Examination.Unreadable; List points = [.. considered.Select(m => new TrendPoint(m.Concurrency, m.Reference.Failed, m.Reference.SessionIndex))]; @@ -327,6 +345,11 @@ private static Examination Examine(AnalysisContext context, string fingerprint) if (Math.Abs(tau) < LocalAnalysisConstants.ParallelSensitivityTau) return Examination.Of(null); + // Left as a tested fingerprint deliberately, unlike the same condition in the duration and + // time providers. Those read it before any statistic; here `CochranArmitage.Of` has already + // run, so the comparison genuinely was made and the family it was made in has to say so. + // Removing it from the denominator after the fact is the one direction a multiplicity + // correction cannot be adjusted in. TestReference? test = context.Tests.ReferenceFor(fingerprint); if (test == null) return Examination.Of(null); diff --git a/src/Xping.Cli/Report/Providers/RetryProvider.cs b/src/Xping.Cli/Report/Providers/RetryProvider.cs index 6adc7de..27354f6 100644 --- a/src/Xping.Cli/Report/Providers/RetryProvider.cs +++ b/src/Xping.Cli/Report/Providers/RetryProvider.cs @@ -355,25 +355,23 @@ internal sealed class RetryProvider : IFindingProvider /// /// /// Every kind here is a count of attempts that happened, so no family is reported and nothing - /// downstream corrects them for multiplicity. + /// downstream corrects them for multiplicity. Reporting no family says nothing about coverage, + /// which is counted separately and shared by all three kinds — see the remark inside. /// - public ProviderReport Analyze(AnalysisContext context) => - ProviderReport.Observations([.. Observed(context)]); - - /// - /// Walks the window, yielding what it observed. - /// - /// The window, sessions and shared indexes. - /// Candidate findings, in any order. - private static IEnumerable Observed(AnalysisContext context) + public ProviderReport Analyze(AnalysisContext context) { ArgumentNullException.ThrowIfNull(context); + var candidates = new List(); + int awaitingRuns = 0; + var currentSessions = new HashSet( context.Window.CurrentSlice.Select(s => s.SessionId)); foreach (string fingerprint in context.Tests.Fingerprints) { + // Charged to nothing: the fingerprint and the reference come from one index, so this is + // an inconsistency inside it and not a measurement the data declined. TestReference? test = context.Tests.ReferenceFor(fingerprint); if (test == null) continue; @@ -381,6 +379,21 @@ private static IEnumerable Observed(AnalysisContext context) IReadOnlyList executions = context.Tests.ExecutionsOf(fingerprint); List runs = RunsOf(context, executions); + // Every run of this test was an outage. All three kinds read the same reduction, and + // none of them can read a test whose every occasion was discounted — so the tally is + // taken here, at the one precondition the three share, rather than per kind. + // + // Deliberately not per kind past this point. The chain below stops at the first kind + // that fires, so whether a later kind could have been measured on this test is a + // question the algorithm never asks; answering it for the tally alone would mean running + // all three on every fingerprint to fill in a number nobody reads. One shared + // precondition is what these three kinds honestly have to say about their coverage. + if (runs.Count == 0 || runs.TrueForAll(r => r.Discounted)) + { + awaitingRuns++; + continue; + } + // One test, one finding. A test qualifying for two of these kinds has not done two // things: it has done one thing that two thresholds both noticed. FindingCandidate? candidate = @@ -389,8 +402,23 @@ private static IEnumerable Observed(AnalysisContext context) Masked(context, test, executions, runs); if (candidate != null) - yield return candidate; + candidates.Add(candidate); } + + // Awaiting runs rather than unreadable, for every one of them. An attempt number is recorded + // by every adapter that records an execution at all, so there is no shape of retry data this + // provider cannot read; what it can be short of is a run of this test that was not an + // outage, and the next ordinary run supplies one. + var notMeasured = new NotMeasuredCount(awaitingRuns, 0); + + return ProviderReport.Observations( + candidates, + new Dictionary + { + [FindingKind.RetryMasked] = notMeasured, + [FindingKind.RetryDeepening] = notMeasured, + [FindingKind.RetryExhausted] = notMeasured + }); } // ------------------------------------------------------------------------------------------- diff --git a/src/Xping.Cli/Report/Providers/TimeSensitiveProvider.cs b/src/Xping.Cli/Report/Providers/TimeSensitiveProvider.cs index 71b9183..a0b6156 100644 --- a/src/Xping.Cli/Report/Providers/TimeSensitiveProvider.cs +++ b/src/Xping.Cli/Report/Providers/TimeSensitiveProvider.cs @@ -264,29 +264,49 @@ public ProviderReport Analyze(AnalysisContext context) var candidates = new List(); int tested = 0; + NotMeasuredCount notMeasured = default; // Built once for the window rather than once per test: every test in a session shares its // clock reading, and resolving it per fingerprint would repeat the same arithmetic for each // of a suite's several hundred tests. Dictionary clocks = ClocksIn(context); + // Every fingerprint, not zero. No session in the window recorded a UTC offset, so no test in + // it could be placed on a clock — which is the same statement the per-fingerprint gate below + // would have made one test at a time, and it has to come out as the same number. A summary + // whose unmeasured count collapsed the moment one session happened to record an offset would + // be describing the store's metadata rather than the suite. if (clocks.Count == 0) - return Report(candidates, tested); + { + return Report( + candidates, + tested, + new NotMeasuredCount(0, context.Tests.Fingerprints.Count)); + } // Fingerprints are ordinal-sorted by the index, so findings come out in the same sequence on // every run whatever order the sessions were read in. foreach (string fingerprint in context.Tests.Fingerprints) { - Examination examination = Examine(context, clocks, fingerprint); + // Resolved here rather than inside `Examine`, so that a fingerprint the index cannot + // resolve leaves the examination alone. It is an inconsistency inside the index and not + // a question the data declined, and it belongs in neither the family nor the tally. + TestReference? test = context.Tests.ReferenceFor(fingerprint); + if (test == null) + continue; + + Examination examination = Examine(context, clocks, test, fingerprint); if (examination.Tested) tested++; + else + notMeasured += examination.NotMeasured; if (examination.Candidate is { } candidate) candidates.Add(candidate); } - return Report(candidates, tested); + return Report(candidates, tested, notMeasured); } /// @@ -294,9 +314,14 @@ public ProviderReport Analyze(AnalysisContext context) /// /// Tests some axis separated. /// Tests at least one axis could be judged on. + /// Tests no axis could be built for, and why not. /// The provider's report. - private static ProviderReport Report(IReadOnlyList candidates, int tested) => - new(candidates, new Dictionary { [FindingKind.TimeSensitive] = tested }); + private static ProviderReport Report( + IReadOnlyList candidates, int tested, NotMeasuredCount notMeasured) => + new( + candidates, + new Dictionary { [FindingKind.TimeSensitive] = tested }, + new Dictionary { [FindingKind.TimeSensitive] = notMeasured }); /// /// Reads the local clock of every session that recorded one. @@ -337,26 +362,31 @@ private static Dictionary ClocksIn(AnalysisContext context) /// Examines one test, saying both whether any axis could be judged and what survived. /// private static Examination Examine( - AnalysisContext context, Dictionary clocks, string fingerprint) + AnalysisContext context, + Dictionary clocks, + TestReference test, + string fingerprint) { Population population = Considered(context, clocks, fingerprint); List considered = population.Considered; + // Not one run of this test sits on a clock. More runs of a suite whose sessions record no + // offset produce more runs that cannot be placed, so this is not answered by waiting. + if (considered.Count == 0) + return Examination.Unreadable; + // Two arms' worth is the least that can be split at all, and checking here saves the axis // work for the overwhelming majority of tests. if (considered.Count < LocalAnalysisConstants.TimeSensitiveMinArmSessions * 2) - return Examination.NotPosed; + return Examination.AwaitingRuns; // One zone for the whole comparison. A machine that moved between zones has two populations // in it, and a local hour drawn from both describes neither; the offset axis in particular - // would read the move as a daylight-saving shift. + // would read the move as a daylight-saving shift. Unreadable rather than awaiting: the runs + // already recorded are the two populations, and adding to either does not merge them. string? zone = SingleZone(considered); if (zone == null) - return Examination.NotPosed; - - TestReference? test = context.Tests.ReferenceFor(fingerprint); - if (test == null) - return Examination.NotPosed; + return Examination.Unreadable; // Every division this test's runs admit, before any of them is judged. The multiplicity the // search has to be charged for has to be known before the first p-value is computed, which @@ -365,8 +395,12 @@ private static Examination Examine( // second multiplicity, one test per fingerprint rather than one per axis. List partitions = [.. Offered(considered)]; + // Enough runs on a clock, and no division of them that puts a floor's worth on each side — + // every run in one evening, say. Another run may fall on the other side of some axis, but + // the runs already recorded contain no such split, and this is the honest reading of what + // is here rather than a prediction about what arrives next. if (partitions.Count == 0) - return Examination.NotPosed; + return Examination.Unreadable; int comparisons = Comparisons(partitions); diff --git a/src/Xping.Cli/Report/Providers/VanishedProvider.cs b/src/Xping.Cli/Report/Providers/VanishedProvider.cs index 0326397..cefa508 100644 --- a/src/Xping.Cli/Report/Providers/VanishedProvider.cs +++ b/src/Xping.Cli/Report/Providers/VanishedProvider.cs @@ -99,6 +99,7 @@ public ProviderReport Analyze(AnalysisContext context) { var candidates = new List(); int tested = 0; + int awaitingRuns = 0; AnalysisWindowSlices slices = AnalysisWindowSlices.From(context); @@ -108,7 +109,7 @@ public ProviderReport Analyze(AnalysisContext context) // coordinator's Benjamini-Hochberg pass is handed a kind that asked nothing instead of a // kind that asked three hundred questions and liked none of the answers. if (slices.BaselineCount == 0 || slices.CurrentCount == 0) - return Report(candidates, tested); + return Report(candidates, tested, context.Tests.Fingerprints.Count); foreach (string fingerprint in context.Tests.Fingerprints) { @@ -117,6 +118,7 @@ public ProviderReport Analyze(AnalysisContext context) if (!slices.BaselineAppearances.TryGetValue(fingerprint, out int appearances) || appearances < LocalAnalysisConstants.VanishedMinBaselineSessions) { + awaitingRuns++; continue; } @@ -127,6 +129,14 @@ public ProviderReport Analyze(AnalysisContext context) // answered no. Counting only the absences would describe a family in which every member // is a discovery and correct for nothing — a suite of three hundred stable tests holding // one absence would report m = 1 and pass it through untouched. + // Ahead of the Fisher test rather than after it, unlike the two gates below. A + // fingerprint the index cannot resolve to a test is an inconsistency inside the index + // and not a question the data declined, so it is charged to neither the family nor the + // tally — and reading it here keeps it out of both. + TestReference? reference = context.Tests.ReferenceFor(fingerprint); + if (reference == null) + continue; + tested++; if (slices.Current.Contains(fingerprint)) @@ -142,10 +152,6 @@ public ProviderReport Analyze(AnalysisContext context) if (pValue > LocalAnalysisConstants.VanishedAlpha) continue; - TestReference? reference = context.Tests.ReferenceFor(fingerprint); - if (reference == null) - continue; - IReadOnlyList executions = context.Tests.ExecutionsOf(fingerprint); // Executions arrive newest-session-first, so the head is the last time it ran. @@ -197,7 +203,7 @@ public ProviderReport Analyze(AnalysisContext context) SeverityCeiling: Severity.Low)); } - return Report(candidates, tested); + return Report(candidates, tested, awaitingRuns); } /// @@ -205,9 +211,27 @@ public ProviderReport Analyze(AnalysisContext context) /// /// Absences the gate let through. /// Fingerprints the absence was measured on. + /// Fingerprints the baseline had not seen enough of to ask. /// The provider's report. - private static ProviderReport Report(IReadOnlyList candidates, int tested) => - new(candidates, new Dictionary { [FindingKind.Vanished] = tested }); + /// + /// Nothing is ever unreadable here, and the published zero says so rather than leaving a reader + /// to wonder. A session appearance is the one observation every adapter records by existing: a + /// test either ran in a run or it did not, and there is no shape of data this kind cannot read. + /// Everything it declines, it declines for want of history — which is why the whole-window + /// return above expands to every fingerprint rather than to nothing. A window that holds no + /// baseline has not measured the suite and could not have; the number has to be the same whether + /// that was decided once at the top or once per fingerprint below, or a reader comparing two + /// reports would see a suite's unmeasured count collapse the moment a second full run arrived. + /// + private static ProviderReport Report( + IReadOnlyList candidates, int tested, int awaitingRuns) => + new( + candidates, + new Dictionary { [FindingKind.Vanished] = tested }, + new Dictionary + { + [FindingKind.Vanished] = new NotMeasuredCount(awaitingRuns, 0) + }); } /// diff --git a/src/Xping.Cli/Report/Rendering/TextReportRenderer.cs b/src/Xping.Cli/Report/Rendering/TextReportRenderer.cs index c617b3a..9604433 100644 --- a/src/Xping.Cli/Report/Rendering/TextReportRenderer.cs +++ b/src/Xping.Cli/Report/Rendering/TextReportRenderer.cs @@ -6,6 +6,7 @@ using System.Globalization; using System.Text; using Xping.Cli.Report.Contract; +using Xping.Cli.Report.Model; namespace Xping.Cli.Report.Rendering; @@ -42,6 +43,12 @@ internal sealed class TextReportRenderer(OutputCapabilities capabilities) : IRep /// private const int FenceWidth = 72; + // Kinds named on the unmeasured line before the rest become a count. Three is what fits beside + // the glyph and the prefix at the widths `ReportVocabulary` labels take; a fourth pushes the + // line past the fence it sits above, and a reader scanning four numbers for the largest is + // being given a table one segment at a time. + private const int UnmeasuredKindsShown = 3; + private const string Fence = "```"; // Width of the " | " the trailer's segments are joined with. @@ -116,9 +123,74 @@ private void WriteHeader(StringBuilder builder, ReportEnvelope envelope) builder.AppendLine(string.Join(separator, counts)); + WriteUnmeasured(builder, summary, separator); WriteCaveats(builder, envelope, separator); } + /// + /// Writes the questions this window's data could not answer at all. + /// + /// + /// + /// Its own line, because it is neither of the things the two lines around it are. The counts + /// line holds suite-wide totals of candidates the report saw and withheld; this is per kind and + /// counts tests no candidate ever existed for. The caveat line holds things that went wrong; + /// a question the recorded data cannot answer is the report working. And thirteen kinds cannot + /// be appended to a line of totals in any case. + /// + /// + /// Only the unreadable half is printed. The counts line already says "awaiting more + /// runs", and a second waiting figure beside it in a different unit — tests here, candidates + /// there — is the confusion this whole change exists to remove. The awaiting half is published + /// in the JSON envelope, where a caller can read the two apart by name. + /// + /// + /// Three kinds and then a count of the rest. At the widths these labels take, three segments + /// and the glyph land around the fence's own width; four are reliably past it, and a reader who + /// has to scan four numbers to find the big one would have been better served by the JSON. + /// + /// + private void WriteUnmeasured(StringBuilder builder, SummaryDto summary, string separator) + { + // Count descending so the largest gap is read first, then by the enum's own order so that + // two kinds with equal counts resolve the same way on every run. Without the tie-break the + // line would depend on dictionary order and two reports over one store could differ. + List> unreadable = + [ + .. summary.NotMeasured + .Where(entry => entry.Value.Unreadable > 0) + .OrderByDescending(entry => entry.Value.Unreadable) + .ThenBy(entry => KindOrder(entry.Key)) + ]; + + if (unreadable.Count == 0) + return; + + var segments = new List(); + + foreach (KeyValuePair entry in unreadable.Take(UnmeasuredKindsShown)) + { + segments.Add( + $"{ReportVocabulary.LabelFor(entry.Key)} " + + entry.Value.Unreadable.ToString(CultureInfo.InvariantCulture)); + } + + if (unreadable.Count > UnmeasuredKindsShown) + segments.Add($"+{unreadable.Count - UnmeasuredKindsShown} more"); + + builder.Append(capabilities.Glyphs.Pending).Append(' ') + .Append("nothing to measure: ") + .AppendLine(string.Join(separator, segments)); + } + + /// + /// Orders a kind by its declaration, for a tie-break that cannot vary between runs. + /// + /// The kind, as the envelope spells it. + /// Its position, or one past the end for a kind this build does not know. + private static int KindOrder(string kind) => + Enum.TryParse(kind, out FindingKind parsed) ? (int)parsed : int.MaxValue; + /// /// Writes the ways in which the report saw less than it wanted to. /// @@ -201,6 +273,15 @@ private string EmptyReport(SummaryDto summary) if (summary.ExcludedNotSignificant > 0) reasons.Add($"{summary.ExcludedNotSignificant} could be chance"); + // Counted in kinds, not tests. The sentence is answering "why is this block empty", and the + // answer is which questions went unasked; how many tests each of them covers is on the line + // above, in the unit that line uses. A suite where every question was unanswerable used to + // reach here and print a green "No findings.", which is the reading #185 was filed about. + int silent = summary.NotMeasured.Count(entry => entry.Value.Unreadable > 0); + + if (silent > 0) + reasons.Add($"{silent} {KindWord(silent)} had nothing to measure"); + return reasons.Count == 0 ? $"{capabilities.Glyphs.Pass} No findings." : $"{capabilities.Glyphs.Pending} Nothing reportable yet: " + @@ -344,6 +425,8 @@ private static string Revision(ContextDto? context) private static string NeedWord(int count) => count == 1 ? "needs" : "need"; + private static string KindWord(int count) => count == 1 ? "kind" : "kinds"; + private static string Format(DateTime value, string format) => value.ToString(format, CultureInfo.InvariantCulture); diff --git a/tests/Xping.Cli.Tests/Commands/CliSurfaceTests.cs b/tests/Xping.Cli.Tests/Commands/CliSurfaceTests.cs index 6c295fb..5ce6e2f 100644 --- a/tests/Xping.Cli.Tests/Commands/CliSurfaceTests.cs +++ b/tests/Xping.Cli.Tests/Commands/CliSurfaceTests.cs @@ -328,7 +328,7 @@ public void JsonEmitsTheVersionedEnvelope() using JsonDocument doc = JsonDocument.Parse(output); JsonElement root = doc.RootElement; - Assert.Equal("1.14", root.GetProperty("schemaVersion").GetString()); + Assert.Equal("1.15", root.GetProperty("schemaVersion").GetString()); Assert.Equal(6, root.GetProperty("window").GetProperty("sessionCount").GetInt32()); Assert.Equal("default", root.GetProperty("window").GetProperty("resolution").GetString()); Assert.Equal(1, root.GetProperty("summary").GetProperty("tests").GetInt32()); @@ -344,7 +344,7 @@ public void JsonIsStillTheEnvelopeWhenSelectedByTheLegacyFlag() Assert.Equal(0, code); using JsonDocument doc = JsonDocument.Parse(output); - Assert.Equal("1.14", doc.RootElement.GetProperty("schemaVersion").GetString()); + Assert.Equal("1.15", doc.RootElement.GetProperty("schemaVersion").GetString()); } [Fact] diff --git a/tests/Xping.Cli.Tests/Report/DurationProviderTests.cs b/tests/Xping.Cli.Tests/Report/DurationProviderTests.cs index a09ea5e..9db95e5 100644 --- a/tests/Xping.Cli.Tests/Report/DurationProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/DurationProviderTests.cs @@ -791,6 +791,80 @@ public void AWindowWithNoUsableRunMedianProducesNoDurationFindingOfEitherKind() Assert.Empty(Analyze(context)); } + /// + /// A test nothing could be computed about is counted, not called healthy. + /// + /// + /// The case #185 was filed from. Both gates decline, and until they said so the test fell + /// through every count in the summary and landed inside "healthy" — which tells a reader it was + /// looked at and is fine. It was not looked at: neither statistic exists for it. And no number + /// of further runs of the same shape produces one, so it belongs on the unreadable side rather + /// than beside the candidates that are only waiting for the store to fill. + /// + [Fact] + public void ATestWhoseRunsAllRecordedAZeroMedianIsCountedRatherThanSilentlyCalledHealthy() + { + AnalysisContext context = Build( + sessions: 10, + subjectMs: o => o < 7 ? 200 : 800, + companionMs: _ => 0); + + Assert.Empty(Analyze(context)); + + // Every test in the fixture, subject and companions alike, since no run in the window has a + // divisor and so nothing in it can be normalised. + int tests = context.Tests.Fingerprints.Count; + + NotMeasuredCount regression = NotMeasured(context, FindingKind.DurationRegression); + NotMeasuredCount unstable = NotMeasured(context, FindingKind.DurationUnstable); + + Assert.Equal(tests, regression.Unreadable); + Assert.Equal(tests, unstable.Unreadable); + + Assert.Equal(0, regression.AwaitingRuns); + Assert.Equal(0, unstable.AwaitingRuns); + } + + /// + /// A test the provider measured and had nothing to say about is not counted. + /// + /// + /// The other half of the distinction, and the one that decides whether the number means + /// anything: counting a steady test as unmeasured would make the tally the test count, and the + /// line would tell a reader nothing they could act on. + /// + [Fact] + public void ASteadyTestIsMeasuredAndThereforeCountedNowhere() + { + AnalysisContext context = Build(sessions: 20, subjectMs: _ => 200); + + Assert.Empty(Analyze(context)); + + Assert.True(NotMeasured(context, FindingKind.DurationRegression).IsEmpty); + Assert.True(NotMeasured(context, FindingKind.DurationUnstable).IsEmpty); + } + + /// + /// A test the window has not seen enough of is waiting, not unreadable. + /// + /// + /// The two reasons differ in what a reader does next. This one empties as the store fills; the + /// zero-median case above never will, and telling someone to wait for it would send them back in + /// a fortnight to be told the same thing. + /// + [Fact] + public void AThinBaselineIsCountedAsAwaitingRunsRatherThanAsUnreadable() + { + // Six comparable baseline runs against the three recent ones, one short of the seven the + // comparison needs. Every duration normalises; there is simply not enough history yet. + AnalysisContext context = Build(sessions: 9, subjectMs: o => o < 3 ? 800 : 200); + + NotMeasuredCount regression = NotMeasured(context, FindingKind.DurationRegression); + + Assert.Equal(0, regression.Unreadable); + Assert.True(regression.AwaitingRuns > 0); + } + [Fact] public void AnInstantBaselineProducesNoRegressionRatherThanAnInfinity() { @@ -1318,6 +1392,9 @@ private static IReadOnlyList Unstables(AnalysisContext context private static int Family(AnalysisContext context, FindingKind kind) => new DurationProvider().Analyze(context).HypothesesTested.GetValueOrDefault(kind); + private static NotMeasuredCount NotMeasured(AnalysisContext context, FindingKind kind) => + new DurationProvider().Analyze(context).NotMeasured.GetValueOrDefault(kind); + /// /// Asserts that exactly one candidate was produced, and returns it. /// diff --git a/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs b/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs index 09617d5..3c8b9a1 100644 --- a/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs @@ -73,9 +73,118 @@ private static TestExecution FailedAttempt(string name, int attempt, int maxRetr errorMessage: "unexpected null", stackTrace: $" at MyApp.Tests.SampleTests.{name}()"); + /// + /// A window that was one long outage is counted, test by test. + /// + /// + /// #185: nothing of any test's own behaviour was observed here — every occasion of every one of + /// them was discounted as environmental — and until the tally existed the whole suite came out + /// of this window reported as healthy. Awaiting runs rather than unreadable: what is missing is + /// an ordinary run, and the next one supplies it. + /// + /// SharedFailure and BrokenFixture keep no tally at all. They are counted in + /// signature groups, and a count of groups under a field every other kind counts tests in is not + /// a number a reader can compare with the one beside it — absence says so where a zero would + /// claim the kind had looked. + /// + /// + [Fact] + public void AWindowThatWasOneLongOutageIsCountedTestByTest() + { + TestSession[] sessions = EveryRunAnOutage(); + + int tests = TestSessionFactory.Context(sessions).Tests.Fingerprints.Count; + + foreach (FindingKind kind in + (FindingKind[]) + [FindingKind.Flaky, FindingKind.AlwaysFailing, FindingKind.TimingOut]) + { + NotMeasuredCount count = NotMeasured(kind, sessions); + + Assert.Equal(tests, count.AwaitingRuns); + Assert.Equal(0, count.Unreadable); + } + + Assert.False( + new FailureModeProvider().Analyze(TestSessionFactory.Context(sessions)) + .NotMeasured.ContainsKey(FindingKind.SharedFailure)); + } + + /// + /// A test the shared cause absorbed is reported, not counted as unmeasured. + /// + /// + /// The discrimination the tally has to make. A test whose every failure was attributed to a + /// signature shared across the suite has not gone unmeasured — it is on the page, under the + /// group — and naming it here as well would state one finding twice in a line whose whole + /// purpose is to name questions whose answers are missing. + /// + [Fact] + public void ATestWhoseEveryFailureWentIntoAClusterIsNotCountedAsUnmeasured() + { + // Four of thirty down together in every run: enough tests to be one cause, and far short of + // the ten failures and three in ten that would read as an outage. + TestSession[] sessions = + [ + .. Enumerable.Range(0, 6).Select(ordinal => TestSessionFactory.Session( + ordinal, + [ + .. Enumerable.Range(0, 4).Select(i => SharedFailure($"Down{i}")), + .. Enumerable.Range(4, 26).Select(i => TestSessionFactory.Execution($"Fine{i}")) + ])) + ]; + + Assert.Equal(0, TestSessionFactory.Context(sessions).EnvironmentalSessionCount); + Assert.Contains(Analyze(sessions), c => c.Kind == FindingKind.SharedFailure); + + Assert.True(NotMeasured(FindingKind.Flaky, sessions).IsEmpty); + } + + /// + /// Builds a window in which every run looked like an outage. + /// + /// + /// Twelve of thirty-one tests down in every one of six runs, which is what SessionView.For + /// reads as environmental. The subject passes throughout, and is discounted with everything + /// else: an environmental run is a run no test's own behaviour was observed on. + /// + private static TestSession[] EveryRunAnOutage() => + [ + .. Enumerable.Range(0, 6).Select(ordinal => TestSessionFactory.Session( + ordinal, + [ + TestSessionFactory.Execution("Subject"), + .. Enumerable.Range(0, 12).Select(i => SharedFailure($"Down{i}")), + .. Enumerable.Range(12, 18).Select(i => TestSessionFactory.Execution($"Fine{i}")) + ])) + ]; + + /// + /// A test that simply never failed was measured, and is counted nowhere. + /// + /// + /// The common case, and the one that decides whether the number means anything: counting every + /// green test would make the tally the test count. + /// + [Fact] + public void ATestThatNeverFailedIsMeasuredAndCountedNowhere() + { + TestSession[] sessions = + [ + .. Enumerable.Range(0, 6).Select(ordinal => + TestSessionFactory.Session(ordinal, "Green")) + ]; + + Assert.Empty(Analyze(sessions)); + Assert.True(NotMeasured(FindingKind.Flaky, sessions).IsEmpty); + } private static List Analyze(params TestSession[] sessions) => [.. new FailureModeProvider().Analyze(TestSessionFactory.Context(sessions)).Candidates]; + private static NotMeasuredCount NotMeasured(FindingKind kind, params TestSession[] sessions) => + new FailureModeProvider().Analyze(TestSessionFactory.Context(sessions)) + .NotMeasured.GetValueOrDefault(kind); + private static FindingCandidate Single(List candidates, FindingKind kind) => Assert.Single(candidates, c => c.Kind == kind); diff --git a/tests/Xping.Cli.Tests/Report/FindingCoordinatorTests.cs b/tests/Xping.Cli.Tests/Report/FindingCoordinatorTests.cs index 7ead725..c8cbcaa 100644 --- a/tests/Xping.Cli.Tests/Report/FindingCoordinatorTests.cs +++ b/tests/Xping.Cli.Tests/Report/FindingCoordinatorTests.cs @@ -3,6 +3,8 @@ * License: [MIT] */ +using System.Collections.ObjectModel; + using Xping.Cli.Report; using Xping.Cli.Report.Model; using Xping.Cli.Report.Providers; @@ -190,6 +192,92 @@ StubProvider[] Providers() => Assert.Equal(0, narrowed.ExcludedNotSignificant); } + /// + /// Narrowing the report to one kind does not change what that kind could not measure. + /// + /// + /// The third of #185's criteria, and the reason the tally is per kind rather than a total. A + /// figure that shrank whenever `--kind` was passed would be describing the invocation instead of + /// the store, and a reader comparing yesterday's full report with today's narrowed one would + /// read the difference as the suite improving. The filter is applied to the tally with the same + /// condition it is applied to the family and the candidates, which is what makes this hold. + /// + [Fact] + public void TheNotMeasuredTallyForAKindIsUnchangedWhenTheReportIsNarrowedToIt() + { + StubProvider[] Providers() => + [ + new("time", FindingKind.TimeSensitive, "Test0", awaitingRuns: 11, unreadable: 4), + new("concurrency", FindingKind.ParallelSensitive, "Test0", unreadable: 97) + ]; + + using var warnings = new StringWriter(); + + AnalysisResult everything = new FindingCoordinator(Providers()).Run(Context(), null, warnings); + + AnalysisResult narrowed = new FindingCoordinator(Providers()).Run( + Context(), new HashSet { FindingKind.TimeSensitive }, warnings); + + Assert.Equal( + new NotMeasuredCount(11, 4), + everything.NotMeasured[FindingKind.TimeSensitive]); + + Assert.Equal( + new NotMeasuredCount(11, 4), + narrowed.NotMeasured[FindingKind.TimeSensitive]); + + // And the kind that was filtered out contributes nothing rather than a zero, so a reader + // cannot mistake "not asked about" for "asked, and every test was readable". + Assert.True(everything.NotMeasured.ContainsKey(FindingKind.ParallelSensitive)); + Assert.False(narrowed.NotMeasured.ContainsKey(FindingKind.ParallelSensitive)); + } + + /// + /// A candidate the floor dropped is not also counted as one nothing could be measured about. + /// + /// + /// The two are opposite statements and #185 exists because they were being told apart nowhere. A + /// candidate at the floor is a claim the provider computed and this pass withheld for resting on + /// too little of the test's history; the tally counts tests no claim was ever computed for. A + /// candidate that reaches the coordinator at all has been measured by definition. + /// + [Fact] + public void ACandidateDroppedAtTheFloorIsNotAlsoCountedAsUnmeasured() + { + var coordinator = new FindingCoordinator( + [new StubProvider("stub", FindingKind.TimeSensitive, "Test0")]); + + using var warnings = new StringWriter(); + AnalysisResult result = coordinator.Run(Context(sessionCount: 4), null, warnings); + + Assert.Equal(1, result.ExcludedLowEvidence); + Assert.True(result.NotMeasured[FindingKind.TimeSensitive].IsEmpty); + } + + /// + /// A provider that throws costs its own tally and nobody else's. + /// + /// + /// The same contract the candidates already have. A metric that fell over has said nothing about + /// its coverage, and publishing a zero for it would say the opposite — that it looked at every + /// test and could read them all. Absence is what the caveat line's "metrics unavailable" is for. + /// + [Fact] + public void AProviderThatThrowsContributesNoTallyAndDoesNotDisturbAnother() + { + var coordinator = new FindingCoordinator( + [ + new ThrowingProvider(), + new StubProvider("stub", FindingKind.TimeSensitive, "Test0", unreadable: 5) + ]); + + using var warnings = new StringWriter(); + AnalysisResult result = coordinator.Run(Context(), null, warnings); + + Assert.Equal(5, result.NotMeasured[FindingKind.TimeSensitive].Unreadable); + Assert.False(result.NotMeasured.ContainsKey(FindingKind.DurationRegression)); + } + /// /// A claim the pass silences hands over to the weaker one its provider was holding back. /// @@ -530,7 +618,9 @@ private sealed class StubProvider( double unreliability = 0.5, double? pValue = null, int hypothesesTested = 0, - int? evidenceSessions = null) + int? evidenceSessions = null, + int awaitingRuns = 0, + int unreadable = 0) : IFindingProvider { public string Name { get; } = name; @@ -548,9 +638,14 @@ public ProviderReport Analyze(AnalysisContext context) if (hypothesesTested > 0) family[kind] = hypothesesTested; + var notMeasured = new Dictionary + { + [kind] = new NotMeasuredCount(awaitingRuns, unreadable) + }; + TestReference? reference = context.Tests.ReferenceFor($"fp-{test}"); if (reference == null) - return new ProviderReport([], family); + return new ProviderReport([], family, notMeasured); return new ProviderReport( [ @@ -565,7 +660,8 @@ public ProviderReport Analyze(AnalysisContext context) evidenceSessions ?? context.Tests.SessionsRunIn($"fp-{test}"), PValue: pValue) ], - family); + family, + notMeasured); } } @@ -634,7 +730,8 @@ private sealed class ThrowingLazyProvider : IFindingProvider public IReadOnlyList Kinds => [FindingKind.ParallelSensitive]; public ProviderReport Analyze(AnalysisContext context) => - ProviderReport.Observations([.. Candidates()]); + ProviderReport.Observations( + [.. Candidates()], ReadOnlyDictionary.Empty); private static IEnumerable Candidates() { diff --git a/tests/Xping.Cli.Tests/Report/ParallelSensitiveProviderTests.cs b/tests/Xping.Cli.Tests/Report/ParallelSensitiveProviderTests.cs index dbca69c..cc2c4f5 100644 --- a/tests/Xping.Cli.Tests/Report/ParallelSensitiveProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/ParallelSensitiveProviderTests.cs @@ -35,6 +35,44 @@ public void ATestThatFailsOnlyWhenTheSuiteIsCrowdedIsSensitive() Assert.Equal(Subject, Named(candidate)); } + /// + /// A suite that never varied its concurrency is counted, and counted as unreadable. + /// + /// + /// #185: the decline used to be silent, so every test in a single-threaded suite was reported as + /// healthy on a question that had never been asked of it. Unreadable rather than awaiting runs, + /// because more runs at the one level the suite uses produce more readings at that level; what + /// this needs is a run at a different one. + /// + [Fact] + public void ASuiteThatNeverVariedItsConcurrencyIsCountedAsUnreadable() + { + List window = Split( + highFailures: 3, lowFailures: 3, lowConcurrency: 4, highConcurrency: 4); + + Assert.Empty(Analyze(window)); + + NotMeasuredCount count = NotMeasured(window); + + Assert.True(count.Unreadable > 0); + + // Structurally zero: this kind has no session floor, so it never declines for want of runs. + Assert.Equal(0, count.AwaitingRuns); + } + + /// + /// A test the trend was computed on and found nothing in is not counted. + /// + [Fact] + public void ATestWhoseTrendWasMeasuredAndSaidNothingIsNotCounted() + { + // Two levels, so the trend test runs; the same failure rate at both, so it says nothing. + List window = Split(highFailures: 2, lowFailures: 2); + + Assert.Empty(Analyze(window)); + Assert.Equal(0, NotMeasured(window).Unreadable); + } + [Fact] public void APinnedSuiteWithOccasionalSerialRunsIsAnalysable() { @@ -733,6 +771,10 @@ .. context.Tests.ExecutionsOf($"fp-{Subject}") private static IReadOnlyList Analyze(List sessions) => new ParallelSensitiveProvider().Analyze(TestSessionFactory.Context([.. sessions])).Candidates; + private static NotMeasuredCount NotMeasured(List sessions) => + new ParallelSensitiveProvider().Analyze(TestSessionFactory.Context([.. sessions])) + .NotMeasured.GetValueOrDefault(FindingKind.ParallelSensitive); + private static FindingCandidate Single(List sessions) => Assert.Single(Analyze(sessions)); diff --git a/tests/Xping.Cli.Tests/Report/ReportEnvelopeTests.cs b/tests/Xping.Cli.Tests/Report/ReportEnvelopeTests.cs index b4f9cca..f47fe2a 100644 --- a/tests/Xping.Cli.Tests/Report/ReportEnvelopeTests.cs +++ b/tests/Xping.Cli.Tests/Report/ReportEnvelopeTests.cs @@ -126,7 +126,7 @@ public void TheEnvelopeCarriesEveryDocumentedSection() JsonElement root = RunJson(); - Assert.Equal("1.14", root.GetProperty("schemaVersion").GetString()); + Assert.Equal("1.15", root.GetProperty("schemaVersion").GetString()); JsonElement window = root.GetProperty("window"); foreach (string key in @@ -139,7 +139,8 @@ public void TheEnvelopeCarriesEveryDocumentedSection() foreach (string key in (string[]) [ "tests", "findings", "healthy", "excludedLowEvidence", "excludedNotSignificant", - "environmentalSessions", "incompleteSessions", "unreadableSessions", "failedProviders" + "notMeasured", "environmentalSessions", "incompleteSessions", "unreadableSessions", + "failedProviders" ]) { Assert.True(summary.TryGetProperty(key, out _), $"summary.{key} missing"); @@ -317,7 +318,7 @@ public void WarningsGoToStandardErrorSoJsonStaysParsable() // Would throw if a warning had been interleaved into stdout. using JsonDocument document = JsonDocument.Parse(output); - Assert.Equal("1.14", document.RootElement.GetProperty("schemaVersion").GetString()); + Assert.Equal("1.15", document.RootElement.GetProperty("schemaVersion").GetString()); } [Fact] diff --git a/tests/Xping.Cli.Tests/Report/RetryProviderTests.cs b/tests/Xping.Cli.Tests/Report/RetryProviderTests.cs index f01ce1e..7d3080a 100644 --- a/tests/Xping.Cli.Tests/Report/RetryProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/RetryProviderTests.cs @@ -91,6 +91,9 @@ private static AnalysisContext Context( private static IReadOnlyList Analyze(AnalysisContext context) => new RetryProvider().Analyze(context).Candidates; + private static NotMeasuredCount NotMeasured(AnalysisContext context, FindingKind kind) => + new RetryProvider().Analyze(context).NotMeasured.GetValueOrDefault(kind); + private static RetryMaskedEvidence EvidenceFrom(AnalysisContext context) => Assert.IsType(Assert.Single(Analyze(context)).Evidence); @@ -223,6 +226,72 @@ public void EvidenceCarriesTheDenominatorsBehindTheRate() Assert.Equal(0.5, evidence.MaskedRate); } + /// + /// A test whose every run was an outage is counted, for all three kinds at once. + /// + /// + /// #185: the three kinds read one reduction of a test's runs, and none of them can read a test + /// whose every occasion was discounted. The tally is taken at that one shared precondition + /// rather than per kind, because the chain that picks between them stops at the first kind that + /// fires and never asks whether a later one could have been measured. + /// + /// Awaiting runs rather than unreadable: an attempt number is recorded by every adapter that + /// records an execution, so what is missing here is an ordinary run and not a shape of data. + /// + /// + [Fact] + public void ATestWhoseEveryRunWasAnOutageIsCountedForAllThreeRetryKinds() + { + AnalysisContext context = EveryRunAnOutage(); + + foreach (FindingKind kind in + (FindingKind[]) + [FindingKind.RetryMasked, FindingKind.RetryDeepening, FindingKind.RetryExhausted]) + { + NotMeasuredCount count = NotMeasured(context, kind); + + Assert.True(count.AwaitingRuns > 0, $"{kind} counted nothing"); + Assert.Equal(0, count.Unreadable); + } + } + + /// + /// A test whose runs were readable and uneventful is counted nowhere. + /// + [Fact] + public void ATestWhoseRetriesWereReadAndSaidNothingIsNotCounted() + { + AnalysisContext context = Context(sessions: 8, maskedSessions: 0); + + Assert.Empty(Analyze(context)); + Assert.True(NotMeasured(context, FindingKind.RetryMasked).IsEmpty); + } + + /// + /// Builds a window in which every run looked like an outage. + /// + /// + /// Twelve of thirty-one tests down in every one of six runs, which is what + /// SessionView.For reads as environmental. The subject passes throughout, so its every + /// occasion is discounted and nothing of its own behaviour is left to read. + /// + private static AnalysisContext EveryRunAnOutage() + { + IEnumerable Filler() => + [ + .. Enumerable.Range(0, 12).Select(i => + TestSessionFactory.Execution($"Fine{i}", TestOutcome.Failed)), + .. Enumerable.Range(12, 18).Select(i => TestSessionFactory.Execution($"Fine{i}")) + ]; + + return TestSessionFactory.Context( + [ + .. Enumerable.Range(0, 6).Select(ordinal => TestSessionFactory.Session( + ordinal, + [TestSessionFactory.Execution(Subject), .. Filler()])) + ]); + } + [Fact] public void AnEnvironmentalRunIsLeftOutOfTheMaskedRate() { diff --git a/tests/Xping.Cli.Tests/Report/ShareableOutputTests.cs b/tests/Xping.Cli.Tests/Report/ShareableOutputTests.cs index 6ccfbd8..6dc9361 100644 --- a/tests/Xping.Cli.Tests/Report/ShareableOutputTests.cs +++ b/tests/Xping.Cli.Tests/Report/ShareableOutputTests.cs @@ -879,6 +879,187 @@ public void TheSummaryCountsWhatWasDroppedForEvidenceAndWhatWasDroppedForSignifi Assert.Contains("6 not significant", counts, StringComparison.Ordinal); } + /// + /// The report names the questions this window's data could not answer. + /// + /// + /// #185: a metric that could compute nothing about a test used to leave no trace, so the test + /// fell through into "healthy" and a reader was told it had been looked at. The line is what + /// stops the count beside it being read as "412 tests were checked". + /// + [Fact] + public void TheReportNamesTheKindsItCouldNotMeasureAndHowManyTests() + { + string report = Render(Envelope( + [], + shown: 0, + total: 0, + lowEvidence: 0, + notSignificant: 0, + notMeasured: new Dictionary(StringComparer.Ordinal) + { + ["ParallelSensitive"] = new NotMeasuredDto(0, 108), + ["DurationRegression"] = new NotMeasuredDto(63, 32) + })); + + string line = Lines(report).Single( + l => l.Contains("nothing to measure:", StringComparison.Ordinal)); + + Assert.Contains("concurrency 108", line, StringComparison.Ordinal); + Assert.Contains("slower 32", line, StringComparison.Ordinal); + + // Largest first, whatever order the envelope happened to list them in. + Assert.True( + line.IndexOf("concurrency", StringComparison.Ordinal) < + line.IndexOf("slower", StringComparison.Ordinal)); + + // The waiting half stays in the JSON. The counts line above already says "awaiting more + // runs" in a different unit, and two waiting figures on one screen is the confusion this + // change exists to remove. + Assert.DoesNotContain("63", line, StringComparison.Ordinal); + } + + /// + /// A window every metric could read prints no such line at all. + /// + [Fact] + public void TheReportSaysNothingAboutMeasurementWhereEverythingWasMeasurable() + { + string report = Render(Envelope( + [], + shown: 0, + total: 0, + lowEvidence: 0, + notSignificant: 0, + notMeasured: new Dictionary(StringComparer.Ordinal) + { + ["Flaky"] = new NotMeasuredDto(0, 0), + ["Vanished"] = new NotMeasuredDto(4, 0) + })); + + Assert.DoesNotContain("nothing to measure:", report, StringComparison.Ordinal); + } + + /// + /// Past three kinds the line stops naming them and says how many more there are. + /// + /// + /// Thirteen kinds do not fit a line above a fence that exists to survive a phone, and a reader + /// scanning four numbers for the largest is being handed a table one segment at a time. The + /// full breakdown is in the JSON envelope. + /// + [Fact] + public void TheUnmeasuredLineNamesThreeKindsAndCountsTheRest() + { + string report = Render(Envelope( + [], + shown: 0, + total: 0, + lowEvidence: 0, + notSignificant: 0, + notMeasured: new Dictionary(StringComparer.Ordinal) + { + ["ParallelSensitive"] = new NotMeasuredDto(0, 50), + ["TimeSensitive"] = new NotMeasuredDto(0, 40), + ["DurationRegression"] = new NotMeasuredDto(0, 30), + ["DurationUnstable"] = new NotMeasuredDto(0, 20), + ["Vanished"] = new NotMeasuredDto(0, 10) + })); + + string line = Lines(report).Single( + l => l.Contains("nothing to measure:", StringComparison.Ordinal)); + + Assert.Contains("+2 more", line, StringComparison.Ordinal); + Assert.DoesNotContain("stopped running", line, StringComparison.Ordinal); + } + + /// + /// Two renders of one envelope order the kinds identically. + /// + /// + /// Equal counts are broken by the kind's declaration order rather than left to the dictionary, + /// because a report has to be byte-identical over unchanged input and a hash order is not. + /// + [Fact] + public void KindsWithEqualCountsAreOrderedByTheKindAndNotByTheDictionary() + { + Dictionary Tally() => + new(StringComparer.Ordinal) + { + ["Vanished"] = new NotMeasuredDto(0, 7), + ["DurationRegression"] = new NotMeasuredDto(0, 7) + }; + + string first = Render(Envelope([], 0, 0, 0, 0, Tally())); + string second = Render(Envelope([], 0, 0, 0, 0, Tally())); + + Assert.Equal(first, second); + + string line = Lines(first).Single( + l => l.Contains("nothing to measure:", StringComparison.Ordinal)); + + // `DurationRegression` is declared before `Vanished`. + Assert.True( + line.IndexOf("slower", StringComparison.Ordinal) < + line.IndexOf("stopped running", StringComparison.Ordinal)); + } + + /// + /// The unmeasured kinds are not appended to the line of counts. + /// + /// + /// Those are suite-wide totals of candidates the report saw and withheld; this is per kind and + /// counts tests no candidate ever existed for. Thirteen kinds cannot join a line of totals, and + /// a reader adding them to it would be adding two different units. + /// + [Fact] + public void TheUnmeasuredKindsAreNotAppendedToTheCountsLine() + { + string report = Render(Envelope( + [Finding("Flaky", "high", "CartTests.Checkout", "failed 7 of 20")], + shown: 1, + total: 1, + lowEvidence: 41, + notSignificant: 6, + notMeasured: new Dictionary(StringComparer.Ordinal) + { + ["ParallelSensitive"] = new NotMeasuredDto(0, 108) + })); + + string counts = Lines(report).Single(l => l.Contains("healthy", StringComparison.Ordinal)); + + Assert.DoesNotContain("nothing to measure:", counts, StringComparison.Ordinal); + Assert.DoesNotContain("108", counts, StringComparison.Ordinal); + } + + /// + /// An empty report whose questions all went unanswered does not report success. + /// + /// + /// The reading #185 was filed about, at its sharpest: a suite nothing could be measured on used + /// to print a green "No findings." Counted in kinds rather than tests, because the sentence + /// answers "why is this block empty" and the per-kind test counts are on the line above. + /// + [Theory] + [InlineData(1, "Nothing reportable yet: 1 kind had nothing to measure.")] + [InlineData(2, "Nothing reportable yet: 2 kinds had nothing to measure.")] + public void AnEmptyReportSaysWhenItsQuestionsWentUnanswered(int kinds, string expected) + { + var tally = new Dictionary(StringComparer.Ordinal) + { + ["ParallelSensitive"] = new NotMeasuredDto(0, 108) + }; + + if (kinds > 1) + tally["TimeSensitive"] = new NotMeasuredDto(0, 27); + + string report = Render(Envelope([], 0, 0, 0, 0, tally)); + + Assert.Contains( + Fenced(report), + line => line.EndsWith(expected, StringComparison.Ordinal)); + } + /// /// An empty report says which kind of empty it is. /// @@ -1003,7 +1184,16 @@ private static ReportEnvelope Envelope(FindingDto[] findings, int shown, int tot Envelope(findings, shown, total, lowEvidence: 0, notSignificant: 0); private static ReportEnvelope Envelope( - FindingDto[] findings, int shown, int total, int lowEvidence, int notSignificant) + FindingDto[] findings, int shown, int total, int lowEvidence, int notSignificant) => + Envelope(findings, shown, total, lowEvidence, notSignificant, notMeasured: null); + + private static ReportEnvelope Envelope( + FindingDto[] findings, + int shown, + int total, + int lowEvidence, + int notSignificant, + IReadOnlyDictionary? notMeasured) { int high = findings.Count(f => f.Severity == "high"); int medium = findings.Count(f => f.Severity == "medium"); @@ -1028,6 +1218,7 @@ private static ReportEnvelope Envelope( 412 - findings.Length, lowEvidence, notSignificant, + notMeasured ?? new Dictionary(StringComparer.Ordinal), 0, 0, 0, diff --git a/tests/Xping.Cli.Tests/Report/TimeSensitiveProviderTests.cs b/tests/Xping.Cli.Tests/Report/TimeSensitiveProviderTests.cs index de06454..46f1042 100644 --- a/tests/Xping.Cli.Tests/Report/TimeSensitiveProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/TimeSensitiveProviderTests.cs @@ -352,6 +352,42 @@ public void ExecutionsWithNoRecordedOffsetAreExcludedRatherThanAssumedToBeOnUtc( Assert.Empty(Analyze(TimeOfDay(eveningFailures: 6, morningFailures: 0, recordOffset: false))); } + /// + /// A window no session recorded a clock in counts every test, not none of them. + /// + /// + /// #185: the exclusion above used to be silent, so a suite whose adapter recorded no offset was + /// reported as healthy on a question nothing in it could answer. Taken at the top of the window + /// rather than per test, so the number has to be the one the per-test gate would have reached + /// one test at a time — otherwise a suite's tally would collapse the moment one session happened + /// to record an offset. + /// + [Fact] + public void AWindowWithNoRecordedClockCountsEveryTestAsUnreadable() + { + List window = + TimeOfDay(eveningFailures: 6, morningFailures: 0, recordOffset: false); + + NotMeasuredCount count = NotMeasured(window); + + Assert.Equal(Context(window).Tests.Fingerprints.Count, count.Unreadable); + Assert.Equal(0, count.AwaitingRuns); + } + + /// + /// A test on a clock that the search found no split in is measured, and counted nowhere. + /// + [Fact] + public void ATestTheSearchJudgedAndFoundNothingInIsNotCounted() + { + // Ten evening runs and ten morning ones, failing at the same rate on both sides. + List window = TimeOfDay( + eveningFailures: 3, morningFailures: 3, evenings: 10, mornings: 10); + + Assert.Empty(Analyze(window)); + Assert.True(NotMeasured(window).IsEmpty); + } + [Fact] public void ASideWithTooFewExecutionsIsNotCompared() { @@ -1171,6 +1207,10 @@ private static List WiderButThinner() private static IReadOnlyList Analyze(List sessions) => new TimeSensitiveProvider().Analyze(Context(sessions)).Candidates; + private static NotMeasuredCount NotMeasured(List sessions) => + new TimeSensitiveProvider().Analyze(Context(sessions)).NotMeasured + .GetValueOrDefault(FindingKind.TimeSensitive); + private static FindingCandidate Single(List sessions) => Assert.Single(Analyze(sessions)); diff --git a/tests/Xping.Cli.Tests/Report/VanishedProviderTests.cs b/tests/Xping.Cli.Tests/Report/VanishedProviderTests.cs index 9a7ef68..a48aa24 100644 --- a/tests/Xping.Cli.Tests/Report/VanishedProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/VanishedProviderTests.cs @@ -39,6 +39,50 @@ private static int Family(AnalysisContext context) => new VanishedProvider().Analyze(context).HypothesesTested .GetValueOrDefault(FindingKind.Vanished); + private static NotMeasuredCount NotMeasured(AnalysisContext context) => + new VanishedProvider().Analyze(context).NotMeasured + .GetValueOrDefault(FindingKind.Vanished); + + /// + /// A test the baseline barely saw is waiting for history, and never unreadable. + /// + /// + /// A session appearance is the one observation every adapter records by existing: a test either + /// ran in a run or it did not. So this kind has no unreadable half at all, and the published + /// zero says so rather than leaving a reader to infer it from an absence. + /// + [Fact] + public void ATestTheBaselineBarelySawIsCountedAsAwaitingRunsAndNeverAsUnreadable() + { + // `Removed` appears in one baseline run, under the floor the absence is measured against; + // `Stable` appears throughout and is measured. + AnalysisContext context = Context(total: 8, presentIn: 1); + + NotMeasuredCount count = NotMeasured(context); + + Assert.Equal(1, count.AwaitingRuns); + Assert.Equal(0, count.Unreadable); + } + + /// + /// A window with no baseline counts every test, not none of them. + /// + /// + /// The whole-window decline has to come out as the same number the per-test gate would have + /// produced one test at a time. A tally that collapsed the moment a second full run arrived + /// would be describing the store's shape rather than the suite's. + /// + [Fact] + public void AWindowWithNoBaselineCountsEveryTestRatherThanNone() + { + AnalysisContext context = Context(total: 1, presentIn: 1); + + NotMeasuredCount count = NotMeasured(context); + + Assert.Equal(context.Tests.Fingerprints.Count, count.AwaitingRuns); + Assert.Equal(0, count.Unreadable); + } + /// /// A test that is still running is an asking that answered no, not an asking that never happened. ///