Skip to content

Commit de6dba3

Browse files
GrantBirkiGrantBirki
authored
Merge pull request #363 from github/check-fixes
bug: `checks` input option is not respecting required checks in the pending state
2 parents c8417fc + 37c13cf commit de6dba3

File tree

4 files changed

+307
-73
lines changed

4 files changed

+307
-73
lines changed

‎__tests__/functions/prechecks.test.js

+160-26
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,9 @@ test('runs prechecks and finds that the IssueOps command is valid for a branch d
303303
isFork: false
304304
})
305305

306-
expect(debugMock).toHaveBeenCalledWith('ignoring ci check: markdown-lint')
306+
expect(debugMock).toHaveBeenCalledWith(
307+
'filterChecks() - ignoring ci check: markdown-lint'
308+
)
307309
})
308310

309311
test('runs prechecks and finds that the IssueOps command is valid for a branch deployment with a few explictly requested checks and a few ignored checks', async () => {
@@ -375,13 +377,113 @@ test('runs prechecks and finds that the IssueOps command is valid for a branch d
375377
isFork: false
376378
})
377379

378-
expect(debugMock).toHaveBeenCalledWith('explicitly including ci check: test')
379380
expect(debugMock).toHaveBeenCalledWith(
380-
'explicitly including ci check: acceptance-test'
381+
'filterChecks() - explicitly including ci check: test'
382+
)
383+
expect(debugMock).toHaveBeenCalledWith(
384+
'filterChecks() - explicitly including ci check: acceptance-test'
385+
)
386+
expect(debugMock).toHaveBeenCalledWith(
387+
'filterChecks() - explicitly including ci check: lint'
388+
)
389+
expect(debugMock).toHaveBeenCalledWith(
390+
'filterChecks() - markdown-lint is not in the explicit list of checks to include (test,acceptance-test,lint)'
391+
)
392+
expect(debugMock).not.toHaveBeenCalledWith(
393+
'filterChecks() - ignoring ci check: markdown-lint'
394+
)
395+
expect(debugMock).toHaveBeenCalledWith(
396+
'filterChecks() - ignoring ci check: lint'
397+
)
398+
})
399+
400+
test('runs prechecks and finds that the IssueOps command is valid for a branch deployment with a few explictly requested checks and a few ignored checks but one CI check is missing', async () => {
401+
octokit.graphql = jest.fn().mockReturnValue({
402+
repository: {
403+
pullRequest: {
404+
reviewDecision: 'APPROVED',
405+
mergeStateStatus: 'CLEAN',
406+
reviews: {
407+
totalCount: 1
408+
},
409+
commits: {
410+
nodes: [
411+
{
412+
commit: {
413+
oid: 'abc123',
414+
checkSuites: {
415+
totalCount: 5
416+
},
417+
statusCheckRollup: {
418+
state: 'FAILURE',
419+
contexts: {
420+
nodes: [
421+
{
422+
isRequired: true,
423+
conclusion: 'SUCCESS',
424+
name: 'test'
425+
},
426+
{
427+
isRequired: false,
428+
conclusion: 'SUCCESS',
429+
name: 'acceptance-test'
430+
},
431+
{
432+
isRequired: true,
433+
conclusion: 'SKIPPED',
434+
name: 'lint'
435+
},
436+
{
437+
isRequired: false,
438+
conclusion: 'FAILURE',
439+
name: 'build'
440+
},
441+
{
442+
isRequired: true,
443+
conclusion: 'FAILURE',
444+
name: 'markdown-lint'
445+
}
446+
]
447+
}
448+
}
449+
}
450+
}
451+
]
452+
}
453+
}
454+
}
455+
})
456+
457+
data.inputs.checks = ['test', 'acceptance-test', 'quality-control', 'lint']
458+
data.inputs.ignored_checks = ['lint']
459+
460+
expect(await prechecks(context, octokit, data)).toStrictEqual({
461+
message:
462+
'### ⚠️ Cannot proceed with deployment\n\n- reviewDecision: `APPROVED`\n- commitStatus: `MISSING`\n\n> The `checks` input option requires that all of the following checks are passing: `test,acceptance-test,quality-control,lint`. However, the following checks are missing: `quality-control`',
463+
status: false
464+
})
465+
466+
expect(warningMock).toHaveBeenCalledWith(
467+
`the ${COLORS.info}checks${COLORS.reset} input option requires that all of the following checks are passing: ${COLORS.highlight}${data.inputs.checks.join(', ')}${COLORS.reset} - however, the following checks are missing: ${COLORS.highlight}quality-control${COLORS.reset}`
468+
)
469+
expect(debugMock).not.toHaveBeenCalledWith(
470+
'filterChecks() - explicitly including ci check: test'
471+
)
472+
expect(debugMock).not.toHaveBeenCalledWith(
473+
'filterChecks() - explicitly including ci check: acceptance-test'
474+
)
475+
expect(debugMock).not.toHaveBeenCalledWith(
476+
'filterChecks() - explicitly including ci check: lint'
477+
)
478+
expect(debugMock).not.toHaveBeenCalledWith(
479+
'filterChecks() - markdown-lint is not in the explicit list of checks to include (test,acceptance-test,lint)'
480+
)
481+
expect(debugMock).not.toHaveBeenCalledWith(
482+
'filterChecks() - ignoring ci check: markdown-lint'
483+
)
484+
expect(debugMock).not.toHaveBeenCalledWith(
485+
'filterChecks() - ignoring ci check: lint'
381486
)
382-
expect(debugMock).toHaveBeenCalledWith('explicitly including ci check: lint')
383-
expect(debugMock).not.toHaveBeenCalledWith('ignoring ci check: markdown-lint')
384-
expect(debugMock).toHaveBeenCalledWith('ignoring ci check: lint')
385487
})
386488

