feat(dev-infra): update pullapprove verification to ensure all groups have reviewers (#42614)

Update the pullapprove verification tooling to ensure a reviewer is defined for
each group. This is being done in preparation for the upcoming change to how
pullapprove billing works. The new billing will work on a seats based approach
rather than flat usage.

PR Close #42614
This commit is contained in:
Joey Perrott 2021-06-21 10:06:24 -07:00 committed by Dylan Hunn
parent 75bbcf7c2f
commit 81a19e4e65
3 changed files with 44 additions and 7 deletions

View File

@ -4917,11 +4917,13 @@ const FALLBACK_GROUP_NAME = 'fallback';
/** A PullApprove group to be able to test files against. */ /** A PullApprove group to be able to test files against. */
class PullApproveGroup { class PullApproveGroup {
constructor(groupName, config, precedingGroups = []) { constructor(groupName, config, precedingGroups = []) {
var _a;
this.groupName = groupName; this.groupName = groupName;
this.precedingGroups = precedingGroups; this.precedingGroups = precedingGroups;
/** List of conditions for the group. */ /** List of conditions for the group. */
this.conditions = []; this.conditions = [];
this._captureConditions(config); this._captureConditions(config);
this.reviewers = (_a = config.reviewers) !== null && _a !== void 0 ? _a : { users: [], teams: [] };
} }
_captureConditions(config) { _captureConditions(config) {
if (config.conditions && this.groupName !== FALLBACK_GROUP_NAME) { if (config.conditions && this.groupName !== FALLBACK_GROUP_NAME) {
@ -5063,12 +5065,16 @@ function verify$1() {
* Whether all group condition lines match at least one file and all files * Whether all group condition lines match at least one file and all files
* are matched by at least one group. * are matched by at least one group.
*/ */
const verificationSucceeded = resultsByGroup.every(r => !r.unmatchedCount) && !unmatchedFiles.length; const allGroupConditionsValid = resultsByGroup.every(r => !r.unmatchedCount) && !unmatchedFiles.length;
/** Whether all groups have at least one reviewer user or team defined. */
const groupsWithoutReviewers = groups.filter(group => Object.keys(group.reviewers).length === 0);
/** The overall result of the verifcation. */
const overallResult = allGroupConditionsValid && groupsWithoutReviewers.length === 0;
/** /**
* Overall result * Overall result
*/ */
logHeader('Overall Result'); logHeader('Overall Result');
if (verificationSucceeded) { if (overallResult) {
info('PullApprove verification succeeded!'); info('PullApprove verification succeeded!');
} }
else { else {
@ -5078,6 +5084,16 @@ function verify$1() {
info(`files/directories have owners and all patterns that appear in`); info(`files/directories have owners and all patterns that appear in`);
info(`the file correspond to actual files/directories in the repo.`); info(`the file correspond to actual files/directories in the repo.`);
} }
/** Reviewers check */
logHeader(`Group Reviewers Check`);
if (groupsWithoutReviewers.length === 0) {
info('All group contain at least one reviewer user or team.');
}
else {
info.group(`Discovered ${groupsWithoutReviewers.length} group(s) without a reviewer defined`);
groupsWithoutReviewers.forEach(g => info(g.groupName));
info.groupEnd();
}
/** /**
* File by file Summary * File by file Summary
*/ */
@ -5108,7 +5124,7 @@ function verify$1() {
unverifiableConditionsInGroups.forEach(group => logGroup(group, 'unverifiableConditions')); unverifiableConditionsInGroups.forEach(group => logGroup(group, 'unverifiableConditions'));
info.groupEnd(); info.groupEnd();
// Provide correct exit code based on verification success. // Provide correct exit code based on verification success.
process.exit(verificationSucceeded ? 0 : 1); process.exit(overallResult ? 0 : 1);
} }
/** Build the parser for the pullapprove commands. */ /** Build the parser for the pullapprove commands. */

View File

@ -19,6 +19,11 @@ interface GroupCondition {
unverifiable: boolean; unverifiable: boolean;
} }
interface GroupReviewers {
users?: string[];
teams?: string[];
}
/** Result of testing files against the group. */ /** Result of testing files against the group. */
export interface PullApproveGroupResult { export interface PullApproveGroupResult {
groupName: string; groupName: string;
@ -40,12 +45,15 @@ const FALLBACK_GROUP_NAME = 'fallback';
/** A PullApprove group to be able to test files against. */ /** A PullApprove group to be able to test files against. */
export class PullApproveGroup { export class PullApproveGroup {
/** List of conditions for the group. */ /** List of conditions for the group. */
conditions: GroupCondition[] = []; readonly conditions: GroupCondition[] = [];
/** List of reviewers for the group. */
readonly reviewers: GroupReviewers;
constructor( constructor(
public groupName: string, config: PullApproveGroupConfig, public groupName: string, config: PullApproveGroupConfig,
readonly precedingGroups: PullApproveGroup[] = []) { readonly precedingGroups: PullApproveGroup[] = []) {
this._captureConditions(config); this._captureConditions(config);
this.reviewers = config.reviewers ?? {users: [], teams: []};
} }
private _captureConditions(config: PullApproveGroupConfig) { private _captureConditions(config: PullApproveGroupConfig) {

View File

@ -49,14 +49,18 @@ export function verify() {
* Whether all group condition lines match at least one file and all files * Whether all group condition lines match at least one file and all files
* are matched by at least one group. * are matched by at least one group.
*/ */
const verificationSucceeded = const allGroupConditionsValid =
resultsByGroup.every(r => !r.unmatchedCount) && !unmatchedFiles.length; resultsByGroup.every(r => !r.unmatchedCount) && !unmatchedFiles.length;
/** Whether all groups have at least one reviewer user or team defined. */
const groupsWithoutReviewers = groups.filter(group => Object.keys(group.reviewers).length === 0);
/** The overall result of the verifcation. */
const overallResult = allGroupConditionsValid && groupsWithoutReviewers.length === 0;
/** /**
* Overall result * Overall result
*/ */
logHeader('Overall Result'); logHeader('Overall Result');
if (verificationSucceeded) { if (overallResult) {
info('PullApprove verification succeeded!'); info('PullApprove verification succeeded!');
} else { } else {
info(`PullApprove verification failed.`); info(`PullApprove verification failed.`);
@ -65,6 +69,15 @@ export function verify() {
info(`files/directories have owners and all patterns that appear in`); info(`files/directories have owners and all patterns that appear in`);
info(`the file correspond to actual files/directories in the repo.`); info(`the file correspond to actual files/directories in the repo.`);
} }
/** Reviewers check */
logHeader(`Group Reviewers Check`);
if (groupsWithoutReviewers.length === 0) {
info('All group contain at least one reviewer user or team.');
} else {
info.group(`Discovered ${groupsWithoutReviewers.length} group(s) without a reviewer defined`);
groupsWithoutReviewers.forEach(g => info(g.groupName));
info.groupEnd();
}
/** /**
* File by file Summary * File by file Summary
*/ */
@ -97,5 +110,5 @@ export function verify() {
info.groupEnd(); info.groupEnd();
// Provide correct exit code based on verification success. // Provide correct exit code based on verification success.
process.exit(verificationSucceeded ? 0 : 1); process.exit(overallResult ? 0 : 1);
} }