diff --git a/src/Xping.Cli/Report/Indexes/SessionOutcomes.cs b/src/Xping.Cli/Report/Indexes/SessionOutcomes.cs index a6fcc6d..7f570b1 100644 --- a/src/Xping.Cli/Report/Indexes/SessionOutcomes.cs +++ b/src/Xping.Cli/Report/Indexes/SessionOutcomes.cs @@ -39,22 +39,6 @@ public static (int Tests, int Failures) Tally(TestSession session) return (tests, failures); } - /// - /// Returns whether a session ended with at least one test failing on its final attempt. - /// - /// The session to inspect. - /// when the session ended red. - public static bool HasFinalFailure(TestSession session) - { - foreach (var outcome in FinalOutcomes(session).Values) - { - if (outcome.Outcome.IsFailure()) - return true; - } - - return false; - } - /// /// Reduces a session to one outcome per test, taken from its last attempt. /// diff --git a/src/Xping.Cli/Report/Indexes/TestIndex.cs b/src/Xping.Cli/Report/Indexes/TestIndex.cs index 4ff42e9..a329e61 100644 --- a/src/Xping.Cli/Report/Indexes/TestIndex.cs +++ b/src/Xping.Cli/Report/Indexes/TestIndex.cs @@ -43,7 +43,6 @@ internal sealed class TestIndex private readonly Dictionary _sessionsRunIn; private readonly Dictionary _references; private readonly Dictionary _sessionPositions; - private readonly HashSet _sessionsWithFinalFailures; private TestIndex( AnalysisWindow window, @@ -52,7 +51,6 @@ private TestIndex( Dictionary sessionsRunIn, Dictionary references, Dictionary sessionPositions, - HashSet sessionsWithFinalFailures, IReadOnlyList fingerprints) { Window = window; @@ -61,7 +59,6 @@ private TestIndex( _sessionsRunIn = sessionsRunIn; _references = references; _sessionPositions = sessionPositions; - _sessionsWithFinalFailures = sessionsWithFinalFailures; Fingerprints = fingerprints; } @@ -201,30 +198,61 @@ public double RunFrequencyOf(string fingerprint) } /// - /// Gets the fraction of a test's failures that landed in a session that ended up failing. + /// Gets the fraction of the sessions a test failed in that it ended red. /// /// The test to measure. /// A value in [0,1]; zero when the test never failed. /// + /// /// Separates a test that fails and blocks the build from one whose failures are always masked by /// a retry. Both are worth fixing; only the first is stopping anyone today. + /// + /// + /// Both counts are sessions, for the reason gives: a retried run + /// records an execution per attempt, so counting attempts put every masked failure in the + /// denominator and none in the numerator, and a test that failed four times in one green session + /// and once in a red one scored 0.20 where the occasions say 0.50. The bias ran hardest against + /// the tests the retry findings report, and it made this term disagree about its unit with the + /// one immediately above it in the same scorer. + /// + /// + /// Blocking means this test ended the session red, read off its deciding attempt via + /// . Asking instead whether the session ended red — as this did — credited a + /// test whose every failure was masked with blocking the build whenever some other test failed + /// finally in the same session, which is the opposite of the separation above. + /// /// public double BlockingRateOf(string fingerprint) { - int failures = 0; - int blocking = 0; + int failedSessions = 0; + int lastFailedSession = -1; + // Executions arrive grouped by session, so a session is new to the tally only when the last + // failure counted came from a different one. The same walk Build takes, and for the same + // reason: a retried run must not count as several occasions. foreach (ExecutionRef reference in ExecutionsOf(fingerprint)) { - if (!reference.Failed) + if (!reference.Failed || reference.SessionIndex == lastFailedSession) continue; - failures++; - if (_sessionsWithFinalFailures.Contains(reference.Session.SessionId)) + failedSessions++; + lastFailedSession = reference.SessionIndex; + } + + if (failedSessions == 0) + return 0; + + int blocking = 0; + + foreach (ExecutionRef run in RunsOf(fingerprint)) + { + if (run.Failed) blocking++; } - return failures == 0 ? 0 : (double)blocking / failures; + // A run that failed is a session the test failed in, so the numerator cannot outrun the + // denominator and the ratio needs no clamping. + return (double)blocking / failedSessions; } /// @@ -313,7 +341,6 @@ public static TestIndex Build(AnalysisWindow window) var sessionsRunIn = new Dictionary(StringComparer.Ordinal); var references = new Dictionary(StringComparer.Ordinal); var sessionPositions = new Dictionary(); - var sessionsWithFinalFailures = new HashSet(); for (int position = 0; position < window.Sessions.Count; position++) { @@ -365,9 +392,6 @@ public static TestIndex Build(AnalysisWindow window) if (!references.ContainsKey(fingerprint)) references[fingerprint] = ToReference(execution); } - - if (SessionOutcomes.HasFinalFailure(session)) - sessionsWithFinalFailures.Add(session.SessionId); } var fingerprints = byFingerprint.Keys.OrderBy(f => f, StringComparer.Ordinal).ToList(); @@ -379,7 +403,6 @@ public static TestIndex Build(AnalysisWindow window) sessionsRunIn, references, sessionPositions, - sessionsWithFinalFailures, fingerprints); } diff --git a/tests/Xping.Cli.Tests/Report/TestIndexTests.cs b/tests/Xping.Cli.Tests/Report/TestIndexTests.cs index b2a37fe..d9abb24 100644 --- a/tests/Xping.Cli.Tests/Report/TestIndexTests.cs +++ b/tests/Xping.Cli.Tests/Report/TestIndexTests.cs @@ -175,7 +175,7 @@ public void ARunAgreesWithSessionOutcomesWhenAttemptsArriveOutOfOrder() TestIndex.Build(TestSessionFactory.Window(session)).RunsOf(SubjectFingerprint)); Assert.True(run.Failed); - Assert.True(SessionOutcomes.HasFinalFailure(session)); + Assert.Equal((1, 1), SessionOutcomes.Tally(session)); } [Fact] @@ -196,6 +196,110 @@ public void RunsAreOrderedNewestSessionFirstLikeExecutions() Assert.Equal(Enumerable.Range(0, 8), runs.Select(r => r.SessionIndex)); } + // --------------------------------------------------------------------------------------- + // Blocking rate + // --------------------------------------------------------------------------------------- + + /// + /// Builds one session in which took attempts + /// and ended the way says. + /// + /// The session's position; 0 is the oldest. + /// How many attempts the subject took. + /// The outcome of the deciding attempt; every earlier one failed. + /// A second test that ends the session red on its only attempt. + private static TestSession Attempted( + int ordinal, + int attempts, + TestOutcome ends, + bool alsoFailing = false) + { + var executions = new List(); + + for (int attempt = 1; attempt <= attempts; attempt++) + { + bool deciding = attempt == attempts; + + executions.Add(TestSessionFactory.Execution( + Subject, + outcome: deciding ? ends : TestOutcome.Failed, + attempt: attempt, + passedOnRetry: deciding && ends == TestOutcome.Passed && attempts > 1, + maxRetries: attempts - 1, + errorMessage: deciding && ends == TestOutcome.Passed ? null : "boom")); + } + + executions.Add(alsoFailing + ? TestSessionFactory.Execution("Neighbour", TestOutcome.Failed, errorMessage: "boom") + : TestSessionFactory.Execution("Neighbour")); + + return TestSessionFactory.Session(ordinal, executions); + } + + [Fact] + public void BlockingRateCountsSessionsRatherThanAttempts() + { + // The bug this pins: four failed attempts in one session that ended green and one failure in + // a session that ended red is five failed executions of which one blocked — 0.20 — where the + // occasions say one of two, 0.50. Counting attempts made a test look less blocking the more + // it retried, which is the opposite of the truth and worst for the tests the retry findings + // already report. + TestIndex index = TestIndex.Build(TestSessionFactory.Window( + Attempted(0, attempts: 5, ends: TestOutcome.Passed), + Attempted(1, attempts: 1, ends: TestOutcome.Failed))); + + Assert.Equal(0.50, index.BlockingRateOf(SubjectFingerprint), 3); + } + + [Fact] + public void AMaskedFailureIsNotBlockingEvenWhenAnotherTestFailedTheSession() + { + // Blocking has to mean this test ended the session red. Asking whether the session ended red + // credited a test whose every failure was masked with blocking the build whenever a + // neighbour failed finally in the same session. + TestIndex index = TestIndex.Build(TestSessionFactory.Window( + Attempted(0, attempts: 3, ends: TestOutcome.Passed, alsoFailing: true), + Attempted(1, attempts: 2, ends: TestOutcome.Passed, alsoFailing: true))); + + Assert.Equal(0, index.BlockingRateOf(SubjectFingerprint)); + } + + [Fact] + public void ATestThatEndsEverySessionRedBlocksEveryTimeItFails() + { + TestIndex index = TestIndex.Build(TestSessionFactory.Window( + Attempted(0, attempts: 1, ends: TestOutcome.Failed), + Attempted(1, attempts: 3, ends: TestOutcome.Failed))); + + Assert.Equal(1.0, index.BlockingRateOf(SubjectFingerprint)); + } + + [Fact] + public void ATimeoutCountsAsAFailureAndAsABlock() + { + // Failure is TestOutcome.Failed or TestOutcome.Timeout, and both counts read it the same way. + TestIndex index = TestIndex.Build(TestSessionFactory.Window( + Attempted(0, attempts: 1, ends: TestOutcome.Timeout))); + + Assert.Equal(1.0, index.BlockingRateOf(SubjectFingerprint)); + } + + [Fact] + public void BlockingRateIsZeroForATestThatNeverFailed() + { + TestIndex index = TestIndex.Build(Window(total: 6, presentIn: 6, attempts: 1)); + + Assert.Equal(0, index.BlockingRateOf(SubjectFingerprint)); + } + + [Fact] + public void BlockingRateIsZeroForAFingerprintTheWindowNeverSaw() + { + TestIndex index = TestIndex.Build(Window(total: 6, presentIn: 6, attempts: 2)); + + Assert.Equal(0, index.BlockingRateOf("fp-NeverRan")); + } + // --------------------------------------------------------------------------------------- // Recency // ---------------------------------------------------------------------------------------