-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[No QA][TS migration] Migrate postTestBuildComment to typescript #37027
Changes from 2 commits
7064cfe
0ead9f4
a5ea70b
dfad426
14f652c
5a33f68
c4a3b32
96dc9e0
3a9f784
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 | ||||
---|---|---|---|---|---|---|
@@ -1,18 +1,23 @@ | ||||||
import * as core from '@actions/core'; | ||||||
import {when} from 'jest-when'; | ||||||
import type {Writable} from 'type-fest'; | ||||||
import ghAction from '../../.github/actions/javascript/postTestBuildComment/postTestBuildComment'; | ||||||
import GithubUtils from '../../.github/libs/GithubUtils'; | ||||||
|
||||||
const mockGetInput = jest.fn(); | ||||||
const mockCreateComment = jest.fn(); | ||||||
const createCommentMock = jest.spyOn(GithubUtils, 'createComment'); | ||||||
const mockListComments = jest.fn(); | ||||||
const mockGraphql = jest.fn(); | ||||||
jest.spyOn(GithubUtils, 'octokit', 'get').mockReturnValue({ | ||||||
issues: { | ||||||
listComments: mockListComments, | ||||||
}, | ||||||
}); | ||||||
jest.spyOn(GithubUtils, 'paginate', 'get').mockReturnValue((endpoint, params) => endpoint(params).then(({data}) => data)); | ||||||
|
||||||
jest.spyOn(GithubUtils, 'paginate', 'get').mockReturnValue(<T, TData>(endpoint: (params: Record<string, T>) => Promise<{data: TData}>, params: Record<string, T>) => | ||||||
endpoint(params).then((response) => response.data), | ||||||
); | ||||||
|
||||||
jest.spyOn(GithubUtils, 'graphql', 'get').mockReturnValue(mockGraphql); | ||||||
|
||||||
jest.mock('@actions/github', () => ({ | ||||||
|
@@ -49,15 +54,18 @@ const message = `:test_tube::test_tube: Use the links below to test this adhoc b | |||||
:eyes: [View the workflow run that generated this build](https://github.com/Expensify/App/actions/runs/1234) :eyes: | ||||||
`; | ||||||
|
||||||
const asMutable = <T>(value: T): Writable<T> => value as Writable<T>; | ||||||
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. I see you are creating this function in second PR , maybe its worth to move it to some utils folder? 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. I agree, suggested the same in another PR |
||||||
|
||||||
describe('Post test build comments action tests', () => { | ||||||
beforeAll(() => { | ||||||
// Mock core module | ||||||
core.getInput = mockGetInput; | ||||||
GithubUtils.createComment = mockCreateComment; | ||||||
asMutable(core).getInput = mockGetInput; | ||||||
}); | ||||||
|
||||||
test('Test GH action', async () => { | ||||||
when(core.getInput).calledWith('PR_NUMBER', {required: true}).mockReturnValue(12); | ||||||
when(core.getInput) | ||||||
.calledWith('PR_NUMBER', {required: true}) | ||||||
.mockReturnValue(12 as unknown as string); | ||||||
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. why number is casted to string? 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. This is casted to string to comply with TS as the return type of code.getInput is a string and if we just change 12 to string the test is not passing, so this was the best solution I could find without changing the logic. 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.
Suggested change
See my next comment |
||||||
when(core.getInput).calledWith('ANDROID', {required: true}).mockReturnValue('success'); | ||||||
when(core.getInput).calledWith('IOS', {required: true}).mockReturnValue('success'); | ||||||
when(core.getInput).calledWith('WEB', {required: true}).mockReturnValue('success'); | ||||||
|
@@ -66,11 +74,12 @@ describe('Post test build comments action tests', () => { | |||||
when(core.getInput).calledWith('IOS_LINK').mockReturnValue('https://expensify.app/IOS_LINK'); | ||||||
when(core.getInput).calledWith('WEB_LINK').mockReturnValue('https://expensify.app/WEB_LINK'); | ||||||
when(core.getInput).calledWith('DESKTOP_LINK').mockReturnValue('https://expensify.app/DESKTOP_LINK'); | ||||||
GithubUtils.createComment.mockResolvedValue(true); | ||||||
createCommentMock.mockResolvedValue(true); | ||||||
mockListComments.mockResolvedValue({ | ||||||
data: [ | ||||||
{ | ||||||
body: ':test_tube::test_tube: Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing!', | ||||||
// eslint-disable-next-line @typescript-eslint/naming-convention | ||||||
node_id: 'IC_abcd', | ||||||
}, | ||||||
], | ||||||
|
@@ -86,7 +95,7 @@ describe('Post test build comments action tests', () => { | |||||
} | ||||||
} | ||||||
`); | ||||||
expect(GithubUtils.createComment).toBeCalledTimes(1); | ||||||
expect(GithubUtils.createComment).toBeCalledWith('App', 12, message); | ||||||
expect(createCommentMock).toBeCalledTimes(1); | ||||||
expect(createCommentMock).toBeCalledWith('App', 12, message); | ||||||
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.
Suggested change
With this change the test should pass correctly |
||||||
}); | ||||||
}); |
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.
why this is changed?
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.
This was changed because we couldn't assign a jest.mock into GithubUtils.createComment, this way we mock the createComment correctly and avoid TS errors.