Skip to content

test: let the README asset check tell files from directories #2952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lidge-jun merged 1 commit into from
Aug 30, 2026
Merged

test: let the README asset check tell files from directories #2952

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
23 changes: 16 additions & 7 deletions tests/repo-hygiene.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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));
Expand Down Expand Up @@ -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();
Comment on lines +251 to +253

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Classify generated package directories without probing the checkout

In a clean checkout, package.json declares gui/dist, but that directory is intentionally generated only by build:gui; the release path confirms this at .github/workflows/release.yml:244-245. This filter therefore misclassifies a current package directory as a file until a GUI build has run, so a legitimate README reference such as gui/dist/logo.png would fail the focused hygiene test in a clean tree even though it is present in every published tarball, while the same test passes when stale build output exists. Determine entry types from the packed manifest or explicitly account for generated package directories rather than relying on the current filesystem.

AGENTS.md reference: AGENTS.md:L17-L17

Useful? React with 👍 / 👎.

});
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([]);
});
});
Loading