387489
test('runs prechecks and finds that the IssueOps command is valid for a branch deployment but checks and ignore checks cancel eachother out', async () => {
@@ -465,22 +567,38 @@ test('runs prechecks and finds that the IssueOps command is valid for a branch d
465567
isFork: false
466568
})
467569

468-
expect(debugMock).toHaveBeenCalledWith('explicitly including ci check: test')
469570
expect(debugMock).toHaveBeenCalledWith(
470-
'explicitly including ci check: acceptance-test'
571+
'filterChecks() - explicitly including ci check: test'
572+
)
573+
expect(debugMock).toHaveBeenCalledWith(
574+
'filterChecks() - explicitly including ci check: acceptance-test'
471575
)
472-
expect(debugMock).toHaveBeenCalledWith('explicitly including ci check: lint')
473576
expect(debugMock).toHaveBeenCalledWith(
474-
'explicitly including ci check: markdown-lint'
475-
)
476-
expect(debugMock).toHaveBeenCalledWith('explicitly including ci check: build')
477-
expect(debugMock).toHaveBeenCalledWith('ignoring ci check: markdown-lint')
478-
expect(debugMock).toHaveBeenCalledWith('ignoring ci check: lint')
479-
expect(debugMock).toHaveBeenCalledWith('ignoring ci check: build')
480-
expect(debugMock).toHaveBeenCalledWith('ignoring ci check: test')
481-
expect(debugMock).toHaveBeenCalledWith('ignoring ci check: acceptance-test')
577+
'filterChecks() - explicitly including ci check: lint'
578+
)
579+
expect(debugMock).toHaveBeenCalledWith(
580+
'filterChecks() - explicitly including ci check: markdown-lint'
581+
)
582+
expect(debugMock).toHaveBeenCalledWith(
583+
'filterChecks() - explicitly including ci check: build'
584+
)
585+
expect(debugMock).toHaveBeenCalledWith(
586+
'filterChecks() - ignoring ci check: markdown-lint'
587+
)
588+
expect(debugMock).toHaveBeenCalledWith(
589+
'filterChecks() - ignoring ci check: lint'
590+
)
591+
expect(debugMock).toHaveBeenCalledWith(
592+
'filterChecks() - ignoring ci check: build'
593+
)
594+
expect(debugMock).toHaveBeenCalledWith(
595+
'filterChecks() - ignoring ci check: test'
596+
)
597+
expect(debugMock).toHaveBeenCalledWith(
598+
'filterChecks() - ignoring ci check: acceptance-test'
599+
)
482600
expect(debugMock).toHaveBeenCalledWith(
483-
'after filtering, no checks remain - this will result in a SUCCESS state as it is treated as if no checks are defined'
601+
'filterChecks() - after filtering, no checks remain - this will result in a SUCCESS state as it is treated as if no checks are defined'
484602
)
485603
})
486604

