Fix fedify/next test-each failure, add fedify/next tests - #978
Fix fedify/next test-each failure, add fedify/next tests#978Bananamilk452 wants to merge 6 commits into
Conversation
✅ Deploy Preview for fedify-json-schema canceled.
|
📝 WalkthroughWalkthroughThe pull request adds Next.js integration tests for request routing and custom handlers. It adds Node.js and Bun package test scripts. The shared ChangesNext.js runtime tests
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
|
|
Next.js does not work well with Deno. Seriously. Instead of adding deno.json to |
8b26d33 to
b84059d
Compare
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@mise.toml`:
- Around line 308-312: Update the package loop condition around the Deno
commands to require both the package-local packages/($pkg)/deno.json and
membership in the root deno.json workspace; only then run mise run check-each
and deno task --filter. Keep packages lacking either configuration out of this
Deno task path.
🪄 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 Plus
Run ID: 25562d94-d015-474f-ad9a-4ec3cfec14cd
⛔ Files ignored due to path filters (1)
deno.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
mise.tomlpackages/next/package.jsonpackages/next/src/index.test.ts
| if ($"packages/($pkg)/deno.json" | path exists) { | ||
| mise run check-each $pkg | ||
| print $"Running Deno tests for package: ($pkg)" | ||
| deno task --filter $"@fedify/($pkg)" test | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Root Deno workspace:"
jq '.workspace' deno.json
echo
echo "Package-local Deno configurations:"
fd --type f '^deno\.json$' packages | sort
echo
echo "Current test-each gate:"
sed -n '301,315p' mise.tomlRepository: fedify-dev/fedify
Length of output: 2803
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "Root Deno config file:"
if [ -f deno.json ]; then
jq '{specifiers: .imports, tasks: (.tasks // {} | keys | sort), tools: .tools // {}, workspace: .workspace}' deno.json
fi
echo
echo "Package-local Deno config relevant fields:"
python3 - <<'PY'
import json, pathlib
root = json.load(open("deno.json"))
workspace = root.get("workspace") or []
for p in sorted(pathlib.Path("packages").glob("*/deno.json")):
name = "packages/" + str(p.parent.relative_to("packages"))
data = json.load(open(p))
print(f"{name}: has_tasks={bool(data.get('tasks'))}, has_specifiers={bool(data.get('imports'))}, in_root_workspace={name in workspace}")
PY
echo
echo "Relevant mise.toml lines:"
sed -n '296,318p' mise.tomlRepository: fedify-dev/fedify
Length of output: 7414
Check root Deno workspace membership before running Deno tasks.
packages/<pkg>/deno.json exists for every package, so the current condition runs check-each and deno task --filter even for packages not listed under deno.json.workspace. Gate these commands on the package being present in the root deno.json workspace as well as having a package-local Deno config.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@mise.toml` around lines 308 - 312, Update the package loop condition around
the Deno commands to require both the package-local packages/($pkg)/deno.json
and membership in the root deno.json workspace; only then run mise run
check-each and deno task --filter. Keep packages lacking either configuration
out of this Deno task path.
- add next as dependency and deno.json - skip Deno tests for packages without deno.json - add test scripts in next/package.json Assisted-by: OpenCode:gpt-5.6-terra
Assisted-by: OpenCode:gpt-5.6-terra
b84059d to
fe605d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/next/src/index.test.ts`:
- Around line 6-7: Replace the test import from `@fedify/fixture` with Node’s
built-in test harness by importing test from node:test in the package test file,
while preserving the existing assertions and test behavior.
🪄 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 Plus
Run ID: 2292e827-1479-4eeb-a805-b0a3e002baf4
📒 Files selected for processing (3)
mise.tomlpackages/next/package.jsonpackages/next/src/index.test.ts
| import { test } from "@fedify/fixture"; | ||
| import { strict as assert } from "node:assert"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Use node:test for this package test file.
This package now runs tests with node --test and bun test. Use Node’s built-in test harness instead of @fedify/fixture.
Based on learnings: use Node’s built-in node:test when adding package tests.
Proposed fix
-import { test } from "`@fedify/fixture`";
import { strict as assert } from "node:assert";
+import { test } from "node:test";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { test } from "@fedify/fixture"; | |
| import { strict as assert } from "node:assert"; | |
| import { strict as assert } from "node:assert"; | |
| import { test } from "node:test"; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/next/src/index.test.ts` around lines 6 - 7, Replace the test import
from `@fedify/fixture` with Node’s built-in test harness by importing test from
node:test in the package test file, while preserving the existing assertions and
test behavior.
Source: Learnings
2 participants
Description
packages/next/src/index.test.tsto cover federation request detection, NodeInfo route handling, middleware delegation, and custom error handling logic.mise.tomltest runner to only run Deno tests if adeno.jsonfile exists for the package, and to always run Node.js and Bun tests with clear output messages.Github Copilot helped me making title of this PR, gpt-5.6-terra helped checking code quality and code creation.