Skip to content

Commit 147e619

Browse files
committed
ensure that prechecks() actually fails if an exact check input is provided (for a CI check) and that check does not exist in the graphql response for passed checks on the pull request
1 parent 3d96a19 commit 147e619

File tree

4 files changed

+184
-23
lines changed

4 files changed

+184
-23
lines changed

__tests__/functions/prechecks.test.js

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,95 @@ test('runs prechecks and finds that the IssueOps command is valid for a branch d
397397
)
398398
})
399399

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+
'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'
486+
)
487+
})
488+
400489
test('runs prechecks and finds that the IssueOps command is valid for a branch deployment but checks and ignore checks cancel eachother out', async () => {
401490
octokit.graphql = jest.fn().mockReturnValue({
402491
repository: {

dist/index.js

Lines changed: 47 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/functions/prechecks.js

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,7 @@ export async function prechecks(context, octokit, data) {
254254

255255
// Grab the statusCheckRollup state from the GraphQL result
256256
var commitStatus
257+
var filterChecksResults
257258
try {
258259
// Check to see if skipCi is set for the environment being used
259260
if (skipCi) {
@@ -272,13 +273,14 @@ export async function prechecks(context, octokit, data) {
272273

273274
// If only the required checks need to pass
274275
} else if (checks === 'required') {
275-
commitStatus = filterChecks(
276+
filterChecksResults = filterChecks(
276277
checks,
277278
result.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup
278279
.contexts.nodes,
279280
ignoredChecks,
280281
true
281282
)
283+
commitStatus = filterChecksResults.status
282284

283285
// If there are CI checked defined, we need to check for the 'state' of the latest commit
284286
} else if (checks === 'all') {
@@ -290,25 +292,27 @@ export async function prechecks(context, octokit, data) {
290292

291293
// if there are ignored checks, we need to filter out the ignored checks from the graphql result
292294
} else {
293-
commitStatus = filterChecks(
295+
filterChecksResults = filterChecks(
294296
checks,
295297
result.repository.pullRequest.commits.nodes[0].commit
296298
.statusCheckRollup.contexts.nodes,
297299
ignoredChecks,
298300
false
299301
)
302+
commitStatus = filterChecksResults.status
300303
}
301304

302305
// if we make it here, checks is not a string (e.g. 'all' or 'required') but it is actually an array of the exact checks...
303306
// that a user wants to pass in order for the deployment to proceed
304307
} else {
305-
commitStatus = filterChecks(
308+
filterChecksResults = filterChecks(
306309
checks,
307310
result.repository.pullRequest.commits.nodes[0].commit.statusCheckRollup
308311
.contexts.nodes,
309312
ignoredChecks,
310313
false
311314
)
315+
commitStatus = filterChecksResults.status
312316
}
313317
} catch (e) {
314318
core.debug(
@@ -653,6 +657,12 @@ export async function prechecks(context, octokit, data) {
653657
message = `### ⚠️ Cannot proceed with deployment\n\n- reviewDecision: \`${reviewDecision}\`\n- commitStatus: \`${commitStatus}\`\n\n> CI checks must be passing in order to continue`
654658
return {message: message, status: false}
655659

660+
// Regardless of the reviewDecision or noop, if the commitStatus is 'MISSING' this means that a user has explicitly requested a CI check to be passing with the `checks: <check1>,<check2>,<etc>` input option, but the check could not be found in the GraphQL result
661+
// In this case, we should alert the user that the check could not be found and exit
662+
} else if (commitStatus === 'MISSING') {
663+
message = `### ⚠️ Cannot proceed with deployment\n\n- reviewDecision: \`${reviewDecision}\`\n- commitStatus: \`${commitStatus}\`\n\n> CI checks must be passing in order to continue`
664+
return {message: filterChecksResults.message, status: false}
665+
656666
// If CI is passing but the PR is missing an approval, let the user know
657667
} else if (
658668
(reviewDecision === 'REVIEW_REQUIRED' ||
@@ -741,19 +751,41 @@ export async function prechecks(context, octokit, data) {
741751
// :param checkResults: An array of check results (objects) from the graphql query
742752
// :param ignoredChecks: An array of check names to ignore
743753
// :param required: A boolean to determine if a check being a required check should be considered
744-
// :returns: A string representing the combined status of the remaining checks (e.g. 'SUCCESS' or 'FAILURE')
754+
// :returns: An object containing a message (if a failure occurs), and a string representing the status of the checks
755+
// example: {message: '...', status: 'SUCCESS'}
756+
// The status will be one of the following: 'SUCCESS', 'FAILURE', 'MISSING'
745757
function filterChecks(checks, checkResults, ignoredChecks, required) {
746758
const healthyCheckStatuses = ['SUCCESS', 'SKIPPED', 'NEUTRAL']
747759

748760
core.debug(`filterChecks() - checks: ${checks}`)
749761
core.debug(`filterChecks() - ignoredChecks: ${ignoredChecks}`)
750762
core.debug(`filterChecks() - required: ${required}`)
751763

764+
// If checks is an array (meaning it isn't just `required` or `all`) and it contains items
765+
const checksProvided = Array.isArray(checks) && checks.length > 0
766+
767+
// If a set of values is provided for the `checks` input option, ensure all of them exist in checkResults
768+
// Example: if `checks` is set to `['test', 'lint', 'build']`, ensure that all of those checks exist in checkResults
769+
if (checksProvided) {
770+
const missingChecks = checks.filter(
771+
ch => !checkResults.some(cr => cr.name === ch)
772+
)
773+
if (missingChecks.length > 0) {
774+
core.warning(
775+
`the ${COLORS.info}checks${COLORS.reset} input option requires that all of the following checks are passing: ${COLORS.highlight}${checks.join(', ')}${COLORS.reset} - however, the following checks are missing: ${COLORS.highlight}${missingChecks.join(', ')}${COLORS.reset}`
776+
)
777+
778+
return {
779+
message: `The \`checks\` input option requires that all of the following checks are passing: \`${checks.join(',')}\`. However, the following checks are missing: \`${missingChecks.join(',')}\``,
780+
status: 'MISSING'
781+
}
782+
}
783+
}
784+
752785
// Filter the checkResults based on user input (checks), ignoring checks, and required flag
753786
const filteredChecks = checkResults
754787
.filter(check => {
755-
// If checks is an array (meaning it isn't just `required` or `all`) and it contains items, filter checks based on it
756-
if (Array.isArray(checks) && checks.length > 0) {
788+
if (checksProvided) {
757789
// check if the `checks` input option explicitly includes the name of the check that was found
758790
const isIncluded = checks.includes(check.name)
759791

@@ -793,12 +825,16 @@ function filterChecks(checks, checkResults, ignoredChecks, required) {
793825

794826
// If no checks remain after filtering, default to SUCCESS
795827
if (filteredChecks.length === 0) {
796-
core.debug(
828+
const message =
797829
'filterChecks() - after filtering, no checks remain - this will result in a SUCCESS state as it is treated as if no checks are defined'
798-
)
799-
return 'SUCCESS'
830+
core.debug(message)
831+
return {message: message, status: 'SUCCESS'}
800832
}
801833

802-
// Return SUCCESS if all checks are healthy, otherwise FAILURE
803-
return allHealthy ? 'SUCCESS' : 'FAILURE'
834+
return {
835+
message: allHealthy
836+
? 'all checks passed'
837+
: 'one or more checks did not pass',
838+
status: allHealthy ? 'SUCCESS' : 'FAILURE'
839+
}
804840
}

0 commit comments

Comments
 (0)