From 4aa796454f70da3403c341e1840958916edace2b Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Tue, 5 Sep 2023 12:26:09 -0300 Subject: [PATCH] Created integration tests (#67) This closes #52 --- .eslintignore | 1 + .gitignore | 3 + src/runner.ts | 4 +- src/test/config.yml | 86 ++++++++++++++ src/test/index.test.ts | 253 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 343 insertions(+), 4 deletions(-) create mode 100644 src/test/config.yml diff --git a/.eslintignore b/.eslintignore index a21f178..010ce4d 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1,3 +1,4 @@ node_modules dist .git +src/test/**/*.yml diff --git a/.gitignore b/.gitignore index 99888df..f758aee 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,6 @@ !.vscode/settings.json .idea .DS_Store + +# Testing +summary-test.html diff --git a/src/runner.ts b/src/runner.ts index 800bd52..241a1c1 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -21,7 +21,7 @@ type ReviewReport = { usersToRequest?: string[]; }; -type RuleReport = { name: string } & ReviewReport; +export type RuleReport = { name: string } & ReviewReport; type ReviewState = [true] | [false, ReviewReport]; @@ -461,7 +461,7 @@ export class ActionRunner { * 3. It generates a status check in the Pull Request * 4. WIP - It assigns the required reviewers to review the PR */ - async runAction(inputs: Omit): Promise & PullRequestReport> { + async runAction(inputs: Pick): Promise & PullRequestReport> { const config = await this.getConfigFile(inputs.configLocation); const prValidation = await this.validatePullRequest(config); diff --git a/src/test/config.yml b/src/test/config.yml new file mode 100644 index 0000000..54e0328 --- /dev/null +++ b/src/test/config.yml @@ -0,0 +1,86 @@ +rules: + - name: CI files + condition: + include: + - ^\.gitlab-ci\.yml + - ^docker/.* + - ^\.github/.* + - ^\.gitlab/.* + - ^\.config/nextest.toml + - ^\.cargo/.* + exclude: + - ^./gitlab/pipeline/zombienet.yml$ + min_approvals: 2 + type: basic + teams: + - ci + - release-engineering + + - name: Core developers + countAuthor: true + condition: + include: + - .* + # excluding files from 'Runtime files' and 'CI files' rules + exclude: + - ^polkadot/runtime/(kusama|polkadot)/src/[^/]+\.rs$ + - ^cumulus/parachains/runtimes/assets/(asset-hub-kusama|asset-hub-polkadot)/src/[^/]+\.rs$ + - ^cumulus/parachains/runtimes/bridge-hubs/(bridge-hub-kusama|bridge-hub-polkadot)/src/[^/]+\.rs$ + - ^cumulus/parachains/runtimes/collectives/collectives-polkadot/src/[^/]+\.rs$ + - ^cumulus/parachains/common/src/[^/]+\.rs$ + - ^substrate/frame/(?!.*(nfts/.*|uniques/.*|babe/.*|grandpa/.*|beefy|merkle-mountain-range/.*|contracts/.*|election|nomination-pools/.*|staking/.*|aura/.*)) + - ^polkadot/runtime/(kusama|polkadot)/src/[^/]+\.rs$ + - ^\.gitlab-ci\.yml + - ^(?!.*\.dic$|.*spellcheck\.toml$)scripts/ci/.* + - ^\.github/.* + min_approvals: 2 + type: basic + teams: + - core-devs + + # cumulus + - name: Runtime files cumulus + countAuthor: true + condition: + include: + - ^cumulus/parachains/runtimes/assets/(asset-hub-kusama|asset-hub-polkadot)/src/[^/]+\.rs$ + - ^cumulus/parachains/runtimes/bridge-hubs/(bridge-hub-kusama|bridge-hub-polkadot)/src/[^/]+\.rs$ + - ^cumulus/parachains/runtimes/collectives/collectives-polkadot/src/[^/]+\.rs$ + - ^cumulus/parachains/common/src/[^/]+\.rs$ + type: and-distinct + reviewers: + - min_approvals: 1 + teams: + - locks-review + - min_approvals: 1 + teams: + - polkadot-review + + # if there are any changes in the bridges subtree (in case of backport changes back to bridges repo) + - name: Bridges subtree files + type: basic + condition: + include: + - ^cumulus/bridges/.* + min_approvals: 1 + teams: + - bridges-core + + # substrate + + - name: FRAME coders substrate + condition: + include: + - ^substrate/frame/(?!.*(nfts/.*|uniques/.*|babe/.*|grandpa/.*|beefy|merkle-mountain-range/.*|contracts/.*|election|nomination-pools/.*|staking/.*|aura/.*)) + type: "and" + reviewers: + - min_approvals: 2 + teams: + - core-devs + - min_approvals: 1 + teams: + - frame-coders + +prevent-review-request: + teams: + - core-devs diff --git a/src/test/index.test.ts b/src/test/index.test.ts index 3a327af..1e8dffb 100644 --- a/src/test/index.test.ts +++ b/src/test/index.test.ts @@ -1,3 +1,252 @@ -test("adds 1 + 2 to equal 3", () => { - expect(1 + 2).toBe(3); +/* eslint-disable @typescript-eslint/ban-ts-comment */ +import { PullRequest, PullRequestReview } from "@octokit/webhooks-types"; +import { existsSync, openSync, readFileSync, unlinkSync } from "fs"; +import { DeepMockProxy, Matcher, mock, mockDeep, MockProxy } from "jest-mock-extended"; +import { join } from "path"; + +import { GitHubChecksApi } from "../github/check"; +import { PullRequestApi } from "../github/pullRequest"; +import { GitHubTeamsApi, TeamApi } from "../github/teams"; +import { ActionLogger, GitHubClient } from "../github/types"; +import { ActionRunner, RuleReport } from "../runner"; + +type ReportName = + | "CI files" + | "Core developers" + | "Runtime files cumulus" + | "Bridges subtree files" + | "FRAME coders substrate"; + +/** Utility method to get a particular report from a list */ +const getReport = (reports: RuleReport[], name: ReportName): RuleReport => { + for (const report of reports) { + if (report.name === name) { + return report; + } + } + throw new Error(`Report ${name} not found. Available reports are: ${reports.map((r) => r.name).join(". ")}`); +}; + +describe("Integration testing", () => { + const file = join(__dirname, "./", "config.yml"); + const config = readFileSync(file, "utf8"); + + const teamsMembers: [string, string[]][] = [ + ["ci", ["ci-1", "ci-2", "ci-3"]], + ["release-engineering", ["re-1", "re-2", "re-3"]], + ["core-devs", ["gavofyork", "bkchr", "core-1", "core-2"]], + ["locks-review", ["gavofyork", "bkchr", "lock-1"]], + ["polkadot-review", ["gavofyork", "bkchr", "pr-1", "pr-2"]], + ["bridges-core", ["bridge-1", "bridge-2", "bridge-3"]], + ["frame-coders", ["frame-1", "frame-2", "frame-3"]], + ]; + + let api: PullRequestApi; + let logger: MockProxy; + let client: DeepMockProxy; + let pr: DeepMockProxy; + let checks: GitHubChecksApi; + let teams: TeamApi; + let runner: ActionRunner; + + const mockReviews = (reviews: (Pick & { login: string })[]) => { + // convert name into ID + const getHash = (input: string): number => { + let hash = 0; + const len = input.length; + for (let i = 0; i < len; i++) { + hash = (hash << 5) - hash + input.charCodeAt(i); + hash |= 0; // to 32bit integer + } + return Math.abs(hash); + }; + + const data = reviews.map(({ state, id, login }) => { + return { state, id, user: { login, id: getHash(login) } }; + }); + + // @ts-ignore because the official type and the library type do not match + client.rest.pulls.listReviews.mockResolvedValue({ data }); + }; + + const summaryTestFile = "./summary-test.html"; + + beforeEach(() => { + logger = mock(); + client = mockDeep(); + pr = mockDeep(); + pr.number = 99; + pr.base.repo.owner.login = "org"; + + api = new PullRequestApi(client, pr, logger, ""); + teams = new GitHubTeamsApi(client, "org", logger); + checks = new GitHubChecksApi(client, pr, logger, "example"); + runner = new ActionRunner(api, teams, checks, logger); + + // @ts-ignore problem with the type being mocked + client.rest.repos.getContent.mockResolvedValue({ data: { content: Buffer.from(config, "utf-8") } }); + mockReviews([]); + for (const [teamName, members] of teamsMembers) { + client.rest.teams.listMembersInOrg + // @ts-ignore as the error is related to the matcher type + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-call + .calledWith(new Matcher<{ team_slug: string }>((value) => value.team_slug === teamName, "Different team name")) + .mockResolvedValue({ + // @ts-ignore as we don't need the full type + data: members.map((m) => { + return { login: m }; + }), + }); + } + + // @ts-ignore missing more of the required types + client.rest.checks.listForRef.mockResolvedValue({ data: { check_runs: [], total_count: 0 } }); + client.rest.checks.create.mockResolvedValue({ + // @ts-ignore missing types + data: { html_url: "demo", title: "title", output: { text: "output" } }, + }); + + // Create file to upload the summary text (else it will fail) + process.env.GITHUB_STEP_SUMMARY = summaryTestFile; + openSync(summaryTestFile, "w"); + }); + + afterEach(() => { + // delete the summary test file + if (existsSync(summaryTestFile)) { + unlinkSync(summaryTestFile); + } + }); + + describe("Error in config", () => { + test("should fail if it can not get the config", async () => { + // @ts-ignore this could also be an error + client.rest.repos.getContent.mockResolvedValue({ data: {} }); + + await expect(runner.runAction({ configLocation: "example" })).rejects.toThrowError("has no content"); + }); + + test("should fail with invalid config", async () => { + const invalidConfig = ` + rules: + - name: Failing case + condition: + include: + - '.*' + exclude: + - 'example' + type: basic + `; + + // @ts-ignore this could also be an error + client.rest.repos.getContent.mockResolvedValue({ data: { content: Buffer.from(invalidConfig, "utf-8") } }); + + await expect(runner.runAction({ configLocation: "example" })).rejects.toThrowError( + "Configuration for rule 'Failing case' is invalid", + ); + }); + + test("should fail with invalid regex", async () => { + const invalidRegex = "(?("; + const invalidConfig = ` + rules: + - name: Default review + condition: + include: + - '${invalidRegex}' + type: basic + teams: + - team-example + `; + + // @ts-ignore this could also be an error + client.rest.repos.getContent.mockResolvedValue({ data: { content: Buffer.from(invalidConfig, "utf-8") } }); + + await expect(runner.runAction({ configLocation: "example" })).rejects.toThrowError( + "Regular expression is invalid", + ); + }); + }); + + describe("Core developers", () => { + test("should request reviews ", async () => { + // @ts-ignore + client.rest.pulls.listFiles.mockResolvedValue({ data: [{ filename: "README.md" }] }); + const result = await runner.runAction({ configLocation: "abc" }); + expect(result.reports).toHaveLength(1); + expect(result.conclusion).toBe("failure"); + const report = getReport(result.reports, "Core developers"); + expect(report.missingReviews).toBe(2); + }); + + test("should request only one review if author is member", async () => { + // @ts-ignore + client.rest.pulls.listFiles.mockResolvedValue({ data: [{ filename: "README.md" }] }); + pr.user.login = "gavofyork"; + const result = await runner.runAction({ configLocation: "abc" }); + expect(result.reports).toHaveLength(1); + expect(result.conclusion).toBe("failure"); + const report = getReport(result.reports, "Core developers"); + expect(report.missingReviews).toBe(1); + }); + + test("should approve PR if it has enough approvals", async () => { + // @ts-ignore + client.rest.pulls.listFiles.mockResolvedValue({ data: [{ filename: "README.md" }] }); + mockReviews([ + { login: "core-1", state: "approved", id: 12 }, + { login: "core-2", state: "approved", id: 123 }, + ]); + const result = await runner.runAction({ configLocation: "abc" }); + expect(result.reports).toHaveLength(0); + expect(result.conclusion).toBe("success"); + }); + }); + + test("should request a runtime upgrade review if the file is from runtime upgrades", async () => { + // @ts-ignore + client.rest.pulls.listFiles.mockResolvedValue({ data: [{ filename: "cumulus/parachains/common/src/example.rs" }] }); + + const result = await runner.runAction({ configLocation: "abc" }); + expect(result.reports).toHaveLength(1); + expect(result.conclusion).toBe("failure"); + const report = getReport(result.reports, "Runtime files cumulus"); + expect(report.missingReviews).toBe(2); + }); + + test("should request only one runtime upgrade review if the file is from runtime upgrades and the author belongs to one of the teams", async () => { + // @ts-ignore + client.rest.pulls.listFiles.mockResolvedValue({ data: [{ filename: "cumulus/parachains/common/src/example.rs" }] }); + pr.user.login = "gavofyork"; + const result = await runner.runAction({ configLocation: "abc" }); + expect(result.reports).toHaveLength(1); + expect(result.conclusion).toBe("failure"); + const report = getReport(result.reports, "Runtime files cumulus"); + expect(report.missingReviews).toBe(2); + }); + + describe("Combinations", () => { + test("should use same reviewer for separate rules", async () => { + client.rest.pulls.listFiles.mockResolvedValue({ + // @ts-ignore + data: [{ filename: "cumulus/parachains/common/src/example.rs" }, { filename: "README.md" }], + }); + mockReviews([{ state: "approved", id: 123, login: "gavofyork" }]); + const newResult = await runner.runAction({ configLocation: "abc" }); + expect(newResult.reports.map((r) => r.missingReviews).reduce((a, b) => a + b, 0)).toBe(3); + }); + + test("should use same reviewers for separate rules", async () => { + client.rest.pulls.listFiles.mockResolvedValue({ + // @ts-ignore + data: [{ filename: "cumulus/parachains/common/src/example.rs" }, { filename: "README.md" }], + }); + mockReviews([ + { state: "approved", id: 123, login: "gavofyork" }, + { state: "approved", id: 124, login: "bkchr" }, + ]); + const result = await runner.runAction({ configLocation: "abc" }); + expect(result.conclusion).toEqual("success"); + }); + }); });