@@ -553,8 +671,12 @@ test('runs prechecks and finds that the IssueOps command is valid for a branch d
553671
isFork: false
554672
})
555673

556-
expect(debugMock).toHaveBeenCalledWith('ignoring ci check: build')
557-
expect(debugMock).toHaveBeenCalledWith('ignoring ci check: markdown-lint')
674+
expect(debugMock).toHaveBeenCalledWith(
675+
'filterChecks() - ignoring ci check: build'
676+
)
677+
expect(debugMock).toHaveBeenCalledWith(
678+
'filterChecks() - ignoring ci check: markdown-lint'
679+
)
558680
})
559681

560682
test('runs prechecks and finds that the IssueOps command is valid for a branch deployment with ALL checks being required but the user has provided some checks to ignore', async () => {
@@ -626,8 +748,12 @@ test('runs prechecks and finds that the IssueOps command is valid for a branch d
626748
expect(debugMock).not.toHaveBeenCalledWith(
627749
'explicitly including ci check: test'
628750
)
629-
expect(debugMock).not.toHaveBeenCalledWith('ignoring ci check: build')
630-
expect(debugMock).not.toHaveBeenCalledWith('ignoring ci check: markdown-lint')
751+
expect(debugMock).not.toHaveBeenCalledWith(
752+
'filterChecks() - ignoring ci check: build'
753+
)
754+
expect(debugMock).not.toHaveBeenCalledWith(
755+
'filterChecks() - ignoring ci check: markdown-lint'
756+
)
631757
})
632758

633759
test('runs prechecks and finds that the IssueOps command is valid for a branch deployment with ALL checks being required but the user has provided some checks to ignore but none match', async () => {
@@ -696,8 +822,12 @@ test('runs prechecks and finds that the IssueOps command is valid for a branch d
696822
status: false
697823
})
698824

699-
expect(debugMock).not.toHaveBeenCalledWith('ignoring ci check: build')
700-
expect(debugMock).not.toHaveBeenCalledWith('ignoring ci check: markdown-lint')
825+
expect(debugMock).not.toHaveBeenCalledWith(
826+
'filterChecks() - ignoring ci check: build'
827+
)
828+
expect(debugMock).not.toHaveBeenCalledWith(
829+
'filterChecks() - ignoring ci check: markdown-lint'
830+
)
701831
})
702832

703833
test('runs prechecks and finds that the IssueOps command is valid for a branch deployment with ALL checks being required and the user did not provided checks to ignore and some are failing', async () => {
@@ -766,8 +896,12 @@ test('runs prechecks and finds that the IssueOps command is valid for a branch d
766896
status: false
767897
})
768898

769-
expect(debugMock).not.toHaveBeenCalledWith('ignoring ci check: build')
770-
expect(debugMock).not.toHaveBeenCalledWith('ignoring ci check: markdown-lint')
899+
expect(debugMock).not.toHaveBeenCalledWith(
900+
'filterChecks() - ignoring ci check: build'
901+
)
902+
expect(debugMock).not.toHaveBeenCalledWith(
903+
'filterChecks() - ignoring ci check: markdown-lint'
904+
)
771905
})
772906

773907
test('runs prechecks and finds that the IssueOps command is valid for a rollback deployment', async () => {

0 commit comments

Comments
 (0)