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.

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

Debugging and Documentation #107

Merged
merged 6 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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