diff --git a/.gitignore b/.gitignore index e071dc5..df2eaf4 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,6 @@ xcuserdata !WallEView.xcodeproj Config/secrets .DS_Store + +# Local files we don't want to commit, like scripts containing secrets +_local diff --git a/Sources/Bot/API/GitHubAPIProtocol.swift b/Sources/Bot/API/GitHubAPIProtocol.swift index 0bed092..cb1327e 100644 --- a/Sources/Bot/API/GitHubAPIProtocol.swift +++ b/Sources/Bot/API/GitHubAPIProtocol.swift @@ -29,5 +29,10 @@ public protocol GitHubAPIProtocol { func postComment(_ comment: String, in pullRequest: PullRequest) -> SignalProducer + /// Note: only fetches issue comments, not Pull Request review comments + func fetchIssueComments(in pullRequest: PullRequest) -> SignalProducer<[IssueComment], GitHubClient.Error> + func removeLabel(_ label: PullRequest.Label, from pullRequest: PullRequest) -> SignalProducer + + func fetchCurrentUser() -> SignalProducer } diff --git a/Sources/Bot/API/Repository.swift b/Sources/Bot/API/Repository.swift index deaa00f..32b6b5d 100644 --- a/Sources/Bot/API/Repository.swift +++ b/Sources/Bot/API/Repository.swift @@ -92,6 +92,14 @@ extension Repository { ) } + func issueComments(in pullRequest: PullRequest) -> Resource<[IssueComment]> { + return Resource( + method: .GET, + path: path(for: "issues/\(pullRequest.number)/comments"), + decoder: decode + ) + } + func merge(head: PullRequest.Branch, into base: PullRequest.Branch) -> Resource { return Resource( method: .POST, @@ -105,7 +113,7 @@ extension Repository { default: return .failure(.api(response)) } - } + } ) } @@ -118,6 +126,14 @@ extension Repository { ) } + var currentUser: Resource { + Resource( + method: .GET, + path: "user", + decoder: decode + ) + } + // TODO: This should be moved to be done as part of the handling of resources in `GitHubClient` private func encode(_ t: T) -> Data { guard let data = try? JSONEncoder().encode(t) diff --git a/Sources/Bot/API/RepositoryAPI.swift b/Sources/Bot/API/RepositoryAPI.swift index 0136543..cf3b157 100644 --- a/Sources/Bot/API/RepositoryAPI.swift +++ b/Sources/Bot/API/RepositoryAPI.swift @@ -49,9 +49,17 @@ public struct RepositoryAPI: GitHubAPIProtocol { return client.request(repository.publish(comment: comment, in: pullRequest)) } + public func fetchIssueComments(in pullRequest: PullRequest) -> SignalProducer<[IssueComment], GitHubClient.Error> { + return client.request(repository.issueComments(in: pullRequest)) + } + public func removeLabel(_ label: PullRequest.Label, from pullRequest: PullRequest) -> SignalProducer { return client.request(repository.removeLabel(label: label, from: pullRequest)) } + + public func fetchCurrentUser() -> SignalProducer { + return client.request(repository.currentUser) + } } extension GitHubClient { diff --git a/Sources/Bot/Models/GitHubUser.swift b/Sources/Bot/Models/GitHubUser.swift new file mode 100644 index 0000000..effcb34 --- /dev/null +++ b/Sources/Bot/Models/GitHubUser.swift @@ -0,0 +1,7 @@ +import Foundation + +public struct GitHubUser: Identifiable, Equatable, Decodable { + public let id: Int + let login: String + let name: String? +} diff --git a/Sources/Bot/Models/IssueComment.swift b/Sources/Bot/Models/IssueComment.swift new file mode 100644 index 0000000..a4a747f --- /dev/null +++ b/Sources/Bot/Models/IssueComment.swift @@ -0,0 +1,15 @@ +import Foundation + +public struct IssueComment: Equatable { + let user: GitHubUser + let body: String + let creationDate: Date +} + +extension IssueComment: Decodable { + private enum CodingKeys: String, CodingKey { + case user + case body + case creationDate = "created_at" + } +} diff --git a/Sources/Bot/Services/DispatchService.swift b/Sources/Bot/Services/DispatchService.swift index bf5ff54..d441f95 100644 --- a/Sources/Bot/Services/DispatchService.swift +++ b/Sources/Bot/Services/DispatchService.swift @@ -13,6 +13,7 @@ public final class DispatchService { private let logger: LoggerProtocol private let gitHubAPI: GitHubAPIProtocol private let scheduler: DateScheduler + private var botUser: GitHubUser? = nil /// Merge services per target branch private var mergeServices: Atomic<[String: MergeService]> @@ -43,10 +44,18 @@ public final class DispatchService { self.mergeServices = Atomic([:]) (mergeServiceLifecycle, mergeServiceLifecycleObserver) = Signal.pipe() - gitHubAPI.fetchPullRequests() - .flatMapError { _ in .value([]) } + gitHubAPI + .fetchCurrentUser() + .map(Optional.some) + .flatMapError { _ in .value(nil) } + .flatMap(.latest) { botUser in + gitHubAPI.fetchPullRequests() + .flatMapError { _ in .value([]) } + .map { (botUser, $0) } + } .observe(on: scheduler) - .startWithValues { pullRequests in + .startWithValues { (botUser, pullRequests) in + self.botUser = botUser self.dispatchInitial(pullRequests: pullRequests) } @@ -117,6 +126,7 @@ public final class DispatchService { requiresAllStatusChecks: requiresAllStatusChecks, statusChecksTimeout: statusChecksTimeout, initialPullRequests: initialPullRequests, + botUser: botUser, logger: logger, gitHubAPI: gitHubAPI, scheduler: scheduler diff --git a/Sources/Bot/Services/MergeService.swift b/Sources/Bot/Services/MergeService.swift index 21e814c..9f747ae 100644 --- a/Sources/Bot/Services/MergeService.swift +++ b/Sources/Bot/Services/MergeService.swift @@ -44,6 +44,7 @@ public final class MergeService { requiresAllStatusChecks: Bool, statusChecksTimeout: TimeInterval, initialPullRequests: [PullRequest] = [], + botUser: GitHubUser?, logger: LoggerProtocol, gitHubAPI: GitHubAPIProtocol, scheduler: DateScheduler = QueueScheduler() @@ -71,7 +72,7 @@ public final class MergeService { scheduler: scheduler, reduce: MergeService.reduce, feedbacks: [ - Feedbacks.whenStarting(initialPullRequests: pullRequestsReadyToInclude, scheduler: scheduler), + Feedbacks.whenStarting(initialPullRequests: pullRequestsReadyToInclude, gitHubAPI: gitHubAPI, botUser: botUser, scheduler: scheduler), Feedbacks.whenReady(github: self.gitHubAPI, 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), @@ -316,9 +317,23 @@ extension MergeService { // MARK: - Feedbacks +fileprivate let acceptedCommentTextIntro = "Your pull request was accepted" + extension MergeService { fileprivate typealias Feedbacks = MergeService + static func acceptedCommentText(index: Int?, queue: String, isBooting: Bool = false) -> String { + let rebootInfo = isBooting ? "WallE just started after a reboot.\n" : "" + let prefix = "\(rebootInfo)\(acceptedCommentTextIntro)" + if let index = index { + // use Zero-width-space character so that GitHub doesn't transform `#N` into a link to Pull Request N + let zws = "\u{200B}" + return "\(prefix) and it's currently #\(zws)\(index + 1) in the `\(queue)` queue, hold tight ⏳" + } else { + return "\(prefix) and is going to be handled right away 🏎 (nothing in queue for `\(queue)`!)" + } + } + fileprivate static func whenAddingPullRequests( github: GitHubAPIProtocol, scheduler: Scheduler @@ -332,19 +347,20 @@ extension MergeService { .enumerated() .map { index, pullRequest -> SignalProducer<(), Never> in - guard previous.pullRequests.firstIndex(of: pullRequest) == nil + guard previous.pullRequests.contains(pullRequest) == false else { return .empty } + let isBooting = previous.status == .starting + if index == 0 && current.isIntegrationOngoing == false { return github.postComment( - "Your pull request was accepted and is going to be handled right away 🏎", + acceptedCommentText(index: nil, queue: current.targetBranch, isBooting: isBooting), in: pullRequest ) .flatMapError { _ in .empty } } else { - let zws = "\u{200B}" // Zero-width space character. Used so that GitHub doesn't transform `#n` into a link to Pull Request n return github.postComment( - "Your pull request was accepted and it's currently #\(zws)\(index + 1) in the `\(current.targetBranch)` queue, hold tight ⏳", + acceptedCommentText(index: index, queue: current.targetBranch, isBooting: isBooting), in: pullRequest ) .flatMapError { _ in .empty } @@ -356,11 +372,43 @@ extension MergeService { }) } - fileprivate static func whenStarting(initialPullRequests: [PullRequest], scheduler: DateScheduler) -> Feedback { + fileprivate static func whenStarting( + initialPullRequests: [PullRequest], + gitHubAPI: GitHubAPIProtocol, + botUser: GitHubUser?, + scheduler: DateScheduler + ) -> Feedback { + typealias DatedPullRequest = (pr: PullRequest, date: Date) + + func isBotMergeComment(_ comment: IssueComment) -> Bool { + return (botUser.map({ comment.user.id == $0.id }) ?? true) // If botUser nil, default to true + && comment.body.contains(acceptedCommentTextIntro) + } + + func datedPullRequest(for pullRequest: PullRequest) -> SignalProducer { + return gitHubAPI.fetchIssueComments(in: pullRequest) + .flatMapError { _ in .value([]) } + .map { comments in + let lastBotCommentDate = comments + .filter(isBotMergeComment) + .map { $0.creationDate } + .max() + return (pullRequest, lastBotCommentDate ?? .distantFuture) + } + } + + func orderByBotCommentDate(_ datedPRs: [DatedPullRequest]) -> [PullRequest] { + return datedPRs.sorted(by: { $0.date < $1.date }).map { $0.pr } + } + return Feedback(predicate: { $0.status == .starting }) { state -> SignalProducer in - return SignalProducer - .value(Event.pullRequestsLoaded(initialPullRequests)) - .observe(on: scheduler) + return SignalProducer.merge( + initialPullRequests.map(datedPullRequest(for:)) + ) + .collect() + .map(orderByBotCommentDate) + .map(Event.pullRequestsLoaded) + .observe(on: scheduler) } } diff --git a/Tests/BotTests/Canned Data/GitHubIssueComment.swift b/Tests/BotTests/Canned Data/GitHubIssueComment.swift new file mode 100644 index 0000000..5312770 --- /dev/null +++ b/Tests/BotTests/Canned Data/GitHubIssueComment.swift @@ -0,0 +1,52 @@ +let GitHubIssueComment = #""" +{ + "url": "https://api.github.com/repos/babylonhealth/Wall-E/pulls/comments/400979441", + "pull_request_review_id": 384816651, + "id": 400979441, + "node_id": "MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDQwMDk3OTQ0MQ==", + "diff_hunk": "@@ -0,0 +1,34 @@\n+import Foundation\n+\n+public struct IssueComment: Equatable {\n+ let user: GitHubUser\n+ let body: String\n+ let creationDate: Date\n+}\n+\n+extension IssueComment: Decodable {\n+ private static let dateFormatter: DateFormatter = {\n+ // 2020-03-30T11:26:18\n+ let df = DateFormatter()\n+ df.locale = Locale(identifier: \"en_US_POSIX\")\n+ df.dateFormat = \"yyyy-MM-dd'T'HH:mm:ssZ\"\n+ return df\n+ }()\n+\n+ private enum CodingKeys: String, CodingKey {\n+ case user\n+ case body\n+ case created_at\n+ }\n+\n+ public init(from decoder: Decoder) throws {\n+ let container = try decoder.container(keyedBy: CodingKeys.self)\n+ self.user = try container.decode(GitHubUser.self, forKey: .user)\n+ self.body = try container.decode(String.self, forKey: .body)\n+ let dateString = try container.decode(String.self, forKey: .created_at)\n+ guard let date = Self.dateFormatter.date(from: dateString) else {", + "path": "Sources/Bot/Models/IssueComment.swift", + "position": 29, + "original_position": 29, + "commit_id": "8b6a72d20ab8f5cb6cd6d6bf9300bb14fdb9e681", + "original_commit_id": "8b6a72d20ab8f5cb6cd6d6bf9300bb14fdb9e681", + "user": { + "login": "AliSoftware", + "id": 216089, + "node_id": "MDQ6VXNlcjIxNjA4OQ==", + "avatar_url": "https://avatars2.githubusercontent.com/u/216089?v=4", + "gravatar_id": "", + "url": "https://api.github.com/users/AliSoftware", + "html_url": "https://github.com/AliSoftware", + "followers_url": "https://api.github.com/users/AliSoftware/followers", + "following_url": "https://api.github.com/users/AliSoftware/following{/other_user}", + "gists_url": "https://api.github.com/users/AliSoftware/gists{/gist_id}", + "starred_url": "https://api.github.com/users/AliSoftware/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/AliSoftware/subscriptions", + "organizations_url": "https://api.github.com/users/AliSoftware/orgs", + "repos_url": "https://api.github.com/users/AliSoftware/repos", + "events_url": "https://api.github.com/users/AliSoftware/events{/privacy}", + "received_events_url": "https://api.github.com/users/AliSoftware/received_events", + "type": "User", + "site_admin": false + }, + "body": "Oh forgot about that one, good point.\r\nAnd just saw that there's actually already code for that in `Decoding.swift` so all that shouldn't even be needed at all indeed. ", + "created_at": "2020-03-31T14:54:28Z", + "updated_at": "2020-03-31T14:54:28Z", + "html_url": "https://github.com/babylonhealth/Wall-E/pull/57#discussion_r400979441", + "pull_request_url": "https://api.github.com/repos/babylonhealth/Wall-E/pulls/57", + "author_association": "CONTRIBUTOR", + "_links": { + "self": { + "href": "https://api.github.com/repos/babylonhealth/Wall-E/pulls/comments/400979441" + }, + "html": { + "href": "https://github.com/babylonhealth/Wall-E/pull/57#discussion_r400979441" + }, + "pull_request": { + "href": "https://api.github.com/repos/babylonhealth/Wall-E/pulls/57" + } + }, + "in_reply_to_id": 400977181 +} +"""# diff --git a/Tests/BotTests/GitHub/GitHubDecodingTests.swift b/Tests/BotTests/GitHub/GitHubDecodingTests.swift index fd6ae80..be40a69 100644 --- a/Tests/BotTests/GitHub/GitHubDecodingTests.swift +++ b/Tests/BotTests/GitHub/GitHubDecodingTests.swift @@ -60,5 +60,15 @@ class GitHubDecodingTests: XCTestCase { fail("Could not parse a pull request with error: \(error)") } } + + func test_parsing_issue_comment() throws { + let response = Bot.Response(statusCode: 200, headers: [:], body: GitHubIssueComment.data(using: .utf8)!) + let commentResult: Result = Bot.decode(response) + let comment = try commentResult.get() + expect(comment.user.id) == 216089 + expect(comment.user.login) == "AliSoftware" + expect(comment.body) == "Oh forgot about that one, good point.\r\nAnd just saw that there's actually already code for that in `Decoding.swift` so all that shouldn't even be needed at all indeed. " + expect(comment.creationDate.timeIntervalSince1970) == 1585666468 + } } diff --git a/Tests/BotTests/MergeService/DispatchServiceTests.swift b/Tests/BotTests/MergeService/DispatchServiceTests.swift index 02ddb19..95d51e1 100644 --- a/Tests/BotTests/MergeService/DispatchServiceTests.swift +++ b/Tests/BotTests/MergeService/DispatchServiceTests.swift @@ -16,7 +16,11 @@ class DispatchServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { pullRequests.map { $0.reference } }, + .getIssueComments { _ in [] }, + .getIssueComments { _ in [] }, + .getIssueComments { _ in [] }, .getPullRequest(returnPR()), .postComment { _, _ in }, .getPullRequest(returnPR()), @@ -62,18 +66,20 @@ class DispatchServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [dev1.reference] }, + .getIssueComments { _ in [] }, .getPullRequest(checkReturnPR(dev1)), - .postComment(checkComment(1, "Your pull request was accepted and is going to be handled right away 🏎")), + .postComment(checkComment(1, MergeService.acceptedCommentText(index: nil, queue: "develop", isBooting: true))), .mergeIntoBranch { head, base in expect(head) == dev1.reference.source expect(base) == dev1.reference.target return .success }, - .postComment(checkComment(2, "Your pull request was accepted and it's currently #\u{200B}1 in the `develop` queue, hold tight ⏳")), + .postComment(checkComment(2, MergeService.acceptedCommentText(index: 0, queue: "develop"))), .getPullRequest(checkReturnPR(rel3)), - .postComment(checkComment(3, "Your pull request was accepted and is going to be handled right away 🏎")), + .postComment(checkComment(3, MergeService.acceptedCommentText(index: nil, queue: "release/app/1.2.3"))), .mergePullRequest(checkPRNumber(3)), .deleteBranch(checkBranch(rel3.reference.source)), @@ -149,16 +155,18 @@ class DispatchServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [dev1.reference] }, + .getIssueComments { _ in [] }, .getPullRequest(checkReturnPR(dev1)), - .postComment(checkComment(1, "Your pull request was accepted and is going to be handled right away 🏎")), + .postComment(checkComment(1, MergeService.acceptedCommentText(index: nil, queue: "develop", isBooting: true))), .mergeIntoBranch { head, base in expect(head.ref) == MergeServiceFixture.defaultBranch expect(base.ref) == developBranch return .success }, - .postComment(checkComment(2, "Your pull request was accepted and it's currently #\u{200B}1 in the `develop` queue, hold tight ⏳")), + .postComment(checkComment(2, MergeService.acceptedCommentText(index: 0, queue: "develop"))), // Note that here we shouldn't have any API call for PR#3 since it doesn't have the integration label @@ -239,8 +247,9 @@ class DispatchServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [prs[0].reference] }, - + .getIssueComments { _ in [] }, .getPullRequest { _ in prs[0] }, .postComment { _, _ in }, .mergePullRequest { _ in }, @@ -325,11 +334,12 @@ class DispatchServiceTests: XCTestCase { } func test_mergeservice_destroyed_when_idle_after_boot() { - let pr = PullRequestMetadata.stub(number: 1) + let pr = PullRequestMetadata.stub(number: 1) // No Merge label, so should get filtered out let branch = pr.reference.target.ref perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [pr.reference] }, ], when: { service, scheduler in @@ -352,13 +362,15 @@ class DispatchServiceTests: XCTestCase { let pr3 = PullRequestMetadata.stub(number: 3, baseRef: branch2, labels: [LabelFixture.integrationLabel], mergeState: .behind) let stubs: [MockGitHubAPI.Stubs] = [ + .getCurrentUser { nil }, .getPullRequests { [pr1.reference] }, + .getIssueComments { _ in [] }, .getPullRequest(checkReturnPR(pr1)), - .postComment(checkComment(1, "Your pull request was accepted and is going to be handled right away 🏎")), + .postComment(checkComment(1, MergeService.acceptedCommentText(index: nil, queue: "branch1", isBooting: true))), .mergeIntoBranch { _, _ in .success }, - .postComment(checkComment(2, "Your pull request was accepted and it's currently #\u{200B}1 in the `branch1` queue, hold tight ⏳")), + .postComment(checkComment(2, MergeService.acceptedCommentText(index: 0, queue: "branch1"))), .getPullRequest(checkReturnPR(pr3)), - .postComment(checkComment(3, "Your pull request was accepted and is going to be handled right away 🏎")), + .postComment(checkComment(3, MergeService.acceptedCommentText(index: nil, queue: "branch2"))), .mergeIntoBranch { _, _ in .success }, ] diff --git a/Tests/BotTests/MergeService/MergeServiceTests.swift b/Tests/BotTests/MergeService/MergeServiceTests.swift index 6c34986..683952a 100644 --- a/Tests/BotTests/MergeService/MergeServiceTests.swift +++ b/Tests/BotTests/MergeService/MergeServiceTests.swift @@ -14,6 +14,7 @@ class MergeServiceTests: XCTestCase { func test_empty_list_of_pull_requests_should_do_nothing() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [] } ], when: { _, scheduler in @@ -28,6 +29,7 @@ class MergeServiceTests: XCTestCase { func test_no_pull_requests_with_integration_label() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [ PullRequestMetadata.stub(number: 1).reference, PullRequestMetadata.stub(number: 2).reference @@ -49,9 +51,131 @@ class MergeServiceTests: XCTestCase { } + func test_order_pull_requests_on_boot() { + let botUser = GitHubUser(id: 123, login: "our-bot", name: "WallEBot") + let devUser = GitHubUser(id: 456, login: "bnl-dev", name: "BuyNLarge") + + let pullRequests = (0...2).map { + PullRequestMetadata.stub(number: $0+10, labels: [LabelFixture.integrationLabel]) + .with(mergeState: .clean) + } + let orderedPRs = [pullRequests[2], pullRequests[0], pullRequests[1]] + + func mockComments(pullRequest: PullRequest) -> [IssueComment] { + func makeDate(year: Int, month: Int, day: Int) -> Date { + return DateComponents( + calendar: Calendar(identifier: .gregorian), + timeZone: TimeZone(secondsFromGMT: 0), + year: year, month: month, day: day, + hour: 12, minute: 30, second: 0 + ).date! + } + + func makeDevComment(year: Int = 2020, month: Int, day: Int) -> IssueComment { + return IssueComment( + user: devUser, + body: "The year is \(year). I'm commenting on day \(day) of month \(month).", + creationDate: makeDate(year: year, month: month, day: day) + ) + } + + func makeBotComment(year: Int = 2020, month: Int, day: Int) -> IssueComment { + return IssueComment( + user: botUser, + body: MergeService.acceptedCommentText(index: 2, queue: "somequeue", isBooting: true), + creationDate: makeDate(year: year, month: month, day: day) + ) + } + + switch pullRequest.number { + case 10: + // With last bot comment in March --> ordered second + return [ + makeBotComment(month: 1, day: 1), // 🤖 -- it's a trap, comment in 1 Jan but not the last + makeDevComment(month: 1, day: 2), // 👨‍💻 + makeBotComment(month: 3, day: 3), // 🤖 -- last bot comment = 3 Mar + makeDevComment(month: 3, day: 4) // 👩‍💻 + ] + case 11: + // Without any bot comment --> ordered last + return [ + makeDevComment(month: 2, day: 1), // 👨‍💻 + makeDevComment(month: 2, day: 2) // 👩‍💻 + ] + case 12: + // With last bot comment in Jan --> ordered first + return [ + makeBotComment(month: 1, day: 10), // 🤖 -- it's a trap, comment after the first one for PR#10 but not the last + makeDevComment(month: 1, day: 11), // 👩‍💻 + makeBotComment(month: 1, day: 12), // 🤖 -- last bot comment = 12 Jan + makeDevComment(month: 1, day: 13) // 👨‍💻 + ] + default: + fatalError("Unexpected PullRequest number") + } + } + + func expectPR(_ expectedNum: UInt) -> (UInt) -> PullRequestMetadata { + return { requestedNum in + expect(requestedNum) == expectedNum + guard let pr = pullRequests.first(where: { $0.reference.number == requestedNum }) else { + fatalError("Requested PR #\(requestedNum) which doesn't exist in this test") + } + return pr + } + } + + func expectComment(_ expectedIndex: Int?, _ expectedPRNum: UInt) -> (String, PullRequest) -> Void { + return { postedMessage, pullRequest in + expect(postedMessage) == MergeService.acceptedCommentText(index: expectedIndex, queue: "master", isBooting: true) + expect(pullRequest.number) == expectedPRNum + } + } + + perform( + stubs: [ + .getCurrentUser { botUser }, + .getPullRequests { pullRequests.map { $0.reference } }, + .getIssueComments(mockComments), + .getIssueComments(mockComments), + .getIssueComments(mockComments), + .getPullRequest(expectPR(12)), + .postComment(expectComment(nil, 12)), + .postComment(expectComment(1, 10)), + .postComment(expectComment(2, 11)), + .mergePullRequest { _ in }, + .deleteBranch { _ in }, + .getPullRequest(expectPR(10)), + .mergePullRequest { _ in }, + .deleteBranch { _ in }, + .getPullRequest(expectPR(11)), + .mergePullRequest { _ in }, + .deleteBranch { _ in } + ], + when: { service, scheduler in + scheduler.advance() + }, + assert: { + expect($0) == [ + .created(branch: MergeServiceFixture.defaultTargetBranch), + .state(.stub(status: .starting)), + .state(.stub(status: .ready, pullRequests: orderedPRs.map { $0.reference })), + .state(.stub(status: .integrating(orderedPRs[0]), pullRequests: orderedPRs.map { $0.reference }.suffix(2).asArray)), + .state(.stub(status: .ready, pullRequests: orderedPRs.map { $0.reference }.suffix(2).asArray)), + .state(.stub(status: .integrating(orderedPRs[1]), pullRequests: orderedPRs.map { $0.reference }.suffix(1).asArray)), + .state(.stub(status: .ready, pullRequests: orderedPRs.map { $0.reference }.suffix(1).asArray)), + .state(.stub(status: .integrating(orderedPRs[2]))), + .state(.stub(status: .ready)), + .state(.stub(status: .idle)) + ] + } + ) + } + func test_pull_request_not_included_on_close() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [] }, ], when: { service, scheduler in @@ -77,7 +201,9 @@ class MergeServiceTests: XCTestCase { func test_pull_request_with_integration_label_and_ready_to_merge() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .clean) }, .postComment { _, _ in }, .mergePullRequest { _ in }, @@ -109,7 +235,11 @@ class MergeServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { pullRequests.map { $0.reference } }, + .getIssueComments { _ in [] }, + .getIssueComments { _ in [] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in pullRequests[0] }, .postComment { _, _ in }, .postComment { _, _ in }, @@ -149,7 +279,9 @@ class MergeServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [target.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in target }, .postComment { _, _ in }, .postComment { _, _ in }, @@ -175,7 +307,9 @@ class MergeServiceTests: XCTestCase { func test_pull_request_with_integration_label_and_behind_target_branch() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, @@ -214,7 +348,9 @@ class MergeServiceTests: XCTestCase { func test_pull_request_blocked_with_successful_status_no_pending_checks() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .blocked) }, .postComment { _, _ in }, .getAllStatusChecks { _ in [.init(state: .success, context: "", description: "")]}, @@ -247,7 +383,9 @@ class MergeServiceTests: XCTestCase { func test_pull_request_blocked_with_successful_status_pending_checks() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .blocked) }, .postComment { _, _ in }, .getAllStatusChecks { _ in @@ -290,7 +428,9 @@ class MergeServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [target.reference] }, + // target.reference is not labelled with Merge label, so we're not fetching issue comments on whenStarting :) .getPullRequest { _ in targetLabeled }, .postComment { _, _ in }, .mergePullRequest { _ in }, @@ -328,12 +468,14 @@ class MergeServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [first.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in first }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, .postComment { message, pullRequest in - expect(message) == "Your pull request was accepted and it's currently #\u{200B}1 in the `master` queue, hold tight ⏳" + expect(message) == MergeService.acceptedCommentText(index: 0, queue: "master") expect(pullRequest.number) == 2 }, .getPullRequest { _ in first.with(mergeState: .clean) }, @@ -384,7 +526,9 @@ class MergeServiceTests: XCTestCase { func test_closing_pull_request_during_integration() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success } @@ -420,7 +564,9 @@ class MergeServiceTests: XCTestCase { func test_removing_the_integration_label_during_integration() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success } @@ -453,7 +599,9 @@ class MergeServiceTests: XCTestCase { func test_pull_request_with_status_checks_failing() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, @@ -494,7 +642,9 @@ class MergeServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, @@ -542,7 +692,9 @@ class MergeServiceTests: XCTestCase { perform( requiresAllStatusChecks: false, stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, @@ -597,7 +749,9 @@ class MergeServiceTests: XCTestCase { perform( requiresAllStatusChecks: true, stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, @@ -649,7 +803,9 @@ class MergeServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, @@ -688,7 +844,9 @@ class MergeServiceTests: XCTestCase { func test_pull_request_with_an_initial_unknown_state_with_recover() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .unknown) }, .postComment { _, _ in }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .unknown) }, @@ -719,7 +877,9 @@ class MergeServiceTests: XCTestCase { func test_pull_request_with_an_initial_unknown_state_without_recover() { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .unknown) }, .postComment { _, _ in }, .getPullRequest { _ in MergeServiceFixture.defaultTarget.with(mergeState: .unknown) }, @@ -756,7 +916,10 @@ class MergeServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [first, second].map { $0.reference} }, + .getIssueComments { _ in [] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in first }, .postComment { _, _ in }, .postComment { _, _ in }, @@ -827,7 +990,12 @@ class MergeServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { Array(allPRs).map { $0.reference} }, + .getIssueComments { _ in [] }, + .getIssueComments { _ in [] }, + .getIssueComments { _ in [] }, + .getIssueComments { _ in [] }, // advance() #1 .getPullRequest { (num: UInt) -> PullRequestMetadata in expect(num) == 2 @@ -907,18 +1075,22 @@ class MergeServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { pullRequests.map { $0.reference } }, + .getIssueComments { _ in [] }, + .getIssueComments { _ in [] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in pullRequests[0] }, .postComment { message, pullRequest in - expect(message) == "Your pull request was accepted and is going to be handled right away 🏎" + expect(message) == MergeService.acceptedCommentText(index: nil, queue: "master", isBooting: true) expect(pullRequest.number) == 144 }, .postComment { message, pullRequest in - expect(message) == "Your pull request was accepted and it's currently #\u{200B}2 in the `master` queue, hold tight ⏳" + expect(message) == MergeService.acceptedCommentText(index: 1, queue: "master", isBooting: true) expect(pullRequest.number) == 233 }, .postComment { message, pullRequest in - expect(message) == "Your pull request was accepted and it's currently #\u{200B}3 in the `master` queue, hold tight ⏳" + expect(message) == MergeService.acceptedCommentText(index: 2, queue: "master", isBooting: true) expect(pullRequest.number) == 377 }, .mergePullRequest { _ in }, @@ -958,7 +1130,9 @@ class MergeServiceTests: XCTestCase { perform( stubs: [ + .getCurrentUser { nil }, .getPullRequests { [MergeServiceFixture.defaultTarget.reference] }, + .getIssueComments { _ in [] }, .getPullRequest { _ in MergeServiceFixture.defaultTarget }, .postComment { _, _ in }, .mergeIntoBranch { _, _ in .success }, diff --git a/Tests/BotTests/Mocks/MockGitHubAPI.swift b/Tests/BotTests/Mocks/MockGitHubAPI.swift index e00c4f0..102f32e 100644 --- a/Tests/BotTests/Mocks/MockGitHubAPI.swift +++ b/Tests/BotTests/Mocks/MockGitHubAPI.swift @@ -14,7 +14,9 @@ struct MockGitHubAPI: GitHubAPIProtocol { case mergeIntoBranch((PullRequest.Branch, PullRequest.Branch) -> MergeResult) case deleteBranch((PullRequest.Branch) -> Void) case postComment((String, PullRequest) -> Void) + case getIssueComments((PullRequest) -> [IssueComment]) case removeLabel((PullRequest.Label, PullRequest) -> Void) + case getCurrentUser(() -> GitHubUser?) } let stubs: [Stubs] @@ -106,6 +108,15 @@ struct MockGitHubAPI: GitHubAPIProtocol { } } + func fetchIssueComments(in pullRequest: PullRequest) -> SignalProducer<[IssueComment], GitHubClient.Error> { + switch nextStub() { + case let .getIssueComments(handler): + return SignalProducer(value: handler(pullRequest)) + default: + fatalError("Stub not found") + } + } + func removeLabel(_ label: PullRequest.Label, from pullRequest: PullRequest) -> SignalProducer<(), GitHubClient.Error> { switch nextStub() { case let .removeLabel(handler): @@ -115,6 +126,22 @@ struct MockGitHubAPI: GitHubAPIProtocol { } } + func fetchCurrentUser() -> SignalProducer { + switch nextStub() { + case let .getCurrentUser(handler): + if let user = handler() { + return SignalProducer(value: user) + } else { + let error = GitHubClient.Error.api(Response(statusCode: 500, headers: [:], body: Data())) + return SignalProducer(error: error) + } + default: + fatalError("Stub not found") + } + } + + + private func nextStub() -> Stubs { return stubCount.modify { stubCount -> Stubs in let nextStub = stubs[stubCount]