forked from paritytech/review-bot
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Created integration tests (paritytech#67)
This closes paritytech#52
- Loading branch information
Showing
5 changed files
with
343 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
node_modules | ||
dist | ||
.git | ||
src/test/**/*.yml |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,6 @@ | |
!.vscode/settings.json | ||
.idea | ||
.DS_Store | ||
|
||
# Testing | ||
summary-test.html |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<ActionLogger>; | ||
let client: DeepMockProxy<GitHubClient>; | ||
let pr: DeepMockProxy<PullRequest>; | ||
let checks: GitHubChecksApi; | ||
let teams: TeamApi; | ||
let runner: ActionRunner; | ||
|
||
const mockReviews = (reviews: (Pick<PullRequestReview, "state" | "id"> & { 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<ActionLogger>(); | ||
client = mockDeep<GitHubClient>(); | ||
pr = mockDeep<PullRequest>(); | ||
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"); | ||
}); | ||
}); | ||
}); |