From 2466f3b81422cc61673992ee14af684cff35026f Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 31 Aug 2023 10:44:07 +0200 Subject: [PATCH 01/10] added configuration file --- src/test/config.yml | 101 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 src/test/config.yml diff --git a/src/test/config.yml b/src/test/config.yml new file mode 100644 index 0000000..60be59d --- /dev/null +++ b/src/test/config.yml @@ -0,0 +1,101 @@ +rules: + - name: CI files + check_type: changed_files + condition: + include: + - ^\.gitlab-ci\.yml + - ^docker/.* + - ^\.github/.* + - ^\.gitlab/.* + - ^\.config/nextest.toml + - ^\.cargo/.* + exclude: + - ^./gitlab/pipeline/zombienet.yml$ + min_approvals: 2 + teams: + - ci + - release-engineering + + - name: Core developers + 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 + teams: + - core-devs + + # cumulus + - name: Runtime files cumulus + condition: + - ^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 + check_type: changed_files + condition: + - ^cumulus/bridges/.* + min_approvals: 1 + teams: + - bridges-core + + # substrate + + - name: FRAME coders substrate + check_type: changed_files + condition: + include: + - ^substrate/frame/(?!.*(nfts/.* + - uniques/.* + - babe/.* + - grandpa/.* + - beefy + - merkle-mountain-range/.* + - contracts/.* + - election + - nomination-pools/.* + - staking/.* + - aura/.*)) + all: + - min_approvals: 2 + teams: + - core-devs + - min_approvals: 1 + teams: + - frame-coders + +prevent-review-request: + teams: + - core-devs From 02c4a594718666b8379441825b7af57ddc75f18d Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 31 Aug 2023 13:36:41 +0200 Subject: [PATCH 02/10] fixed config file --- src/test/config.yml | 42 ++++++++++++++---------------------------- 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/src/test/config.yml b/src/test/config.yml index 60be59d..4d308d9 100644 --- a/src/test/config.yml +++ b/src/test/config.yml @@ -12,13 +12,16 @@ rules: exclude: - ^./gitlab/pipeline/zombienet.yml$ min_approvals: 2 + type: basic teams: - ci - release-engineering - name: Core developers + countAuthor: true condition: - include: .* + include: + - .* # excluding files from 'Runtime files' and 'CI files' rules exclude: - ^polkadot/runtime/(kusama|polkadot)/src/[^/]+\.rs$ @@ -26,29 +29,21 @@ rules: - ^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/.*)) + - ^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/.* + - ^(?!.*\.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$ @@ -64,8 +59,9 @@ rules: # if there are any changes in the bridges subtree (in case of backport changes back to bridges repo) - name: Bridges subtree files - check_type: changed_files + type: basic condition: + include: - ^cumulus/bridges/.* min_approvals: 1 teams: @@ -74,21 +70,11 @@ rules: # substrate - name: FRAME coders substrate - check_type: changed_files condition: include: - - ^substrate/frame/(?!.*(nfts/.* - - uniques/.* - - babe/.* - - grandpa/.* - - beefy - - merkle-mountain-range/.* - - contracts/.* - - election - - nomination-pools/.* - - staking/.* - - aura/.*)) - all: + - ^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 From 1cb771a9d38889c08778cb12cbc4b370b85e6c8a Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 31 Aug 2023 13:36:59 +0200 Subject: [PATCH 03/10] added basic tests --- src/runner.ts | 2 +- src/test/index.test.ts | 90 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/src/runner.ts b/src/runner.ts index 800bd52..0dd22df 100644 --- a/src/runner.ts +++ b/src/runner.ts @@ -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/index.test.ts b/src/test/index.test.ts index 3a327af..1073420 100644 --- a/src/test/index.test.ts +++ b/src/test/index.test.ts @@ -1,3 +1,89 @@ -test("adds 1 + 2 to equal 3", () => { - expect(1 + 2).toBe(3); +/* eslint-disable @typescript-eslint/ban-ts-comment */ +import { PullRequest } from "@octokit/webhooks-types"; +import { readFileSync } from "fs"; +import { DeepMockProxy, 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 } from "../runner"; + +describe("Integration testing", () => { + const file = join(__dirname, "./", "config.yml"); + const config = readFileSync(file, "utf8"); + + let api: PullRequestApi; + let logger: MockProxy; + let client: DeepMockProxy; + let pr: DeepMockProxy; + let checks: GitHubChecksApi; + let teams: TeamApi; + let runner: ActionRunner; + 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") } }); + }); + + 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", + ); + }); + }); }); From e8bb5386d1f56d393f92b3c4f763df9ddbcbd1ab Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Thu, 31 Aug 2023 16:24:22 +0200 Subject: [PATCH 04/10] added set up for the test --- src/test/index.test.ts | 47 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/src/test/index.test.ts b/src/test/index.test.ts index 1073420..222550b 100644 --- a/src/test/index.test.ts +++ b/src/test/index.test.ts @@ -1,7 +1,7 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ -import { PullRequest } from "@octokit/webhooks-types"; +import { PullRequest, PullRequestReview } from "@octokit/webhooks-types"; import { readFileSync } from "fs"; -import { DeepMockProxy, mock, mockDeep, MockProxy } from "jest-mock-extended"; +import { DeepMockProxy, Matcher, mock, mockDeep, MockProxy } from "jest-mock-extended"; import { join } from "path"; import { GitHubChecksApi } from "../github/check"; @@ -14,6 +14,15 @@ 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"]], + ["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; @@ -21,6 +30,22 @@ describe("Integration testing", () => { let checks: GitHubChecksApi; let teams: TeamApi; let runner: ActionRunner; + + const generateReviewer = ( + state: "commented" | "changes_requested" | "approved" | "dismissed", + user: string, + ): PullRequestReview => + ({ + state, + user: { login: user, id: Math.floor(Math.random() * 1000) }, + id: Math.floor(Math.random() * 1000), + }) as PullRequestReview; + + const mockReviews = (reviews: PullRequestReview[]) => { + // @ts-ignore because the official type and the library type do not match + client.rest.pulls.listReviews.mockResolvedValue({ data: reviews }); + }; + beforeEach(() => { logger = mock(); client = mockDeep(); @@ -35,6 +60,19 @@ describe("Integration testing", () => { // @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 }; + }), + }); + } }); describe("Error in config", () => { @@ -86,4 +124,9 @@ describe("Integration testing", () => { ); }); }); + test("should not report problems on empty files", async () => { + // @ts-ignore + client.rest.pulls.listFiles.mockResolvedValue({ data: [{ filename: "README.md" }] }); + await runner.runAction({ configLocation: "abc" }); + }); }); From 440336a805742ed2e4dcc62cb33df1ce4e13a632 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Mon, 4 Sep 2023 13:43:06 -0300 Subject: [PATCH 05/10] improve test files --- .gitignore | 3 +++ src/runner.ts | 2 +- src/test/config.yml | 1 - 3 files changed, 4 insertions(+), 2 deletions(-) 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 0dd22df..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]; diff --git a/src/test/config.yml b/src/test/config.yml index 4d308d9..d539bde 100644 --- a/src/test/config.yml +++ b/src/test/config.yml @@ -1,6 +1,5 @@ rules: - name: CI files - check_type: changed_files condition: include: - ^\.gitlab-ci\.yml From c43a347f213b207accc6d7219d763beec1566502 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Mon, 4 Sep 2023 13:43:12 -0300 Subject: [PATCH 06/10] added more cases --- src/test/index.test.ts | 120 ++++++++++++++++++++++++++++++++++++++--- 1 file changed, 113 insertions(+), 7 deletions(-) diff --git a/src/test/index.test.ts b/src/test/index.test.ts index 222550b..4c4ce17 100644 --- a/src/test/index.test.ts +++ b/src/test/index.test.ts @@ -1,6 +1,6 @@ /* eslint-disable @typescript-eslint/ban-ts-comment */ import { PullRequest, PullRequestReview } from "@octokit/webhooks-types"; -import { readFileSync } from "fs"; +import { existsSync, openSync, readFileSync, unlinkSync } from "fs"; import { DeepMockProxy, Matcher, mock, mockDeep, MockProxy } from "jest-mock-extended"; import { join } from "path"; @@ -8,7 +8,24 @@ import { GitHubChecksApi } from "../github/check"; import { PullRequestApi } from "../github/pullRequest"; import { GitHubTeamsApi, TeamApi } from "../github/teams"; import { ActionLogger, GitHubClient } from "../github/types"; -import { ActionRunner } from "../runner"; +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"); @@ -19,6 +36,7 @@ describe("Integration testing", () => { ["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"]], ]; @@ -41,11 +59,27 @@ describe("Integration testing", () => { id: Math.floor(Math.random() * 1000), }) as PullRequestReview; - const mockReviews = (reviews: PullRequestReview[]) => { + const mockReviews = (reviews: (Pick & { login: string })[]) => { + 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: reviews }); + client.rest.pulls.listReviews.mockResolvedValue({data}); }; + const summaryTestFile = "./summary-test.html"; + beforeEach(() => { logger = mock(); client = mockDeep(); @@ -73,6 +107,24 @@ describe("Integration testing", () => { }), }); } + + // @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", () => { @@ -124,9 +176,63 @@ describe("Integration testing", () => { ); }); }); - test("should not report problems on empty files", async () => { + + 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"); + console.log(report); + 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: "README.md" }] }); - await runner.runAction({ configLocation: "abc" }); + 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"); + console.log(report); + expect(report.missingReviews).toBe(2); }); }); From c3267a1abecfe6ce141e5620cffd61c40b56c184 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Mon, 4 Sep 2023 15:47:10 -0300 Subject: [PATCH 07/10] fixed some linting issues --- .eslintignore | 1 + src/test/config.yml | 2 +- src/test/index.test.ts | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) 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/src/test/config.yml b/src/test/config.yml index d539bde..73ed6ee 100644 --- a/src/test/config.yml +++ b/src/test/config.yml @@ -41,7 +41,7 @@ rules: # cumulus - name: Runtime files cumulus countAuthor: true - condition: + 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$ diff --git a/src/test/index.test.ts b/src/test/index.test.ts index 4c4ce17..92afbf3 100644 --- a/src/test/index.test.ts +++ b/src/test/index.test.ts @@ -75,7 +75,7 @@ describe("Integration testing", () => { }); // @ts-ignore because the official type and the library type do not match - client.rest.pulls.listReviews.mockResolvedValue({data}); + client.rest.pulls.listReviews.mockResolvedValue({ data }); }; const summaryTestFile = "./summary-test.html"; From e009ad3d8d6f7228bbb295b71646246584d1042f Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Mon, 4 Sep 2023 17:47:11 -0300 Subject: [PATCH 08/10] removed unused method --- src/test/index.test.ts | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/test/index.test.ts b/src/test/index.test.ts index 92afbf3..bde5394 100644 --- a/src/test/index.test.ts +++ b/src/test/index.test.ts @@ -49,17 +49,8 @@ describe("Integration testing", () => { let teams: TeamApi; let runner: ActionRunner; - const generateReviewer = ( - state: "commented" | "changes_requested" | "approved" | "dismissed", - user: string, - ): PullRequestReview => - ({ - state, - user: { login: user, id: Math.floor(Math.random() * 1000) }, - id: Math.floor(Math.random() * 1000), - }) as PullRequestReview; - const mockReviews = (reviews: (Pick & { login: string })[]) => { + // convert name into ID const getHash = (input: string): number => { let hash = 0; const len = input.length; From da6c97ebf23bf58898ead510116c4d87dee6814a Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Tue, 5 Sep 2023 10:09:25 -0300 Subject: [PATCH 09/10] added combination of rule tests --- src/test/config.yml | 4 ++-- src/test/index.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/test/config.yml b/src/test/config.yml index 73ed6ee..54e0328 100644 --- a/src/test/config.yml +++ b/src/test/config.yml @@ -19,10 +19,10 @@ rules: - name: Core developers countAuthor: true condition: - include: + include: - .* # excluding files from 'Runtime files' and 'CI files' rules - exclude: + 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$ diff --git a/src/test/index.test.ts b/src/test/index.test.ts index bde5394..0155f85 100644 --- a/src/test/index.test.ts +++ b/src/test/index.test.ts @@ -226,4 +226,31 @@ describe("Integration testing", () => { console.log(report); 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" }); + console.log("nre result", JSON.stringify(newResult, null, 2)); + 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: "" }, { filename: "README.md" }], + }); + mockReviews([ + { state: "approved", id: 123, login: "gavofyork" }, + { state: "approved", id: 124, login: "bkchr" }, + ]); + const result = await runner.runAction({ configLocation: "abc" }); + console.log("nre result", JSON.stringify(result, null, 2)); + expect(result.conclusion).toEqual("success"); + }); + }); }); From 4e18286bb481ca7b94993893126f52145a0af192 Mon Sep 17 00:00:00 2001 From: Javier Bullrich Date: Tue, 5 Sep 2023 12:24:53 -0300 Subject: [PATCH 10/10] fixed missing file --- src/test/index.test.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/test/index.test.ts b/src/test/index.test.ts index 0155f85..1e8dffb 100644 --- a/src/test/index.test.ts +++ b/src/test/index.test.ts @@ -187,7 +187,6 @@ describe("Integration testing", () => { expect(result.reports).toHaveLength(1); expect(result.conclusion).toBe("failure"); const report = getReport(result.reports, "Core developers"); - console.log(report); expect(report.missingReviews).toBe(1); }); @@ -223,7 +222,6 @@ describe("Integration testing", () => { expect(result.reports).toHaveLength(1); expect(result.conclusion).toBe("failure"); const report = getReport(result.reports, "Runtime files cumulus"); - console.log(report); expect(report.missingReviews).toBe(2); }); @@ -235,21 +233,19 @@ describe("Integration testing", () => { }); mockReviews([{ state: "approved", id: 123, login: "gavofyork" }]); const newResult = await runner.runAction({ configLocation: "abc" }); - console.log("nre result", JSON.stringify(newResult, null, 2)); 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: "" }, { filename: "README.md" }], + 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" }); - console.log("nre result", JSON.stringify(result, null, 2)); expect(result.conclusion).toEqual("success"); }); });