Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor: Use httptest for HTTP layer testing #1092

Merged
merged 4 commits into from
Jan 28, 2025

Conversation

jcscottiii
Copy link
Collaborator

@jcscottiii jcscottiii commented Jan 24, 2025

This PR improves the reliability and maintainability of our HTTP tests by transitioning from direct calls to the generated OpenAPI server methods to using the httptest package for true HTTP-level testing.

Motivation:

Currently, our tests bypass the HTTP layer, which limits their ability to identify issues with request handling, middleware, and serialization. This refactor provides a more accurate representation of real-world scenarios and enables better testing of components like the upcoming auth middleware.

Changes:

  • httptest Integration: All HTTP handler tests now utilize httptest to simulate actual HTTP requests and responses. This involves updating test inputs to *http.Request and expected outputs to *http.Response.
  • Centralized Assertions: Common assertion logic has been extracted into helper functions like assertTestServerRequest and assertMockCallCount to reduce code duplication and improve consistency across tests.
  • OpenAPI Updates: Added TODO notes to deprecate outdated parameters in the OpenAPI document to maintain API consistency with other parameters.

Benefits:

  • Increased Test Accuracy: Tests now accurately reflect the HTTP request lifecycle.
  • Improved Code Maintainability: Centralized assertions reduce redundancy and simplify test updates.
  • Enhanced Testability: Enables easier testing of middleware and other HTTP-related components.

Future Work:

  • This refactor facilitates the implementation of the auth middleware in a subsequent PR. Needed for the saved searches work.

@jcscottiii jcscottiii added this pull request to the merge queue Jan 28, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 28, 2025
Right now, our http unit tests aren't really http tests. They were great
go get started with it is not the long term solution. Also, I am looking
at adding the auth middleware, I will need to change some things around.
It would be great to actually assert the http layer before those changes.
@jcscottiii jcscottiii force-pushed the jcscottiii/http-test-helpers branch from 29e8f21 to 576af3b Compare January 28, 2025 12:42
@jcscottiii jcscottiii enabled auto-merge January 28, 2025 12:43
@jcscottiii jcscottiii added this pull request to the merge queue Jan 28, 2025
Merged via the queue into main with commit 1455feb Jan 28, 2025
6 checks passed
@jcscottiii jcscottiii deleted the jcscottiii/http-test-helpers branch January 28, 2025 13:20
@past past mentioned this pull request Jan 29, 2025
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