This is an automated email from the ASF dual-hosted git repository.
damccorm pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git
The following commit(s) were added to refs/heads/master by this push:
new 72d387f3c7e [PR Bot] Select single main and backup reviewer using
label priority (#40315)
72d387f3c7e is described below
commit 72d387f3c7ed78e18c0e0b8487eaa7e91eaccce8
Author: Danny McCormick <[email protected]>
AuthorDate: Tue Oct 13 14:31:56 2026 +0000
[PR Bot] Select single main and backup reviewer using label priority
(#40315)
* [PR Bot] Select single main and backup reviewer using label priority
* Choose randomly among tied highest-priority labels
---
.github/REVIEWERS.yml | 12 ++++
scripts/ci/pr-bot/findPrsNeedingAttention.ts | 17 +++++-
scripts/ci/pr-bot/processNewPrs.ts | 14 ++++-
scripts/ci/pr-bot/shared/commentStrings.ts | 9 +++
scripts/ci/pr-bot/shared/reviewerConfig.ts | 44 ++++++++++----
scripts/ci/pr-bot/shared/reviewersForLabel.ts | 49 ++++++++++++---
scripts/ci/pr-bot/shared/userCommand.ts | 28 ++++++++-
scripts/ci/pr-bot/test/commentStringsTest.ts | 13 ++++
scripts/ci/pr-bot/test/reviewerConfigTest.ts | 79 +++++++++++++++----------
scripts/ci/pr-bot/test/reviewersForLabelTest.ts | 46 ++++++++++++++
10 files changed, 254 insertions(+), 57 deletions(-)
diff --git a/.github/REVIEWERS.yml b/.github/REVIEWERS.yml
index f3c27bec3cf..90034a624f1 100644
--- a/.github/REVIEWERS.yml
+++ b/.github/REVIEWERS.yml
@@ -17,14 +17,18 @@
# add yourself to that label's exclusionList
# FallbackReviewers is for reviewers who can review any area of the code base
that might
# not receive a label. These should generally be more experienced committers.
+# Priority determines which label is used to select a reviewer when a PR has
multiple labels
+# (lower number = higher priority).
labels:
- name: Go
+ priority: 2
reviewers:
- jrmccluskey
- lostluck
- shunping
exclusionList: []
- name: Python
+ priority: 2
reviewers:
- claudevdm
- damccorm
@@ -33,6 +37,7 @@ labels:
- shunping
exclusionList: []
- name: Java
+ priority: 2
reviewers:
- Abacn
- ahmedabu98
@@ -41,28 +46,34 @@ labels:
- kennknowles
exclusionList: []
- name: spanner
+ priority: 1
reviewers:
- nielm
- name: bigtable
+ priority: 1
reviewers:
- igorbernstein2
- mutianf
exclusionList: []
- name: delta
+ priority: 1
reviewers:
- ahmedabu98
- chamikaramj
exclusionList: []
- name: iceberg
+ priority: 1
reviewers:
- ahmedabu98
- chamikaramj
exclusionList: []
- name: yaml
+ priority: 1
reviewers:
- chamikaramj
- derrickaw
- name: Build
+ priority: 3
reviewers:
- Abacn
- Amar3tto
@@ -70,6 +81,7 @@ labels:
- derrickaw
exclusionList: []
- name: website
+ priority: 3
reviewers:
- Abacn
- damccorm
diff --git a/scripts/ci/pr-bot/findPrsNeedingAttention.ts
b/scripts/ci/pr-bot/findPrsNeedingAttention.ts
index 4b1d97201f9..829a69d6d59 100644
--- a/scripts/ci/pr-bot/findPrsNeedingAttention.ts
+++ b/scripts/ci/pr-bot/findPrsNeedingAttention.ts
@@ -112,9 +112,11 @@ async function assignToNewReviewers(
reviewersToExclude.push(pull.user.login);
const reviewersForLabels: { [key: string]: string[] } =
reviewerConfig.getReviewersForLabels(labelObjects, reviewersToExclude);
- const fallbackReviewers = reviewerConfig.getFallbackReviewers();
- for (const labelObject of labelObjects) {
- const label = labelObject.name;
+ const fallbackReviewers =
+ reviewerConfig.getFallbackReviewers(reviewersToExclude);
+ let backupReviewersForLabels: { [key: string]: string } = {};
+ prState.reviewersAssignedForLabels = {};
+ for (const label of Object.keys(reviewersForLabels)) {
let availableReviewers = reviewersForLabels[label];
if (availableReviewers && availableReviewers.length > 0) {
let reviewersState = await stateClient.getReviewersForLabelState(label);
@@ -122,8 +124,16 @@ async function assignToNewReviewers(
availableReviewers,
fallbackReviewers
);
+ let backupReviewer = reviewersState.getBackupReviewer(
+ availableReviewers,
+ chosenReviewer,
+ fallbackReviewers
+ );
reviewerStateToUpdate[label] = reviewersState;
prState.reviewersAssignedForLabels[label] = chosenReviewer;
+ if (backupReviewer) {
+ backupReviewersForLabels[label] = backupReviewer;
+ }
}
}
@@ -132,6 +142,7 @@ async function assignToNewReviewers(
pull.number,
commentStrings.assignNewReviewer(prState.reviewersAssignedForLabels, {
labels: pull.labels,
+ backupReviewers: backupReviewersForLabels,
})
);
await github.requestPrReviewers(
diff --git a/scripts/ci/pr-bot/processNewPrs.ts
b/scripts/ci/pr-bot/processNewPrs.ts
index 2f24f8e08f4..f027b75f4fa 100644
--- a/scripts/ci/pr-bot/processNewPrs.ts
+++ b/scripts/ci/pr-bot/processNewPrs.ts
@@ -344,8 +344,11 @@ export async function processPull(
// Pick reviewers to assign. Store them in reviewerStateToUpdate and update
the prState object with those reviewers (and their associated labels)
let reviewerStateToUpdate: { [key: string]: typeof ReviewersForLabel } = {};
+ let backupReviewersForLabels: { [key: string]: string } = {};
+ const exclusionList = [pull.user.login];
const reviewersForLabels: { [key: string]: string[] } =
- reviewerConfig.getReviewersForLabels(pull.labels, [pull.user.login]);
+ reviewerConfig.getReviewersForLabels(pull.labels, exclusionList);
+ const fallbackReviewers = reviewerConfig.getFallbackReviewers(exclusionList);
var labels = Object.keys(reviewersForLabels);
if (!labels || labels.length === 0) {
return;
@@ -354,8 +357,16 @@ export async function processPull(
let availableReviewers = reviewersForLabels[label];
let reviewersState = await stateClient.getReviewersForLabelState(label);
let chosenReviewer = reviewersState.assignNextReviewer(availableReviewers);
+ let backupReviewer = reviewersState.getBackupReviewer(
+ availableReviewers,
+ chosenReviewer,
+ fallbackReviewers
+ );
reviewerStateToUpdate[label] = reviewersState;
prState.reviewersAssignedForLabels[label] = chosenReviewer;
+ if (backupReviewer) {
+ backupReviewersForLabels[label] = backupReviewer;
+ }
}
console.log(`Assigning reviewers for PR ${pull.number}`);
@@ -363,6 +374,7 @@ export async function processPull(
pull.number,
commentStrings.assignReviewer(prState.reviewersAssignedForLabels, {
labels: pull.labels,
+ backupReviewers: backupReviewersForLabels,
})
);
await github.requestPrReviewers(
diff --git a/scripts/ci/pr-bot/shared/commentStrings.ts
b/scripts/ci/pr-bot/shared/commentStrings.ts
index f1a39be4c55..edf16d27b83 100644
--- a/scripts/ci/pr-bot/shared/commentStrings.ts
+++ b/scripts/ci/pr-bot/shared/commentStrings.ts
@@ -22,6 +22,7 @@ const { NO_MATCHING_LABEL } = require("./constants");
export interface AssignReviewerOptions {
labels?: (string | Label)[];
notices?: string[];
+ backupReviewers?: { [label: string]: string };
}
// Custom notices for specific labels
@@ -63,11 +64,15 @@ export function assignReviewer(
for (let label in labelToReviewerMapping) {
let reviewer = labelToReviewerMapping[label];
+ let backupReviewer = options?.backupReviewers?.[label];
if (label === NO_MATCHING_LABEL) {
commentString += `R: @${reviewer} added as fallback since no labels
match configuration\n`;
} else {
commentString += `R: @${reviewer} for label ${label}.\n`;
}
+ if (backupReviewer) {
+ commentString += `Backup reviewer: ${backupReviewer} (if the main
reviewer is unresponsive, you can escalate to them by tagging their
username)\n`;
+ }
}
commentString += formatNotices(labelToReviewerMapping, options);
@@ -158,11 +163,15 @@ export function assignNewReviewer(
for (const label in labelToReviewerMapping) {
const reviewer = labelToReviewerMapping[label];
+ const backupReviewer = options?.backupReviewers?.[label];
if (label === NO_MATCHING_LABEL) {
commentString += `R: @${reviewer} added as fallback since no labels
match configuration\n`;
} else {
commentString += `R: @${reviewer} for label ${label}.\n`;
}
+ if (backupReviewer) {
+ commentString += `Backup reviewer: ${backupReviewer} (if the main
reviewer is unresponsive, you can escalate to them by tagging their
username)\n`;
+ }
}
commentString += formatNotices(labelToReviewerMapping, options);
diff --git a/scripts/ci/pr-bot/shared/reviewerConfig.ts
b/scripts/ci/pr-bot/shared/reviewerConfig.ts
index 8fc0d32b014..8feb7c2df6e 100644
--- a/scripts/ci/pr-bot/shared/reviewerConfig.ts
+++ b/scripts/ci/pr-bot/shared/reviewerConfig.ts
@@ -44,30 +44,52 @@ export class ReviewerConfig {
}
// Given a list of labels and an exclusion list of reviewers not to include
(e.g. the author)
- // returns all possible reviewers for each label
+ // returns all possible reviewers for the single highest-priority matching
label
+ // (lower priority number = higher priority; ties broken randomly).
getReviewersForLabels(
labels: Label[],
exclusionList: string[]
): { [key: string]: string[] } {
- let reviewersFound = false;
- let labelToReviewerMapping = {};
- labels.forEach((label) => {
+ let labelToReviewerMapping: { [key: string]: string[] } = {};
+ let bestPriority: number = Number.MAX_SAFE_INTEGER;
+ let bestMatches: { label: string; reviewers: string[] }[] = [];
+
+ for (const label of labels || []) {
let reviewers = this.getReviewersForLabel(label.name, exclusionList);
if (reviewers.length > 0) {
- labelToReviewerMapping[label.name] = reviewers;
- reviewersFound = true;
+ const priority = this.getPriorityForLabel(label.name);
+ if (bestMatches.length === 0 || priority < bestPriority) {
+ bestPriority = priority;
+ bestMatches = [{ label: label.name, reviewers }];
+ } else if (priority === bestPriority) {
+ bestMatches.push({ label: label.name, reviewers });
+ }
}
- });
- if (!reviewersFound) {
+ }
+ if (bestMatches.length > 0) {
+ const chosenMatch =
+ bestMatches[Math.floor(Math.random() * bestMatches.length)];
+ labelToReviewerMapping[chosenMatch.label] = chosenMatch.reviewers;
+ } else {
const fallbackReviewers = this.getFallbackReviewers(exclusionList);
if (fallbackReviewers.length > 0) {
- labelToReviewerMapping[NO_MATCHING_LABEL] =
- this.getFallbackReviewers(exclusionList);
+ labelToReviewerMapping[NO_MATCHING_LABEL] = fallbackReviewers;
}
}
return labelToReviewerMapping;
}
+ getPriorityForLabel(label: string): number {
+ const labelObjects = this.config.labels;
+ const labelObject = labelObjects.find(
+ (labelObject) => labelObject.name.toLowerCase() === label.toLowerCase()
+ );
+ if (!labelObject || typeof labelObject.priority !== "number") {
+ return Number.MAX_SAFE_INTEGER;
+ }
+ return labelObject.priority;
+ }
+
// Get possible reviewers excluding the author.
getReviewersForLabel(label: string, exclusionList: string[]): string[] {
const labelObjects = this.config.labels;
@@ -106,6 +128,8 @@ export class ReviewerConfig {
# add yourself to that label's exclusionList
# FallbackReviewers is for reviewers who can review any area of the code base
that might
# not receive a label. These should generally be more experienced committers.
+# Priority determines which label is used to select a reviewer when a PR has
multiple labels
+# (lower number = higher priority).
${yaml.dump(this.config)}`;
fs.writeFileSync(this.configPath, contents, { encoding: "utf-8" });
diff --git a/scripts/ci/pr-bot/shared/reviewersForLabel.ts
b/scripts/ci/pr-bot/shared/reviewersForLabel.ts
index f46f5359a8b..40b0a6c2a57 100644
--- a/scripts/ci/pr-bot/shared/reviewersForLabel.ts
+++ b/scripts/ci/pr-bot/shared/reviewersForLabel.ts
@@ -40,16 +40,14 @@ export class ReviewersForLabel {
}
}
- // Given a list of available reviewers,
- // returns the next reviewer up based on who has reviewed least recently.
- // Updates this object to reflect their assignment.
- assignNextReviewer(availableReviewers: string[]): string {
- if (availableReviewers.length === 0) {
- throw new Error(`No reviewers available for label ${this.label}`);
+ private findLeastRecentlyAssignedReviewer(
+ availableReviewers: string[]
+ ): string {
+ if (!availableReviewers || availableReviewers.length === 0) {
+ return "";
}
if (!this.dateOfLastReviewAssignment[availableReviewers[0]]) {
- this.dateOfLastReviewAssignment[availableReviewers[0]] = Date.now();
return availableReviewers[0];
}
@@ -59,7 +57,6 @@ export class ReviewersForLabel {
for (let i = 0; i < availableReviewers.length; i++) {
let availableReviewer = availableReviewers[i];
if (!this.dateOfLastReviewAssignment[availableReviewer]) {
- this.dateOfLastReviewAssignment[availableReviewer] = Date.now();
return availableReviewer;
}
if (earliestDate > this.dateOfLastReviewAssignment[availableReviewer]) {
@@ -68,10 +65,44 @@ export class ReviewersForLabel {
}
}
- this.dateOfLastReviewAssignment[earliestReviewer] = Date.now();
return earliestReviewer;
}
+ // Given a list of available reviewers,
+ // returns the next reviewer up based on who has reviewed least recently.
+ // Updates this object to reflect their assignment.
+ assignNextReviewer(availableReviewers: string[]): string {
+ if (availableReviewers.length === 0) {
+ throw new Error(`No reviewers available for label ${this.label}`);
+ }
+
+ const chosenReviewer =
+ this.findLeastRecentlyAssignedReviewer(availableReviewers);
+ this.dateOfLastReviewAssignment[chosenReviewer] = Date.now();
+ return chosenReviewer;
+ }
+
+ // Returns a backup reviewer from the same label (excluding the main
reviewer)
+ // without updating dateOfLastReviewAssignment. Falls back to
fallbackReviewers
+ // if the label has no other eligible reviewers.
+ getBackupReviewer(
+ availableReviewers: string[],
+ mainReviewer: string,
+ fallbackReviewers: string[] = []
+ ): string {
+ const remainingLabelReviewers = (availableReviewers || []).filter(
+ (reviewer) => reviewer !== mainReviewer
+ );
+ if (remainingLabelReviewers.length > 0) {
+ return this.findLeastRecentlyAssignedReviewer(remainingLabelReviewers);
+ }
+
+ const remainingFallbackReviewers = (fallbackReviewers || []).filter(
+ (reviewer) => reviewer !== mainReviewer
+ );
+ return this.findLeastRecentlyAssignedReviewer(remainingFallbackReviewers);
+ }
+
// Given the up to date list of available reviewers (excluding the author),
// returns the next reviewer up based on who has reviewed least recently.
// Updates this object to reflect their assignment.
diff --git a/scripts/ci/pr-bot/shared/userCommand.ts
b/scripts/ci/pr-bot/shared/userCommand.ts
index 8c09013553e..79852c475f4 100644
--- a/scripts/ci/pr-bot/shared/userCommand.ts
+++ b/scripts/ci/pr-bot/shared/userCommand.ts
@@ -92,12 +92,23 @@ async function assignToNextReviewer(
labelOfReviewer
);
const pullAuthor = github.getPullAuthorFromPayload(payload);
+ const exclusionList = [commentAuthor, pullAuthor];
let availableReviewers = reviewerConfig.getReviewersForLabel(
labelOfReviewer,
- [commentAuthor, pullAuthor]
+ exclusionList
);
+ const fallbackReviewers =
+ reviewerConfig.getFallbackReviewers(exclusionList);
let chosenReviewer = reviewersState.assignNextReviewer(availableReviewers);
+ let backupReviewer = reviewersState.getBackupReviewer(
+ availableReviewers,
+ chosenReviewer,
+ fallbackReviewers
+ );
prState.reviewersAssignedForLabels[labelOfReviewer] = chosenReviewer;
+ const backupReviewersForLabels: { [key: string]: string } = backupReviewer
+ ? { [labelOfReviewer]: backupReviewer }
+ : {};
// Comment assigning reviewer
console.log(`Assigning ${chosenReviewer}`);
@@ -107,6 +118,7 @@ async function assignToNextReviewer(
pullNumber,
commentStrings.assignReviewer(prState.reviewersAssignedForLabels, {
labels: existingLabels,
+ backupReviewers: backupReviewersForLabels,
})
);
await github.requestPrReviewers(pullNumber, [chosenReviewer]);
@@ -206,11 +218,14 @@ async function assignReviewerSet(
const existingLabels = payload.issue?.labels || payload.pull_request?.labels;
const pullAuthor = github.getPullAuthorFromPayload(payload);
+ const exclusionList = [pullAuthor];
const reviewersForLabels = reviewerConfig.getReviewersForLabels(
existingLabels,
- [pullAuthor]
+ exclusionList
);
+ const fallbackReviewers = reviewerConfig.getFallbackReviewers(exclusionList);
let reviewerStateToUpdate = {};
+ let backupReviewersForLabels: { [key: string]: string } = {};
var labels = Object.keys(reviewersForLabels);
if (!labels || labels.length == 0) {
await github.addPrComment(
@@ -224,14 +239,23 @@ async function assignReviewerSet(
let availableReviewers = reviewersForLabels[label];
let reviewersState = await stateClient.getReviewersForLabelState(label);
let chosenReviewer = reviewersState.assignNextReviewer(availableReviewers);
+ let backupReviewer = reviewersState.getBackupReviewer(
+ availableReviewers,
+ chosenReviewer,
+ fallbackReviewers
+ );
reviewerStateToUpdate[label] = reviewersState;
prState.reviewersAssignedForLabels[label] = chosenReviewer;
+ if (backupReviewer) {
+ backupReviewersForLabels[label] = backupReviewer;
+ }
}
console.log(`Assigning reviewers for pr ${pullNumber}`);
await github.addPrComment(
pullNumber,
commentStrings.assignReviewer(prState.reviewersAssignedForLabels, {
labels: existingLabels,
+ backupReviewers: backupReviewersForLabels,
})
);
await github.requestPrReviewers(
diff --git a/scripts/ci/pr-bot/test/commentStringsTest.ts
b/scripts/ci/pr-bot/test/commentStringsTest.ts
index f2a16ca1c63..62456fd4994 100644
--- a/scripts/ci/pr-bot/test/commentStringsTest.ts
+++ b/scripts/ci/pr-bot/test/commentStringsTest.ts
@@ -41,5 +41,18 @@ describe("commentStrings", function () {
const comment = commentStrings.assignReviewer({ core: "reviewer1" });
assert(comment.includes("review with scrutiny"));
});
+
+ it("should mention backup reviewer without tagging them with @", function
() {
+ const comment = commentStrings.assignReviewer(
+ { Java: "mainReviewer" },
+ {
+ labels: [{ name: "Java" }],
+ backupReviewers: { Java: "backupUser" },
+ }
+ );
+ assert(comment.includes("R: @mainReviewer for label Java."));
+ assert(comment.includes("Backup reviewer: backupUser"));
+ assert(!comment.includes("@backupUser"));
+ });
});
});
diff --git a/scripts/ci/pr-bot/test/reviewerConfigTest.ts
b/scripts/ci/pr-bot/test/reviewerConfigTest.ts
index 589aab635bc..95c39188824 100644
--- a/scripts/ci/pr-bot/test/reviewerConfigTest.ts
+++ b/scripts/ci/pr-bot/test/reviewerConfigTest.ts
@@ -22,15 +22,22 @@ const { ReviewerConfig } =
require("../shared/reviewerConfig");
const configPath = "test-config.yml";
const configContents = `labels:
- name: "Go"
+ priority: 2
reviewers: ["testReviewer1", "testReviewer2"]
exclusionList: ["testReviewer3"] # These users will never be suggested as
reviewers
# I don't know the other areas well enough to assess who the normal
committers/contributors who might want to be reviewers are
- name: "Java"
+ priority: 3
reviewers: ["testReviewer3", "testReviewer2"]
exclusionList: [] # These users will never be suggested as reviewers
- name: "Python"
+ priority: 1
reviewers: ["testReviewer4"]
exclusionList: [] # These users will never be suggested as reviewers
+- name: "Kafka"
+ priority: 1
+ reviewers: ["testReviewer5"]
+ exclusionList: []
fallbackReviewers: ["testReviewer5", "testReviewer1", "testReviewer3"] # List
of committers to use when no label matches
`;
describe("ReviewerConfig", function () {
@@ -46,12 +53,13 @@ describe("ReviewerConfig", function () {
});
describe("getReviewersForLabels()", function () {
- it("should return all reviewers configured for all labels", function () {
+ it("should return reviewers only for the highest-priority matching label",
function () {
const config = new ReviewerConfig(configPath);
const reviewersForLabels = config.getReviewersForLabels(
- [{ name: "Go" }, { name: "Java" }],
+ [{ name: "Java" }, { name: "Go" }],
[]
);
+ assert.deepEqual(Object.keys(reviewersForLabels), ["Go"]);
assert(
reviewersForLabels["Go"].find(
(reviewer) => reviewer === "testReviewer1"
@@ -76,44 +84,51 @@ describe("ReviewerConfig", function () {
),
"Return value for Go label should not include testReviewer4"
);
-
- assert(
- reviewersForLabels["Java"].find(
- (reviewer) => reviewer === "testReviewer3"
- ),
- "Return value for Java label should include testReviewer3"
- );
- assert(
- reviewersForLabels["Java"].find(
- (reviewer) => reviewer === "testReviewer2"
- ),
- "Return value for Java label should include testReviewer2"
- );
- assert(
- !reviewersForLabels["Java"].find(
- (reviewer) => reviewer === "testReviewer4"
- ),
- "Return value for Java label should not include testReviewer4"
- );
- assert(
- !reviewersForLabels["Java"].find(
- (reviewer) => reviewer === "testReviewer1"
- ),
- "Return value for Java label should not include testReviewer1"
- );
assert(
- !reviewersForLabels["Java"].find(
- (reviewer) => reviewer === "testReviewer5"
- ),
- "Return value for Java label should not include testReviewer5"
+ Object.keys(reviewersForLabels).indexOf("Java") == -1,
+ "Lower-priority Java label should not be included when Go matches"
);
-
assert(
Object.keys(reviewersForLabels).indexOf("Python") == -1,
"No reviewers should be included for python"
);
});
+ it("should choose randomly when multiple labels share the highest
priority", function () {
+ const config = new ReviewerConfig(configPath);
+ const origRandom = Math.random;
+ try {
+ Math.random = () => 0.1;
+ const firstChoice = config.getReviewersForLabels(
+ [{ name: "Python" }, { name: "Kafka" }, { name: "Java" }],
+ []
+ );
+ assert.deepEqual(Object.keys(firstChoice), ["Python"]);
+
+ Math.random = () => 0.9;
+ const secondChoice = config.getReviewersForLabels(
+ [{ name: "Python" }, { name: "Kafka" }, { name: "Java" }],
+ []
+ );
+ assert.deepEqual(Object.keys(secondChoice), ["Kafka"]);
+ } finally {
+ Math.random = origRandom;
+ }
+ });
+
+ it("should fall back to next highest-priority label if top priority label
has all reviewers excluded", function () {
+ const config = new ReviewerConfig(configPath);
+ const reviewersForLabels = config.getReviewersForLabels(
+ [{ name: "Python" }, { name: "Java" }],
+ ["testReviewer4"]
+ );
+ assert.deepEqual(Object.keys(reviewersForLabels), ["Java"]);
+ assert.deepEqual(reviewersForLabels["Java"], [
+ "testReviewer3",
+ "testReviewer2",
+ ]);
+ });
+
it("should return no entry if a label is not configured", function () {
const config = new ReviewerConfig(configPath);
const reviewersForLabels = config.getReviewersForLabels(
diff --git a/scripts/ci/pr-bot/test/reviewersForLabelTest.ts
b/scripts/ci/pr-bot/test/reviewersForLabelTest.ts
index f4257505059..899b0519c1f 100644
--- a/scripts/ci/pr-bot/test/reviewersForLabelTest.ts
+++ b/scripts/ci/pr-bot/test/reviewersForLabelTest.ts
@@ -132,4 +132,50 @@ describe("ReviewersForLabel", function () {
assert.throws(() => reviewersForGo.assignNextReviewer([]));
});
});
+
+ describe("getBackupReviewer()", function () {
+ it("should pick the next least recently assigned reviewer from the label
without updating timestamps", function () {
+ const dateOfLastReviewAssignment = {
+ testReviewer1: 10,
+ testReviewer2: 20,
+ testReviewer3: 30,
+ };
+ let reviewersForGo = new ReviewersForLabel("Go", {
+ dateOfLastReviewAssignment: { ...dateOfLastReviewAssignment },
+ });
+
+ const backup = reviewersForGo.getBackupReviewer(
+ ["testReviewer1", "testReviewer2", "testReviewer3"],
+ "testReviewer1",
+ ["fallback1"]
+ );
+ assert.equal(backup, "testReviewer2");
+ assert.deepEqual(
+ reviewersForGo.dateOfLastReviewAssignment,
+ dateOfLastReviewAssignment
+ );
+ });
+
+ it("should fall back to fallbackReviewers when label only has one
available reviewer", function () {
+ const dateOfLastReviewAssignment = {
+ testReviewer1: 10,
+ fallback1: 50,
+ fallback2: 15,
+ };
+ let reviewersForSpanner = new ReviewersForLabel("spanner", {
+ dateOfLastReviewAssignment: { ...dateOfLastReviewAssignment },
+ });
+
+ const backup = reviewersForSpanner.getBackupReviewer(
+ ["testReviewer1"],
+ "testReviewer1",
+ ["testReviewer1", "fallback1", "fallback2"]
+ );
+ assert.equal(backup, "fallback2");
+ assert.deepEqual(
+ reviewersForSpanner.dateOfLastReviewAssignment,
+ dateOfLastReviewAssignment
+ );
+ });
+ });
});
