Skip to content

Commit e656d33

Browse files
committed
tests: Use 'Filing' (and 'Result') instead of 'Finding' in tests
1 parent be47761 commit e656d33

File tree

4 files changed

+48
-31
lines changed

4 files changed

+48
-31
lines changed

.github/workflows/test.yml

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ jobs:
6161
id: cache_key
6262
shell: bash
6363
run: |
64-
echo "cache_key=$(printf 'cached_findings-%s-%s.json' "${{ github.ref_name }}" "${{ matrix.site }}" | tr -cs 'A-Za-z0-9._-' '_')" >> $GITHUB_OUTPUT
64+
echo "cache_key=$(printf 'cached_filings-%s-%s.json' "${{ matrix.site }}" "${{ github.ref_name }}" | tr -cs 'A-Za-z0-9._-' '_')" >> $GITHUB_OUTPUT
6565
6666
- name: Scan site (${{ matrix.site }})
6767
uses: ./
@@ -78,28 +78,28 @@ jobs:
7878
token: ${{ secrets.GH_TOKEN }}
7979
cache_key: ${{ steps.cache_key.outputs.cache_key }}
8080

81-
- name: Retrieve cached findings
81+
- name: Retrieve cached filings
8282
uses: ./.github/actions/gh-cache/restore
8383
with:
8484
path: ${{ steps.cache_key.outputs.cache_key }}
8585
token: ${{ secrets.GITHUB_TOKEN }}
8686

