Test/http response

[eafalkens]

HttpResponse is responsible for representing and mutating the HTTP response sent back to the client.

To ensure reliability and correctness, we should add unit tests that verify proper status handling, header storage and retrieval, and body handling. This will help guarantee a stable response system.

Summary by CodeRabbit

• Tests
  • Refactored test suite with improved structure and upgraded assertions for better code quality.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors HttpResponseTest to use JUnit 5 assertions and a fixture-based test setup. Previously monolithic tests are split into focused, single-responsibility tests that validate HttpResponse defaults: status code 200, status text "OK", empty headers and body, and null-safety for status text. HttpResponseWriterTest receives minor whitespace cleanup.

Changes

Cohort / File(s)	Summary
Test Fixture & Assertion Refactoring src/test/java/org/juv25d/http/HttpResponseTest.java	Makes class public, introduces @BeforeEach fixture setup, migrates to JUnit 5 assertions, splits monolithic tests into focused per-behavior tests validating default status code (200), status text ("OK"), empty headers, empty body, and NullPointerException on null status text.
Whitespace Cleanup src/test/java/org/juv25d/http/HttpResponseWriterTest.java	Removes trailing blank line at end of class.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• feat: make HttpResponse mutable and implement NotFoundPlugin default #39: Introduces setStatusText(), setBody(), and setHeader() setters with null-checks that these new tests directly exercise and validate.
• Initialize safe defaults in HttpResponse no-arg constructorFix/httpresponse defaults #87: Implements HttpResponse constructor defaults and statusText validation that these expanded tests now verify and assert against.
• feature/basic-http-responses #24: Introduces the HttpResponse API and HttpResponseWriter behavior that this test suite targets and validates.

Suggested reviewers

• addee1
• SandraNelj

Poem

🐰 Hops through test refactoring with glee,
Splitting monoliths into tests clean and free,
With fixtures and assertions so bright,
Default behaviors now tested just right!
JUnit 5's path shines oh-so-sweet,
Making test validation complete! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Test/http response' is vague and generic, using a non-descriptive format that doesn't clearly convey the specific changes being made to the test suite.	Use a more descriptive title that clearly indicates the scope of testing, such as 'Add unit tests for HttpResponse class' or 'Add HttpResponse unit tests for status, headers, and body handling'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch test/HttpResponse

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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 `@src/test/java/org/juv25d/http/HttpResponseTest.java`:
- Around line 27-30: Annotate the test method
shouldThrowExceptionWhenStatusTextIsNull with a NullAway suppression so the
intentional null-safety test can pass static analysis; specifically add
`@SuppressWarnings`("NullAway") (or the project's configured NullAway suppression
token) to the method declaration of shouldThrowExceptionWhenStatusTextIsNull to
silence NullAway for that test only.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e46bb6b and bc28a4e.

📒 Files selected for processing (2)

• src/test/java/org/juv25d/http/HttpResponseTest.java
• src/test/java/org/juv25d/http/HttpResponseWriterTest.java

💤 Files with no reviewable changes (1)

• src/test/java/org/juv25d/http/HttpResponseWriterTest.java

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Suppress NullAway for intentional null-safety test.

The pipeline fails because NullAway detects that null is passed where @NonNull is required. Since this test intentionally verifies null-rejection behavior, suppress NullAway for this specific line.

🛠️ Proposed fix to suppress NullAway

     `@Test`
+    `@SuppressWarnings`("NullAway")
     void shouldThrowExceptionWhenStatusTextIsNull() {
         assertThrows(NullPointerException.class, () -> response.setStatusText(null));
     }

🧰 Tools

🪛 GitHub Actions: CI Pipeline

[error] 29-29: NullAway: passing @Nullable parameter 'null' where @NonNull is required.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/juv25d/http/HttpResponseTest.java` around lines 27 - 30,
Annotate the test method shouldThrowExceptionWhenStatusTextIsNull with a
NullAway suppression so the intentional null-safety test can pass static
analysis; specifically add `@SuppressWarnings`("NullAway") (or the project's
configured NullAway suppression token) to the method declaration of
shouldThrowExceptionWhenStatusTextIsNull to silence NullAway for that test only.