33
44const assert = require ( 'assert' ) ;
55const {
6+ MARKER ,
67 matchesPattern,
78 labelFromPattern,
89 attributeFiles,
@@ -11,9 +12,33 @@ const {
1112 buildBody,
1213 parseExcluded,
1314 serializeExcluded,
15+ withExcluded,
1416 planSynchronizeSwaps,
17+ coordinateReviewers,
1518} = require ( './review-checklist.js' ) ;
1619
20+ // Minimal recording mock for the github.rest surface coordinateReviewers
21+ // touches. Each call resolves successfully and is appended to its call log.
22+ function makeGithubMock ( ) {
23+ const calls = { removeRequestedReviewers : [ ] , requestReviewers : [ ] , addAssignees : [ ] } ;
24+ return {
25+ calls,
26+ rest : {
27+ pulls : {
28+ removeRequestedReviewers : async ( opts ) => { calls . removeRequestedReviewers . push ( opts . reviewers [ 0 ] ) ; } ,
29+ requestReviewers : async ( opts ) => { calls . requestReviewers . push ( opts . reviewers [ 0 ] ) ; } ,
30+ } ,
31+ issues : {
32+ addAssignees : async ( opts ) => { calls . addAssignees . push ( opts . assignees [ 0 ] ) ; } ,
33+ } ,
34+ } ,
35+ } ;
36+ }
37+
38+ function makeCore ( ) {
39+ return { info : ( ) => { } , warning : ( ) => { } , setFailed : ( ) => { } } ;
40+ }
41+
1742let passed = 0 ;
1843let failed = 0 ;
1944
@@ -28,6 +53,14 @@ function test(name, fn) {
2853 }
2954}
3055
56+ // coordinateReviewers exercises real async API calls (mocked). test() doesn't
57+ // await, so an async fn's assertions would run after the pass/fail tally is
58+ // already printed — queue these separately and await them before the summary.
59+ const asyncTests = [ ] ;
60+ function asyncTest ( name , fn ) {
61+ asyncTests . push ( { name, fn } ) ;
62+ }
63+
3164// ----------------------------------------------------------------
3265// matchesPattern — anchored directory patterns
3366// ----------------------------------------------------------------
@@ -500,6 +533,34 @@ test('serializeExcluded: round-trips through parseExcluded', () => {
500533 assert . deepStrictEqual ( [ ...roundTripped ] . sort ( ) , [ 'alice' , 'bob' ] ) ;
501534} ) ;
502535
536+ // ----------------------------------------------------------------
537+ // withExcluded — used by /remove-reviewer to persist a deliberate removal
538+ // into the checklist comment's exclusion marker.
539+ // ----------------------------------------------------------------
540+
541+ test ( 'withExcluded: replaces an existing marker in place, preserving the rest of the body' , ( ) => {
542+ const body = `${ MARKER } ;
543+ const updated = withExcluded ( body , new Set ( [ 'alice' , 'bob' ] ) ) ;
544+ assert . ok ( updated . includes ( 'some checklist text' ) ) ;
545+ assert . ok ( updated . startsWith ( MARKER ) ) ;
546+ assert . deepStrictEqual ( [ ...parseExcluded ( updated ) ] . sort ( ) , [ 'alice' , 'bob' ] ) ;
547+ // Only one marker present afterward — not appended alongside the old one.
548+ assert . strictEqual ( updated . split ( 'hdf5-review-checklist-excluded:' ) . length - 1 , 1 ) ;
549+ } ) ;
550+
551+ test ( 'withExcluded: appends a marker when the body has none' , ( ) => {
552+ const body = `${ MARKER } ;
553+ const updated = withExcluded ( body , new Set ( [ 'alice' ] ) ) ;
554+ assert . ok ( updated . includes ( 'some checklist text' ) ) ;
555+ assert . deepStrictEqual ( [ ...parseExcluded ( updated ) ] , [ 'alice' ] ) ;
556+ } ) ;
557+
558+ test ( 'withExcluded: round-trips an empty set to the empty marker' , ( ) => {
559+ const body = `${ MARKER } ;
560+ const updated = withExcluded ( body , new Set ( ) ) ;
561+ assert . strictEqual ( parseExcluded ( updated ) . size , 0 ) ;
562+ } ) ;
563+
503564// ----------------------------------------------------------------
504565// planSynchronizeSwaps
505566// ----------------------------------------------------------------
@@ -651,10 +712,143 @@ test('planSynchronizeSwaps: dismissedOwner already covering a different area is
651712 assert . strictEqual ( swaps . length , 0 ) ;
652713} ) ;
653714
715+ // ----------------------------------------------------------------
716+ // coordinateReviewers — ready_for_review CODEOWNERS-avalanche pruning
717+ // ----------------------------------------------------------------
718+
719+ function makeCoordinateBaseArgs ( overrides ) {
720+ const area = makeArea ( '.github' , [ 'hyoklee' , 'lrknox' , 'jhendersonHDF' , 'glennsong09' ] , 5 ) ;
721+ return {
722+ owner : 'HDFGroup' , repo : 'hdf5' , pr_number : 1 ,
723+ prData : {
724+ user : { login : 'lrknox' } ,
725+ draft : false ,
726+ requested_reviewers : [ { login : 'hyoklee' } , { login : 'jhendersonHDF' } , { login : 'glennsong09' } ] ,
727+ } ,
728+ allCodeOwners : new Set ( [ 'hyoklee' , 'lrknox' , 'jhendersonHDF' , 'glennsong09' ] ) ,
729+ catchAllOwners : new Set ( ) ,
730+ touchedAreas : [ area ] ,
731+ reviewerLoad : { } ,
732+ excludedReviewers : new Set ( ) ,
733+ allReviews : [ ] ,
734+ LINE_THRESHOLD : 300 ,
735+ AREA_THRESHOLDS : { } ,
736+ PUBLIC_HEADER : / p u b l i c \. h $ / ,
737+ ...overrides ,
738+ } ;
739+ }
740+
741+ asyncTest ( 'coordinateReviewers: ready_for_review prunes the CODEOWNERS avalanche to one load-balanced pick' , async ( ) => {
742+ const github = makeGithubMock ( ) ;
743+ const context = { eventName : 'pull_request_target' , payload : { action : 'ready_for_review' , sender : { type : 'User' } } } ;
744+ const args = makeCoordinateBaseArgs ( ) ;
745+
746+ const { confirmedRequested } = await coordinateReviewers ( github , context , makeCore ( ) , args ) ;
747+
748+ // hyoklee is first non-author owner in CODEOWNERS order with equal (zero) load.
749+ assert . deepStrictEqual ( [ ...confirmedRequested ] , [ 'hyoklee' ] ) ;
750+ // The other two avalanche-assigned owners get removed.
751+ assert . ok ( github . calls . removeRequestedReviewers . includes ( 'jhendersonHDF' ) ) ;
752+ assert . ok ( github . calls . removeRequestedReviewers . includes ( 'glennsong09' ) ) ;
753+ assert . strictEqual ( github . calls . removeRequestedReviewers . length , 2 ) ;
754+ // hyoklee was already requested, so no redundant request call.
755+ assert . strictEqual ( github . calls . requestReviewers . length , 0 ) ;
756+ } ) ;
757+
758+ asyncTest ( 'coordinateReviewers: ready_for_review on a draft-opened PR (still draft) does not prune' , async ( ) => {
759+ // Sanity check the branch ordering: if somehow still draft (shouldn't
760+ // happen for a real ready_for_review payload, but guards the isDraft
761+ // branch precedence), the draft path's "leave alone" rule wins.
762+ const github = makeGithubMock ( ) ;
763+ const context = { eventName : 'pull_request_target' , payload : { action : 'ready_for_review' , sender : { type : 'User' } } } ;
764+ const args = makeCoordinateBaseArgs ( { prData : { ...makeCoordinateBaseArgs ( ) . prData , draft : true } } ) ;
765+
766+ const { confirmedRequested } = await coordinateReviewers ( github , context , makeCore ( ) , args ) ;
767+
768+ assert . strictEqual ( github . calls . removeRequestedReviewers . length , 0 ) ;
769+ assert . deepStrictEqual ( [ ...confirmedRequested ] . sort ( ) , [ 'glennsong09' , 'hyoklee' , 'jhendersonHDF' ] ) ;
770+ } ) ;
771+
772+ asyncTest ( 'coordinateReviewers: plain synchronize (no dismissed reviews) is left to additive fill, not pruned' , async ( ) => {
773+ // Contrast case: a synchronize with no dismissed reviews must NOT trigger
774+ // avalanche-style pruning — only opened/reopened/ready_for_review do. All
775+ // three avalanche-style owners stay; nothing is removed, nothing new is
776+ // requested since the area is already covered.
777+ const github = makeGithubMock ( ) ;
778+ const context = { eventName : 'pull_request_target' , payload : { action : 'synchronize' , sender : { type : 'User' } } } ;
779+ const args = makeCoordinateBaseArgs ( ) ;
780+
781+ const { confirmedRequested } = await coordinateReviewers ( github , context , makeCore ( ) , args ) ;
782+
783+ assert . strictEqual ( github . calls . removeRequestedReviewers . length , 0 ) ;
784+ assert . strictEqual ( github . calls . requestReviewers . length , 0 ) ;
785+ assert . deepStrictEqual ( [ ...confirmedRequested ] . sort ( ) , [ 'glennsong09' , 'hyoklee' , 'jhendersonHDF' ] ) ;
786+ } ) ;
787+
788+ // ----------------------------------------------------------------
789+ // coordinateReviewers — bot-self-triggered review_request_removed must not
790+ // create a sticky exclusion (the bot's own removeUnselected/removeRequestedReviewers
791+ // calls fire this very event and would otherwise self-trigger a run that reads
792+ // its own bookkeeping removal as a deliberate human decision).
793+ // ----------------------------------------------------------------
794+
795+ function makeRemovalContext ( senderType ) {
796+ return {
797+ eventName : 'pull_request_target' ,
798+ payload : {
799+ action : 'review_request_removed' ,
800+ requested_reviewer : { login : 'jhendersonHDF' } ,
801+ sender : { type : senderType } ,
802+ } ,
803+ } ;
804+ }
805+
806+ asyncTest ( 'coordinateReviewers: bot-sender review_request_removed does not persist a sticky exclusion' , async ( ) => {
807+ const github = makeGithubMock ( ) ;
808+ const args = makeCoordinateBaseArgs ( {
809+ prData : {
810+ user : { login : 'lrknox' } ,
811+ draft : false ,
812+ requested_reviewers : [ { login : 'hyoklee' } , { login : 'glennsong09' } ] ,
813+ } ,
814+ } ) ;
815+
816+ const { excludedReviewers } = await coordinateReviewers ( github , makeRemovalContext ( 'Bot' ) , makeCore ( ) , args ) ;
817+
818+ assert . ok ( ! excludedReviewers . has ( 'jhendersonHDF' ) ) ;
819+ } ) ;
820+
821+ asyncTest ( 'coordinateReviewers: human-sender review_request_removed does persist a sticky exclusion' , async ( ) => {
822+ const github = makeGithubMock ( ) ;
823+ const args = makeCoordinateBaseArgs ( {
824+ prData : {
825+ user : { login : 'lrknox' } ,
826+ draft : false ,
827+ requested_reviewers : [ { login : 'hyoklee' } , { login : 'glennsong09' } ] ,
828+ } ,
829+ } ) ;
830+
831+ const { excludedReviewers } = await coordinateReviewers ( github , makeRemovalContext ( 'User' ) , makeCore ( ) , args ) ;
832+
833+ assert . ok ( excludedReviewers . has ( 'jhendersonHDF' ) ) ;
834+ } ) ;
835+
654836// ----------------------------------------------------------------
655837// Summary
656838// ----------------------------------------------------------------
657839
658- console . log ( '' ) ;
659- console . log ( `${ passed } ${ failed } ) ;
660- process . exit ( failed > 0 ? 1 : 0 ) ;
840+ ( async ( ) => {
841+ for ( const { name, fn } of asyncTests ) {
842+ try {
843+ await fn ( ) ;
844+ console . log ( `✓ ${ name } ) ;
845+ passed ++ ;
846+ } catch ( e ) {
847+ console . log ( `✗ ${ name } ${ e . message } ) ;
848+ failed ++ ;
849+ }
850+ }
851+ console . log ( '' ) ;
852+ console . log ( `${ passed } ${ failed } ) ;
853+ process . exit ( failed > 0 ? 1 : 0 ) ;
854+ } ) ( ) ;
0 commit comments