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.

Sign up for GitHub

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

[TS migration] Migrate GithubUtilsTest to typescript #36427

Merged
merged 8 commits into from
Feb 26, 2024
Merged

[TS migration] Migrate GithubUtilsTest to typescript #36427

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a9c81c3
[TS migration] Migrate GithubUtilsTest to typescript
ruben-rebelo Feb 13, 2024
eb8a2cd
[TS migration] Code improvements
ruben-rebelo Feb 13, 2024
ed6b6b2
[TS migration][GithubUtilsTest] Addressed feedback
ruben-rebelo Feb 14, 2024
5ce94eb
Merge branch 'main' into ts-migration/githubutilstest
ruben-rebelo Feb 20, 2024
c37bbba
[TS migration][GithubUtilsTest] Added missing TODO
ruben-rebelo Feb 20, 2024
a6cf95a
[TS migration][GithubUtilsTest] Feedback
ruben-rebelo Feb 20, 2024
4535da7
[TS migration][GIthubutilstest] Updated with typefest
ruben-rebelo Feb 20, 2024
960ebb7
[TS migration][GithubUtilsTest] Disabled name convention rule for the…
ruben-rebelo Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 118 additions & 36 deletions tests/unit/GithubUtilsTest.js → tests/unit/GithubUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,69 @@
/**
* @jest-environment node
*/
const core = require('@actions/core');
const GithubUtils = require('../../.github/libs/GithubUtils');
import * as core from '@actions/core';
import GithubUtils from '../../.github/libs/GithubUtils';

const mockGetInput = jest.fn();
const mockListIssues = jest.fn();

type DeployBlockers = {
url: string;
number: number;
isResolved: boolean;
};

type PR = {
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
type PR = {
type PullRequest = {

url: string;
number: number;
isVerified: boolean;
};

type Label = {
id: number;
number?: number;
isVerified?: boolean;
// eslint-disable-next-line @typescript-eslint/naming-convention
node_id: string;
url: string;
name: string;
color: string;
default: boolean;
description: string;
};

type ExpectedReponse = {
PRList: PR[];
labels: Label[];
tag: string;
title: string;
url: string;
number: number;
deployBlockers: DeployBlockers[];
internalQAPRList: string[];
isTimingDashboardChecked: boolean;
isFirebaseChecked: boolean;
isGHStatusChecked: boolean;
};

type Issue = {
url: string;
title: string;
labels: Label[];
body: string;
};

type ObjectMethodData<T> = {
data: T;
};

type Mutable<T> = {-readonly [P in keyof T]: T[P]};
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you can reuse Writable type form type-fest lib


const asMutable = <T>(value: T): Mutable<T> => value as Mutable<T>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice usage ❤️


beforeAll(() => {
// Mock core module
core.getInput = mockGetInput;
asMutable(core).getInput = mockGetInput;

// Mock octokit module
const moctokit = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is possible to use a type from GitHub library here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find a way to make the mock complaint with the GitHub lib typing.
There was too a lot of missing fields which we do not use on the unit test that needed to be populated and couldn't find a solution for all of them, the best way I think is to have GithubUtils correctly typed so this one would automatically get the correct types.

Expand All @@ -19,16 +73,18 @@
Promise.resolve({
data: {
...arg,
// eslint-disable-next-line @typescript-eslint/naming-convention
html_url: 'https://github.com/Expensify/App/issues/29',
},
}),
),
listForRepo: mockListIssues,
},
},
paginate: jest.fn().mockImplementation((objectMethod) => objectMethod().then(({data}) => data)),
paginate: jest.fn().mockImplementation(<T>(objectMethod: () => Promise<ObjectMethodData<T>>) => objectMethod().then(({data}) => data)),
};

GithubUtils.internalOctokit = moctokit;

Check failure on line 87 in tests/unit/GithubUtilsTest.ts

View workflow job for this annotation

GitHub Actions / typecheck 

Type '{ rest: { issues: { create: jest.Mock<any, any, any>; listForRepo: jest.Mock<any, any, any>; }; }; paginate: jest.Mock<any, any, any>; }' is not assignable to type 'Octokit & Api & { paginate: PaginateInterface; } & { paginate: PaginateInterface; }'.
});

