From 48345a0f0584d4cf57086f5e1d6d05c047d3a62c Mon Sep 17 00:00:00 2001 From: Rodolphe Stoclin Date: Fri, 23 Jan 2026 21:58:16 +0100 Subject: [PATCH 1/5] feat(action): add base branch size comparison in PR comments --- packages/action/__tests__/compare.test.ts | 221 +++++++++++++++++++++ packages/action/src/compare.ts | 228 ++++++++++++++++++++++ packages/action/src/index.ts | 7 +- packages/action/src/reporters/comment.ts | 40 ++-- 4 files changed, 483 insertions(+), 13 deletions(-) create mode 100644 packages/action/__tests__/compare.test.ts create mode 100644 packages/action/src/compare.ts diff --git a/packages/action/__tests__/compare.test.ts b/packages/action/__tests__/compare.test.ts new file mode 100644 index 000000000..809a7c3c0 --- /dev/null +++ b/packages/action/__tests__/compare.test.ts @@ -0,0 +1,221 @@ +/*! node-minify action tests - MIT Licensed */ + +import { describe, expect, test } from "vitest"; +import { + calculateTotalChange, + formatChange, + hasIncrease, +} from "../src/compare.ts"; +import type { ComparisonResult } from "../src/types.ts"; + +describe("formatChange", () => { + test("formats size increase with warning emoji", () => { + const comparison: ComparisonResult = { + file: "app.min.js", + baseSize: 1000, + currentSize: 1050, + change: 5, + isNew: false, + }; + expect(formatChange(comparison)).toBe("+5.0% ⚠️"); + }); + + test("formats size decrease with success emoji", () => { + const comparison: ComparisonResult = { + file: "app.min.js", + baseSize: 1000, + currentSize: 900, + change: -10, + isNew: false, + }; + expect(formatChange(comparison)).toBe("-10.0% ✅"); + }); + + test("formats no change", () => { + const comparison: ComparisonResult = { + file: "app.min.js", + baseSize: 1000, + currentSize: 1000, + change: 0, + isNew: false, + }; + expect(formatChange(comparison)).toBe("+0.0% ✅"); + }); + + test("returns 'new' for new files", () => { + const comparison: ComparisonResult = { + file: "app.min.js", + baseSize: null, + currentSize: 1000, + change: null, + isNew: true, + }; + expect(formatChange(comparison)).toBe("new"); + }); + + test("returns 'new' when change is null", () => { + const comparison: ComparisonResult = { + file: "app.min.js", + baseSize: null, + currentSize: 500, + change: null, + isNew: false, + }; + expect(formatChange(comparison)).toBe("new"); + }); +}); + +describe("hasIncrease", () => { + test("detects size increase", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: 1000, + currentSize: 900, + change: -10, + isNew: false, + }, + { + file: "b.js", + baseSize: 1000, + currentSize: 1100, + change: 10, + isNew: false, + }, + ]; + expect(hasIncrease(comparisons)).toBe(true); + }); + + test("returns false when all files decreased", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: 1000, + currentSize: 900, + change: -10, + isNew: false, + }, + { + file: "b.js", + baseSize: 1000, + currentSize: 950, + change: -5, + isNew: false, + }, + ]; + expect(hasIncrease(comparisons)).toBe(false); + }); + + test("ignores new files", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: null, + currentSize: 1000, + change: null, + isNew: true, + }, + ]; + expect(hasIncrease(comparisons)).toBe(false); + }); + + test("returns false for empty array", () => { + expect(hasIncrease([])).toBe(false); + }); +}); + +describe("calculateTotalChange", () => { + test("calculates total change across files", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: 1000, + currentSize: 800, + change: -20, + isNew: false, + }, + { + file: "b.js", + baseSize: 1000, + currentSize: 900, + change: -10, + isNew: false, + }, + ]; + + const result = calculateTotalChange(comparisons); + expect(result.totalBaseSize).toBe(2000); + expect(result.totalCurrentSize).toBe(1700); + expect(result.totalChangePercent).toBe(-15); + }); + + test("excludes new files from base calculation", () => { + const comparisons: ComparisonResult[] = [ + { + file: "existing.js", + baseSize: 1000, + currentSize: 800, + change: -20, + isNew: false, + }, + { + file: "new.js", + baseSize: null, + currentSize: 500, + change: null, + isNew: true, + }, + ]; + + const result = calculateTotalChange(comparisons); + expect(result.totalBaseSize).toBe(1000); + expect(result.totalCurrentSize).toBe(800); + expect(result.totalChangePercent).toBe(-20); + }); + + test("handles all new files", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: null, + currentSize: 500, + change: null, + isNew: true, + }, + { + file: "b.js", + baseSize: null, + currentSize: 300, + change: null, + isNew: true, + }, + ]; + + const result = calculateTotalChange(comparisons); + expect(result.totalBaseSize).toBe(0); + expect(result.totalCurrentSize).toBe(800); + expect(result.totalChangePercent).toBeNull(); + }); + + test("handles empty array", () => { + const result = calculateTotalChange([]); + expect(result.totalBaseSize).toBe(0); + expect(result.totalCurrentSize).toBe(0); + expect(result.totalChangePercent).toBeNull(); + }); + + test("handles size increase", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: 1000, + currentSize: 1200, + change: 20, + isNew: false, + }, + ]; + + const result = calculateTotalChange(comparisons); + expect(result.totalChangePercent).toBe(20); + }); +}); diff --git a/packages/action/src/compare.ts b/packages/action/src/compare.ts new file mode 100644 index 000000000..8d200c84e --- /dev/null +++ b/packages/action/src/compare.ts @@ -0,0 +1,228 @@ +/*! + * node-minify + * Copyright (c) 2011-2026 Rodolphe Stoclin + * MIT Licensed + */ + +import { info, warning } from "@actions/core"; +import { context, getOctokit } from "@actions/github"; +import type { ComparisonResult, MinifyResult } from "./types.ts"; + +/** + * Compares minified file sizes against the base branch for pull requests. + * + * Fetches the minified output files from the base branch (if they exist) and + * computes the size difference. This allows PR comments to show before/after + * comparisons. + * + * @param result - The minification result containing file information + * @param githubToken - GitHub API token for fetching base branch files + * @returns Array of comparison results for each file, or empty array if not a PR or no token + */ +export async function compareWithBase( + result: MinifyResult, + githubToken: string | undefined +): Promise { + if (!githubToken) { + warning("No GitHub token provided, skipping base branch comparison"); + return []; + } + + const pullRequest = context.payload.pull_request; + if (!pullRequest) { + info("Not a pull request, skipping base branch comparison"); + return []; + } + + const baseBranch = pullRequest.base.ref as string; + const octokit = getOctokit(githubToken); + const { owner, repo } = context.repo; + + const results: ComparisonResult[] = []; + + for (const fileResult of result.files) { + const comparison = await compareFile( + octokit, + owner, + repo, + baseBranch, + fileResult.file, + fileResult.minifiedSize + ); + results.push(comparison); + } + + return results; +} + +/** + * Compares a single file against the base branch version. + * + * @param octokit - Authenticated Octokit instance + * @param owner - Repository owner + * @param repo - Repository name + * @param baseBranch - Base branch ref (e.g., "main") + * @param filePath - Path to the file to compare + * @param currentSize - Current minified size in bytes + * @returns Comparison result with base size (null if file is new) and change percentage + */ +async function compareFile( + octokit: ReturnType, + owner: string, + repo: string, + baseBranch: string, + filePath: string, + currentSize: number +): Promise { + try { + const { data } = await octokit.rest.repos.getContent({ + owner, + repo, + path: filePath, + ref: baseBranch, + }); + + // getContent returns different shapes depending on the path + // For a single file, it returns an object with content + if (Array.isArray(data)) { + // Path is a directory, not a file + return { + file: filePath, + baseSize: null, + currentSize, + change: null, + isNew: true, + }; + } + + if (data.type !== "file" || !("size" in data)) { + // Not a regular file (could be a symlink or submodule) + return { + file: filePath, + baseSize: null, + currentSize, + change: null, + isNew: true, + }; + } + + const baseSize = data.size; + const change = + baseSize > 0 + ? ((currentSize - baseSize) / baseSize) * 100 + : currentSize > 0 + ? 100 + : 0; + + return { + file: filePath, + baseSize, + currentSize, + change, + isNew: false, + }; + } catch (error) { + // File doesn't exist in base branch (new file) or other error + if ( + error instanceof Error && + "status" in error && + (error as { status: number }).status === 404 + ) { + return { + file: filePath, + baseSize: null, + currentSize, + change: null, + isNew: true, + }; + } + + // Log unexpected errors but don't fail the action + warning( + `Failed to fetch base branch version of ${filePath}: ${error instanceof Error ? error.message : String(error)}` + ); + + return { + file: filePath, + baseSize: null, + currentSize, + change: null, + isNew: true, + }; + } +} + +/** + * Formats a comparison result for display (e.g., in PR comments). + * + * @param comparison - The comparison result to format + * @returns Formatted string like "+2.5% ⚠️", "-1.6% ✅", or "new" for new files + */ +export function formatChange(comparison: ComparisonResult): string { + if (comparison.isNew || comparison.change === null) { + return "new"; + } + + const sign = comparison.change >= 0 ? "+" : ""; + const emoji = comparison.change > 0 ? "⚠️" : "✅"; + + return `${sign}${comparison.change.toFixed(1)}% ${emoji}`; +} + +/** + * Checks if any comparison shows a size increase. + * + * @param comparisons - Array of comparison results + * @returns True if any file increased in size + */ +export function hasIncrease(comparisons: ComparisonResult[]): boolean { + return comparisons.some( + (c) => !c.isNew && c.change !== null && c.change > 0 + ); +} + +/** + * Calculates the total size change across all compared files. + * + * @param comparisons - Array of comparison results + * @returns Object with totalBaseSize, totalCurrentSize, and totalChangePercent (null if no comparable files) + */ +export function calculateTotalChange(comparisons: ComparisonResult[]): { + totalBaseSize: number; + totalCurrentSize: number; + totalChangePercent: number | null; +} { + const comparable = comparisons.filter( + (c) => !c.isNew && c.baseSize !== null + ); + + if (comparable.length === 0) { + return { + totalBaseSize: 0, + totalCurrentSize: comparisons.reduce( + (sum, c) => sum + c.currentSize, + 0 + ), + totalChangePercent: null, + }; + } + + const totalBaseSize = comparable.reduce( + (sum, c) => sum + (c.baseSize ?? 0), + 0 + ); + const totalCurrentSize = comparable.reduce( + (sum, c) => sum + c.currentSize, + 0 + ); + const totalChangePercent = + totalBaseSize > 0 + ? ((totalCurrentSize - totalBaseSize) / totalBaseSize) * 100 + : 0; + + return { + totalBaseSize, + totalCurrentSize, + totalChangePercent, + }; +} diff --git a/packages/action/src/index.ts b/packages/action/src/index.ts index 057edbbc7..02f706d76 100644 --- a/packages/action/src/index.ts +++ b/packages/action/src/index.ts @@ -8,6 +8,7 @@ import { info, setFailed } from "@actions/core"; import { context } from "@actions/github"; import { runBenchmark } from "./benchmark.ts"; import { checkThresholds } from "./checks.ts"; +import { compareWithBase } from "./compare.ts"; import { parseInputs, validateCompressor } from "./inputs.ts"; import { runMinification } from "./minify.ts"; import { setBenchmarkOutputs, setMinifyOutputs } from "./outputs.ts"; @@ -42,7 +43,11 @@ async function run(): Promise { } if (inputs.reportPRComment && context.payload.pull_request) { - await postPRComment(result, inputs.githubToken); + const comparisons = await compareWithBase( + result, + inputs.githubToken + ); + await postPRComment(result, inputs.githubToken, comparisons); } if (inputs.reportAnnotations) { diff --git a/packages/action/src/reporters/comment.ts b/packages/action/src/reporters/comment.ts index b244e2f79..4f1cd2193 100644 --- a/packages/action/src/reporters/comment.ts +++ b/packages/action/src/reporters/comment.ts @@ -7,7 +7,8 @@ import { info, warning } from "@actions/core"; import { context, getOctokit } from "@actions/github"; import { prettyBytes } from "@node-minify/utils"; -import type { MinifyResult } from "../types.ts"; +import { formatChange } from "../compare.ts"; +import type { ComparisonResult, MinifyResult } from "../types.ts"; const COMMENT_TAG = ""; @@ -16,10 +17,12 @@ const COMMENT_TAG = ""; * * @param result - Minification results containing per-file metrics, totals, compressor name, and execution time * @param githubToken - GitHub API token used to authenticate requests; when `undefined`, the function skips posting + * @param comparisons - Optional comparison results against base branch for showing size changes */ export async function postPRComment( result: MinifyResult, - githubToken: string | undefined + githubToken: string | undefined, + comparisons: ComparisonResult[] = [] ): Promise { if (!githubToken) { warning("No GitHub token provided, skipping PR comment"); @@ -36,7 +39,7 @@ export async function postPRComment( const octokit = getOctokit(githubToken); const { owner, repo } = context.repo; - const body = generateCommentBody(result); + const body = generateCommentBody(result, comparisons); const comments = await octokit.paginate( octokit.rest.issues.listComments, @@ -79,22 +82,35 @@ export async function postPRComment( /** * Builds the Markdown body for the node-minify PR comment, including a per-file table, totals, and a configuration section. * - * @param result - Minification results used to populate the report (expects `files`, `totalOriginalSize`, `totalMinifiedSize`, `totalReduction`, `compressor`, and `totalTimeMs`) - * @returns The Markdown string for the PR comment, beginning with `COMMENT_TAG` and containing the file table, total summary, and configuration details + * @param result - Minification results used to populate the report + * @param comparisons - Comparison results against base branch + * @returns The Markdown string for the PR comment */ -function generateCommentBody(result: MinifyResult): string { +function generateCommentBody( + result: MinifyResult, + comparisons: ComparisonResult[] +): string { + const hasComparisons = comparisons.length > 0; + const comparisonMap = new Map(comparisons.map((c) => [c.file, c])); + const filesTable = result.files - .map( - (f) => - `| \`${f.file}\` | ${prettyBytes(f.originalSize)} | ${prettyBytes(f.minifiedSize)} | ${f.reduction.toFixed(1)}% |` - ) + .map((f) => { + const comparison = comparisonMap.get(f.file); + const changeCol = hasComparisons + ? ` ${comparison ? formatChange(comparison) : "-"} |` + : ""; + return `| \`${f.file}\` | ${prettyBytes(f.originalSize)} | ${prettyBytes(f.minifiedSize)} | ${f.reduction.toFixed(1)}% |${changeCol}`; + }) .join("\n"); + const headerChange = hasComparisons ? " vs Base |" : ""; + const headerSep = hasComparisons ? "---------|" : ""; + return `${COMMENT_TAG} ## 📦 node-minify Report -| File | Original | Minified | Reduction | -|------|----------|----------|-----------| +| File | Original | Minified | Reduction |${headerChange} +|------|----------|----------|-----------|${headerSep} ${filesTable} **Total:** ${prettyBytes(result.totalOriginalSize)} → ${prettyBytes(result.totalMinifiedSize)} (${result.totalReduction.toFixed(1)}% reduction) From 1111fcee17fd95064cf07e7ead403f7aa7b992de Mon Sep 17 00:00:00 2001 From: Rodolphe Stoclin Date: Fri, 23 Jan 2026 22:07:09 +0100 Subject: [PATCH 2/5] test(action): add coverage for compareWithBase function --- packages/action/__tests__/compare.test.ts | 250 +++++++++++++++++++++- 1 file changed, 248 insertions(+), 2 deletions(-) diff --git a/packages/action/__tests__/compare.test.ts b/packages/action/__tests__/compare.test.ts index 809a7c3c0..f0ae9d7ed 100644 --- a/packages/action/__tests__/compare.test.ts +++ b/packages/action/__tests__/compare.test.ts @@ -1,12 +1,29 @@ /*! node-minify action tests - MIT Licensed */ -import { describe, expect, test } from "vitest"; +import { beforeEach, describe, expect, test, vi } from "vitest"; + +vi.mock("@actions/core", () => ({ + info: vi.fn(), + warning: vi.fn(), +})); + +vi.mock("@actions/github", () => ({ + context: { + payload: {}, + repo: { owner: "test-owner", repo: "test-repo" }, + }, + getOctokit: vi.fn(), +})); + +import { info, warning } from "@actions/core"; +import { context, getOctokit } from "@actions/github"; import { calculateTotalChange, + compareWithBase, formatChange, hasIncrease, } from "../src/compare.ts"; -import type { ComparisonResult } from "../src/types.ts"; +import type { ComparisonResult, MinifyResult } from "../src/types.ts"; describe("formatChange", () => { test("formats size increase with warning emoji", () => { @@ -219,3 +236,232 @@ describe("calculateTotalChange", () => { expect(result.totalChangePercent).toBe(20); }); }); + +describe("compareWithBase", () => { + const mockResult: MinifyResult = { + files: [ + { + file: "dist/app.min.js", + originalSize: 10000, + minifiedSize: 3000, + reduction: 70, + timeMs: 50, + }, + ], + compressor: "terser", + totalOriginalSize: 10000, + totalMinifiedSize: 3000, + totalReduction: 70, + totalTimeMs: 50, + }; + + beforeEach(() => { + vi.resetAllMocks(); + (context as { payload: Record }).payload = {}; + }); + + test("returns empty array when no token provided", async () => { + const result = await compareWithBase(mockResult, undefined); + + expect(result).toEqual([]); + expect(warning).toHaveBeenCalledWith( + "No GitHub token provided, skipping base branch comparison" + ); + }); + + test("returns empty array when not a pull request", async () => { + (context as { payload: Record }).payload = {}; + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result).toEqual([]); + expect(info).toHaveBeenCalledWith( + "Not a pull request, skipping base branch comparison" + ); + }); + + test("compares files against base branch", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockResolvedValue({ + data: { type: "file", size: 3500 }, + }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result).toHaveLength(1); + expect(result[0]?.file).toBe("dist/app.min.js"); + expect(result[0]?.baseSize).toBe(3500); + expect(result[0]?.currentSize).toBe(3000); + expect(result[0]?.isNew).toBe(false); + expect(result[0]?.change).toBeCloseTo(-14.29, 1); + + expect(mockGetContent).toHaveBeenCalledWith({ + owner: "test-owner", + repo: "test-repo", + path: "dist/app.min.js", + ref: "main", + }); + }); + + test("handles new file (404 error)", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const notFoundError = Object.assign(new Error("Not Found"), { + status: 404, + }); + const mockGetContent = vi.fn().mockRejectedValue(notFoundError); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result).toHaveLength(1); + expect(result[0]?.isNew).toBe(true); + expect(result[0]?.baseSize).toBeNull(); + expect(result[0]?.change).toBeNull(); + }); + + test("handles directory response as new file", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockResolvedValue({ + data: [{ type: "file", name: "index.js" }], + }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result[0]?.isNew).toBe(true); + }); + + test("handles symlink/submodule as new file", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockResolvedValue({ + data: { type: "symlink", target: "some-target" }, + }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result[0]?.isNew).toBe(true); + }); + + test("handles unexpected errors gracefully", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const unexpectedError = new Error("Network error"); + const mockGetContent = vi.fn().mockRejectedValue(unexpectedError); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result[0]?.isNew).toBe(true); + expect(warning).toHaveBeenCalledWith( + expect.stringContaining("Failed to fetch base branch version") + ); + }); + + test("handles zero base size", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockResolvedValue({ + data: { type: "file", size: 0 }, + }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result[0]?.baseSize).toBe(0); + expect(result[0]?.change).toBe(100); + }); + + test("handles multiple files", async () => { + const multiFileResult: MinifyResult = { + files: [ + { + file: "dist/a.js", + originalSize: 5000, + minifiedSize: 1500, + reduction: 70, + timeMs: 25, + }, + { + file: "dist/b.js", + originalSize: 5000, + minifiedSize: 1500, + reduction: 70, + timeMs: 25, + }, + ], + compressor: "terser", + totalOriginalSize: 10000, + totalMinifiedSize: 3000, + totalReduction: 70, + totalTimeMs: 50, + }; + + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi + .fn() + .mockResolvedValueOnce({ data: { type: "file", size: 1600 } }) + .mockResolvedValueOnce({ data: { type: "file", size: 1400 } }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(multiFileResult, "fake-token"); + + expect(result).toHaveLength(2); + expect(mockGetContent).toHaveBeenCalledTimes(2); + }); +}); From 5c9c47a36f082adce34f73f609f4747b85983203 Mon Sep 17 00:00:00 2001 From: Rodolphe Stoclin Date: Fri, 23 Jan 2026 22:08:47 +0100 Subject: [PATCH 3/5] fix(action): totalCurrentSize now includes all files, not just comparable --- packages/action/__tests__/compare.test.ts | 4 +-- packages/action/src/compare.ts | 39 +++++++++++------------ 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/packages/action/__tests__/compare.test.ts b/packages/action/__tests__/compare.test.ts index f0ae9d7ed..c20fdc76a 100644 --- a/packages/action/__tests__/compare.test.ts +++ b/packages/action/__tests__/compare.test.ts @@ -186,8 +186,8 @@ describe("calculateTotalChange", () => { const result = calculateTotalChange(comparisons); expect(result.totalBaseSize).toBe(1000); - expect(result.totalCurrentSize).toBe(800); - expect(result.totalChangePercent).toBe(-20); + expect(result.totalCurrentSize).toBe(1300); + expect(result.totalChangePercent).toBe(30); }); test("handles all new files", () => { diff --git a/packages/action/src/compare.ts b/packages/action/src/compare.ts index 8d200c84e..f4b560153 100644 --- a/packages/action/src/compare.ts +++ b/packages/action/src/compare.ts @@ -38,19 +38,18 @@ export async function compareWithBase( const octokit = getOctokit(githubToken); const { owner, repo } = context.repo; - const results: ComparisonResult[] = []; - - for (const fileResult of result.files) { - const comparison = await compareFile( - octokit, - owner, - repo, - baseBranch, - fileResult.file, - fileResult.minifiedSize - ); - results.push(comparison); - } + const results = await Promise.all( + result.files.map((fileResult) => + compareFile( + octokit, + owner, + repo, + baseBranch, + fileResult.file, + fileResult.minifiedSize + ) + ) + ); return results; } @@ -192,6 +191,11 @@ export function calculateTotalChange(comparisons: ComparisonResult[]): { totalCurrentSize: number; totalChangePercent: number | null; } { + const totalCurrentSize = comparisons.reduce( + (sum, c) => sum + c.currentSize, + 0 + ); + const comparable = comparisons.filter( (c) => !c.isNew && c.baseSize !== null ); @@ -199,10 +203,7 @@ export function calculateTotalChange(comparisons: ComparisonResult[]): { if (comparable.length === 0) { return { totalBaseSize: 0, - totalCurrentSize: comparisons.reduce( - (sum, c) => sum + c.currentSize, - 0 - ), + totalCurrentSize, totalChangePercent: null, }; } @@ -211,10 +212,6 @@ export function calculateTotalChange(comparisons: ComparisonResult[]): { (sum, c) => sum + (c.baseSize ?? 0), 0 ); - const totalCurrentSize = comparable.reduce( - (sum, c) => sum + c.currentSize, - 0 - ); const totalChangePercent = totalBaseSize > 0 ? ((totalCurrentSize - totalBaseSize) / totalBaseSize) * 100 From 05ed67573ac63ab1a74806d52c407e0a1b8e9568 Mon Sep 17 00:00:00 2001 From: Rodolphe Stoclin Date: Fri, 23 Jan 2026 22:13:05 +0100 Subject: [PATCH 4/5] test(action): add edge case tests for branch coverage --- packages/action/__tests__/compare.test.ts | 76 +++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/packages/action/__tests__/compare.test.ts b/packages/action/__tests__/compare.test.ts index c20fdc76a..c59c7b742 100644 --- a/packages/action/__tests__/compare.test.ts +++ b/packages/action/__tests__/compare.test.ts @@ -235,6 +235,23 @@ describe("calculateTotalChange", () => { const result = calculateTotalChange(comparisons); expect(result.totalChangePercent).toBe(20); }); + + test("handles zero total base size with comparable files", () => { + const comparisons: ComparisonResult[] = [ + { + file: "a.js", + baseSize: 0, + currentSize: 100, + change: 100, + isNew: false, + }, + ]; + + const result = calculateTotalChange(comparisons); + expect(result.totalBaseSize).toBe(0); + expect(result.totalCurrentSize).toBe(100); + expect(result.totalChangePercent).toBe(0); + }); }); describe("compareWithBase", () => { @@ -464,4 +481,63 @@ describe("compareWithBase", () => { expect(result).toHaveLength(2); expect(mockGetContent).toHaveBeenCalledTimes(2); }); + + test("handles zero base size with zero current size", async () => { + const zeroResult: MinifyResult = { + files: [ + { + file: "dist/empty.js", + originalSize: 0, + minifiedSize: 0, + reduction: 0, + timeMs: 1, + }, + ], + compressor: "terser", + totalOriginalSize: 0, + totalMinifiedSize: 0, + totalReduction: 0, + totalTimeMs: 1, + }; + + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockResolvedValue({ + data: { type: "file", size: 0 }, + }); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(zeroResult, "fake-token"); + + expect(result[0]?.baseSize).toBe(0); + expect(result[0]?.change).toBe(0); + }); + + test("handles non-Error thrown values", async () => { + (context as { payload: Record }).payload = { + pull_request: { base: { ref: "main" } }, + }; + + const mockGetContent = vi.fn().mockRejectedValue("string error"); + + vi.mocked(getOctokit).mockReturnValue({ + rest: { + repos: { getContent: mockGetContent }, + }, + } as unknown as ReturnType); + + const result = await compareWithBase(mockResult, "fake-token"); + + expect(result[0]?.isNew).toBe(true); + expect(warning).toHaveBeenCalledWith( + expect.stringContaining("string error") + ); + }); }); From 0e8ac02687707e10a3db7fb9ec1f255b49775f4a Mon Sep 17 00:00:00 2001 From: Rodolphe Stoclin Date: Fri, 23 Jan 2026 22:17:50 +0100 Subject: [PATCH 5/5] fix(action): use comparable files for totalChangePercent calculation Previously totalChangePercent incorrectly included new files' sizes in the current total, inflating the percent when new files were present. Now uses only comparable files' current sizes for accurate before/after comparison. --- packages/action/__tests__/compare.test.ts | 3 ++- packages/action/src/compare.ts | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/action/__tests__/compare.test.ts b/packages/action/__tests__/compare.test.ts index c59c7b742..55fc7212e 100644 --- a/packages/action/__tests__/compare.test.ts +++ b/packages/action/__tests__/compare.test.ts @@ -187,7 +187,8 @@ describe("calculateTotalChange", () => { const result = calculateTotalChange(comparisons); expect(result.totalBaseSize).toBe(1000); expect(result.totalCurrentSize).toBe(1300); - expect(result.totalChangePercent).toBe(30); + // totalChangePercent uses only comparable files (800 vs 1000 = -20%) + expect(result.totalChangePercent).toBe(-20); }); test("handles all new files", () => { diff --git a/packages/action/src/compare.ts b/packages/action/src/compare.ts index f4b560153..2c54723f8 100644 --- a/packages/action/src/compare.ts +++ b/packages/action/src/compare.ts @@ -212,9 +212,13 @@ export function calculateTotalChange(comparisons: ComparisonResult[]): { (sum, c) => sum + (c.baseSize ?? 0), 0 ); + const comparableCurrentSize = comparable.reduce( + (sum, c) => sum + c.currentSize, + 0 + ); const totalChangePercent = totalBaseSize > 0 - ? ((totalCurrentSize - totalBaseSize) / totalBaseSize) * 100 + ? ((comparableCurrentSize - totalBaseSize) / totalBaseSize) * 100 : 0; return {