Document @fedify/fixture boundary and add a usage checker#747
Merged
dahlia merged 5 commits intofedify-dev:mainfrom Apr 28, 2026
Merged
Document @fedify/fixture boundary and add a usage checker#747dahlia merged 5 commits intofedify-dev:mainfrom
@fedify/fixture boundary and add a usage checker#747dahlia merged 5 commits intofedify-dev:mainfrom
Conversation
`@fedify/fixture` is a private workspace package — it is absent from the published artifacts on npm and JSR, so any non-test file that imports it would fail to resolve once the dependent package is installed from a registry. The convention is therefore to keep `@fedify/fixture` confined to `**/*.test.ts` files. To make that convention easier for contributors to follow, this change adds a `mise run check:fixture-usage` task backed by a small Deno script. The script scans `packages/<pkg>/src/` and points out any non-`*.test.ts` file that imports or re-exports from `@fedify/fixture`, so a contributor catches the slip locally before opening a PR. The task is also wired into the aggregate `mise run check` so it runs alongside the other code-quality checks. The script recognises every common import/export form (default, namespace, named, mixed, type-only, side-effect, re-exports, multi-line braces, and subpath specifiers). It is intentionally documented as a coarse textual scan: dynamic `import()`, `require()`, indirect re-exports, and a `*.test.ts` file being re-exported from a shipped entry point are not detected, so the check is a convenience for contributors rather than a guarantee for reviewers. Genuine exceptions live in an `ALLOWLIST` constant inside the script, with an inline comment per entry explaining the justification. The current entries cover the cfworkers test harness re-exports in `packages/fedify/src/testing/` and a JSDoc `@example` mention in `packages/testing/src/mq-tester.ts`. Assisted-by: Claude Code:claude-opus-4-7
Add a README to packages/fixture/ that explains what the package exports and how to use it from other workspace packages, so new contributors can pick up the testing infrastructure without reading every dependent test file first. The README walks through the four entry points (`test()`, `testDefinitions`, `mockDocumentLoader()`, and `TestSpanExporter` / `createTestTracerProvider()`), shows the fixture-tree convention, and notes the runtime-specific quirks (LogTape buffering, Bun and Node.js dispatch, the Cloudflare Workers `.test` hostname trick). Add a "Writing tests with @fedify/fixture" section to CONTRIBUTING.md that points contributors at the new README, gives a one-line summary per export, and spells out the boundary rule: imports of `@fedify/fixture` must stay in `**/*.test.ts` files because the package is private and absent from the published artifacts. Both documents mention `mise run check:fixture-usage` as a convenience check while making clear it is a coarse textual scan and not a substitute for code review. Assisted-by: Claude Code:claude-opus-4-7
📝 WalkthroughWalkthroughDocuments the monorepo-private Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
issues-auto-labeler
Bot
added
component/build
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new private workspace package, @fedify/fixture, to provide shared test infrastructure across the monorepo. It includes a test() wrapper for cross-runtime compatibility, a fixture-backed document loader, and OpenTelemetry testing utilities. To prevent accidental leaks of this private package into published artifacts, a new linting script scripts/check_fixture_usage.ts and a corresponding mise task have been added. Feedback focuses on correcting a misleading jsr: import specifier in the documentation and addressing potential issues with JSR source-based distribution where raw imports of the private package might persist in exported files.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/fixture/README.md`:
- Around line 109-113: The README currently implies the LogTape buffering/reset
behavior is universal; narrow the wording to state this applies only to Deno by
updating the sentence to something like "In Deno, test() configures LogTape
before every test and resets it afterwards" and mention that Node/Bun do not use
this Deno-specific branch; verify against the implementation in
packages/fixture/src/test.ts (look for the test() function and the LogTape
setup/reset logic) and ensure the README language references Deno and the
LOG=always environment variable only for that runtime.
In `@scripts/check_fixture_usage.ts`:
- Around line 73-83: The relative path computed with relative(projectRoot,
entry.path) (stored in rel) may contain OS-native backslashes on Windows,
causing allowed.has(rel) and rel.endsWith(".test.ts") checks to fail; normalize
rel to use forward slashes (replace backslashes with '/') before performing the
.endsWith and allowed.has checks so comparisons match the ALLOWLIST format used
elsewhere (e.g., ensure the code around entry/walk, relative, rel, and allowed
uses the canonicalized forward-slash rel).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 578faafc-2b8a-4d00-986c-bd8b0b3a98e0
📒 Files selected for processing (4)
CONTRIBUTING.mdmise.tomlpackages/fixture/README.mdscripts/check_fixture_usage.ts
dahlia
requested changes
Apr 28, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <copilot@github.com>
Drop the misleading deno.json snippet from the fixture README: the workspace already resolves @fedify/fixture from the in-tree source at the repository root, so contributors do not need to add an `imports` entry, and showing one suggested otherwise. Scope the LogTape buffering note to Deno, since the Bun and Node.js dispatchers do not configure LogTape, and replace the two separate `LOG=always` invocations with the single `mise run test:deno` form that contributors actually use. Fix the JSR reference link to point at jsr.io (the canonical host) rather than the jsr.dev redirect. Drop the gratuitous spaces around the slash separating `TestSpanExporter` and `createTestTracerProvider()` in CONTRIBUTING.md so the pair reads as a single label. Assisted-by: Claude Code:claude-opus-4-7
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/check_fixture_usage.ts`:
- Around line 79-85: The skip and match regexes used in the walk(packagesDir,
...) call are not separator-safe: walk yields absolute paths so the skip Regex
'^packages${SEPARATOR}fixture' never matches, and interpolating SEPARATOR into a
RegExp produces unescaped backslashes on Windows. Update the patterns used in
the walk call (the match and skip arrays) to use a platform-agnostic separator
character class like '[/\\]' (or build via a helper e.g., sepClass) instead of
interpolating SEPARATOR, and ensure the skip pattern targets paths that may be
absolute (e.g., include a preceding any-path component or use character-class
separators) so the skip actually matches fixture package paths when iterating
packagesDir.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 120e2c5c-44e7-4944-9eb1-b1e8ba6edd69
📒 Files selected for processing (4)
AGENTS.mdCONTRIBUTING.mdpackages/fixture/README.mdscripts/check_fixture_usage.ts
dahlia
approved these changes
Apr 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #746.
Summary
test(),testDefinitions,mockDocumentLoader(),TestSpanExporter/createTestTracerProvider()), the fixture-tree convention, and the runtime-specific quirks (LogTape buffering, Bun and Node.js dispatch, the Cloudflare Workers.testhostname trick).@fedify/fixture” section to CONTRIBUTING.md that summarises each export and states the boundary rule: imports of@fedify/fixturemust stay in**/*.test.tsfiles because the package is private and absent from the published artifacts.mise run check:fixture-usagetask backed by scripts/check_fixture_usage.ts. The script scanspackages/<pkg>/src/and points out any non-*.test.tsfile that imports or re-exports from@fedify/fixture. It recognises every common import/export form (default, namespace, named, mixed, type-only, side-effect, re-exports, multi-line braces, subpath specifiers). It is wired into the aggregatemise run check.ALLOWLISTconstant inside the script with an inline justification per entry. Current entries cover the cfworkers test-harness re-exports under packages/fedify/src/testing/ and a JSDoc@examplemention in packages/testing/src/mq-tester.ts.The checker is positioned as a contributor convenience, not as rigorous verification: dynamic
import(),require(), indirect re-exports, and a*.test.tsfile being re-exported from a shipped entry point are not detected. Both documents say so explicitly.Test plan
mise run check:fixture-usagepasses on the current tree.mise run checkruns the new task alongside the other checks.import "@fedify/fixture"to a non-*.test.tsfile underpackages/<pkg>/src/causes the checker to fail with a pointer to that file.ALLOWLISTentry while the corresponding file still imports@fedify/fixturecauses the checker to fail.Assisted-by: Claude Code:claude-opus-4-7