-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[NoQA] [TS migration] Migrate checkDeployBlockersTest to Typescript #37035
Changes from 2 commits
6b08f6e
6c411a2
21c3904
78ca6c6
ee8bcd1
eae030d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
import type {Writable} from 'type-fest'; | ||
|
||
const asMutable = <T>(value: T): Writable<T> => value as Writable<T>; | ||
|
||
export default asMutable; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,12 +2,19 @@ | |||||
* @jest-environment node | ||||||
*/ | ||||||
import * as core from '@actions/core'; | ||||||
import _ from 'underscore'; | ||||||
import asMutable from '@src/types/utils/AsMutable'; | ||||||
import run from '../../.github/actions/javascript/checkDeployBlockers/checkDeployBlockers'; | ||||||
import GithubUtils from '../../.github/libs/GithubUtils'; | ||||||
|
||||||
type CommentData = {body: string}; | ||||||
type Comment = { | ||||||
data?: CommentData[]; | ||||||
}; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be good to have stylistic consistency and keep this all on the same line like we do for |
||||||
|
||||||
type PullRequest = {url: string; isQASuccess: boolean}; | ||||||
|
||||||
// Static mock function for core.getInput | ||||||
const mockGetInput = jest.fn().mockImplementation((arg) => { | ||||||
const mockGetInput = jest.fn().mockImplementation((arg: string): string | number | undefined => { | ||||||
if (arg === 'GITHUB_TOKEN') { | ||||||
return 'fake_token'; | ||||||
} | ||||||
|
@@ -23,8 +30,8 @@ const mockListComments = jest.fn(); | |||||
|
||||||
beforeAll(() => { | ||||||
// Mock core module | ||||||
core.getInput = mockGetInput; | ||||||
core.setOutput = mockSetOutput; | ||||||
asMutable(core).getInput = mockGetInput; | ||||||
asMutable(core).setOutput = mockSetOutput; | ||||||
|
||||||
// Mock octokit module | ||||||
const moctokit = { | ||||||
|
@@ -35,10 +42,12 @@ beforeAll(() => { | |||||
}, | ||||||
}, | ||||||
}; | ||||||
|
||||||
// @ts-expect-error TODO: Remove this once GithubUtils (https://github.com/Expensify/App/issues/25382) is migrated to TypeScript. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you please add a comment in that issue noting that this will need to be updated once it's been migrated? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is always a clean up issue at the end with the intention of going through all this TODOs and remove them. |
||||||
GithubUtils.internalOctokit = moctokit; | ||||||
}); | ||||||
|
||||||
let baseComments = []; | ||||||
let baseComments: Comment = {}; | ||||||
beforeEach(() => { | ||||||
baseComments = { | ||||||
data: [ | ||||||
|
@@ -65,11 +74,11 @@ afterAll(() => { | |||||
jest.clearAllMocks(); | ||||||
}); | ||||||
|
||||||
function checkbox(isClosed) { | ||||||
function checkbox(isClosed: boolean): string { | ||||||
return isClosed ? '[x]' : '[ ]'; | ||||||
} | ||||||
|
||||||
function mockIssue(prList, deployBlockerList) { | ||||||
function mockIssue(prList: PullRequest[], deployBlockerList?: PullRequest[]) { | ||||||
return { | ||||||
data: { | ||||||
number: 1, | ||||||
|
@@ -79,25 +88,27 @@ function mockIssue(prList, deployBlockerList) { | |||||
**Compare Changes:** https://github.com/Expensify/App/compare/production...staging | ||||||
|
||||||
**This release contains changes from the following pull requests:** | ||||||
${_.map( | ||||||
prList, | ||||||
({url, isQASuccess}) => ` | ||||||
${prList | ||||||
.map( | ||||||
({url, isQASuccess}) => ` | ||||||
Comment on lines
+91
to
+92
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NAB: These can be combined into a single line. Same for lines 106 and 107 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prettier is automatically moving this to a new line. Can't really change this. |
||||||
- ${checkbox(isQASuccess)} ${url} | ||||||
`, | ||||||
)} | ||||||
) | ||||||
.join('\n')} | ||||||
${ | ||||||
!_.isEmpty(deployBlockerList) | ||||||
!deployBlockerList || deployBlockerList.length < 0 | ||||||
? ` | ||||||
|
||||||
**Deploy Blockers:**` | ||||||
: '' | ||||||
} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NAB: Since you didn't introduce this code but there's some stray whitespace here
Suggested change
|
||||||
${_.map( | ||||||
deployBlockerList, | ||||||
({url, isQASuccess}) => ` | ||||||
${deployBlockerList | ||||||
?.map( | ||||||
({url, isQASuccess}) => ` | ||||||
- ${checkbox(isQASuccess)} ${url} | ||||||
`, | ||||||
)} | ||||||
) | ||||||
.join('\n')} | ||||||
cc @Expensify/applauseleads | ||||||
`, | ||||||
}, | ||||||
|
@@ -108,51 +119,46 @@ describe('checkDeployBlockers', () => { | |||||
const allClearIssue = mockIssue([{url: 'https://github.com/Expensify/App/pull/6882', isQASuccess: true}]); | ||||||
|
||||||
describe('checkDeployBlockers', () => { | ||||||
test('Test an issue with all checked items and :shipit:', () => { | ||||||
test('Test an issue with all checked items and :shipit:', async () => { | ||||||
mockGetIssue.mockResolvedValue(allClearIssue); | ||||||
mockListComments.mockResolvedValue(baseComments); | ||||||
return run().then(() => { | ||||||
expect(mockSetOutput).toHaveBeenCalledWith('HAS_DEPLOY_BLOCKERS', false); | ||||||
}); | ||||||
await expect(run()).resolves.toBeUndefined(); | ||||||
expect(mockSetOutput).toHaveBeenCalledWith('HAS_DEPLOY_BLOCKERS', false); | ||||||
}); | ||||||
|
||||||
test('Test an issue with all boxes checked but no :shipit:', () => { | ||||||
test('Test an issue with all boxes checked but no :shipit:', async () => { | ||||||
mockGetIssue.mockResolvedValue(allClearIssue); | ||||||
const extraComments = { | ||||||
data: [...baseComments.data, {body: 'This issue either has unchecked QA steps or has not yet been stamped with a :shipit: comment. Reopening!'}], | ||||||
data: [...(baseComments?.data ?? []), {body: 'This issue either has unchecked QA steps or has not yet been stamped with a :shipit: comment. Reopening!'}], | ||||||
}; | ||||||
mockListComments.mockResolvedValue(extraComments); | ||||||
return run().then(() => { | ||||||
expect(mockSetOutput).toHaveBeenCalledWith('HAS_DEPLOY_BLOCKERS', true); | ||||||
}); | ||||||
await expect(run()).resolves.toBeUndefined(); | ||||||
expect(mockSetOutput).toHaveBeenCalledWith('HAS_DEPLOY_BLOCKERS', true); | ||||||
}); | ||||||
|
||||||
test('Test an issue with all boxes checked but no comments', () => { | ||||||
test('Test an issue with all boxes checked but no comments', async () => { | ||||||
mockGetIssue.mockResolvedValue(allClearIssue); | ||||||
mockListComments.mockResolvedValue({data: []}); | ||||||
return run().then(() => { | ||||||
expect(mockSetOutput).toHaveBeenCalledWith('HAS_DEPLOY_BLOCKERS', true); | ||||||
}); | ||||||
await expect(run()).resolves.toBeUndefined(); | ||||||
expect(mockSetOutput).toHaveBeenCalledWith('HAS_DEPLOY_BLOCKERS', true); | ||||||
}); | ||||||
|
||||||
test('Test an issue with all QA checked but not all deploy blockers', () => { | ||||||
test('Test an issue with all QA checked but not all deploy blockers', async () => { | ||||||
mockGetIssue.mockResolvedValue( | ||||||
mockIssue([{url: 'https://github.com/Expensify/App/pull/6882', isQASuccess: true}], [{url: 'https://github.com/Expensify/App/pull/6883', isQASuccess: false}]), | ||||||
); | ||||||
mockListComments.mockResolvedValue(baseComments); | ||||||
return run().then(() => { | ||||||
expect(mockSetOutput).toHaveBeenCalledWith('HAS_DEPLOY_BLOCKERS', true); | ||||||
}); | ||||||
await expect(run()).resolves.toBeUndefined(); | ||||||
expect(mockSetOutput).toHaveBeenCalledWith('HAS_DEPLOY_BLOCKERS', true); | ||||||
}); | ||||||
|
||||||
test('Test an issue with all QA checked and all deploy blockers resolved', () => { | ||||||
test('Test an issue with all QA checked and all deploy blockers resolved', async () => { | ||||||
mockGetIssue.mockResolvedValue( | ||||||
mockIssue([{url: 'https://github.com/Expensify/App/pull/6882', isQASuccess: true}], [{url: 'https://github.com/Expensify/App/pull/6883', isQASuccess: true}]), | ||||||
); | ||||||
mockListComments.mockResolvedValue(baseComments); | ||||||
return run().then(() => { | ||||||
expect(mockSetOutput).toHaveBeenCalledWith('HAS_DEPLOY_BLOCKERS', false); | ||||||
}); | ||||||
await expect(run()).resolves.toBeUndefined(); | ||||||
expect(mockSetOutput).toHaveBeenCalledWith('HAS_DEPLOY_BLOCKERS', false); | ||||||
}); | ||||||
}); | ||||||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, but could we rename it to
asMutable.ts
?