Skip to content

Test Express response forwarding - #1217

Merged
dahlia merged 1 commit into
fedify-dev:mainfrom
Lumia1108:issue-856-test-express-response-forwarding
Oct 4, 2026
Merged

dahlia merged 1 commit into
fedify-dev:mainfrom
Lumia1108:issue-856-test-express-response-forwarding

Conversation

@Lumia1108

Copy link
Copy Markdown
Contributor

This adds regression coverage confirming that @fedify/express forwards a Fedify response's status, headers, and every streamed body chunk to the Express response. The response mock now records headers so that the test can verify header forwarding case-insensitively.

Closes #856

Testing

All checks passed locally:

  • mise run test:deno packages/express/src/index.test.ts
  • mise run check-each express
  • mise run test-each express

AI assistance

I used Codex (GPT-5.6) to understand the relevant code and review the final diff.

I personally wrote the changes and verified them locally.

Add regression coverage for forwarding response status, headers, and every streamed body chunk to Express.

Codex was used to explain the relevant code and review the test. I personally wrote and verified the implementation.

Assisted-by: Codex:GPT-5.6
@netlify

netlify Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for fedify-json-schema canceled.

Name Link
🔨 Latest commit 1af83de
🔍 Latest deploy log https://app.netlify.com/projects/fedify-json-schema/deploys/6ac10e120893840008e3a3cc

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
CONTRIBUTING.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 2f108663-5950-4dd4-a328-5e073194472a
📥 Commits

Reviewing files that changed from the base of the PR and between 49d937e and 1af83de.

📒 Files selected for processing (1)
  • packages/express/src/index.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

The Express test mock now records response headers. A new test checks that integrateFederation() forwards a Fedify response's status, header, and streamed body.

Changes

Response forwarding test

Layer / File(s) Summary
Response mock and forwarding test
packages/express/src/index.test.ts
The response mock stores headers by lowercase name and provides getHeader. The new test checks forwarding of status 201, a header, and a streamed body.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 1af83

The new test covers response status, headers, and streamed body forwarding. No merge-blocking issue is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the regression test for Express response forwarding.
Description check ✅ Passed The description explains the test coverage, response mock changes, linked issue, and reported checks. It is directly related to the changeset.
Linked Issues check ✅ Passed Issue #856 asks for a test in packages/express/src/index.test.ts that verifies integrateFederation() forwards response status, headers, and body. The PR summary confirms a new test covers status, …
Out of Scope Changes check ✅ Passed The changes are limited to the Express integration test. The response mock changes and import reordering support the regression test for issue #856. No unrelated changes are reported.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dahlia dahlia self-assigned this Oct 4, 2026
@dahlia dahlia added component/integration Web framework integration component/testing Testing utilities (@fedify/testing) integration/express Express.js integration (@fedify/express) labels Oct 4, 2026
@dahlia dahlia added this to the Fedify 2.5 milestone Oct 4, 2026
@dahlia
dahlia merged commit 26ad469 into fedify-dev:main Oct 4, 2026
26 checks passed
@Lumia1108
Lumia1108 deleted the issue-856-test-express-response-forwarding branch October 4, 2026 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/integration Web framework integration component/testing Testing utilities (@fedify/testing) integration/express Express.js integration (@fedify/express)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test response forwarding in @fedify/express

2 participants