87-
- name: Add PR URLs to findings
87+
- name: Add PR URLs to filings
8888
uses: actions/github-script@v8
8989
with:
9090
github-token: ${{ secrets.GITHUB_TOKEN }}
9191
script: |
9292
const fs = require('fs');
93-
if (!process.env.FINDINGS_PATH || !fs.existsSync(process.env.FINDINGS_PATH)) {
94-
core.info("Skipping 'Add PR URLs to findings' (no cached findings).");
93+
if (!process.env.CACHE_PATH || !fs.existsSync(process.env.CACHE_PATH)) {
94+
core.info("Skipping 'Add PR URLs to filings' (no cached filings).");
9595
return;
9696
}
97-
const findings = JSON.parse(fs.readFileSync(process.env.FINDINGS_PATH, 'utf-8'));
98-
for (const finding of findings) {
99-
if (!finding.issueUrl) {
97+
const filings = JSON.parse(fs.readFileSync(process.env.CACHE_PATH, 'utf-8'));
98+
for (const filing of filings) {
99+
if (!filing?.issue.url) {
100100
continue;
101101
}
102-
const { owner, repo, issueNumber } = /https:\/\/github\.com\/(?<owner>[^/]+)\/(?<repo>[^/]+)\/issues\/(?<issueNumber>\d+)/.exec(finding.issueUrl).groups;
102+
const { owner, repo, issueNumber } = /https:\/\/github\.com\/(?<owner>[^/]+)\/(?<repo>[^/]+)\/issues\/(?<issueNumber>\d+)/.exec(filing.issue.url).groups;
103103
const query = `query($owner: String!, $repo: String!, $issueNumber: Int!) {
104104
repository(owner: $owner, name: $repo) {
105105
issue(number: $issueNumber) {
@@ -117,35 +117,35 @@ jobs:
117117
const timelineNodes = result?.repository?.issue?.timelineItems?.nodes || [];
118118
const pullRequestNode = timelineNodes.find(n => n?.source?.url || n?.subject?.url);
119119
if (pullRequestNode) {
120-
finding.pullRequestUrl = pullRequestNode.source?.url || pullRequestNode.subject?.url;
120+
filing.pullRequest = { url: pullRequestNode.source?.url || pullRequestNode.subject?.url };
121121
} else {
122-
core.info(`No pull request found for issue: ${finding.issueUrl}`);
122+
core.info(`No pull request found for issue: ${filing.issue.url}`);
123123
}
124124
}
125-
fs.writeFileSync(process.env.FINDINGS_PATH, JSON.stringify(findings));
125+
fs.writeFileSync(process.env.CACHE_PATH, JSON.stringify(filings));
126126
env:
127-
FINDINGS_PATH: ${{ steps.cache_key.outputs.cache_key }}
127+
CACHE_PATH: ${{ steps.cache_key.outputs.cache_key }}
128128

129129
- name: Validate scan results (${{ matrix.site }})
130130
run: |
131131
npm ci
132132
npm run test
133133
env:
134134
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
135-
FINDINGS_PATH: ${{ steps.cache_key.outputs.cache_key }}
135+
CACHE_PATH: ${{ steps.cache_key.outputs.cache_key }}
136136

137137
- name: Clean up issues and pull requests
138138
if: ${{ always() }}
139139
shell: bash
140140
run: |
141141
set -euo pipefail
142142
if [[ ! -f "${{ steps.cache_key.outputs.cache_key }}" ]]; then
143-
echo "Skipping 'Clean up issues and pull requests' (no cached findings)."
143+
echo "Skipping 'Clean up issues and pull requests' (no cached filings)."
144144
exit 0
145145
fi
146146
jq -r '
147147
(if type=="string" then fromjson else . end)
148-
| .[] | .issueUrl, .pullRequestUrl
148+
| .[] | .issue.url, .pullRequest.url
149149
| select(. != null)
150150
' "${{ steps.cache_key.outputs.cache_key }}" \
151151
| while read -r URL; do
@@ -162,7 +162,7 @@ jobs:
162162
env:
163163
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
164164

165-
- name: Clean up cached findings
165+
- name: Clean up cached filings
166166
if: ${{ always() }}
167167
uses: ./.github/actions/gh-cache/delete
168168
with:

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ Trigger the workflow manually or automatically based on your configuration. The
9696
| `urls` | Yes | Newline-delimited list of URLs to scan | `https://primer.style`<br>`https://primer.style/octicons` |
9797
| `repository` | Yes | Repository (with owner) for issues and PRs | `primer/primer-docs` |
9898
| `token` | Yes | PAT with write permissions (see above) | `${{ secrets.GH_TOKEN }}` |
99-
| `cache_key` | Yes | Key for caching findings across runs<br>Allowed: `A-Za-z0-9._/-` | `cached_findings-main-primer.style.json` |
99+
| `cache_key` | Yes | Key for caching findings across runs<br>Allowed: `A-Za-z0-9._/-` | `cached_filings-main-primer.style.json` |
100100
| `login_url` | No | If scanned pages require authentication, the URL of the login page | `https://github.com/login` |
101101
| `username` | No | If scanned pages require authentication, the username to use for login | `some-user` |
102102
| `password` | No | If scanned pages require authentication, the password to use for login | `correct-horse-battery-staple` |

tests/site-with-errors.test.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,23 @@
11
import type { Endpoints } from "@octokit/types"
2-
import type { Finding } from "./types.d.js";
2+
import type { Result } from "./types.d.js";
33
import fs from "node:fs";
44
import { describe, it, expect, beforeAll } from "vitest";
55
import { Octokit } from "@octokit/core";
66
import { throttling } from "@octokit/plugin-throttling";
77
const OctokitWithThrottling = Octokit.plugin(throttling);
88

99
describe("site-with-errors", () => {
10-
let findings: Finding[];
10+
let results: Result[];
1111

1212
beforeAll(() => {
13-
expect(process.env.FINDINGS_PATH).toBeDefined();
14-
expect(fs.existsSync(process.env.FINDINGS_PATH!)).toBe(true);
15-
findings = JSON.parse(fs.readFileSync(process.env.FINDINGS_PATH!, "utf-8"));
13+
expect(process.env.CACHE_PATH).toBeDefined();
14+
expect(fs.existsSync(process.env.CACHE_PATH!)).toBe(true);
15+
results = JSON.parse(fs.readFileSync(process.env.CACHE_PATH!, "utf-8"));
1616
});
1717

18-
it("cache has expected findings", () => {
19-
const actual = findings.map(({ issueUrl, pullRequestUrl, solutionLong, ...finding }) => {
18+
it("cache has expected results", () => {
19+
const actual = results.map(({ issue: { url: issueUrl }, pullRequest: { url: pullRequestUrl }, findings }) => {
20+
const { solutionLong, ...finding } = findings[0];
2021
// Check volatile fields for existence only
2122
expect(issueUrl).toBeDefined();
2223
expect(pullRequestUrl).toBeDefined();
@@ -113,8 +114,8 @@ describe("site-with-errors", () => {
113114
},
114115
}
115116
});
116-
// Fetch issues referenced in the findings file
117-
issues = await Promise.all(findings.map(async ({ issueUrl }) => {
117+
// Fetch issues referenced in the cache file
118+
issues = await Promise.all(results.map(async ({ issue: { url: issueUrl } }) => {
118119
expect(issueUrl).toBeDefined();
119120
const { owner, repo, issueNumber } =
120121
/https:\/\/github\.com\/(?<owner>[^/]+)\/(?<repo>[^/]+)\/issues\/(?<issueNumber>\d+)/.exec(issueUrl!)!.groups!;
@@ -127,7 +128,7 @@ describe("site-with-errors", () => {
127128
return issue;
128129
}));
129130
// Fetch pull requests referenced in the findings file
130-
pullRequests = await Promise.all(findings.map(async ({ pullRequestUrl }) => {
131+
pullRequests = await Promise.all(results.map(async ({ pullRequest: { url: pullRequestUrl } }) => {
131132
expect(pullRequestUrl).toBeDefined();
132133
const { owner, repo, pullNumber } =
133134
/https:\/\/github\.com\/(?<owner>[^/]+)\/(?<repo>[^/]+)\/pull\/(?<pullNumber>\d+)/.exec(pullRequestUrl!)!.groups!;

tests/types.d.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,22 @@ export type Finding = {
77
problemUrl: string;
88
solutionShort: string;
99
solutionLong?: string;
10-
issueUrl?: string;
11-
pullRequestUrl?: string;
12-
}
10+
};
11+
12+
export type Issue = {
13+
id: number;
14+
nodeId: string;
15+
url: string;
16+
title: string;
17+
state?: "open" | "reopened" | "closed";
18+
};
19+
20+
export type PullRequest = {
21+
url: string;
22+
};
23+
24+
export type Result = {
25+
findings: Finding[];
26+
issue: Issue;
27+
pullRequest: PullRequest;
28+
};

0 commit comments

Comments
 (0)