From 7b2fa9032a5345131c9c9c186630080cfd949e1d Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sun, 30 Aug 2026 08:23:31 +0900 Subject: [PATCH] test: let the README asset check tell files from directories The shipped-asset check treated every `package.json` `files` entry as a possible directory prefix. `assets/banner.png` therefore vouched for `assets/banner.png/missing.gif`, and `LICENSE` for `LICENSE/missing.png`. The intent was right: a directory entry does ship everything beneath it, and the existing comment correctly rejects deciding that by looking for a dot in the name. But prefix matching alone cannot tell the two cases apart either. Ask the filesystem which entries are directories, and let only those act as prefixes. The check is a guard against broken images on the npm package page, so a false negative here is exactly the failure it exists to catch. --- tests/repo-hygiene.test.ts | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/repo-hygiene.test.ts b/tests/repo-hygiene.test.ts index 6132a9776a..c1de41184b 100644 --- a/tests/repo-hygiene.test.ts +++ b/tests/repo-hygiene.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { existsSync } from "node:fs"; +import { existsSync, statSync } from "node:fs"; import { fileURLToPath } from "node:url"; const repoRoot = fileURLToPath(new URL("../", import.meta.url)); @@ -244,13 +244,22 @@ describe("devlog is tracked, with no submodule left behind", () => { // end state, and a `toBeGreaterThan(0)` guard here would fail the suite for doing it. const relative = [...readme.matchAll(/src="(?!https?:)([^"]+)"/g)].map((match) => match[1]!); - const missing = relative.filter((asset) => { - if (shipped.includes(asset)) return false; - // A directory entry ships everything beneath it. Decided by whether the tarball path is a - // prefix, not by whether the name contains a dot: `LICENSE` has no dot and is a file, and - // a future `assets` entry would have no dot and be a directory. - return !shipped.some((entry) => asset.startsWith(`${entry}/`)); + // A directory entry ships everything beneath it; a regular-file entry ships only itself. + // Deciding that by prefix alone let `assets/banner.png` vouch for a nonexistent + // `assets/banner.png/missing.gif`, so a broken README reference could pass. Ask the + // filesystem what each entry actually is instead of inferring it from the name. + const shippedDirectories = shipped.filter((entry) => { + const path = new URL(`../${entry}`, import.meta.url); + return existsSync(path) && statSync(path).isDirectory(); }); + const isShipped = (asset: string): boolean => + shipped.includes(asset) + || shippedDirectories.some((directory) => asset.startsWith(`${directory}/`)); + + expect(isShipped("assets/banner.png/missing.gif")).toBe(false); + expect(isShipped("LICENSE/missing.png")).toBe(false); + + const missing = relative.filter((asset) => !isShipped(asset)); expect(missing).toEqual([]); }); });