Skip to content
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

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');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this is changed?

Copy link
Contributor Author

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.

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', () => ({
Expand Down Expand Up @@ -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>;
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor

Choose a reason for hiding this comment

The 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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why number is casted to string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.mockReturnValue(12 as unknown as string);
.mockReturnValue('12');

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');
Expand All @@ -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',
},
],
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
expect(createCommentMock).toBeCalledWith('App', 12, message);
expect(createCommentMock).toBeCalledWith('App', '12', message);

With this change the test should pass correctly

});
});
Loading