Skip to content

Commit 55fbd88

Browse files
authored
Merge pull request Expensify#67706 from ikevin127/ikevin127-coverageUpdates
2 parents a8175e0 + 9955e6e commit 55fbd88

6 files changed

Lines changed: 129 additions & 65 deletions

File tree

.github/actions/javascript/postTestCoverageComment/index.js

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11633,18 +11633,32 @@ function generateCoverageData(coverage, changedFiles, baseCoverage) {
1163311633
// If not found, try to find by matching the end of the path
1163411634
if (!fileCoverage) {
1163511635
const coverageKeys = Object.keys(coverage).filter((key) => key !== 'total');
11636-
const matchingKey = coverageKeys.find((key) => key.endsWith(file) || key.endsWith(file.replace(/^src\//, '')));
11636+
// Try multiple matching strategies
11637+
const normalizedFile = file.startsWith('src/') ? file : `src/${file}`;
11638+
const matchingKey = coverageKeys.find((key) => {
11639+
const normalizedKey = key.replace(/\\/g, '/'); // Handle Windows paths
11640+
return normalizedKey === normalizedFile || normalizedKey === file || (normalizedKey.includes(file) && normalizedKey.split('/').pop() === file.split('/').pop());
11641+
});
1163711642
if (matchingKey) {
1163811643
fileCoverage = coverage[matchingKey];
1163911644
}
1164011645
}
11646+
// For files without coverage data, still include them with 0% coverage if they should have coverage
11647+
const shouldHaveCoverage = !file.endsWith('.d.ts') && !file.includes('/types.ts') && !file.match(/\.(stories|spec|test)\.(ts|tsx|js|jsx)$/);
11648+
const noCoverageFiles = shouldHaveCoverage
11649+
? {
11650+
file,
11651+
coverage: 0,
11652+
lines: '0/0',
11653+
}
11654+
: null;
1164111655
return fileCoverage
1164211656
? {
1164311657
file,
1164411658
coverage: fileCoverage.lines.pct,
1164511659
lines: `${fileCoverage.lines.covered}/${fileCoverage.lines.total}`,
1164611660
}
11647-
: null;
11661+
: noCoverageFiles;
1164811662
})
1164911663
.filter((item) => item !== null)
1165011664
.sort((a, b) => b.coverage - a.coverage);
@@ -11702,7 +11716,7 @@ function getNestedValue(obj, nestPath) {
1170211716
*/
1170311717
function getCoverageStatus(current, baseline) {
1170411718
const diff = current - (baseline ?? 0);
11705-
if (!baseline || Math.abs(diff) < 0.01) {
11719+
if (baseline === undefined || baseline === null || Math.abs(diff) < 0.01) {
1170611720
return { emoji: '', status: '', diff: 0 };
1170711721
}
1170811722
if (diff > 0) {
@@ -11715,7 +11729,7 @@ function getCoverageStatus(current, baseline) {
1171511729
*/
1171611730
function calculateChange(current, baseline) {
1171711731
const diff = current - baseline;
11718-
if (!baseline || Math.abs(diff) < 0.01) {
11732+
if (baseline === undefined || baseline === null || Math.abs(diff) < 0.01) {
1171911733
return '0.0%';
1172011734
}
1172111735
// Negative sign is already handled by the diff calculation
@@ -11752,7 +11766,8 @@ function generateCoverageSection(coverageData, artifactUrl, workflowRunId) {
1175211766
{{#hasChangedFiles}}
1175311767
| File | Coverage | Lines |
1175411768
|------|----------|-------|
11755-
{{#changedFiles}}| \`{{displayFile}}\` | {{coverage}}% | {{lines}} |{{/changedFiles}}
11769+
{{#changedFiles}}| \`{{displayFile}}\` | {{coverage}}% | {{lines}} |
11770+
{{/changedFiles}}
1175611771
{{/hasChangedFiles}}
1175711772
{{^hasChangedFiles}}*No coverage changed files found.*{{/hasChangedFiles}}
1175811773
**🔄 Overall Coverage Summary**

.github/actions/javascript/postTestCoverageComment/postTestCoverageComment.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,18 +142,37 @@ function generateCoverageData(coverage: CoverageSummary, changedFiles: string[],
142142
// If not found, try to find by matching the end of the path
143143
if (!fileCoverage) {
144144
const coverageKeys = Object.keys(coverage).filter((key) => key !== 'total');
145-
const matchingKey = coverageKeys.find((key) => key.endsWith(file) || key.endsWith(file.replace(/^src\//, '')));
145+
146+
// Try multiple matching strategies
147+
const normalizedFile = file.startsWith('src/') ? file : `src/${file}`;
148+
const matchingKey = coverageKeys.find((key) => {
149+
const normalizedKey = key.replace(/\\/g, '/'); // Handle Windows paths
150+
return normalizedKey === normalizedFile || normalizedKey === file || (normalizedKey.includes(file) && normalizedKey.split('/').pop() === file.split('/').pop());
151+
});
152+
146153
if (matchingKey) {
147154
fileCoverage = coverage[matchingKey];
148155
}
149156
}
157+
158+
// For files without coverage data, still include them with 0% coverage if they should have coverage
159+
const shouldHaveCoverage = !file.endsWith('.d.ts') && !file.includes('/types.ts') && !file.match(/\.(stories|spec|test)\.(ts|tsx|js|jsx)$/);
160+
161+
const noCoverageFiles = shouldHaveCoverage
162+
? {
163+
file,
164+
coverage: 0,
165+
lines: '0/0',
166+
}
167+
: null;
168+
150169
return fileCoverage
151170
? {
152171
file,
153172
coverage: fileCoverage.lines.pct,
154173
lines: `${fileCoverage.lines.covered}/${fileCoverage.lines.total}`,
155174
}
156-
: null;
175+
: noCoverageFiles;
157176
})
158177
.filter((item): item is NonNullable<typeof item> => item !== null)
159178
.sort((a, b) => b.coverage - a.coverage);
@@ -221,7 +240,7 @@ function getNestedValue(obj: TemplateData, nestPath: string) {
221240
*/
222241
function getCoverageStatus(current: number, baseline?: number): {emoji: string; status: string; diff: number} {
223242
const diff = current - (baseline ?? 0);
224-
if (!baseline || Math.abs(diff) < 0.01) {
243+
if (baseline === undefined || baseline === null || Math.abs(diff) < 0.01) {
225244
return {emoji: '', status: '', diff: 0};
226245
}
227246
if (diff > 0) {
@@ -235,7 +254,7 @@ function getCoverageStatus(current: number, baseline?: number): {emoji: string;
235254
*/
236255
function calculateChange(current: number, baseline: number): string {
237256
const diff = current - baseline;
238-
if (!baseline || Math.abs(diff) < 0.01) {
257+
if (baseline === undefined || baseline === null || Math.abs(diff) < 0.01) {
239258
return '0.0%';
240259
}
241260
// Negative sign is already handled by the diff calculation
@@ -274,7 +293,8 @@ function generateCoverageSection(coverageData: CoverageData, artifactUrl: string
274293
{{#hasChangedFiles}}
275294
| File | Coverage | Lines |
276295
|------|----------|-------|
277-
{{#changedFiles}}| \`{{displayFile}}\` | {{coverage}}% | {{lines}} |{{/changedFiles}}
296+
{{#changedFiles}}| \`{{displayFile}}\` | {{coverage}}% | {{lines}} |
297+
{{/changedFiles}}
278298
{{/hasChangedFiles}}
279299
{{^hasChangedFiles}}*No coverage changed files found.*{{/hasChangedFiles}}
280300
**🔄 Overall Coverage Summary**

.github/actions/javascript/waitforJestTests/index.js

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11579,32 +11579,38 @@ async function waitForJestTests() {
1157911579
const maxWaitTime = 30 * 60 * 1000; // 30 minutes
1158011580
const pollInterval = 10 * 1000; // 10 seconds
1158111581
const startTime = Date.now();
11582-
// Get PR number and SHA (backwards compatible with manual dispatch)
11582+
// Get PR number from either context or manual input
1158311583
let prNumber;
11584-
let headSha;
11584+
const inputsPRNumber = github_1.context.payload.inputs?.pr_number;
1158511585
if (github_1.context.payload.pull_request?.number) {
11586+
// Regular PR context
1158611587
prNumber = github_1.context.payload.pull_request.number;
11587-
headSha = github_1.context.sha;
11588-
console.log(`Using PR context - PR #${prNumber}, SHA: ${headSha}`);
11588+
console.log(`Using PR context - PR #${prNumber}`);
11589+
}
11590+
else if (inputsPRNumber) {
11591+
// Manual workflow dispatch
11592+
prNumber = Number(inputsPRNumber);
11593+
console.log(`Using manual dispatch - PR #${prNumber}`);
1158911594
}
1159011595
else {
11591-
prNumber = Number(github_1.context.payload.inputs?.pr_number ?? '0');
11592-
if (!prNumber) {
11593-
core.setFailed('PR number is required when pull_request context is not available.');
11594-
return;
11595-
}
11596-
// Get PR details to find the head SHA
11597-
console.log(`Using workflow inputs - Getting details for PR #${prNumber}`);
11598-
const prResponse = await GithubUtils_1.default.octokit.pulls.get({
11599-
owner: github_1.context.repo.owner,
11600-
repo: github_1.context.repo.repo,
11601-
pull_number: prNumber,
11602-
});
11603-
headSha = prResponse.data.head.sha;
11604-
console.log(`PR #${prNumber} head SHA: ${headSha}`);
11596+
core.setFailed('PR number is required but not available in context or inputs.');
11597+
return;
1160511598
}
11606-
console.log(`Looking for test workflow runs for PR #${github_1.context.payload.pull_request?.number}`);
11607-
console.log(`Head SHA: ${github_1.context.sha}`);
11599+
if (!prNumber) {
11600+
core.setFailed('Invalid PR number.');
11601+
return;
11602+
}
11603+
// Single API call to get PR details and extract headSha
11604+
console.log(`Getting PR details for PR #${prNumber}...`);
11605+
const prResponse = await GithubUtils_1.default.octokit.pulls.get({
11606+
owner: github_1.context.repo.owner,
11607+
repo: github_1.context.repo.repo,
11608+
pull_number: prNumber,
11609+
});
11610+
const headSha = prResponse.data.head.sha;
11611+
console.log(`PR #${prNumber} head SHA: ${headSha}`);
11612+
console.log(`Looking for test workflow runs for PR #${prNumber}`);
11613+
console.log(`Head SHA: ${headSha}`);
1160811614
while (Date.now() - startTime < maxWaitTime) {
1160911615
// Get all recent workflow runs for this repo
1161011616
const workflows = await GithubUtils_1.default.listWorkflowRunsForRepo({
@@ -11617,7 +11623,7 @@ async function waitForJestTests() {
1161711623
// Check if it's the Jest Unit Tests workflow specifically
1161811624
const isTestWorkflow = run.name === CONST_1.default.TEST_WORKFLOW_NAME || run.path === CONST_1.default.TEST_WORKFLOW_PATH;
1161911625
// Check if it's for our PR's head SHA
11620-
const matchesSHA = run.head_sha === github_1.context.sha;
11626+
const matchesSHA = run.head_sha === headSha;
1162111627
// For manual dispatch or when no PR context, be more flexible with event types
1162211628
// For regular PR events, maintain existing strict logic
1162311629
const isValidEvent = github_1.context.payload.pull_request
@@ -11646,7 +11652,7 @@ async function waitForJestTests() {
1164611652
});
1164711653
const inProgressTestRuns = inProgressWorkflows.data.workflow_runs.filter((run) => {
1164811654
const isTestWorkflow = run.name === CONST_1.default.TEST_WORKFLOW_NAME || run.path === CONST_1.default.TEST_WORKFLOW_PATH;
11649-
const matchesSHA = run.head_sha === github_1.context.sha;
11655+
const matchesSHA = run.head_sha === headSha;
1165011656
const isValidEvent = github_1.context.payload.pull_request
1165111657
? run.event === CONST_1.default.RUN_EVENT.PULL_REQUEST || run.event === CONST_1.default.RUN_EVENT.PULL_REQUEST_TARGET
1165211658
: run.event === CONST_1.default.RUN_EVENT.PULL_REQUEST || run.event === CONST_1.default.RUN_EVENT.PULL_REQUEST_TARGET || run.event === CONST_1.default.RUN_EVENT.PUSH;
@@ -11660,7 +11666,7 @@ async function waitForJestTests() {
1166011666
console.log('No matching test workflow runs found, checking if tests are required...');
1166111667
// Check if there might be no test workflow triggered
1166211668
// This could happen if the PR doesn't have testable changes
11663-
const allRecentRuns = workflows.data.workflow_runs.filter((run) => run.head_sha === github_1.context.sha);
11669+
const allRecentRuns = workflows.data.workflow_runs.filter((run) => run.head_sha === headSha);
1166411670
console.log(`Found ${allRecentRuns.length} workflow runs for this SHA.`);
1166511671
// Assume tests passed if no workflow runs exist
1166611672
if (allRecentRuns.length === 0) {

.github/actions/javascript/waitforJestTests/waitForJestTests.ts

Lines changed: 29 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,36 +9,41 @@ async function waitForJestTests(): Promise<void> {
99
const pollInterval = 10 * 1000; // 10 seconds
1010
const startTime = Date.now();
1111

12-
// Get PR number and SHA (backwards compatible with manual dispatch)
12+
// Get PR number from either context or manual input
1313
let prNumber: number;
14-
let headSha: string;
14+
const inputsPRNumber = (context.payload.inputs as {pr_number: number})?.pr_number;
1515

1616
if (context.payload.pull_request?.number) {
17+
// Regular PR context
1718
prNumber = context.payload.pull_request.number;
18-
headSha = context.sha;
19-
console.log(`Using PR context - PR #${prNumber}, SHA: ${headSha}`);
19+
console.log(`Using PR context - PR #${prNumber}`);
20+
} else if (inputsPRNumber) {
21+
// Manual workflow dispatch
22+
prNumber = Number(inputsPRNumber);
23+
console.log(`Using manual dispatch - PR #${prNumber}`);
2024
} else {
21-
prNumber = Number((context.payload.inputs as Record<string, {pr_number: number}>)?.pr_number ?? '0');
25+
core.setFailed('PR number is required but not available in context or inputs.');
26+
return;
27+
}
2228

23-
if (!prNumber) {
24-
core.setFailed('PR number is required when pull_request context is not available.');
25-
return;
26-
}
29+
if (!prNumber) {
30+
core.setFailed('Invalid PR number.');
31+
return;
32+
}
2733

28-
// Get PR details to find the head SHA
29-
console.log(`Using workflow inputs - Getting details for PR #${prNumber}`);
30-
const prResponse = await GithubUtils.octokit.pulls.get({
31-
owner: context.repo.owner,
32-
repo: context.repo.repo,
33-
pull_number: prNumber,
34-
});
34+
// Single API call to get PR details and extract headSha
35+
console.log(`Getting PR details for PR #${prNumber}...`);
36+
const prResponse = await GithubUtils.octokit.pulls.get({
37+
owner: context.repo.owner,
38+
repo: context.repo.repo,
39+
pull_number: prNumber,
40+
});
3541

36-
headSha = prResponse.data.head.sha;
37-
console.log(`PR #${prNumber} head SHA: ${headSha}`);
38-
}
42+
const headSha = prResponse.data.head.sha;
43+
console.log(`PR #${prNumber} head SHA: ${headSha}`);
3944

40-
console.log(`Looking for test workflow runs for PR #${context.payload.pull_request?.number}`);
41-
console.log(`Head SHA: ${context.sha}`);
45+
console.log(`Looking for test workflow runs for PR #${prNumber}`);
46+
console.log(`Head SHA: ${headSha}`);
4247

4348
while (Date.now() - startTime < maxWaitTime) {
4449
// Get all recent workflow runs for this repo
@@ -54,7 +59,7 @@ async function waitForJestTests(): Promise<void> {
5459
// Check if it's the Jest Unit Tests workflow specifically
5560
const isTestWorkflow = run.name === CONST.TEST_WORKFLOW_NAME || run.path === CONST.TEST_WORKFLOW_PATH;
5661
// Check if it's for our PR's head SHA
57-
const matchesSHA = run.head_sha === context.sha;
62+
const matchesSHA = run.head_sha === headSha;
5863
// For manual dispatch or when no PR context, be more flexible with event types
5964
// For regular PR events, maintain existing strict logic
6065
const isValidEvent = context.payload.pull_request
@@ -87,7 +92,7 @@ async function waitForJestTests(): Promise<void> {
8792

8893
const inProgressTestRuns = inProgressWorkflows.data.workflow_runs.filter((run) => {
8994
const isTestWorkflow = run.name === CONST.TEST_WORKFLOW_NAME || run.path === CONST.TEST_WORKFLOW_PATH;
90-
const matchesSHA = run.head_sha === context.sha;
95+
const matchesSHA = run.head_sha === headSha;
9196
const isValidEvent = context.payload.pull_request
9297
? run.event === CONST.RUN_EVENT.PULL_REQUEST || run.event === CONST.RUN_EVENT.PULL_REQUEST_TARGET
9398
: run.event === CONST.RUN_EVENT.PULL_REQUEST || run.event === CONST.RUN_EVENT.PULL_REQUEST_TARGET || run.event === CONST.RUN_EVENT.PUSH;
@@ -102,7 +107,7 @@ async function waitForJestTests(): Promise<void> {
102107
console.log('No matching test workflow runs found, checking if tests are required...');
103108
// Check if there might be no test workflow triggered
104109
// This could happen if the PR doesn't have testable changes
105-
const allRecentRuns = workflows.data.workflow_runs.filter((run) => run.head_sha === context.sha);
110+
const allRecentRuns = workflows.data.workflow_runs.filter((run) => run.head_sha === headSha);
106111
console.log(`Found ${allRecentRuns.length} workflow runs for this SHA.`);
107112

108113
// Assume tests passed if no workflow runs exist

.github/scripts/checkCoverageChanges.sh

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ set -euo pipefail
66
# -----------------------------
77
# The PR branch comes from either the base repo or a fork.
88
# We need the base repository's main branch for comparison.
9-
git remote add upstream "https://github.com/${GITHUB_REPOSITORY}.git" 2>/dev/null || true
9+
git remote add upstream "https://github.com/Expensify/App.git" 2>/dev/null || true
1010

1111
# -----------------------------
1212
# 2. Attempt shallow fetch first
@@ -24,17 +24,29 @@ git fetch upstream main --depth=$DEPTH
2424
if ! git merge-base upstream/main HEAD >/dev/null 2>&1; then
2525
echo "No merge base found with depth=$DEPTH, fetching full history..."
2626

27-
# Try unshallowing the entire repo (pulls full commit history).
28-
# If already full, this will be a no-op.
29-
git fetch --unshallow upstream || git fetch upstream main
27+
if git rev-parse --is-shallow-repository > /dev/null 2>&1 && [ "$(git rev-parse --is-shallow-repository)" = "true" ]; then
28+
echo "Repository is shallow, unshallowing..."
29+
git fetch --unshallow upstream || git fetch --unshallow || true
30+
fi
31+
git fetch upstream main --depth=1000 || git fetch upstream main || true
3032
fi
3133

3234
# -----------------------------
3335
# 4. Define the diff range
3436
# -----------------------------
3537
# Using three-dot notation (A...B) shows only the commits in HEAD
3638
# that aren't in upstream/main (i.e. just the PR changes).
37-
DIFF_RANGE="upstream/main...HEAD"
39+
if git rev-parse upstream/main > /dev/null 2>&1; then
40+
DIFF_RANGE="upstream/main...HEAD"
41+
echo "Using upstream/main as base for comparison"
42+
elif git rev-parse origin/main > /dev/null 2>&1; then
43+
DIFF_RANGE="origin/main...HEAD"
44+
echo "Fallback: Using origin/main as base for comparison"
45+
else
46+
DIFF_RANGE="HEAD~10...HEAD"
47+
echo "Fallback: Using HEAD~10 as base for comparison"
48+
fi
49+
echo "Diff range: $DIFF_RANGE"
3850

3951
# -----------------------------
4052
# 5. Collect changed src/ files

.github/scripts/generateBaselineCoverage.sh

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,15 @@
44
# Save current coverage first (from PR branch)
55
mv coverage pr-coverage
66

7-
# Fetch and checkout main branch files
8-
git fetch origin main
9-
git checkout origin/main -- .
7+
# Copy changed_files.txt to a backup
8+
cp changed_files.txt changed_files_backup.txt
9+
10+
# Fetch and checkout upstream (main) branch files
11+
git fetch upstream main
12+
git checkout upstream/main -- .
13+
14+
# Restore the backup to changed_files.txt
15+
cp changed_files_backup.txt changed_files.txt
1016

1117
# Install dependencies
1218
npm install

0 commit comments

Comments
 (0)