diff --git a/Sources/App/Extensions/EnvironmentProperties.swift b/Sources/App/Extensions/EnvironmentProperties.swift index 92c421a..39c4970 100644 --- a/Sources/App/Extensions/EnvironmentProperties.swift +++ b/Sources/App/Extensions/EnvironmentProperties.swift @@ -26,6 +26,11 @@ extension Environment { return ["yes", "1", "true"].contains(stringValue.lowercased()) } + static func statusChecksTimeout() throws -> TimeInterval? { + let value: String = try Environment.get("STATUS_CHECKS_TIMEOUT") + return TimeInterval(value) + } + static func mergeLabel() throws -> PullRequest.Label { return PullRequest.Label(name: try Environment.get("MERGE_LABEL")) } diff --git a/Sources/App/configure.swift b/Sources/App/configure.swift index af2fe96..fd38ed9 100644 --- a/Sources/App/configure.swift +++ b/Sources/App/configure.swift @@ -42,7 +42,8 @@ private func makeMergeService(with logger: LoggerProtocol, _ gitHubEventsService return MergeService( integrationLabel: try Environment.mergeLabel(), topPriorityLabels: try Environment.topPriorityLabels(), - requiresAllStatusChecks: try Environment.requiresAllGitHubStatusChecks(), + requiresAllStatusChecks: try Environment.requiresAllGitHubStatusChecks(), + statusChecksTimeout: try Environment.statusChecksTimeout() ?? 90.minutes, logger: logger, gitHubAPI: gitHubAPI, gitHubEvents: gitHubEventsService diff --git a/Sources/Bot/Services/MergeService.swift b/Sources/Bot/Services/MergeService.swift index 652bc3c..af67cc9 100644 --- a/Sources/Bot/Services/MergeService.swift +++ b/Sources/Bot/Services/MergeService.swift @@ -22,7 +22,7 @@ public final class MergeService { integrationLabel: PullRequest.Label, topPriorityLabels: [PullRequest.Label], requiresAllStatusChecks: Bool, - statusChecksTimeout: TimeInterval = 60.minutes, + statusChecksTimeout: TimeInterval, logger: LoggerProtocol, gitHubAPI: GitHubAPIProtocol, gitHubEvents: GitHubEventsServiceProtocol, @@ -316,7 +316,7 @@ extension MergeService { logger.log("📣 Status check `\(change.context)` finished with result: `\(change.state)` (SHA: `\(change.sha)`)") } // Checks can complete and lead to new checks which can be included posteriorly leading to a small time - // window where all checks have passed but just until the next check is added and stars running. This + // window where all checks have passed but just until the next check is added and starts running. This // hopefully prevents those false positives by making sure we wait some time before checking if all // checks have passed .debounce(60.0, on: scheduler) @@ -344,7 +344,7 @@ extension MergeService { .timeout(after: context.statusChecksTimeout, raising: TimeoutError.timedOut, on: scheduler) .flatMapError { error in switch error { - case .timedOut: return .value(.statusChecksDidComplete(.failed(context.pullRequestMetadata))) + case .timedOut: return .value(.statusChecksDidComplete(.timedOut(context.pullRequestMetadata))) } } } @@ -489,6 +489,7 @@ extension MergeService { case synchronizationFailed case checkingCommitChecksFailed case checksFailing + case timedOut case blocked case unknown } @@ -620,6 +621,7 @@ extension MergeService { enum StatusChecksResult { case failed(PullRequestMetadata) case passed(PullRequestMetadata) + case timedOut(PullRequestMetadata) } enum IntegrationStatus { @@ -690,6 +692,8 @@ extension MergeService.State { return self.with(status: .integrating(pullRequest)) case let .statusChecksDidComplete(.failed(pullRequest)): return self.with(status: .integrationFailed(pullRequest, .checksFailing)) + case let .statusChecksDidComplete(.timedOut(pullRequest)): + return self.with(status: .integrationFailed(pullRequest, .timedOut)) case let .pullRequestDidChange(.exclude(pullRequestExcluded)) where metadata.reference.number == pullRequestExcluded.number: return self.with(status: .ready) default: diff --git a/Tests/BotTests/MergeService/MergeServiceTests.swift b/Tests/BotTests/MergeService/MergeServiceTests.swift index e1378a5..c0daac1 100644 --- a/Tests/BotTests/MergeService/MergeServiceTests.swift +++ b/Tests/BotTests/MergeService/MergeServiceTests.swift @@ -589,7 +589,9 @@ class MergeServiceTests: XCTestCase { .getPullRequest { _ in MergeServiceFixture.defaultTarget }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, - .postComment { _, _ in }, + .postComment { comment, _ in + expect(comment) == "@John Doe unfortunately the integration failed with code: `timedOut`." + }, .removeLabel { _, _ in } ], when: { service, scheduler in @@ -608,7 +610,7 @@ class MergeServiceTests: XCTestCase { MergeService.State.stub(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), MergeService.State.stub(status: .integrating(MergeServiceFixture.defaultTarget)), MergeService.State.stub(status: .runningStatusChecks(MergeServiceFixture.defaultTarget.with(mergeState: .blocked))), - MergeService.State.stub(status: .integrationFailed(MergeServiceFixture.defaultTarget.with(mergeState: .blocked), .checksFailing)), + MergeService.State.stub(status: .integrationFailed(MergeServiceFixture.defaultTarget.with(mergeState: .blocked), .timedOut)), MergeService.State.stub(status: .ready), MergeService.State.stub(status: .idle) ] diff --git a/Tests/BotTests/XCTestManifests.swift b/Tests/BotTests/XCTestManifests.swift index 4e9a144..9fef1e2 100644 --- a/Tests/BotTests/XCTestManifests.swift +++ b/Tests/BotTests/XCTestManifests.swift @@ -10,6 +10,7 @@ extension GitHubAPITests { ("test_fetch_commit_status", test_fetch_commit_status), ("test_fetch_pull_request_number", test_fetch_pull_request_number), ("test_fetch_pull_requests", test_fetch_pull_requests), + ("test_fetch_required_status_checks", test_fetch_required_status_checks), ("test_merge_branch_up_to_date", test_merge_branch_up_to_date), ("test_merge_branch_with_success", test_merge_branch_with_success), ("test_merge_pull_request", test_merge_pull_request), @@ -73,6 +74,8 @@ extension MergeServiceTests { ("test_pull_request_with_integration_label_and_conflicts", test_pull_request_with_integration_label_and_conflicts), ("test_pull_request_with_integration_label_and_ready_to_merge", test_pull_request_with_integration_label_and_ready_to_merge), ("test_pull_request_with_multiple_status_checks", test_pull_request_with_multiple_status_checks), + ("test_pull_request_with_non_required_failed_status_checks_requiresAllStatusChecks_off", test_pull_request_with_non_required_failed_status_checks_requiresAllStatusChecks_off), + ("test_pull_request_with_non_required_failed_status_checks_requiresAllStatusChecks_on", test_pull_request_with_non_required_failed_status_checks_requiresAllStatusChecks_on), ("test_pull_request_with_status_checks_failing", test_pull_request_with_status_checks_failing), ("test_pull_requests_receive_feedback_when_accepted", test_pull_requests_receive_feedback_when_accepted), ("test_removing_the_integration_label_during_integration", test_removing_the_integration_label_during_integration),