Skip to content

Commit 1c8e9e5

Browse files
authored
Merge pull request #380 from github/committer-fixes
bug: handle edge cases where the `committer` object is missing a login attribute
2 parents ae6decb + 25104a6 commit 1c8e9e5

File tree

5 files changed

+98
-5
lines changed

5 files changed

+98
-5
lines changed

Diff for: __tests__/main.test.js

+81
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,87 @@ test('successfully runs the action with deployment confirmation', async () => {
288288
)
289289
})
290290

291+
test('successfully runs the action with deployment confirmation and when the committer is not set', async () => {
292+
process.env.INPUT_DEPLOYMENT_CONFIRMATION = 'true'
293+
294+
jest
295+
.spyOn(deploymentConfirmation, 'deploymentConfirmation')
296+
.mockImplementation(() => {
297+
return true
298+
})
299+
300+
jest.spyOn(github, 'getOctokit').mockImplementation(() => {
301+
return {
302+
rest: {
303+
issues: {
304+
createComment: jest.fn().mockReturnValueOnce({
305+
data: {id: 123456}
306+
})
307+
},
308+
repos: {
309+
createDeployment: createDeploymentMock,
310+
createDeploymentStatus: jest.fn().mockImplementation(() => {
311+
return {data: {}}
312+
}),
313+
getCommit: jest.fn().mockImplementation(() => {
314+
return {
315+
data: {
316+
sha: mock_sha,
317+
html_url: `https://github.com/corp/test/commit/${mock_sha}`,
318+
commit: {
319+
author: {
320+
date: '2024-10-15T12:00:00Z'
321+
},
322+
verification: no_verification
323+
},
324+
committer: {}
325+
}
326+
}
327+
})
328+
},
329+
pulls: {
330+
get: jest.fn().mockImplementation(() => {
331+
return {data: {head: {ref: 'test-ref'}}, status: 200}
332+
})
333+
}
334+
}
335+
}
336+
})
337+
338+
expect(await run()).toBe('success')
339+
expect(setOutputMock).toHaveBeenCalledWith('deployment_id', 123)
340+
expect(setOutputMock).toHaveBeenCalledWith('comment_body', '.deploy')
341+
expect(setOutputMock).toHaveBeenCalledWith('triggered', 'true')
342+
expect(setOutputMock).toHaveBeenCalledWith('comment_id', 123)
343+
expect(setOutputMock).toHaveBeenCalledWith('ref', 'test-ref')
344+
expect(setOutputMock).toHaveBeenCalledWith('noop', false)
345+
expect(setOutputMock).toHaveBeenCalledWith('continue', 'true')
346+
expect(saveStateMock).toHaveBeenCalledWith('isPost', 'true')
347+
expect(saveStateMock).toHaveBeenCalledWith('actionsToken', 'faketoken')
348+
expect(saveStateMock).toHaveBeenCalledWith('environment', 'production')
349+
expect(saveStateMock).toHaveBeenCalledWith('comment_id', 123)
350+
expect(saveStateMock).toHaveBeenCalledWith('ref', 'test-ref')
351+
expect(saveStateMock).toHaveBeenCalledWith('noop', false)
352+
expect(setOutputMock).toHaveBeenCalledWith('type', 'deploy')
353+
expect(saveStateMock).toHaveBeenCalledWith('deployment_id', 123)
354+
expect(saveStateMock).toHaveBeenCalledWith('sha', 'abc123')
355+
expect(debugMock).toHaveBeenCalledWith('production_environment: true')
356+
expect(debugMock).toHaveBeenCalledWith(
357+
'deploymentConfirmation() was successful - continuing with the deployment'
358+
)
359+
expect(warningMock).toHaveBeenCalledWith(
360+
'⚠️ could not find the login of the committer - https://github.com/github/branch-deploy/issues/379'
361+
)
362+
expect(saveStateMock).not.toHaveBeenCalledWith('environment_url', String)
363+
expect(setOutputMock).not.toHaveBeenCalledWith('environment_url', String)
364+
expect(infoMock).toHaveBeenCalledWith(
365+
`🧑‍🚀 commit sha to deploy: ${COLORS.highlight}${mock_sha}${COLORS.reset}`
366+
)
367+
expect(infoMock).toHaveBeenCalledWith(
368+
`🚀 ${COLORS.success}deployment started!${COLORS.reset}`
369+
)
370+
})
371+
291372
test('rejects the deployment when deployment confirmation is set, but does not succeed', async () => {
292373
process.env.INPUT_DEPLOYMENT_CONFIRMATION = 'true'
293374

Diff for: dist/index.js

+8-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: dist/index.js.map

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: src/main.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -466,9 +466,15 @@ export async function run() {
466466
headers: API_HEADERS
467467
})
468468

469-
const committer = commitData.data.committer.login
469+
const committer = commitData.data?.committer?.login
470470
const commit_html_url = commitData.data.html_url
471471

472+
if (committer === null || committer === undefined) {
473+
core.warning(
474+
'⚠️ could not find the login of the committer - https://github.com/github/branch-deploy/issues/379'
475+
)
476+
}
477+
472478
// Run commit safety checks
473479
const commitSafetyCheckResults = await commitSafetyChecks(context, {
474480
commit: commitData.data.commit,

Diff for: src/version.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
// - v1.1.1-rc.1
77
// - etc
88

9-
export const VERSION = 'v10.4.0'
9+
export const VERSION = 'v10.4.1'

0 commit comments

Comments
 (0)