From 0089cffa5436ac6689cce8f9d80c9d17ceaf040e Mon Sep 17 00:00:00 2001 From: xping-admin Date: Mon, 7 Sep 2026 17:29:04 +0200 Subject: [PATCH] fix(cli): band evidence on the runs a finding rests on, not the runs its test appeared in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EvidenceLevelResolver.CountSessions read TestIndex.SessionsRunIn — every session the subject ran in — and FindingCoordinator banded the printed evidence level on that. No provider computes its claim from all of them. ClocksIn drops every session that recorded no UtcOffset and every environmental one; ParallelSensitive drops executions with no orchestration record; a duration reading can only be normalised when its own run recorded a positive median; the failure-mode and retry kinds discount environmental runs; Vanished sets aside the runs that covered only part of the suite. So a test present in all twenty window sessions but readable in ten published a split computed from ten runs as `evidence high`. The mismatch predates the 8/15 bands; those bands brought it into the top one. FindingCandidate now carries EvidenceSessions, required rather than defaulted so the next kind added cannot inherit this in silence, and the coordinator bands on it. Where a kind already publishes the figure — worse.sessions + other.sessions, trend.sessions, comparedSessions, baselineSessions — the same value is handed over, so the level and the counts a reader can check it against cannot drift. It is read off the candidate that is actually reported, so an Instead handover bands on its own denominator rather than on that of the claim it replaced. Finding and the JSON envelope publish it as evidenceSessions; the schema goes to 1.14. The reporting floor deliberately did not move. EvidenceLevelResolver's class remark defends against a test being confidently flagged by one metric and silently dropped by another, and that hazard is about emission: MinimumSessionsPerTestToReport still reads the subject's whole history, for every kind alike, and nothing that was reported before stops being reported. What moves is the label, which is a description of one claim rather than a gate on it — so two findings about one test may now carry different levels, exactly as they already carry different population markers. The trade is recorded in the class remark and in docs/internals/finding-populations.md, which gains the per-kind table. Clustered failures do not shorten the count: they remove a failure, not a run, which is what the population doc already says of sessionsConsidered. Findings over the sample store are byte-identical to main but for the new field. Closes #182 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TMQcZLBGFTd8wz3mcfn9vc --- docs/cli/command-reference.md | 7 +- docs/getting-started/local-first.md | 2 +- docs/internals/finding-populations.md | 36 ++++++++ .../Report/Contract/EnvelopeBuilder.cs | 1 + .../Report/Contract/ReportEnvelope.cs | 15 ++-- src/Xping.Cli/Report/FindingCoordinator.cs | 28 +++++-- .../Report/LocalAnalysisConstants.cs | 17 +++- src/Xping.Cli/Report/Model/Finding.cs | 10 ++- src/Xping.Cli/Report/Model/Severity.cs | 10 ++- .../Report/Providers/DurationProvider.cs | 36 +++++++- .../Report/Providers/FailureModeProvider.cs | 39 +++++++-- .../Report/Providers/IFindingProvider.cs | 32 +++++-- .../Providers/ParallelSensitiveProvider.cs | 7 ++ .../Report/Providers/RetryProvider.cs | 21 ++++- .../Report/Providers/TimeSensitiveProvider.cs | 6 ++ .../Report/Providers/VanishedProvider.cs | 7 ++ .../Report/Scoring/EvidenceLevelResolver.cs | 42 ++++++++-- .../Commands/CliSurfaceTests.cs | 4 +- .../Report/DurationProviderTests.cs | 42 ++++++++++ tests/Xping.Cli.Tests/Report/ExitCodeTests.cs | 1 + .../Report/FailureModeProviderTests.cs | 12 +++ .../Report/FindingCoordinatorTests.cs | 83 ++++++++++++++++++- .../Report/FindingOrderTests.cs | 1 + .../Report/ParallelSensitiveProviderTests.cs | 32 +++++++ .../Report/ReportEnvelopeTests.cs | 12 ++- .../Report/RetryProviderTests.cs | 46 +++++++--- .../Report/ShareableOutputTests.cs | 1 + .../Report/TimeSensitiveProviderTests.cs | 26 ++++++ .../Report/VanishedProviderTests.cs | 26 ++++++ 29 files changed, 542 insertions(+), 60 deletions(-) diff --git a/docs/cli/command-reference.md b/docs/cli/command-reference.md index b25ac5a4..fbe023de 100644 --- a/docs/cli/command-reference.md +++ b/docs/cli/command-reference.md @@ -110,6 +110,8 @@ Nothing inside the fence exceeds 72 columns, so it survives a phone and a quoted ### The population marker +**The evidence level counts the runs behind the finding, not the runs behind the test.** `evidence moderate` means the claim was computed from somewhere between 8 and 15 independent runs — a `time-sensitive` split reads only the runs whose machine recorded a clock, and a `slower` finding only the runs whose durations could be normalised. So one test can carry two findings at two different levels, and that is the two claims resting on different amounts of data rather than an inconsistency. The exact number is `evidenceSessions` in the JSON. Whether a finding is shown at all is a separate bar, and that one does read the test's whole history in the window. + The marker between the evidence level and the finding id — `all runs`, `-env` or `-env-cluster` — says **which runs went into the denominator** of that finding's rate. Every finding carries one, including the ones that set nothing aside, so that "we counted everything" and "this build did not say" never read alike. | Marker | The rate is over | @@ -335,11 +337,11 @@ 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: +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: ```json { - "schemaVersion": "1.13", + "schemaVersion": "1.14", "window": { "sessionCount": 20, "resolution": "default", "currentSliceSize": 3 }, "context": { "sha": "a3f9c2e", "branch": "main", "assembly": "Checkout.Tests" }, "summary": { @@ -357,6 +359,7 @@ Every finding carries a `headline` — the same sentence the rendered report pri "kind": "Flaky", "severity": "high", "evidenceLevel": "moderate", + "evidenceSessions": 12, "population": "excludesEnvironmentalAndClustered", "subject": { "type": "test", "fullyQualifiedName": "…", "assembly": "Checkout.Tests", "sourceFile": "tests/Billing/SummaryTests.cs", "sourceLineNumber": 88 }, diff --git a/docs/getting-started/local-first.md b/docs/getting-started/local-first.md index 0de1134b..b8939419 100644 --- a/docs/getting-started/local-first.md +++ b/docs/getting-started/local-first.md @@ -108,7 +108,7 @@ the claim. | `HIGH` / `MED` / `LOW` | Impact ranking. Findings are ordered most severe first, so the top of the block is the part worth reading. | | `flaky`, `slower`, `stopped running` | The *kind* — what the finding claims. There are sixteen; the [CLI reference](../cli/command-reference.md#finding-kinds) lists them all. | | The counts line | The measurement the claim rests on, in plain numbers. | -| `evidence low\|moderate\|high` | How much history stands behind it. A `low`-evidence finding is a lead, not a verdict. | +| `evidence low\|moderate\|high` | How many runs stand behind *this claim* — which is not always every run the test appeared in, since a finding can only count the runs it could measure. A `low`-evidence finding is a lead, not a verdict. | | `f_2a91c0de` | A stable id for that finding, so you can refer to it in a ticket or diff two reports. | The report body sits inside a fenced code block and stays under 72 columns, so pasting it into diff --git a/docs/internals/finding-populations.md b/docs/internals/finding-populations.md index 89c553fc..a490ae61 100644 --- a/docs/internals/finding-populations.md +++ b/docs/internals/finding-populations.md @@ -86,6 +86,42 @@ double-count. This is the arithmetic the field names exist to make possible: a t executions, ten clustered failures and two of its own publishes `2 of 12`, and `12` is not how many times it ran. +## What each kind's evidence level counts + +The `evidence low|moderate|high` label on a finding is banded on the **runs that finding was +computed from** — not on the runs its test appeared in. They are different numbers for every kind in +the table above, and for the same reasons: a discounted run is not an occasion the test's own +behaviour was observed on, and a run the kind could not read at all is not an occasion either. + +| kind | runs the level counts | +|---|---| +| `Flaky`, `AlwaysFailing`, `TimingOut` | runs of the test, less the environmental ones | +| `RetryMasked` | the same | +| `RetryDeepening` | the settled runs in both arms of the comparison | +| `RetryExhausted` | `runsConsidered` | +| `SharedFailure`, `BrokenFixture` | runs the cluster's best-evidenced member ran in — nothing is set aside | +| `DurationRegression` | `current.comparedSessions` + `baseline.comparedSessions` | +| `DurationUnstable` | the runs behind the normalised readings the dispersion was taken over | +| `ParallelSensitive` | `trend.sessions` | +| `TimeSensitive` | `worse.sessions` + `other.sessions` | +| `Vanished` | `baselineSessions` — the habit the absence is a change from | + +Wherever the kind already publishes that figure, the level is banded on the published one, so the +label and the counts a reader can check it against cannot drift apart. The number itself reaches the +JSON as `evidenceSessions` on every finding, beside `evidenceLevel`. + +**Two findings about one test may therefore carry different levels.** That is the same statement the +population marker already makes, one layer up: the kinds do not count the same runs, so they do not +have the same amount of evidence either. A `TimeSensitive` split over the ten runs that recorded a +clock is not better evidenced because the test also ran in ten that did not. + +**What is *not* banded this way is whether the finding is reported at all.** The reporting floor — +`MinimumSessionsPerTestToReport`, applied in `FindingCoordinator` — reads the runs the subject +appeared in, for every kind alike. Emission has to be one rule, or a test flagged by one metric is +silently dropped by another with nothing on screen to explain it. So a claim resting on two runs of +a test with twenty runs of history is still printed. It is printed saying `evidence low`, which is +what the level is for. + ## 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 c2b13786..d0374f45 100644 --- a/src/Xping.Cli/Report/Contract/EnvelopeBuilder.cs +++ b/src/Xping.Cli/Report/Contract/EnvelopeBuilder.cs @@ -115,6 +115,7 @@ private static FindingDto BuildFinding(Finding finding) finding.Kind.ToString(), ToCamelCase(finding.Severity.ToString()), ToCamelCase(finding.EvidenceLevel.ToString()), + finding.EvidenceSessions, ToCamelCase(PopulationRules.For(finding.Kind).ToString()), BuildSubject(finding.Subject), headline, diff --git a/src/Xping.Cli/Report/Contract/ReportEnvelope.cs b/src/Xping.Cli/Report/Contract/ReportEnvelope.cs index c0d39d10..1673e3da 100644 --- a/src/Xping.Cli/Report/Contract/ReportEnvelope.cs +++ b/src/Xping.Cli/Report/Contract/ReportEnvelope.cs @@ -39,12 +39,11 @@ internal sealed record ReportEnvelope( /// Moves whenever anything a consumer reads changes shape, and the per-kind evidence payloads /// are part of that even though this document describes them as opaque: a script that reached /// into evidence for a field this build no longer emits is reading a contract, and - /// leaving the number still would tell it nothing had moved. 1.13 is where the summary gained - /// partialSessions and VanishedEvidence gained partialSessionsSetAside — - /// the runs that covered only part of the suite, which an absence is counted on neither side - /// of. + /// leaving the number still would tell it nothing had moved. 1.14 is where every finding gained + /// 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.13"; + public const string CurrentSchemaVersion = "1.14"; } /// @@ -122,6 +121,11 @@ internal sealed record SummaryDto( /// What the finding claims. /// How much attention it deserves. /// How much data it rests on. +/// +/// Independent runs this finding was computed from — what was +/// banded from. Not the window's run count, and not the runs the test appeared in: a kind that +/// discounts environmental runs, or that cannot read a run at all, measured over fewer. +/// /// /// Which executions the counts and rates below were taken over. The report ranks findings of /// different kinds against each other and the kinds do not all count the same population, so two @@ -137,6 +141,7 @@ internal sealed record FindingDto( string Kind, string Severity, string EvidenceLevel, + int EvidenceSessions, string Population, SubjectDto Subject, string Headline, diff --git a/src/Xping.Cli/Report/FindingCoordinator.cs b/src/Xping.Cli/Report/FindingCoordinator.cs index 2774e055..234029ef 100644 --- a/src/Xping.Cli/Report/FindingCoordinator.cs +++ b/src/Xping.Cli/Report/FindingCoordinator.cs @@ -66,7 +66,7 @@ public AnalysisResult Run( // time. Whether a p-value is worth reporting depends on how many fingerprints the kind was // tested on and on what the other survivors read, neither of which is known until every // provider has run. - var surviving = new List<(FindingCandidate Candidate, int Sessions)>(); + var surviving = new List(); var tested = new Dictionary(); int lowEvidence = 0; @@ -107,15 +107,22 @@ public AnalysisResult Run( if (kinds != null && !kinds.Contains(candidate.Kind)) continue; - int sessions = EvidenceLevelResolver.CountSessions(candidate.Subject, context.Tests); + // The subject's own history, not the candidate's. The floor asks whether this test + // has been around long enough to be judged at all, which is a property of the test + // and has to be answered the same way for every kind — otherwise one metric flags a + // test that another silently drops, with nothing on screen to explain it. What the + // candidate measured over decides its evidence band further down, and only that. + int subjectSessions = + EvidenceLevelResolver.CountSessions(candidate.Subject, context.Tests); - if (!EvidenceLevelResolver.MeetsReportingFloor(sessions, context.Window.SessionCount)) + if (!EvidenceLevelResolver.MeetsReportingFloor( + subjectSessions, context.Window.SessionCount)) { lowEvidence++; continue; } - surviving.Add((candidate, sessions)); + surviving.Add(candidate); } } @@ -124,7 +131,7 @@ public AnalysisResult Run( var findings = new List(); int notSignificant = 0; - foreach ((FindingCandidate collected, int sessions) in surviving) + foreach (FindingCandidate collected in surviving) { FindingCandidate? candidate = Reported(collected, cutoffs); @@ -140,7 +147,12 @@ public AnalysisResult Run( FindingId.Compute(candidate.Kind, candidate.Subject.SortKey), candidate.Kind, candidate.Cap(ImpactScorer.Band(impact)), - EvidenceLevelResolver.Resolve(sessions), + + // Read off the candidate that is actually being reported, so an `Instead` handover + // is banded on what the replacement claim was measured over rather than on what the + // claim it replaced was. + EvidenceLevelResolver.Resolve(candidate.EvidenceSessions), + candidate.EvidenceSessions, candidate.Subject, candidate.Evidence, candidate.DrillDownCommand, @@ -175,12 +187,12 @@ public AnalysisResult Run( /// /// private static Dictionary Cutoffs( - IReadOnlyList<(FindingCandidate Candidate, int Sessions)> surviving, + IReadOnlyList surviving, IReadOnlyDictionary tested) { var byKind = new Dictionary>(); - foreach ((FindingCandidate candidate, _) in surviving) + foreach (FindingCandidate candidate in surviving) { if (candidate.PValue is not { } p) continue; diff --git a/src/Xping.Cli/Report/LocalAnalysisConstants.cs b/src/Xping.Cli/Report/LocalAnalysisConstants.cs index a2c07b8c..0b0bf758 100644 --- a/src/Xping.Cli/Report/LocalAnalysisConstants.cs +++ b/src/Xping.Cli/Report/LocalAnalysisConstants.cs @@ -42,6 +42,13 @@ internal static class LocalAnalysisConstants /// session that retried five times cleared an execution-denominated floor of five on its own — /// exactly the shape the floor exists to exclude, and worst for the tests that retry most. /// + /// + /// The subject's sessions, deliberately, and not the ones a given finding was computed from — + /// which is what bands. Whether a test has enough + /// history to be judged has to be answered once for every kind, or one metric flags a test that + /// another silently drops with nothing on screen to explain the difference. A claim resting on + /// less than its subject's history is still reported; it is reported saying low. + /// /// public const int MinimumSessionsPerTestToReport = 5; @@ -430,10 +437,16 @@ internal static class LocalAnalysisConstants public const double SeverityMediumThreshold = 0.30; /// - /// Sessions a test must have run in before its evidence is better than Low (8). + /// Runs a finding must rest on before its evidence is better than Low (8). /// /// /// + /// The runs the finding was computed from, not the runs its test appeared in — see + /// for why those are different numbers and why only + /// this one is banded. It brings the CLI closer to Cloud rather than further from it: Cloud's + /// effective sample size is computed over the rows its own analysis read. + /// + /// /// Banded on sessions, which is the unit Xping Cloud already bands on: its /// EvidenceLevelThresholds classifies an effective sample size computed over runs that /// RunCollapser has reduced to one row per test per session. The unit agrees; the numbers @@ -449,7 +462,7 @@ internal static class LocalAnalysisConstants public const int EvidenceModerateSessions = 8; /// - /// Sessions above which evidence is High (15). + /// Runs above which evidence is High (15). /// public const int EvidenceHighSessions = 15; diff --git a/src/Xping.Cli/Report/Model/Finding.cs b/src/Xping.Cli/Report/Model/Finding.cs index ca7386ce..947d3183 100644 --- a/src/Xping.Cli/Report/Model/Finding.cs +++ b/src/Xping.Cli/Report/Model/Finding.cs @@ -92,7 +92,14 @@ internal abstract record FindingEvidence; /// Stable short identity; see . /// What the finding claims. /// Banded from . -/// How much data the claim rests on. +/// How much data the claim rests on, banded. +/// +/// The number was banded from: independent runs the claim was +/// computed from, which is generally fewer than the runs the subject appeared in. Carried and +/// published alongside the band for the reason every other threshold in the report publishes its +/// input — a level a reader cannot reconcile with the counts beside it is a level they have to take +/// on trust. +/// /// The test or group the finding is about. /// The kind-specific payload. /// @@ -108,6 +115,7 @@ internal sealed record Finding( FindingKind Kind, Severity Severity, EvidenceLevel EvidenceLevel, + int EvidenceSessions, FindingSubject Subject, FindingEvidence Evidence, string DrillDownCommand, diff --git a/src/Xping.Cli/Report/Model/Severity.cs b/src/Xping.Cli/Report/Model/Severity.cs index 3ca4a601..93dbfd8b 100644 --- a/src/Xping.Cli/Report/Model/Severity.cs +++ b/src/Xping.Cli/Report/Model/Severity.cs @@ -26,7 +26,7 @@ internal enum Severity } /// -/// How much data a finding rests on, measured in sessions the subject ran in within the window. +/// How much data a finding rests on, measured in the runs it was computed from. /// /// /// @@ -35,6 +35,14 @@ internal enum Severity /// must not be labelled as though it had forty. /// /// +/// The finding's runs, not the subject's. Every kind measures over a subset of the runs its test +/// appeared in — environmental runs are discounted, and a run whose session recorded no clock or +/// whose durations cannot be normalised is one the question could not be asked of. The number is +/// carried on the finding as EvidenceSessions and published beside the level, so a reader +/// can see which of the two it came from. Whether a finding is emitted at all is a separate +/// judgement, made on the subject — see . +/// +/// /// The unit matches Xping Cloud, which bands an effective sample size computed over runs collapsed /// to one row per test per session. The thresholds do not, and deliberately — /// explains why. Local windows are diff --git a/src/Xping.Cli/Report/Providers/DurationProvider.cs b/src/Xping.Cli/Report/Providers/DurationProvider.cs index 00d1ba9e..b1f18b06 100644 --- a/src/Xping.Cli/Report/Providers/DurationProvider.cs +++ b/src/Xping.Cli/Report/Providers/DurationProvider.cs @@ -577,6 +577,13 @@ private static Examination Regression( DrillDownCommand: DrillDown.ForTest(FindingKind.DurationRegression, test), + // The runs the two-sample test actually read, both arms — the same two figures the + // evidence publishes as `comparedSessions`. Narrower than the runs the test appeared in + // twice over: a run whose own median was not positive normalises nothing, and a long + // baseline is capped at `MaxComparedSessions`. + EvidenceSessions: + baselineProfile.Compared.Count + currentProfile.Compared.Count, + PValue: pValue)); } @@ -665,7 +672,12 @@ private static double Probability(double value) => LastOccurrenceIn: TestIndex.NewestSession(all), - DrillDownCommand: DrillDown.ForTest(FindingKind.DurationUnstable, test)); + DrillDownCommand: DrillDown.ForTest(FindingKind.DurationUnstable, test), + + // 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); } /// @@ -871,6 +883,11 @@ private static Profile Build( var normalised = new List(executions.Count); var sessions = new HashSet(); + // The runs behind `normalised`, which are fewer than `sessions` whenever a run recorded no + // positive median of its own — the xUnit adapter produces one for a failure raised outside + // the timed invocation, and a run made mostly of those normalises nothing. + var normalisedSessions = new HashSet(); + // One entry per run the test appeared in, holding every attempt it made there. Kept in the // order the runs are reached so the truncation below can take the most recent ones. var perSession = new List<(int Index, Guid Session, List Attempts)>(); @@ -882,7 +899,10 @@ private static Profile Build( sessions.Add(reference.Session.SessionId); if (medians.TryGetValue(reference.Session.SessionId, out double median)) + { normalised.Add(Milliseconds(reference) / median); + normalisedSessions.Add(reference.Session.SessionId); + } if (!comparable) continue; @@ -903,7 +923,12 @@ private static Profile Build( normalised.Sort(); return new Profile( - executions, raw, normalised, sessions.Count, Compared(perSession, medians)); + executions, + raw, + normalised, + sessions.Count, + normalisedSessions.Count, + Compared(perSession, medians)); } /// @@ -990,6 +1015,12 @@ private static double PercentIncrease(double before, double after) => /// of that however correlated the two are. /// /// Distinct runs the executions came from. + /// + /// Distinct runs behind — the occasions the dispersion rests on, + /// which is what the instability finding's evidence level is banded from. A strict subset of + /// : a run with no positive median of its own contributes raw + /// readings and no normalised ones. + /// /// /// One normalised reading per run, ascending. What the two-sample comparison reads, and a /// strict subset of the runs behind — see Compared. @@ -999,6 +1030,7 @@ private sealed record Profile( List Raw, List Normalised, int Sessions, + int NormalisedSessions, List Compared) { public int Executions => Raw.Count; diff --git a/src/Xping.Cli/Report/Providers/FailureModeProvider.cs b/src/Xping.Cli/Report/Providers/FailureModeProvider.cs index 83d4be5e..5df38fb4 100644 --- a/src/Xping.Cli/Report/Providers/FailureModeProvider.cs +++ b/src/Xping.Cli/Report/Providers/FailureModeProvider.cs @@ -473,9 +473,11 @@ private static FindingCandidate SharedFailure(AnalysisContext context, Signature ? BuildSharedEvidence(context, cluster, members) : BuildBrokenFixtureEvidence(context, cluster, members, site.Value); + var subject = new FindingSubject.Group(groupId, references); + return new FindingCandidate( kind, - new FindingSubject.Group(groupId, references), + subject, evidence, unreliability, @@ -483,7 +485,14 @@ private static FindingCandidate SharedFailure(AnalysisContext context, Signature // so the head is the last time this cluster was seen. LastOccurrenceIn: cluster.Failures[0].Session, - DrillDown.ForGroup(kind, assembly)); + DrillDown.ForGroup(kind, assembly), + + // The two kinds that set nothing aside — an environmental run is a shared cause seen + // from underneath, so discounting one here would silence the finding that explains it. + // With nothing removed, the runs the claim was computed over are the runs its subject + // ran in, and the cluster is measured by its best-evidenced member for the reason it is + // ranked by one: it is worth opening if any single member stands behind it. + EvidenceSessions: EvidenceLevelResolver.CountSessions(subject, context.Tests)); } /// @@ -592,6 +601,15 @@ private static SharedFailureEvidence BuildSharedEvidence( int environmental = 0; int clusteredOut = 0; + // The occasions behind whichever of the three kinds below is emitted. Counted from the + // discount rather than from `considered`, because the two discounts do not remove the same + // thing: an environmental run is a run this test's own behaviour was never observed on, + // while a clustered failure removes a failure and leaves the run — the test still ran there + // and still did not fail on its own account. That is the same distinction + // `sessionsConsidered` is built on a few lines down, and reading it off `considered` would + // quietly disagree with it whenever a session held nothing but clustered failures. + var occasions = new HashSet(); + foreach (ExecutionRef reference in all) { switch (DiscountFor(context, reference, clustered)) @@ -601,9 +619,11 @@ private static SharedFailureEvidence BuildSharedEvidence( break; case Discount.Clustered: clusteredOut++; + occasions.Add(reference.Session.SessionId); break; default: considered.Add(reference); + occasions.Add(reference.Session.SessionId); break; } } @@ -666,7 +686,8 @@ private static SharedFailureEvidence BuildSharedEvidence( timeouts, sessionsConsidered, environmental, - clusteredOut); + clusteredOut, + occasions.Count); } // Modal rather than sole. Failure modes are compared by exact hash over the exception type, @@ -711,7 +732,8 @@ private static SharedFailureEvidence BuildSharedEvidence( WilsonInterval.LowerBound(failures.Count, considered.Count), LastOccurrenceIn: lastFailureIn, - DrillDown.ForTest(FindingKind.AlwaysFailing, test)); + DrillDown.ForTest(FindingKind.AlwaysFailing, test), + EvidenceSessions: occasions.Count); } // Everything else that failed at all. Either the failure mode varies between runs, or one @@ -753,7 +775,8 @@ private static SharedFailureEvidence BuildSharedEvidence( FlakyUnreliability(failureRate, failures.Count, considered.Count), LastOccurrenceIn: lastFailureIn, - DrillDown.ForTest(FindingKind.Flaky, test)); + DrillDown.ForTest(FindingKind.Flaky, test), + EvidenceSessions: occasions.Count); } /// @@ -791,7 +814,8 @@ private static FindingCandidate TimingOut( List timeouts, int sessionsConsidered, int environmental, - int clusteredOut) + int clusteredOut, + int occasions) { double timeoutRate = (double)timeouts.Count / considered.Count; @@ -838,7 +862,8 @@ [.. ordered.Take(MaxExemplars).Select(t => ToExemplar(context, t))], WilsonInterval.LowerBound(timeouts.Count, considered.Count), LastOccurrenceIn: TestIndex.NewestSession(timeouts), - DrillDown.ForTest(FindingKind.TimingOut, test)); + DrillDown.ForTest(FindingKind.TimingOut, test), + EvidenceSessions: occasions); } /// diff --git a/src/Xping.Cli/Report/Providers/IFindingProvider.cs b/src/Xping.Cli/Report/Providers/IFindingProvider.cs index cd83f7b7..36721c7f 100644 --- a/src/Xping.Cli/Report/Providers/IFindingProvider.cs +++ b/src/Xping.Cli/Report/Providers/IFindingProvider.cs @@ -13,10 +13,10 @@ namespace Xping.Cli.Report.Providers; /// What a provider hands back for one subject, before severity and evidence are resolved. /// /// -/// A provider states what it observed and how unreliable that makes the subject. It does not band -/// severity, compute an evidence level or apply the reporting floor: those are defined once, for -/// every kind, by the coordinator. A provider that scored its own findings would drift from the -/// others the first time a threshold moved. +/// A provider states what it observed, how unreliable that makes the subject, and how many +/// independent runs it observed it over. It does not band severity, band evidence or apply the +/// reporting floor: those are defined once, for every kind, by the coordinator. A provider that +/// scored its own findings would drift from the others the first time a threshold moved. /// /// What this candidate claims. /// The test or group it claims it about. @@ -35,6 +35,24 @@ namespace Xping.Cli.Report.Providers; /// the two separately could hand over two that disagree. /// /// The exact CLI invocation that expands this finding. +/// +/// Independent runs this claim was computed from — the denominator the coordinator bands +/// on. +/// +/// Not the window's run count, and not the runs the subject merely appeared in. Every provider +/// measures over a subset of those: environmental runs are discounted, a run whose session recorded +/// no UTC offset cannot be placed on a clock, a run made of zero-duration executions cannot be +/// normalised, an execution whose adapter recorded no concurrency cannot be placed at a level. A +/// split computed from ten runs labelled with the twenty the test appeared in is a claim of evidence +/// the finding has not got, and it is the top band that the overclaim lands in. +/// +/// +/// Required rather than defaulted, deliberately. A default would be some other number — the +/// subject's, most likely — and the next kind added would inherit this bug in silence. Where the +/// count is already published on , hand over that same value rather than +/// recomputing it, so the band and the counts a reader can see cannot disagree. +/// +/// /// /// How probable an observation this extreme would be if the kind's claim were false, or /// where no hypothesis was tested. Null is not "not computed yet": most kinds @@ -62,7 +80,10 @@ namespace Xping.Cli.Report.Providers; /// /// The subject must be the same, so that the reporting floor already applied to this candidate /// applies unchanged to its replacement. The replacement should carry no : -/// it was not in any family, so there is no multiplicity for it to be charged with. +/// it was not in any family, so there is no multiplicity for it to be charged with. It carries its +/// own , because a different claim about one subject is +/// generally measured over a different population — DurationUnstable reads what could be +/// normalised where DurationRegression read what could be compared. /// /// internal sealed record FindingCandidate( @@ -72,6 +93,7 @@ internal sealed record FindingCandidate( double Unreliability, TestSession LastOccurrenceIn, string DrillDownCommand, + int EvidenceSessions, double? PValue = null, Severity? SeverityCeiling = null, FindingCandidate? Instead = null) diff --git a/src/Xping.Cli/Report/Providers/ParallelSensitiveProvider.cs b/src/Xping.Cli/Report/Providers/ParallelSensitiveProvider.cs index a2dca779..ff90b5ba 100644 --- a/src/Xping.Cli/Report/Providers/ParallelSensitiveProvider.cs +++ b/src/Xping.Cli/Report/Providers/ParallelSensitiveProvider.cs @@ -376,6 +376,13 @@ private static Examination Examine(AnalysisContext context, string fingerprint) DrillDownCommand: DrillDown.ForTest(FindingKind.ParallelSensitive, test), + // Sessions rather than the executions the trend was measured on, and the same figure + // the trend publishes. Concurrency genuinely varies between attempts within a run, so + // the executions are real readings — but they are not separate occasions, and evidence + // is a claim about occasions. An adapter that filled no orchestration record for most + // of the window leaves a trend over whatever few runs it did fill, and this says so. + EvidenceSessions: DistinctSessions(considered), + PValue: statistic.PValue)); } diff --git a/src/Xping.Cli/Report/Providers/RetryProvider.cs b/src/Xping.Cli/Report/Providers/RetryProvider.cs index 14fd499e..6adc7ded 100644 --- a/src/Xping.Cli/Report/Providers/RetryProvider.cs +++ b/src/Xping.Cli/Report/Providers/RetryProvider.cs @@ -628,7 +628,14 @@ [.. exhausted.Take(MaxExemplars).Select(ToExemplar)], // test that ran out of retries a fortnight ago and has been clean since decays. LastOccurrenceIn: TestIndex.NewestSession(exhausted.Select(r => r.Final)), - DrillDownCommand: DrillDown.ForTest(FindingKind.RetryExhausted, test)); + DrillDownCommand: DrillDown.ForTest(FindingKind.RetryExhausted, test), + + // Every non-environmental run of this test — the population the exhaustion rate is + // taken over, which is not the runs it appeared in whenever an outage discounted one. + // Not `retried.Count`: a run that never needed a retry is still an occasion on which + // this test's retry behaviour was observed, and it is the observation that says the + // behaviour is not universal. + EvidenceSessions: considered.Count); } /// @@ -761,6 +768,11 @@ [.. currentGreen.Take(MaxExemplars).Select(ToExemplar)], DrillDownCommand: DrillDown.ForTest(FindingKind.RetryDeepening, test), + // The runs on both sides of the comparison, which is what a difference between two + // medians rests on. Runs that failed finally are in neither arm — the two medians are + // of settled runs — so this is again narrower than the runs the test appeared in. + EvidenceSessions: currentGreen.Count + baselineGreen.Count, + // Nothing has failed a build here. Left uncapped, the generic impact formula would rank a // frequently-run test that still goes green above one that is failing today, because // "runs constantly and got slightly more expensive" scores well on every term it reads. @@ -934,7 +946,12 @@ private static RetryDepthProfile Profile( LastOccurrenceIn: newest.Session, - DrillDownCommand: DrillDown.ForTest(FindingKind.RetryMasked, test)); + DrillDownCommand: DrillDown.ForTest(FindingKind.RetryMasked, test), + + // Runs rather than the executions the rate above is taken over. Attempts within one run + // are the very correlation this kind is measuring, so counting them as occasions would + // let the behaviour inflate its own evidence. + EvidenceSessions: runs.Count - discountedSessions.Count); } private static bool IsMasked(ExecutionRef reference) => diff --git a/src/Xping.Cli/Report/Providers/TimeSensitiveProvider.cs b/src/Xping.Cli/Report/Providers/TimeSensitiveProvider.cs index 85472449..71b9183c 100644 --- a/src/Xping.Cli/Report/Providers/TimeSensitiveProvider.cs +++ b/src/Xping.Cli/Report/Providers/TimeSensitiveProvider.cs @@ -426,6 +426,12 @@ private static Examination Examine( DrillDownCommand: DrillDown.ForTest(FindingKind.TimeSensitive, test), + // The runs that could be placed on a clock, which is what the split divided and what + // the two published arm counts add up to. Every other run of this test was either + // discounted as environmental or came from a session that recorded no UTC offset, and a + // test present in twenty runs but readable in ten holds ten runs of evidence. + EvidenceSessions: population.Considered.Count, + // Unrounded, unlike the copy in the evidence. This is the number the coordinator's // Benjamini-Hochberg pass sorts on, and rounding two neighbouring p-values onto each // other would reorder the ranked list that pass walks down. diff --git a/src/Xping.Cli/Report/Providers/VanishedProvider.cs b/src/Xping.Cli/Report/Providers/VanishedProvider.cs index 869f60ce..03263974 100644 --- a/src/Xping.Cli/Report/Providers/VanishedProvider.cs +++ b/src/Xping.Cli/Report/Providers/VanishedProvider.cs @@ -180,6 +180,13 @@ public ProviderReport Analyze(AnalysisContext context) DrillDownCommand: DrillDown.ForTest(FindingKind.Vanished, reference), + // The habit, which is the whole of what this claim rests on. The current arm holds + // none of this test's appearances by construction, and the runs that covered only + // part of the suite were set aside from both slices before any of them were + // counted — so the runs the test appeared in and the runs the claim was computed + // from are not the same number. + EvidenceSessions: appearances, + // Unrounded: this is the number the coordinator's Benjamini-Hochberg pass sorts on, // and the rounded copy in the evidence is only what gets written down. PValue: pValue, diff --git a/src/Xping.Cli/Report/Scoring/EvidenceLevelResolver.cs b/src/Xping.Cli/Report/Scoring/EvidenceLevelResolver.cs index eb99fa9a..ebda2b6b 100644 --- a/src/Xping.Cli/Report/Scoring/EvidenceLevelResolver.cs +++ b/src/Xping.Cli/Report/Scoring/EvidenceLevelResolver.cs @@ -12,16 +12,40 @@ namespace Xping.Cli.Report.Scoring; /// Decides how much data a finding rests on, and whether it rests on enough to report at all. /// /// -/// Applied centrally rather than per provider. Two providers disagreeing about what counts as enough -/// evidence would produce a report where a test is confidently flagged by one metric and silently -/// dropped by another, with nothing on screen to explain the difference. +/// +/// The two questions are answered from different numbers, and the split is the point. +/// +/// +/// Whether to report is decided centrally, on the subject's own history — +/// and . Letting each provider decide +/// that would produce a report where a test is confidently flagged by one metric and silently +/// dropped by another, with nothing on screen to explain the difference. Emission stays one rule, +/// applied once, to every kind alike. +/// +/// +/// How much it rests on is banded on the candidate's own denominator, which only the provider +/// knows. Every provider measures over a subset of the runs its subject appeared in: environmental +/// runs are discounted, a session that recorded no UTC offset cannot be placed on a clock, a run of +/// zero-duration executions cannot be normalised. A split computed from ten runs and labelled with +/// the twenty the test appeared in claims evidence it has not got, and under bands that fit a +/// twenty-run window it claims the top one. +/// +/// +/// So two findings about one test may now carry different levels. That is the same thing the report +/// already says with its population marker — see docs/internals/finding-populations.md — and +/// it is a description of a claim rather than a gate on it, so it costs none of what the central +/// floor above is there to protect. +/// /// internal static class EvidenceLevelResolver { /// - /// Bands a subject's session count into an evidence level. + /// Bands a finding's own denominator into an evidence level. /// - /// Distinct sessions the subject ran in, within the window. + /// + /// Independent runs the claim was computed from — FindingCandidate.EvidenceSessions, not + /// the runs the subject appeared in. + /// /// The level. public static EvidenceLevel Resolve(int sessions) => sessions switch { @@ -31,13 +55,19 @@ internal static class EvidenceLevelResolver }; /// - /// Counts the sessions a finding rests on. + /// Counts the sessions the subject ran in — the reporting floor's denominator. /// /// The test or group the finding is about. /// The shared index. /// The distinct session count. /// /// + /// The subject's whole history in the window, whatever any one provider then measured over. + /// That is what the floor wants to know — has this test been around long enough to be judged — + /// and it is deliberately not what bands. A kind that does publish this + /// figure as its own denominator, because it sets nothing aside, may hand it over as such. + /// + /// /// A group is measured by its best-evidenced member, matching how it is scored: the cluster is /// worth reporting if any one member is well enough evidenced to stand behind. /// diff --git a/tests/Xping.Cli.Tests/Commands/CliSurfaceTests.cs b/tests/Xping.Cli.Tests/Commands/CliSurfaceTests.cs index 50e55ecc..6c295fbb 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.13", root.GetProperty("schemaVersion").GetString()); + Assert.Equal("1.14", 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.13", doc.RootElement.GetProperty("schemaVersion").GetString()); + Assert.Equal("1.14", 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 998c54ec..a09ea5e6 100644 --- a/tests/Xping.Cli.Tests/Report/DurationProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/DurationProviderTests.cs @@ -734,6 +734,48 @@ public void ASessionWhoseMedianIsZeroIsLeftOutOfTheNormalisationRatherThanDivide Assert.Equal(0.008333, evidence.Shift.PValue); } + [Fact] + public void TheRegressionEvidenceCountsTheRunsBothArmsCompared() + { + // #182. Eleven runs of the subject, ten of which carry a comparable reading: the run whose + // own median was zero normalises nothing. Banded on the eleven the test appeared in, the + // level would claim a run the two-sample test never read. + AnalysisContext context = Build( + sessions: 11, + subjectMs: o => o < 8 ? 200 : 800, + companionMs: o => o == 3 ? 0 : CompanionMs); + + FindingCandidate candidate = Single(Analyze(context)); + var evidence = Assert.IsType(candidate.Evidence); + + Assert.Equal(11, context.Tests.SessionsRunIn($"fp-{Subject}")); + Assert.Equal( + evidence.Baseline.ComparedSessions + evidence.Current.ComparedSessions, + candidate.EvidenceSessions); + Assert.Equal(10, candidate.EvidenceSessions); + } + + [Fact] + public void TheInstabilityEvidenceCountsTheRunsThatCouldBeNormalised() + { + // #182 again, on the other kind. Two of the twelve runs are made of zero-duration + // executions, so they have no divisor and contribute no normalised reading — but the + // subject ran in them, and the dispersion is what the finding rests on. + AnalysisContext context = Build( + sessions: 12, + subjectMs: o => o < 6 ? 300 : 100, + companionMs: o => o is 3 or 4 ? 0 : CompanionMs); + + FindingCandidate candidate = Assert.Single(Unstables(context)); + var evidence = Assert.IsType(candidate.Evidence); + + Assert.Equal(12, context.Tests.SessionsRunIn($"fp-{Subject}")); + Assert.Equal(10, candidate.EvidenceSessions); + + // One reading per run here, so the two agree — what they do not agree with is the twelve. + Assert.Equal(evidence.NormalisedExecutions, candidate.EvidenceSessions); + } + [Fact] public void AWindowWithNoUsableRunMedianProducesNoDurationFindingOfEitherKind() { diff --git a/tests/Xping.Cli.Tests/Report/ExitCodeTests.cs b/tests/Xping.Cli.Tests/Report/ExitCodeTests.cs index 69c8721d..3c8f55e9 100644 --- a/tests/Xping.Cli.Tests/Report/ExitCodeTests.cs +++ b/tests/Xping.Cli.Tests/Report/ExitCodeTests.cs @@ -16,6 +16,7 @@ private static Finding Build(Severity severity) => FindingKind.Flaky, severity, EvidenceLevel.Moderate, + 10, new FindingSubject.SingleTest( new TestReference("fp-A", "N.C.M", "M", null, null, "A.Tests")), new StubEvidence(), diff --git a/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs b/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs index dc8e94b6..09617d58 100644 --- a/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs @@ -977,6 +977,18 @@ .. Enumerable.Range(9, 18).Select(i => Passing($"Fine{i}")) evidence.DiscountedClustered); Assert.Equal(0.25, evidence.FailureRate); + + // #182. Alpha ran in all six runs; the outage is not one of the occasions its own behaviour + // was observed on, so the finding rests on five. The cluster is not deducted — it removed a + // failure, not a run: Alpha still ran there and still did not fail on its own account. + FindingCandidate flaky = For(candidates, "Alpha"); + Assert.Equal(6, context.Tests.SessionsRunIn("fp-Alpha")); + Assert.Equal(5, flaky.EvidenceSessions); + + // The shared cause keeps every run, environmental ones included — an outage is a shared + // cause seen from underneath — so its own evidence is the whole of its members' history. + FindingCandidate shared = Single(candidates, FindingKind.SharedFailure); + Assert.Equal(6, shared.EvidenceSessions); } [Fact] diff --git a/tests/Xping.Cli.Tests/Report/FindingCoordinatorTests.cs b/tests/Xping.Cli.Tests/Report/FindingCoordinatorTests.cs index 2c37b7bd..7ead7254 100644 --- a/tests/Xping.Cli.Tests/Report/FindingCoordinatorTests.cs +++ b/tests/Xping.Cli.Tests/Report/FindingCoordinatorTests.cs @@ -284,14 +284,15 @@ public void TheKindFilterSkipsProvidersThatCannotContribute() } [Fact] - public void EvidenceLevelFollowsTheSubjectsSessionCount() + public void EvidenceLevelFollowsTheCandidatesOwnDenominator() { var coordinator = new FindingCoordinator( [new StubProvider("stub", FindingKind.Flaky, "Test0")]); using var warnings = new StringWriter(); - // The subject runs once in every session, so its session count is the window's. + // The stub measures over every run its subject appeared in, and the subject runs once in + // every session, so all three numbers coincide here. The next test is where they part. Assert.Equal( EvidenceLevel.Low, coordinator.Run(Context(sessionCount: 7), null, warnings).Findings[0].EvidenceLevel); @@ -305,6 +306,66 @@ public void EvidenceLevelFollowsTheSubjectsSessionCount() coordinator.Run(Context(sessionCount: 16), null, warnings).Findings[0].EvidenceLevel); } + [Fact] + public void AClaimMeasuredOverFewerRunsThanItsTestRanInIsBandedOnTheFewer() + { + // #182. A test present in all twenty runs of the window, whose finding was computed from + // ten of them - the shape every provider produces, because every provider drops runs it + // cannot read or has discounted. Banded on the subject this published as `high`, which is + // evidence the split has not got and the top of a three-level scale besides. + var coordinator = new FindingCoordinator( + [new StubProvider("stub", FindingKind.Flaky, "Test0", evidenceSessions: 10)]); + + using var warnings = new StringWriter(); + Finding finding = coordinator.Run(Context(sessionCount: 20), null, warnings).Findings[0]; + + Assert.Equal(20, Context(sessionCount: 20).Tests.SessionsRunIn("fp-Test0")); + Assert.Equal(EvidenceLevel.Moderate, finding.EvidenceLevel); + + // Published beside the band, so a reader can see which of the two numbers it came from. + Assert.Equal(10, finding.EvidenceSessions); + } + + [Fact] + public void TheReportingFloorStillReadsTheSubjectRatherThanTheClaim() + { + // The half of #182 that deliberately did not move. Whether a test is worth reporting at all + // has to be answered the same way for every kind, or one metric flags a test that another + // silently drops with nothing on screen to explain it. So a claim resting on two runs is + // still emitted where its subject has history - it is emitted saying `low`. + var coordinator = new FindingCoordinator( + [new StubProvider("stub", FindingKind.Flaky, "Test0", evidenceSessions: 2)]); + + using var warnings = new StringWriter(); + AnalysisResult result = coordinator.Run(Context(sessionCount: 20), null, warnings); + + Assert.Equal(0, result.ExcludedLowEvidence); + Assert.Equal(EvidenceLevel.Low, Assert.Single(result.Findings).EvidenceLevel); + } + + [Fact] + public void AnAlternativeIsBandedOnItsOwnDenominatorRatherThanTheClaimItReplaces() + { + // The superseding stub's alternative measures over half the runs the silenced claim did, + // which is the normal case rather than a contrivance: DurationUnstable reads what could be + // normalised where the DurationRegression it stands in for read what could be compared. + // Banding the handover on the original would label the replacement with the evidence of a + // finding that was never printed. + var coordinator = new FindingCoordinator( + [ + new SupersedingProvider( + FindingKind.DurationRegression, FindingKind.DurationUnstable, 0.9, 100) + ]); + + using var warnings = new StringWriter(); + Finding finding = Assert.Single( + coordinator.Run(Context(sessionCount: 20), null, warnings).Findings); + + Assert.Equal(FindingKind.DurationUnstable, finding.Kind); + Assert.Equal(10, finding.EvidenceSessions); + Assert.Equal(EvidenceLevel.Moderate, finding.EvidenceLevel); + } + [Fact] public void FindingIdsAreStableAcrossRepeatedReports() { @@ -455,6 +516,12 @@ private static Finding OnlyFinding(AnalysisContext context) /// pass reads. Left at their defaults the stub is an observation of something that happened, /// which is the shape RetryMasked and SharedFailure have and the shape every test /// written before that pass existed assumed. + /// + /// defaults to every session the subject ran in, which is + /// the shape of a kind that sets nothing aside. A test that wants the mismatch this stub cannot + /// otherwise produce — a claim measured over fewer runs than its subject appeared in — passes + /// its own. + /// /// private sealed class StubProvider( string name, @@ -462,7 +529,8 @@ private sealed class StubProvider( string test, double unreliability = 0.5, double? pValue = null, - int hypothesesTested = 0) + int hypothesesTested = 0, + int? evidenceSessions = null) : IFindingProvider { public string Name { get; } = name; @@ -493,6 +561,8 @@ public ProviderReport Analyze(AnalysisContext context) unreliability, LastOccurrenceIn: context.Window.Sessions[0], DrillDownCommand: "xping report", + EvidenceSessions: + evidenceSessions ?? context.Tests.SessionsRunIn($"fp-{test}"), PValue: pValue) ], family); @@ -529,6 +599,7 @@ public ProviderReport Analyze(AnalysisContext context) 0.5, LastOccurrenceIn: context.Window.Sessions[0], DrillDownCommand: "xping report", + EvidenceSessions: context.Tests.SessionsRunIn("fp-Test0"), PValue: pValue, Instead: new FindingCandidate( alternative, @@ -536,7 +607,11 @@ public ProviderReport Analyze(AnalysisContext context) new StubEvidence(2), 0.4, LastOccurrenceIn: context.Window.Sessions[0], - DrillDownCommand: "xping report")) + DrillDownCommand: "xping report", + + // Half the runs the claim it replaces was measured over, so the + // handover can be seen to band on its own denominator. + EvidenceSessions: context.Tests.SessionsRunIn("fp-Test0") / 2)) ], family); } diff --git a/tests/Xping.Cli.Tests/Report/FindingOrderTests.cs b/tests/Xping.Cli.Tests/Report/FindingOrderTests.cs index f728a244..dec01e6f 100644 --- a/tests/Xping.Cli.Tests/Report/FindingOrderTests.cs +++ b/tests/Xping.Cli.Tests/Report/FindingOrderTests.cs @@ -21,6 +21,7 @@ private static Finding Build( kind, severity, EvidenceLevel.Moderate, + 10, new FindingSubject.SingleTest( new TestReference(fingerprint, "N.C.M", "M", null, null, "A.Tests")), new StubEvidence(), diff --git a/tests/Xping.Cli.Tests/Report/ParallelSensitiveProviderTests.cs b/tests/Xping.Cli.Tests/Report/ParallelSensitiveProviderTests.cs index 96b38140..dbca69c0 100644 --- a/tests/Xping.Cli.Tests/Report/ParallelSensitiveProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/ParallelSensitiveProviderTests.cs @@ -259,6 +259,38 @@ public void ALevelKeepsItsExecutionDenominatorAndItsRunCountApart() Assert.All(evidence.Levels, l => Assert.Equal(10, l.Sessions)); } + [Fact] + public void TheEvidenceCountsTheRunsAConcurrencyCouldBeReadFrom() + { + // #182. Ten runs the adapter recorded a level for and eight it did not, all eighteen + // running the subject. The trend is over the ten, and banding it on the eighteen the test + // appeared in would label a curve built on a subset with the history of the whole. + List sessions = Split(highFailures: 5, lowFailures: 0); + + for (int i = 0; i < 8; i++) + { + sessions.Add(TestSessionFactory.Session(10 + i, + [ + TestSessionFactory.Execution( + Subject, + executionId: TestSessionFactory.ExecutionIdFor( + Subject, 10 + i, TestOutcome.Passed)) + ])); + } + + AnalysisContext context = TestSessionFactory.Context([.. sessions]); + FindingCandidate candidate = Assert.Single( + new ParallelSensitiveProvider().Analyze(context).Candidates); + + Assert.Equal(18, context.Tests.SessionsRunIn($"fp-{Subject}")); + Assert.Equal(10, candidate.EvidenceSessions); + + // The same figure the trend publishes, so the band cannot drift from the counts beside it. + var evidence = Assert.IsType(candidate.Evidence); + Assert.Equal(candidate.EvidenceSessions, evidence.Trend.Sessions); + Assert.Equal(8, evidence.ExecutionsWithoutConcurrency); + } + [Fact] public void TheTrendCarriesItsUnroundedProbabilityToTheCoordinator() { diff --git a/tests/Xping.Cli.Tests/Report/ReportEnvelopeTests.cs b/tests/Xping.Cli.Tests/Report/ReportEnvelopeTests.cs index 6dcf7021..b4f9cca8 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.13", root.GetProperty("schemaVersion").GetString()); + Assert.Equal("1.14", root.GetProperty("schemaVersion").GetString()); JsonElement window = root.GetProperty("window"); foreach (string key in @@ -167,6 +167,14 @@ public void AFindingCarriesItsSubjectAndSourceLocation() Assert.Equal("low", finding.GetProperty("severity").GetString()); Assert.Equal("low", finding.GetProperty("evidenceLevel").GetString()); + // The number the band above was decided from, published so it can be reconciled with the + // counts inside `evidence` rather than taken on trust. For a vanished test it is the + // baseline runs it appeared in, which is what its absence is a change from — not the runs + // in the window, and not the runs it ran in on either side of the slice boundary. + Assert.Equal( + finding.GetProperty("evidence").GetProperty("baselineSessions").GetInt32(), + finding.GetProperty("evidenceSessions").GetInt32()); + // Which executions the counts below were taken over. Vanished counts session appearances, so // it discounts nothing — and says so rather than leaving the reader to infer it. Assert.Equal("allExecutions", finding.GetProperty("population").GetString()); @@ -309,7 +317,7 @@ public void WarningsGoToStandardErrorSoJsonStaysParsable() // Would throw if a warning had been interleaved into stdout. using JsonDocument document = JsonDocument.Parse(output); - Assert.Equal("1.13", document.RootElement.GetProperty("schemaVersion").GetString()); + Assert.Equal("1.14", 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 11b1d1da..f01ce1e2 100644 --- a/tests/Xping.Cli.Tests/Report/RetryProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/RetryProviderTests.cs @@ -257,12 +257,12 @@ [.. MaskedPair(), .. Filler(failing: 0)])), Assert.Equal(1, context.EnvironmentalSessionCount); - RetryMaskedEvidence evidence = Assert.IsType( - Assert.Single( - Analyze(context), - c => c.Subject is FindingSubject.SingleTest test && - test.Test.TestFingerprint == $"fp-{Subject}") - .Evidence); + FindingCandidate candidate = Assert.Single( + Analyze(context), + c => c.Subject is FindingSubject.SingleTest test && + test.Test.TestFingerprint == $"fp-{Subject}"); + + RetryMaskedEvidence evidence = Assert.IsType(candidate.Evidence); // Three plain passes and three masked pairs. The outage's pair is in neither count. Assert.Equal(3, evidence.MaskedOccurrences); @@ -278,6 +278,12 @@ [.. MaskedPair(), .. Filler(failing: 0)])), // 3 of 9 rather than the 4 of 11 the undiscounted window would have published. Assert.Equal(0.333, evidence.MaskedRate); + + // #182. The subject ran in all seven runs; six are occasions its own retry behaviour was + // observed on. Counted in runs rather than in the nine executions the rate is over, because + // attempts within one run are the very correlation this kind is measuring. + Assert.Equal(7, context.Tests.SessionsRunIn($"fp-{Subject}")); + Assert.Equal(6, candidate.EvidenceSessions); } [Fact] @@ -513,12 +519,14 @@ public void ThePerTestFloorCountsSessionsRatherThanAttempts( [InlineData(8, 0, 8, "Moderate")] [InlineData(15, 0, 15, "Moderate")] [InlineData(15, 1, 16, "High")] - public void EvidenceIsBandedBySessionsOfTheSubject( + public void EvidenceIsBandedInSessionsRatherThanAttempts( int maskedSessions, int padding, int expectedSessions, string expected) { // Sessions, not executions: seven masked sessions are fourteen executions, and banding those // would call one week of a twice-retrying test better evidenced than a fortnight of a clean - // one. + // one. Nothing here is discounted, so the runs the claim was computed over and the runs the + // subject appeared in are the same number — `AnEnvironmentalRunIsLeftOutOfTheMaskedRate` is + // where they part. AnalysisContext context = Context(sessions: 24, maskedSessions, padding); Assert.Equal(expectedSessions, context.Tests.SessionsRunIn($"fp-{Subject}")); @@ -1047,11 +1055,21 @@ public void AnEnvironmentalRunIsDiscountedFromExhaustionAndCounted() : TestSessionFactory.Session(ordinal, [TestSessionFactory.Execution(Subject)])); } - RetryExhaustedEvidence evidence = ExhaustedFrom(TestSessionFactory.Context([.. built])); + AnalysisContext context = TestSessionFactory.Context([.. built]); + FindingCandidate candidate = Assert.Single(Analyze(context)); + + var evidence = Assert.IsType(candidate.Evidence); Assert.Equal(1, evidence.DiscountedEnvironmentalRuns); Assert.Equal(4, evidence.ExhaustedRuns); Assert.Equal(8, evidence.RunsConsidered); + + // #182. The nine runs the subject appeared in, less the outage — and the eight rather than + // the four retried ones, because a run that needed no retry is still an occasion the retry + // behaviour was observed on, and it is the observation that says the behaviour is not + // universal. + Assert.Equal(9, context.Tests.SessionsRunIn($"fp-{Subject}")); + Assert.Equal(evidence.RunsConsidered, candidate.EvidenceSessions); } // =========================================================================================== @@ -1314,10 +1332,18 @@ public void AnEnvironmentalRunIsLeftOutOfBothArmsAndCounted() AnalysisContext context = Depths( sessions: 12, baselineAttempts: 1, currentAttempts: 3, outageOrdinal: 0); - RetryDeepeningEvidence evidence = DeepeningFrom(context); + FindingCandidate candidate = Assert.Single(Analyze(context)); + var evidence = Assert.IsType(candidate.Evidence); Assert.Equal(1, evidence.DiscountedEnvironmentalRuns); Assert.Equal(8, evidence.Baseline.Runs); + + // #182. Both arms of the comparison and nothing else: the outage is in neither, and a + // difference between two medians rests on the runs those medians were taken over. + Assert.Equal(12, context.Tests.SessionsRunIn($"fp-{Subject}")); + Assert.Equal( + evidence.Baseline.RunsSettledGreen + evidence.Current.RunsSettledGreen, + candidate.EvidenceSessions); } // =========================================================================================== diff --git a/tests/Xping.Cli.Tests/Report/ShareableOutputTests.cs b/tests/Xping.Cli.Tests/Report/ShareableOutputTests.cs index 554faf95..6ccfbd86 100644 --- a/tests/Xping.Cli.Tests/Report/ShareableOutputTests.cs +++ b/tests/Xping.Cli.Tests/Report/ShareableOutputTests.cs @@ -988,6 +988,7 @@ private static FindingDto Finding(string kind, string severity, string name, str kind, severity, "moderate", + 10, ToCamelCase(PopulationRules.For(Enum.Parse(kind)).ToString()), new SubjectDto("test", "fp", name, name, null, null, "MyApp.Tests", null, null, null), headline, diff --git a/tests/Xping.Cli.Tests/Report/TimeSensitiveProviderTests.cs b/tests/Xping.Cli.Tests/Report/TimeSensitiveProviderTests.cs index c54165c4..de064540 100644 --- a/tests/Xping.Cli.Tests/Report/TimeSensitiveProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/TimeSensitiveProviderTests.cs @@ -595,6 +595,32 @@ public void AnOutageDoesNotManufactureAGapOutOfTheBinItLandedIn() // Scoring inputs // --------------------------------------------------------------------------------------- + [Fact] + public void TheEvidenceCountsTheRunsThatCouldBePlacedOnAClock() + { + // #182. Twelve runs carrying an offset and eight that recorded none, all twenty running the + // subject. The split is over the twelve; banded on the twenty the test appeared in, this + // published a comparison of six against six as the top of a three-level scale. + List sessions = TimeOfDay(eveningFailures: 6, morningFailures: 0); + + for (int i = 0; i < 8; i++) + sessions.Add(Session(12 + i, Local(20 + i, 14), [Execution(12 + i, false)], null)); + + AnalysisContext context = Context(sessions); + FindingCandidate candidate = Assert.Single( + new TimeSensitiveProvider().Analyze(context).Candidates); + + Assert.Equal(20, context.Tests.SessionsRunIn($"fp-{Subject}")); + Assert.Equal(12, candidate.EvidenceSessions); + + // The same twelve the two published arms add up to, so the level and the counts a reader + // can see cannot disagree. + var evidence = Assert.IsType(candidate.Evidence); + Assert.Equal( + candidate.EvidenceSessions, evidence.Worse.Sessions + evidence.Other.Sessions); + Assert.Equal(8, evidence.RunsWithoutClock); + } + [Fact] public void TheFindingIsDatedByTheFailuresThatDroveIt() { diff --git a/tests/Xping.Cli.Tests/Report/VanishedProviderTests.cs b/tests/Xping.Cli.Tests/Report/VanishedProviderTests.cs index 5192840b..9a7ef68e 100644 --- a/tests/Xping.Cli.Tests/Report/VanishedProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/VanishedProviderTests.cs @@ -428,6 +428,32 @@ public void AFilteredBaselineIsNotAHabitTheTestFailedToKeep() Assert.Equal(1.0, evidence.BaselineRunRate); } + [Fact] + public void TheEvidenceCountsTheHabitRatherThanEveryRunTheTestAppearedIn() + { + // #182. D ran in twenty runs of this window: seventeen that covered the suite, and three + // filtered ones that named only D. The absence is a change from the seventeen — the + // filtered three are set aside from both slices before anything is counted, and the current + // slice holds none of D's appearances by construction. + var sessions = new List(); + for (int i = 0; i < 17; i++) + sessions.Add(TestSessionFactory.Session(i, "A", "B", "C", "D")); + for (int i = 17; i < 20; i++) + sessions.Add(TestSessionFactory.Session(i, "D")); + for (int i = 20; i < 23; i++) + sessions.Add(TestSessionFactory.Session(i, "A", "B", "C")); + + AnalysisContext context = TestSessionFactory.Context([.. sessions]); + FindingCandidate candidate = Assert.Single(Analyze(context)); + + var evidence = Assert.IsType(candidate.Evidence); + + Assert.Equal(20, context.Tests.SessionsRunIn("fp-D")); + Assert.Equal(3, evidence.PartialSessionsSetAside); + Assert.Equal(17, candidate.EvidenceSessions); + Assert.Equal(evidence.BaselineSessions, candidate.EvidenceSessions); + } + [Fact] public void TheEvidenceSaysHowManyRunsCoveredOnlyPartOfTheSuite() {