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..dc8e94b 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( @@ -48,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]; @@ -536,6 +553,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. @@ -1062,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() {