From 70710ec10c3b1805c23340b42e9a9b6cdf560d75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Tue, 10 Dec 2024 08:56:43 +0100 Subject: [PATCH 1/4] Use static imports for GitHubSearchClauses --- .../lottery/github/GitHubRepository.java | 27 ++++++++++++------- 1 file changed, 17 insertions(+), 10 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 12c3092..e61d942 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -1,5 +1,12 @@ package io.quarkus.github.lottery.github; +import static io.quarkus.github.lottery.github.GitHubSearchClauses.anyLabel; +import static io.quarkus.github.lottery.github.GitHubSearchClauses.assignee; +import static io.quarkus.github.lottery.github.GitHubSearchClauses.author; +import static io.quarkus.github.lottery.github.GitHubSearchClauses.isIssue; +import static io.quarkus.github.lottery.github.GitHubSearchClauses.label; +import static io.quarkus.github.lottery.github.GitHubSearchClauses.repo; +import static io.quarkus.github.lottery.github.GitHubSearchClauses.updatedBefore; import static io.quarkus.github.lottery.util.Streams.toStream; import static io.quarkus.github.lottery.util.UncheckedIOFunction.uncheckedIO; @@ -95,8 +102,8 @@ private GHRepository repository() throws IOException { private GHIssueSearchBuilder searchIssues() { return client().searchIssues() - .q(GitHubSearchClauses.repo(ref)) - .q(GitHubSearchClauses.isIssue()); + .q(repo(ref)) + .q(isIssue()); } private DynamicGraphQLClient graphQLClient() { @@ -121,7 +128,7 @@ public Optional fetchLotteryConfig() throws IOException { public Stream issuesLastUpdatedBefore(Instant updatedBefore) { return toStream(searchIssues() .isOpen() - .q(GitHubSearchClauses.updatedBefore(updatedBefore)) + .q(updatedBefore(updatedBefore)) .sort(GHIssueSearchBuilder.Sort.UPDATED) .order(GHDirection.DESC) .list()) @@ -139,8 +146,8 @@ public Stream issuesLastUpdatedBefore(Instant updatedBefore) { public Stream issuesWithLabelLastUpdatedBefore(String label, Instant updatedBefore) { return toStream(searchIssues() .isOpen() - .q(GitHubSearchClauses.label(label)) - .q(GitHubSearchClauses.updatedBefore(updatedBefore)) + .q(label(label)) + .q(updatedBefore(updatedBefore)) .sort(GHIssueSearchBuilder.Sort.UPDATED) .order(GHDirection.DESC) .list()) @@ -163,9 +170,9 @@ public Stream issuesLastActedOnByAndLastUpdatedBefore(Set initial IssueActionSide lastActionSide, Instant updatedBefore) { return toStream(searchIssues() .isOpen() - .q(GitHubSearchClauses.anyLabel(initialActionLabels)) - .q(GitHubSearchClauses.label(filterLabel)) - .q(GitHubSearchClauses.updatedBefore(updatedBefore)) + .q(anyLabel(initialActionLabels)) + .q(label(filterLabel)) + .q(updatedBefore(updatedBefore)) .sort(GHIssueSearchBuilder.Sort.UPDATED) .order(GHDirection.DESC) .list()) @@ -305,9 +312,9 @@ public void update(String topicSuffix, String markdownBody, boolean comment) private Stream getDedicatedIssues() throws IOException { var builder = searchIssues() - .q(GitHubSearchClauses.author(appLogin())); + .q(author(appLogin())); if (ref.assignee() != null) { - builder.q(GitHubSearchClauses.assignee(ref.assignee())); + builder.q(assignee(ref.assignee())); } return toStream(builder.list()) .filter(ref.expectedSuffixStart() != null From 9aa97625b75dbaaed0e28b100d656184c8700c0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 11 Dec 2024 08:47:42 +0100 Subject: [PATCH 2/4] Test configuration fetching with minimal file (some fiels not set) --- .../github/lottery/config/LotteryConfig.java | 9 +++ .../github/lottery/GitHubServiceTest.java | 65 +++++++++++++++++++ 2 files changed, 74 insertions(+) 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 a59c7e1..2109b2b 100644 --- a/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java +++ b/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java @@ -24,6 +24,15 @@ public record LotteryConfig( public static final String FILE_NAME = "quarkus-github-lottery.yml"; + public LotteryConfig( + @JsonProperty(required = true) Notifications notifications, + @JsonProperty(required = true) Buckets buckets, + List participants) { + this.notifications = notifications; + this.buckets = buckets; + this.participants = participants == null ? List.of() : participants; + } + public record Notifications( @JsonProperty(required = true) CreateIssuesConfig createIssues) { public record CreateIssuesConfig( diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index 0a3b3cd..530b690 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -292,6 +292,71 @@ void fetchLotteryConfig() throws IOException { }); } + @Test + void fetchLotteryConfig_minimal() throws IOException { + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); + + given() + .github(mocks -> { + var repositoryMock = mocks.repository(repoRef.repositoryName()); + mocks.configFile(repositoryMock, "quarkus-github-lottery.yml") + .fromString(""" + notifications: + createIssues: + repository: "quarkusio/quarkus-lottery-reports" + buckets: + triage: + label: "triage/needs-triage" + delay: PT0S + timeout: P3D + maintenance: + feedback: + labels: ["triage/needs-feedback"] + needed: + delay: P21D + timeout: P3D + provided: + delay: P7D + timeout: P3D + stale: + delay: P60D + timeout: P14D + stewardship: + delay: P60D + timeout: P14D + """); + }) + .when(() -> { + var repo = gitHubService.repository(repoRef); + + assertThat(repo.fetchLotteryConfig()) + .isNotEmpty() + .get().usingRecursiveComparison().isEqualTo(new LotteryConfig( + new LotteryConfig.Notifications( + new LotteryConfig.Notifications.CreateIssuesConfig( + "quarkusio/quarkus-lottery-reports")), + new LotteryConfig.Buckets( + new LotteryConfig.Buckets.Triage( + "triage/needs-triage", + Duration.ZERO, Duration.ofDays(3)), + new LotteryConfig.Buckets.Maintenance( + new LotteryConfig.Buckets.Maintenance.Feedback( + List.of("triage/needs-feedback"), + new LotteryConfig.Buckets.Maintenance.Feedback.Needed( + Duration.ofDays(21), Duration.ofDays(3)), + new LotteryConfig.Buckets.Maintenance.Feedback.Provided( + Duration.ofDays(7), Duration.ofDays(3))), + new LotteryConfig.Buckets.Maintenance.Stale( + Duration.ofDays(60), Duration.ofDays(14))), + new LotteryConfig.Buckets.Stewardship( + Duration.ofDays(60), Duration.ofDays(14))), + List.of())); + }) + .then().github(mocks -> { + verifyNoMoreInteractions(mocks.ghObjects()); + }); + } + @Test void issuesLastUpdatedBefore() throws IOException { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); From 9d0e19856610ee9efbebf6b7d5774f480dc265da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 11 Dec 2024 08:50:02 +0100 Subject: [PATCH 3/4] Allow ignoring issues by labels for stale/stewardship --- README.adoc | 14 ++- .../github/lottery/config/LotteryConfig.java | 16 +-- .../quarkus/github/lottery/draw/Lottery.java | 9 +- .../lottery/github/GitHubRepository.java | 27 ++++-- .../lottery/github/GitHubSearchClauses.java | 4 + .../github/lottery/GitHubServiceTest.java | 97 +++++++++++++++++-- .../github/lottery/HistoryServiceTest.java | 4 +- .../lottery/LotterySingleRepositoryTest.java | 26 ++--- 8 files changed, 156 insertions(+), 41 deletions(-) diff --git a/README.adoc b/README.adoc index cd4b419..d279945 100644 --- a/README.adoc +++ b/README.adoc @@ -187,11 +187,12 @@ participants: maxIssues: 2 stale: maxIssues: 5 + ignoreLabels: ["triage/on-ice"] ---- `labels`:: The labels identifying issues you are interested in, as a maintainer. -Issues mentioned in notifications will have at least one of those labels. +Issues mentioned in notifications will have at least one of these labels. + Array of Strings, mandatory, no default. `days`:: @@ -213,6 +214,11 @@ How many issues, at most, you wish to be included in the "Stale" category for each notification. + Integer, mandatory, no default. +`stale.ignoreLabels`:: +The labels identifying issues that should be ignored for the "Stale" category. +Issues mentioned in notifications will never have any one of these labels. ++ +Array of Strings, optional, defaults to an empty array. [[participants-stewardship]] === Stewardship @@ -244,6 +250,7 @@ participants: stewardship: days: ["MONDAY"] maxIssues: 5 + ignoreLabels: ["triage/on-ice"] ---- `days`:: @@ -255,6 +262,11 @@ How many issues, at most, you wish to be included in the "stewardship" category for each notification. + Integer, mandatory, no default. +`ignoreLabels`:: +The labels identifying issues that should be ignored for the "stewardship" category. +Issues mentioned in notifications will never have any one of these labels. ++ +Array of Strings, optional, defaults to an empty array. [[participants-suspending]] === Suspending notifications 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 2109b2b..4693076 100644 --- a/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java +++ b/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java @@ -90,23 +90,27 @@ public Provided(@JsonProperty(required = true) Duration delay, } public record Stale( - @JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification) { + @JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification, + @JsonProperty(required = true) List ignoreLabels) { // https://stackoverflow.com/a/71539100/6692043 // Also gives us a less verbose constructor for tests @JsonCreator - public Stale(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout) { - this(new Notification(delay, timeout)); + public Stale(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout, + @JsonProperty(required = false) List ignoreLabels) { + this(new Notification(delay, timeout), ignoreLabels == null ? List.of() : ignoreLabels); } } } public record Stewardship( - @JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification) { + @JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification, + @JsonProperty(required = false) List ignoreLabels) { // https://stackoverflow.com/a/71539100/6692043 // Also gives us a less verbose constructor for tests @JsonCreator - public Stewardship(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout) { - this(new Notification(delay, timeout)); + public Stewardship(@JsonProperty(required = true) Duration delay, @JsonProperty(required = true) Duration timeout, + @JsonProperty(required = false) List ignoreLabels) { + this(new Notification(delay, timeout), ignoreLabels == null ? List.of() : ignoreLabels); } } diff --git a/src/main/java/io/quarkus/github/lottery/draw/Lottery.java b/src/main/java/io/quarkus/github/lottery/draw/Lottery.java index d2a096c..b5c6e78 100644 --- a/src/main/java/io/quarkus/github/lottery/draw/Lottery.java +++ b/src/main/java/io/quarkus/github/lottery/draw/Lottery.java @@ -95,7 +95,7 @@ void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List history.lastNotificationTimedOutForIssueNumber(issue.number())) .iterator(), allWinnings)); @@ -155,9 +155,11 @@ void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List(config.maintenance().stale().ignoreLabels()); var history = lotteryHistory.stale(); draws.add(stale.createDraw( - repo.issuesWithLabelLastUpdatedBefore(areaLabel, cutoff) + repo.issuesWithLabelLastUpdatedBefore(areaLabel, ignoreLabels, cutoff) .filter(issue -> history.lastNotificationTimedOutForIssueNumber(issue.number())) .iterator(), allWinnings)); @@ -176,9 +178,10 @@ void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List allWinnings) throws IOException { if (stewardship.bucket.hasParticipation()) { var cutoff = now.minus(config.stewardship().notification().delay()); + var ignoreLabels = new LinkedHashSet<>(config.stewardship().ignoreLabels()); var history = lotteryHistory.stewardship(); draws.add(bucket.createDraw( - repo.issuesLastUpdatedBefore(cutoff) + repo.issuesLastUpdatedBefore(ignoreLabels, cutoff) .filter(issue -> history.lastNotificationTimedOutForIssueNumber(issue.number())) .iterator(), allWinnings)); 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 e61d942..ab0c45a 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -5,6 +5,7 @@ import static io.quarkus.github.lottery.github.GitHubSearchClauses.author; import static io.quarkus.github.lottery.github.GitHubSearchClauses.isIssue; import static io.quarkus.github.lottery.github.GitHubSearchClauses.label; +import static io.quarkus.github.lottery.github.GitHubSearchClauses.not; import static io.quarkus.github.lottery.github.GitHubSearchClauses.repo; import static io.quarkus.github.lottery.github.GitHubSearchClauses.updatedBefore; import static io.quarkus.github.lottery.util.Streams.toStream; @@ -121,37 +122,43 @@ public Optional fetchLotteryConfig() throws IOException { /** * Lists issues that were last updated before the given instant. * + * @param ignoreLabels GitHub labels. Issues assigned with any of these labels are ignored (not returned). * @param updatedBefore An instant; all returned issues must have been last updated before that instant. * @return A lazily populated stream of matching issues. * @throws java.io.UncheckedIOException In case of I/O failure. */ - public Stream issuesLastUpdatedBefore(Instant updatedBefore) { - return toStream(searchIssues() + public Stream issuesLastUpdatedBefore(Set ignoreLabels, Instant updatedBefore) { + var builder = searchIssues() .isOpen() .q(updatedBefore(updatedBefore)) .sort(GHIssueSearchBuilder.Sort.UPDATED) - .order(GHDirection.DESC) - .list()) - .map(toIssueRecord()); + .order(GHDirection.DESC); + if (!ignoreLabels.isEmpty()) { + builder.q(not(anyLabel(ignoreLabels))); + } + return toStream(builder.list()).map(toIssueRecord()); } /** * Lists issues with the given label that were last updated before the given instant. * * @param label A GitHub label; if non-null, all returned issues must have been assigned that label. + * @param ignoreLabels GitHub labels. Issues assigned with any of these labels are ignored (not returned). * @param updatedBefore An instant; all returned issues must have been last updated before that instant. * @return A lazily populated stream of matching issues. * @throws java.io.UncheckedIOException In case of I/O failure. */ - public Stream issuesWithLabelLastUpdatedBefore(String label, Instant updatedBefore) { - return toStream(searchIssues() + public Stream issuesWithLabelLastUpdatedBefore(String label, Set ignoreLabels, Instant updatedBefore) { + var builder = searchIssues() .isOpen() .q(label(label)) .q(updatedBefore(updatedBefore)) .sort(GHIssueSearchBuilder.Sort.UPDATED) - .order(GHDirection.DESC) - .list()) - .map(toIssueRecord()); + .order(GHDirection.DESC); + if (!ignoreLabels.isEmpty()) { + builder.q(not(anyLabel(ignoreLabels))); + } + return toStream(builder.list()).map(toIssueRecord()); } /** diff --git a/src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java b/src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java index 70ef8b0..abd1007 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java @@ -9,6 +9,10 @@ public final class GitHubSearchClauses { private GitHubSearchClauses() { } + public static String not(String clause) { + return "-" + clause; + } + public static String repo(GitHubRepositoryRef ref) { return "repo:" + ref.repositoryName(); } diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index 530b690..3a1488c 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -163,9 +163,11 @@ void fetchLotteryConfig() throws IOException { stale: delay: P60D timeout: P14D + ignoreLabels: ["triage/on-ice"] stewardship: delay: P60D timeout: P14D + ignoreLabels: ["triage/on-ice"] participants: - username: "yrodiere" triage: @@ -233,9 +235,10 @@ void fetchLotteryConfig() throws IOException { new LotteryConfig.Buckets.Maintenance.Feedback.Provided( Duration.ofDays(7), Duration.ofDays(3))), new LotteryConfig.Buckets.Maintenance.Stale( - Duration.ofDays(60), Duration.ofDays(14))), + Duration.ofDays(60), Duration.ofDays(14), + List.of("triage/on-ice"))), new LotteryConfig.Buckets.Stewardship( - Duration.ofDays(60), Duration.ofDays(14))), + Duration.ofDays(60), Duration.ofDays(14), List.of("triage/on-ice"))), List.of( new LotteryConfig.Participant("yrodiere", Optional.empty(), @@ -324,6 +327,7 @@ void fetchLotteryConfig_minimal() throws IOException { stewardship: delay: P60D timeout: P14D + participants: """); }) .when(() -> { @@ -347,9 +351,9 @@ void fetchLotteryConfig_minimal() throws IOException { new LotteryConfig.Buckets.Maintenance.Feedback.Provided( Duration.ofDays(7), Duration.ofDays(3))), new LotteryConfig.Buckets.Maintenance.Stale( - Duration.ofDays(60), Duration.ofDays(14))), + Duration.ofDays(60), Duration.ofDays(14), List.of())), new LotteryConfig.Buckets.Stewardship( - Duration.ofDays(60), Duration.ofDays(14))), + Duration.ofDays(60), Duration.ofDays(14), List.of())), List.of())); }) .then().github(mocks -> { @@ -381,7 +385,46 @@ void issuesLastUpdatedBefore() throws IOException { .when(() -> { var repo = gitHubService.repository(repoRef); - assertThat(repo.issuesLastUpdatedBefore(cutoff)) + assertThat(repo.issuesLastUpdatedBefore(Set.of(), cutoff)) + .containsExactlyElementsOf(stubIssueList(1, 3, 2, 4)); + }) + .then().github(mocks -> { + 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()); + }); + } + + @Test + void issuesLastUpdatedBefore_ignoreLabels() throws IOException { + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); + + Instant now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC); + Instant cutoff = now.minus(1, ChronoUnit.DAYS); + + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); + given() + .github(mocks -> { + 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); + + assertThat(repo.issuesLastUpdatedBefore(Set.of("triage/on-ice"), cutoff)) .containsExactlyElementsOf(stubIssueList(1, 3, 2, 4)); }) .then().github(mocks -> { @@ -391,6 +434,7 @@ void issuesLastUpdatedBefore() throws IOException { verify(searchIssuesBuilderMock).q("updated:<2017-11-05T06:00"); verify(searchIssuesBuilderMock).sort(GHIssueSearchBuilder.Sort.UPDATED); verify(searchIssuesBuilderMock).order(GHDirection.DESC); + verify(searchIssuesBuilderMock).q("-label:triage/on-ice"); verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); @@ -420,7 +464,47 @@ void issuesWithLabelLastUpdatedBefore() throws IOException { .when(() -> { var repo = gitHubService.repository(repoRef); - assertThat(repo.issuesWithLabelLastUpdatedBefore("triage/needs-triage", cutoff)) + assertThat(repo.issuesWithLabelLastUpdatedBefore("triage/needs-triage", Set.of(), cutoff)) + .containsExactlyElementsOf(stubIssueList(1, 3, 2, 4)); + }) + .then().github(mocks -> { + 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()); + }); + } + + @Test + void issuesWithLabelLastUpdatedBefore_ignoreLabels() throws IOException { + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); + + Instant now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC); + Instant cutoff = now.minus(1, ChronoUnit.DAYS); + + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); + given() + .github(mocks -> { + 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); + + assertThat(repo.issuesWithLabelLastUpdatedBefore("triage/needs-triage", Set.of("triage/on-ice"), cutoff)) .containsExactlyElementsOf(stubIssueList(1, 3, 2, 4)); }) .then().github(mocks -> { @@ -431,6 +515,7 @@ void issuesWithLabelLastUpdatedBefore() throws IOException { verify(searchIssuesBuilderMock).order(GHDirection.DESC); verify(searchIssuesBuilderMock).q("label:triage/needs-triage"); verify(searchIssuesBuilderMock).q("updated:<2017-11-05T06:00"); + verify(searchIssuesBuilderMock).q("-label:triage/on-ice"); verifyNoMoreInteractions(searchIssuesBuilderMock); verifyNoMoreInteractions(mocks.ghObjects()); }); diff --git a/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java b/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java index 1c258ea..657f3c6 100644 --- a/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java @@ -59,9 +59,9 @@ private static LotteryConfig defaultConfig() { new LotteryConfig.Buckets.Maintenance.Feedback.Provided( Duration.ofDays(7), Duration.ofDays(3))), new LotteryConfig.Buckets.Maintenance.Stale( - Duration.ofDays(60), Duration.ofDays(14))), + Duration.ofDays(60), Duration.ofDays(14), List.of("triage/on-ice"))), new LotteryConfig.Buckets.Stewardship( - Duration.ofDays(60), Duration.ofDays(14))), + Duration.ofDays(60), Duration.ofDays(14), List.of("triage/on-ice"))), List.of()); } diff --git a/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java b/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java index af7f4cd..9afa19c 100644 --- a/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java +++ b/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java @@ -70,9 +70,9 @@ private static LotteryConfig defaultConfig(List parti new LotteryConfig.Buckets.Maintenance.Feedback.Provided( Duration.ofDays(7), Duration.ofDays(3))), new LotteryConfig.Buckets.Maintenance.Stale( - Duration.ofDays(60), Duration.ofDays(14))), + Duration.ofDays(60), Duration.ofDays(14), List.of("triage/on-ice"))), new LotteryConfig.Buckets.Stewardship( - Duration.ofDays(60), Duration.ofDays(14))), + Duration.ofDays(60), Duration.ofDays(14), List.of("triage/on-ice"))), participants); } @@ -284,7 +284,7 @@ void triage() throws IOException { Optional.empty()))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); - when(repoMock.issuesWithLabelLastUpdatedBefore("needs-triage", now)) + when(repoMock.issuesWithLabelLastUpdatedBefore("needs-triage", Set.of(), now)) .thenAnswer(ignored -> stubIssueList(1, 3, 2, 4).stream()); mockNotifiable("yrodiere", ZoneOffset.UTC); @@ -325,7 +325,7 @@ void triage_issueAlreadyHasNonTimedOutNotification() throws IOException { Optional.empty()))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); - when(repoMock.issuesWithLabelLastUpdatedBefore("needs-triage", now)) + when(repoMock.issuesWithLabelLastUpdatedBefore("needs-triage", Set.of(), now)) .thenAnswer(ignored -> stubIssueList(1, 3, 2, 4).stream()); mockNotifiable("yrodiere", ZoneOffset.UTC); @@ -381,7 +381,7 @@ void maintenance() throws IOException { Set.of("triage/needs-reproducer", "triage/needs-feedback"), "area/hibernate-orm", IssueActionSide.OUTSIDER, feedbackProvidedCutoff)) .thenAnswer(ignored -> stubIssueList(201, 202, 203).stream()); - when(repoMock.issuesWithLabelLastUpdatedBefore("area/hibernate-orm", staleCutoff)) + when(repoMock.issuesWithLabelLastUpdatedBefore("area/hibernate-orm", Set.of("triage/on-ice"), staleCutoff)) .thenAnswer(ignored -> stubIssueList(301, 302, 303, 304, 305, 306).stream()); when(repoMock.issuesLastActedOnByAndLastUpdatedBefore( @@ -392,7 +392,7 @@ void maintenance() throws IOException { Set.of("triage/needs-reproducer", "triage/needs-feedback"), "area/hibernate-search", IssueActionSide.OUTSIDER, feedbackProvidedCutoff)) .thenAnswer(ignored -> stubIssueList(501, 502, 503).stream()); - when(repoMock.issuesWithLabelLastUpdatedBefore("area/hibernate-search", staleCutoff)) + when(repoMock.issuesWithLabelLastUpdatedBefore("area/hibernate-search", Set.of("triage/on-ice"), staleCutoff)) .thenAnswer(ignored -> stubIssueList(601, 602, 603, 604, 605, 606).stream()); mockNotifiable("yrodiere", ZoneOffset.UTC); @@ -455,7 +455,7 @@ void maintenance_issueAlreadyHasTimedOutNotification() throws IOException { Set.of("triage/needs-reproducer", "triage/needs-feedback"), "area/hibernate-orm", IssueActionSide.OUTSIDER, feedbackProvidedCutoff)) .thenAnswer(ignored -> stubIssueList(201, 202, 203).stream()); - when(repoMock.issuesWithLabelLastUpdatedBefore("area/hibernate-orm", staleCutoff)) + when(repoMock.issuesWithLabelLastUpdatedBefore("area/hibernate-orm", Set.of("triage/on-ice"), staleCutoff)) .thenAnswer(ignored -> stubIssueList(301, 302, 303, 304, 305, 306).stream()); when(repoMock.issuesLastActedOnByAndLastUpdatedBefore( @@ -466,7 +466,7 @@ void maintenance_issueAlreadyHasTimedOutNotification() throws IOException { Set.of("triage/needs-reproducer", "triage/needs-feedback"), "area/hibernate-search", IssueActionSide.OUTSIDER, feedbackProvidedCutoff)) .thenAnswer(ignored -> stubIssueList(501, 502, 503).stream()); - when(repoMock.issuesWithLabelLastUpdatedBefore("area/hibernate-search", staleCutoff)) + when(repoMock.issuesWithLabelLastUpdatedBefore("area/hibernate-search", Set.of("triage/on-ice"), staleCutoff)) .thenAnswer(ignored -> stubIssueList(601, 602, 603, 604, 605, 606).stream()); mockNotifiable("yrodiere", ZoneOffset.UTC); @@ -522,7 +522,7 @@ void stewardship() throws IOException { new LotteryConfig.Participant.Participation(3)))))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); - when(repoMock.issuesLastUpdatedBefore(stewardshipCutoff)) + when(repoMock.issuesLastUpdatedBefore(Set.of("triage/on-ice"), stewardshipCutoff)) .thenAnswer(ignored -> stubIssueList(1, 3, 2, 4).stream()); mockNotifiable("geoand", ZoneOffset.UTC); @@ -563,7 +563,7 @@ void stewardship_issueAlreadyHasNonTimedOutNotification() throws IOException { new LotteryConfig.Participant.Participation(3)))))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); - when(repoMock.issuesLastUpdatedBefore(stewardshipCutoff)) + when(repoMock.issuesLastUpdatedBefore(Set.of("triage/on-ice"), stewardshipCutoff)) .thenAnswer(ignored -> stubIssueList(1, 3, 2, 4).stream()); mockNotifiable("geoand", ZoneOffset.UTC); @@ -626,10 +626,10 @@ void stewardship_doesNotAffectMaintenance() throws IOException { Set.of("triage/needs-reproducer", "triage/needs-feedback"), "area/hibernate-search", IssueActionSide.OUTSIDER, feedbackProvidedCutoff)) .thenAnswer(ignored -> stubIssueList(501, 502, 503).stream()); - when(repoMock.issuesWithLabelLastUpdatedBefore("area/hibernate-search", staleCutoff)) + when(repoMock.issuesWithLabelLastUpdatedBefore("area/hibernate-search", Set.of("triage/on-ice"), staleCutoff)) .thenAnswer(ignored -> stubIssueList(601, 602, 603, 604, 605, 606).stream()); - when(repoMock.issuesLastUpdatedBefore(stewardshipCutoff)) + when(repoMock.issuesLastUpdatedBefore(Set.of("triage/on-ice"), stewardshipCutoff)) .thenAnswer(ignored -> stubIssueList(401, 501, 601, 701).stream()); mockNotifiable("yrodiere", ZoneOffset.UTC); @@ -706,7 +706,7 @@ void multiParticipants_evenSpread() throws IOException { Optional.empty()))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); - when(repoMock.issuesWithLabelLastUpdatedBefore("needs-triage", now)) + when(repoMock.issuesWithLabelLastUpdatedBefore("needs-triage", Set.of(), now)) .thenAnswer(ignored -> stubIssueList(1, 3, 2, 4).stream()); mockNotifiable("yrodiere", ZoneOffset.UTC); From 8f40f01dfe268223a8689fb94eac2391293d5019 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 11 Dec 2024 11:47:06 +0100 Subject: [PATCH 4/4] Add small description of each category in notifications --- README.adoc | 41 +++------ .../github/lottery/LotteryService.java | 13 +-- .../github/lottery/draw/LotteryReport.java | 8 ++ .../github/lottery/draw/Participant.java | 6 +- .../lottery/message/MessageFormatter.java | 13 +++ .../MessageFormatter/notificationBody.md | 44 ++++++++-- .../notificationBodyBucketContent.md | 2 + .../github/lottery/HistoryServiceTest.java | 2 - .../lottery/LotterySingleRepositoryTest.java | 21 +++-- .../github/lottery/MessageFormatterTest.java | 86 +++++++++++++++++-- .../lottery/NotificationServiceTest.java | 5 ++ .../lottery/draw/LotteryReportTest.java | 2 + .../github/lottery/util/MockHelper.java | 8 ++ 13 files changed, 190 insertions(+), 61 deletions(-) diff --git a/README.adoc b/README.adoc index d279945..c1861c2 100644 --- a/README.adoc +++ b/README.adoc @@ -98,13 +98,10 @@ See below for details about each section. [[participants-triage]] === Triage -If the `triage` section is present, you will get notified when new issues are created -and need to be routed to the right person/team. +If the `triage` section is present, you will get notified about issues that haven't been assigned an area yet. -Addressing these notifications generally involves removing the `triage/needs-triage` label, -adding the correct `+area/*+` labels, and optionally pinging the relevant maintainers -(though the https://github.com/quarkusio/quarkus-github-bot#triage-issues[Quarkus GitHub Bot should generally take care of that part] -once you add `+area/*+` labels). +Please add an area label, remove the "needs triage" (e.g. `triage/needs-triage`) label, +and ping the relevant maintainers if necessary. Of course, some more discussion might be necessary before that, and that's fine: issues that don't change will reappear in another notification, a few days later. @@ -141,30 +138,17 @@ that may be stalled and require intervention from maintainers or reporters. Issues in "maintenance" notifications will be split in three categories: Feedback Needed:: -These issues have the `triage/needs-reproducer` label, -and it looks like the Quarkus team was the last to comment on the issue, -quite some time ago. +Issues with missing reproducer/information. + -Depending on the actual content of the issue, you might want to: -+ -* send a (gentle!) reminder to the reporter that we need feedback (a reproducer, more information, ...) before we can do anything. -* or, if it's really been too long, close the issue because we cannot work on it. +Please ping the reporter, or close the issue if it's taking too long. Feedback Provided:: -These issues have the `triage/needs-reproducer` label, -and it looks like someone who is not from the Quarkus team was the last to comment on the issue, -quite some time ago. +Issues with newly provided reproducer/information. + -There might be feedback (a reproducer, more information, ...) there, -in which case you might want to remove the `triage/needs-reproducer` label -and have a closer look. +Please have a closer look, possibly remove the "needs feedback" (e.g. `triage/needs-reproducer`) label, and plan further work. Stale:: -These issues have not been updated for a very long time. -+ -Depending on the actual content of the issue, you might want to: +Issues last updated a long time ago. + -* prioritize the issue and work on it soon; -* or send a reminder to someone you've been waiting on; -* or close the issue because it's no longer relevant. +Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ... Of course, in every situation, simply continuing the conversation, pinging someone, or even doing nothing at all are perfectly acceptable responses: @@ -228,12 +212,9 @@ core contributors who spend significant time working on GitHub issues. If you don't already know what this section is about, you probably don't want to use it. -If the `stewardship` section is present, you will get notified about issues that just became stale, -across the whole project, without any consideration for the labels assigned to those issues. +If the `stewardship` section is present, you will get notified about Issues across all areas last updated a long time ago. -Depending on the actual content of the issue, you might want to simply continue the conversation, -ping someone, close the issue, or even do nothing at all: -it's all up to you, and issues that don't change will reappear in another notification, a few days later. +Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ... NOTE: Notifications to stewards are sent independently of notifications to maintainers, so that the work of maintainers won't be affected by the work of stewards. diff --git a/src/main/java/io/quarkus/github/lottery/LotteryService.java b/src/main/java/io/quarkus/github/lottery/LotteryService.java index 8ad8b00..5a158ee 100644 --- a/src/main/java/io/quarkus/github/lottery/LotteryService.java +++ b/src/main/java/io/quarkus/github/lottery/LotteryService.java @@ -7,6 +7,7 @@ import java.time.ZoneOffset; import java.time.ZonedDateTime; import java.util.ArrayList; +import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; @@ -16,13 +17,13 @@ import io.quarkus.github.lottery.config.LotteryConfig; import io.quarkus.github.lottery.draw.DrawRef; import io.quarkus.github.lottery.draw.Lottery; -import io.quarkus.github.lottery.draw.Participant; -import io.quarkus.github.lottery.history.LotteryHistory; import io.quarkus.github.lottery.draw.LotteryReport; +import io.quarkus.github.lottery.draw.Participant; import io.quarkus.github.lottery.github.GitHubRepository; import io.quarkus.github.lottery.github.GitHubRepositoryRef; import io.quarkus.github.lottery.github.GitHubService; import io.quarkus.github.lottery.history.HistoryService; +import io.quarkus.github.lottery.history.LotteryHistory; import io.quarkus.github.lottery.notification.NotificationService; import io.quarkus.github.lottery.notification.Notifier; import io.quarkus.logging.Log; @@ -89,7 +90,7 @@ private void doDrawForRepository(GitHubRepository repo, LotteryConfig lotteryCon lottery.draw(repo, history); - var sent = notifyParticipants(notifier, participants); + var sent = notifyParticipants(lotteryConfig, notifier, participants); if (!sent.isEmpty()) { try { historyService.append(drawRef, lotteryConfig, sent); @@ -130,10 +131,12 @@ private List registerParticipants(DrawRef drawRef, Lottery lottery, return participants; } - private List notifyParticipants(Notifier notifier, List participants) { + private List notifyParticipants(LotteryConfig lotteryConfig, + Notifier notifier, List participants) { List sent = new ArrayList<>(); for (var participant : participants) { - var report = participant.report(); + var report = participant.report(lotteryConfig.buckets().triage().label(), + new LinkedHashSet<>(lotteryConfig.buckets().maintenance().feedback().labels())); try { Log.debugf("Sending report: %s", report); notifier.send(report); diff --git a/src/main/java/io/quarkus/github/lottery/draw/LotteryReport.java b/src/main/java/io/quarkus/github/lottery/draw/LotteryReport.java index ef640cf..cd63b6f 100644 --- a/src/main/java/io/quarkus/github/lottery/draw/LotteryReport.java +++ b/src/main/java/io/quarkus/github/lottery/draw/LotteryReport.java @@ -4,6 +4,7 @@ import java.time.ZoneId; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -17,6 +18,7 @@ public record LotteryReport( DrawRef drawRef, String username, Optional timezone, + Config config, Optional triage, Optional feedbackNeeded, Optional feedbackProvided, @@ -33,6 +35,12 @@ public boolean hasContent() { return buckets().anyMatch(Bucket::hasContent); } + public record Config( + String triageLabel, + Set feedbackLabels, + Set maintenanceLabels) { + } + public record Bucket( List issues) { 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 1f8fb4a..eef5344 100644 --- a/src/main/java/io/quarkus/github/lottery/draw/Participant.java +++ b/src/main/java/io/quarkus/github/lottery/draw/Participant.java @@ -77,8 +77,12 @@ public void participate(Lottery lottery) { stewardship.ifPresent(lottery.stewardship()::participate); } - public LotteryReport report() { + public LotteryReport report(String triageLabel, Set feedbackLabels) { return new LotteryReport(drawRef, username, timezone, + new LotteryReport.Config( + triageLabel, + feedbackLabels, + maintenance.map(m -> m.labels).orElseGet(Set::of)), triage.map(Participation::issues).map(LotteryReport.Bucket::new), maintenance.flatMap(m -> m.feedbackNeeded).map(Participation::issues).map(LotteryReport.Bucket::new), maintenance.flatMap(m -> m.feedbackProvided).map(Participation::issues).map(LotteryReport.Bucket::new), diff --git a/src/main/java/io/quarkus/github/lottery/message/MessageFormatter.java b/src/main/java/io/quarkus/github/lottery/message/MessageFormatter.java index 0def485..ca9eda6 100644 --- a/src/main/java/io/quarkus/github/lottery/message/MessageFormatter.java +++ b/src/main/java/io/quarkus/github/lottery/message/MessageFormatter.java @@ -1,7 +1,9 @@ package io.quarkus.github.lottery.message; import java.time.temporal.Temporal; +import java.util.Collection; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import jakarta.enterprise.context.ApplicationScoped; @@ -28,6 +30,9 @@ public class MessageFormatter { private static final String PAYLOAD_BEGIN = ""; + public record Config(String triageLabel, Set feedbackLabels) { + } + @Inject ObjectMapper jsonObjectMapper; @@ -103,6 +108,14 @@ static String repositoryName(DrawRef drawRef) { return drawRef.repositoryRef().repositoryName(); } + static String asMarkdownLabel(String label) { + return "`" + label + "`"; + } + + static String asMarkdownLabel(Collection labels) { + return labels.stream().map(TemplateExtensions::asMarkdownLabel).collect(Collectors.joining("/")); + } + } @TemplateExtension(namespace = "github") diff --git a/src/main/resources/templates/MessageFormatter/notificationBody.md b/src/main/resources/templates/MessageFormatter/notificationBody.md index 6e6846f..242a908 100644 --- a/src/main/resources/templates/MessageFormatter/notificationBody.md +++ b/src/main/resources/templates/MessageFormatter/notificationBody.md @@ -1,25 +1,53 @@ Hey @{report.username}, here's your report for {report.repositoryName} on {report.localDate}. +{#if !report.config.maintenanceLabels.isEmpty} + +Maintenance areas: {report.config.maintenanceLabels.asMarkdownLabel}. +{/if} {#if report.triage.present} # Triage -{#include MessageFormatter/notificationBodyBucketContent bucket=report.triage.get() /} +{#include MessageFormatter/notificationBodyBucketContent bucket=report.triage.get()} + +Issues that haven't been assigned an area yet. Please add an area label, remove the {report.config.triageLabel.asMarkdownLabel} label, optionally ping maintainers. + +{/include} + {/if} {#if report.feedbackNeeded.present} -# Feedback needed (reproducer, information, ...) -{#include MessageFormatter/notificationBodyBucketContent bucket=report.feedbackNeeded.get() /} +# Feedback needed +{#include MessageFormatter/notificationBodyBucketContent bucket=report.feedbackNeeded.get()} + +Issues with missing reproducer/information. Please ping the reporter, or close the issue if it's taking too long. + +{/include} + {/if} {#if report.feedbackProvided.present} -# Feedback provided (reproducer, information, ...) -{#include MessageFormatter/notificationBodyBucketContent bucket=report.feedbackProvided.get() /} +# Feedback provided +{#include MessageFormatter/notificationBodyBucketContent bucket=report.feedbackProvided.get()} + +Issues with newly provided reproducer/information. Please have a closer look, possibly remove the {report.config.feedbackLabels.asMarkdownLabel} label, and plan further work. + +{/include} + {/if} {#if report.stale.present} # Stale -{#include MessageFormatter/notificationBodyBucketContent bucket=report.stale.get() /} +{#include MessageFormatter/notificationBodyBucketContent bucket=report.stale.get()} + +Issues last updated a long time ago. Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ... + +{/include} + {/if} {#if report.stewardship.present} # Stewardship -{#include MessageFormatter/notificationBodyBucketContent bucket=report.stewardship.get() /} -{/if} +{#include MessageFormatter/notificationBodyBucketContent bucket=report.stewardship.get()} + +Issues across all areas last updated a long time ago. Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ... +{/include} + +{/if} --- If you no longer want to receive these notifications, just close [any issue assigned to you in the notification repository](https://github.com/{notificationRepositoryName}/issues/assigned/@me). Reopening the issue will resume the notifications. diff --git a/src/main/resources/templates/MessageFormatter/notificationBodyBucketContent.md b/src/main/resources/templates/MessageFormatter/notificationBodyBucketContent.md index 4a4810f..4ae7d8f 100644 --- a/src/main/resources/templates/MessageFormatter/notificationBodyBucketContent.md +++ b/src/main/resources/templates/MessageFormatter/notificationBodyBucketContent.md @@ -1,7 +1,9 @@ {@io.quarkus.github.lottery.draw.LotteryReport$Bucket bucket} {#if bucket.issues.isEmpty} + No issues in this category this time. {#else} +{#insert /} {#for issue in bucket.issues} - [#{issue.number}]({issue.url}) {issue.title} {/for} diff --git a/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java b/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java index 657f3c6..812cfe8 100644 --- a/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java @@ -2,8 +2,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; diff --git a/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java b/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java index 9afa19c..521665f 100644 --- a/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java +++ b/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java @@ -1,6 +1,7 @@ package io.quarkus.github.lottery; import static io.quarkus.github.lottery.util.MockHelper.stubIssueList; +import static io.quarkus.github.lottery.util.MockHelper.stubReportConfig; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyInt; @@ -31,8 +32,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import io.quarkus.github.lottery.github.GitHubInstallationRef; -import io.quarkus.github.lottery.github.IssueActionSide; import org.mockito.ArgumentCaptor; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @@ -40,9 +39,11 @@ import io.quarkus.github.lottery.config.LotteryConfig; import io.quarkus.github.lottery.draw.DrawRef; import io.quarkus.github.lottery.draw.LotteryReport; +import io.quarkus.github.lottery.github.GitHubInstallationRef; import io.quarkus.github.lottery.github.GitHubRepository; import io.quarkus.github.lottery.github.GitHubRepositoryRef; import io.quarkus.github.lottery.github.GitHubService; +import io.quarkus.github.lottery.github.IssueActionSide; import io.quarkus.github.lottery.history.HistoryService; import io.quarkus.github.lottery.history.LotteryHistory; import io.quarkus.github.lottery.notification.NotificationService; @@ -60,7 +61,7 @@ private static LotteryConfig defaultConfig(List parti new LotteryConfig.Notifications.CreateIssuesConfig("quarkusio/quarkus-lottery-reports")), new LotteryConfig.Buckets( new LotteryConfig.Buckets.Triage( - "needs-triage", + "triage/needs-triage", Duration.ZERO, Duration.ofDays(3)), new LotteryConfig.Buckets.Maintenance( new LotteryConfig.Buckets.Maintenance.Feedback( @@ -284,7 +285,7 @@ void triage() throws IOException { Optional.empty()))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); - when(repoMock.issuesWithLabelLastUpdatedBefore("needs-triage", Set.of(), now)) + when(repoMock.issuesWithLabelLastUpdatedBefore("triage/needs-triage", Set.of(), now)) .thenAnswer(ignored -> stubIssueList(1, 3, 2, 4).stream()); mockNotifiable("yrodiere", ZoneOffset.UTC); @@ -296,6 +297,7 @@ void triage() throws IOException { lotteryService.draw(); verify(notifierMock).send(new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig(), Optional.of(new LotteryReport.Bucket(stubIssueList(1, 3, 2))), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); @@ -325,7 +327,7 @@ void triage_issueAlreadyHasNonTimedOutNotification() throws IOException { Optional.empty()))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); - when(repoMock.issuesWithLabelLastUpdatedBefore("needs-triage", Set.of(), now)) + when(repoMock.issuesWithLabelLastUpdatedBefore("triage/needs-triage", Set.of(), now)) .thenAnswer(ignored -> stubIssueList(1, 3, 2, 4).stream()); mockNotifiable("yrodiere", ZoneOffset.UTC); @@ -340,6 +342,7 @@ void triage_issueAlreadyHasNonTimedOutNotification() throws IOException { // Since the last notification for issue with number 3 didn't time out yet, // it will be skipped and we'll notify about another issue. verify(notifierMock).send(new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig(), Optional.of(new LotteryReport.Bucket(stubIssueList(1, 2, 4))), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); @@ -410,6 +413,7 @@ void maintenance() throws IOException { lotteryService.draw(); verify(notifierMock).send(new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig("area/hibernate-orm", "area/hibernate-search"), Optional.empty(), Optional.of(new LotteryReport.Bucket(stubIssueList(101, 401, 102, 402))), Optional.of(new LotteryReport.Bucket(stubIssueList(201, 501))), @@ -489,6 +493,7 @@ void maintenance_issueAlreadyHasTimedOutNotification() throws IOException { // Since the last notification for issues with number 401, 201, 302 didn't time out yet, // they will be skipped and we'll notify about the next issues instead. verify(notifierMock).send(new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig("area/hibernate-orm", "area/hibernate-search"), Optional.empty(), Optional.of(new LotteryReport.Bucket(stubIssueList(101, 402, 102, 403))), Optional.of(new LotteryReport.Bucket(stubIssueList(202, 501))), @@ -534,6 +539,7 @@ void stewardship() throws IOException { lotteryService.draw(); verify(notifierMock).send(new LotteryReport(drawRef, "geoand", Optional.empty(), + stubReportConfig(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket(stubIssueList(1, 3, 2))))); @@ -578,6 +584,7 @@ void stewardship_issueAlreadyHasNonTimedOutNotification() throws IOException { // Since the last notification for issue with number 3 didn't time out yet, // it will be skipped and we'll notify about another issue. verify(notifierMock).send(new LotteryReport(drawRef, "geoand", Optional.empty(), + stubReportConfig(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket(stubIssueList(1, 2, 4))))); @@ -651,6 +658,7 @@ void stewardship_doesNotAffectMaintenance() throws IOException { lotteryService.draw(); verify(notifierMock).send(new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig("area/hibernate-search"), Optional.empty(), Optional.of(new LotteryReport.Bucket(stubIssueList(401, 402, 403, 404))), Optional.of(new LotteryReport.Bucket(stubIssueList(501, 502))), @@ -659,6 +667,7 @@ void stewardship_doesNotAffectMaintenance() throws IOException { Optional.empty())); verify(notifierMock).send(new LotteryReport(drawRef, "gsmet", Optional.empty(), + stubReportConfig(), Optional.empty(), Optional.empty(), Optional.empty(), @@ -706,7 +715,7 @@ void multiParticipants_evenSpread() throws IOException { Optional.empty()))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); - when(repoMock.issuesWithLabelLastUpdatedBefore("needs-triage", Set.of(), now)) + when(repoMock.issuesWithLabelLastUpdatedBefore("triage/needs-triage", Set.of(), now)) .thenAnswer(ignored -> stubIssueList(1, 3, 2, 4).stream()); mockNotifiable("yrodiere", ZoneOffset.UTC); diff --git a/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java b/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java index b7af66a..766b9cc 100644 --- a/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java +++ b/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java @@ -1,6 +1,7 @@ package io.quarkus.github.lottery; import static io.quarkus.github.lottery.util.MockHelper.stubIssueList; +import static io.quarkus.github.lottery.util.MockHelper.stubReportConfig; import static org.assertj.core.api.Assertions.assertThat; import java.io.IOException; @@ -16,11 +17,11 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import io.quarkus.github.lottery.github.GitHubInstallationRef; import org.mockito.junit.jupiter.MockitoExtension; import io.quarkus.github.lottery.draw.DrawRef; import io.quarkus.github.lottery.draw.LotteryReport; +import io.quarkus.github.lottery.github.GitHubInstallationRef; import io.quarkus.github.lottery.github.GitHubRepositoryRef; import io.quarkus.github.lottery.message.MessageFormatter; import io.quarkus.test.junit.QuarkusTest; @@ -71,6 +72,7 @@ void formatNotificationTopicText() { @Test void formatNotificationTopicSuffixText() { var lotteryReport = new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); assertThat(messageFormatter.formatNotificationTopicSuffixText(lotteryReport)) @@ -80,6 +82,7 @@ void formatNotificationTopicSuffixText() { @Test void formatNotificationTopicSuffixText_exoticTimezone() { var lotteryReport = new LotteryReport(drawRef, "yrodiere", Optional.of(ZoneId.of("America/Los_Angeles")), + stubReportConfig(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); assertThat(messageFormatter.formatNotificationTopicSuffixText(lotteryReport)) @@ -89,6 +92,7 @@ void formatNotificationTopicSuffixText_exoticTimezone() { @Test void formatNotificationBodyMarkdown_triage_empty() { var lotteryReport = new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig(), Optional.of(new LotteryReport.Bucket(List.of())), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); @@ -98,6 +102,7 @@ void formatNotificationBodyMarkdown_triage_empty() { Hey @yrodiere, here's your report for quarkusio/quarkus on 2017-11-06T06:00:00Z. # Triage + No issues in this category this time. --- @@ -110,6 +115,7 @@ void formatNotificationBodyMarkdown_triage_empty() { @Test void formatNotificationBodyMarkdown_triage_simple() { var lotteryReport = new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig(), Optional.of(new LotteryReport.Bucket(stubIssueList(1, 3))), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); @@ -119,6 +125,9 @@ void formatNotificationBodyMarkdown_triage_simple() { Hey @yrodiere, here's your report for quarkusio/quarkus on 2017-11-06T06:00:00Z. # Triage + + Issues that haven't been assigned an area yet. Please add an area label, remove the `triage/needs-triage` label, optionally ping maintainers. + - [#1](http://github.com/quarkusio/quarkus/issues/1) Title for issue 1 - [#3](http://github.com/quarkusio/quarkus/issues/3) Title for issue 3 @@ -132,6 +141,7 @@ void formatNotificationBodyMarkdown_triage_simple() { @Test void formatNotificationBodyMarkdown_maintenance_empty() { var lotteryReport = new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig("area/hibernate-orm", "area/hibernate-search"), Optional.empty(), Optional.of(new LotteryReport.Bucket(List.of())), Optional.of(new LotteryReport.Bucket(List.of())), @@ -142,11 +152,18 @@ void formatNotificationBodyMarkdown_maintenance_empty() { """ Hey @yrodiere, here's your report for quarkusio/quarkus on 2017-11-06T06:00:00Z. - # Feedback needed (reproducer, information, ...) + Maintenance areas: `area/hibernate-orm`/`area/hibernate-search`. + + # Feedback needed + No issues in this category this time. - # Feedback provided (reproducer, information, ...) + + # Feedback provided + No issues in this category this time. + # Stale + No issues in this category this time. --- @@ -159,6 +176,7 @@ void formatNotificationBodyMarkdown_maintenance_empty() { @Test void formatNotificationBodyMarkdown_maintenance_someEmpty() { var lotteryReport = new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig("area/hibernate-orm", "area/hibernate-search"), Optional.empty(), Optional.of(new LotteryReport.Bucket(stubIssueList(1, 3))), Optional.of(new LotteryReport.Bucket(List.of())), @@ -169,12 +187,21 @@ void formatNotificationBodyMarkdown_maintenance_someEmpty() { """ Hey @yrodiere, here's your report for quarkusio/quarkus on 2017-11-06T06:00:00Z. - # Feedback needed (reproducer, information, ...) + Maintenance areas: `area/hibernate-orm`/`area/hibernate-search`. + + # Feedback needed + + Issues with missing reproducer/information. Please ping the reporter, or close the issue if it's taking too long. + - [#1](http://github.com/quarkusio/quarkus/issues/1) Title for issue 1 - [#3](http://github.com/quarkusio/quarkus/issues/3) Title for issue 3 - # Feedback provided (reproducer, information, ...) + + # Feedback provided + No issues in this category this time. + # Stale + No issues in this category this time. --- @@ -187,6 +214,7 @@ void formatNotificationBodyMarkdown_maintenance_someEmpty() { @Test void formatNotificationBodyMarkdown_maintenance_simple() { var lotteryReport = new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig("area/hibernate-orm", "area/hibernate-search"), Optional.empty(), Optional.of(new LotteryReport.Bucket(stubIssueList(1, 3))), Optional.of(new LotteryReport.Bucket(stubIssueList(4, 5))), @@ -197,13 +225,26 @@ void formatNotificationBodyMarkdown_maintenance_simple() { """ Hey @yrodiere, here's your report for quarkusio/quarkus on 2017-11-06T06:00:00Z. - # Feedback needed (reproducer, information, ...) + Maintenance areas: `area/hibernate-orm`/`area/hibernate-search`. + + # Feedback needed + + Issues with missing reproducer/information. Please ping the reporter, or close the issue if it's taking too long. + - [#1](http://github.com/quarkusio/quarkus/issues/1) Title for issue 1 - [#3](http://github.com/quarkusio/quarkus/issues/3) Title for issue 3 - # Feedback provided (reproducer, information, ...) + + # Feedback provided + + Issues with newly provided reproducer/information. Please have a closer look, possibly remove the `triage/needs-reproducer`/`triage/needs-feedback` label, and plan further work. + - [#4](http://github.com/quarkusio/quarkus/issues/4) Title for issue 4 - [#5](http://github.com/quarkusio/quarkus/issues/5) Title for issue 5 + # Stale + + Issues last updated a long time ago. Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ... + - [#2](http://github.com/quarkusio/quarkus/issues/2) Title for issue 2 - [#7](http://github.com/quarkusio/quarkus/issues/7) Title for issue 7 @@ -217,6 +258,7 @@ void formatNotificationBodyMarkdown_maintenance_simple() { @Test void formatNotificationBodyMarkdown_stewardship_empty() { var lotteryReport = new LotteryReport(drawRef, "geoand", Optional.empty(), + stubReportConfig(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket(List.of()))); @@ -226,6 +268,7 @@ void formatNotificationBodyMarkdown_stewardship_empty() { Hey @geoand, here's your report for quarkusio/quarkus on 2017-11-06T06:00:00Z. # Stewardship + No issues in this category this time. --- @@ -238,6 +281,7 @@ void formatNotificationBodyMarkdown_stewardship_empty() { @Test void formatNotificationBodyMarkdown_stewardship_simple() { var lotteryReport = new LotteryReport(drawRef, "geoand", Optional.empty(), + stubReportConfig(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket(stubIssueList(1, 3)))); @@ -247,6 +291,9 @@ void formatNotificationBodyMarkdown_stewardship_simple() { Hey @geoand, here's your report for quarkusio/quarkus on 2017-11-06T06:00:00Z. # Stewardship + + Issues across all areas last updated a long time ago. Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ... + - [#1](http://github.com/quarkusio/quarkus/issues/1) Title for issue 1 - [#3](http://github.com/quarkusio/quarkus/issues/3) Title for issue 3 @@ -260,6 +307,7 @@ void formatNotificationBodyMarkdown_stewardship_simple() { @Test void formatNotificationBodyMarkdown_all() { var lotteryReport = new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig(), Optional.of(new LotteryReport.Bucket(stubIssueList(1, 3))), Optional.of(new LotteryReport.Bucket(stubIssueList(4, 5))), Optional.of(new LotteryReport.Bucket(stubIssueList(2, 7))), @@ -271,18 +319,37 @@ void formatNotificationBodyMarkdown_all() { Hey @yrodiere, here's your report for quarkusio/quarkus on 2017-11-06T06:00:00Z. # Triage + + Issues that haven't been assigned an area yet. Please add an area label, remove the `triage/needs-triage` label, optionally ping maintainers. + - [#1](http://github.com/quarkusio/quarkus/issues/1) Title for issue 1 - [#3](http://github.com/quarkusio/quarkus/issues/3) Title for issue 3 - # Feedback needed (reproducer, information, ...) + + # Feedback needed + + Issues with missing reproducer/information. Please ping the reporter, or close the issue if it's taking too long. + - [#4](http://github.com/quarkusio/quarkus/issues/4) Title for issue 4 - [#5](http://github.com/quarkusio/quarkus/issues/5) Title for issue 5 - # Feedback provided (reproducer, information, ...) + + # Feedback provided + + Issues with newly provided reproducer/information. Please have a closer look, possibly remove the `triage/needs-reproducer`/`triage/needs-feedback` label, and plan further work. + - [#2](http://github.com/quarkusio/quarkus/issues/2) Title for issue 2 - [#7](http://github.com/quarkusio/quarkus/issues/7) Title for issue 7 + # Stale + + Issues last updated a long time ago. Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ... + - [#8](http://github.com/quarkusio/quarkus/issues/8) Title for issue 8 - [#9](http://github.com/quarkusio/quarkus/issues/9) Title for issue 9 + # Stewardship + + Issues across all areas last updated a long time ago. Please have a closer look, re-prioritize, ping someone, label as "on ice", close the issue, ... + - [#10](http://github.com/quarkusio/quarkus/issues/10) Title for issue 10 - [#11](http://github.com/quarkusio/quarkus/issues/11) Title for issue 11 @@ -296,6 +363,7 @@ void formatNotificationBodyMarkdown_all() { @Test void formatNotificationBodyMarkdown_exoticTimezone() { var lotteryReport = new LotteryReport(drawRef, "yrodiere", Optional.of(ZoneId.of("America/Los_Angeles")), + stubReportConfig(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); diff --git a/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java b/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java index f0c3f01..7f3dcea 100644 --- a/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java @@ -1,6 +1,7 @@ package io.quarkus.github.lottery; import static io.quarkus.github.lottery.util.MockHelper.stubIssueList; +import static io.quarkus.github.lottery.util.MockHelper.stubReportConfig; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; @@ -117,6 +118,7 @@ void send() throws IOException { verifyNoMoreInteractions(gitHubServiceMock, notificationRepoMock, messageFormatterMock); var lotteryReport1 = new LotteryReport(drawRef, "yrodiere", Optional.empty(), + stubReportConfig("area/hibernate-orm", "area/hibernate-search"), Optional.of(new LotteryReport.Bucket(stubIssueList(1, 3))), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); @@ -133,6 +135,7 @@ void send() throws IOException { verifyNoMoreInteractions(gitHubServiceMock, notificationRepoMock, messageFormatterMock); var lotteryReport2 = new LotteryReport(drawRef, "gsmet", Optional.empty(), + stubReportConfig("area/hibernate-validator"), Optional.empty(), Optional.of(new LotteryReport.Bucket(stubIssueList(4, 5))), Optional.of(new LotteryReport.Bucket(stubIssueList(7, 8))), @@ -151,6 +154,7 @@ void send() throws IOException { verifyNoMoreInteractions(gitHubServiceMock, notificationRepoMock, messageFormatterMock); var lotteryReport3 = new LotteryReport(drawRef, "geoand", Optional.empty(), + stubReportConfig(), Optional.of(new LotteryReport.Bucket(stubIssueList(11, 12))), Optional.empty(), Optional.empty(), @@ -169,6 +173,7 @@ void send() throws IOException { verifyNoMoreInteractions(gitHubServiceMock, notificationRepoMock, messageFormatterMock); var lotteryReport4 = new LotteryReport(drawRef, "jsmith", Optional.empty(), + stubReportConfig(), Optional.of(new LotteryReport.Bucket(stubIssueList())), Optional.empty(), Optional.empty(), diff --git a/src/test/java/io/quarkus/github/lottery/draw/LotteryReportTest.java b/src/test/java/io/quarkus/github/lottery/draw/LotteryReportTest.java index 845021e..3371e33 100644 --- a/src/test/java/io/quarkus/github/lottery/draw/LotteryReportTest.java +++ b/src/test/java/io/quarkus/github/lottery/draw/LotteryReportTest.java @@ -1,6 +1,7 @@ package io.quarkus.github.lottery.draw; import static io.quarkus.github.lottery.util.MockHelper.stubIssueList; +import static io.quarkus.github.lottery.util.MockHelper.stubReportConfig; import static org.assertj.core.api.Assertions.assertThat; import java.time.LocalDateTime; @@ -36,6 +37,7 @@ void setup() { void buckets() { List buckets = new ArrayList<>(); var report = new LotteryReport(drawRef, "geoand", Optional.empty(), + stubReportConfig(), newBucket(buckets), newBucket(buckets), newBucket(buckets), 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 9a56fc4..92ea906 100644 --- a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java +++ b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java @@ -9,9 +9,11 @@ import java.net.MalformedURLException; import java.net.URL; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.stream.IntStream; +import io.quarkus.github.lottery.draw.LotteryReport; import org.kohsuke.github.GHIssue; import org.kohsuke.github.GHIssueComment; import org.kohsuke.github.GHIssueEvent; @@ -63,6 +65,12 @@ private static Issue stubIssue(int number) { return new Issue(number, "Title for issue " + number, url(number)); } + public static LotteryReport.Config stubReportConfig(String... maintenanceLabels) { + return new LotteryReport.Config("triage/needs-triage", + new LinkedHashSet<>(List.of("triage/needs-reproducer", "triage/needs-feedback")), + new LinkedHashSet<>(List.of(maintenanceLabels))); + } + public static GHIssue mockIssueForLottery(GitHubMockContext context, int number) throws IOException { GHIssue mock = context.issue(10000L + number);