From 8ca2b730cffb45821e5b5faa8fd0a87af844ea11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Wed, 11 Dec 2024 14:43:39 +0100 Subject: [PATCH 1/3] Cache retrieval of user permissions Because if we're going to inspect comments, it's going to be a lot of inspected users. --- .../lottery/github/GitHubRepository.java | 23 +++++++++++++++---- .../github/lottery/util/MockHelper.java | 2 +- 2 files changed, 19 insertions(+), 6 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 167f77f..9ad4267 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -14,6 +14,7 @@ import java.io.IOException; import java.sql.Date; import java.time.Clock; +import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.HashMap; @@ -31,9 +32,11 @@ import org.kohsuke.github.GHIssueSearchBuilder; import org.kohsuke.github.GHIssueState; import org.kohsuke.github.GHRepository; -import org.kohsuke.github.GHUser; import org.kohsuke.github.GitHub; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; + import io.quarkiverse.githubapp.ConfigFile; import io.quarkiverse.githubapp.GitHubClientProvider; import io.quarkiverse.githubapp.GitHubConfigFileProvider; @@ -54,6 +57,7 @@ public class GitHubRepository implements AutoCloseable { private final GitHubConfigFileProvider configFileProvider; private final MessageFormatter messageFormatter; private final GitHubRepositoryRef ref; + private final LoadingCache noContextIssueActionSideCache; private GitHub client; private GHRepository repository; @@ -66,6 +70,10 @@ public GitHubRepository(Clock clock, GitHubClientProvider clientProvider, GitHub this.configFileProvider = configFileProvider; this.messageFormatter = messageFormatter; this.ref = ref; + this.noContextIssueActionSideCache = Caffeine.newBuilder() + .maximumSize(100) + .expireAfterWrite(Duration.ofMinutes(1)) + .build(this::computeNoContextIssueActionSide); } @Override @@ -211,18 +219,23 @@ private IssueActionSide lastActionSide(GHIssue ghIssue, Set initialActio // No action since the label was assigned. return IssueActionSide.TEAM; } - return getIssueActionSide(ghIssue, lastComment.get().getUser()); + return getIssueActionSide(ghIssue, lastComment.get().getUser().getLogin()); } - private IssueActionSide getIssueActionSide(GHIssue issue, GHUser user) throws IOException { - if (issue.getUser().getLogin().equals(user.getLogin())) { + private IssueActionSide getIssueActionSide(GHIssue issue, String login) throws IOException { + if (issue.getUser().getLogin().equals(login)) { // 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)) { + // Caching in case a same user is encountered multiple times in the same run. + return noContextIssueActionSideCache.get(login); + } + + private IssueActionSide computeNoContextIssueActionSide(String login) throws IOException { + return switch (repository().getPermission(login)) { case ADMIN, WRITE, UNKNOWN -> IssueActionSide.TEAM; // "Unknown" includes "triage" case READ, NONE -> IssueActionSide.OUTSIDER; }; 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 c7ea255..d2568d5 100644 --- a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java +++ b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java @@ -225,7 +225,7 @@ public static GHUser mockUserForInspectedComments(GitHubMockContext context, GHR GHUser mock = context.ghObject(GHUser.class, id); when(mock.getLogin()).thenReturn(login); if (permissionType != null) { - when(repositoryMock.getPermission(mock)).thenReturn(permissionType); + when(repositoryMock.getPermission(login)).thenReturn(permissionType); } return mock; } From b9ec0284e304caac6a14c9e1736e6bf313451708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Thu, 12 Dec 2024 13:04:14 +0100 Subject: [PATCH 2/3] Add missing assertions in tests --- .../java/io/quarkus/github/lottery/GitHubServiceTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index 7929c69..dae1c80 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -674,11 +674,14 @@ void issuesLastActedOnByAndLastUpdatedBefore_team() throws IOException { }) .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); verify(searchIssuesBuilderMock).q("label:triage/needs-feedback,triage/needs-reproducer"); verify(searchIssuesBuilderMock).q("label:area/hibernate-search"); + verifyNoMoreInteractions(searchIssuesBuilderMock); verify(issue1QueryCommentsBuilderMock).since(issue1ActionLabelEvent); verify(issue2QueryCommentsBuilderMock).since(issue2ActionLabelEvent); @@ -845,11 +848,14 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { }) .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); verify(searchIssuesBuilderMock).q("label:triage/needs-feedback,triage/needs-reproducer"); verify(searchIssuesBuilderMock).q("label:area/hibernate-search"); + verifyNoMoreInteractions(searchIssuesBuilderMock); verify(issue1QueryCommentsBuilderMock).since(issue1ActionLabelEvent); verify(issue2QueryCommentsBuilderMock).since(issue2ActionLabelEvent); From 455f83cbd53bc198e5f377bf09de5daec2ce1484 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Thu, 12 Dec 2024 11:41:12 +0100 Subject: [PATCH 3/3] New maintenance category for newly created issues/PRs --- README.adoc | 46 +++++- .../github/lottery/LotteryService.java | 20 ++- .../github/lottery/config/LotteryConfig.java | 17 +++ .../quarkus/github/lottery/draw/Lottery.java | 42 +++++- .../github/lottery/draw/LotteryReport.java | 5 +- .../github/lottery/draw/Participant.java | 16 ++- .../lottery/github/GitHubRepository.java | 53 ++++++- .../lottery/github/GitHubSearchClauses.java | 34 ++++- .../lottery/history/LotteryHistory.java | 7 + .../templates/MessageFormatter/historyBody.md | 4 + .../MessageFormatter/notificationBody.md | 9 ++ .../github/lottery/GitHubServiceTest.java | 132 ++++++++++++++++++ .../github/lottery/HistoryServiceTest.java | 16 ++- .../lottery/LotterySingleRepositoryTest.java | 72 ++++++++-- .../github/lottery/MessageFormatterTest.java | 105 ++++++++++++-- .../lottery/NotificationServiceTest.java | 3 + .../lottery/PullRequestConfigCheckTest.java | 8 ++ .../lottery/draw/LotteryReportTest.java | 1 + .../github/lottery/util/MockHelper.java | 22 +++ 19 files changed, 567 insertions(+), 45 deletions(-) diff --git a/README.adoc b/README.adoc index aeca79d..ccd984f 100644 --- a/README.adoc +++ b/README.adoc @@ -135,8 +135,12 @@ If the `maintenance` section is present, you will get notified about issues/PRs related to a specific area (e.g. `area/hibernate-orm`) that may be stalled and require intervention from maintainers or reporters. -Issues/PRs in "maintenance" notifications will be split in three categories: +Issues/PRs in "maintenance" notifications will be split in several categories: +Created:: +Issues or PRs that just got created in your area. ++ +Please review, ask for reproducer/information, or plan future work. Feedback Needed:: Issues with missing reproducer/information. + @@ -164,6 +168,8 @@ participants: maintenance: labels: ["area/hibernate-orm", "area/hibernate-search", "area/elasticsearch"] days: ["MONDAY", "TUESDAY", "WEDNESDAY", "THURSDAY", "FRIDAY"] + created: + maxIssues: 5 feedback: needed: maxIssues: 4 @@ -182,21 +188,26 @@ Array of Strings, mandatory, no default. On which days you wish to get notified about maintenance. + Array of ``WeekDay``s, mandatory, no default. +`created.maxIssues`:: +How many issues/PRs, at most, you wish to be included in the "Created" category +for each notification. ++ +Integer, mandatory if the `created` section is present, no default. `feedback.needed.maxIssues`:: How many issues/PRs, at most, you wish to be included in the "Feedback needed" category for each notification. + -Integer, mandatory, no default. +Integer, mandatory if the `feedback` section is present, no default. `feedback.provided.maxIssues`:: How many issues/PRs, at most, you wish to be included in the "Feedback provided" category for each notification. + -Integer, mandatory, no default. +Integer, mandatory if the `feedback` section is present, no default. `stale.maxIssues`:: How many issues/PRs, at most, you wish to be included in the "Stale" category for each notification. + -Integer, mandatory, no default. +Integer, mandatory if the `stale` section is present, no default. [[participants-stewardship]] === Stewardship @@ -291,6 +302,11 @@ buckets: delay: PT0S timeout: P3D maintenance: + created: + delay: PT0S + timeout: P1D + expiry: P14D + ignoreLabels: ["triage/on-ice"] feedback: labels: ["triage/needs-reproducer"] needed: @@ -328,6 +344,28 @@ How much time to wait after an issue/PR was last notified about before including it again in the lottery in the "triage" bucket. + String in https://en.wikipedia.org/wiki/ISO_8601#Durations[ISO-8601 duration format], mandatory, no default. ++ +`buckets.maintenance.created.delay`:: +How much time to wait after the creation of an issue/PR +before including it in the lottery in the "created" bucket. ++ +String in https://en.wikipedia.org/wiki/ISO_8601#Durations[ISO-8601 duration format], mandatory, no default. +`buckets.maintenance.created.timeout`:: +How much time to wait after an issue/PR was last notified about +before including it again in the lottery in the "created" bucket. ++ +String in https://en.wikipedia.org/wiki/ISO_8601#Durations[ISO-8601 duration format], mandatory, no default. ++ +`buckets.maintenance.created.expiry`:: +How much time to wait after the creation of an issue/PR +before excluding it from the lottery in the "created" bucket. ++ +String in https://en.wikipedia.org/wiki/ISO_8601#Durations[ISO-8601 duration format], mandatory, no default. +`buckets.maintenance.created.ignoreLabels`:: +The labels identifying GitHub issues/PRs that should be ignored for the "created" bucket. +Issues/PRs with one of these labels will never be added to the bucket. ++ +Array of Strings, optional, defaults to an empty array. `buckets.maintenance.feedback.labels`:: The labels identifying GitHub issues for which feedback (a reproducer, more information, ...) was requested. + diff --git a/src/main/java/io/quarkus/github/lottery/LotteryService.java b/src/main/java/io/quarkus/github/lottery/LotteryService.java index 5a158ee..00da1ed 100644 --- a/src/main/java/io/quarkus/github/lottery/LotteryService.java +++ b/src/main/java/io/quarkus/github/lottery/LotteryService.java @@ -7,13 +7,18 @@ import java.time.ZoneOffset; import java.time.ZonedDateTime; import java.util.ArrayList; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; +import java.util.Set; import jakarta.enterprise.context.ApplicationScoped; import jakarta.inject.Inject; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.quarkus.github.lottery.config.LotteryConfig; import io.quarkus.github.lottery.draw.DrawRef; import io.quarkus.github.lottery.draw.Lottery; @@ -32,6 +37,7 @@ @ApplicationScoped public class LotteryService { + private static final Logger log = LoggerFactory.getLogger(LotteryService.class); @Inject GitHubService gitHubService; @@ -81,7 +87,19 @@ private void doDrawForRepository(GitHubRepository repo, LotteryConfig lotteryCon var now = Instant.now(clock); var drawRef = new DrawRef(repo.ref(), now); - Lottery lottery = new Lottery(now, lotteryConfig.buckets()); + // Note: this map only gives partial information -- some maintainers may not be registered for the lottery. + // That's why the information is only used for optimization (to skip issues that we know for sure aren't relevant). + var maintainerUsernamesByAreaLabel = new HashMap>(); + for (LotteryConfig.Participant participant : lotteryConfig.participants()) { + participant.maintenance().ifPresent(m -> { + for (String label : m.labels()) { + maintainerUsernamesByAreaLabel.computeIfAbsent(label, key -> new LinkedHashSet<>()) + .add(participant.username()); + } + }); + } + + Lottery lottery = new Lottery(now, lotteryConfig.buckets(), maintainerUsernamesByAreaLabel); try (var notifier = notificationService.notifier(drawRef, lotteryConfig.notifications())) { var history = historyService.fetch(drawRef, lotteryConfig); 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 4693076..3b4a4f4 100644 --- a/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java +++ b/src/main/java/io/quarkus/github/lottery/config/LotteryConfig.java @@ -58,9 +58,25 @@ public Triage(@JsonProperty(required = true) String label, } public record Maintenance( + @JsonProperty(required = true) Created created, @JsonProperty(required = true) Feedback feedback, @JsonProperty(required = true) Stale stale) { + public record Created( + @JsonUnwrapped @JsonProperty(access = JsonProperty.Access.READ_ONLY) Notification notification, + @JsonProperty(required = true) Duration expiry, + @JsonProperty(required = true) List ignoreLabels) { + // https://stackoverflow.com/a/71539100/6692043 + // Also gives us a less verbose constructor for tests + @JsonCreator + public Created(@JsonProperty(required = true) Duration delay, + @JsonProperty(required = true) Duration timeout, + @JsonProperty(required = true) Duration expiry, + @JsonProperty(required = false) List ignoreLabels) { + this(new Notification(delay, timeout), expiry, ignoreLabels == null ? List.of() : ignoreLabels); + } + } + public record Feedback( @JsonProperty(required = true) List labels, @JsonProperty(required = true) Needed needed, @@ -141,6 +157,7 @@ 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, + @JsonProperty Optional created, Optional feedback, @JsonProperty Optional stale) { public record Feedback( 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 c15a96e..c3be040 100644 --- a/src/main/java/io/quarkus/github/lottery/draw/Lottery.java +++ b/src/main/java/io/quarkus/github/lottery/draw/Lottery.java @@ -27,14 +27,16 @@ public final class Lottery { private final Instant now; private final LotteryConfig.Buckets config; + private final Map> maintainerUsernamesByAreaLabel; private final Random random; private final Triage triage; private final Map maintenanceByLabel; private final Stewardship stewardship; - public Lottery(Instant now, LotteryConfig.Buckets config) { + public Lottery(Instant now, LotteryConfig.Buckets config, Map> maintainerUsernamesByAreaLabel) { this.now = now; this.config = config; + this.maintainerUsernamesByAreaLabel = maintainerUsernamesByAreaLabel; this.random = new Random(); this.triage = new Triage(); this.maintenanceByLabel = new LinkedHashMap<>(); @@ -46,7 +48,11 @@ Bucket triage() { } Maintenance maintenance(String areaLabel) { - return maintenanceByLabel.computeIfAbsent(areaLabel, Maintenance::new); + return maintenanceByLabel.computeIfAbsent(areaLabel, this::createMaintenance); + } + + private Maintenance createMaintenance(String areaLabel) { + return new Maintenance(areaLabel, maintainerUsernamesByAreaLabel.getOrDefault(areaLabel, Set.of())); } Bucket stewardship() { @@ -105,18 +111,30 @@ void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List maintainerUsernames; + private final Bucket created; private final Bucket feedbackNeeded; private final Bucket feedbackProvided; private final Bucket stale; - Maintenance(String areaLabel) { + Maintenance(String areaLabel, Set maintainerUsernames) { this.areaLabel = areaLabel; + this.maintainerUsernames = maintainerUsernames; String namePrefix = "maintenance - '" + areaLabel + "' - "; + created = new Bucket(namePrefix + "created"); feedbackNeeded = new Bucket(namePrefix + "feedbackNeeded"); feedbackProvided = new Bucket(namePrefix + "feedbackProvided"); stale = new Bucket(namePrefix + "stale"); } + public void addMaintainer(String username) { + maintainerUsernames.add(username); + } + + Bucket created() { + return created; + } + Bucket feedbackNeeded() { return feedbackNeeded; } @@ -131,6 +149,24 @@ Bucket stale() { void createDraws(GitHubRepository repo, LotteryHistory lotteryHistory, List draws, Set allWinnings) throws IOException { + if (created.hasParticipation()) { + var maxCutoff = now.minus(config.maintenance().created().notification().delay()); + var minCutoff = now.minus(config.maintenance().created().expiry()); + // Remove duplicates, but preserve order + var ignoreLabels = new LinkedHashSet(); + // Ignore issues with feedback request labels, + // since they evidently got some attention from the team already. + ignoreLabels.addAll(config.maintenance().feedback().labels()); + ignoreLabels.addAll(config.maintenance().created().ignoreLabels()); + var history = lotteryHistory.created(); + draws.add(created.createDraw( + repo.issuesOrPullRequestsNeverActedOnByTeamAndCreatedBetween(areaLabel, ignoreLabels, + maintainerUsernames, minCutoff, + maxCutoff) + .filter(issue -> history.lastNotificationTimedOutForIssueNumber(issue.number())) + .iterator(), + allWinnings)); + } // Remove duplicates, but preserve order Set needFeedbackLabels = new LinkedHashSet<>(config.maintenance().feedback().labels()); if (feedbackNeeded.hasParticipation()) { 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 cd63b6f..d7962aa 100644 --- a/src/main/java/io/quarkus/github/lottery/draw/LotteryReport.java +++ b/src/main/java/io/quarkus/github/lottery/draw/LotteryReport.java @@ -20,13 +20,14 @@ public record LotteryReport( Optional timezone, Config config, Optional triage, + Optional created, Optional feedbackNeeded, Optional feedbackProvided, Optional stale, Optional stewardship) { Stream buckets() { - return Stream.of(triage, feedbackNeeded, feedbackProvided, stale, stewardship) + return Stream.of(triage, created, feedbackNeeded, feedbackProvided, stale, stewardship) .filter(Optional::isPresent) .map(Optional::get); } @@ -59,6 +60,7 @@ public record Serialized( public Serialized serialized() { return new Serialized(drawRef.instant(), username, triage.map(Bucket::serialized), + created.map(Bucket::serialized), feedbackNeeded.map(Bucket::serialized), feedbackProvided.map(Bucket::serialized), stale.map(Bucket::serialized), @@ -69,6 +71,7 @@ public record Serialized( Instant instant, String username, Optional triage, + Optional created, @JsonAlias("reproducerNeeded") Optional feedbackNeeded, @JsonAlias("reproducerProvided") Optional feedbackProvided, Optional stale, 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 eef5344..a24504a 100644 --- a/src/main/java/io/quarkus/github/lottery/draw/Participant.java +++ b/src/main/java/io/quarkus/github/lottery/draw/Participant.java @@ -84,6 +84,7 @@ public LotteryReport report(String triageLabel, Set feedbackLabels) { feedbackLabels, maintenance.map(m -> m.labels).orElseGet(Set::of)), triage.map(Participation::issues).map(LotteryReport.Bucket::new), + maintenance.flatMap(m -> m.created).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), maintenance.flatMap(m -> m.stale).map(Participation::issues).map(LotteryReport.Bucket::new), @@ -92,6 +93,8 @@ public LotteryReport report(String triageLabel, Set feedbackLabels) { private static final class Maintenance { public static Optional create(String username, LotteryConfig.Participant.Maintenance config) { + var created = config.created() + .flatMap(p -> Participation.create(username, p)); 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) @@ -99,24 +102,30 @@ public static Optional create(String username, LotteryConfig.Partic var stale = config.stale() .flatMap(p -> Participation.create(username, p)); - if (feedbackNeeded.isEmpty() && feedbackProvided.isEmpty() && stale.isEmpty()) { + if (created.isEmpty() && feedbackNeeded.isEmpty() && feedbackProvided.isEmpty() && stale.isEmpty()) { return Optional.empty(); } - return Optional.of(new Maintenance(config.labels(), feedbackNeeded, feedbackProvided, stale)); + return Optional.of(new Maintenance(username, config.labels(), created, feedbackNeeded, feedbackProvided, stale)); } + private final String username; private final Set labels; + private final Optional created; private final Optional feedbackNeeded; private final Optional feedbackProvided; private final Optional stale; - private Maintenance(List labels, Optional feedbackNeeded, + private Maintenance(String username, List labels, + Optional created, + Optional feedbackNeeded, Optional feedbackProvided, Optional stale) { + this.username = username; // Remove duplicates, but preserve order this.labels = new LinkedHashSet<>(labels); + this.created = created; this.feedbackNeeded = feedbackNeeded; this.feedbackProvided = feedbackProvided; this.stale = stale; @@ -125,6 +134,7 @@ private Maintenance(List labels, Optional feedbackNeeded, public void participate(Lottery lottery) { for (String label : labels) { Lottery.Maintenance maintenance = lottery.maintenance(label); + created.ifPresent(maintenance.created()::participate); feedbackNeeded.ifPresent(maintenance.feedbackNeeded()::participate); feedbackProvided.ifPresent(maintenance.feedbackProvided()::participate); stale.ifPresent(maintenance.stale()::participate); 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 9ad4267..27f9f09 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubRepository.java @@ -3,11 +3,13 @@ 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.commenter; +import static io.quarkus.github.lottery.github.GitHubSearchClauses.created; 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.github.GitHubSearchClauses.updated; import static io.quarkus.github.lottery.util.Streams.toStream; import static io.quarkus.github.lottery.util.UncheckedIOFunction.uncheckedIO; @@ -22,6 +24,7 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.function.Predicate; import java.util.stream.Stream; import org.kohsuke.github.GHDirection; @@ -143,7 +146,7 @@ public Optional fetchLotteryConfig() throws IOException { public Stream issuesOrPullRequestsLastUpdatedBefore(Set ignoreLabels, Instant updatedBefore) { var builder = searchIssuesOrPullRequests() .isOpen() - .q(updatedBefore(updatedBefore)) + .q(updated(null, updatedBefore)) .sort(GHIssueSearchBuilder.Sort.UPDATED) .order(GHDirection.DESC); if (!ignoreLabels.isEmpty()) { @@ -166,7 +169,7 @@ public Stream issuesOrPullRequestsWithLabelLastUpdatedBefore(String label var builder = searchIssuesOrPullRequests() .isOpen() .q(label(label)) - .q(updatedBefore(updatedBefore)) + .q(updated(null, updatedBefore)) .sort(GHIssueSearchBuilder.Sort.UPDATED) .order(GHDirection.DESC); if (!ignoreLabels.isEmpty()) { @@ -193,7 +196,7 @@ public Stream issuesLastActedOnByAndLastUpdatedBefore(Set initial .isOpen() .q(anyLabel(initialActionLabels)) .q(label(filterLabel)) - .q(updatedBefore(updatedBefore)) + .q(updated(null, updatedBefore)) .sort(GHIssueSearchBuilder.Sort.UPDATED) .order(GHDirection.DESC) .list()) @@ -202,6 +205,42 @@ public Stream issuesLastActedOnByAndLastUpdatedBefore(Set initial .map(toIssueRecord()); } + /** + * Lists issues or pull requests with the given labels that were never acted on (commented) + * by the team and were last updated before the given instant. + * + * @param filterLabel A secondary GitHub label; all returned issues must have been assigned that label. + * This label is not relevant to determining the last action. + * @param ignoreLabels GitHub labels. Issues assigned with any of these labels are ignored (not returned). + * @param ignoreCommentedBy GitHub usernames. Issues commented on by any of these users are ignored (not returned). + * @param createdAfter An instant; all returned issues must have been created after that instant. + * @param createdBefore An instant; all returned issues must have been created before that instant. + * @return A lazily populated stream of matching issues. + * @throws java.io.UncheckedIOException In case of I/O failure. + */ + public Stream issuesOrPullRequestsNeverActedOnByTeamAndCreatedBetween(String filterLabel, + Set ignoreLabels, Set ignoreCommentedBy, + Instant createdAfter, Instant createdBefore) { + var builder = searchIssuesOrPullRequests() + .isOpen() + .q(label(filterLabel)) + .q(not(anyLabel(ignoreLabels))) + .q(created(createdAfter, createdBefore)) + .sort(GHIssueSearchBuilder.Sort.CREATED) + .order(GHDirection.ASC); + for (String username : ignoreCommentedBy) { + builder.q(not(commenter(username))); + } + return toStream(builder.list()) + // Note: "ignoreCommentedBy" cannot be assumed to be the full list of maintainers, + // so we use "ignoreCommentedBy" for optimization + // (to skip issues that we know for sure aren't relevant), + // but we still need to have a closer look at issues afterward + // (to skip issues commented on by team members which are not in "ignoreCommentedBy"). + .filter(this::hasNoTeamAction) + .map(toIssueRecord()); + } + private IssueActionSide lastActionSide(GHIssue ghIssue, Set initialActionLabels) throws IOException { // Optimization: don't even fetch older comments as they wouldn't affect the result // (we're looking for the *last* action). @@ -222,6 +261,12 @@ private IssueActionSide lastActionSide(GHIssue ghIssue, Set initialActio return getIssueActionSide(ghIssue, lastComment.get().getUser().getLogin()); } + private boolean hasNoTeamAction(GHIssue ghIssue) { + return getNonBotCommentsSince(ghIssue, null) + .map(uncheckedIO((GHIssueComment c) -> getIssueActionSide(ghIssue, c.getUser().getLogin()))) + .noneMatch(Predicate.isEqual(IssueActionSide.TEAM)); + } + private IssueActionSide getIssueActionSide(GHIssue issue, String login) throws IOException { if (issue.getUser().getLogin().equals(login)) { // This is the reporter; even if part of the team, 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 abd1007..80dc616 100644 --- a/src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java +++ b/src/main/java/io/quarkus/github/lottery/github/GitHubSearchClauses.java @@ -3,9 +3,15 @@ import java.time.Instant; import java.time.ZoneOffset; import java.util.Set; +import java.util.function.Function; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public final class GitHubSearchClauses { + private static final Logger log = LoggerFactory.getLogger(GitHubSearchClauses.class); + private GitHubSearchClauses() { } @@ -13,6 +19,18 @@ public static String not(String clause) { return "-" + clause; } + public static String range(String field, T min, T max, Function renderer) { + if (min == null && max == null) { + return " "; + } else if (min == null) { + return field + ":<" + renderer.apply(max); + } else if (max == null) { + return field + ":>=" + renderer.apply(min); + } else { + return field + ":" + renderer.apply(min) + ".." + renderer.apply(max); + } + } + public static String repo(GitHubRepositoryRef ref) { return "repo:" + ref.repositoryName(); } @@ -29,8 +47,12 @@ 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 created(Instant min, Instant max) { + return range("created", min, max, GitHubSearchClauses::renderInstant); + } + + public static String updated(Instant min, Instant max) { + return range("updated", min, max, GitHubSearchClauses::renderInstant); } public static String author(String author) { @@ -40,4 +62,12 @@ public static String author(String author) { public static String assignee(String assignee) { return "assignee:" + assignee; } + + public static String commenter(String commenter) { + return "commenter:" + commenter; + } + + private static String renderInstant(Instant instant) { + return instant.atOffset(ZoneOffset.UTC).toLocalDateTime().toString(); + } } diff --git a/src/main/java/io/quarkus/github/lottery/history/LotteryHistory.java b/src/main/java/io/quarkus/github/lottery/history/LotteryHistory.java index cf6c32c..634ab53 100644 --- a/src/main/java/io/quarkus/github/lottery/history/LotteryHistory.java +++ b/src/main/java/io/quarkus/github/lottery/history/LotteryHistory.java @@ -33,6 +33,7 @@ private static Instant min(Instant first, Instant... others) { private final Instant since; private final Bucket triage; + private final Bucket created; private final Bucket feedbackNeeded; private final Bucket feedbackProvided; private final Bucket stale; @@ -48,6 +49,7 @@ public LotteryHistory(Instant now, LotteryConfig.Buckets config) { // a new notification will be sent for the same ticket. // For that mechanism, we need to retrieve all past notifications that did not time out. Instant triageNotificationTimeoutCutoff = now.minus(config.triage().notification().timeout()); + Instant createdNotificationTimeoutCutoff = now.minus(config.maintenance().created().notification().timeout()); Instant feedbackNeededNotificationTimeoutCutoff = now .minus(config.maintenance().feedback().needed().notification().timeout()); Instant feedbackProvidedNotificationTimeoutCutoff = now @@ -59,6 +61,7 @@ public LotteryHistory(Instant now, LotteryConfig.Buckets config) { staleNotificationTimeoutCutoff, stewardshipNotificationTimeoutCutoff); this.triage = new Bucket(triageNotificationTimeoutCutoff); + this.created = new Bucket(createdNotificationTimeoutCutoff); this.feedbackNeeded = new Bucket(feedbackNeededNotificationTimeoutCutoff); this.feedbackProvided = new Bucket(feedbackProvidedNotificationTimeoutCutoff); this.stale = new Bucket(staleNotificationTimeoutCutoff); @@ -90,6 +93,10 @@ public Bucket triage() { return triage; } + public Bucket created() { + return created; + } + public Bucket feedbackNeeded() { return feedbackNeeded; } diff --git a/src/main/resources/templates/MessageFormatter/historyBody.md b/src/main/resources/templates/MessageFormatter/historyBody.md index 232e480..5d073df 100644 --- a/src/main/resources/templates/MessageFormatter/historyBody.md +++ b/src/main/resources/templates/MessageFormatter/historyBody.md @@ -6,6 +6,10 @@ Here are the reports for {drawRef.repositoryName} on {drawRef.instant}. ## Triage {#include MessageFormatter/historyBodyBucketContent bucket=report.triage.get() /} {/if} +{#if report.created.present} +## Created +{#include MessageFormatter/historyBodyBucketContent bucket=report.created.get() /} +{/if} {#if report.feedbackNeeded.present} ## Feedback needed {#include MessageFormatter/historyBodyBucketContent bucket=report.feedbackNeeded.get() /} diff --git a/src/main/resources/templates/MessageFormatter/notificationBody.md b/src/main/resources/templates/MessageFormatter/notificationBody.md index 014ab07..b26fdae 100644 --- a/src/main/resources/templates/MessageFormatter/notificationBody.md +++ b/src/main/resources/templates/MessageFormatter/notificationBody.md @@ -12,6 +12,15 @@ Maintenance areas: {report.config.maintenanceLabels.asMarkdownLabel}. {/include} +{/if} +{#if report.created.present} +# Created +{#include MessageFormatter/notificationBodyBucketContent bucket=report.created.get()} + +Issues or PRs that just got created in your area. Please review, ask for reproducer/information, or plan future work. + +{/include} + {/if} {#if report.feedbackNeeded.present} # Feedback needed diff --git a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java index dae1c80..2bcaa4f 100644 --- a/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/GitHubServiceTest.java @@ -152,6 +152,11 @@ void fetchLotteryConfig() throws IOException { delay: PT0S timeout: P3D maintenance: + created: + delay: PT0S + timeout: P1D + expiry: P14D + ignoreLabels: ["triage/on-ice"] feedback: labels: ["triage/needs-feedback", "triage/needs-reproducer"] needed: @@ -176,6 +181,8 @@ void fetchLotteryConfig() throws IOException { maintenance: labels: ["area/hibernate-orm", "area/hibernate-search"] days: ["MONDAY"] + created: + maxIssues: 5 feedback: needed: maxIssues: 4 @@ -228,6 +235,9 @@ void fetchLotteryConfig() throws IOException { "triage/needs-triage", Duration.ZERO, Duration.ofDays(3)), new LotteryConfig.Buckets.Maintenance( + new LotteryConfig.Buckets.Maintenance.Created( + Duration.ZERO, Duration.ofDays(1), Duration.ofDays(14), + List.of("triage/on-ice")), new LotteryConfig.Buckets.Maintenance.Feedback( List.of("triage/needs-feedback", "triage/needs-reproducer"), new LotteryConfig.Buckets.Maintenance.Feedback.Needed( @@ -248,6 +258,7 @@ void fetchLotteryConfig() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), + Optional.of(new LotteryConfig.Participant.Participation(5)), Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), new LotteryConfig.Participant.Participation(2))), @@ -266,6 +277,7 @@ void fetchLotteryConfig() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/someobscurelibrary"), Set.of(DayOfWeek.MONDAY), + Optional.empty(), Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(1), new LotteryConfig.Participant.Participation(1))), @@ -284,6 +296,7 @@ void fetchLotteryConfig() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/someotherobscurelibrary"), Set.of(DayOfWeek.MONDAY), + Optional.empty(), Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(1), new LotteryConfig.Participant.Participation(1))), @@ -313,6 +326,10 @@ void fetchLotteryConfig_minimal() throws IOException { delay: PT0S timeout: P3D maintenance: + created: + delay: PT0S + timeout: P1D + expiry: P14D feedback: labels: ["triage/needs-feedback"] needed: @@ -344,6 +361,9 @@ void fetchLotteryConfig_minimal() throws IOException { "triage/needs-triage", Duration.ZERO, Duration.ofDays(3)), new LotteryConfig.Buckets.Maintenance( + new LotteryConfig.Buckets.Maintenance.Created( + Duration.ofDays(0), Duration.ofDays(1), Duration.ofDays(14), + List.of()), new LotteryConfig.Buckets.Maintenance.Feedback( List.of("triage/needs-feedback"), new LotteryConfig.Buckets.Maintenance.Feedback.Needed( @@ -866,6 +886,118 @@ void issuesLastActedOnByAndLastUpdatedBefore_outsider() throws IOException { }); } + @Test + void issuesOrPullRequestsNeverActedOnByTeamAndCreatedBetween() throws IOException { + var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus"); + + Instant now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC); + Instant minCutoff = now.minus(14, ChronoUnit.DAYS); + Instant maxCutoff = now.minus(0, ChronoUnit.DAYS); + + var searchIssuesBuilderMock = Mockito.mock(GHIssueSearchBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); + var issue1QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); + var issue2QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); + var issue3QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + withSettings().defaultAnswer(Answers.RETURNS_SELF)); + var issue4QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + 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)); + var issue8QueryCommentsBuilderMock = Mockito.mock(GHIssueCommentQueryBuilder.class, + 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", + 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(clientMock.searchIssues()).thenReturn(searchIssuesBuilderMock); + // Pull requests should always be filtered out + var issue1Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 1, randomReporterUser); + var issue2Mock = mockIssueForLottery(mocks, 2, randomReporterUser); + var issue3Mock = mockIssueForLottery(mocks, 3, randomReporterUser); + var issue4Mock = mockIssueForLottery(mocks, 4); + var issue5Mock = mockIssueForLotteryFilteredOutByRepository(mocks, 5, randomReporterUser); + 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 issue1CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 101, noneUser), + mockIssueComment(mocks, 102, readUser), + mockIssueComment(mocks, 103, adminUser)); + when(issue1Mock.queryComments()).thenReturn(issue1QueryCommentsBuilderMock); + when(issue1QueryCommentsBuilderMock.list()).thenReturn(issue1CommentsMocks); + + var issue2CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 202, readUser)); + when(issue2Mock.queryComments()).thenReturn(issue2QueryCommentsBuilderMock); + when(issue2QueryCommentsBuilderMock.list()).thenReturn(issue2CommentsMocks); + + var issue3CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 302, noneUser)); + when(issue3Mock.queryComments()).thenReturn(issue3QueryCommentsBuilderMock); + when(issue3QueryCommentsBuilderMock.list()).thenReturn(issue3CommentsMocks); + + PagedSearchIterable issue4CommentsMocks = mockPagedIterable(); + when(issue4Mock.queryComments()).thenReturn(issue4QueryCommentsBuilderMock); + when(issue4QueryCommentsBuilderMock.list()).thenReturn(issue4CommentsMocks); + + 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 issue7CommentsMocks = mockPagedIterable(mockIssueComment(mocks, 701, readUser), + mockIssueComment(mocks, 702, 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 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); + + assertThat(repo.issuesOrPullRequestsNeverActedOnByTeamAndCreatedBetween( + "area/hibernate-search", + new LinkedHashSet<>(List.of("triage/needs-feedback", "triage/needs-reproducer", "triage/on-ice")), + Set.of("yrodiere"), + minCutoff, maxCutoff)) + .containsExactlyElementsOf(stubIssueList(2, 3, 4, 7, 8)); + }) + .then().github(mocks -> { + verify(searchIssuesBuilderMock).q("repo:" + repoRef.repositoryName()); + verify(searchIssuesBuilderMock).isOpen(); + verify(searchIssuesBuilderMock).sort(GHIssueSearchBuilder.Sort.CREATED); + verify(searchIssuesBuilderMock).order(GHDirection.ASC); + verify(searchIssuesBuilderMock).q("label:area/hibernate-search"); + verify(searchIssuesBuilderMock).q("created:2017-10-23T06:00..2017-11-06T06:00"); + verify(searchIssuesBuilderMock).q("-label:triage/needs-feedback,triage/needs-reproducer,triage/on-ice"); + verify(searchIssuesBuilderMock).q("-commenter:yrodiere"); + verifyNoMoreInteractions(searchIssuesBuilderMock); + + verifyNoMoreInteractions(mocks.ghObjects()); + }); + } + @Test void topic_extractComments_dedicatedIssueDoesNotExist() throws Exception { var repoRef = new GitHubRepositoryRef(installationRef, "quarkusio/quarkus-lottery-reports"); diff --git a/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java b/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java index 812cfe8..5ea1d8b 100644 --- a/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/HistoryServiceTest.java @@ -50,6 +50,8 @@ private static LotteryConfig defaultConfig() { "triage/needs-triage", Duration.ZERO, Duration.ofDays(3)), new LotteryConfig.Buckets.Maintenance( + new LotteryConfig.Buckets.Maintenance.Created( + Duration.ZERO, Duration.ofDays(1), Duration.ofDays(14), List.of("triage/on-ice")), new LotteryConfig.Buckets.Maintenance.Feedback( List.of("triage/needs-reproducer", "triage/needs-feedback"), new LotteryConfig.Buckets.Maintenance.Feedback.Needed( @@ -140,11 +142,11 @@ void lastNotificationToday_notNotified() throws Exception { .thenReturn(List.of( new LotteryReport.Serialized(now.minus(2, ChronoUnit.DAYS), "jane", Optional.of(new LotteryReport.Bucket.Serialized(List.of(6, 7))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()), new LotteryReport.Serialized(now.minus(1, ChronoUnit.HOURS), "gsmet", Optional.of(new LotteryReport.Bucket.Serialized(List.of(1, 2))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()))); var history = historyService.fetch(drawRef, config); @@ -178,15 +180,15 @@ void lastNotificationToday_notifiedRecently() throws Exception { .thenReturn(List.of( new LotteryReport.Serialized(now.minus(2, ChronoUnit.DAYS), "jane", Optional.of(new LotteryReport.Bucket.Serialized(List.of(6, 7))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()), new LotteryReport.Serialized(now.minus(1, ChronoUnit.HOURS), "gsmet", Optional.of(new LotteryReport.Bucket.Serialized(List.of(1, 2))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()), new LotteryReport.Serialized(now.minus(9, ChronoUnit.HOURS), "yrodiere", Optional.of(new LotteryReport.Bucket.Serialized(List.of(4, 5))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()))); var history = historyService.fetch(drawRef, config); @@ -246,11 +248,11 @@ void lastNotificationTimedOutForIssueNumber() throws Exception { .thenReturn(List.of( new LotteryReport.Serialized(now.minus(1, ChronoUnit.DAYS), "gsmet", Optional.of(new LotteryReport.Bucket.Serialized(List.of(1, 2))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()), new LotteryReport.Serialized(now.minus(7, ChronoUnit.DAYS), "yrodiere", Optional.of(new LotteryReport.Bucket.Serialized(List.of(42))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()))); var history = historyService.fetch(drawRef, config); diff --git a/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java b/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java index 2eb8f4f..8e6faa2 100644 --- a/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java +++ b/src/test/java/io/quarkus/github/lottery/LotterySingleRepositoryTest.java @@ -68,6 +68,8 @@ private static LotteryConfig defaultConfig(List parti "triage/needs-triage", Duration.ZERO, Duration.ofDays(3)), new LotteryConfig.Buckets.Maintenance( + new LotteryConfig.Buckets.Maintenance.Created( + Duration.ZERO, Duration.ofDays(1), Duration.ofDays(14), List.of("triage/on-ice")), new LotteryConfig.Buckets.Maintenance.Feedback( List.of("triage/needs-reproducer", "triage/needs-feedback"), new LotteryConfig.Buckets.Maintenance.Feedback.Needed( @@ -92,6 +94,8 @@ private static LotteryConfig defaultConfig(List parti GitHubInstallationRef installationRef; GitHubRepositoryRef repoRef; Instant now; + Instant createdMinCutoff; + Instant createdMaxCutoff; Instant feedbackNeededCutoff; Instant feedbackProvidedCutoff; Instant staleCutoff; @@ -118,6 +122,8 @@ void setup() throws IOException { // Note tests below assume this is at least 1AM now = LocalDateTime.of(2017, 11, 6, 6, 0).toInstant(ZoneOffset.UTC); + createdMinCutoff = now.minus(14, ChronoUnit.DAYS); + createdMaxCutoff = now.minus(0, ChronoUnit.DAYS); feedbackNeededCutoff = now.minus(21, ChronoUnit.DAYS); feedbackProvidedCutoff = now.minus(7, ChronoUnit.DAYS); staleCutoff = now.minus(60, ChronoUnit.DAYS); @@ -160,6 +166,7 @@ void days_differentDay_defaultTimezone() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.TUESDAY), + Optional.of(new LotteryConfig.Participant.Participation(5)), Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), new LotteryConfig.Participant.Participation(2))), @@ -190,6 +197,7 @@ void days_differentDay_explicitTimezone() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), + Optional.of(new LotteryConfig.Participant.Participation(5)), Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), new LotteryConfig.Participant.Participation(2))), @@ -222,6 +230,7 @@ void alreadyNotifiedToday() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), + Optional.of(new LotteryConfig.Participant.Participation(5)), Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), new LotteryConfig.Participant.Participation(2))), @@ -255,6 +264,7 @@ void ignoring() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), + Optional.of(new LotteryConfig.Participant.Participation(5)), Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), new LotteryConfig.Participant.Participation(2))), @@ -306,7 +316,7 @@ void triage() throws IOException { verify(historyServiceMock).append(drawRef, config, List.of( new LotteryReport.Serialized(drawRef.instant(), "yrodiere", Optional.of(new LotteryReport.Bucket.Serialized(List.of(1, 3, 2))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()))); verify(notifierMock).close(); @@ -348,7 +358,7 @@ void triage_issueAlreadyHasNonTimedOutNotification() throws IOException { verify(historyServiceMock).append(drawRef, config, List.of( new LotteryReport.Serialized(drawRef.instant(), "yrodiere", Optional.of(new LotteryReport.Bucket.Serialized(List.of(1, 2, 4))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()))); verify(notifierMock).close(); @@ -367,6 +377,7 @@ void maintenance() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), + Optional.of(new LotteryConfig.Participant.Participation(5)), Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), new LotteryConfig.Participant.Participation(2))), @@ -374,6 +385,11 @@ void maintenance() throws IOException { Optional.empty()))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); + when(repoMock.issuesOrPullRequestsNeverActedOnByTeamAndCreatedBetween("area/hibernate-orm", + Set.of("triage/needs-reproducer", "triage/needs-feedback", "triage/on-ice"), + Set.of("yrodiere"), + createdMinCutoff, createdMaxCutoff)) + .thenAnswer(ignored -> stubIssueList(701, 702, 703, 704, 705, 706).stream()); when(repoMock.issuesLastActedOnByAndLastUpdatedBefore( Set.of("triage/needs-reproducer", "triage/needs-feedback"), "area/hibernate-orm", IssueActionSide.TEAM, feedbackNeededCutoff)) @@ -386,6 +402,11 @@ void maintenance() throws IOException { staleCutoff)) .thenAnswer(ignored -> stubIssueList(301, 302, 303, 304, 305, 306).stream()); + when(repoMock.issuesOrPullRequestsNeverActedOnByTeamAndCreatedBetween("area/hibernate-search", + Set.of("triage/needs-reproducer", "triage/needs-feedback", "triage/on-ice"), + Set.of("yrodiere"), + createdMinCutoff, createdMaxCutoff)) + .thenAnswer(ignored -> stubIssueList(801, 802, 803, 804, 805, 806).stream()); when(repoMock.issuesLastActedOnByAndLastUpdatedBefore( Set.of("triage/needs-reproducer", "triage/needs-feedback"), "area/hibernate-search", IssueActionSide.TEAM, feedbackNeededCutoff)) @@ -400,6 +421,9 @@ void maintenance() throws IOException { mockNotifiable("yrodiere", ZoneOffset.UTC); + var historyCreatedMock = mock(LotteryHistory.Bucket.class); + when(historyMock.created()).thenReturn(historyCreatedMock); + when(historyCreatedMock.lastNotificationTimedOutForIssueNumber(anyInt())).thenReturn(true); var historyFeedbackNeededMock = mock(LotteryHistory.Bucket.class); when(historyMock.feedbackNeeded()).thenReturn(historyFeedbackNeededMock); when(historyFeedbackNeededMock.lastNotificationTimedOutForIssueNumber(anyInt())).thenReturn(true); @@ -414,6 +438,7 @@ void maintenance() throws IOException { verify(notifierMock).send(stubReportMaintenance(drawRef, "yrodiere", Optional.empty(), List.of("area/hibernate-orm", "area/hibernate-search"), + stubIssueList(701, 801, 702, 802, 703), stubIssueList(101, 401, 102, 402), stubIssueList(201, 501), stubIssueList(301, 601, 302, 602, 303))); @@ -421,6 +446,7 @@ void maintenance() throws IOException { verify(historyServiceMock).append(drawRef, config, List.of( new LotteryReport.Serialized(drawRef.instant(), "yrodiere", Optional.empty(), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(701, 801, 702, 802, 703))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(101, 401, 102, 402))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(201, 501))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(301, 601, 302, 602, 303))), @@ -442,6 +468,7 @@ void maintenance_issueAlreadyHasTimedOutNotification() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-orm", "area/hibernate-search"), Set.of(DayOfWeek.MONDAY), + Optional.of(new LotteryConfig.Participant.Participation(5)), Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), new LotteryConfig.Participant.Participation(2))), @@ -449,6 +476,11 @@ void maintenance_issueAlreadyHasTimedOutNotification() throws IOException { Optional.empty()))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); + when(repoMock.issuesOrPullRequestsNeverActedOnByTeamAndCreatedBetween("area/hibernate-orm", + Set.of("triage/needs-reproducer", "triage/needs-feedback", "triage/on-ice"), + Set.of("yrodiere"), + createdMinCutoff, createdMaxCutoff)) + .thenAnswer(ignored -> stubIssueList(701, 702, 703, 704, 705, 706).stream()); when(repoMock.issuesLastActedOnByAndLastUpdatedBefore( Set.of("triage/needs-reproducer", "triage/needs-feedback"), "area/hibernate-orm", IssueActionSide.TEAM, feedbackNeededCutoff)) @@ -461,6 +493,11 @@ void maintenance_issueAlreadyHasTimedOutNotification() throws IOException { staleCutoff)) .thenAnswer(ignored -> stubIssueList(301, 302, 303, 304, 305, 306).stream()); + when(repoMock.issuesOrPullRequestsNeverActedOnByTeamAndCreatedBetween("area/hibernate-search", + Set.of("triage/needs-reproducer", "triage/needs-feedback", "triage/on-ice"), + Set.of("yrodiere"), + createdMinCutoff, createdMaxCutoff)) + .thenAnswer(ignored -> stubIssueList(801, 802, 803, 804, 805, 806).stream()); when(repoMock.issuesLastActedOnByAndLastUpdatedBefore( Set.of("triage/needs-reproducer", "triage/needs-feedback"), "area/hibernate-search", IssueActionSide.TEAM, feedbackNeededCutoff)) @@ -475,6 +512,10 @@ void maintenance_issueAlreadyHasTimedOutNotification() throws IOException { mockNotifiable("yrodiere", ZoneOffset.UTC); + var historyCreatedMock = mock(LotteryHistory.Bucket.class); + when(historyMock.created()).thenReturn(historyCreatedMock); + when(historyCreatedMock.lastNotificationTimedOutForIssueNumber(anyInt())).thenReturn(true); + when(historyCreatedMock.lastNotificationTimedOutForIssueNumber(702)).thenReturn(false); var historyFeedbackNeededMock = mock(LotteryHistory.Bucket.class); when(historyMock.feedbackNeeded()).thenReturn(historyFeedbackNeededMock); when(historyFeedbackNeededMock.lastNotificationTimedOutForIssueNumber(anyInt())).thenReturn(true); @@ -490,10 +531,11 @@ void maintenance_issueAlreadyHasTimedOutNotification() throws IOException { lotteryService.draw(); - // Since the last notification for issues with number 401, 201, 302 didn't time out yet, + // Since the last notification for issues with number 401, 201, 302, 702 didn't time out yet, // they will be skipped and we'll notify about the next issues instead. verify(notifierMock).send(stubReportMaintenance(drawRef, "yrodiere", Optional.empty(), List.of("area/hibernate-orm", "area/hibernate-search"), + stubIssueList(701, 801, 703, 802, 704), stubIssueList(101, 402, 102, 403), stubIssueList(202, 501), stubIssueList(301, 601, 303, 602, 304))); @@ -501,6 +543,7 @@ void maintenance_issueAlreadyHasTimedOutNotification() throws IOException { verify(historyServiceMock).append(drawRef, config, List.of( new LotteryReport.Serialized(drawRef.instant(), "yrodiere", Optional.empty(), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(701, 801, 703, 802, 704))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(101, 402, 102, 403))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(202, 501))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(301, 601, 303, 602, 304))), @@ -542,7 +585,7 @@ void stewardship() throws IOException { verify(historyServiceMock).append(drawRef, config, List.of( new LotteryReport.Serialized(drawRef.instant(), "geoand", Optional.empty(), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket.Serialized(List.of(1, 3, 2)))))); verify(notifierMock).close(); @@ -584,7 +627,7 @@ void stewardship_issueAlreadyHasNonTimedOutNotification() throws IOException { verify(historyServiceMock).append(drawRef, config, List.of( new LotteryReport.Serialized(drawRef.instant(), "geoand", Optional.empty(), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket.Serialized(List.of(1, 2, 4)))))); verify(notifierMock).close(); @@ -603,6 +646,7 @@ void stewardship_doesNotAffectMaintenance() throws IOException { Optional.of(new LotteryConfig.Participant.Maintenance( List.of("area/hibernate-search"), Set.of(DayOfWeek.MONDAY), + Optional.of(new LotteryConfig.Participant.Participation(5)), Optional.of(new LotteryConfig.Participant.Maintenance.Feedback( new LotteryConfig.Participant.Participation(4), new LotteryConfig.Participant.Participation(2))), @@ -617,6 +661,11 @@ void stewardship_doesNotAffectMaintenance() throws IOException { new LotteryConfig.Participant.Participation(10)))))); when(repoMock.fetchLotteryConfig()).thenReturn(Optional.of(config)); + when(repoMock.issuesOrPullRequestsNeverActedOnByTeamAndCreatedBetween("area/hibernate-search", + Set.of("triage/needs-reproducer", "triage/needs-feedback", "triage/on-ice"), + Set.of("yrodiere"), + createdMinCutoff, createdMaxCutoff)) + .thenAnswer(ignored -> stubIssueList(801, 802, 803, 804, 805, 806).stream()); when(repoMock.issuesLastActedOnByAndLastUpdatedBefore( Set.of("triage/needs-reproducer", "triage/needs-feedback"), "area/hibernate-search", IssueActionSide.TEAM, feedbackNeededCutoff)) @@ -635,6 +684,9 @@ void stewardship_doesNotAffectMaintenance() throws IOException { mockNotifiable("yrodiere", ZoneOffset.UTC); mockNotifiable("gsmet", ZoneOffset.UTC); + var historyCreatedMock = mock(LotteryHistory.Bucket.class); + when(historyMock.created()).thenReturn(historyCreatedMock); + when(historyCreatedMock.lastNotificationTimedOutForIssueNumber(anyInt())).thenReturn(true); var historyFeedbackNeededMock = mock(LotteryHistory.Bucket.class); when(historyMock.feedbackNeeded()).thenReturn(historyFeedbackNeededMock); when(historyFeedbackNeededMock.lastNotificationTimedOutForIssueNumber(anyInt())).thenReturn(true); @@ -653,6 +705,7 @@ void stewardship_doesNotAffectMaintenance() throws IOException { verify(notifierMock).send(stubReport(drawRef, "yrodiere", Optional.empty(), stubReportConfig("area/hibernate-search"), Optional.empty(), + Optional.of(stubIssueList(801, 802, 803, 804, 805)), Optional.of(stubIssueList(401, 402, 403, 404)), Optional.of(stubIssueList(501, 502)), // Notifications to stewards don't prevent notifications to maintainers @@ -662,24 +715,21 @@ void stewardship_doesNotAffectMaintenance() throws IOException { verify(notifierMock).send(stubReport(drawRef, "gsmet", Optional.empty(), stubReportConfig(), Optional.empty(), - Optional.empty(), - Optional.empty(), - Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), // Notifications to maintainers don't prevent notifications to stewards Optional.of(stubIssueList(401, 501, 601, 701)))); verify(historyServiceMock).append(drawRef, config, List.of( new LotteryReport.Serialized(drawRef.instant(), "yrodiere", Optional.empty(), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(801, 802, 803, 804, 805))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(401, 402, 403, 404))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(501, 502))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(601, 602, 603, 604, 605))), Optional.empty()), new LotteryReport.Serialized(drawRef.instant(), "gsmet", Optional.empty(), - Optional.empty(), - Optional.empty(), - Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket.Serialized(List.of(401, 501, 601, 701)))))); verify(notifierMock).close(); diff --git a/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java b/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java index 74f2ebe..22d1c14 100644 --- a/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java +++ b/src/test/java/io/quarkus/github/lottery/MessageFormatterTest.java @@ -77,6 +77,7 @@ void formatNotificationTopicText() { void formatNotificationTopicSuffixText() { var lotteryReport = stubReport(drawRef, "yrodiere", Optional.empty(), stubReportConfig(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); assertThat(messageFormatter.formatNotificationTopicSuffixText(lotteryReport)) @@ -87,6 +88,7 @@ void formatNotificationTopicSuffixText() { void formatNotificationTopicSuffixText_exoticTimezone() { var lotteryReport = stubReport(drawRef, "yrodiere", Optional.of(ZoneId.of("America/Los_Angeles")), stubReportConfig(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); assertThat(messageFormatter.formatNotificationTopicSuffixText(lotteryReport)) @@ -142,6 +144,7 @@ void formatNotificationBodyMarkdown_maintenance_empty() { List.of("area/hibernate-orm", "area/hibernate-search"), List.of(), List.of(), + List.of(), List.of()); assertThat(messageFormatter.formatNotificationBodyMarkdown(lotteryReport, notificationRepoRef)) .isEqualTo( @@ -150,6 +153,10 @@ void formatNotificationBodyMarkdown_maintenance_empty() { Maintenance areas: `area/hibernate-orm`/`area/hibernate-search`. + # Created + + No issues in this category this time. + # Feedback needed No issues in this category this time. @@ -173,6 +180,7 @@ void formatNotificationBodyMarkdown_maintenance_empty() { void formatNotificationBodyMarkdown_maintenance_someEmpty() { var lotteryReport = stubReportMaintenance(drawRef, "yrodiere", Optional.empty(), List.of("area/hibernate-orm", "area/hibernate-search"), + List.of(), stubIssueList(1, 3), List.of(), List.of()); @@ -183,6 +191,10 @@ void formatNotificationBodyMarkdown_maintenance_someEmpty() { Maintenance areas: `area/hibernate-orm`/`area/hibernate-search`. + # Created + + No issues in this category this time. + # Feedback needed Issues with missing reproducer/information. Please ping the reporter, or close the issue if it's taking too long. @@ -209,6 +221,7 @@ void formatNotificationBodyMarkdown_maintenance_someEmpty() { void formatNotificationBodyMarkdown_maintenance_simple() { var lotteryReport = stubReportMaintenance(drawRef, "yrodiere", Optional.empty(), List.of("area/hibernate-orm", "area/hibernate-search"), + stubIssueList(8, 9), stubIssueList(1, 3), stubIssueList(4, 5), stubIssueList(2, 7)); @@ -219,6 +232,13 @@ void formatNotificationBodyMarkdown_maintenance_simple() { Maintenance areas: `area/hibernate-orm`/`area/hibernate-search`. + # Created + + Issues or PRs that just got created in your area. Please review, ask for reproducer/information, or plan future work. + + - [#8](http://github.com/quarkusio/quarkus/issues/8) Title for issue 8 + - [#9](http://github.com/quarkusio/quarkus/issues/9) Title for issue 9 + # Feedback needed Issues with missing reproducer/information. Please ping the reporter, or close the issue if it's taking too long. @@ -295,6 +315,7 @@ void formatNotificationBodyMarkdown_all() { var lotteryReport = stubReport(drawRef, "yrodiere", Optional.empty(), stubReportConfig(), Optional.of(stubIssueList(1, 3)), + Optional.of(stubIssueList(12, 13)), Optional.of(stubIssueList(4, 5)), Optional.of(stubIssueList(2, 7)), Optional.of(stubIssueList(8, 9)), @@ -311,6 +332,13 @@ void formatNotificationBodyMarkdown_all() { - [#1](http://github.com/quarkusio/quarkus/issues/1) Title for issue 1 - [#3](http://github.com/quarkusio/quarkus/issues/3) Title for issue 3 + # Created + + Issues or PRs that just got created in your area. Please review, ask for reproducer/information, or plan future work. + + - [#12](http://github.com/quarkusio/quarkus/issues/12) Title for issue 12 + - [#13](http://github.com/quarkusio/quarkus/issues/13) Title for issue 13 + # Feedback needed Issues with missing reproducer/information. Please ping the reporter, or close the issue if it's taking too long. @@ -351,7 +379,7 @@ void formatNotificationBodyMarkdown_exoticTimezone() { var lotteryReport = stubReport(drawRef, "yrodiere", Optional.of(ZoneId.of("America/Los_Angeles")), stubReportConfig(), Optional.empty(), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); assertThat(messageFormatter.formatNotificationBodyMarkdown(lotteryReport, notificationRepoRef)) .startsWith(""" @@ -370,31 +398,33 @@ void formatHistoryBodyMarkdown() throws IOException { var lotteryReports = List.of( new LotteryReport.Serialized(drawRef.instant(), "yrodiere", Optional.of(new LotteryReport.Bucket.Serialized(List.of(1, 3))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()), new LotteryReport.Serialized(drawRef.instant(), "gsmet", Optional.of(new LotteryReport.Bucket.Serialized(List.of(2, 4))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()), new LotteryReport.Serialized(drawRef.instant(), "rick", Optional.of(new LotteryReport.Bucket.Serialized(List.of())), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()), new LotteryReport.Serialized(drawRef.instant(), "jane", Optional.empty(), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(13, 14))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(7, 8))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(9))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(10, 11, 12))), Optional.empty()), new LotteryReport.Serialized(drawRef.instant(), "jsmith", Optional.of(new LotteryReport.Bucket.Serialized(List.of(25, 26))), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(45, 46))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(27, 28))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(29))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(30, 31, 32))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(41, 42)))), new LotteryReport.Serialized(drawRef.instant(), "geoand", Optional.empty(), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket.Serialized(List.of(51, 52))))); String formatted = messageFormatter.formatHistoryBodyMarkdown(drawRef, lotteryReports); assertThat(formatted) @@ -416,6 +446,9 @@ void formatHistoryBodyMarkdown() throws IOException { No issues in this category this time. # jane + ## Created + - quarkusio/quarkus#13 + - quarkusio/quarkus#14 ## Feedback needed - quarkusio/quarkus#7 - quarkusio/quarkus#8 @@ -430,6 +463,9 @@ void formatHistoryBodyMarkdown() throws IOException { ## Triage - quarkusio/quarkus#25 - quarkusio/quarkus#26 + ## Created + - quarkusio/quarkus#45 + - quarkusio/quarkus#46 ## Feedback needed - quarkusio/quarkus#27 - quarkusio/quarkus#28 @@ -461,17 +497,18 @@ void extractPayloadFromHistoryBodyMarkdown_oldFormatWithReproducer() throws IOEx var lotteryReports = List.of( new LotteryReport.Serialized(drawRef.instant(), "yrodiere", Optional.of(new LotteryReport.Bucket.Serialized(List.of(1, 3))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()), new LotteryReport.Serialized(drawRef.instant(), "gsmet", Optional.of(new LotteryReport.Bucket.Serialized(List.of(2, 4))), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()), new LotteryReport.Serialized(drawRef.instant(), "rick", Optional.of(new LotteryReport.Bucket.Serialized(List.of())), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()), new LotteryReport.Serialized(drawRef.instant(), "jane", + Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket.Serialized(List.of(7, 8))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(9))), @@ -479,13 +516,14 @@ void extractPayloadFromHistoryBodyMarkdown_oldFormatWithReproducer() throws IOEx Optional.empty()), new LotteryReport.Serialized(drawRef.instant(), "jsmith", Optional.of(new LotteryReport.Bucket.Serialized(List.of(25, 26))), + Optional.empty(), Optional.of(new LotteryReport.Bucket.Serialized(List.of(27, 28))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(29))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(30, 31, 32))), Optional.of(new LotteryReport.Bucket.Serialized(List.of(41, 42)))), new LotteryReport.Serialized(drawRef.instant(), "geoand", Optional.empty(), - Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.of(new LotteryReport.Bucket.Serialized(List.of(51, 52))))); String formatted = """ Foo @@ -502,4 +540,53 @@ void extractPayloadFromHistoryBodyMarkdown_oldFormatWithReproducer() throws IOEx .usingRecursiveFieldByFieldElementComparator() .isEqualTo(lotteryReports); } + + @Test + void extractPayloadFromHistoryBodyMarkdown_oldFormatWithoutCreated() throws IOException { + var lotteryReports = List.of( + new LotteryReport.Serialized(drawRef.instant(), "yrodiere", + Optional.of(new LotteryReport.Bucket.Serialized(List.of(1, 3))), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty()), + new LotteryReport.Serialized(drawRef.instant(), "gsmet", + Optional.of(new LotteryReport.Bucket.Serialized(List.of(2, 4))), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty()), + new LotteryReport.Serialized(drawRef.instant(), "rick", + Optional.of(new LotteryReport.Bucket.Serialized(List.of())), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty()), + new LotteryReport.Serialized(drawRef.instant(), "jane", + Optional.empty(), + Optional.empty(), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(7, 8))), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(9))), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(10, 11, 12))), + Optional.empty()), + new LotteryReport.Serialized(drawRef.instant(), "jsmith", + Optional.of(new LotteryReport.Bucket.Serialized(List.of(25, 26))), + Optional.empty(), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(27, 28))), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(29))), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(30, 31, 32))), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(41, 42)))), + new LotteryReport.Serialized(drawRef.instant(), "geoand", + Optional.empty(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), + Optional.of(new LotteryReport.Bucket.Serialized(List.of(51, 52))))); + String formatted = """ + Foo + + bar + foobar + + + """; + + assertThat(messageFormatter.extractPayloadFromHistoryBodyMarkdown(formatted)) + .usingRecursiveFieldByFieldElementComparator() + .isEqualTo(lotteryReports); + } } \ No newline at end of file diff --git a/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java b/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java index 69f1bd8..1b61dc8 100644 --- a/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java +++ b/src/test/java/io/quarkus/github/lottery/NotificationServiceTest.java @@ -136,6 +136,7 @@ void send() throws IOException { var lotteryReport2 = stubReportMaintenance(drawRef, "gsmet", Optional.empty(), List.of("area/hibernate-validator"), + stubIssueList(11, 12), stubIssueList(4, 5), stubIssueList(7, 8), stubIssueList(9, 10)); @@ -157,6 +158,7 @@ void send() throws IOException { Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.of(stubIssueList(13, 14))); var markdownNotification3 = "Notif 3"; when(messageFormatterMock.formatNotificationTopicText(drawRef, "geoand")) @@ -176,6 +178,7 @@ void send() throws IOException { Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.of(stubIssueList())); var markdownNotification4 = "Notif 4"; when(messageFormatterMock.formatNotificationTopicText(drawRef, "jsmith")) diff --git a/src/test/java/io/quarkus/github/lottery/PullRequestConfigCheckTest.java b/src/test/java/io/quarkus/github/lottery/PullRequestConfigCheckTest.java index 3101407..5aff08f 100644 --- a/src/test/java/io/quarkus/github/lottery/PullRequestConfigCheckTest.java +++ b/src/test/java/io/quarkus/github/lottery/PullRequestConfigCheckTest.java @@ -84,6 +84,10 @@ void pullRequestContainsFile_syntaxOk() throws IOException { delay: PT0S timeout: P3D maintenance: + created: + delay: PT0S + timeout: P1D + expiry: P14D feedback: labels: ["triage/needs-reproducer", "triage/needs-feedback"] needed: @@ -157,6 +161,10 @@ void pullRequestContainsFile_syntaxError() throws IOException { delay: PT0S timeout: P3D maintenance: + created: + delay: PT0S + timeout: P1D + expiry: P14D feedback: labels: ["triage/needs-reproducer", "triage/needs-feedback"] needed: 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 3371e33..0abb2a4 100644 --- a/src/test/java/io/quarkus/github/lottery/draw/LotteryReportTest.java +++ b/src/test/java/io/quarkus/github/lottery/draw/LotteryReportTest.java @@ -42,6 +42,7 @@ void buckets() { newBucket(buckets), newBucket(buckets), newBucket(buckets), + newBucket(buckets), newBucket(buckets)); assertThat(report.buckets()).containsExactlyInAnyOrderElementsOf(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 d2568d5..8fe91a4 100644 --- a/src/test/java/io/quarkus/github/lottery/util/MockHelper.java +++ b/src/test/java/io/quarkus/github/lottery/util/MockHelper.java @@ -83,6 +83,21 @@ public static LotteryReport stubReportTriage(DrawRef drawRef, Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), + Optional.empty()); + } + + public static LotteryReport stubReportCreated(DrawRef drawRef, + String username, + Optional timezone, + List maintenanceLabels, + List created) { + return stubReport(drawRef, username, timezone, stubReportConfig(maintenanceLabels.toArray(String[]::new)), + Optional.empty(), + Optional.of(created), + Optional.empty(), + Optional.empty(), + Optional.empty(), Optional.empty()); } @@ -93,6 +108,7 @@ public static LotteryReport stubReportFeedback(DrawRef drawRef, List feedbackNeeded, List feedbackProvided) { return stubReport(drawRef, username, timezone, stubReportConfig(maintenanceLabels.toArray(String[]::new)), + Optional.empty(), Optional.empty(), Optional.of(feedbackNeeded), Optional.of(feedbackProvided), @@ -108,6 +124,7 @@ public static LotteryReport stubReportStale(DrawRef drawRef, Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.of(stale), Optional.empty()); } @@ -116,11 +133,13 @@ public static LotteryReport stubReportMaintenance(DrawRef drawRef, String username, Optional timezone, List maintenanceLabels, + List created, List feedbackNeeded, List feedbackProvided, List stale) { return stubReport(drawRef, username, timezone, stubReportConfig(maintenanceLabels.toArray(String[]::new)), Optional.empty(), + Optional.of(created), Optional.of(feedbackNeeded), Optional.of(feedbackProvided), Optional.of(stale), @@ -136,6 +155,7 @@ public static LotteryReport stubReportStewardship(DrawRef drawRef, Optional.empty(), Optional.empty(), Optional.empty(), + Optional.empty(), Optional.of(stewardship)); } @@ -144,12 +164,14 @@ public static LotteryReport stubReport(DrawRef drawRef, Optional timezone, LotteryReport.Config config, Optional> triage, + Optional> created, Optional> feedbackNeeded, Optional> feedbackProvided, Optional> stale, Optional> stewardship) { return new LotteryReport(drawRef, username, timezone, config, triage.map(LotteryReport.Bucket::new), + created.map(LotteryReport.Bucket::new), feedbackNeeded.map(LotteryReport.Bucket::new), feedbackProvided.map(LotteryReport.Bucket::new), stale.map(LotteryReport.Bucket::new),