Skip to content
Open
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
108 changes: 108 additions & 0 deletions .github/scripts/optimized post-pr-metrics.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
const fs = require("fs")
const path = require("path")

const BASELINE_FILE = path.join(".metrics-baseline", "metrics.json")
const COMMENT_TAG = "<!-- pr-metrics -->"

// ---------- helpers ----------

function readNumber(value) {
const n = Number(value)
return Number.isFinite(n) && n >= 0 ? n : null
}

function readCoverage(value) {
const n = Number(value)
return Number.isFinite(n) && n >= 0 && n <= 100 ? n : null
}

function safeReadBaseline() {
try {
if (!fs.existsSync(BASELINE_FILE)) return null
const raw = fs.readFileSync(BASELINE_FILE, "utf8")
return JSON.parse(raw)
} catch {
return null // corrupted file safe fallback
}
}

function formatBytes(bytes) {
if (bytes == null) return "—"
return `${(bytes / 1024).toFixed(1)} KB`
}

function diffText(current, baseline, formatter = v => v) {
if (current == null) return "—"
if (!baseline) return formatter(current)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Falsy check skips diff when baseline is zero

Medium Severity

The diffText function uses if (!baseline) to check for a missing baseline, but this falsiness check also matches when baseline is 0. A baseline bundle size of 0 or baseline coverage of 0% would skip the diff calculation entirely and display the value without any change indicator, as if no baseline existed. The check needs to be baseline == null to properly distinguish "no baseline" from a zero-valued baseline.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6193de5. Configure here.


const diff = current - baseline
if (diff === 0) return `${formatter(current)} (no change)`

const sign = diff > 0 ? "▲" : "▼"
return `${formatter(current)} (${sign} ${formatter(Math.abs(diff))})`
}

function buildComment({ bundle, baselineBundle, coverage, baselineCoverage }) {
const bundleRow = diffText(bundle, baselineBundle, formatBytes)
const coverageRow = diffText(coverage, baselineCoverage, v => `${v.toFixed(1)}%`)

return `${COMMENT_TAG}
## 📊 PR Metrics

| Metric | Value |
|---|---|
| Bundle size (gzip) | ${bundleRow} |
| Test coverage | ${coverageRow} |
`
}

// ---------- main function ----------

async function postPrMetrics({ github, context }) {
// Skip conditions
if (process.env.BUILD_OUTCOME !== "success") return
if (process.env.PREVIEW_READY !== "true" && process.env.PREVIEW_READY !== "false") return
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated PREVIEW_READY guard suppresses all metrics posting

Medium Severity

The PREVIEW_READY env var controls whether the Render deploy preview is ready for Lighthouse testing, yet it's used here as a top-level gate for the entire function. This script doesn't even include Lighthouse metrics — it only posts bundle size and coverage. If the preview step is skipped or doesn't output a value, PREVIEW_READY will be undefined, causing the function to silently return without posting any metrics comment at all. The original post-pr-metrics.cjs correctly uses PREVIEW_READY only for the Lighthouse section, not as a top-level guard.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6193de5. Configure here.


const bundle = readNumber(process.env.BUNDLE_GZIP)
const coverage = readCoverage(process.env.COVERAGE)

const baseline = safeReadBaseline()
const baselineBundle = baseline?.bundleSize ?? null
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Baseline bundle size bypasses readNumber validation

Low Severity

baselineBundle is read directly from the JSON file without validation, while baselineCoverage on the next line is properly validated through readCoverage. The readNumber helper exists for exactly this purpose. If the baseline file contains a non-numeric bundleSize value, diffText would compute current - baseline producing NaN, and formatBytes(NaN) would render as "NaN KB" in the PR comment.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6193de5. Configure here.

const baselineCoverage = readCoverage(baseline?.coverage)

const body = buildComment({
bundle,
baselineBundle,
coverage,
baselineCoverage,
})

const { owner, repo } = context.repo
const issue_number = context.issue.number

const comments = await github.rest.issues.listComments({
owner,
repo,
issue_number,
})

const existing = comments.data.find(c => c.body?.includes(COMMENT_TAG))

if (existing) {
await github.rest.issues.updateComment({
owner,
repo,
comment_id: existing.id,
body,
})
} else {
await github.rest.issues.createComment({
owner,
repo,
issue_number,
body,
})
}
}

module.exports = { postPrMetrics }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Production code in unreferenced file with test extension

Medium Severity

This file appears to be accidentally committed. It has a .test.js extension but contains production implementation code (not tests), its filename contains a space (optimized post-pr-metrics.test.js), and its postPrMetrics export is never imported or referenced anywhere in the codebase — the workflow still uses post-pr-metrics.cjs. The existing post-pr-metrics.test.js file contains the actual tests. This seems like a scratch/draft file that was not meant to be committed.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6193de5. Configure here.