Skip to content

Conversation

@0xEbrahim
Copy link

@0xEbrahim 0xEbrahim commented Oct 17, 2025

Pull Request Template

related: #2436
issue: #2434

Description:

This PR refactors the HTTP service test suite to eliminate repetitive response validation logic and improve consistency across test cases.
It also fixes a mismatch between the expected and actual HTTP methods in one of the tests (previously PUT was expected instead of PATCH).

Breaking Changes (if applicable):

Added validateResponse Test Helper

  • Declared in pkg/gofr/service/new_test.go
  • Includes t.Helper() to mark it as a testing helper and satisfy the thelper linter rule.
  • Handles response body closure safely (defer resp.Body.Close()).
  • Performs unified assertions for success and error scenarios (require.NoError, assert.NotNil, etc.).
  • Accepts hasError flag to differentiate between positive and negative test cases.

Refactored All HTTP Method Tests

  • Replaced duplicated response validation code with validateResponse calls.
  • Unified structure across all GET, POST, PUT, PATCH, and DELETE test cases.
  • Improved readability and consistency of test output and assertion style.

Fixed HTTP Method Assertion

  • A test previously asserted http.MethodPut while the service was using PATCH.
  • The assertion has been corrected to match the intended HTTP method and avoid false test failures.

Additional Information:

  • Mention any relevant dependencies or external libraries used.
  • Include screenshots or code snippets (if necessary) to clarify the changes.

Checklist:

  • I have formatted my code using goimport and golangci-lint.
  • All new code is covered by unit tests.
  • This PR does not decrease the overall code coverage.
  • I have reviewed the code comments and documentation for clarity.

Thank you for your contribution!

@0xEbrahim
Copy link
Author

@Umang01-hash please see this

@0xEbrahim
Copy link
Author

@Umang01-hash I created a factory function to generate the service

Copy link
Member

@Umang01-hash Umang01-hash left a comment

Choose a reason for hiding this comment

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

@0xEbrahim The changes in all the other files apart from pkg/gofr/service/new_test.go are not necessary and also don't fall in the scope of this PR. Can you please remove them?

@0xEbrahim 0xEbrahim force-pushed the rewrite-new_test.go branch from b30fbe4 to 533fe02 Compare October 24, 2025 07:37
@0xEbrahim 0xEbrahim force-pushed the rewrite-new_test.go branch from 533fe02 to bbaddcc Compare October 24, 2025 07:46
@0xEbrahim
Copy link
Author

@Umang01-hash I think it is alright now, unless you have more comments

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.

3 participants