Skip to content

test(response): cover blob body and header setting#128

Merged
Sorikairox merged 4 commits into
mainfrom
claude/github-discussion-121-sg0vop
Jun 25, 2026
Merged

test(response): cover blob body and header setting#128
Sorikairox merged 4 commits into
mainfrom
claude/github-discussion-121-sg0vop

Conversation

@Sorikairox

@Sorikairox Sorikairox commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Addresses discussion #121: "How can I set the headers of the response and the body as a blob?"

Adds tests to spec/response-decorator.test.ts asserting the three supported ways to do this:

  1. Return a Response with a Blob body + custom headers (Content-Type, Content-Disposition, custom header) — sent as-is.
  2. @Res() to mutate response headers while returning a plain value (Danet merges context.res.headers).
  3. @Context() to set the body and headers via Hono helpers (c.header(), c.body()).

The matching user-facing documentation lives in a companion PR on Savory/docs.

🤖 Generated with Claude Code


Generated by Claude Code

Summary by CodeRabbit

  • Tests
    • Added new integration coverage for three response-handling scenarios using parameter decorators.
    • Verified binary (Blob) response bodies and headers like Content-Type and Content-Disposition.
    • Verified custom headers when using provided response objects and when setting headers via context.
    • Confirmed existing HTTP status code coverage remains unchanged.
  • Refactor
    • Improved type annotations for request/response decorator helpers (Req/Res) for better type safety.
  • Chores
    • Bumped the project version.

…context

Asserts the behaviors documented for discussion #121: returning a Response
with a Blob body and custom headers, setting headers with @res() while
returning a value, and setting body/headers with @context().

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnF4fzmM7CwBfq7zXBnQsF
@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9c3eb593-f998-40e7-97b5-e7effb0c6c7e

📥 Commits

Reviewing files that changed from the base of the PR and between 3722fdf and ff3a1f9.

📒 Files selected for processing (1)
  • deno.json
✅ Files skipped from review due to trivial changes (1)
  • deno.json

📝 Walkthrough

Walkthrough

The controller adds routes that return a Blob, mutate headers through @Res(), and set headers and bodies through @Context(). The tests call each route and assert the returned headers and response content. The exported Req and Res decorator constants were also retyped, and the project version was bumped.

Changes

Response decorator coverage

Layer / File(s) Summary
Decorator return types
src/router/controller/params/decorators.ts
Updates the exported Req and Res constants to use () => DecoratorFunction type annotations.
Routes and assertions
spec/response-decorator.test.ts
Adds Blob, @Res(), and @Context() routes to SimpleController and expands the Deno integration tests to verify their headers and response bodies.
Version bump
deno.json
Updates the project version from 2.11.0 to 2.11.1.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hop hop, the headers gleam,
A Blob and tests in a tidy stream.
With @Res() and @Context(), all is bright,
The decorators twitch just right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning It summarizes the change, but it omits the required Issue Ticket Number, Type of change, and checklist sections from the template. Add the missing template sections: issue number, change type checkboxes, and the checklist items, or mark them clearly if not applicable.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: adding response/body/header tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/github-discussion-121-sg0vop

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.

@Req and @res are built with createParamDecorator, which returns a
decorator factory (() => DecoratorFunction), but they were typed as
DecoratorFunction directly. This made the documented @Req()/@res() usage
fail to type-check. Type them as factories, matching @Header/@context.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnF4fzmM7CwBfq7zXBnQsF

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
spec/response-decorator.test.ts (2)

15-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use tabs for indentation.

The added controller routes (and the new tests below) are indented with spaces. Project convention is tabs.

As per coding guidelines: "Use tabs for indentation".

🤖 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 `@spec/response-decorator.test.ts` around lines 15 - 39, The new controller
routes in response-decorator.test.ts are using spaces instead of the project’s
tab indentation. Update the affected methods in the controller snippet,
including blobWithHeaders, setHeaderWithRes, and setBodyWithContext, so their
indentation matches the rest of the file and uses tabs consistently.

Source: Coding guidelines


64-116: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider grouping related assertions with testContext.step(...).

The three new cases each spin up, init, listen, and close their own DanetApplication. Since they cover the same response-decorator surface, they could be consolidated into a single Deno.test using testContext.step(...) to reduce app-lifecycle boilerplate and keep related assertions together.

As per coding guidelines: "Use testContext.step(...) to group related assertions in one Deno.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 `@spec/response-decorator.test.ts` around lines 64 - 116, Group the three
response-decorator assertions into a single Deno.test using
testContext.step(...) instead of separate test cases. Reuse one DanetApplication
lifecycle for the shared setup/teardown, and place the Blob, `@Res`, and `@Context`
checks into named steps so the related assertions stay together while reducing
repeated init/listen/close boilerplate.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@spec/response-decorator.test.ts`:
- Around line 15-39: The new controller routes in response-decorator.test.ts are
using spaces instead of the project’s tab indentation. Update the affected
methods in the controller snippet, including blobWithHeaders, setHeaderWithRes,
and setBodyWithContext, so their indentation matches the rest of the file and
uses tabs consistently.
- Around line 64-116: Group the three response-decorator assertions into a
single Deno.test using testContext.step(...) instead of separate test cases.
Reuse one DanetApplication lifecycle for the shared setup/teardown, and place
the Blob, `@Res`, and `@Context` checks into named steps so the related assertions
stay together while reducing repeated init/listen/close boilerplate.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bfe0c0b7-a3d5-460c-88a7-6661c9cc15c1

📥 Commits

Reviewing files that changed from the base of the PR and between 80e7c9b and 33f34ba.

📒 Files selected for processing (1)
  • spec/response-decorator.test.ts

claude added 2 commits June 25, 2026 01:43
Hono's c.body() Data type does not accept a raw Uint8Array under Deno 2's
stricter lib types. Use a string body so the test type-checks on both
Deno v1.x and v2.x.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnF4fzmM7CwBfq7zXBnQsF
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnF4fzmM7CwBfq7zXBnQsF
@Sorikairox
Sorikairox merged commit d5538d5 into main Jun 25, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants