diff --git a/Sources/App/Extensions/EnvironmentProperties.swift b/Sources/App/Extensions/EnvironmentProperties.swift index 79e8177..92c421a 100644 --- a/Sources/App/Extensions/EnvironmentProperties.swift +++ b/Sources/App/Extensions/EnvironmentProperties.swift @@ -19,6 +19,13 @@ extension Environment { return try Environment.get("GITHUB_REPOSITORY") } + static func requiresAllGitHubStatusChecks() throws -> Bool { + guard let stringValue: String = try? Environment.get("REQUIRES_ALL_STATUS_CHECKS") else { + return false // defaults to only consider required checks + } + return ["yes", "1", "true"].contains(stringValue.lowercased()) + } + 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 b5de858..af2fe96 100644 --- a/Sources/App/configure.swift +++ b/Sources/App/configure.swift @@ -42,6 +42,7 @@ private func makeMergeService(with logger: LoggerProtocol, _ gitHubEventsService return MergeService( integrationLabel: try Environment.mergeLabel(), topPriorityLabels: try Environment.topPriorityLabels(), + requiresAllStatusChecks: try Environment.requiresAllGitHubStatusChecks(), logger: logger, gitHubAPI: gitHubAPI, gitHubEvents: gitHubEventsService diff --git a/Sources/Bot/API/GitHubAPIProtocol.swift b/Sources/Bot/API/GitHubAPIProtocol.swift index 1e9e8b8..80cd8f0 100644 --- a/Sources/Bot/API/GitHubAPIProtocol.swift +++ b/Sources/Bot/API/GitHubAPIProtocol.swift @@ -10,6 +10,8 @@ public protocol GitHubAPIProtocol { func fetchCommitStatus(for pullRequest: PullRequest) -> SignalProducer + func fetchRequiredStatusChecks(for branch: PullRequest.Branch) -> SignalProducer + /// Merges one branch into another. /// /// - SeeAlso: https://developer.github.com/v3/repos/merging/ diff --git a/Sources/Bot/API/Repository.swift b/Sources/Bot/API/Repository.swift index 5742692..1a1feb6 100644 --- a/Sources/Bot/API/Repository.swift +++ b/Sources/Bot/API/Repository.swift @@ -51,6 +51,14 @@ extension Repository { ) } + func requiredStatusChecks(branch: PullRequest.Branch) -> Resource { + return Resource( + method: .GET, + path: path(for: "branches/\(branch.ref)/protection/required_status_checks"), + decoder: decode + ) + } + func deleteBranch(branch: PullRequest.Branch) -> Resource { return Resource( method: .DELETE, diff --git a/Sources/Bot/API/RepositoryAPI.swift b/Sources/Bot/API/RepositoryAPI.swift index 481fcda..4f9bc06 100644 --- a/Sources/Bot/API/RepositoryAPI.swift +++ b/Sources/Bot/API/RepositoryAPI.swift @@ -29,6 +29,11 @@ public struct RepositoryAPI: GitHubAPIProtocol { .mapError(AnyError.init) } + public func fetchRequiredStatusChecks(for branch: PullRequest.Branch) -> SignalProducer { + return client.request(repository.requiredStatusChecks(branch: branch)) + .mapError(AnyError.init) + } + public func merge(head: PullRequest.Branch, into base: PullRequest.Branch) -> SignalProducer { return client.request(repository.merge(head: head, into: base)) .mapError(AnyError.init) diff --git a/Sources/Bot/Models/CommitState.swift b/Sources/Bot/Models/CommitState.swift index f729031..2210dc0 100644 --- a/Sources/Bot/Models/CommitState.swift +++ b/Sources/Bot/Models/CommitState.swift @@ -14,6 +14,38 @@ public struct CommitState: Decodable, Equatable { public let context: String } + /// Combined state for _all_ the checks + /// - `.failure` if any of the status checks (`statuses.map(\.state)`) reports an error or `.failure` + /// - `.pending` if there are no statuses, or a status check is `.pending` + /// - `.success` if the latest status for all checks is `.success` + /// See https://developer.github.com/v3/repos/statuses/#get-the-combined-status-for-a-specific-ref public let state: State + + /// Individual status of each check public let statuses: [Status] } + + +extension CommitState.State { + + /// Compute the combined state for a set of states. + /// This logic replicates the behavior provided by the GitHub API (see `CommitState.state` property provided by GitHub API above) + /// + /// We use this method ourselves to: + /// - compute a similar combined state as GitHub API provides, but for only a subset of checks (especially only considering _required_ checks) + /// - create stubs for `CommitState` in the tests to replicate the GitHub API behavior + /// - Parameter states: List of states to compute the combined state for + /// - Returns + /// - `.failure` if any of the status checks reports a `.failure` + /// - `.pending` if there are no statuses, or a status check is `.pending` + /// - `.success` if the latest status for all checks is `.success` + static func combinedState(for states: [CommitState.State]) -> CommitState.State { + if states.contains(.failure) { + return .failure + } else if states.contains(.pending) { + return .pending + } else { + return .success + } + } +} diff --git a/Sources/Bot/Models/RequiredStatusCheck.swift b/Sources/Bot/Models/RequiredStatusCheck.swift new file mode 100644 index 0000000..5f5601c --- /dev/null +++ b/Sources/Bot/Models/RequiredStatusCheck.swift @@ -0,0 +1,13 @@ +import Foundation + +public struct RequiredStatusChecks: Decodable, Equatable { + /// Require branches to be up to date before merging? + public let isStrict: Bool + /// Names of status checks marked as Required + public let contexts: [String] + + enum CodingKeys: String, CodingKey { + case isStrict = "strict" + case contexts + } +} diff --git a/Sources/Bot/Services/MergeService.swift b/Sources/Bot/Services/MergeService.swift index f440bc8..7d16918 100644 --- a/Sources/Bot/Services/MergeService.swift +++ b/Sources/Bot/Services/MergeService.swift @@ -21,6 +21,7 @@ public final class MergeService { public init( integrationLabel: PullRequest.Label, topPriorityLabels: [PullRequest.Label], + requiresAllStatusChecks: Bool, statusChecksTimeout: TimeInterval = 60.minutes, logger: LoggerProtocol, gitHubAPI: GitHubAPIProtocol, @@ -43,8 +44,8 @@ public final class MergeService { feedbacks: [ Feedbacks.whenStarting(github: self.gitHubAPI, scheduler: scheduler), Feedbacks.whenReady(github: self.gitHubAPI, scheduler: scheduler), - Feedbacks.whenIntegrating(github: self.gitHubAPI, pullRequestChanges: pullRequestChanges, scheduler: scheduler), - Feedbacks.whenRunningStatusChecks(github: self.gitHubAPI, logger: logger, statusChecksCompletion: statusChecksCompletion, scheduler: scheduler), + Feedbacks.whenIntegrating(github: self.gitHubAPI, requiresAllStatusChecks: requiresAllStatusChecks, pullRequestChanges: pullRequestChanges, scheduler: scheduler), + Feedbacks.whenRunningStatusChecks(github: self.gitHubAPI, logger: logger, requiresAllStatusChecks: requiresAllStatusChecks, statusChecksCompletion: statusChecksCompletion, scheduler: scheduler), Feedbacks.whenIntegrationFailed(github: self.gitHubAPI, logger: logger, scheduler: scheduler), Feedbacks.pullRequestChanges(pullRequestChanges: pullRequestChanges, scheduler: scheduler), Feedbacks.whenAddingPullRequests(github: self.gitHubAPI, scheduler: scheduler) @@ -175,6 +176,7 @@ extension MergeService { fileprivate static func whenIntegrating( github: GitHubAPIProtocol, + requiresAllStatusChecks: Bool, pullRequestChanges: Signal<(PullRequestMetadata, PullRequest.Action), NoError>, scheduler: DateScheduler ) -> Feedback { @@ -190,7 +192,8 @@ extension MergeService { else { return .value(.integrationDidChangeStatus(.done, metadata)) } switch metadata.mergeState { - case .clean: + case .clean, + .unstable where !requiresAllStatusChecks: return github.mergePullRequest(metadata.reference) .flatMap(.latest) { () -> SignalProducer<(), NoError> in github.deleteBranch(named: metadata.reference.source) @@ -234,7 +237,7 @@ extension MergeService { case .pending: return .value(.integrationDidChangeStatus(.updating, metadata)) case .failure: - return .value(.integrationDidChangeStatus(.failed(.checkingCommitChecksFailed), metadata)) + return .value(.integrationDidChangeStatus(.failed(.checksFailing), metadata)) case .success: return github.fetchPullRequest(number: metadata.reference.number) .map { metadata in @@ -280,6 +283,7 @@ extension MergeService { fileprivate static func whenRunningStatusChecks( github: GitHubAPIProtocol, logger: LoggerProtocol, + requiresAllStatusChecks: Bool, statusChecksCompletion: Signal, scheduler: DateScheduler ) -> Feedback { @@ -315,13 +319,19 @@ extension MergeService { // window where all checks have passed but just until the next check is added and stars running. This // hopefully prevents those false positives by making sure we wait some time before checking if all // checks have passed - .debounce(60, on: scheduler) + .debounce(60.0, on: scheduler) .flatMap(.latest) { change in github.fetchPullRequest(number: pullRequest.number) .flatMap(.latest) { github.fetchCommitStatus(for: $0.reference).zip(with: .value($0)) } + .flatMap(.latest) { commitStatus, pullRequestMetadataRefreshed -> SignalProducer<(CommitState.State, PullRequestMetadata), AnyError> in + let requiredStateProducer = requiresAllStatusChecks + ? .value(commitStatus.state) + : getRequiredChecksState(github: github, targetBranch: pullRequest.target, commitState: commitStatus) + return requiredStateProducer.zip(with: .value(pullRequestMetadataRefreshed)) + } .flatMapError { _ in .empty } - .filterMap { commitStatus, pullRequestMetadataRefreshed in - switch commitStatus.state { + .filterMap { state, pullRequestMetadataRefreshed in + switch state { case .pending: return nil case .failure: @@ -402,6 +412,25 @@ extension MergeService { .map(Event.pullRequestDidChange) } } + + /// Returns the consolidated status of all _required_ checks only + /// i.e. returns .failure or .pending if one of the required check is in .failure or .pending respectively + /// and return .success only if all required states are .success + fileprivate static func getRequiredChecksState( + github: GitHubAPIProtocol, + targetBranch: PullRequest.Branch, + commitState: CommitState + ) -> SignalProducer { + if commitState.state == .success { + return .value(.success) + } + return github.fetchRequiredStatusChecks(for: targetBranch).map { (requiredStatusChecks) -> CommitState.State in + let requiredStates = requiredStatusChecks.contexts.map { requiredContext in + commitState.statuses.first(where: { $0.context == requiredContext })?.state ?? .pending + } + return CommitState.State.combinedState(for: requiredStates) + } + } } // MARK: - System types diff --git a/Tests/BotTests/Canned Data/GitHubPullRequest.swift b/Tests/BotTests/Canned Data/GitHubPullRequest.swift index 94fa9c5..eb07401 100644 --- a/Tests/BotTests/Canned Data/GitHubPullRequest.swift +++ b/Tests/BotTests/Canned Data/GitHubPullRequest.swift @@ -1,5 +1,3 @@ -import Foundation - let GitHubPullRequest = """ { "id": 1, @@ -429,4 +427,4 @@ let GitHubPullRequest = """ "changed_files": 5, "maintainer_can_modify": true } -""" \ No newline at end of file +""" diff --git a/Tests/BotTests/Canned Data/GitHubPullRequestEvent.swift b/Tests/BotTests/Canned Data/GitHubPullRequestEvent.swift index 15beafe..1247e58 100644 --- a/Tests/BotTests/Canned Data/GitHubPullRequestEvent.swift +++ b/Tests/BotTests/Canned Data/GitHubPullRequestEvent.swift @@ -1,5 +1,3 @@ -import Foundation - let GitHubPullRequestEvent: String = """ { "action": "closed", diff --git a/Tests/BotTests/Canned Data/GitHubRequiredStatusChecks.swift b/Tests/BotTests/Canned Data/GitHubRequiredStatusChecks.swift new file mode 100644 index 0000000..ee62684 --- /dev/null +++ b/Tests/BotTests/Canned Data/GitHubRequiredStatusChecks.swift @@ -0,0 +1,17 @@ +let GitHubRequiredStatusChecks: String = """ +{ + "url": "https://api.github.com/repos/babylonhealth/babylon-ios/branches/develop/protection/required_status_checks", + "strict": true, + "contexts": [ + "ci/circleci: Build: SDK", + "ci/circleci: UnitTests: Ascension", + "ci/circleci: UnitTests: BabylonKSA", + "ci/circleci: UnitTests: BabylonUS", + "ci/circleci: UnitTests: Telus", + "ci/circleci: SnapshotTests: BabylonChatBotUI", + "ci/circleci: SnapshotTests: BabylonUI", + "ci/circleci: SnapshotTests: Babylon" + ], + "contexts_url": "https://api.github.com/repos/babylonhealth/babylon-ios/branches/develop/protection/required_status_checks/contexts" +} +""" diff --git a/Tests/BotTests/Canned Data/GitHubStatusEvent.swift b/Tests/BotTests/Canned Data/GitHubStatusEvent.swift index 1eb1f7f..18eb96b 100644 --- a/Tests/BotTests/Canned Data/GitHubStatusEvent.swift +++ b/Tests/BotTests/Canned Data/GitHubStatusEvent.swift @@ -1,5 +1,3 @@ -import Foundation - let GitHubStatusEvent: String = """ { "id": 5018968172, diff --git a/Tests/BotTests/GitHub/GitHubAPITests.swift b/Tests/BotTests/GitHub/GitHubAPITests.swift index 0afb305..730cc2c 100644 --- a/Tests/BotTests/GitHub/GitHubAPITests.swift +++ b/Tests/BotTests/GitHub/GitHubAPITests.swift @@ -93,6 +93,41 @@ class GitHubAPITests: XCTestCase { } } + func test_fetch_required_status_checks() { + perform(stub: + Interceptor.load( + stubs: [ + Interceptor.Stub( + response: Interceptor.Stub.Response( + url: URL(string: "https://api.github.com/repos/babylonhealth/babylon-ios/branches/develop/protection/required_status_checks")!, + statusCode: 200, + body: GitHubRequiredStatusChecks.data(using: .utf8)! + ) + )] + ) + ) { client in + + let api = RepositoryAPI(client: client, repository: .init(owner: "golang", name: "go")) + + let result = api.fetchRequiredStatusChecks(for: target.target).first()?.value + + expect(result).toNot(beNil()) + expect(result) == RequiredStatusChecks( + isStrict: true, + contexts: [ + "ci/circleci: Build: SDK", + "ci/circleci: UnitTests: Ascension", + "ci/circleci: UnitTests: BabylonKSA", + "ci/circleci: UnitTests: BabylonUS", + "ci/circleci: UnitTests: Telus", + "ci/circleci: SnapshotTests: BabylonChatBotUI", + "ci/circleci: SnapshotTests: BabylonUI", + "ci/circleci: SnapshotTests: Babylon" + ] + ) + } + } + func test_delete_branch() { perform(stub: diff --git a/Tests/BotTests/MergeService/MergeServiceHealthcheckTests.swift b/Tests/BotTests/MergeService/MergeServiceHealthcheckTests.swift index 6b55472..d38fbd6 100644 --- a/Tests/BotTests/MergeService/MergeServiceHealthcheckTests.swift +++ b/Tests/BotTests/MergeService/MergeServiceHealthcheckTests.swift @@ -12,27 +12,27 @@ class MergeServiceHealthcheckTests: XCTestCase { when: { input, scheduler in scheduler.advance() - input.send(value: makeState(status: .starting)) + input.send(value: MergeService.State.stub(status: .starting)) scheduler.advance() - input.send(value: makeState(status: .idle)) + input.send(value: MergeService.State.stub(status: .idle)) scheduler.advance() - input.send(value: makeState(status: .ready)) + input.send(value: MergeService.State.stub(status: .ready)) scheduler.advance() - input.send(value: makeState(status: .integrating(MergeServiceFixture.defaultTarget))) + input.send(value: MergeService.State.stub(status: .integrating(MergeServiceFixture.defaultTarget))) scheduler.advance() - input.send(value: makeState(status: .ready)) + input.send(value: MergeService.State.stub(status: .ready)) scheduler.advance() - input.send(value: makeState(status: .idle)) + input.send(value: MergeService.State.stub(status: .idle)) scheduler.advance() }, @@ -49,28 +49,28 @@ class MergeServiceHealthcheckTests: XCTestCase { scheduler.advance() - input.send(value: makeState(status: .starting)) + input.send(value: MergeService.State.stub(status: .starting)) scheduler.advance() - input.send(value: makeState(status: .idle)) + input.send(value: MergeService.State.stub(status: .idle)) scheduler.advance() - input.send(value: makeState(status: .ready)) + input.send(value: MergeService.State.stub(status: .ready)) scheduler.advance() - input.send(value: makeState(status: .runningStatusChecks(MergeServiceFixture.defaultTarget))) + input.send(value: MergeService.State.stub(status: .runningStatusChecks(MergeServiceFixture.defaultTarget))) scheduler.advance(by: .minutes(2 * MergeServiceFixture.defaultStatusChecksTimeout)) - input.send(value: makeState(status: .integrationFailed(MergeServiceFixture.defaultTarget, .checksFailing))) + input.send(value: MergeService.State.stub(status: .integrationFailed(MergeServiceFixture.defaultTarget, .checksFailing))) scheduler.advance() - input.send(value: makeState(status: .ready)) - input.send(value: makeState(status: .idle)) + input.send(value: MergeService.State.stub(status: .ready)) + input.send(value: MergeService.State.stub(status: .idle)) scheduler.advance() diff --git a/Tests/BotTests/MergeService/MergeServiceTests.swift b/Tests/BotTests/MergeService/MergeServiceTests.swift index 7f04cb3..e1378a5 100644 --- a/Tests/BotTests/MergeService/MergeServiceTests.swift +++ b/Tests/BotTests/MergeService/MergeServiceTests.swift @@ -22,8 +22,8 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .idle) ] } ) @@ -43,8 +43,8 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .idle) ] } ) @@ -65,11 +65,11 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), + MergeService.State.stub(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -104,15 +104,15 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: pullRequests.map { $0.reference }), - makeState(status: .integrating(pullRequests[0]), pullRequests: pullRequests.map { $0.reference }.suffix(2).asArray), - makeState(status: .ready, pullRequests: pullRequests.map { $0.reference }.suffix(2).asArray), - makeState(status: .integrating(pullRequests[1]), pullRequests: pullRequests.map { $0.reference }.suffix(1).asArray), - makeState(status: .ready, pullRequests: pullRequests.map { $0.reference }.suffix(1).asArray), - makeState(status: .integrating(pullRequests[2])), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .ready, pullRequests: pullRequests.map { $0.reference }), + MergeService.State.stub(status: .integrating(pullRequests[0]), pullRequests: pullRequests.map { $0.reference }.suffix(2).asArray), + MergeService.State.stub(status: .ready, pullRequests: pullRequests.map { $0.reference }.suffix(2).asArray), + MergeService.State.stub(status: .integrating(pullRequests[1]), pullRequests: pullRequests.map { $0.reference }.suffix(1).asArray), + MergeService.State.stub(status: .ready, pullRequests: pullRequests.map { $0.reference }.suffix(1).asArray), + MergeService.State.stub(status: .integrating(pullRequests[2])), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -135,12 +135,12 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [target.reference]), - makeState(status: .integrating(target)), - makeState(status: .integrationFailed(target, .conflicts)), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .ready, pullRequests: [target.reference]), + MergeService.State.stub(status: .integrating(target)), + MergeService.State.stub(status: .integrationFailed(target, .conflicts)), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -154,7 +154,7 @@ class MergeServiceTests: XCTestCase { .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .clean) }, - .getCommitStatus { _ in CommitState(state: .success, statuses: []) }, + .getCommitStatus { _ in CommitState.stub(states: [.success]) }, .mergePullRequest { _ in }, .deleteBranch { _ in } ], @@ -168,27 +168,19 @@ class MergeServiceTests: XCTestCase { scheduler.advance() - service.eventsObserver.send(value: .status( - StatusEvent( - sha: "abcdef", - context: "", - description: "N/A", - state: .success, - branches: [.init(name: MergeServiceFixture.defaultBranch)] - ) - )) + service.sendStatusEvent(state: .success) scheduler.advance(by: .seconds(60)) }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget)), - makeState(status: .runningStatusChecks(MergeServiceFixture.defaultTarget.with(mergeState: .blocked))), - makeState(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + 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: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -200,7 +192,7 @@ class MergeServiceTests: XCTestCase { .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .blocked) }, .postComment { _, _ in }, - .getCommitStatus { _ in CommitState(state: .success, statuses: []) }, + .getCommitStatus { _ in CommitState.stub(states: [.success]) }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .clean) }, .mergePullRequest { _ in }, .deleteBranch { _ in } @@ -208,26 +200,18 @@ class MergeServiceTests: XCTestCase { when: { service, scheduler in scheduler.advance() - service.eventsObserver.send(value: .status( - StatusEvent( - sha: "abcdef", - context: "", - description: "N/A", - state: .success, - branches: [.init(name: MergeServiceFixture.defaultBranch)] - ) - )) + service.sendStatusEvent(state: .success) scheduler.advance() }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .blocked))), - makeState(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), + MergeService.State.stub(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .blocked))), + MergeService.State.stub(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -255,12 +239,12 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .idle), - makeState(status: .ready, pullRequests: [targetLabeled.reference]), - makeState(status: .integrating(targetLabeled)), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .idle), + MergeService.State.stub(status: .ready, pullRequests: [targetLabeled.reference]), + MergeService.State.stub(status: .integrating(targetLabeled)), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -282,7 +266,7 @@ class MergeServiceTests: XCTestCase { expect(pullRequest.number) == 2 }, .getPullRequest { _ in first.with(mergeState: .clean) }, - .getCommitStatus { _ in CommitState(state: .success, statuses: []) }, + .getCommitStatus { _ in CommitState.stub(states: [.success]) }, .mergePullRequest { _ in }, .deleteBranch { _ in }, .getPullRequest { _ in second }, @@ -305,30 +289,22 @@ class MergeServiceTests: XCTestCase { scheduler.advance() - service.eventsObserver.send(value: .status( - StatusEvent( - sha: "abcdef", - context: "", - description: "N/A", - state: .success, - branches: [.init(name: MergeServiceFixture.defaultBranch)] - ) - )) + service.sendStatusEvent(state: .success) scheduler.advance(by: .seconds(60)) }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [first.reference]), - makeState(status: .integrating(first)), - makeState(status: .runningStatusChecks(first.with(mergeState: .blocked))), - makeState(status: .runningStatusChecks(first.with(mergeState: .blocked)), pullRequests: [second.reference]), - makeState(status: .integrating(first.with(mergeState: .clean)), pullRequests: [second.reference]), - makeState(status: .ready, pullRequests: [second.reference]), - makeState(status: .integrating(second)), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .ready, pullRequests: [first.reference]), + MergeService.State.stub(status: .integrating(first)), + MergeService.State.stub(status: .runningStatusChecks(first.with(mergeState: .blocked))), + MergeService.State.stub(status: .runningStatusChecks(first.with(mergeState: .blocked)), pullRequests: [second.reference]), + MergeService.State.stub(status: .integrating(first.with(mergeState: .clean)), pullRequests: [second.reference]), + MergeService.State.stub(status: .ready, pullRequests: [second.reference]), + MergeService.State.stub(status: .integrating(second)), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -359,12 +335,12 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget)), - makeState(status: .runningStatusChecks(MergeServiceFixture.defaultTarget.with(mergeState: .blocked))), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + 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: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -395,12 +371,12 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget)), - makeState(status: .runningStatusChecks(MergeServiceFixture.defaultTarget.with(mergeState: .blocked))), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + 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: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -414,7 +390,8 @@ class MergeServiceTests: XCTestCase { .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .blocked) }, - .getCommitStatus { _ in CommitState(state: .failure, statuses: []) }, + .getCommitStatus { _ in CommitState.stub(states: [.failure]) }, + .getRequiredStatusChecks { _ in RequiredStatusChecks.stub(indices: [0]) }, .postComment { _, _ in }, .removeLabel { _, _ in } ], @@ -427,33 +404,27 @@ class MergeServiceTests: XCTestCase { scheduler.advance() - service.eventsObserver.send(value: .status( - StatusEvent( - sha: "abcdef", - context: "", - description: "N/A", - state: .failure, - branches: [.init(name: MergeServiceFixture.defaultBranch)] - ) - )) + service.sendStatusEvent(state: .failure) scheduler.advance(by: .seconds(60)) }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget)), - makeState(status: .runningStatusChecks(MergeServiceFixture.defaultTarget.with(mergeState: .blocked))), - makeState(status: .integrationFailed(MergeServiceFixture.defaultTarget.with(mergeState: .blocked), .checksFailing)), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + 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: .ready), + MergeService.State.stub(status: .idle) ] } ) } func test_pull_request_with_multiple_status_checks() { + let requiredStatusChecks = RequiredStatusChecks.stub(indices: [0,1,2,3,4]) + perform( stubs: [ .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, @@ -461,11 +432,13 @@ class MergeServiceTests: XCTestCase { .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .blocked) }, - .getCommitStatus { _ in CommitState(state: .pending, statuses: []) }, + .getCommitStatus { _ in CommitState.stub(states: [.pending]) }, + .getRequiredStatusChecks{ _ in requiredStatusChecks }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .blocked) }, - .getCommitStatus { _ in CommitState(state: .pending, statuses: []) }, + .getCommitStatus { _ in CommitState.stub(states: [.pending]) }, + .getRequiredStatusChecks{ _ in requiredStatusChecks }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .clean) }, - .getCommitStatus { _ in CommitState(state: .success, statuses: []) }, + .getCommitStatus { _ in CommitState.stub(states: [.success]) }, .mergePullRequest { _ in }, .deleteBranch { _ in } ], @@ -479,34 +452,135 @@ class MergeServiceTests: XCTestCase { scheduler.advance() for _ in 1...3 { - - service.eventsObserver.send(value: .status( - StatusEvent( - sha: "abcdef", - context: "", - description: "N/A", - state: .success, - branches: [StatusEvent.Branch(name: MergeServiceFixture.defaultBranch)] - ) - )) - + service.sendStatusEvent(state: .success) scheduler.advance(by: .seconds(60)) } }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget)), - makeState(status: .runningStatusChecks(MergeServiceFixture.defaultTarget.with(mergeState: .blocked))), - makeState(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + 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: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) + ] + } + ) + } + + func test_pull_request_with_non_required_failed_status_checks_requiresAllStatusChecks_off() { + let requiredStatusChecks = RequiredStatusChecks.stub(indices: [0,1,3]) + + perform( + requiresAllStatusChecks: false, + stubs: [ + .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getPullRequest { _ in MergeServiceFixture.defaultTarget }, + .postComment { _, _ in }, + .mergeIntoBranch { _, _ in .success }, + // 1 + .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .blocked) }, + .getCommitStatus { _ in CommitState.stub(states: [.pending, .pending, .pending]) }, + .getRequiredStatusChecks { _ in requiredStatusChecks }, + // 2 + .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .blocked) }, + .getCommitStatus { _ in CommitState.stub(states: [.success, .success, .pending, .pending, .pending]) }, + .getRequiredStatusChecks { _ in requiredStatusChecks }, + // 3 + .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .unstable) }, + .getCommitStatus { _ in CommitState.stub(states: [.success, .success, .pending, .success, .failure]) }, + .getRequiredStatusChecks { _ in requiredStatusChecks }, + // 4 + .mergePullRequest { _ in }, + .deleteBranch { _ in } + ], + when: { service, scheduler in + scheduler.advance() + + service.eventsObserver.send(value: .pullRequest( + .init(action: .synchronize, pullRequestMetadata: MergeServiceFixture.defaultTarget.with(mergeState: .blocked))) + ) + + scheduler.advance() // 1 + + scheduler.advance(by: .seconds(60)) // 2 + service.sendStatusEvent(index: 0, state: .success) + // service.sendStatusEvent(index: 1, state: .success) + scheduler.advance(by: .seconds(60)) // 3 + service.sendStatusEvent(index: 3, state: .success) + scheduler.advance(by: .seconds(60)) // 4 + service.sendStatusEvent(index: 2, state: .failure) + scheduler.advance(by: .seconds(60)) // 5 + }, + assert: { + expect($0) == [ + MergeService.State.stub(status: .starting), + 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: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .unstable))), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) } + func test_pull_request_with_non_required_failed_status_checks_requiresAllStatusChecks_on() { + perform( + requiresAllStatusChecks: true, + stubs: [ + .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getPullRequest { _ in MergeServiceFixture.defaultTarget }, + .postComment { _, _ in }, + .mergeIntoBranch { _, _ in .success }, + // 1 + .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .blocked) }, + .getCommitStatus { _ in CommitState.stub(states: [.pending, .pending, .pending]) }, + // 2 + .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .blocked) }, + .getCommitStatus { _ in CommitState.stub(states: [.success, .success, .pending, .pending, .pending]) }, + // 3 + .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .unstable) }, + .getCommitStatus { _ in CommitState.stub(states: [.success, .success, .pending, .success, .failure]) }, + // 4 + .postComment { _, _ in }, + .removeLabel { _, _ in } + ], + when: { service, scheduler in + scheduler.advance() + + service.eventsObserver.send(value: .pullRequest( + .init(action: .synchronize, pullRequestMetadata: MergeServiceFixture.defaultTarget.with(mergeState: .blocked))) + ) + + scheduler.advance() // 1 + + scheduler.advance(by: .seconds(60)) // 2 + service.sendStatusEvent(index: 0, state: .success) + // service.sendStatusEvent(index: 1, state: .success) + scheduler.advance(by: .seconds(60)) // 3 + service.sendStatusEvent(index: 3, state: .success) + scheduler.advance(by: .seconds(60)) // 4 + service.sendStatusEvent(index: 2, state: .failure) + scheduler.advance(by: .seconds(60)) // 5 + }, + assert: { + expect($0) == [ + MergeService.State.stub(status: .starting), + 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: .unstable), .checksFailing)), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) + ] + } + ) + } + func test_pull_request_fails_integration_after_timeout() { perform( @@ -530,13 +604,13 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget)), - makeState(status: .runningStatusChecks(MergeServiceFixture.defaultTarget.with(mergeState: .blocked))), - makeState(status: .integrationFailed(MergeServiceFixture.defaultTarget.with(mergeState: .blocked), .checksFailing)), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + 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: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -559,12 +633,12 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .unknown))), - makeState(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), + MergeService.State.stub(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .unknown))), + MergeService.State.stub(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -589,12 +663,12 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .unknown))), - makeState(status: .integrationFailed(MergeServiceFixture.defaultTarget.with(mergeState: .unknown), .unknown)), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), + MergeService.State.stub(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .unknown))), + MergeService.State.stub(status: .integrationFailed(MergeServiceFixture.defaultTarget.with(mergeState: .unknown), .unknown)), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -613,7 +687,7 @@ class MergeServiceTests: XCTestCase { .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, .getPullRequest { _ in first.with(mergeState: .clean) }, - .getCommitStatus { _ in CommitState(state: .success, statuses: []) }, + .getCommitStatus { _ in CommitState.stub(states: [.success]) }, .mergePullRequest { _ in }, .deleteBranch { _ in } ], @@ -633,28 +707,20 @@ class MergeServiceTests: XCTestCase { scheduler.advance() - service.eventsObserver.send(value: .status( - StatusEvent( - sha: "abcdef", - context: "", - description: "N/A", - state: .failure, - branches: [.init(name: MergeServiceFixture.defaultBranch)] - ) - )) + service.sendStatusEvent(state: .failure) scheduler.advance(by: .seconds(60)) }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [first, second].map { $0.reference}), - makeState(status: .integrating(first), pullRequests: [second.reference]), - makeState(status: .runningStatusChecks(first.with(mergeState: .blocked)), pullRequests: [second.reference]), - makeState(status: .runningStatusChecks(first.with(mergeState: .blocked))), - makeState(status: .integrating(first.with(mergeState: .clean))), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .ready, pullRequests: [first, second].map { $0.reference}), + MergeService.State.stub(status: .integrating(first), pullRequests: [second.reference]), + MergeService.State.stub(status: .runningStatusChecks(first.with(mergeState: .blocked)), pullRequests: [second.reference]), + MergeService.State.stub(status: .runningStatusChecks(first.with(mergeState: .blocked))), + MergeService.State.stub(status: .integrating(first.with(mergeState: .clean))), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -705,7 +771,7 @@ class MergeServiceTests: XCTestCase { expect(num) == 2 return pr2.with(mergeState: .clean) }, - .getCommitStatus { _ in CommitState(state: .success, statuses: []) }, + .getCommitStatus { _ in CommitState.stub(states: [.success]) }, .mergePullRequest { (pr: PullRequest) -> Void in expect(pr.number) == 2 }, .deleteBranch { _ in }, ] @@ -731,35 +797,27 @@ class MergeServiceTests: XCTestCase { scheduler.advance() // #3 - service.eventsObserver.send(value: .status( - StatusEvent( - sha: "abcdef", - context: "", - description: "N/A", - state: .success, - branches: [.init(name: pr2.reference.source.ref)] - ) - )) + service.sendStatusEvent(state: .success, branches: [.init(name: pr2.reference.source.ref)]) scheduler.advance(by: .seconds(60)) // #4 }, assert: { let pr3_tp = pr3.with(labels: [LabelFixture.integrationLabel, LabelFixture.topPriorityLabels[1]]) expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [pr2, pr1, pr3, pr4].map{$0.reference}), - makeState(status: .integrating(pr2), pullRequests: [pr1, pr3, pr4].map{$0.reference}), - makeState(status: .integrating(pr2), pullRequests: [pr3_tp, pr1, pr4].map{$0.reference}), - makeState(status: .runningStatusChecks(pr2.with(mergeState: .blocked)), pullRequests: [pr3_tp, pr1, pr4].map{$0.reference}), - makeState(status: .integrating(pr2.with(mergeState: .clean)), pullRequests: [pr3_tp, pr1, pr4].map{$0.reference}), - makeState(status: .ready, pullRequests: [pr3_tp, pr1, pr4].map{$0.reference}), - makeState(status: .integrating(pr3), pullRequests: [pr1, pr4].map{$0.reference}), - makeState(status: .ready, pullRequests: [pr1, pr4].map{$0.reference}), - makeState(status: .integrating(pr1), pullRequests: [pr4].map{$0.reference}), - makeState(status: .ready, pullRequests: [pr4].map{$0.reference}), - makeState(status: .integrating(pr4)), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .ready, pullRequests: [pr2, pr1, pr3, pr4].map{$0.reference}), + MergeService.State.stub(status: .integrating(pr2), pullRequests: [pr1, pr3, pr4].map{$0.reference}), + MergeService.State.stub(status: .integrating(pr2), pullRequests: [pr3_tp, pr1, pr4].map{$0.reference}), + MergeService.State.stub(status: .runningStatusChecks(pr2.with(mergeState: .blocked)), pullRequests: [pr3_tp, pr1, pr4].map{$0.reference}), + MergeService.State.stub(status: .integrating(pr2.with(mergeState: .clean)), pullRequests: [pr3_tp, pr1, pr4].map{$0.reference}), + MergeService.State.stub(status: .ready, pullRequests: [pr3_tp, pr1, pr4].map{$0.reference}), + MergeService.State.stub(status: .integrating(pr3), pullRequests: [pr1, pr4].map{$0.reference}), + MergeService.State.stub(status: .ready, pullRequests: [pr1, pr4].map{$0.reference}), + MergeService.State.stub(status: .integrating(pr1), pullRequests: [pr4].map{$0.reference}), + MergeService.State.stub(status: .ready, pullRequests: [pr4].map{$0.reference}), + MergeService.State.stub(status: .integrating(pr4)), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -805,15 +863,15 @@ class MergeServiceTests: XCTestCase { }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: pullRequests.map { $0.reference }), - makeState(status: .integrating(pullRequests[0]), pullRequests: pullRequests.map { $0.reference }.suffix(2).asArray), - makeState(status: .ready, pullRequests: pullRequests.map { $0.reference }.suffix(2).asArray), - makeState(status: .integrating(pullRequests[1]), pullRequests: pullRequests.map { $0.reference }.suffix(1).asArray), - makeState(status: .ready, pullRequests: pullRequests.map { $0.reference }.suffix(1).asArray), - makeState(status: .integrating(pullRequests[2])), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + MergeService.State.stub(status: .ready, pullRequests: pullRequests.map { $0.reference }), + MergeService.State.stub(status: .integrating(pullRequests[0]), pullRequests: pullRequests.map { $0.reference }.suffix(2).asArray), + MergeService.State.stub(status: .ready, pullRequests: pullRequests.map { $0.reference }.suffix(2).asArray), + MergeService.State.stub(status: .integrating(pullRequests[1]), pullRequests: pullRequests.map { $0.reference }.suffix(1).asArray), + MergeService.State.stub(status: .ready, pullRequests: pullRequests.map { $0.reference }.suffix(1).asArray), + MergeService.State.stub(status: .integrating(pullRequests[2])), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -822,7 +880,8 @@ class MergeServiceTests: XCTestCase { func test_pull_request_does_not_fail_prematurely_if_checks_complete_before_adding_the_following_checks() { var expectedPullRequest = MergeServiceFixture.defaultTarget.with(mergeState: .blocked) - var expectedCommitStatus = CommitState(state: .success, statuses: []) + var expectedCommitStatus = CommitState.stub(states: [.success]) + let expectedRequiredStatusChecks = RequiredStatusChecks.stub(indices: [0]) perform( stubs: [ @@ -832,6 +891,7 @@ class MergeServiceTests: XCTestCase { .mergeIntoBranch { _, _ in .success }, .getPullRequest { _ in expectedPullRequest }, .getCommitStatus { _ in expectedCommitStatus }, + .getRequiredStatusChecks { _ in expectedRequiredStatusChecks }, .getPullRequest { _ in expectedPullRequest }, .getCommitStatus { _ in expectedCommitStatus }, .mergePullRequest { _ in }, @@ -846,50 +906,34 @@ class MergeServiceTests: XCTestCase { scheduler.advance() - service.eventsObserver.send(value: .status( - StatusEvent( - sha: "abcdef", - context: "", - description: "N/A", - state: .success, - branches: [.init(name: MergeServiceFixture.defaultBranch)] - ) - )) + service.sendStatusEvent(state: .success) scheduler.advance(by: .seconds(30)) // Simulate a new check being added - expectedCommitStatus = CommitState(state: .pending, statuses: []) + expectedCommitStatus = CommitState.stub(states: [.pending, .success]) scheduler.advance(by: .seconds(30)) // Simulate all checks being successful - service.eventsObserver.send(value: .status( - StatusEvent( - sha: "abcdef", - context: "", - description: "N/A", - state: .success, - branches: [.init(name: MergeServiceFixture.defaultBranch)] - ) - )) + service.sendStatusEvent(state: .success) expectedPullRequest = MergeServiceFixture.defaultTarget.with(mergeState: .clean) - expectedCommitStatus = CommitState(state: .success, statuses: []) + expectedCommitStatus = CommitState.stub(states: [.success, .success]) scheduler.advance(by: .seconds(60)) }, assert: { expect($0) == [ - makeState(status: .starting), - makeState(status: .ready, pullRequests: [MergeServiceFixture.defaultTarget.reference]), - makeState(status: .integrating(MergeServiceFixture.defaultTarget)), - makeState(status: .runningStatusChecks(MergeServiceFixture.defaultTarget.with(mergeState: .blocked))), - makeState(status: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), - makeState(status: .ready), - makeState(status: .idle) + MergeService.State.stub(status: .starting), + 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: .integrating(MergeServiceFixture.defaultTarget.with(mergeState: .clean))), + MergeService.State.stub(status: .ready), + MergeService.State.stub(status: .idle) ] } ) @@ -906,9 +950,26 @@ class MergeServiceTests: XCTestCase { init() { (events, eventsObserver) = Signal.pipe() } + + func sendStatusEvent( + index: Int = 0, + state: StatusEvent.State, + branches: [StatusEvent.Branch] = [.init(name: MergeServiceFixture.defaultBranch)] + ) { + eventsObserver.send(value: .status( + StatusEvent( + sha: "abcdef", + context: CommitState.stubContextName(index), + description: "N/A", + state: state, + branches: branches + ) + )) + } } private func perform( + requiresAllStatusChecks: Bool = false, stubs: [MockGitHubAPI.Stubs], when: (MockGitHubEventsService, TestScheduler) -> Void, assert: ([MergeService.State]) -> Void @@ -921,6 +982,7 @@ class MergeServiceTests: XCTestCase { let service = MergeService( integrationLabel: LabelFixture.integrationLabel, topPriorityLabels: LabelFixture.topPriorityLabels, + requiresAllStatusChecks: requiresAllStatusChecks, statusChecksTimeout: MergeServiceFixture.defaultStatusChecksTimeout, logger: MockLogger(), gitHubAPI: gitHubAPI, diff --git a/Tests/BotTests/MergeService/Stubs/CommitState+Stub.swift b/Tests/BotTests/MergeService/Stubs/CommitState+Stub.swift new file mode 100644 index 0000000..0381a84 --- /dev/null +++ b/Tests/BotTests/MergeService/Stubs/CommitState+Stub.swift @@ -0,0 +1,26 @@ +@testable import Bot + +extension CommitState { + static func stub(states: [CommitState.State]) -> CommitState { + guard !states.isEmpty else { + return CommitState(state: .pending, statuses: []) + } + + return .init( + state: CommitState.State.combinedState(for: states), + statuses: states.enumerated().map { item in + CommitState.Status.stub(state: item.element, context: CommitState.stubContextName(item.offset)) + } + ) + } + + static func stubContextName(_ index: Int) -> String { + return "Check #\(index+1)" + } +} + +extension CommitState.Status { + static func stub(state: CommitState.State, context: String) -> CommitState.Status { + return .init(state: state, description: "\(context) is \(state)", context: context) + } +} diff --git a/Tests/BotTests/MergeService/Stubs/MergeService+Fixture.swift b/Tests/BotTests/MergeService/Stubs/MergeService+Stub.swift similarity index 56% rename from Tests/BotTests/MergeService/Stubs/MergeService+Fixture.swift rename to Tests/BotTests/MergeService/Stubs/MergeService+Stub.swift index 5775eeb..9db1957 100644 --- a/Tests/BotTests/MergeService/Stubs/MergeService+Fixture.swift +++ b/Tests/BotTests/MergeService/Stubs/MergeService+Stub.swift @@ -21,20 +21,22 @@ struct MergeServiceFixture { // MARK: - Helpers -func makeState( - status: MergeService.State.Status, - pullRequests: [PullRequest] = [], - integrationLabel: PullRequest.Label = LabelFixture.integrationLabel, - topPriorityLabels: [PullRequest.Label] = LabelFixture.topPriorityLabels, - statusChecksTimeout: TimeInterval = MergeServiceFixture.defaultStatusChecksTimeout -) -> MergeService.State { - return .init( - integrationLabel: integrationLabel, - topPriorityLabels: topPriorityLabels, - statusChecksTimeout: statusChecksTimeout, - pullRequests: pullRequests, - status: status - ) +extension MergeService.State { + static func stub( + status: MergeService.State.Status, + pullRequests: [PullRequest] = [], + integrationLabel: PullRequest.Label = LabelFixture.integrationLabel, + topPriorityLabels: [PullRequest.Label] = LabelFixture.topPriorityLabels, + statusChecksTimeout: TimeInterval = MergeServiceFixture.defaultStatusChecksTimeout + ) -> MergeService.State { + return .init( + integrationLabel: integrationLabel, + topPriorityLabels: topPriorityLabels, + statusChecksTimeout: statusChecksTimeout, + pullRequests: pullRequests, + status: status + ) + } } extension ArraySlice { diff --git a/Tests/BotTests/MergeService/Stubs/RequiredStatusChecks+Stub.swift b/Tests/BotTests/MergeService/Stubs/RequiredStatusChecks+Stub.swift new file mode 100644 index 0000000..2b61313 --- /dev/null +++ b/Tests/BotTests/MergeService/Stubs/RequiredStatusChecks+Stub.swift @@ -0,0 +1,17 @@ +@testable import Bot + +extension RequiredStatusChecks { + static func stub( + isStrict: Bool = true, + contexts: [String] + ) -> RequiredStatusChecks { + return .init(isStrict: isStrict, contexts: contexts) + } + + static func stub( + isStrict: Bool = true, + indices: [Int] + ) -> RequiredStatusChecks { + return .stub(isStrict: isStrict, contexts: indices.map(CommitState.stubContextName)) + } +} diff --git a/Tests/BotTests/Mocks/MockGitHubAPI.swift b/Tests/BotTests/Mocks/MockGitHubAPI.swift index 5eaae7a..0aab1b0 100644 --- a/Tests/BotTests/Mocks/MockGitHubAPI.swift +++ b/Tests/BotTests/Mocks/MockGitHubAPI.swift @@ -10,6 +10,7 @@ struct MockGitHubAPI: GitHubAPIProtocol { case getPullRequests(() -> [PullRequest]) case getPullRequest((UInt) -> PullRequestMetadata) case getCommitStatus((PullRequest) -> CommitState) + case getRequiredStatusChecks((PullRequest.Branch) -> RequiredStatusChecks) case mergePullRequest((PullRequest) -> Void) case mergeIntoBranch((PullRequest.Branch, PullRequest.Branch) -> MergeResult) case deleteBranch((PullRequest.Branch) -> Void) @@ -52,6 +53,15 @@ struct MockGitHubAPI: GitHubAPIProtocol { } } + func fetchRequiredStatusChecks(for branch: PullRequest.Branch) -> SignalProducer { + switch nextStub() { + case let .getRequiredStatusChecks(handler): + return SignalProducer(value: handler(branch)) + default: + fatalError("Stub not found") + } + } + func merge(head: PullRequest.Branch, into base: PullRequest.Branch) -> SignalProducer { switch nextStub() { case let .mergeIntoBranch(handler):