Skip to content

Conversation

@hugo-andrade
Copy link
Contributor

@hugo-andrade hugo-andrade commented Aug 19, 2025

Description

Add missing unit test coverage for UnbufferedBody.WriteAsJSON and UnbufferedBody.WriteAsXML to assert the internal reassignment of w.reader = io.NopCloser(&buf). New tests write structured JSON/XML, then read back via ReadAsString to validate the buffer content and ensure no regressions.

Motivation and Context

Improves test coverage for body wrapper serialization paths, specifically the final lines in WriteAsJSON and WriteAsXML that were previously unverified. This guards against future refactors breaking reader reassignment and ensures consistent output formatting (no unexpected trailing newlines for XML, JSON includes encoder newline).

How Has This Been Tested?

  • Added two new test cases:
    • WriteAsJSON assigns reader contents
    • WriteAsXML assigns reader contents
  • Ran go test ./... (all tests passing).
  • Verified expected vs actual output strings (adjusted XML expectation to exclude newline).

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Related Issues:

N/A (test coverage hardening; no linked issue).

Copilot AI review requested due to automatic review settings August 19, 2025 14:26
@codecov
Copy link

codecov bot commented Aug 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (8e2ade0) to head (eaec3b0).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #31   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           28        28           
  Lines          779       779           
=========================================
  Hits           779       779           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes several behavioral issues in the HTTP wrapper body functionality and enhances test coverage. The changes address incorrect buffer handling and improve error handling edge cases.

  • Fixed buffer reset issue in BufferedBody's WriteAsString method
  • Removed incorrect reader reassignment in UnbufferedBody's ReadAsString method
  • Enhanced test coverage for edge cases and error scenarios

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
wrapper_body.go Fixed buffer reset in WriteAsString and removed incorrect reader reassignment in ReadAsString
wrapper_body_test.go Added comprehensive test cases for buffer operations, error scenarios, and edge cases
request_test.go Enhanced retry mechanism test coverage with additional backoff strategies and edge cases
client_test.go Improved test assertions using testify/assert for better error reporting
Comments suppressed due to low confidence (1)

wrapper_body_test.go:253

  • Removing the unused setup field from the test struct without adding equivalent functionality in the new test cases may indicate incomplete test migration. Verify that all previously tested setup scenarios are covered by the new test structure.
		reader        io.ReadCloser

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@hugo-andrade hugo-andrade merged commit 3fe9537 into main Aug 19, 2025
3 checks passed
@hugo-andrade hugo-andrade deleted the feature/middlewares branch August 19, 2025 14:42
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