afterEach(() => {
Expand All @@ -38,12 +94,13 @@

describe('GithubUtils', () => {
describe('getStagingDeployCash', () => {
const baseIssue = {
const baseIssue: Issue = {
url: 'https://api.github.com/repos/Andrew-Test-Org/Public-Test-Repo/issues/29',
title: 'Andrew Test Issue',
labels: [
{
id: 2783847782,
// eslint-disable-next-line @typescript-eslint/naming-convention
node_id: 'MDU6TGFiZWwyNzgzODQ3Nzgy',
url: 'https://api.github.com/repos/Andrew-Test-Org/Public-Test-Repo/labels/StagingDeployCash',
name: 'StagingDeployCash',
Expand All @@ -60,7 +117,7 @@
issueWithDeployBlockers.body +=
'\r\n**Deploy Blockers:**\r\n- [ ] https://github.com/Expensify/App/issues/1\r\n- [x] https://github.com/Expensify/App/issues/2\r\n- [ ] https://github.com/Expensify/App/pull/1234\r\n';

const baseExpectedResponse = {
const baseExpectedResponse: ExpectedReponse = {
PRList: [
{
url: 'https://github.com/Expensify/App/pull/21',
Expand All @@ -85,6 +142,7 @@
description: '',
id: 2783847782,
name: 'StagingDeployCash',
// eslint-disable-next-line @typescript-eslint/naming-convention
node_id: 'MDU6TGFiZWwyNzgzODQ3Nzgy',
url: 'https://api.github.com/repos/Andrew-Test-Org/Public-Test-Repo/labels/StagingDeployCash',
},
Expand Down Expand Up @@ -119,13 +177,13 @@
];

test('Test finding an open issue with no PRs successfully', () => {
const bareIssue = {
const bareIssue: Issue = {
...baseIssue,
// eslint-disable-next-line max-len
body: '**Release Version:** `1.0.1-47`\r\n**Compare Changes:** https://github.com/Expensify/App/compare/production...staging\r\n\r\ncc @Expensify/applauseleads\n',
};

const bareExpectedResponse = {
const bareExpectedResponse: ExpectedReponse = {
...baseExpectedResponse,
PRList: [],
};
Expand Down Expand Up @@ -193,7 +251,9 @@
['https://github.com/Expensify/Expensify/issues/156481'],
['https://docs.google.com/document/d/1mMFh-m1seOES48r3zNqcvfuTvr3qOAsY6n5rP4ejdXE/edit?ts=602420d2#'],
])('getPullRequestNumberFromURL("%s")', (input) => {
expect(() => GithubUtils.getPullRequestNumberFromURL(input)).toThrow(new Error(`Provided URL ${input} is not a Github Pull Request!`));
expect(() => {
GithubUtils.getPullRequestNumberFromURL(input);
}).toThrow(new Error(`Provided URL ${input} is not a Github Pull Request!`));
});
});
});
Expand All @@ -218,7 +278,9 @@
['https://github.com/Expensify/Expensify/pull/156481'],
['https://docs.google.com/document/d/1mMFh-m1seOES48r3zNqcvfuTvr3qOAsY6n5rP4ejdXE/edit?ts=602420d2#'],
])('getIssueNumberFromURL("%s")', (input) => {
expect(() => GithubUtils.getIssueNumberFromURL(input)).toThrow(new Error(`Provided URL ${input} is not a Github Issue!`));
expect(() => {
GithubUtils.getIssueNumberFromURL(input);
}).toThrow(new Error(`Provided URL ${input} is not a Github Issue!`));
});
});
});
Expand Down Expand Up @@ -247,7 +309,9 @@
test.each([['https://www.google.com/'], ['https://docs.google.com/document/d/1mMFh-m1seOES48r3zNqcvfuTvr3qOAsY6n5rP4ejdXE/edit?ts=602420d2#']])(
'getIssueOrPullRequestNumberFromURL("%s")',
(input) => {
expect(() => GithubUtils.getIssueOrPullRequestNumberFromURL(input)).toThrow(new Error(`Provided URL ${input} is not a valid Github Issue or Pull Request!`));
expect(() => {
GithubUtils.getIssueOrPullRequestNumberFromURL(input);
}).toThrow(new Error(`Provided URL ${input} is not a valid Github Issue or Pull Request!`));
},
);
});
Expand All @@ -259,46 +323,53 @@
{
number: 1,
title: 'Test PR 1',
// eslint-disable-next-line @typescript-eslint/naming-convention
html_url: 'https://github.com/Expensify/App/pull/1',
user: {login: 'testUser'},
labels: [],
},
{
number: 2,
title: 'Test PR 2',
// eslint-disable-next-line @typescript-eslint/naming-convention
html_url: 'https://github.com/Expensify/App/pull/2',
user: {login: 'testUser'},
labels: [],
},
{
number: 3,
title: 'Test PR 3',
// eslint-disable-next-line @typescript-eslint/naming-convention
Copy link
Contributor

Choose a reason for hiding this comment

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

How about disabling it in whole file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree

html_url: 'https://github.com/Expensify/App/pull/3',
user: {login: 'testUser'},
labels: [],
},
{
number: 4,
title: '[NO QA] Test No QA PR uppercase',
// eslint-disable-next-line @typescript-eslint/naming-convention
html_url: 'https://github.com/Expensify/App/pull/4',
user: {login: 'testUser'},
labels: [],
},
{
number: 5,
title: '[NoQa] Test No QA PR Title Case',
// eslint-disable-next-line @typescript-eslint/naming-convention
html_url: 'https://github.com/Expensify/App/pull/5',
user: {login: 'testUser'},
labels: [],
},
{
number: 6,
title: '[Internal QA] Test Internal QA PR',
// eslint-disable-next-line @typescript-eslint/naming-convention
html_url: 'https://github.com/Expensify/App/pull/6',
user: {login: 'testUser'},
labels: [
{
id: 1234,
// eslint-disable-next-line @typescript-eslint/naming-convention
node_id: 'MDU6TGFiZWwyMDgwNDU5NDY=',
url: 'https://api.github.com/Expensify/App/labels/InternalQA',
name: 'InternalQA',
Expand All @@ -318,11 +389,13 @@
{
number: 7,
title: '[Internal QA] Another Test Internal QA PR',
// eslint-disable-next-line @typescript-eslint/naming-convention
html_url: 'https://github.com/Expensify/App/pull/7',
user: {login: 'testUser'},
labels: [
{
id: 1234,
// eslint-disable-next-line @typescript-eslint/naming-convention
node_id: 'MDU6TGFiZWwyMDgwNDU5NDY=',
url: 'https://api.github.com/Expensify/App/labels/InternalQA',
name: 'InternalQA',
Expand Down Expand Up @@ -350,12 +423,13 @@
list: jest.fn().mockResolvedValue({data: mockPRs}),
},
},
paginate: jest.fn().mockImplementation((objectMethod) => objectMethod().then(({data}) => data)),
paginate: jest.fn().mockImplementation(<T>(objectMethod: () => Promise<ObjectMethodData<T>>) => objectMethod().then(({data}) => data)),
}),
}));

const octokit = mockGithub().getOctokit();
const githubUtils = class extends GithubUtils {};
// @ts-expect-error TODO: Remove this once GithubUtils (https://github.com/Expensify/App/issues/25382) is migrated to TypeScript.
githubUtils.internalOctokit = octokit;
const tag = '1.0.2-12';
const basePRList = [
Expand Down Expand Up @@ -404,8 +478,8 @@
`${lineBreak}${closedCheckbox}${basePRList[5]}` +
`${lineBreak}`;

test('Test no verified PRs', () =>
githubUtils.generateStagingDeployCashBody(tag, basePRList).then((issueBody) => {
test('Test no verified PRs', () => {
githubUtils.generateStagingDeployCashBody(tag, basePRList).then((issueBody: string) => {
expect(issueBody).toBe(
`${baseExpectedOutput}` +
`${openCheckbox}${basePRList[2]}` +
Expand All @@ -419,10 +493,11 @@
`${lineBreak}${openCheckbox}${ghVerification}` +
`${lineBreakDouble}${ccApplauseLeads}`,
);
}));
});
});

test('Test some verified PRs', () =>
githubUtils.generateStagingDeployCashBody(tag, basePRList, [basePRList[0]]).then((issueBody) => {
test('Test some verified PRs', () => {
githubUtils.generateStagingDeployCashBody(tag, basePRList, [basePRList[0]]).then((issueBody: string) => {
expect(issueBody).toBe(
`${baseExpectedOutput}` +
`${openCheckbox}${basePRList[2]}` +
Expand All @@ -436,10 +511,11 @@
`${lineBreak}${openCheckbox}${ghVerification}` +
`${lineBreakDouble}${ccApplauseLeads}`,
);
}));
});
});

test('Test all verified PRs', () =>
githubUtils.generateStagingDeployCashBody(tag, basePRList, basePRList).then((issueBody) => {
test('Test all verified PRs', () => {
githubUtils.generateStagingDeployCashBody(tag, basePRList, basePRList).then((issueBody: string) => {
expect(issueBody).toBe(
`${allVerifiedExpectedOutput}` +
`${lineBreak}${deployerVerificationsHeader}` +
Expand All @@ -448,10 +524,11 @@
`${lineBreak}${openCheckbox}${ghVerification}` +
`${lineBreakDouble}${ccApplauseLeads}`,
);
}));
});
});

test('Test no resolved deploy blockers', () =>
githubUtils.generateStagingDeployCashBody(tag, basePRList, basePRList, baseDeployBlockerList).then((issueBody) => {
test('Test no resolved deploy blockers', () => {
githubUtils.generateStagingDeployCashBody(tag, basePRList, basePRList, baseDeployBlockerList).then((issueBody: string) => {
expect(issueBody).toBe(
`${allVerifiedExpectedOutput}` +
`${lineBreak}${deployBlockerHeader}` +
Expand All @@ -463,10 +540,11 @@
`${lineBreak}${openCheckbox}${ghVerification}${lineBreak}` +
`${lineBreak}${ccApplauseLeads}`,
);
}));
});
});

test('Test some resolved deploy blockers', () =>
githubUtils.generateStagingDeployCashBody(tag, basePRList, basePRList, baseDeployBlockerList, [baseDeployBlockerList[0]]).then((issueBody) => {
test('Test some resolved deploy blockers', () => {
githubUtils.generateStagingDeployCashBody(tag, basePRList, basePRList, baseDeployBlockerList, [baseDeployBlockerList[0]]).then((issueBody: string) => {
expect(issueBody).toBe(
`${allVerifiedExpectedOutput}` +
`${lineBreak}${deployBlockerHeader}` +
Expand All @@ -478,10 +556,11 @@
`${lineBreak}${openCheckbox}${ghVerification}` +
`${lineBreakDouble}${ccApplauseLeads}`,
);
}));
});
});

test('Test all resolved deploy blockers', () =>
githubUtils.generateStagingDeployCashBody(tag, basePRList, basePRList, baseDeployBlockerList, baseDeployBlockerList).then((issueBody) => {
test('Test all resolved deploy blockers', () => {
githubUtils.generateStagingDeployCashBody(tag, basePRList, basePRList, baseDeployBlockerList, baseDeployBlockerList).then((issueBody: string) => {
expect(issueBody).toBe(
`${baseExpectedOutput}` +
`${closedCheckbox}${basePRList[2]}` +
Expand All @@ -498,10 +577,11 @@
`${lineBreak}${openCheckbox}${ghVerification}` +
`${lineBreakDouble}${ccApplauseLeads}`,
);
}));
});
});

test('Test internalQA PRs', () =>
githubUtils.generateStagingDeployCashBody(tag, [...basePRList, ...internalQAPRList]).then((issueBody) => {
test('Test internalQA PRs', () => {
githubUtils.generateStagingDeployCashBody(tag, [...basePRList, ...internalQAPRList]).then((issueBody: string) => {
expect(issueBody).toBe(
`${baseExpectedOutput}` +
`${openCheckbox}${basePRList[2]}` +
Expand All @@ -518,10 +598,11 @@
`${lineBreak}${openCheckbox}${ghVerification}` +
`${lineBreakDouble}${ccApplauseLeads}`,
);
}));
});
});

test('Test some verified internalQA PRs', () =>
githubUtils.generateStagingDeployCashBody(tag, [...basePRList, ...internalQAPRList], [], [], [], [internalQAPRList[0]]).then((issueBody) => {
test('Test some verified internalQA PRs', () => {
githubUtils.generateStagingDeployCashBody(tag, [...basePRList, ...internalQAPRList], [], [], [], [internalQAPRList[0]]).then((issueBody: string) => {
expect(issueBody).toBe(
`${baseExpectedOutput}` +
`${openCheckbox}${basePRList[2]}` +
Expand All @@ -538,7 +619,8 @@
`${lineBreak}${openCheckbox}${ghVerification}` +
`${lineBreakDouble}${ccApplauseLeads}`,
);
}));
});
});
});

describe('getPullRequestURLFromNumber', () => {
Expand Down
Loading