diff --git a/BuildMonitor.Test/GitHubRunErrorsTests.cs b/BuildMonitor.Test/GitHubRunErrorsTests.cs index 3cd3e6f..2300207 100644 --- a/BuildMonitor.Test/GitHubRunErrorsTests.cs +++ b/BuildMonitor.Test/GitHubRunErrorsTests.cs @@ -66,6 +66,61 @@ public void ASecondUpdateOfAFailedRunDoesNotFetchItsJobsAgain() Assert.IsFalse(fetchAgain); } + /// + /// A re-run keeps the run id. When attempt 2 also failed and was never seen running, the stored run + /// was already a failure with attempt 1's errors, so its jobs were never fetched again + /// (ktsu-dev/BuildMonitor#394). + /// + [TestMethod] + public void AFailedReRunOfAFailedRunFetchesItsJobsAgain() + { + // Arrange + DateTimeOffset firstAttempt = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); + Run run = FailedRun(); + run.Started = firstAttempt; + run.Errors = ["[build] OLD: error CS1002 from attempt 1"]; + + // Act + GitHub.StartAttempt(run, firstAttempt.AddHours(1)); + + // Assert + Assert.IsEmpty(run.Errors, "Attempt 1's errors don't describe attempt 2"); + Assert.AreEqual(firstAttempt.AddHours(1), run.Started); + Assert.IsTrue(GitHub.ShouldFetchRunErrors(RunStatus.Failure, RunStatus.Failure, run.Errors), "The new attempt's failure should be read from its jobs"); + } + + [TestMethod] + public void RereadingAnUnchangedFailedRunKeepsItsErrorsAndDoesNotRefetch() + { + // Arrange + DateTimeOffset started = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); + Run run = FailedRun(); + run.Started = started; + run.Errors = ["[build] error CS1002: ; expected"]; + + // Act + GitHub.StartAttempt(run, started); + + // Assert + AssertErrors(run, "[build] error CS1002: ; expected"); + Assert.IsFalse(GitHub.ShouldFetchRunErrors(RunStatus.Failure, RunStatus.Failure, run.Errors), "The #311 guarantee: an unchanged failed run is not refetched on every update"); + } + + [TestMethod] + public void ARunReadForTheFirstTimeKeepsWhateverItHas() + { + // Arrange + Run run = FailedRun(); + DateTimeOffset started = new(2026, 11, 3, 9, 0, 0, TimeSpan.Zero); + + // Act + GitHub.StartAttempt(run, started); + + // Assert + Assert.AreEqual(started, run.Started); + Assert.IsFalse(GitHub.IsNewAttempt(default, started)); + } + [TestMethod] [DataRow(WorkflowJobConclusion.Failure, true)] [DataRow(WorkflowJobConclusion.TimedOut, true)] diff --git a/BuildMonitor/Providers/GitHub.cs b/BuildMonitor/Providers/GitHub.cs index d71259c..ddfbaf7 100644 --- a/BuildMonitor/Providers/GitHub.cs +++ b/BuildMonitor/Providers/GitHub.cs @@ -515,7 +515,7 @@ internal static RunStatus MapWorkflowRunStatus(StringEnum run private async Task UpdateRunFromWorkflowAsync(Run run, WorkflowRun gitHubRun) { - run.Started = gitHubRun.RunStartedAt; + StartAttempt(run, gitHubRun.RunStartedAt); run.LastUpdated = gitHubRun.UpdatedAt; run.Branch = gitHubRun.HeadBranch.As(); @@ -559,11 +559,36 @@ private async Task UpdateRunFromWorkflowAsync(Run run, WorkflowRun gitHubRun) /// Whether a run's jobs should be fetched for error details: when it has just failed, or is a /// failure whose errors have not been fetched yet. A completed fetch always leaves at least one /// error behind (see ), so it is not repeated on every update - /// (ktsu-dev/BuildMonitor#311). + /// (ktsu-dev/BuildMonitor#311). A new attempt of a failed run has its errors dropped by + /// , so it is fetched again too (ktsu-dev/BuildMonitor#394). /// internal static bool ShouldFetchRunErrors(RunStatus status, RunStatus previousStatus, IReadOnlyList errors) => status == RunStatus.Failure && (previousStatus != RunStatus.Failure || errors.Count == 0); + /// + /// Whether a run read from GitHub is a later attempt than the one stored: a re-run keeps the run + /// id and gets a new start time. A run that has never been read has no stored start time, and is + /// not a re-run. + /// + internal static bool IsNewAttempt(DateTimeOffset storedStarted, DateTimeOffset started) => + storedStarted != default && started != storedStarted; + + /// + /// Records the start time GitHub reports for a run. A re-run keeps the run id but starts a new + /// attempt with its own start time, and the previous attempt's errors no longer describe the run, + /// so they are dropped, which makes fetch them afresh if the new + /// attempt failed too (ktsu-dev/BuildMonitor#394). + /// + internal static void StartAttempt(Run run, DateTimeOffset started) + { + if (IsNewAttempt(run.Started, started)) + { + run.Errors = []; + } + + run.Started = started; + } + /// /// Whether a job's conclusion is one that fails its run. A job that hit its timeout ends as /// timed_out, not failure, and is just as much the reason the run failed.