Skip to content

Commit

Permalink
Merge pull request #107 from github/config-issues
Browse files Browse the repository at this point in the history
Debugging and Documentation
  • Loading branch information
GrantBirki authored Oct 17, 2023
2 parents d860124 + e428b06 commit 84266cf
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 70 deletions.
5 changes: 5 additions & 0 deletions __tests__/github-provider.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
const nock = require("nock");
const { GitHubProvider } = require("../src/github-provider");

import * as core from "@actions/core";

jest.spyOn(core, "debug").mockImplementation(() => {});
jest.spyOn(core, "info").mockImplementation(() => {});

test("It creates an approved review", async () => {
process.env["GITHUB_REPOSITORY"] = "foo/bar";

Expand Down
9 changes: 6 additions & 3 deletions __tests__/privileged-requester.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { GitHubProvider } from "../src/github-provider";
import { PrivilegedRequester } from "../src/privileged-requester";

import * as core from "@actions/core";

jest.spyOn(core, "debug").mockImplementation(() => {});
jest.spyOn(core, "info").mockImplementation(() => {});

test("We receive the expected config content", async () => {
let configContent = `---
requesters:
Expand Down Expand Up @@ -28,9 +33,7 @@ requesters:

test("We do not receive the expected config content", async () => {
let provider = new GitHubProvider("token");
jest
.spyOn(provider, "getConfigContent")
.mockImplementation(() => false);
jest.spyOn(provider, "getConfigContent").mockImplementation(() => false);
expect(provider.getConfigContent()).toBe(false);

let privilegedRequester = new PrivilegedRequester(provider);
Expand Down
8 changes: 2 additions & 6 deletions __tests__/pull-request.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ test("We can list commits", async () => {
{ author: { login: "danhoerst" } },
];
let provider = new GitHubProvider("token");
jest
.spyOn(provider, "listPRCommits")
.mockImplementation(() => prCommits);
jest.spyOn(provider, "listPRCommits").mockImplementation(() => prCommits);
expect(provider.listPRCommits()).toBe(prCommits);

let pullRequest = new PullRequest(provider);
Expand All @@ -39,9 +37,7 @@ test("We can list commits", async () => {
test("We can list labels", async () => {
let prLabels = [{ name: "bug" }, { name: "feature-request" }];
let provider = new GitHubProvider("token");
jest
.spyOn(provider, "listLabelsOnPR")
.mockImplementation(() => prLabels);
jest.spyOn(provider, "listLabelsOnPR").mockImplementation(() => prLabels);
expect(provider.listLabelsOnPR()).toBe(prLabels);

let pullRequest = new PullRequest(provider);
Expand Down
60 changes: 15 additions & 45 deletions __tests__/runner.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ afterEach(() => {
describe("processCommits", () => {
test("We process commits successfully", async () => {
let prCommits = [{ author: { login: "robot" } }];
jest
.spyOn(pullRequest, "listCommits")
.mockImplementation(() => prCommits);
jest.spyOn(pullRequest, "listCommits").mockImplementation(() => prCommits);
expect(pullRequest.listCommits()).toBe(prCommits);

let commits = await runner.processCommits("robot");
Expand All @@ -39,9 +37,7 @@ describe("processCommits", () => {
{ author: { login: "robot" } },
{ author: { login: "danhoerst" } },
];
jest
.spyOn(pullRequest, "listCommits")
.mockImplementation(() => prCommits);
jest.spyOn(pullRequest, "listCommits").mockImplementation(() => prCommits);
expect(pullRequest.listCommits()).toBe(prCommits);

let commits = await runner.processCommits("robot");
Expand All @@ -63,9 +59,7 @@ index 2f4e8d9..93c2072 100644
if: blah
with:
name: dist`;
jest
.spyOn(pullRequest, "getDiff")
.mockImplementation(() => prDiff);
jest.spyOn(pullRequest, "getDiff").mockImplementation(() => prDiff);
expect(pullRequest.getDiff()).toBe(prDiff);

let diff = await runner.processDiff();
Expand All @@ -86,9 +80,7 @@ index 2f4e8d9..93c2072 100644
if: blah
with:
name: dist`;
jest
.spyOn(pullRequest, "getDiff")
.mockImplementation(() => prDiff);
jest.spyOn(pullRequest, "getDiff").mockImplementation(() => prDiff);
expect(pullRequest.getDiff()).toBe(prDiff);

let diff = await runner.processDiff();
Expand Down Expand Up @@ -117,9 +109,7 @@ describe("labelsEqual", () => {
describe("processLabels", () => {
test("We process labels successfully", async () => {
let prLabels = [{ name: "bug" }, { name: "feature-request" }];
jest
.spyOn(pullRequest, "listLabels")
.mockImplementation(() => prLabels);
jest.spyOn(pullRequest, "listLabels").mockImplementation(() => prLabels);
expect(pullRequest.listLabels()).toBe(prLabels);

let commits = await runner.processLabels({
Expand All @@ -130,9 +120,7 @@ describe("processLabels", () => {

test("We process labels unsuccessfully", async () => {
let prLabels = [{ name: "bug" }, { name: "feature-request" }];
jest
.spyOn(pullRequest, "listLabels")
.mockImplementation(() => prLabels);
jest.spyOn(pullRequest, "listLabels").mockImplementation(() => prLabels);
expect(pullRequest.listLabels()).toBe(prLabels);

let commits = await runner.processLabels({
Expand All @@ -148,15 +136,11 @@ describe("processPrivilegedReviewer", () => {
process.env["INPUT_CHECKLABELS"] = "true";
process.env["INPUT_CHECKDIFF"] = "true";
let prLabels = [{ name: "bug" }, { name: "feature-request" }];
jest
.spyOn(pullRequest, "listLabels")
.mockImplementation(() => prLabels);
jest.spyOn(pullRequest, "listLabels").mockImplementation(() => prLabels);
expect(pullRequest.listLabels()).toBe(prLabels);

let prCommits = [{ author: { login: "robot" } }];
jest
.spyOn(pullRequest, "listCommits")
.mockImplementation(() => prCommits);
jest.spyOn(pullRequest, "listCommits").mockImplementation(() => prCommits);
expect(pullRequest.listCommits()).toBe(prCommits);

let prDiff = `| diff --git a/.github/workflows/check-dist.yml b/.github/workflows/check-dist.yml
Expand All @@ -171,9 +155,7 @@ index 2f4e8d9..93c2072 100644
if: blah
with:
name: dist`;
jest
.spyOn(pullRequest, "getDiff")
.mockImplementation(() => prDiff);
jest.spyOn(pullRequest, "getDiff").mockImplementation(() => prDiff);
expect(pullRequest.getDiff()).toBe(prDiff);

let processed = await runner.processPrivilegedReviewer("robot", {
Expand All @@ -189,9 +171,7 @@ index 2f4e8d9..93c2072 100644
{ author: { login: "robot" } },
{ author: { login: "malicious" } },
];
jest
.spyOn(pullRequest, "listCommits")
.mockImplementation(() => prCommits);
jest.spyOn(pullRequest, "listCommits").mockImplementation(() => prCommits);
expect(pullRequest.listCommits()).toBe(prCommits);

let processed = await runner.processPrivilegedReviewer("robot", {
Expand All @@ -205,9 +185,7 @@ index 2f4e8d9..93c2072 100644
{ author: { login: "robot" } },
{ author: { login: "malicious" } },
];
jest
.spyOn(pullRequest, "listCommits")
.mockImplementation(() => prCommits);
jest.spyOn(pullRequest, "listCommits").mockImplementation(() => prCommits);
expect(pullRequest.listCommits()).toBe(prCommits);

let processed = await runner.processPrivilegedReviewer("robot", {
Expand All @@ -219,9 +197,7 @@ index 2f4e8d9..93c2072 100644
test("We process labels unsuccessfully with the option enabled", async () => {
process.env["INPUT_CHECKLABELS"] = "true";
let prLabels = [{ name: "bug" }, { name: "feature-request" }];
jest
.spyOn(pullRequest, "listLabels")
.mockImplementation(() => prLabels);
jest.spyOn(pullRequest, "listLabels").mockImplementation(() => prLabels);
expect(pullRequest.listLabels()).toBe(prLabels);

let processed = await runner.processPrivilegedReviewer("robot", {
Expand All @@ -232,9 +208,7 @@ index 2f4e8d9..93c2072 100644

test("We allow bad labels when the option to check them is not set", async () => {
let prLabels = [{ name: "bug" }, { name: "feature-request" }];
jest
.spyOn(pullRequest, "listLabels")
.mockImplementation(() => prLabels);
jest.spyOn(pullRequest, "listLabels").mockImplementation(() => prLabels);
expect(pullRequest.listLabels()).toBe(prLabels);

let processed = await runner.processPrivilegedReviewer("robot", {
Expand All @@ -258,9 +232,7 @@ index 2f4e8d9..93c2072 100644
if: blah
with:
name: dist`;
jest
.spyOn(pullRequest, "getDiff")
.mockImplementation(() => prDiff);
jest.spyOn(pullRequest, "getDiff").mockImplementation(() => prDiff);
expect(pullRequest.getDiff()).toBe(prDiff);

let processed = await runner.processPrivilegedReviewer("robot", {
Expand All @@ -283,9 +255,7 @@ index 2f4e8d9..93c2072 100644
if: blah
with:
name: dist`;
jest
.spyOn(pullRequest, "getDiff")
.mockImplementation(() => prDiff);
jest.spyOn(pullRequest, "getDiff").mockImplementation(() => prDiff);
expect(pullRequest.getDiff()).toBe(prDiff);

let processed = await runner.processPrivilegedReviewer("robot", {
Expand Down
5 changes: 4 additions & 1 deletion .github/workflows/privileged-requester.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ on:

permissions:
pull-requests: write
contents: read

jobs:
check:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: checkout
uses: actions/checkout@v4

- uses: ./
with:
myToken: ${{ secrets.GITHUB_TOKEN }}
Expand Down
35 changes: 28 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,32 @@ The workflow should be configured like:
> Where `vX.X.X` is the latest release version found on the releases page
```yaml
name: privileged-requester
on:
pull_request:
types: [opened, synchronize, reopened, labeled, unlabeled]

permissions:
pull-requests: write
contents: read

jobs:
check:
runs-on: ubuntu-latest
steps:
# checkout the repository
- uses: actions/checkout@v4

# run privileged-requester
- uses: github/[email protected]
- name: checkout
uses: actions/checkout@v4

- uses: github/[email protected]
with:
myToken: ${{ secrets.GITHUB_TOKEN }}
robotUserToken: ${{ secrets.REPO_GITHUB_TOKEN }}
path: config/privileged-requester.yaml
prCreator: ${{ github.event.pull_request.user.login }}
prNumber: ${{ github.event.pull_request.number }}
checkCommits: "true"
checkDiff: "true"
checkLabels: "true"
```
See the example in [the workflow folder](.github/workflows/privileged-requester.yml)
Expand Down Expand Up @@ -51,7 +68,7 @@ However, you can configure the Action to run with a different repo scoped token

Here are the configuration options for this Action:

## Inputs 📥
### Inputs 📥

| Input | Required? | Default | Description |
|-----------| --------- |---------------------------------------------| ----------- |
Expand All @@ -64,8 +81,12 @@ Here are the configuration options for this Action:
| `checkDiff` | yes | `"true"` | An option to check that the PR diff only has a removal diff, with no additions |
| `checkLabels` | yes | `"true"` | An option to check that the labels on the PR match those defined in the privileged requester config |

## Outputs 📤
### Outputs 📤

| Output | Description |
| ------ | ----------- |
| `approved` | The string `"true"` if the privileged-requester approved the pull request |

## First Time Setup

It should be noted that this Action looks at the `default` branch for its configuration file. This means that if you add this Action through a pull request, it will look at the default branch and _fail_ because it cannot find the config file that has not landed on `main` / `master` yet. After merging the pull request that adds this Action to your repository, it should work as expected.
18 changes: 15 additions & 3 deletions dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

6 changes: 5 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import { Runner } from "./src/runner";
const core = require("@actions/core");
let myToken = core.getInput("myToken");
const robotUserToken = core.getInput("robotUserToken");
if (robotUserToken !== "") {
if (
robotUserToken !== "" ||
robotUserToken !== undefined ||
robotUserToken !== null
) {
core.info("Robot User configured. I will use that PAT instead.");
myToken = robotUserToken;
}
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"prepare": "ncc build index.js -o dist --source-map --license licenses.txt",
"test": "jest",
"bundle": "npm run format && npm run package",
"all": "npm run lint && npm run prepare && npm run test"
"all": "npm run lint && npm run format && npm run prepare && npm run test"
},
"repository": {
"type": "git",
Expand Down
8 changes: 7 additions & 1 deletion src/github-provider.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ class GitHubProvider {
}

async createReview(prNumber, reviewEvent) {
core.debug(`prNumber: ${prNumber}`);
core.debug(`reviewEvent: ${reviewEvent}`);

await this.octokit.rest.pulls.createReview({
owner: github.context.repo.owner,
repo: github.context.repo.repo,
Expand All @@ -20,11 +23,14 @@ class GitHubProvider {
}

async getConfigContent() {
const path = core.getInput("path", { required: true });
core.info(`config path: ${path}`);

// getContent defaults to the main branch
const { data: configContent } = await this.octokit.rest.repos.getContent({
owner: github.context.repo.owner,
repo: github.context.repo.repo,
path: core.getInput("path"),
path: path,
mediaType: { format: "raw" },
});
return configContent;
Expand Down
Loading

0 comments on commit 84266cf

Please sign in to comment.