Skip to content

Commit

Permalink
feat: allow user defined pull request number (#369)
Browse files Browse the repository at this point in the history
* allow user defined pull request number

* use user defined PR number only if it is not in the context

* log the user-defined PR number

* address review comments: user defined PR number takes precedence; input validation

* Enhance the description of pr-number in README

Co-authored-by: David Losert <[email protected]>

---------

Co-authored-by: David Losert <[email protected]>
  • Loading branch information
xli12 and davelosert authored Jun 11, 2024
1 parent 2a9001a commit 9455b83
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 25 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ This action requires the `pull-request: write` permission to add a comment to yo
| `file-coverage-mode` | Defines how file-based coverage is reported. Possible values are `all`, `changes` or `none`. | `changes` |
| `name` | Give the report a custom name. This is useful if you want multiple reports for different test suites within the same PR. Needs to be unique. | '' |
| `json-summary-compare-path` | The path to the json summary file to compare against. If given, will display a trend indicator and the difference in the summary. Respects the `working-directory` option. | undefined |
| `pr-number` | The number of the PR to post a comment to (if any) | If in the context of a PR, the number of that PR.<br/> If in the context of a triggered workflow, the PR of the triggering workflow. <br/>If no PR context is found, it defaults to `undefined` |

#### File Coverage Mode

Expand Down
4 changes: 4 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ inputs:
required: false
description: 'The name of the coverage report. Can be used to execute this action multiple times. '
default: ''
pr-number:
required: false
description: 'An optional, user-defined pull request number.'
default: ''
runs:
using: 'node20'
main: 'dist/index.js'
Expand Down
10 changes: 6 additions & 4 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ const run = async () => {
jsonSummaryComparePath,
name,
thresholds,
workingDirectory
workingDirectory,
processedPrNumber
} = await readOptions();

const jsonSummary = await parseVitestJsonSummary(jsonSummaryPath);

let jsonSummaryCompare;
if(jsonSummaryComparePath) {
if (jsonSummaryComparePath) {
jsonSummaryCompare = await parseVitestJsonSummary(jsonSummaryComparePath);
}

Expand All @@ -48,7 +49,8 @@ const run = async () => {
try {
await writeSummaryToPR({
summary,
markerPostfix: getMarkerPostfix({ name, workingDirectory })
markerPostfix: getMarkerPostfix({ name, workingDirectory }),
userDefinedPrNumber: processedPrNumber
});
} catch (error) {
if (error instanceof RequestError && (error.status === 404 || error.status === 403)) {
Expand Down
19 changes: 15 additions & 4 deletions src/inputs/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,28 @@ async function readOptions() {
const jsonFinalPath = path.resolve(workingDirectory, core.getInput('json-final-path'));


const jsonSummaryCompareInput = core.getInput('json-summary-compare-path');
const jsonSummaryCompareInput = core.getInput('json-summary-compare-path');
let jsonSummaryComparePath;
if(jsonSummaryCompareInput) {
if (jsonSummaryCompareInput) {
jsonSummaryComparePath = path.resolve(workingDirectory, jsonSummaryCompareInput);
}

const name = core.getInput('name');

// ViteConfig is optional, as it is only required for thresholds. If no vite config is provided, we will not include thresholds in the final report.
// Get the user-defined pull-request number and perform input validation
const prNumber = core.getInput('pr-number');
let processedPrNumber: number | undefined = Number(prNumber);
if (!Number.isSafeInteger(processedPrNumber) || processedPrNumber <= 0) {
processedPrNumber = undefined;
}
if (processedPrNumber) {
core.info(`Received pull-request number: ${processedPrNumber}`);
}

// ViteConfig is optional, as it is only required for thresholds. If no vite config is provided, we will not include thresholds in the final report.
const viteConfigPath = await getViteConfigPath(workingDirectory, core.getInput("vite-config-path"));
const thresholds = viteConfigPath ? await parseCoverageThresholds(viteConfigPath) : {};

return {
fileCoverageMode,
jsonFinalPath,
Expand All @@ -35,6 +45,7 @@ async function readOptions() {
name,
thresholds,
workingDirectory,
processedPrNumber,
}
}

Expand Down
40 changes: 23 additions & 17 deletions src/writeSummaryToPR.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,23 @@ const gitHubToken = core.getInput('github-token').trim();
const octokit: Octokit = github.getOctokit(gitHubToken);
const COMMENT_MARKER = (markerPostfix = 'root') => `<!-- vitest-coverage-report-marker-${markerPostfix} -->`;

type Octokit = ReturnType<typeof github.getOctokit>;
const writeSummaryToPR = async ({ summary, markerPostfix }: {
summary: typeof core.summary;
markerPostfix?: string;
type Octokit = ReturnType<typeof github.getOctokit>;
const writeSummaryToPR = async ({ summary, markerPostfix, userDefinedPrNumber }: {
summary: typeof core.summary;
markerPostfix?: string;
userDefinedPrNumber?: number;
}) => {
// If in the context of a pull-request, get the pull-request number
let pullRequestNumber = github.context.payload.pull_request?.number;
// The user-defined pull request number takes precedence
let pullRequestNumber = userDefinedPrNumber;

// This is to allow commenting on pull_request from forks
if (github.context.eventName === 'workflow_run') {
pullRequestNumber = await getPullRequestNumberFromTriggeringWorkflow(octokit);
if (!pullRequestNumber) {
// If in the context of a pull-request, get the pull-request number
pullRequestNumber = github.context.payload.pull_request?.number;

// This is to allow commenting on pull_request from forks
if (github.context.eventName === 'workflow_run') {
pullRequestNumber = await getPullRequestNumberFromTriggeringWorkflow(octokit);
}
}

if (!pullRequestNumber) {
Expand Down Expand Up @@ -71,23 +77,23 @@ async function getPullRequestNumberFromTriggeringWorkflow(octokit: Octokit): Pro
run_id: originalWorkflowRunId,
});

if(originalWorkflowRun.event !== 'pull_request') {
if (originalWorkflowRun.event !== 'pull_request') {
core.info('The triggering workflow is not a pull-request. Skipping comment creation.');
return undefined;
}

// 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) {
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(!pullRequest) {
if (!pullRequest) {
core.info('Could not find the pull-request for the triggering workflow run. Skipping comment creation.');
return undefined;
}
Expand All @@ -100,11 +106,11 @@ async function findPullRequest(octokit: Octokit, headSha: string) {
core.startGroup(`Querying REST API for Pull-Requests.`);
const pullRequestsIterator = octokit.paginate.iterator(
octokit.rest.pulls.list, {
owner: github.context.repo.owner,
repo: github.context.repo.repo,
per_page: 30
owner: github.context.repo.owner,
repo: github.context.repo.repo,
per_page: 30
});

for await (const { data: pullRequests } of pullRequestsIterator) {
core.info(`Found ${pullRequests.length} pull-requests in this page.`)
for (const pullRequest of pullRequests) {
Expand Down

0 comments on commit 9455b83

Please sign in to comment.