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 7 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 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🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
|
Next.js does not work well with Deno. Seriously. Instead of adding deno.json to |
8b26d33 to
b84059d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.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
2chanhaeng
left a comment
2chanhaeng
left a comment
There was a problem hiding this comment.
Great work! Please check the point that I commented on.
2chanhaeng
left a comment
2chanhaeng
left a comment
There was a problem hiding this comment.
Please add packages/next to test.exclude in root deno.json
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/package.json`:
- Line 62: Update the test scripts in packages/next/package.json by adding
pretest and pretest:bun hooks that run pnpm build, matching the packages/amqp
pattern so dist/ is built before pnpm test or the Bun test command executes.
🪄 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: 907b44d1-6698-4b78-8c5d-8e8ed8bdf9cc
📒 Files selected for processing (3)
deno.jsonmise.tomlpackages/next/package.json
| "prepack": "pnpm build", | ||
| "prepublish": "pnpm build" | ||
| "prepublish": "pnpm build", | ||
| "test": "node --test dist/", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
scripts = json.loads(
Path("packages/next/package.json").read_text()
).get("scripts", {})
for name in ("prepublish", "pretest", "test", "pretest:bun", "test:bun"):
print(f"{name}: {scripts.get(name)!r}")
if scripts.get("test") == "node --test dist/" and scripts.get("pretest") != "pnpm build":
raise SystemExit("Node.js tests are not gated by a build")
if scripts.get("test:bun") and scripts.get("pretest:bun") != "pnpm build":
raise SystemExit("Bun tests are not gated by a build")Repository: fedify-dev/fedify
Length of output: 385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== packages/next/package.json scripts =="
python3 - <<'PY'
import json
from pathlib import Path
p=Path("packages/next/package.json")
data=json.loads(p.read_text())
for k in sorted(data.get("scripts", {}).keys()):
if k in ("pretest","test","pretest:bun","test:bun","prepublish"):
print(f"{k}: {data['scripts'][k]}")
PY
echo
echo "== relevant mises =="
rg -n 'packages/next|test:"|test:bun|pnpm --filter|test b?un|pnpm test' mise.toml packages -g 'mise.toml' -g 'package.json' || true
echo
echo "== packages/amqp test/build scripts =="
python3 - <<'PY'
import json
from pathlib import Path
for name in ("packages/amqp/package.json","packages/fet/package.json",):
p=Path(name)
if not p.exists(): continue
data=json.loads(p.read_text())
print(name)
for k in ("pretest","test","pretest:bun","test:bun"):
print(f" {k}: {data['scripts'].get(k)!r}")
PYRepository: fedify-dev/fedify
Length of output: 7043
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package manager / lifecycle settings =="
for f in package.json .npmrc pnpm-workspace.yaml .npm/global; do
if [ -f "$f" ]; then
echo "-- $f --"
sed -n '1,220p' "$f"
fi
done
echo
echo "== precise script lifecycle hooks in package.json =="
python3 - <<'PY'
import json
from pathlib import Path
p=Path("package.json")
data=json.loads(p.read_text())
print("packageManager:", data.get("packageManager"))
for k, v in data.get("scripts", {}).items():
if k.startswith(("pre") or "pre") in k:
print(f"{k}: {v}")
PYRepository: fedify-dev/fedify
Length of output: 3798
Gate the dist-based tests with build hooks.
packages/next/package.json runs node --test dist/, but test has no pretest hook. mise.toml invokes pnpm test, so prepublish does not satisfy this contract. Use the same pattern as packages/amqp: add pretest and pretest:bun that run pnpm build before testing against dist/.
🤖 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/package.json` at line 62, Update the test scripts in
packages/next/package.json by adding pretest and pretest:bun hooks that run pnpm
build, matching the packages/amqp pattern so dist/ is built before pnpm test or
the Bun test command executes.
|
I made a change! Please take an another look! |
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.