From 23cdaef64718478e0eddcbf595192599bedea86c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 9 Dec 2024 13:01:53 +0100 Subject: [PATCH 1/5] Make maintenance.feedback / maintenance.stale participation optional So that people that are not interested don't need to set maxIssues to 0 explicitly. --- .../github/lottery/config/LotteryConfig.java | 4 +- .../github/lottery/draw/Participant.java | 9 ++-- .../github/lottery/GitHubServiceTest.java | 34 +++++++++++---- .../lottery/LotterySingleRepositoryTest.java | 42 +++++++++---------- 4 files changed, 56 insertions(+), 33 deletions(-) diff --git a/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java b/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java index cbddbc3..a59c7e1 100644 --- a/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java +++ b/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java @@ -128,8 +128,8 @@ public record Maintenance( // TODO default to all labels configured for this user in .github/quarkus-bot.yml @JsonProperty(required = true) List labels, @JsonProperty(required = true) @JsonDeserialize(as = TreeSet.class) Set days, - Feedback feedback, - @JsonProperty(required = true) Participation stale) { + Optional feedback, + @JsonProperty Optional stale) { public record Feedback( @JsonProperty(required = true) Participation needed, @JsonProperty(required = true) Participation provided) { diff --git a/src/main/java/io/quarkus/github/lottery/draw/Participant.java b/src/main/java/io/quarkus/github/lottery/draw/Participant.java index 35d94f9..1f8fb4a 100644 --- a/src/main/java/io/quarkus/github/lottery/draw/Participant.java +++ b/src/main/java/io/quarkus/github/lottery/draw/Participant.java @@ -88,9 +88,12 @@ public LotteryReport report() { private static final class Maintenance { public static Optional create(String username, LotteryConfig.Participant.Maintenance config) { - var feedbackNeeded = Participation.create(username, config.feedback().needed()); - var feedbackProvided = Participation.create(username, config.feedback().provided()); - var stale = Participation.create(username, config.stale()); + var feedbackNeeded = config.feedback().map(LotteryConfig.Participant.Maintenance.Feedback::needed) + .flatMap(p -> Participation.create(username, p)); + var feedbackProvided = config.feedback().map(LotteryConfig.Participant.Maintenance.Feedback::provided) + .flatMap(p -> Participation.create(username, p)); + var stale = config.stale() + .flatMap(p -> Participation.create(username, p)); if (feedbackNeeded.isEmpty() && feedbackProvided.isEmpty() && stale.isEmpty()) { return Optional.empty(); diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index 93a4e0e..4edddae 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -200,6 +200,15 @@ void fetchLotteryConfig() throws IOException { stewardship: days: ["MONDAY"] maxIssues: 10 + - username: "jblack" + maintenance: + labels: ["area/someotherobscurelibrary"] + days: ["MONDAY"] + feedback: + needed: + maxIssues: 1 + provided: + maxIssues: 1 """); }) .when(() -> { @@ -235,10 +244,10 @@ void fetchLotteryConfig() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), - new LotteryConfig.Participant.Maintenance.Feedback( + Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), - new LotteryConfig.Participant.Participation(2)), - new LotteryConfig.Participant.Participation(5))), + new LotteryConfig.Participant.Participation(2))), + Optional.of(new LotteryConfig.Participant.Participation(5)))), Optional.empty()), new LotteryConfig.Participant("gsmet", Optional.of(ZoneId.of("Europe/Paris")), @@ -253,10 +262,10 @@ void fetchLotteryConfig() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/someobscurelibrary"), Set.of(DayOfWeek.MONDAY), - new LotteryConfig.Participant.Maintenance.Feedback( + Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(1), - new LotteryConfig.Participant.Participation(1)), - new LotteryConfig.Participant.Participation(5))), + new LotteryConfig.Participant.Participation(1))), + Optional.of(new LotteryConfig.Participant.Participation(5)))), Optional.empty()), new LotteryConfig.Participant("geoand", Optional.empty(), @@ -264,7 +273,18 @@ void fetchLotteryConfig() throws IOException { Optional.empty(), Optional.of(new LotteryConfig.Participant.Stewardship( Set.of(DayOfWeek.MONDAY), - new LotteryConfig.Participant.Participation(10))))))); + new LotteryConfig.Participant.Participation(10)))), + new LotteryConfig.Participant("jblack", + Optional.empty(), + Optional.empty(), + Optional.of(new LotteryConfig.Participant.Maintenance( + List.of("area/someotherobscurelibrary"), + Set.of(DayOfWeek.MONDAY), + Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( + new LotteryConfig.Participant.Participation(1), + new LotteryConfig.Participant.Participation(1))), + Optional.empty())), + Optional.empty())))); }) .then().github(mocks -> { verifyNoMoreInteractions(mocks.ghObjects()); diff --git a/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java b/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java index b9aa502..af7f4cd 100644 --- a/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java +++ b/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java @@ -155,10 +155,10 @@ void days_differentDay_defaultTimezone() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.TUESDAY), - new LotteryConfig.Participant.Maintenance.Feedback( + Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), - new LotteryConfig.Participant.Participation(2)), - new LotteryConfig.Participant.Participation(5))), + new LotteryConfig.Participant.Participation(2))), + Optional.of(new LotteryConfig.Participant.Participation(5)))), Optional.of(new LotteryConfig.Participant.Stewardship( Set.of(DayOfWeek.TUESDAY), new LotteryConfig.Participant.Participation(10)))))); @@ -185,10 +185,10 @@ void days_differentDay_explicitTimezone() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), - new LotteryConfig.Participant.Maintenance.Feedback( + Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), - new LotteryConfig.Participant.Participation(2)), - new LotteryConfig.Participant.Participation(5))), + new LotteryConfig.Participant.Participation(2))), + Optional.of(new LotteryConfig.Participant.Participation(5)))), Optional.of(new LotteryConfig.Participant.Stewardship( Set.of(DayOfWeek.TUESDAY), new LotteryConfig.Participant.Participation(10)))))); @@ -217,10 +217,10 @@ void alreadyNotifiedToday() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), - new LotteryConfig.Participant.Maintenance.Feedback( + Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), - new LotteryConfig.Participant.Participation(2)), - new LotteryConfig.Participant.Participation(5))), + new LotteryConfig.Participant.Participation(2))), + Optional.of(new LotteryConfig.Participant.Participation(5)))), Optional.of(new LotteryConfig.Participant.Stewardship( Set.of(DayOfWeek.TUESDAY), new LotteryConfig.Participant.Participation(10)))))); @@ -250,10 +250,10 @@ void ignoring() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), - new LotteryConfig.Participant.Maintenance.Feedback( + Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), - new LotteryConfig.Participant.Participation(2)), - new LotteryConfig.Participant.Participation(5))), + new LotteryConfig.Participant.Participation(2))), + Optional.of(new LotteryConfig.Participant.Participation(5)))), Optional.of(new LotteryConfig.Participant.Stewardship( Set.of(DayOfWeek.TUESDAY), new LotteryConfig.Participant.Participation(10)))))); @@ -366,10 +366,10 @@ void maintenance() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), - new LotteryConfig.Participant.Maintenance.Feedback( + Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), - new LotteryConfig.Participant.Participation(2)), - new LotteryConfig.Participant.Participation(5))), + new LotteryConfig.Participant.Participation(2))), + Optional.of(new LotteryConfig.Participant.Participation(5)))), Optional.empty()))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); @@ -440,10 +440,10 @@ void maintenance_issueAlreadyHasTimedOutNotification() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), - new LotteryConfig.Participant.Maintenance.Feedback( + Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), - new LotteryConfig.Participant.Participation(2)), - new LotteryConfig.Participant.Participation(5))), + new LotteryConfig.Participant.Participation(2))), + Optional.of(new LotteryConfig.Participant.Participation(5)))), Optional.empty()))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); @@ -604,10 +604,10 @@ void stewardship_doesNotAffectMaintenance() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-search"), Set.of(DayOfWeek.MONDAY), - new LotteryConfig.Participant.Maintenance.Feedback( + Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), - new LotteryConfig.Participant.Participation(2)), - new LotteryConfig.Participant.Participation(5))), + new LotteryConfig.Participant.Participation(2))), + Optional.of(new LotteryConfig.Participant.Participation(5)))), Optional.empty()), new LotteryConfig.Participant("gsmet", Optional.empty(), From 1ee5d92fb8a1c6382e00d5906fbbad355f3f4554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 9 Dec 2024 13:22:18 +0100 Subject: [PATCH 2/5] Ignore bots when determining who last commented on an issue --- .../lottery/github/GitHubInstallationRef.java | 4 +- .../lottery/github/GitHubRepository.java | 25 +++- .../github/lottery/util/GitHubConstants.java | 10 ++ .../github/lottery/GitHubServiceTest.java | 132 +++++++++++------- .../github/lottery/util/MockHelper.java | 41 +++++- 5 files changed, 152 insertions(+), 60 deletions(-) create mode 100644 src/main/java/io/quarkus/github/lottery/util/GitHubConstants.java diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubInstallationRef.java b/src/main/java/io/quarkus/github/lottery/github/GitHubInstallationRef.java index 95bd619..ce0cbd2 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubInstallationRef.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubInstallationRef.java @@ -1,5 +1,7 @@ package io.quarkus.github.lottery.github; +import io.quarkus.github.lottery.util.GitHubConstants; + /** * A reference to a GitHub application installation. * @@ -9,7 +11,7 @@ public record GitHubInstallationRef(String appSlug, long installationId) { public String appLogin() { - return appSlug() + "[bot]"; + return appSlug() + GitHubConstants.BOT_LOGIN_SUFFIX; } } diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java index 186c956..c10a331 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -31,6 +31,7 @@ import io.quarkiverse.githubapp.GitHubConfigFileProvider; import io.quarkus.github.lottery.config.LotteryConfig; import io.quarkus.github.lottery.message.MessageFormatter; +import io.quarkus.github.lottery.util.GitHubConstants; import io.quarkus.github.lottery.util.Streams; import io.quarkus.logging.Log; import io.smallrye.graphql.client.dynamic.api.DynamicGraphQLClient; @@ -194,12 +195,9 @@ private IssueActionSide lastActionSide(GHIssue ghIssue, Set initialActio lastEventActionSideInstant = event.getCreatedAt().toInstant(); } } - GHIssueCommentQueryBuilder queryCommentsBuilder = ghIssue.queryComments(); - if (lastEventActionSideInstant != null) { - queryCommentsBuilder.since(Date.from(lastEventActionSideInstant)); - } - Optional lastComment = toStream(queryCommentsBuilder.list()).reduce(Streams.last()); + Optional lastComment = getNonBotCommentsSince(ghIssue, lastEventActionSideInstant) + .reduce(Streams.last()); if (lastComment.isEmpty()) { // No action since the label was assigned. return IssueActionSide.TEAM; @@ -337,10 +335,25 @@ private GHIssue createDedicatedIssue(String title, String lastCommentMarkdownBod private Stream getAppCommentsSince(GHIssue issue, Instant since) { String appLogin = appLogin(); - return toStream(issue.queryComments().since(Date.from(since)).list()) + GHIssueCommentQueryBuilder queryCommentsBuilder = issue.queryComments(); + if (since != null) { + queryCommentsBuilder.since(Date.from(since)); + } + return toStream(queryCommentsBuilder.list()) .filter(uncheckedIO((GHIssueComment comment) -> appLogin.equals(comment.getUser().getLogin()))::apply); } + private Stream getNonBotCommentsSince(GHIssue issue, Instant since) { + GHIssueCommentQueryBuilder queryCommentsBuilder = issue.queryComments(); + if (since != null) { + queryCommentsBuilder.since(Date.from(since)); + } + return toStream(queryCommentsBuilder.list()) + // Relying on the login rather than getType(), because that would involve an additional request. + .filter(uncheckedIO((GHIssueComment comment) -> !comment.getUser().getLogin() + .endsWith(GitHubConstants.BOT_LOGIN_SUFFIX))::apply); + } + private void minimizeOutdatedComment(GHIssueComment comment) { try { Map variables = new HashMap<>(); diff --git a/src/main/java/io/quarkus/github/lottery/util/GitHubConstants.java b/src/main/java/io/quarkus/github/lottery/util/GitHubConstants.java new file mode 100644 index 0000000..27418e1 --- /dev/null +++ b/src/main/java/io/quarkus/github/lottery/util/GitHubConstants.java @@ -0,0 +1,10 @@ +package io.quarkus.github.lottery.util; + +public final class GitHubConstants { + + private GitHubConstants() { + } + + public static final String BOT_LOGIN_SUFFIX = "[bot]"; + +} diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index 4edddae..a10384b 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -1,6 +1,7 @@ package io.quarkus.github.lottery; import static io.quarkiverse.githubapp.testing.GitHubAppTesting.given; +import static io.quarkus.github.lottery.util.MockHelper.mockIssueComment; import static io.quarkus.github.lottery.util.MockHelper.mockIssueEvent; import static io.quarkus.github.lottery.util.MockHelper.mockIssueForLottery; import static io.quarkus.github.lottery.util.MockHelper.mockIssueForLotteryFilteredOutByRepository; @@ -8,6 +9,7 @@ import static io.quarkus.github.lottery.util.MockHelper.mockLabel; import static io.quarkus.github.lottery.util.MockHelper.mockPagedIterable; import static io.quarkus.github.lottery.util.MockHelper.mockPullRequestForLotteryFilteredOutByRepository; +import static io.quarkus.github.lottery.util.MockHelper.mockUserForInspectedComments; import static io.quarkus.github.lottery.util.MockHelper.stubIssueList; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; @@ -382,6 +384,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { Date afterCutoff = Date.from(cutoff.plus(1, ChronoUnit.HOURS)); Date issue1ActionLabelEvent = Date.from(cutoff.minus(1, ChronoUnit.DAYS)); Date issue2ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); + Date issue7ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); var queryIssuesNeedsFeedbackBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); @@ -397,6 +400,8 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { withSettings().defaultAnswer(Answers.RETURNS_SELF)); var issue5QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); + var issue7QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { var repositoryMock = mocks.repository(repoRef.repositoryName()); @@ -410,19 +415,20 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { var issue4Mock = mockIssueForLottery(mocks, 4, beforeCutoff); var issue5Mock = mockIssueForLottery(mocks, 5, beforeCutoff); var issue6Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 6, afterCutoff); + var issue7Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 7, beforeCutoff); var issuesNeedsFeedbackMocks = mockPagedIterable(issue1Mock, issue3Mock, issue5Mock); when(queryIssuesNeedsFeedbackBuilderMock.list()).thenReturn(issuesNeedsFeedbackMocks); - var issuesNeedsReproducerMocks = mockPagedIterable(prMock, issue2Mock, issue4Mock, issue6Mock); + var issuesNeedsReproducerMocks = mockPagedIterable(prMock, issue2Mock, issue4Mock, issue6Mock, issue7Mock); when(queryIssuesNeedsReproducerBuilderMock.list()).thenReturn(issuesNeedsReproducerMocks); - var adminUser = mocks.ghObject(GHUser.class, 1L); - when(repositoryMock.getPermission(adminUser)).thenReturn(GHPermissionType.ADMIN); - var writeUser = mocks.ghObject(GHUser.class, 2L); - when(repositoryMock.getPermission(writeUser)).thenReturn(GHPermissionType.WRITE); - var readUser = mocks.ghObject(GHUser.class, 3L); - when(repositoryMock.getPermission(readUser)).thenReturn(GHPermissionType.READ); - var noneUser = mocks.ghObject(GHUser.class, 4L); - when(repositoryMock.getPermission(noneUser)).thenReturn(GHPermissionType.NONE); + var adminUser = mockUserForInspectedComments(mocks, repositoryMock, 1L, "someadmin", + GHPermissionType.ADMIN); + var writeUser = mockUserForInspectedComments(mocks, repositoryMock, 2L, "somewriter", + GHPermissionType.WRITE); + var readUser = mockUserForInspectedComments(mocks, repositoryMock, 3L, "somereader", GHPermissionType.READ); + var noneUser = mockUserForInspectedComments(mocks, repositoryMock, 4L, "somestranger", + GHPermissionType.NONE); + var botUser = mockUserForInspectedComments(mocks, repositoryMock, 5L, "somebot[bot]"); var needsReproducerLabelMock = mockLabel("triage/needs-reproducer"); var areaHibernateSearchLabelMock = mockLabel("area/hibernate-search"); @@ -437,10 +443,8 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { var issue1EventsMocks = mockPagedIterable(issue1Event1Mock, issue1Event2Mock, issue1Event3Mock, issue1Event4Mock); when(issue1Mock.listEvents()).thenReturn(issue1EventsMocks); - var issue1Comment1Mock = mocks.issueComment(101); - var issue1Comment2Mock = mocks.issueComment(102); - when(issue1Comment2Mock.getUser()).thenReturn(adminUser); - var issue1CommentsMocks = mockPagedIterable(issue1Comment1Mock, issue1Comment2Mock); + var issue1CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 101, noneUser), + mockIssueComment(mocks, 102, adminUser)); when(issue1Mock.queryComments()).thenReturn(issue1QueryCommentsBuilderMock); when(issue1QueryCommentsBuilderMock.list()).thenReturn(issue1CommentsMocks); @@ -454,19 +458,15 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { var issue2EventsMocks = mockPagedIterable(issue2Event1Mock, issue2Event2Mock, issue2Event3Mock, issue2Event4Mock); when(issue2Mock.listEvents()).thenReturn(issue2EventsMocks); - var issue2Comment1Mock = mocks.issueComment(201); - var issue2Comment2Mock = mocks.issueComment(202); - when(issue2Comment2Mock.getUser()).thenReturn(readUser); - var issue2CommentsMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock); + var issue2CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 201, adminUser), + mockIssueComment(mocks, 202, readUser)); when(issue2Mock.queryComments()).thenReturn(issue2QueryCommentsBuilderMock); when(issue2QueryCommentsBuilderMock.list()).thenReturn(issue2CommentsMocks); PagedSearchIterable issue3EventsMocks = mockPagedIterable(); when(issue3Mock.listEvents()).thenReturn(issue3EventsMocks); - var issue3Comment1Mock = mocks.issueComment(301); - var issue3Comment2Mock = mocks.issueComment(302); - when(issue3Comment2Mock.getUser()).thenReturn(noneUser); - var issue3CommentsMocks = mockPagedIterable(issue3Comment1Mock, issue3Comment2Mock); + var issue3CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 301, adminUser), + mockIssueComment(mocks, 302, noneUser)); when(issue3Mock.queryComments()).thenReturn(issue3QueryCommentsBuilderMock); when(issue3QueryCommentsBuilderMock.list()).thenReturn(issue3CommentsMocks); @@ -480,12 +480,27 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { var issue5Event2Mock = mockIssueEvent("locked"); var issue5EventsMocks = mockPagedIterable(issue5Event1Mock, issue5Event2Mock); when(issue5Mock.listEvents()).thenReturn(issue5EventsMocks); - var issue5Comment1Mock = mocks.issueComment(501); - var issue5Comment2Mock = mocks.issueComment(502); - when(issue5Comment2Mock.getUser()).thenReturn(writeUser); - var issue5CommentsMocks = mockPagedIterable(issue5Comment1Mock, issue5Comment2Mock); + var issue5CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 501, noneUser), + mockIssueComment(mocks, 502, writeUser)); when(issue5Mock.queryComments()).thenReturn(issue5QueryCommentsBuilderMock); when(issue5QueryCommentsBuilderMock.list()).thenReturn(issue5CommentsMocks); + + // This is like issue 2, but a bot commented after the user -- which should be ignored. + var issue7Event1Mock = mockIssueEvent("created"); + var issue7Event2Mock = mockIssueEvent("labeled"); + when(issue7Event2Mock.getLabel()).thenReturn(needsReproducerLabelMock); + when(issue7Event2Mock.getCreatedAt()).thenReturn(issue7ActionLabelEvent); + var issue7Event3Mock = mockIssueEvent("labeled"); + when(issue7Event3Mock.getLabel()).thenReturn(areaHibernateSearchLabelMock); + var issue7Event4Mock = mockIssueEvent("locked"); + var issue7EventsMocks = mockPagedIterable(issue7Event1Mock, + issue7Event2Mock, issue7Event3Mock, issue7Event4Mock); + when(issue7Mock.listEvents()).thenReturn(issue7EventsMocks); + var issue7CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 701, noneUser), + mockIssueComment(mocks, 702, readUser), + mockIssueComment(mocks, 703, botUser)); + when(issue7Mock.queryComments()).thenReturn(issue7QueryCommentsBuilderMock); + when(issue7QueryCommentsBuilderMock.list()).thenReturn(issue7CommentsMocks); }) .when(() -> { var repo = gitHubService.repository(repoRef); @@ -510,6 +525,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { verify(issue1QueryCommentsBuilderMock).since(issue1ActionLabelEvent); verify(issue2QueryCommentsBuilderMock).since(issue2ActionLabelEvent); + verify(issue7QueryCommentsBuilderMock).since(issue7ActionLabelEvent); verifyNoMoreInteractions(mocks.ghObjects()); }); @@ -525,6 +541,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { Date afterCutoff = Date.from(cutoff.plus(1, ChronoUnit.HOURS)); Date issue1ActionLabelEvent = Date.from(cutoff.minus(1, ChronoUnit.DAYS)); Date issue2ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); + Date issue7ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); var queryIssuesNeedsReproducerBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); @@ -540,6 +557,8 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { withSettings().defaultAnswer(Answers.RETURNS_SELF)); var issue5QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); + var issue7QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { var repositoryMock = mocks.repository(repoRef.repositoryName()); @@ -555,19 +574,20 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { var issue4Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 4, beforeCutoff); var issue5Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 5, beforeCutoff); var issue6Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 6, afterCutoff); - var issuesNeedsFeedbackMocks = mockPagedIterable(prMock, issue2Mock, issue4Mock, issue6Mock); + var issue7Mock = mockIssueForLottery(mocks, 7, beforeCutoff); + var issuesNeedsFeedbackMocks = mockPagedIterable(prMock, issue2Mock, issue4Mock, issue6Mock, issue7Mock); when(queryIssuesNeedsFeedbackBuilderMock.list()).thenReturn(issuesNeedsFeedbackMocks); var issuesNeedsReproducerMocks = mockPagedIterable(issue1Mock, issue3Mock, issue5Mock); when(queryIssuesNeedsReproducerBuilderMock.list()).thenReturn(issuesNeedsReproducerMocks); - var adminUser = mocks.ghObject(GHUser.class, 1L); - when(repositoryMock.getPermission(adminUser)).thenReturn(GHPermissionType.ADMIN); - var writeUser = mocks.ghObject(GHUser.class, 2L); - when(repositoryMock.getPermission(writeUser)).thenReturn(GHPermissionType.WRITE); - var readUser = mocks.ghObject(GHUser.class, 3L); - when(repositoryMock.getPermission(readUser)).thenReturn(GHPermissionType.READ); - var noneUser = mocks.ghObject(GHUser.class, 4L); - when(repositoryMock.getPermission(noneUser)).thenReturn(GHPermissionType.NONE); + var adminUser = mockUserForInspectedComments(mocks, repositoryMock, 1L, "someadmin", + GHPermissionType.ADMIN); + var writeUser = mockUserForInspectedComments(mocks, repositoryMock, 2L, "somewriter", + GHPermissionType.WRITE); + var readUser = mockUserForInspectedComments(mocks, repositoryMock, 3L, "somereader", GHPermissionType.READ); + var noneUser = mockUserForInspectedComments(mocks, repositoryMock, 4L, "somestranger", + GHPermissionType.NONE); + var botUser = mockUserForInspectedComments(mocks, repositoryMock, 5L, "somebot[bot]"); var needsReproducerLabelMock = mockLabel("triage/needs-reproducer"); var areaHibernateSearchLabelMock = mockLabel("area/hibernate-search"); @@ -582,10 +602,8 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { var issue1EventsMocks = mockPagedIterable(issue1Event1Mock, issue1Event2Mock, issue1Event3Mock, issue1Event4Mock); when(issue1Mock.listEvents()).thenReturn(issue1EventsMocks); - var issue1Comment1Mock = mocks.issueComment(101); - var issue1Comment2Mock = mocks.issueComment(102); - when(issue1Comment2Mock.getUser()).thenReturn(adminUser); - var issue1CommentsMocks = mockPagedIterable(issue1Comment1Mock, issue1Comment2Mock); + var issue1CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 101, noneUser), + mockIssueComment(mocks, 102, adminUser)); when(issue1Mock.queryComments()).thenReturn(issue1QueryCommentsBuilderMock); when(issue1QueryCommentsBuilderMock.list()).thenReturn(issue1CommentsMocks); @@ -599,19 +617,15 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { var issue2EventsMocks = mockPagedIterable(issue2Event1Mock, issue2Event2Mock, issue2Event3Mock, issue2Event4Mock); when(issue2Mock.listEvents()).thenReturn(issue2EventsMocks); - var issue2Comment1Mock = mocks.issueComment(201); - var issue2Comment2Mock = mocks.issueComment(202); - when(issue2Comment2Mock.getUser()).thenReturn(readUser); - var issue2CommentsMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock); + var issue2CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 201, adminUser), + mockIssueComment(mocks, 202, readUser)); when(issue2Mock.queryComments()).thenReturn(issue2QueryCommentsBuilderMock); when(issue2QueryCommentsBuilderMock.list()).thenReturn(issue2CommentsMocks); PagedSearchIterable issue3EventsMocks = mockPagedIterable(); when(issue3Mock.listEvents()).thenReturn(issue3EventsMocks); - var issue3Comment1Mock = mocks.issueComment(301); - var issue3Comment2Mock = mocks.issueComment(302); - when(issue3Comment2Mock.getUser()).thenReturn(noneUser); - var issue3CommentsMocks = mockPagedIterable(issue3Comment1Mock, issue3Comment2Mock); + var issue3CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 301, adminUser), + mockIssueComment(mocks, 302, noneUser)); when(issue3Mock.queryComments()).thenReturn(issue3QueryCommentsBuilderMock); when(issue3QueryCommentsBuilderMock.list()).thenReturn(issue3CommentsMocks); @@ -625,12 +639,27 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { var issue5Event2Mock = mockIssueEvent("locked"); var issue5EventsMocks = mockPagedIterable(issue5Event1Mock, issue5Event2Mock); when(issue5Mock.listEvents()).thenReturn(issue5EventsMocks); - var issue5Comment1Mock = mocks.issueComment(501); - var issue5Comment2Mock = mocks.issueComment(502); - when(issue5Comment2Mock.getUser()).thenReturn(writeUser); - var issue5CommentsMocks = mockPagedIterable(issue5Comment1Mock, issue5Comment2Mock); + var issue5CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 501, noneUser), + mockIssueComment(mocks, 502, writeUser)); when(issue5Mock.queryComments()).thenReturn(issue5QueryCommentsBuilderMock); when(issue5QueryCommentsBuilderMock.list()).thenReturn(issue5CommentsMocks); + + // This is like issue 2, but a bot commented after the user -- which should be ignored. + var issue7Event1Mock = mockIssueEvent("created"); + var issue7Event2Mock = mockIssueEvent("labeled"); + when(issue7Event2Mock.getLabel()).thenReturn(needsReproducerLabelMock); + when(issue7Event2Mock.getCreatedAt()).thenReturn(issue7ActionLabelEvent); + var issue7Event3Mock = mockIssueEvent("labeled"); + when(issue7Event3Mock.getLabel()).thenReturn(areaHibernateSearchLabelMock); + var issue7Event4Mock = mockIssueEvent("locked"); + var issue7EventsMocks = mockPagedIterable(issue7Event1Mock, + issue7Event2Mock, issue7Event3Mock, issue7Event4Mock); + when(issue7Mock.listEvents()).thenReturn(issue7EventsMocks); + var issue7CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 701, adminUser), + mockIssueComment(mocks, 702, readUser), + mockIssueComment(mocks, 703, botUser)); + when(issue7Mock.queryComments()).thenReturn(issue7QueryCommentsBuilderMock); + when(issue7QueryCommentsBuilderMock.list()).thenReturn(issue7CommentsMocks); }) .when(() -> { var repo = gitHubService.repository(repoRef); @@ -638,7 +667,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { assertThat(repo.issuesLastActedOnByAndLastUpdatedBefore( new LinkedHashSet<>(List.of("triage/needs-feedback", "triage/needs-reproducer")), "area/hibernate-search", IssueActionSide.OUTSIDER, cutoff)) - .containsExactlyElementsOf(stubIssueList(2, 3)); + .containsExactlyElementsOf(stubIssueList(2, 3, 7)); }) .then().github(mocks -> { verify(queryIssuesNeedsFeedbackBuilderMock).state(GHIssueState.OPEN); @@ -655,6 +684,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { verify(issue1QueryCommentsBuilderMock).since(issue1ActionLabelEvent); verify(issue2QueryCommentsBuilderMock).since(issue2ActionLabelEvent); + verify(issue7QueryCommentsBuilderMock).since(issue7ActionLabelEvent); verifyNoMoreInteractions(mocks.ghObjects()); }); diff --git a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java index f066808..b7fe6bb 100644 --- a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java +++ b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java @@ -13,19 +13,24 @@ import java.util.List; import java.util.stream.IntStream; -import io.quarkiverse.githubapp.testing.dsl.GitHubMockContext; -import io.quarkus.github.lottery.github.Issue; import org.kohsuke.github.GHIssue; +import org.kohsuke.github.GHIssueComment; import org.kohsuke.github.GHIssueEvent; import org.kohsuke.github.GHLabel; +import org.kohsuke.github.GHPermissionType; import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHPullRequestFileDetail; +import org.kohsuke.github.GHRepository; +import org.kohsuke.github.GHUser; import org.kohsuke.github.PagedIterator; import org.kohsuke.github.PagedSearchIterable; import org.mockito.Answers; import org.mockito.Mockito; import org.mockito.quality.Strictness; +import io.quarkiverse.githubapp.testing.dsl.GitHubMockContext; +import io.quarkus.github.lottery.github.Issue; + public class MockHelper { @SafeVarargs @@ -109,4 +114,36 @@ public static GHPullRequestFileDetail mockGHPullRequestFileDetail(String filenam return mock; } + public static GHUser mockUserForInspectedComments(GitHubMockContext context, GHRepository repositoryMock, + long id, String login) + throws IOException { + return mockUserForInspectedComments(context, repositoryMock, id, login, null); + } + + public static GHUser mockUserForInspectedComments(GitHubMockContext context, GHRepository repositoryMock, + long id, String login, GHPermissionType permissionType) + throws IOException { + GHUser mock = context.ghObject(GHUser.class, id); + when(mock.getLogin()).thenReturn(login); + if (permissionType != null) { + when(repositoryMock.getPermission(mock)).thenReturn(permissionType); + } + return mock; + } + + public static GHIssueComment mockIssueComment(GitHubMockContext context, long id, GHUser author) + throws IOException { + return mockIssueComment(context, id, author, null); + } + + public static GHIssueComment mockIssueComment(GitHubMockContext context, long id, GHUser author, String body) + throws IOException { + GHIssueComment mock = context.issueComment(id); + when(mock.getUser()).thenReturn(author); + if (body != null) { + when(mock.getBody()).thenReturn(body); + } + return mock; + } + } From b616efb457ef3d80ba62c0663251f310e7a63271 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 9 Dec 2024 14:20:35 +0100 Subject: [PATCH 3/5] Simplify a bit more mocking --- .../github/lottery/GitHubServiceTest.java | 87 +++++++------------ 1 file changed, 29 insertions(+), 58 deletions(-) diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index a10384b..c06e469 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -780,11 +780,8 @@ void topic_extractComments_dedicatedIssueExists_appCommentsDoNotExist() throws E when(someoneElseMock.getLogin()).thenReturn("yrodiere"); when(issue2Mock.queryComments()).thenReturn(queryCommentsBuilderMock); - var issue2Comment1Mock = mocks.issueComment(201); - when(issue2Comment1Mock.getUser()).thenReturn(someoneElseMock); - var issue2Comment2Mock = mocks.issueComment(202); - when(issue2Comment2Mock.getUser()).thenReturn(someoneElseMock); - var issue2CommentMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock); + var issue2CommentMocks = mockPagedIterable(mockIssueComment(mocks, 201, someoneElseMock), + mockIssueComment(mocks, 202, someoneElseMock)); when(queryCommentsBuilderMock.list()).thenReturn(issue2CommentMocks); }) .when(() -> { @@ -871,15 +868,10 @@ void topic_extractComments_dedicatedIssueExists_appCommentsExist() throws Except when(someoneElseMock.getLogin()).thenReturn("yrodiere"); when(issue2Mock.queryComments()).thenReturn(queryCommentsBuilderMock); - var issue2Comment1Mock = mocks.issueComment(202); - when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); - when(issue2Comment1Mock.getBody()).thenReturn("issue2Comment1Mock#body"); - var issue2Comment2Mock = mocks.issueComment(203); - when(issue2Comment2Mock.getUser()).thenReturn(mySelfMock); - when(issue2Comment2Mock.getBody()).thenReturn("issue2Comment2Mock#body"); - var issue2Comment3Mock = mocks.issueComment(204); - when(issue2Comment3Mock.getUser()).thenReturn(someoneElseMock); - var issue2CommentMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock, issue2Comment3Mock); + var issue2CommentMocks = mockPagedIterable( + mockIssueComment(mocks, 201, mySelfMock, "issue2Comment1Mock#body"), + mockIssueComment(mocks, 202, mySelfMock, "issue2Comment2Mock#body"), + mockIssueComment(mocks, 203, someoneElseMock)); when(queryCommentsBuilderMock.list()).thenReturn(issue2CommentMocks); }) .when(() -> { @@ -925,14 +917,9 @@ void topic_extractComments_dedicatedIssueExists_appCommentsExist_withConfusingOt when(someoneElseMock.getLogin()).thenReturn("yrodiere"); when(issue2Mock.queryComments()).thenReturn(queryCommentsBuilderMock); - var issue2Comment1Mock = mocks.issueComment(202); - when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); - when(issue2Comment1Mock.getBody()).thenReturn("issue2Comment1Mock#body"); - var issue2Comment2Mock = mocks.issueComment(203); - when(issue2Comment2Mock.getUser()).thenReturn(mySelfMock); - when(issue2Comment2Mock.getBody()).thenReturn("issue2Comment2Mock#body"); - var issue2Comment3Mock = mocks.issueComment(204); - when(issue2Comment3Mock.getUser()).thenReturn(someoneElseMock); + var issue2Comment1Mock = mockIssueComment(mocks, 202, mySelfMock, "issue2Comment1Mock#body"); + var issue2Comment2Mock = mockIssueComment(mocks, 203, mySelfMock, "issue2Comment2Mock#body"); + var issue2Comment3Mock = mockIssueComment(mocks, 204, someoneElseMock); var issue2CommentMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock, issue2Comment3Mock); when(queryCommentsBuilderMock.list()).thenReturn(issue2CommentMocks); }) @@ -987,17 +974,13 @@ void topic_comment_dedicatedIssueExists_open() throws Exception { when(someoneElseMock.getLogin()).thenReturn("yrodiere"); when(issue2Mock.queryComments()).thenReturn(queryCommentsBuilderMock); - var issue2Comment1Mock = mocks.issueComment(201); - when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); - var issue2Comment2Mock = mocks.issueComment(202); - when(issue2Comment2Mock.getUser()).thenReturn(mySelfMock); - var issue2Comment3Mock = mocks.issueComment(203); - when(issue2Comment3Mock.getUser()).thenReturn(someoneElseMock); - var issue2CommentMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock, issue2Comment3Mock); + var commentToMinimizeMock = mockIssueComment(mocks, 202, mySelfMock); + when(commentToMinimizeMock.getNodeId()).thenReturn(commentToMinimizeNodeId); + var issue2CommentMocks = mockPagedIterable(mockIssueComment(mocks, 201, mySelfMock), + commentToMinimizeMock, + mockIssueComment(mocks, 203, someoneElseMock)); when(queryCommentsBuilderMock.list()).thenReturn(issue2CommentMocks); - when(issue2Comment2Mock.getNodeId()).thenReturn(commentToMinimizeNodeId); - when(messageFormatterMock.formatDedicatedIssueBodyMarkdown("yrodiere's report for quarkusio/quarkus", "Some content")) .thenReturn("Dedicated issue body"); @@ -1062,17 +1045,13 @@ void topic_comment_dedicatedIssueExists_noTopicSuffix() throws Exception { when(someoneElseMock.getLogin()).thenReturn("yrodiere"); when(issue2Mock.queryComments()).thenReturn(queryCommentsBuilderMock); - var issue2Comment1Mock = mocks.issueComment(201); - when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); - var issue2Comment2Mock = mocks.issueComment(202); - when(issue2Comment2Mock.getUser()).thenReturn(mySelfMock); - var issue2Comment3Mock = mocks.issueComment(203); - when(issue2Comment3Mock.getUser()).thenReturn(someoneElseMock); - var issue2CommentMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock, issue2Comment3Mock); + var commentToMinimizeMock = mockIssueComment(mocks, 202, mySelfMock); + when(commentToMinimizeMock.getNodeId()).thenReturn(commentToMinimizeNodeId); + var issue2CommentMocks = mockPagedIterable(mockIssueComment(mocks, 201, mySelfMock), + mockIssueComment(mocks, 202, mySelfMock), + mockIssueComment(mocks, 203, someoneElseMock)); when(queryCommentsBuilderMock.list()).thenReturn(issue2CommentMocks); - when(issue2Comment2Mock.getNodeId()).thenReturn(commentToMinimizeNodeId); - when(messageFormatterMock.formatDedicatedIssueBodyMarkdown("Lottery history for quarkusio/quarkus", "Some content")) .thenReturn("Dedicated issue body"); @@ -1135,17 +1114,13 @@ void topic_comment_dedicatedIssueExists_withConfusingOther() throws Exception { when(someoneElseMock.getLogin()).thenReturn("yrodiere"); when(issue2Mock.queryComments()).thenReturn(queryCommentsBuilderMock); - var issue2Comment1Mock = mocks.issueComment(201); - when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); - var issue2Comment2Mock = mocks.issueComment(202); - when(issue2Comment2Mock.getUser()).thenReturn(mySelfMock); - var issue2Comment3Mock = mocks.issueComment(203); - when(issue2Comment3Mock.getUser()).thenReturn(someoneElseMock); - var issue2CommentMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock, issue2Comment3Mock); + var commentToMinimizeMock = mockIssueComment(mocks, 202, mySelfMock); + when(commentToMinimizeMock.getNodeId()).thenReturn(commentToMinimizeNodeId); + var issue2CommentMocks = mockPagedIterable(mockIssueComment(mocks, 201, mySelfMock), + mockIssueComment(mocks, 202, mySelfMock), + mockIssueComment(mocks, 203, someoneElseMock)); when(queryCommentsBuilderMock.list()).thenReturn(issue2CommentMocks); - when(issue2Comment2Mock.getNodeId()).thenReturn(commentToMinimizeNodeId); - when(messageFormatterMock.formatDedicatedIssueBodyMarkdown("Lottery history for quarkusio/quarkus", "Some content")) .thenReturn("Dedicated issue body"); @@ -1209,17 +1184,13 @@ void topic_comment_dedicatedIssueExists_closed() throws Exception { when(someoneElseMock.getLogin()).thenReturn("yrodiere"); when(issue2Mock.queryComments()).thenReturn(queryCommentsBuilderMock); - var issue2Comment1Mock = mocks.issueComment(201); - when(issue2Comment1Mock.getUser()).thenReturn(mySelfMock); - var issue2Comment2Mock = mocks.issueComment(202); - when(issue2Comment2Mock.getUser()).thenReturn(mySelfMock); - var issue2Comment3Mock = mocks.issueComment(203); - when(issue2Comment3Mock.getUser()).thenReturn(someoneElseMock); - var issue2CommentMocks = mockPagedIterable(issue2Comment1Mock, issue2Comment2Mock, issue2Comment3Mock); + var commentToMinimizeMock = mockIssueComment(mocks, 202, mySelfMock); + when(commentToMinimizeMock.getNodeId()).thenReturn(commentToMinimizeNodeId); + var issue2CommentMocks = mockPagedIterable(mockIssueComment(mocks, 201, mySelfMock), + mockIssueComment(mocks, 202, mySelfMock), + mockIssueComment(mocks, 203, someoneElseMock)); when(queryCommentsBuilderMock.list()).thenReturn(issue2CommentMocks); - when(issue2Comment2Mock.getNodeId()).thenReturn(commentToMinimizeNodeId); - when(messageFormatterMock.formatDedicatedIssueBodyMarkdown("yrodiere's report for quarkusio/quarkus", "Some content")) .thenReturn("Dedicated issue body"); From d0936734bb26996a56a25b483926e7483aa57c74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 9 Dec 2024 15:08:00 +0100 Subject: [PATCH 4/5] Improve team/outsider detection 1. People with just "triage" permission are still considered team members 2. Issue reporters are always considered "outsiders" for the purposes of this app. --- .../lottery/github/GitHubRepository.java | 18 ++- .../github/lottery/GitHubServiceTest.java | 108 +++++++++++++----- .../github/lottery/util/MockHelper.java | 22 ++++ 3 files changed, 114 insertions(+), 34 deletions(-) diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java index c10a331..dfb6d59 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -24,6 +24,7 @@ import org.kohsuke.github.GHIssueQueryBuilder; import org.kohsuke.github.GHIssueState; import org.kohsuke.github.GHRepository; +import org.kohsuke.github.GHUser; import org.kohsuke.github.GitHub; import io.quarkiverse.githubapp.ConfigFile; @@ -202,9 +203,20 @@ private IssueActionSide lastActionSide(GHIssue ghIssue, Set initialActio // No action since the label was assigned. return IssueActionSide.TEAM; } - return switch (repository().getPermission(lastComment.get().getUser())) { - case ADMIN, WRITE -> IssueActionSide.TEAM; - case READ, UNKNOWN, NONE -> IssueActionSide.OUTSIDER; + return getIssueActionSide(ghIssue, lastComment.get().getUser()); + } + + private IssueActionSide getIssueActionSide(GHIssue issue, GHUser user) throws IOException { + if (issue.getUser().getLogin().equals(user.getLogin())) { + // This is the reporter; even if part of the team, + // we'll consider he's acting as an outsider here, + // because he's unlikely to ask for feedback from himself. + return IssueActionSide.OUTSIDER; + } + + return switch (repository().getPermission(user)) { + case ADMIN, WRITE, UNKNOWN -> IssueActionSide.TEAM; // "Unknown" includes "triage" + case READ, NONE -> IssueActionSide.OUTSIDER; }; } diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index c06e469..af53aa1 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -385,6 +385,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { Date issue1ActionLabelEvent = Date.from(cutoff.minus(1, ChronoUnit.DAYS)); Date issue2ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); Date issue7ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); + Date issue8ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); var queryIssuesNeedsFeedbackBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); @@ -402,25 +403,12 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { withSettings().defaultAnswer(Answers.RETURNS_SELF)); var issue7QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); + var issue8QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { var repositoryMock = mocks.repository(repoRef.repositoryName()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesNeedsFeedbackBuilderMock) - .thenReturn(queryIssuesNeedsReproducerBuilderMock); - var prMock = mockPullRequestForLotteryFilteredOutByRepository(mocks, 0); - var issue1Mock = mockIssueForLottery(mocks, 1, beforeCutoff); - var issue2Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 2, beforeCutoff); - var issue3Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 3, beforeCutoff); - var issue4Mock = mockIssueForLottery(mocks, 4, beforeCutoff); - var issue5Mock = mockIssueForLottery(mocks, 5, beforeCutoff); - var issue6Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 6, afterCutoff); - var issue7Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 7, beforeCutoff); - var issuesNeedsFeedbackMocks = mockPagedIterable(issue1Mock, issue3Mock, issue5Mock); - when(queryIssuesNeedsFeedbackBuilderMock.list()).thenReturn(issuesNeedsFeedbackMocks); - var issuesNeedsReproducerMocks = mockPagedIterable(prMock, issue2Mock, issue4Mock, issue6Mock, issue7Mock); - when(queryIssuesNeedsReproducerBuilderMock.list()).thenReturn(issuesNeedsReproducerMocks); - var adminUser = mockUserForInspectedComments(mocks, repositoryMock, 1L, "someadmin", GHPermissionType.ADMIN); var writeUser = mockUserForInspectedComments(mocks, repositoryMock, 2L, "somewriter", @@ -429,6 +417,24 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { var noneUser = mockUserForInspectedComments(mocks, repositoryMock, 4L, "somestranger", GHPermissionType.NONE); var botUser = mockUserForInspectedComments(mocks, repositoryMock, 5L, "somebot[bot]"); + var randomReporterUser = mockUserForInspectedComments(mocks, repositoryMock, 6L, "somereporter"); + + when(repositoryMock.queryIssues()).thenReturn(queryIssuesNeedsFeedbackBuilderMock) + .thenReturn(queryIssuesNeedsReproducerBuilderMock); + var prMock = mockPullRequestForLotteryFilteredOutByRepository(mocks, 0); + var issue1Mock = mockIssueForLottery(mocks, 1, beforeCutoff, randomReporterUser); + var issue2Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 2, beforeCutoff, randomReporterUser); + var issue3Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 3, beforeCutoff, randomReporterUser); + var issue4Mock = mockIssueForLottery(mocks, 4, beforeCutoff); + var issue5Mock = mockIssueForLottery(mocks, 5, beforeCutoff, randomReporterUser); + var issue6Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 6, afterCutoff); + var issue7Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 7, beforeCutoff, randomReporterUser); + var issue8Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 8, beforeCutoff, writeUser); + var issuesNeedsFeedbackMocks = mockPagedIterable(issue1Mock, issue3Mock, issue5Mock); + when(queryIssuesNeedsFeedbackBuilderMock.list()).thenReturn(issuesNeedsFeedbackMocks); + var issuesNeedsReproducerMocks = mockPagedIterable(prMock, issue2Mock, issue4Mock, issue6Mock, issue7Mock, + issue8Mock); + when(queryIssuesNeedsReproducerBuilderMock.list()).thenReturn(issuesNeedsReproducerMocks); var needsReproducerLabelMock = mockLabel("triage/needs-reproducer"); var areaHibernateSearchLabelMock = mockLabel("area/hibernate-search"); @@ -501,6 +507,22 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { mockIssueComment(mocks, 703, botUser)); when(issue7Mock.queryComments()).thenReturn(issue7QueryCommentsBuilderMock); when(issue7QueryCommentsBuilderMock.list()).thenReturn(issue7CommentsMocks); + + // This is like issue 2, but the reporter is a team member -- so should be considered as an outsider. + var issue8Event1Mock = mockIssueEvent("created"); + var issue8Event2Mock = mockIssueEvent("labeled"); + when(issue8Event2Mock.getLabel()).thenReturn(needsReproducerLabelMock); + when(issue8Event2Mock.getCreatedAt()).thenReturn(issue8ActionLabelEvent); + var issue8Event3Mock = mockIssueEvent("labeled"); + when(issue8Event3Mock.getLabel()).thenReturn(areaHibernateSearchLabelMock); + var issue8Event4Mock = mockIssueEvent("locked"); + var issue8EventsMocks = mockPagedIterable(issue8Event1Mock, + issue8Event2Mock, issue8Event3Mock, issue8Event4Mock); + when(issue8Mock.listEvents()).thenReturn(issue8EventsMocks); + var issue8CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 801, noneUser), + mockIssueComment(mocks, 802, writeUser)); + when(issue8Mock.queryComments()).thenReturn(issue8QueryCommentsBuilderMock); + when(issue8QueryCommentsBuilderMock.list()).thenReturn(issue8CommentsMocks); }) .when(() -> { var repo = gitHubService.repository(repoRef); @@ -526,6 +548,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { verify(issue1QueryCommentsBuilderMock).since(issue1ActionLabelEvent); verify(issue2QueryCommentsBuilderMock).since(issue2ActionLabelEvent); verify(issue7QueryCommentsBuilderMock).since(issue7ActionLabelEvent); + verify(issue8QueryCommentsBuilderMock).since(issue8ActionLabelEvent); verifyNoMoreInteractions(mocks.ghObjects()); }); @@ -542,6 +565,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { Date issue1ActionLabelEvent = Date.from(cutoff.minus(1, ChronoUnit.DAYS)); Date issue2ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); Date issue7ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); + Date issue8ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); var queryIssuesNeedsReproducerBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); @@ -559,36 +583,41 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { withSettings().defaultAnswer(Answers.RETURNS_SELF)); var issue7QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); + var issue8QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { var repositoryMock = mocks.repository(repoRef.repositoryName()); + var adminUser = mockUserForInspectedComments(mocks, repositoryMock, 1L, "someadmin", + GHPermissionType.ADMIN); + var writeUser = mockUserForInspectedComments(mocks, repositoryMock, 2L, "somewriter", + GHPermissionType.WRITE); + var readUser = mockUserForInspectedComments(mocks, repositoryMock, 3L, "somereader", GHPermissionType.READ); + var noneUser = mockUserForInspectedComments(mocks, repositoryMock, 4L, "somestranger", + GHPermissionType.NONE); + var botUser = mockUserForInspectedComments(mocks, repositoryMock, 5L, "somebot[bot]"); + var randomReporterUser = mockUserForInspectedComments(mocks, repositoryMock, 6L, "somereporter"); + when(repositoryMock.queryIssues()) .thenReturn(queryIssuesNeedsFeedbackBuilderMock) .thenReturn(queryIssuesNeedsReproducerBuilderMock); // Pull requests should always be filtered out var prMock = mockPullRequestForLotteryFilteredOutByRepository(mocks, 0); - var issue1Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 1, beforeCutoff); - var issue2Mock = mockIssueForLottery(mocks, 2, beforeCutoff); - var issue3Mock = mockIssueForLottery(mocks, 3, beforeCutoff); + var issue1Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 1, beforeCutoff, randomReporterUser); + var issue2Mock = mockIssueForLottery(mocks, 2, beforeCutoff, randomReporterUser); + var issue3Mock = mockIssueForLottery(mocks, 3, beforeCutoff, randomReporterUser); var issue4Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 4, beforeCutoff); - var issue5Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 5, beforeCutoff); + var issue5Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 5, beforeCutoff, randomReporterUser); var issue6Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 6, afterCutoff); - var issue7Mock = mockIssueForLottery(mocks, 7, beforeCutoff); - var issuesNeedsFeedbackMocks = mockPagedIterable(prMock, issue2Mock, issue4Mock, issue6Mock, issue7Mock); + var issue7Mock = mockIssueForLottery(mocks, 7, beforeCutoff, randomReporterUser); + var issue8Mock = mockIssueForLottery(mocks, 8, beforeCutoff, writeUser); + var issuesNeedsFeedbackMocks = mockPagedIterable(prMock, issue2Mock, issue4Mock, issue6Mock, issue7Mock, + issue8Mock); when(queryIssuesNeedsFeedbackBuilderMock.list()).thenReturn(issuesNeedsFeedbackMocks); var issuesNeedsReproducerMocks = mockPagedIterable(issue1Mock, issue3Mock, issue5Mock); when(queryIssuesNeedsReproducerBuilderMock.list()).thenReturn(issuesNeedsReproducerMocks); - var adminUser = mockUserForInspectedComments(mocks, repositoryMock, 1L, "someadmin", - GHPermissionType.ADMIN); - var writeUser = mockUserForInspectedComments(mocks, repositoryMock, 2L, "somewriter", - GHPermissionType.WRITE); - var readUser = mockUserForInspectedComments(mocks, repositoryMock, 3L, "somereader", GHPermissionType.READ); - var noneUser = mockUserForInspectedComments(mocks, repositoryMock, 4L, "somestranger", - GHPermissionType.NONE); - var botUser = mockUserForInspectedComments(mocks, repositoryMock, 5L, "somebot[bot]"); - var needsReproducerLabelMock = mockLabel("triage/needs-reproducer"); var areaHibernateSearchLabelMock = mockLabel("area/hibernate-search"); @@ -660,6 +689,22 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { mockIssueComment(mocks, 703, botUser)); when(issue7Mock.queryComments()).thenReturn(issue7QueryCommentsBuilderMock); when(issue7QueryCommentsBuilderMock.list()).thenReturn(issue7CommentsMocks); + + // This is like issue 2, but the reporter is a team member -- so should be considered as an outsider. + var issue8Event1Mock = mockIssueEvent("created"); + var issue8Event2Mock = mockIssueEvent("labeled"); + when(issue8Event2Mock.getLabel()).thenReturn(needsReproducerLabelMock); + when(issue8Event2Mock.getCreatedAt()).thenReturn(issue8ActionLabelEvent); + var issue8Event3Mock = mockIssueEvent("labeled"); + when(issue8Event3Mock.getLabel()).thenReturn(areaHibernateSearchLabelMock); + var issue8Event4Mock = mockIssueEvent("locked"); + var issue8EventsMocks = mockPagedIterable(issue8Event1Mock, + issue8Event2Mock, issue8Event3Mock, issue8Event4Mock); + when(issue8Mock.listEvents()).thenReturn(issue8EventsMocks); + var issue8CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 801, noneUser), + mockIssueComment(mocks, 802, writeUser)); + when(issue8Mock.queryComments()).thenReturn(issue8QueryCommentsBuilderMock); + when(issue8QueryCommentsBuilderMock.list()).thenReturn(issue8CommentsMocks); }) .when(() -> { var repo = gitHubService.repository(repoRef); @@ -667,7 +712,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { assertThat(repo.issuesLastActedOnByAndLastUpdatedBefore( new LinkedHashSet<>(List.of("triage/needs-feedback", "triage/needs-reproducer")), "area/hibernate-search", IssueActionSide.OUTSIDER, cutoff)) - .containsExactlyElementsOf(stubIssueList(2, 3, 7)); + .containsExactlyElementsOf(stubIssueList(2, 3, 7, 8)); }) .then().github(mocks -> { verify(queryIssuesNeedsFeedbackBuilderMock).state(GHIssueState.OPEN); @@ -685,6 +730,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { verify(issue1QueryCommentsBuilderMock).since(issue1ActionLabelEvent); verify(issue2QueryCommentsBuilderMock).since(issue2ActionLabelEvent); verify(issue7QueryCommentsBuilderMock).since(issue7ActionLabelEvent); + verify(issue8QueryCommentsBuilderMock).since(issue8ActionLabelEvent); verifyNoMoreInteractions(mocks.ghObjects()); }); diff --git a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java index b7fe6bb..2ea16c0 100644 --- a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java +++ b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java @@ -76,6 +76,18 @@ public static GHIssue mockIssueForLottery(GitHubMockContext context, int number, return mock; } + public static GHIssue mockIssueForLottery(GitHubMockContext context, int number, Date updatedAt, GHUser reporter) + throws IOException { + GHIssue mock = context.issue(10000L + number); + when(mock.isPullRequest()).thenReturn(false); + when(mock.getNumber()).thenReturn(number); + when(mock.getTitle()).thenReturn("Title for issue " + number); + when(mock.getHtmlUrl()).thenReturn(url(number)); + when(mock.getUpdatedAt()).thenReturn(updatedAt); + when(mock.getUser()).thenReturn(reporter); + return mock; + } + public static GHIssue mockIssueForLotteryFilteredOutByRepository(GitHubMockContext context, int number, Date updatedAt) throws IOException { GHIssue mock = context.issue(10000L + number); @@ -90,6 +102,16 @@ public static GHPullRequest mockPullRequestForLotteryFilteredOutByRepository(Git return mock; } + public static GHIssue mockIssueForLotteryFilteredOutByRepository(GitHubMockContext context, int number, Date updatedAt, + GHUser reporter) + throws IOException { + GHIssue mock = context.issue(10000L + number); + when(mock.isPullRequest()).thenReturn(false); + when(mock.getUpdatedAt()).thenReturn(updatedAt); + when(mock.getUser()).thenReturn(reporter); + return mock; + } + public static GHIssue mockIssueForNotification(GitHubMockContext context, long id, String title) { GHIssue mock = context.issue(id); when(mock.getTitle()).thenReturn(title); From 2ce45757b3047e30aa60dd4d14e06674b17fc68d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Mon, 9 Dec 2024 17:00:56 +0100 Subject: [PATCH 5/5] Use the "search issues" API instead of the "query issues" API Because it's (much) more flexible. --- .../lottery/github/GitHubRepository.java | 83 ++-- .../lottery/github/GitHubSearchClauses.java | 39 ++ src/main/resources/application.properties | 1 + .../github/lottery/GitHubServiceTest.java | 427 +++++++++--------- .../github/lottery/util/MockHelper.java | 24 +- 5 files changed, 291 insertions(+), 283 deletions(-) create mode 100644 src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java index dfb6d59..8df87c7 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -21,7 +21,7 @@ import org.kohsuke.github.GHIssueComment; import org.kohsuke.github.GHIssueCommentQueryBuilder; import org.kohsuke.github.GHIssueEvent; -import org.kohsuke.github.GHIssueQueryBuilder; +import org.kohsuke.github.GHIssueSearchBuilder; import org.kohsuke.github.GHIssueState; import org.kohsuke.github.GHRepository; import org.kohsuke.github.GHUser; @@ -94,6 +94,12 @@ private GHRepository repository() throws IOException { return repository; } + private GHIssueSearchBuilder searchIssues() { + return client().searchIssues() + .q(GitHubSearchClauses.repo(ref)) + .q(GitHubSearchClauses.isIssue()); + } + private DynamicGraphQLClient graphQLClient() { if (graphQLClient == null) { graphQLClient = clientProvider.getInstallationGraphQLClient(ref.installationRef().installationId()); @@ -111,17 +117,15 @@ public Optional fetchLotteryConfig() throws IOException { * * @param updatedBefore An instant; all returned issues must have been last updated before that instant. * @return A lazily populated stream of matching issues. - * @throws IOException In case of I/O failure. * @throws java.io.UncheckedIOException In case of I/O failure. */ - public Stream issuesLastUpdatedBefore(Instant updatedBefore) throws IOException { - return toStream(repository().queryIssues() - .state(GHIssueState.OPEN) - .sort(GHIssueQueryBuilder.Sort.UPDATED) - .direction(GHDirection.DESC) + public Stream issuesLastUpdatedBefore(Instant updatedBefore) { + return toStream(searchIssues() + .isOpen() + .q(GitHubSearchClauses.updatedBefore(updatedBefore)) + .sort(GHIssueSearchBuilder.Sort.UPDATED) + .order(GHDirection.DESC) .list()) - .filter(notPullRequest()) - .filter(updatedBefore(updatedBefore)) .map(toIssueRecord()); } @@ -131,17 +135,16 @@ public Stream issuesLastUpdatedBefore(Instant updatedBefore) throws IOExc * @param label A GitHub label; if non-null, all returned issues must have been assigned that label. * @param updatedBefore An instant; all returned issues must have been last updated before that instant. * @return A lazily populated stream of matching issues. - * @throws IOException In case of I/O failure. * @throws java.io.UncheckedIOException In case of I/O failure. */ - public Stream issuesWithLabelLastUpdatedBefore(String label, Instant updatedBefore) throws IOException { - return toStream(repository().queryIssues().label(label) - .state(GHIssueState.OPEN) - .sort(GHIssueQueryBuilder.Sort.UPDATED) - .direction(GHDirection.DESC) + public Stream issuesWithLabelLastUpdatedBefore(String label, Instant updatedBefore) { + return toStream(searchIssues() + .isOpen() + .q(GitHubSearchClauses.label(label)) + .q(GitHubSearchClauses.updatedBefore(updatedBefore)) + .sort(GHIssueSearchBuilder.Sort.UPDATED) + .order(GHDirection.DESC) .list()) - .filter(notPullRequest()) - .filter(updatedBefore(updatedBefore)) .map(toIssueRecord()); } @@ -155,35 +158,21 @@ public Stream issuesWithLabelLastUpdatedBefore(String label, Instant upda * This label is not relevant to determining the last action. * @param updatedBefore An instant; all returned issues must have been last updated before that instant. * @return A lazily populated stream of matching issues. - * @throws IOException In case of I/O failure. * @throws java.io.UncheckedIOException In case of I/O failure. */ public Stream issuesLastActedOnByAndLastUpdatedBefore(Set initialActionLabels, String filterLabel, - IssueActionSide lastActionSide, Instant updatedBefore) throws IOException { - var theRepository = repository(); - var streams = initialActionLabels.stream() - .map(initialActionLabel -> toStream(theRepository.queryIssues() - .label(initialActionLabel) - .label(filterLabel) - .state(GHIssueState.OPEN) - .sort(GHIssueQueryBuilder.Sort.UPDATED) - .direction(GHDirection.DESC) - .list()) - .filter(notPullRequest()) - .filter(updatedBefore(updatedBefore)) - .filter(uncheckedIO((GHIssue ghIssue) -> lastActionSide - .equals(lastActionSide(ghIssue, initialActionLabels)))::apply) - .map(toIssueRecord())) - .toList(); - return Streams.interleave(streams); - } - - private Predicate updatedBefore(Instant updatedBefore) { - return uncheckedIO((GHIssue ghIssue) -> ghIssue.getUpdatedAt().toInstant().isBefore(updatedBefore))::apply; - } - - private Predicate notPullRequest() { - return (GHIssue ghIssue) -> !ghIssue.isPullRequest(); + IssueActionSide lastActionSide, Instant updatedBefore) { + return toStream(searchIssues() + .isOpen() + .q(GitHubSearchClauses.anyLabel(initialActionLabels)) + .q(GitHubSearchClauses.label(filterLabel)) + .q(GitHubSearchClauses.updatedBefore(updatedBefore)) + .sort(GHIssueSearchBuilder.Sort.UPDATED) + .order(GHDirection.DESC) + .list()) + .filter(uncheckedIO((GHIssue ghIssue) -> lastActionSide + .equals(lastActionSide(ghIssue, initialActionLabels)))::apply) + .map(toIssueRecord()); } private IssueActionSide lastActionSide(GHIssue ghIssue, Set initialActionLabels) throws IOException { @@ -312,12 +301,12 @@ public void comment(String topicSuffix, String markdownBody) } private Stream getDedicatedIssues() throws IOException { - var builder = repository().queryIssues().creator(appLogin()); + var builder = searchIssues() + .q(GitHubSearchClauses.author(appLogin())); if (ref.assignee() != null) { - builder.assignee(ref.assignee()); + builder.q(GitHubSearchClauses.assignee(ref.assignee())); } - builder.state(GHIssueState.ALL); - return Streams.toStream(builder.list()) + return toStream(builder.list()) .filter(ref.expectedSuffixStart() != null ? issue -> issue.getTitle().startsWith(ref.topic() + ref.expectedSuffixStart()) // Try exact match in this case to avoid confusion if there are two issues and one is diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java b/src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java new file mode 100644 index 0000000..70ef8b0 --- /dev/null +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java @@ -0,0 +1,39 @@ +package io.quarkus.github.lottery.github; + +import java.time.Instant; +import java.time.ZoneOffset; +import java.util.Set; + +public final class GitHubSearchClauses { + + private GitHubSearchClauses() { + } + + public static String repo(GitHubRepositoryRef ref) { + return "repo:" + ref.repositoryName(); + } + + public static String isIssue() { + return "is:issue"; + } + + public static String anyLabel(Set labels) { + return label(String.join(",", labels)); + } + + public static String label(String label) { + return "label:" + label; + } + + public static String updatedBefore(Instant updatedBefore) { + return "updated:<" + updatedBefore.atOffset(ZoneOffset.UTC).toLocalDateTime().toString(); + } + + public static String author(String author) { + return "author:" + author; + } + + public static String assignee(String assignee) { + return "assignee:" + assignee; + } +} diff --git a/src/main/resources/application.properties b/src/main/resources/application.properties index ef445ce..0e8463a 100644 --- a/src/main/resources/application.properties +++ b/src/main/resources/application.properties @@ -13,6 +13,7 @@ quarkus.info.enabled=true %dev.quarkus.scheduler.enabled=false %dev.quarkus.log.min-level=TRACE %dev.quarkus.log.category."io.quarkus.github.lottery".level=TRACE +%dev.quarkus.log.category."org.kohsuke.github.GitHubClient".level=TRACE %prod.quarkus.openshift.labels."app"=quarkus-github-lottery # Renew the SSL certificate automatically diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index af53aa1..682e3d2 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -8,7 +8,6 @@ import static io.quarkus.github.lottery.util.MockHelper.mockIssueForNotification; import static io.quarkus.github.lottery.util.MockHelper.mockLabel; import static io.quarkus.github.lottery.util.MockHelper.mockPagedIterable; -import static io.quarkus.github.lottery.util.MockHelper.mockPullRequestForLotteryFilteredOutByRepository; import static io.quarkus.github.lottery.util.MockHelper.mockUserForInspectedComments; import static io.quarkus.github.lottery.util.MockHelper.stubIssueList; import static org.assertj.core.api.Assertions.assertThat; @@ -49,7 +48,7 @@ import org.kohsuke.github.GHIssueComment; import org.kohsuke.github.GHIssueCommentQueryBuilder; import org.kohsuke.github.GHIssueEvent; -import org.kohsuke.github.GHIssueQueryBuilder; +import org.kohsuke.github.GHIssueSearchBuilder; import org.kohsuke.github.GHIssueState; import org.kohsuke.github.GHPermissionType; import org.kohsuke.github.GHRepository; @@ -96,7 +95,7 @@ void setup() { void listRepositories() throws IOException { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { @@ -130,7 +129,7 @@ void listRepositories() throws IOException { .containsExactlyInAnyOrder(repoRef); }) .then().github(mocks -> { - verifyNoMoreInteractions(queryIssuesBuilderMock); + verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -299,24 +298,20 @@ void issuesLastUpdatedBefore() throws IOException { Instant now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC); Instant cutoff = now.minus(1, ChronoUnit.DAYS); - Date beforeCutoff = Date.from(cutoff.minus(1, ChronoUnit.DAYS)); - Date afterCutoff = Date.from(cutoff.plus(1, ChronoUnit.HOURS)); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); - - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); - var prMock = mockPullRequestForLotteryFilteredOutByRepository(mocks, 0); - var issue1Mock = mockIssueForLottery(mocks, 1, beforeCutoff); - var issue2Mock = mockIssueForLottery(mocks, 3, beforeCutoff); - var issue3Mock = mockIssueForLottery(mocks, 2, beforeCutoff); - var issue4Mock = mockIssueForLottery(mocks, 4, beforeCutoff); - var issue5Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 5, afterCutoff); - var issuesMocks = mockPagedIterable(prMock, issue1Mock, issue2Mock, issue3Mock, issue4Mock, issue5Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + var clientMock = mocks.installationClient(installationRef.installationId()); + + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); + var issue1Mock = mockIssueForLottery(mocks, 1); + var issue2Mock = mockIssueForLottery(mocks, 3); + var issue3Mock = mockIssueForLottery(mocks, 2); + var issue4Mock = mockIssueForLottery(mocks, 4); + var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock, issue3Mock, issue4Mock); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); }) .when(() -> { var repo = gitHubService.repository(repoRef); @@ -325,10 +320,13 @@ void issuesLastUpdatedBefore() throws IOException { .containsExactlyElementsOf(stubIssueList(1, 3, 2, 4)); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).state(GHIssueState.OPEN); - verify(queryIssuesBuilderMock).sort(GHIssueQueryBuilder.Sort.UPDATED); - verify(queryIssuesBuilderMock).direction(GHDirection.DESC); - verifyNoMoreInteractions(queryIssuesBuilderMock); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).isOpen(); + verify(searchIssuesBuilderMock).q("updated:<2017-11-05T06:00"); + verify(searchIssuesBuilderMock).sort(GHIssueSearchBuilder.Sort.UPDATED); + verify(searchIssuesBuilderMock).order(GHDirection.DESC); + verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -339,24 +337,20 @@ void issuesWithLabelLastUpdatedBefore() throws IOException { Instant now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC); Instant cutoff = now.minus(1, ChronoUnit.DAYS); - Date beforeCutoff = Date.from(cutoff.minus(1, ChronoUnit.DAYS)); - Date afterCutoff = Date.from(cutoff.plus(1, ChronoUnit.HOURS)); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); - - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); - var prMock = mockPullRequestForLotteryFilteredOutByRepository(mocks, 0); - var issue1Mock = mockIssueForLottery(mocks, 1, beforeCutoff); - var issue2Mock = mockIssueForLottery(mocks, 3, beforeCutoff); - var issue3Mock = mockIssueForLottery(mocks, 2, beforeCutoff); - var issue4Mock = mockIssueForLottery(mocks, 4, beforeCutoff); - var issue5Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 5, afterCutoff); - var issuesMocks = mockPagedIterable(prMock, issue1Mock, issue2Mock, issue3Mock, issue4Mock, issue5Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + var clientMock = mocks.installationClient(installationRef.installationId()); + + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); + var issue1Mock = mockIssueForLottery(mocks, 1); + var issue2Mock = mockIssueForLottery(mocks, 3); + var issue3Mock = mockIssueForLottery(mocks, 2); + var issue4Mock = mockIssueForLottery(mocks, 4); + var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock, issue3Mock, issue4Mock); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); }) .when(() -> { var repo = gitHubService.repository(repoRef); @@ -365,11 +359,14 @@ void issuesWithLabelLastUpdatedBefore() throws IOException { .containsExactlyElementsOf(stubIssueList(1, 3, 2, 4)); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).state(GHIssueState.OPEN); - verify(queryIssuesBuilderMock).sort(GHIssueQueryBuilder.Sort.UPDATED); - verify(queryIssuesBuilderMock).direction(GHDirection.DESC); - verify(queryIssuesBuilderMock).label("triage/needs-triage"); - verifyNoMoreInteractions(queryIssuesBuilderMock); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).isOpen(); + verify(searchIssuesBuilderMock).sort(GHIssueSearchBuilder.Sort.UPDATED); + verify(searchIssuesBuilderMock).order(GHDirection.DESC); + verify(searchIssuesBuilderMock).q("label:triage/needs-triage"); + verify(searchIssuesBuilderMock).q("updated:<2017-11-05T06:00"); + verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -380,16 +377,12 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { Instant now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC); Instant cutoff = now.minus(1, ChronoUnit.DAYS); - Date beforeCutoff = Date.from(cutoff.minus(1, ChronoUnit.DAYS)); - Date afterCutoff = Date.from(cutoff.plus(1, ChronoUnit.HOURS)); Date issue1ActionLabelEvent = Date.from(cutoff.minus(1, ChronoUnit.DAYS)); Date issue2ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); Date issue7ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); Date issue8ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); - var queryIssuesNeedsFeedbackBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, - withSettings().defaultAnswer(Answers.RETURNS_SELF)); - var queryIssuesNeedsReproducerBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var issue1QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); @@ -407,6 +400,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { + var clientMock = mocks.installationClient(installationRef.installationId()); var repositoryMock = mocks.repository(repoRef.repositoryName()); var adminUser = mockUserForInspectedComments(mocks, repositoryMock, 1L, "someadmin", @@ -419,22 +413,21 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { var botUser = mockUserForInspectedComments(mocks, repositoryMock, 5L, "somebot[bot]"); var randomReporterUser = mockUserForInspectedComments(mocks, repositoryMock, 6L, "somereporter"); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesNeedsFeedbackBuilderMock) - .thenReturn(queryIssuesNeedsReproducerBuilderMock); - var prMock = mockPullRequestForLotteryFilteredOutByRepository(mocks, 0); - var issue1Mock = mockIssueForLottery(mocks, 1, beforeCutoff, randomReporterUser); - var issue2Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 2, beforeCutoff, randomReporterUser); - var issue3Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 3, beforeCutoff, randomReporterUser); - var issue4Mock = mockIssueForLottery(mocks, 4, beforeCutoff); - var issue5Mock = mockIssueForLottery(mocks, 5, beforeCutoff, randomReporterUser); - var issue6Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 6, afterCutoff); - var issue7Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 7, beforeCutoff, randomReporterUser); - var issue8Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 8, beforeCutoff, writeUser); - var issuesNeedsFeedbackMocks = mockPagedIterable(issue1Mock, issue3Mock, issue5Mock); - when(queryIssuesNeedsFeedbackBuilderMock.list()).thenReturn(issuesNeedsFeedbackMocks); - var issuesNeedsReproducerMocks = mockPagedIterable(prMock, issue2Mock, issue4Mock, issue6Mock, issue7Mock, - issue8Mock); - when(queryIssuesNeedsReproducerBuilderMock.list()).thenReturn(issuesNeedsReproducerMocks); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock) + .thenReturn(searchIssuesBuilderMock); + var issue1Mock = mockIssueForLottery(mocks, 1, randomReporterUser); + var issue2Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 2, randomReporterUser); + var issue3Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 3, randomReporterUser); + var issue4Mock = mockIssueForLottery(mocks, 4); + var issue5Mock = mockIssueForLottery(mocks, 5, randomReporterUser); + // issue6 used to be one after the cutoff, filtered out on the client side, + // but that's handled server-side now. + // Keeping this gap in numbering to avoid an even worse diff. + var issue7Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 7, randomReporterUser); + var issue8Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 8, writeUser); + var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock, issue3Mock, issue4Mock, issue5Mock, + issue7Mock, issue8Mock); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); var needsReproducerLabelMock = mockLabel("triage/needs-reproducer"); var areaHibernateSearchLabelMock = mockLabel("area/hibernate-search"); @@ -533,17 +526,12 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { .containsExactlyElementsOf(stubIssueList(1, 4, 5)); }) .then().github(mocks -> { - verify(queryIssuesNeedsFeedbackBuilderMock).state(GHIssueState.OPEN); - verify(queryIssuesNeedsFeedbackBuilderMock).sort(GHIssueQueryBuilder.Sort.UPDATED); - verify(queryIssuesNeedsFeedbackBuilderMock).direction(GHDirection.DESC); - verify(queryIssuesNeedsFeedbackBuilderMock).label("triage/needs-feedback"); - verify(queryIssuesNeedsFeedbackBuilderMock).label("area/hibernate-search"); - verify(queryIssuesNeedsReproducerBuilderMock).state(GHIssueState.OPEN); - verify(queryIssuesNeedsReproducerBuilderMock).sort(GHIssueQueryBuilder.Sort.UPDATED); - verify(queryIssuesNeedsReproducerBuilderMock).direction(GHDirection.DESC); - verify(queryIssuesNeedsReproducerBuilderMock).label("triage/needs-reproducer"); - verify(queryIssuesNeedsReproducerBuilderMock).label("area/hibernate-search"); - verifyNoMoreInteractions(queryIssuesNeedsReproducerBuilderMock, queryIssuesNeedsFeedbackBuilderMock); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).isOpen(); + verify(searchIssuesBuilderMock).sort(GHIssueSearchBuilder.Sort.UPDATED); + verify(searchIssuesBuilderMock).order(GHDirection.DESC); + verify(searchIssuesBuilderMock).q("label:triage/needs-feedback,triage/needs-reproducer"); + verify(searchIssuesBuilderMock).q("label:area/hibernate-search"); verify(issue1QueryCommentsBuilderMock).since(issue1ActionLabelEvent); verify(issue2QueryCommentsBuilderMock).since(issue2ActionLabelEvent); @@ -560,16 +548,12 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { Instant now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC); Instant cutoff = now.minus(1, ChronoUnit.DAYS); - Date beforeCutoff = Date.from(cutoff.minus(1, ChronoUnit.DAYS)); - Date afterCutoff = Date.from(cutoff.plus(1, ChronoUnit.HOURS)); Date issue1ActionLabelEvent = Date.from(cutoff.minus(1, ChronoUnit.DAYS)); Date issue2ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); Date issue7ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); Date issue8ActionLabelEvent = Date.from(cutoff.minus(2, ChronoUnit.DAYS)); - var queryIssuesNeedsReproducerBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, - withSettings().defaultAnswer(Answers.RETURNS_SELF)); - var queryIssuesNeedsFeedbackBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var issue1QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); @@ -587,6 +571,7 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { + var clientMock = mocks.installationClient(installationRef.installationId()); var repositoryMock = mocks.repository(repoRef.repositoryName()); var adminUser = mockUserForInspectedComments(mocks, repositoryMock, 1L, "someadmin", @@ -599,24 +584,21 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { var botUser = mockUserForInspectedComments(mocks, repositoryMock, 5L, "somebot[bot]"); var randomReporterUser = mockUserForInspectedComments(mocks, repositoryMock, 6L, "somereporter"); - when(repositoryMock.queryIssues()) - .thenReturn(queryIssuesNeedsFeedbackBuilderMock) - .thenReturn(queryIssuesNeedsReproducerBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); // Pull requests should always be filtered out - var prMock = mockPullRequestForLotteryFilteredOutByRepository(mocks, 0); - var issue1Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 1, beforeCutoff, randomReporterUser); - var issue2Mock = mockIssueForLottery(mocks, 2, beforeCutoff, randomReporterUser); - var issue3Mock = mockIssueForLottery(mocks, 3, beforeCutoff, randomReporterUser); - var issue4Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 4, beforeCutoff); - var issue5Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 5, beforeCutoff, randomReporterUser); - var issue6Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 6, afterCutoff); - var issue7Mock = mockIssueForLottery(mocks, 7, beforeCutoff, randomReporterUser); - var issue8Mock = mockIssueForLottery(mocks, 8, beforeCutoff, writeUser); - var issuesNeedsFeedbackMocks = mockPagedIterable(prMock, issue2Mock, issue4Mock, issue6Mock, issue7Mock, - issue8Mock); - when(queryIssuesNeedsFeedbackBuilderMock.list()).thenReturn(issuesNeedsFeedbackMocks); - var issuesNeedsReproducerMocks = mockPagedIterable(issue1Mock, issue3Mock, issue5Mock); - when(queryIssuesNeedsReproducerBuilderMock.list()).thenReturn(issuesNeedsReproducerMocks); + var issue1Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 1, randomReporterUser); + var issue2Mock = mockIssueForLottery(mocks, 2, randomReporterUser); + var issue3Mock = mockIssueForLottery(mocks, 3, randomReporterUser); + var issue4Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 4); + var issue5Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 5, randomReporterUser); + // issue6 used to be one after the cutoff, filtered out on the client side, + // but that's handled server-side now. + // Keeping this gap in numbering to avoid an even worse diff. + var issue7Mock = mockIssueForLottery(mocks, 7, randomReporterUser); + var issue8Mock = mockIssueForLottery(mocks, 8, writeUser); + var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock, issue3Mock, issue4Mock, issue5Mock, + issue7Mock, issue8Mock); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); var needsReproducerLabelMock = mockLabel("triage/needs-reproducer"); var areaHibernateSearchLabelMock = mockLabel("area/hibernate-search"); @@ -715,17 +697,12 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { .containsExactlyElementsOf(stubIssueList(2, 3, 7, 8)); }) .then().github(mocks -> { - verify(queryIssuesNeedsFeedbackBuilderMock).state(GHIssueState.OPEN); - verify(queryIssuesNeedsFeedbackBuilderMock).sort(GHIssueQueryBuilder.Sort.UPDATED); - verify(queryIssuesNeedsFeedbackBuilderMock).direction(GHDirection.DESC); - verify(queryIssuesNeedsFeedbackBuilderMock).label("triage/needs-feedback"); - verify(queryIssuesNeedsFeedbackBuilderMock).label("area/hibernate-search"); - verify(queryIssuesNeedsReproducerBuilderMock).state(GHIssueState.OPEN); - verify(queryIssuesNeedsReproducerBuilderMock).sort(GHIssueQueryBuilder.Sort.UPDATED); - verify(queryIssuesNeedsReproducerBuilderMock).direction(GHDirection.DESC); - verify(queryIssuesNeedsReproducerBuilderMock).label("triage/needs-reproducer"); - verify(queryIssuesNeedsReproducerBuilderMock).label("area/hibernate-search"); - verifyNoMoreInteractions(queryIssuesNeedsReproducerBuilderMock, queryIssuesNeedsFeedbackBuilderMock); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).isOpen(); + verify(searchIssuesBuilderMock).sort(GHIssueSearchBuilder.Sort.UPDATED); + verify(searchIssuesBuilderMock).order(GHDirection.DESC); + verify(searchIssuesBuilderMock).q("label:triage/needs-feedback,triage/needs-reproducer"); + verify(searchIssuesBuilderMock).q("label:area/hibernate-search"); verify(issue1QueryCommentsBuilderMock).since(issue1ActionLabelEvent); verify(issue2QueryCommentsBuilderMock).since(issue2ActionLabelEvent); @@ -741,18 +718,18 @@ void topic_extractComments_dedicatedIssueDoesNotExist() throws Exception { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var since = LocalDateTime.of(2017, 11, 6, 19, 0).toInstant(ZoneOffset.UTC); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issue2Mock = mockIssueForNotification(mocks, 2, "Another unrelated issue"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); }) .when(() -> { var repo = gitHubService.repository(repoRef); @@ -762,9 +739,10 @@ void topic_extractComments_dedicatedIssueDoesNotExist() throws Exception { .isEmpty(); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); - verifyNoMoreInteractions(queryIssuesBuilderMock); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); + verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -774,18 +752,19 @@ void topic_extractComments_dedicatedIssueDoesNotExist_withConfusingOther() throw var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var since = LocalDateTime.of(2017, 11, 6, 19, 0).toInstant(ZoneOffset.UTC); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()) + .thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "Lottery history for quarkusio/quarkusio.github.io"); var issue2Mock = mockIssueForNotification(mocks, 2, "Another unrelated issue"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); }) .when(() -> { var repo = gitHubService.repository(repoRef); @@ -795,9 +774,10 @@ void topic_extractComments_dedicatedIssueDoesNotExist_withConfusingOther() throw .isEmpty(); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); - verifyNoMoreInteractions(queryIssuesBuilderMock); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); + verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -807,20 +787,20 @@ void topic_extractComments_dedicatedIssueExists_appCommentsDoNotExist() throws E var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var since = LocalDateTime.of(2017, 11, 6, 19, 0).toInstant(ZoneOffset.UTC); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var queryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issue2Mock = mockIssueForNotification(mocks, 2, "Lottery history for quarkusio/quarkus"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); var someoneElseMock = mocks.ghObject(GHUser.class, 2L); when(someoneElseMock.getLogin()).thenReturn("yrodiere"); @@ -838,11 +818,12 @@ void topic_extractComments_dedicatedIssueExists_appCommentsDoNotExist() throws E .isEmpty(); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); verify(queryCommentsBuilderMock).since(Date.from(since)); - verifyNoMoreInteractions(queryIssuesBuilderMock, queryCommentsBuilderMock); + verifyNoMoreInteractions(searchIssuesBuilderMock, queryCommentsBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -852,20 +833,20 @@ void topic_extractComments_dedicatedIssueExists_appCommentsExist_allTooOld() thr var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var since = LocalDateTime.of(2017, 11, 6, 19, 0).toInstant(ZoneOffset.UTC); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var queryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issue2Mock = mockIssueForNotification(mocks, 2, "Lottery history for quarkusio/quarkus"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); when(issue2Mock.queryComments()).thenReturn(queryCommentsBuilderMock); PagedSearchIterable issue2CommentMocks = mockPagedIterable(); @@ -879,11 +860,12 @@ void topic_extractComments_dedicatedIssueExists_appCommentsExist_allTooOld() thr .isEmpty(); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); verify(queryCommentsBuilderMock).since(Date.from(since)); - verifyNoMoreInteractions(queryIssuesBuilderMock, queryCommentsBuilderMock); + verifyNoMoreInteractions(searchIssuesBuilderMock, queryCommentsBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -893,20 +875,20 @@ void topic_extractComments_dedicatedIssueExists_appCommentsExist() throws Except var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var since = LocalDateTime.of(2017, 11, 6, 19, 0).toInstant(ZoneOffset.UTC); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var queryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issue2Mock = mockIssueForNotification(mocks, 2, "Lottery history for quarkusio/quarkus"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); var mySelfMock = mocks.ghObject(GHUser.class, 1L); when(mySelfMock.getLogin()).thenReturn(installationRef.appLogin()); @@ -928,11 +910,12 @@ void topic_extractComments_dedicatedIssueExists_appCommentsExist() throws Except .containsExactly("issue2Comment1Mock#body", "issue2Comment2Mock#body"); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); verify(queryCommentsBuilderMock).since(Date.from(since)); - verifyNoMoreInteractions(queryIssuesBuilderMock, queryCommentsBuilderMock); + verifyNoMoreInteractions(searchIssuesBuilderMock, queryCommentsBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -942,20 +925,20 @@ void topic_extractComments_dedicatedIssueExists_appCommentsExist_withConfusingOt var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); var since = LocalDateTime.of(2017, 11, 6, 19, 0).toInstant(ZoneOffset.UTC); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var queryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "Lottery history for quarkusio/quarkusio.github.io"); var issue2Mock = mockIssueForNotification(mocks, 2, "Lottery history for quarkusio/quarkus"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); var mySelfMock = mocks.ghObject(GHUser.class, 1L); when(mySelfMock.getLogin()).thenReturn(installationRef.appLogin()); @@ -977,11 +960,12 @@ void topic_extractComments_dedicatedIssueExists_appCommentsExist_withConfusingOt .containsExactly("issue2Comment1Mock#body", "issue2Comment2Mock#body"); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); verify(queryCommentsBuilderMock).since(Date.from(since)); - verifyNoMoreInteractions(queryIssuesBuilderMock, queryCommentsBuilderMock); + verifyNoMoreInteractions(searchIssuesBuilderMock, queryCommentsBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -996,21 +980,21 @@ void topic_comment_dedicatedIssueExists_open() throws Exception { var clockMock = Clock.fixed(now, ZoneOffset.UTC); QuarkusMock.installMockForType(clockMock, Clock.class); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var queryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issue2Mock = mockIssueForNotification(mocks, 2, "yrodiere's report for quarkusio/quarkus (updated 2017-11-05T06:00:00Z)"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); when(issue2Mock.getState()).thenReturn(GHIssueState.OPEN); @@ -1038,9 +1022,10 @@ void topic_comment_dedicatedIssueExists_open() throws Exception { .comment(" (updated 2017-11-06T06:00:00Z)", "Some content"); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).assignee("yrodiere"); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); + verify(searchIssuesBuilderMock).q("assignee:yrodiere"); verify(queryCommentsBuilderMock).since(Date.from(now.minus(21, ChronoUnit.DAYS))); var mapCaptor = ArgumentCaptor.forClass(Map.class); @@ -1051,7 +1036,7 @@ void topic_comment_dedicatedIssueExists_open() throws Exception { verify(mocks.issue(2)).setBody("Dedicated issue body"); verify(mocks.issue(2)).comment("Some content"); - verifyNoMoreInteractions(queryIssuesBuilderMock); + verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); assertThat(mapCaptor.getValue()).containsValue(commentToMinimizeNodeId); @@ -1068,20 +1053,20 @@ void topic_comment_dedicatedIssueExists_noTopicSuffix() throws Exception { var clockMock = Clock.fixed(now, ZoneOffset.UTC); QuarkusMock.installMockForType(clockMock, Clock.class); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var queryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issue2Mock = mockIssueForNotification(mocks, 2, "Lottery history for quarkusio/quarkus"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); when(issue2Mock.getState()).thenReturn(GHIssueState.OPEN); @@ -1109,8 +1094,9 @@ void topic_comment_dedicatedIssueExists_noTopicSuffix() throws Exception { .comment("", "Some content"); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); verify(queryCommentsBuilderMock).since(Date.from(now.minus(21, ChronoUnit.DAYS))); var mapCaptor = ArgumentCaptor.forClass(Map.class); @@ -1120,7 +1106,7 @@ void topic_comment_dedicatedIssueExists_noTopicSuffix() throws Exception { verify(mocks.issue(2)).setBody("Dedicated issue body"); verify(mocks.issue(2)).comment("Some content"); - verifyNoMoreInteractions(messageFormatterMock, queryIssuesBuilderMock); + verifyNoMoreInteractions(messageFormatterMock, searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); assertThat(mapCaptor.getValue()).containsValue(commentToMinimizeNodeId); @@ -1137,20 +1123,20 @@ void topic_comment_dedicatedIssueExists_withConfusingOther() throws Exception { var clockMock = Clock.fixed(now, ZoneOffset.UTC); QuarkusMock.installMockForType(clockMock, Clock.class); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var queryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "Lottery history for quarkusio/quarkusio.github.io"); var issue2Mock = mockIssueForNotification(mocks, 2, "Lottery history for quarkusio/quarkus"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); when(issue2Mock.getState()).thenReturn(GHIssueState.OPEN); @@ -1178,8 +1164,9 @@ void topic_comment_dedicatedIssueExists_withConfusingOther() throws Exception { .comment("", "Some content"); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); verify(queryCommentsBuilderMock).since(Date.from(now.minus(21, ChronoUnit.DAYS))); var mapCaptor = ArgumentCaptor.forClass(Map.class); @@ -1189,7 +1176,7 @@ void topic_comment_dedicatedIssueExists_withConfusingOther() throws Exception { verify(mocks.issue(2)).setBody("Dedicated issue body"); verify(mocks.issue(2)).comment("Some content"); - verifyNoMoreInteractions(messageFormatterMock, queryIssuesBuilderMock); + verifyNoMoreInteractions(messageFormatterMock, searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); assertThat(mapCaptor.getValue()).containsValue(commentToMinimizeNodeId); @@ -1206,21 +1193,21 @@ void topic_comment_dedicatedIssueExists_closed() throws Exception { var clockMock = Clock.fixed(now, ZoneOffset.UTC); QuarkusMock.installMockForType(clockMock, Clock.class); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var queryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issue2Mock = mockIssueForNotification(mocks, 2, "yrodiere's report for quarkusio/quarkus (updated 2017-11-05T06:00:00Z)"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); when(issue2Mock.getState()).thenReturn(GHIssueState.CLOSED); @@ -1248,9 +1235,10 @@ void topic_comment_dedicatedIssueExists_closed() throws Exception { .comment(" (updated 2017-11-06T06:00:00Z)", "Some content"); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).assignee("yrodiere"); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); + verify(searchIssuesBuilderMock).q("assignee:yrodiere"); verify(mocks.issue(2)).setTitle("yrodiere's report for quarkusio/quarkus (updated 2017-11-06T06:00:00Z)"); verify(mocks.issue(2)).reopen(); @@ -1263,7 +1251,7 @@ void topic_comment_dedicatedIssueExists_closed() throws Exception { verify(mocks.issue(2)).setBody("Dedicated issue body"); verify(mocks.issue(2)).comment("Some content"); - verifyNoMoreInteractions(messageFormatterMock, queryIssuesBuilderMock); + verifyNoMoreInteractions(messageFormatterMock, searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); assertThat(mapCaptor.getValue()).containsValue(commentToMinimizeNodeId); @@ -1273,19 +1261,20 @@ void topic_comment_dedicatedIssueExists_closed() throws Exception { @Test void topic_comment_dedicatedIssueDoesNotExist() throws IOException { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var issueBuilderMock = Mockito.mock(GHIssueBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { + var clientMock = mocks.installationClient(installationRef.installationId()); var repositoryMock = mocks.repository(repoRef.repositoryName()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issuesMocks = mockPagedIterable(issue1Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); when(repositoryMock.createIssue(any())).thenReturn(issueBuilderMock); var issue2Mock = mocks.issue(2); @@ -1304,16 +1293,17 @@ void topic_comment_dedicatedIssueDoesNotExist() throws IOException { .then().github(mocks -> { var repositoryMock = mocks.repository(repoRef.repositoryName()); - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).assignee("yrodiere"); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); + verify(searchIssuesBuilderMock).q("assignee:yrodiere"); verify(repositoryMock) .createIssue("yrodiere's report for quarkusio/quarkus (updated 2017-11-06T06:00:00Z)"); verify(issueBuilderMock).assignee("yrodiere"); verify(issueBuilderMock).body("Dedicated issue body"); verify(mocks.issue(2)).comment("Some content"); - verifyNoMoreInteractions(messageFormatterMock, queryIssuesBuilderMock, issueBuilderMock); + verifyNoMoreInteractions(messageFormatterMock, searchIssuesBuilderMock, issueBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -1321,19 +1311,20 @@ void topic_comment_dedicatedIssueDoesNotExist() throws IOException { @Test void topic_comment_dedicatedIssueDoesNotExist_withConfusingOther() throws IOException { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); var issueBuilderMock = Mockito.mock(GHIssueBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { + var clientMock = mocks.installationClient(installationRef.installationId()); var repositoryMock = mocks.repository(repoRef.repositoryName()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "yrodiere's report for quarkusio/quarkusio.githbub.io"); var issuesMocks = mockPagedIterable(issue1Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); when(repositoryMock.createIssue(any())).thenReturn(issueBuilderMock); var issue2Mock = mocks.issue(2); @@ -1352,16 +1343,17 @@ void topic_comment_dedicatedIssueDoesNotExist_withConfusingOther() throws IOExce .then().github(mocks -> { var repositoryMock = mocks.repository(repoRef.repositoryName()); - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).assignee("yrodiere"); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); + verify(searchIssuesBuilderMock).q("assignee:yrodiere"); verify(repositoryMock) .createIssue("yrodiere's report for quarkusio/quarkus (updated 2017-11-06T06:00:00Z)"); verify(issueBuilderMock).assignee("yrodiere"); verify(issueBuilderMock).body("Dedicated issue body"); verify(mocks.issue(2)).comment("Some content"); - verifyNoMoreInteractions(messageFormatterMock, queryIssuesBuilderMock, issueBuilderMock); + verifyNoMoreInteractions(messageFormatterMock, searchIssuesBuilderMock, issueBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -1374,19 +1366,19 @@ void topic_isClosed_dedicatedIssueExists_open() throws Exception { var clockMock = Clock.fixed(now, ZoneOffset.UTC); QuarkusMock.installMockForType(clockMock, Clock.class); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issue2Mock = mockIssueForNotification(mocks, 2, "yrodiere's report for quarkusio/quarkus (updated 2017-11-05T06:00:00Z)"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); when(issue2Mock.getState()).thenReturn(GHIssueState.OPEN); }) @@ -1398,11 +1390,12 @@ void topic_isClosed_dedicatedIssueExists_open() throws Exception { .isFalse(); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).assignee("yrodiere"); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); + verify(searchIssuesBuilderMock).q("assignee:yrodiere"); - verifyNoMoreInteractions(queryIssuesBuilderMock); + verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -1415,19 +1408,19 @@ void topic_isClosed_dedicatedIssueExists_closed() throws Exception { var clockMock = Clock.fixed(now, ZoneOffset.UTC); QuarkusMock.installMockForType(clockMock, Clock.class); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issue2Mock = mockIssueForNotification(mocks, 2, "yrodiere's report for quarkusio/quarkus (updated 2017-11-05T06:00:00Z)"); var issuesMocks = mockPagedIterable(issue1Mock, issue2Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); when(issue2Mock.getState()).thenReturn(GHIssueState.CLOSED); }) @@ -1439,11 +1432,12 @@ void topic_isClosed_dedicatedIssueExists_closed() throws Exception { .isTrue(); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).assignee("yrodiere"); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); + verify(searchIssuesBuilderMock).q("assignee:yrodiere"); - verifyNoMoreInteractions(queryIssuesBuilderMock); + verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } @@ -1451,17 +1445,17 @@ void topic_isClosed_dedicatedIssueExists_closed() throws Exception { @Test void topic_isClosed_dedicatedIssueDoesNotExist() throws IOException { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); - var queryIssuesBuilderMock = Mockito.mock(GHIssueQueryBuilder.ForRepository.class, + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, withSettings().defaultAnswer(Answers.RETURNS_SELF)); given() .github(mocks -> { - var repositoryMock = mocks.repository(repoRef.repositoryName()); + var clientMock = mocks.installationClient(installationRef.installationId()); - when(repositoryMock.queryIssues()).thenReturn(queryIssuesBuilderMock); + when(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); var issue1Mock = mockIssueForNotification(mocks, 1, "An unrelated issue"); var issuesMocks = mockPagedIterable(issue1Mock); - when(queryIssuesBuilderMock.list()).thenReturn(issuesMocks); + when(searchIssuesBuilderMock.list()).thenReturn(issuesMocks); }) .when(() -> { var repo = gitHubService.repository(repoRef); @@ -1471,11 +1465,12 @@ void topic_isClosed_dedicatedIssueDoesNotExist() throws IOException { .isFalse(); }) .then().github(mocks -> { - verify(queryIssuesBuilderMock).creator(installationRef.appLogin()); - verify(queryIssuesBuilderMock).assignee("yrodiere"); - verify(queryIssuesBuilderMock).state(GHIssueState.ALL); + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).q("is:issue"); + verify(searchIssuesBuilderMock).q("author:" + installationRef.appLogin()); + verify(searchIssuesBuilderMock).q("assignee:yrodiere"); - verifyNoMoreInteractions(queryIssuesBuilderMock); + verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); } diff --git a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java index 2ea16c0..9a56fc4 100644 --- a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java +++ b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java @@ -8,7 +8,6 @@ import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; -import java.util.Date; import java.util.Iterator; import java.util.List; import java.util.stream.IntStream; @@ -18,7 +17,6 @@ import org.kohsuke.github.GHIssueEvent; import org.kohsuke.github.GHLabel; import org.kohsuke.github.GHPermissionType; -import org.kohsuke.github.GHPullRequest; import org.kohsuke.github.GHPullRequestFileDetail; import org.kohsuke.github.GHRepository; import org.kohsuke.github.GHUser; @@ -65,49 +63,35 @@ private static Issue stubIssue(int number) { return new Issue(number, "Title for issue " + number, url(number)); } - public static GHIssue mockIssueForLottery(GitHubMockContext context, int number, Date updatedAt) + public static GHIssue mockIssueForLottery(GitHubMockContext context, int number) throws IOException { GHIssue mock = context.issue(10000L + number); - when(mock.isPullRequest()).thenReturn(false); when(mock.getNumber()).thenReturn(number); when(mock.getTitle()).thenReturn("Title for issue " + number); when(mock.getHtmlUrl()).thenReturn(url(number)); - when(mock.getUpdatedAt()).thenReturn(updatedAt); return mock; } - public static GHIssue mockIssueForLottery(GitHubMockContext context, int number, Date updatedAt, GHUser reporter) + public static GHIssue mockIssueForLottery(GitHubMockContext context, int number, GHUser reporter) throws IOException { GHIssue mock = context.issue(10000L + number); - when(mock.isPullRequest()).thenReturn(false); when(mock.getNumber()).thenReturn(number); when(mock.getTitle()).thenReturn("Title for issue " + number); when(mock.getHtmlUrl()).thenReturn(url(number)); - when(mock.getUpdatedAt()).thenReturn(updatedAt); when(mock.getUser()).thenReturn(reporter); return mock; } - public static GHIssue mockIssueForLotteryFilteredOutByRepository(GitHubMockContext context, int number, Date updatedAt) + public static GHIssue mockIssueForLotteryFilteredOutByRepository(GitHubMockContext context, int number) throws IOException { GHIssue mock = context.issue(10000L + number); - when(mock.isPullRequest()).thenReturn(false); - when(mock.getUpdatedAt()).thenReturn(updatedAt); return mock; } - public static GHPullRequest mockPullRequestForLotteryFilteredOutByRepository(GitHubMockContext context, int number) { - GHPullRequest mock = context.pullRequest(10000L + number); - when(mock.isPullRequest()).thenReturn(true); - return mock; - } - - public static GHIssue mockIssueForLotteryFilteredOutByRepository(GitHubMockContext context, int number, Date updatedAt, + public static GHIssue mockIssueForLotteryFilteredOutByRepository(GitHubMockContext context, int number, GHUser reporter) throws IOException { GHIssue mock = context.issue(10000L + number); - when(mock.isPullRequest()).thenReturn(false); - when(mock.getUpdatedAt()).thenReturn(updatedAt); when(mock.getUser()).thenReturn(reporter); return mock; }