From d8367707dfc094bb687b881ef99ef1d1168afb8d Mon Sep 17 00:00:00 2001 From: xping-admin Date: Mon, 7 Sep 2026 11:30:57 +0200 Subject: [PATCH 1/2] perf(cli): count a cluster's members once, not once per member (#175) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #175 reported that `SharedFailure` counts each cluster member's failures with a linear scan over the whole cluster, inside the loop over the cluster's members — `O(|Fingerprints| x |Failures|)` with an ordinal string comparison in the inner loop, once per cluster. The scaling factor is the payload the kind exists to report. A broken fixture takes out every test in an assembly, so the members and the failures grow together and the pathological input is precisely the successful case. The default window reads 20 sessions, which bounds it; `--since` reads up to 1000. `SignatureGroup` gains `FailuresByFingerprint`, counted in the same walk of the same list that already produces `Fingerprints`, `MaxTestsInOneSession` and `SessionCount` — so the counts are free there, and there is no longer a second place that could decide what "this member's failures" means. `SharedFailure` reads them. The loop keeps its shape: `references` and `members` are appended together only when the fingerprint resolves, and the subject is built from the first while the evidence is built from the second. `Spread` was the other half, on the same list and hit once per cluster. It ordered every failure under a four-key comparator — materialising `ExecutionId.ToString("N")` for each one — to read three entries off the front. It now keeps the earliest failure per key in one pass and orders those, which is the same answer: the entries of the ordered list that introduce a new key are exactly the per-key minima. The top-up branch, reached only below three distinct keys, selects into a bounded shortlist rather than sorting. The last comparison key stays the hex form rather than `Guid.CompareTo`, which orders differently; it is now formatted only for two attempts of one test in one run. Measured on 500 runs x 1000 tests all failing on one signature: cluster analysis 22,960ms to 75ms, with the emitted evidence byte-identical. At the issue's headline 1000 x 2000 it is 339ms. What now bounds that store is `SignatureIndex.Build` signing two million failed executions at 6.7s — linear, and a separate question from this one. Two new tests. Nothing in the suite asserted `ClusterMember.Failures` at all, and nothing pinned which exemplars a cluster picks rather than how many. Both pass against the pre-change provider, which is what makes them evidence that the selection is unchanged rather than merely self-consistent. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TMQcZLBGFTd8wz3mcfn9vc --- .../Report/Indexes/SignatureIndex.cs | 26 ++++- .../Report/Providers/FailureModeProvider.cs | 109 ++++++++++++++---- .../Report/FailureModeProviderTests.cs | 56 ++++++++- 3 files changed, 164 insertions(+), 27 deletions(-) diff --git a/src/Xping.Cli/Report/Indexes/SignatureIndex.cs b/src/Xping.Cli/Report/Indexes/SignatureIndex.cs index 03bbe53..0ab6793 100644 --- a/src/Xping.Cli/Report/Indexes/SignatureIndex.cs +++ b/src/Xping.Cli/Report/Indexes/SignatureIndex.cs @@ -34,6 +34,11 @@ internal sealed record SignatureOccurrence( /// The signature, readable components and all. /// Distinct tests that failed with it, in ordinal order. /// Every failure carrying it, newest first. +/// +/// How many of those failures each test contributed. Counted here rather than by whoever needs the +/// number, so that the count of a member's failures and the decision that it is a member come from +/// one walk of one list and cannot disagree. +/// /// /// The most distinct tests it hit within a single session — the measurement the shared-failure /// threshold is applied to. @@ -46,6 +51,7 @@ internal sealed record SignatureGroup( FailureSignature Signature, IReadOnlyList Fingerprints, IReadOnlyList Failures, + IReadOnlyDictionary FailuresByFingerprint, int MaxTestsInOneSession, int SessionCount, int OldestSessionIndex, @@ -254,11 +260,20 @@ private static Dictionary BuildPerSignature( .ThenBy(f => f.Execution.ExecutionId.ToString("N", CultureInfo.InvariantCulture), StringComparer.Ordinal)]; - var fingerprints = failures - .Select(f => f.Execution.Identity.TestFingerprint) - .Distinct(StringComparer.Ordinal) - .OrderBy(f => f, StringComparer.Ordinal) - .ToList(); + // Membership and each member's share of the failures come out of the same walk. Counting + // them separately later would be quadratic in the size of the cluster, and a cluster is + // widest exactly when the finding matters most: one broken fixture takes the whole + // assembly with it, so the members and the failures grow together. + var failuresByFingerprint = new Dictionary(StringComparer.Ordinal); + foreach (ExecutionRef failure in failures) + { + string fingerprint = failure.Execution.Identity.TestFingerprint; + failuresByFingerprint[fingerprint] = + failuresByFingerprint.TryGetValue(fingerprint, out int seen) ? seen + 1 : 1; + } + + List fingerprints = + [.. failuresByFingerprint.Keys.OrderBy(f => f, StringComparer.Ordinal)]; // The shared-failure threshold is about one run: three tests failing the same way in the // same session share something. Three tests failing the same way in three different @@ -277,6 +292,7 @@ private static Dictionary BuildPerSignature( signatures[entry.Key], fingerprints, failures, + failuresByFingerprint, maxTestsInOneSession, sessionCount, failures.Max(f => f.SessionIndex), diff --git a/src/Xping.Cli/Report/Providers/FailureModeProvider.cs b/src/Xping.Cli/Report/Providers/FailureModeProvider.cs index 1569957..83d4be5 100644 --- a/src/Xping.Cli/Report/Providers/FailureModeProvider.cs +++ b/src/Xping.Cli/Report/Providers/FailureModeProvider.cs @@ -447,9 +447,7 @@ private static FindingCandidate SharedFailure(AnalysisContext context, Signature references.Add(reference); - int failures = cluster.Failures.Count( - f => string.Equals( - f.Execution.Identity.TestFingerprint, fingerprint, StringComparison.Ordinal)); + cluster.FailuresByFingerprint.TryGetValue(fingerprint, out int failures); members.Add(new ClusterMember(fingerprint, reference.FullyQualifiedName, failures)); @@ -985,27 +983,52 @@ private static Func SignatureHashOf(AnalysisContext contex private static List Spread( IReadOnlyList failures, Func key) { - List ordered = [.. failures - .OrderBy(f => f.SessionIndex) - .ThenBy(f => f.Execution.Retry?.AttemptNumber ?? 1) - .ThenBy(f => f.Execution.Identity.TestFingerprint, StringComparer.Ordinal) - .ThenBy(f => f.Execution.ExecutionId.ToString("N", CultureInfo.InvariantCulture), - StringComparer.Ordinal)]; - - var seen = new HashSet(StringComparer.Ordinal); - List chosen = []; + // Selected rather than sorted. A cluster's failure list is the largest collection the report + // handles — every test in a broken assembly, once per run — and ordering all of it to read + // three entries off the front was the expensive half of assembling that finding. + var earliestPerKey = new Dictionary(StringComparer.Ordinal); - foreach (ExecutionRef reference in ordered) + foreach (ExecutionRef reference in failures) { - if (chosen.Count == MaxExemplars) - break; - - if (seen.Add(key(reference))) - chosen.Add(reference); + if (!earliestPerKey.TryGetValue(key(reference), out ExecutionRef? held) || + ByExemplarOrder(reference, held) < 0) + { + earliestPerKey[key(reference)] = reference; + } } + // The entries of the ordered list that introduce a new key are exactly the earliest failure + // under each key, so ordering those answers the same question the ordered walk did. No two + // of them can tie: a tie needs the same session, attempt, test and execution id, which is + // one execution and so one key. + List candidates = [.. earliestPerKey.Values]; + candidates.Sort(ByExemplarOrder); + + List chosen = [.. candidates.Take(MaxExemplars)]; + // Then top up in the same order, so a test with one failure mode still gets three exemplars. - foreach (ExecutionRef reference in ordered) + if (chosen.Count < MaxExemplars) + TopUp(failures, chosen); + + return chosen; + } + + /// + /// Fills the remaining exemplar slots from the earliest failures overall, repeating a key. + /// + /// + /// Reached only when the failures span fewer keys than there are exemplars to fill, so fewer + /// than entries are already spoken for. Holding twice that many + /// leaves the shortlist unable to run dry before is full. + /// + private static void TopUp(IReadOnlyList failures, List chosen) + { + List shortlist = new(MaxExemplars * 2); + + foreach (ExecutionRef reference in failures) + Offer(shortlist, reference, MaxExemplars * 2); + + foreach (ExecutionRef reference in shortlist) { if (chosen.Count == MaxExemplars) break; @@ -1013,8 +1036,54 @@ private static List Spread( if (!chosen.Contains(reference)) chosen.Add(reference); } + } - return chosen; + /// + /// Keeps a short list holding the earliest failures offered to it. + /// + private static void Offer(List shortlist, ExecutionRef reference, int capacity) + { + if (shortlist.Count == capacity && ByExemplarOrder(reference, shortlist[^1]) >= 0) + return; + + int at = shortlist.Count; + while (at > 0 && ByExemplarOrder(reference, shortlist[at - 1]) < 0) + at--; + + shortlist.Insert(at, reference); + + if (shortlist.Count > capacity) + shortlist.RemoveAt(shortlist.Count - 1); + } + + /// + /// Orders failures the way exemplars are picked: newest run first, then earliest attempt. + /// + /// + /// Total, so the same window always yields the same exemplars. Sessions are indexed newest + /// first, which is why ascending order here reads as most recent. + /// + private static int ByExemplarOrder(ExecutionRef left, ExecutionRef right) + { + int bySession = left.SessionIndex.CompareTo(right.SessionIndex); + if (bySession != 0) + return bySession; + + int byAttempt = (left.Execution.Retry?.AttemptNumber ?? 1) + .CompareTo(right.Execution.Retry?.AttemptNumber ?? 1); + if (byAttempt != 0) + return byAttempt; + + int byTest = string.CompareOrdinal( + left.Execution.Identity.TestFingerprint, right.Execution.Identity.TestFingerprint); + if (byTest != 0) + return byTest; + + // Ordinal over the hex form rather than Guid.CompareTo, which orders differently. Reached + // only by two attempts of one test in one run, so the formatting is off the hot path. + return string.CompareOrdinal( + left.Execution.ExecutionId.ToString("N", CultureInfo.InvariantCulture), + right.Execution.ExecutionId.ToString("N", CultureInfo.InvariantCulture)); } private static FailureExemplar ToExemplar(AnalysisContext context, ExecutionRef reference) diff --git a/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs b/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs index e4d57b5..53acd0a 100644 --- a/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs @@ -24,9 +24,13 @@ public sealed class FailureModeProviderTests private static TestExecution Passing(string name) => TestSessionFactory.Execution(name); /// A failure every test in a fixture can share, so it clusters. - private static TestExecution SharedFailure(string name) => + private static TestExecution SharedFailure(string name, int durationMs = 100) => TestSessionFactory.Execution( - name, TestOutcome.Failed, exceptionType: SharedType, errorMessage: SharedMessage); + name, + TestOutcome.Failed, + durationMs: durationMs, + exceptionType: SharedType, + errorMessage: SharedMessage); /// The same shared failure, recorded in a named lifecycle member. private static TestExecution FixtureFailure( @@ -536,6 +540,54 @@ public void ThreeTestsFailingAlikeInOneRunBecomeOneFinding() evidence.Members.Select(m => m.Fingerprint)); } + [Fact] + public void EachClusterMemberCarriesItsOwnFailureCount() + { + // The members share a cause, not a history. A test the cluster took out three times is not + // evidence of the same weight as one it took out once, and the report says which is which. + TestSession[] sessions = + [ + .. FourQuietRuns(), + TestSessionFactory.Session( + 4, [SharedFailure("Alpha"), SharedFailure("Beta"), SharedFailure("Gamma")]), + TestSessionFactory.Session( + 5, [SharedFailure("Alpha"), SharedFailure("Beta"), Passing("Gamma")]), + TestSessionFactory.Session( + 6, [SharedFailure("Alpha"), Passing("Beta"), Passing("Gamma")]) + ]; + + var evidence = Assert.IsType( + Single(Analyze(sessions), FindingKind.SharedFailure).Evidence); + + Assert.Equal( + ["fp-Alpha", "fp-Beta", "fp-Gamma"], + evidence.Members.Select(m => m.Fingerprint)); + Assert.Equal([3, 2, 1], evidence.Members.Select(m => m.Failures)); + Assert.Equal(6, evidence.Failures); + } + + [Fact] + public void ClusterExemplarsAreOneMemberEachInOrdinalOrder() + { + // Which three, not just how many. Six members failed in one run, so the three exemplars are + // the ordinally first three — Alpha, Beta and Delta, which is not the order they were + // recorded in. Durations stand in for identity, since the exemplars of one cluster carry + // the same exception and message by construction. + TestSession[] sessions = + [ + .. Enumerable.Range(0, 4).Select(ordinal => + TestSessionFactory.Session(ordinal, [.. ClusterFixtureTests.Select(Passing)])), + TestSessionFactory.Session( + 4, + [.. ClusterFixtureTests.Select((name, index) => SharedFailure(name, 10 + index))]) + ]; + + var evidence = Assert.IsType( + Single(Analyze(sessions), FindingKind.SharedFailure).Evidence); + + Assert.Equal([10L, 11L, 13L], evidence.Exemplars.Select(e => e.DurationMs)); + } + /// /// The point of the failure site: the same three failures, but every one of them recorded in the /// same setup method, so the report can name the member to fix instead of listing three tests. From 9fff503f2b5de0b9b8889dbb31dfb33a968b8259 Mon Sep 17 00:00:00 2001 From: xping-admin Date: Mon, 7 Sep 2026 11:56:49 +0200 Subject: [PATCH 2/2] test(cli): pin which exemplars are chosen, not just how many MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #201. `TopUp` was reached only by tests asserting the exemplar count, and the cluster-order test added with the refactor has six distinct keys, so it never enters that branch. A change promising byte-identical evidence needs the repeated-key case held to the identities it published before, not to its length. Four tests, each naming a rule the ordering makes and none of them reachable through the count alone: One mode, five failures: the three exemplars are the three most recent, so they describe what the test does now. Two modes, four failures: each mode is shown once before either is shown twice, so the third exemplar is the newest of what is left rather than the third newest failure. That distinction is the reason the spread exists and nothing asserted it. A run that recorded its seventh retry attempt first: the exemplars are still its earliest attempts. Nothing promises an adapter writes attempts in order, and the bounded shortlist must not depend on it — this is also the only fixture in which a better failure is offered after the shortlist is full. Two executions of one test in one run, from an adapter tracking no retry metadata: they agree on everything the order reads except their execution ids, and the tie-break `TestSessionFactory` already derives its ids to keep testable is what separates them. All four pass against the pre-change provider, which is what makes them evidence about the selection rather than about the new implementation agreeing with itself. Patch coverage of the branch goes to 100%: what was uncovered was the shortlist's insertion and eviction, the attempt comparison against a failure carrying no retry metadata, and the execution-id fallback. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TMQcZLBGFTd8wz3mcfn9vc --- .../Report/FailureModeProviderTests.cs | 121 +++++++++++++++++- 1 file changed, 120 insertions(+), 1 deletion(-) diff --git a/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs b/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs index 53acd0a..dc8e94b 100644 --- a/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs +++ b/tests/Xping.Cli.Tests/Report/FailureModeProviderTests.cs @@ -52,14 +52,27 @@ private static TestExecution OtherSharedFailure(string name) => errorMessage: "The operation timed out"); /// A failure whose signature is the test's own. - private static TestExecution Failure(string name, string message = "unexpected null") => + private static TestExecution Failure( + string name, string message = "unexpected null", int durationMs = 100) => TestSessionFactory.Execution( name, TestOutcome.Failed, + durationMs: durationMs, exceptionType: "System.InvalidOperationException", errorMessage: message, stackTrace: $" at MyApp.Tests.SampleTests.{name}()"); + /// The same failure, recorded as one attempt of a run that retried. + private static TestExecution FailedAttempt(string name, int attempt, int maxRetries) => + TestSessionFactory.Execution( + name, + TestOutcome.Failed, + attempt: attempt, + maxRetries: maxRetries, + exceptionType: "System.InvalidOperationException", + errorMessage: "unexpected null", + stackTrace: $" at MyApp.Tests.SampleTests.{name}()"); + private static List Analyze(params TestSession[] sessions) => [.. new FailureModeProvider().Analyze(TestSessionFactory.Context(sessions)).Candidates]; @@ -1114,6 +1127,112 @@ public void ExemplarsCoverDistinctFailureModesBeforeRepeatingOne() Assert.Equal(3, evidence.Exemplars.Select(e => e.SignatureHash).Distinct().Count()); } + [Fact] + public void ThreeExemplarsOfOneFailureModeAreTheThreeMostRecent() + { + // A test that only ever fails one way still gets three exemplars, and which three is not + // incidental: they are the most recent, so they describe what the test does now rather than + // what it did a fortnight ago. + TestSession[] sessions = [.. Enumerable.Range(0, 10).Select(ordinal => + TestSessionFactory.Session( + ordinal, + [ordinal >= 5 + ? Failure("Subject", durationMs: 100 + ordinal) + : Passing("Subject")]))]; + + var evidence = Assert.IsType( + Single(Analyze(sessions), FindingKind.Flaky).Evidence); + + Assert.Equal([109L, 108L, 107L], evidence.Exemplars.Select(e => e.DurationMs)); + } + + [Fact] + public void TheThirdExemplarRepeatsAModeOnlyAfterBothHaveBeenShown() + { + // Two modes, three slots. Each mode is shown once before either is shown twice, so the + // third exemplar is the newest of what is left over — which is not the third newest + // failure, and is the whole point of spreading them. + TestSession[] sessions = [.. Enumerable.Range(0, 10).Select(ordinal => + TestSessionFactory.Session( + ordinal, + [ordinal >= 6 + ? Failure("Subject", ordinal == 7 ? "mode b" : "mode a", 100 + ordinal) + : Passing("Subject")]))]; + + var evidence = Assert.IsType( + Single(Analyze(sessions), FindingKind.Flaky).Evidence); + + // Newest of mode a, then newest of mode b, then the next newest failure overall. + Assert.Equal([109L, 107L, 108L], evidence.Exemplars.Select(e => e.DurationMs)); + } + + [Fact] + public void ExemplarsOfARetriedRunAreItsEarliestAttempts() + { + // Nothing promises an adapter writes a run's attempts in order, and the exemplars must not + // depend on it: this run recorded its seventh attempt first. + TestSession[] sessions = + [ + .. Enumerable.Range(0, 7).Select(ordinal => + TestSessionFactory.Session(ordinal, [Passing("Subject")])), + TestSessionFactory.Session(7, [Failure("Subject")]), + TestSessionFactory.Session(8, [Failure("Subject")]), + TestSessionFactory.Session( + 9, + [.. Enumerable.Range(1, 7).Reverse().Select( + attempt => FailedAttempt("Subject", attempt, maxRetries: 6))]) + ]; + + var evidence = Assert.IsType( + Single(Analyze(sessions), FindingKind.Flaky).Evidence); + + Assert.Equal([1, 2, 3], evidence.Exemplars.Select(e => e.AttemptNumber)); + } + + [Fact] + public void TwoFailuresRecordedAlikeInOneRunStillOrderTheSameWay() + { + // The last tie-break. Two executions of one test in one run, recorded by an adapter that + // tracks no retry metadata, agree on everything the order reads except their execution ids + // — and the exemplars have to pick the same one every time, or the published report moves + // between runs over an unchanged store. + TestExecution second = TestSessionFactory.Execution( + "Subject", + TestOutcome.Failed, + durationMs: 100, + executionId: new Guid("00000000-0000-0000-0000-0000000000bb"), + retry: false, + exceptionType: "System.InvalidOperationException", + errorMessage: "unexpected null", + stackTrace: " at MyApp.Tests.SampleTests.Subject()"); + + TestExecution first = TestSessionFactory.Execution( + "Subject", + TestOutcome.Failed, + durationMs: 300, + executionId: new Guid("00000000-0000-0000-0000-0000000000aa"), + retry: false, + exceptionType: "System.InvalidOperationException", + errorMessage: "unexpected null", + stackTrace: " at MyApp.Tests.SampleTests.Subject()"); + + TestSession[] sessions = + [ + .. Enumerable.Range(0, 7).Select(ordinal => + TestSessionFactory.Session(ordinal, [Passing("Subject")])), + TestSessionFactory.Session(7, [Failure("Subject", durationMs: 400)]), + TestSessionFactory.Session(8, [Failure("Subject", durationMs: 200)]), + + // Recorded id-descending, which is not the order they are published in. + TestSessionFactory.Session(9, [second, first]) + ]; + + var evidence = Assert.IsType( + Single(Analyze(sessions), FindingKind.Flaky).Evidence); + + Assert.Equal([300L, 100L, 200L], evidence.Exemplars.Select(e => e.DurationMs)); + } + [Fact] public void ALongMessageIsCutToTheBudgetAndMarked() {