Skip to content

Commit

Permalink
chore: Refactors and unifies the getPullRequestNumber functionality
Browse files Browse the repository at this point in the history
  • Loading branch information
davelosert committed Aug 26, 2024
1 parent 2850518 commit 93958d0
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 98 deletions.
2 changes: 1 addition & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import {
parseVitestJsonSummary,
} from "./inputs/parseJsonReports.js";
import { createOctokit } from "./octokit.js";
import { generateCommitSHAUrl } from "./report/generateCommitSHAUrl.js";
import { generateFileCoverageHtml } from "./report/generateFileCoverageHtml.js";
import { generateHeadline } from "./report/generateHeadline.js";
import { generateSummaryTableHtml } from "./report/generateSummaryTableHtml.js";
import type { JsonSummary } from "./types/JsonSummary.js";
import { writeSummaryToPR } from "./writeSummaryToPR.js";
import { generateCommitSHAUrl } from "./report/generateCommitSHAUrl.js";

const run = async () => {
const octokit = createOctokit();
Expand Down
4 changes: 2 additions & 2 deletions src/inputs/getPullChanges.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { beforeEach, describe, expect, it, vi, Mock } from "vitest";
import { Mock, beforeEach, describe, expect, it, vi } from "vitest";
import type { Octokit } from "../octokit";
import { FileCoverageMode } from "./FileCoverageMode";
import { getPullChanges } from "./getPullChanges";
import type { Octokit } from "../octokit";

// Avoid logs
vi.mock("@actions/core");
Expand Down
4 changes: 2 additions & 2 deletions src/inputs/getPullChanges.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as core from "@actions/core";
import * as github from "@actions/github";
import { RequestError } from "@octokit/request-error";
import { FileCoverageMode } from "./FileCoverageMode";
import type { Octokit } from "../octokit";
import { FileCoverageMode } from "./FileCoverageMode";

interface Params {
fileCoverageMode: FileCoverageMode;
Expand All @@ -13,7 +13,7 @@ interface Params {
export async function getPullChanges({
fileCoverageMode,
prNumber,
octokit
octokit,
}: Params): Promise<string[]> {
// Skip Changes collection if we don't need it
if (fileCoverageMode === FileCoverageMode.None) {
Expand Down
117 changes: 117 additions & 0 deletions src/inputs/getPullRequestNumber.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import type * as core from "@actions/core";
import {
type Mock,
afterEach,
beforeEach,
describe,
expect,
it,
vi,
} from "vitest";
import type { Octokit } from "../octokit";
import { getPullRequestNumber } from "./getPullRequestNumber";

type Core = typeof core;

// Avoid logs
vi.mock("@actions/core", async (importOriginal): Promise<Core> => {
const original: Core = await importOriginal();
return {
...original,
startGroup: vi.fn(),
info: vi.fn(),
warning: vi.fn(),
debug: vi.fn(),
};
});

const mockContext = vi.hoisted(() => ({
repo: {
owner: "owner",
repo: "repo",
},
payload: {},
eventName: "",
}));
vi.mock("@actions/github", () => ({
context: mockContext,
}));

describe("getPullRequestNumber()", () => {
let mockOctokit: Octokit;
beforeEach(() => {
vi.clearAllMocks();
mockOctokit = {
paginate: {
iterator: vi.fn(),
},
rest: {
pulls: {
list: vi.fn(),
},
},
} as unknown as Octokit;
});

afterEach(() => {
mockContext.payload = {};
mockContext.eventName = "";
vi.unstubAllEnvs();
});

it("returns the PR number from the input 'pr-number' if valid ", async () => {
vi.stubEnv("INPUT_PR-NUMBER", "123");

const result = await getPullRequestNumber(mockOctokit);
expect(result).toBe(123);
});

it("returns the PR number from payload.pull_request context if found", async () => {
mockContext.payload = {
pull_request: {
number: 456,
},
};

const result = await getPullRequestNumber(mockOctokit);
expect(result).toBe(456);
});

it("returns the PR number from payload.workflow_run context if found", async () => {
mockContext.eventName = "workflow_run";
mockContext.payload = {
workflow_run: {
pull_requests: [{ number: 789 }],
head_sha: "testsha",
},
};

const result = await getPullRequestNumber(mockOctokit);
expect(result).toBe(789);
});

it("calls the API to find PR number by the head_sha of the payload.workflow_run when called from a fork", async () => {
mockContext.eventName = "workflow_run";
mockContext.payload = {
workflow_run: {
pull_requests: [],
head_sha: "testsha",
},
};

(mockOctokit.paginate.iterator as Mock).mockReturnValue([
{
data: [{ number: 101, head: { sha: "testsha" } }],
},
]);

const result = await getPullRequestNumber(mockOctokit);
expect(result).toBe(101);
});

it("returns undefined if no pr number is found", async () => {
mockContext.payload = {};
const result = await getPullRequestNumber(mockOctokit);
expect(result).toBeUndefined();
});
});
83 changes: 36 additions & 47 deletions src/inputs/getPullRequestNumber.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,66 +2,54 @@ import * as core from "@actions/core";
import * as github from "@actions/github";
import type { Octokit } from "../octokit";

async function getPullRequestNumberFromTriggeringWorkflow(
async function getPullRequestNumber(
octokit: Octokit,
): Promise<number | undefined> {
core.info(
"Trying to get the triggering workflow in order to find the pull-request-number to comment the results on...",
);
// Get the user-defined pull-request number and perform input validation
const prNumberFromInput = core.getInput("pr-number");
const processedPrNumber: number | undefined = Number(prNumberFromInput);

if (!github.context.payload.workflow_run) {
core.info(
"The triggering workflow does not have a workflow_run payload. Skipping comment creation.",
);
return undefined;
// Check if it is a full integer. Check for non-null as qhen the option is not set, the parsed input will be an empty string
// which becomes 0 when parsed to a number.
if (Number.isSafeInteger(processedPrNumber) && processedPrNumber !== 0) {
core.info(`Received pull-request number: ${processedPrNumber}`);
return processedPrNumber;
}

const originalWorkflowRunId = github.context.payload.workflow_run.id;

const { data: originalWorkflowRun } =
await octokit.rest.actions.getWorkflowRun({
owner: github.context.repo.owner,
repo: github.context.repo.repo,
run_id: originalWorkflowRunId,
});

if (originalWorkflowRun.event !== "pull_request") {
if (github.context.payload.pull_request) {
core.info(
"The triggering workflow is not a pull-request. Skipping comment creation.",
`Found pull-request number in the action's "payload.pull_request" context: ${github.context.payload.pull_request.number}`,
);
return undefined;
return github.context.payload.pull_request.number;
}

// When the actual pull-request is not coming from a fork, the pull_request object is correctly populated and we can shortcut here
if (
originalWorkflowRun.pull_requests &&
originalWorkflowRun.pull_requests.length > 0
) {
return originalWorkflowRun.pull_requests[0].number;
}

// When the actual pull-request is coming from a fork, the pull_request object is not populated (see https://github.com/orgs/community/discussions/25220)
core.info(
`Trying to find the pull-request for the triggering workflow run with id: ${originalWorkflowRunId} (${originalWorkflowRun.url}) with HEAD_SHA ${originalWorkflowRun.head_sha}...`,
);

// The way to find the pull-request in this scenario is to query all existing pull_requests on the target repository and find the one with the same HEAD_SHA as the original workflow run
const pullRequest = await findPullRequest(
octokit,
originalWorkflowRun.head_sha,
);
if (github.context.eventName === "workflow_run") {
// Workflow_runs triggered from non-forked PRs will have the PR number in the payload
if (github.context.payload.workflow_run.pull_requests.length > 0) {
core.debug(
`Found pull-request number in the action's "payload.workflow_run" context: ${github.context.payload.workflow_run.pull_requests[0].number}`,
);
return github.context.payload.workflow_run.pull_requests[0].number;
}

if (!pullRequest) {
core.info(
"Could not find the pull-request for the triggering workflow run. Skipping comment creation.",
// ... in all other cases, we have to call the API to get a matching PR number
core.debug(
"Trying to find pull-request number in payload.workflow_run context by calling the API",
);
return await findPullRequestBySHA(
octokit,
github.context.payload.workflow_run.head_sha,
);
return undefined;
}

return pullRequest.number;
core.info("No pull-request number found. Comment creation will be skipped!");
return undefined;
}

async function findPullRequest(octokit: Octokit, headSha: string) {
async function findPullRequestBySHA(
octokit: Octokit,
headSha: string,
): Promise<number | undefined> {
core.startGroup("Querying REST API for pull-requests.");
const pullRequestsIterator = octokit.paginate.iterator(
octokit.rest.pulls.list,
Expand All @@ -79,12 +67,13 @@ async function findPullRequest(octokit: Octokit, headSha: string) {
`Comparing: ${pullRequest.number} sha: ${pullRequest.head.sha} with expected: ${headSha}.`,
);
if (pullRequest.head.sha === headSha) {
return pullRequest;
return pullRequest.number;
}
}
}
core.endGroup();
core.info(`Could not find the pull-request for commit "${headSha}".`);
return undefined;
}

export { getPullRequestNumberFromTriggeringWorkflow };
export { getPullRequestNumber };
45 changes: 3 additions & 42 deletions src/inputs/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import * as github from "@actions/github";
import type { Octokit } from "../octokit";
import type { Thresholds } from "../types/Threshold";
import { type FileCoverageMode, getCoverageModeFrom } from "./FileCoverageMode";
import { getPullRequestNumberFromTriggeringWorkflow } from "./getPullRequestNumber";
import { getPullRequestNumber } from "./getPullRequestNumber";
import { getViteConfigPath } from "./getViteConfigPath";
import { parseCoverageThresholds } from "./parseCoverageThresholds";

Expand Down Expand Up @@ -58,9 +58,9 @@ async function readOptions(octokit: Octokit): Promise<Options> {
? await parseCoverageThresholds(viteConfigPath)
: {};

// Get the user-defined pull-request number and perform input validation
const prNumber = await getPrNumber(octokit);
const commitSHA = getCommitSHA();
// Get the user-defined pull-request number and perform input validation
const prNumber = await getPullRequestNumber(octokit);

return {
fileCoverageMode,
Expand All @@ -75,45 +75,6 @@ async function readOptions(octokit: Octokit): Promise<Options> {
};
}

async function getPrNumber(octokit: Octokit): Promise<number | undefined> {
// Get the user-defined pull-request number and perform input validation
const prNumberFromInput = core.getInput("pr-number");
const processedPrNumber: number | undefined = Number(prNumberFromInput);

// Check if it is a full integer. Check for non-null as qhen the option is not set, the parsed input will be an empty string
// which becomes 0 when parsed to a number.
if (Number.isSafeInteger(processedPrNumber) && processedPrNumber !== 0) {
core.debug(`Received pull-request number: ${processedPrNumber}`);
return processedPrNumber;
}

if (github.context.payload.pull_request) {
core.debug(
`Found pull-request number in payload.pull_request context: ${github.context.payload.pull_request.number}`,
);
return github.context.payload.pull_request.number;
}

if (github.context.eventName === "workflow_run") {
// Workflow_runs triggered from non-forked PRs will have the PR number in the payload
if (github.context.payload.workflow_run.pull_requests.length > 0) {
core.debug(
`Found pull-request number in payload.workflow_run context: ${github.context.payload.workflow_run.pull_requests[0].number}`,
);
return github.context.payload.workflow_run.pull_requests[0].number;
}

// ... in all other cases, we have to call the API to get a matching PR number
core.debug(
"Trying to find pull-request number in payload.workflow_run context by calling the API",
);
return await getPullRequestNumberFromTriggeringWorkflow(octokit);
}

core.debug("No pull-request number found.");
return undefined;
}

function getCommitSHA(): string {
if (
github.context.eventName === "pull_request" ||
Expand Down
4 changes: 2 additions & 2 deletions src/report/generateCommitSHAUrl.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { describe, it, expect, vi } from "vitest";
import { describe, expect, it, vi } from "vitest";
import { generateCommitSHAUrl } from "./generateCommitSHAUrl";

const mockContext = vi.hoisted(() => ({
repo: {
owner: "owner",
repo: "repo",
},
serverUrl: "https://github.com",
serverUrl: "https://github.com",
payload: {},
}));
vi.mock("@actions/github", () => ({
Expand Down
4 changes: 2 additions & 2 deletions src/writeSummaryToPR.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { beforeEach, describe, expect, it, vi, Mock } from "vitest";
import * as core from "@actions/core";
import * as github from "@actions/github";
import { writeSummaryToPR } from "./writeSummaryToPR";
import { Mock, beforeEach, describe, expect, it, vi } from "vitest";
import type { Octokit } from "./octokit";
import { writeSummaryToPR } from "./writeSummaryToPR";

vi.mock("@actions/core");

Expand Down

0 comments on commit 93958d0

Please sign in to comment.