[POC] Unit test Gin router and handler together

[Yiling-J]

Problem:

The routing registration section in api.go lacks unit tests and is impossible to test. This is because all the routing registrations and handler initializations are done inside the NewRouter method, making it impossible to mock or use dependency injection. The routing registration should also be included in unit tests, as route handling is entirely dependent on string operations and lacks type safety constraints.

Industry Approach (Example: Grafana):

Route Registration:

• Grafana uses the HTTPServer struct, where all services are registered on the HTTPServer: http_server.go
• Grafana does not have a separate handler layer. All route handlers are registered directly within the HTTPServer. For example: api/user.go
• During route registration, HTTPServer is first initialized via wire, and then the registerRoutes method is called to register routes: api/api.go

Route Testing:

Grafana, a 10-year-old project, uses various methods to test routes. I prefer the following one: api/quota_test.go

• A dedicated test HTTPServer is created using SetupAPITestServer, and all routes are registered. SetupAPITestServer supports passing ops functions (func(hs *HTTPServer)), which helps mock various services that the HTTPServer depends on.
• While writing tests, required services are mocked and passed through the ops function.
• HTTP requests are built using actual API paths, with headers or body added as necessary.
• During tests, methods like server.NewGetRequest, server.NewPostRequest are used to send requests. Since routes have already been registered, the system will automatically match the request’s URL and method to execute the corresponding handler.

Current Status of CSGHub Server:

• All route registration and handler initialization are done inside the NewRouter method, making it impossible to mock or use dependency injection.
• There is a separate handler layer, and all routes are registered within the corresponding handler’s method.
• Unit tests for the handler layer already exist.
• Current handler unit tests manually build the gin.CreateTestContext, which is unrelated to the route layer and does not include route-matching logic.

Refactoring Objective:

Since there is a handler layer, we cannot directly adopt Grafana’s approach. The least intrusive method is to complete route initialization and testing within the handler’s unit tests and replace gin.CreateTestContext with real requests.

Refactoring Details:

• The router layer will introduce dependency injection by adding a Server interface and BaseServer/ServerImpl structs, placing all handlers under the Server struct. Because there are many handlers, wire will be used for automatic injection, separating handler initialization from route registration.
• wire will generate injection files for ce, ee, and saas.
• A NewTestServer method will be added, similar to Grafana’s SetupAPITestServer, to create a test server and inject mock handlers via an option function.
• Common methods like NewGetRequest, NewPostRequest, etc., will be added to the TestServer.
• The handler/discussion_test.go and handler/repo_test.go files will be modified as examples of the new test approach.
• Middleware will be refactored to use struct initialization. Since middleware depends on userComponent and mirrorComponent, these components must be injected to test the router.

Important Notes:

• Handlers must be structs, not interfaces. Since route methods are registered on handler methods, if the handler is a struct, it can be nil and still register correctly. However, if it’s an interface, registering a nil interface will fail.
• When rewriting the existing handler tests using the new approach, be sure to change the package name from package handler to package handler_test. This is because the router layer sits above the handler layer, and not changing the package will cause a circular reference.
• When modifying the BaseServer in api.go or the ServerImpl structure in api_ce.go/api_ee.go/api_saas.go (modifications like adding or deleting fields), run make wire again to regenerate the mocks.

[starship-github]

🚀 The StarShip CodeReviewer has been triggered with action(s): review, evaluate, describe, linter.

[starship-github]

The StarShip CodeReviewer was triggered but terminated because it encountered an issue: review of the Merge Request (MR) could not be completed successfully.

Tips

CodeReview Commands (invoked as MR or PR comments)

• @codegpt /review to trigger an code review.
• @codegpt /evaluate to trigger code evaluation process.
• @codegpt /describe to regenerate the summary of the MR.
• @codegpt /secscan to scan security vulnerabilities for the MR or the Repository.
• @codegpt /help to get help.

CodeReview Discussion Chat

There are 2 ways to chat with Starship CodeReview:

• Review comments: Directly reply to a review comment made by StarShip.
  Example:
  • @codegpt How to fix this bug?
• Files and specific lines of code (under the "Files changed" tab):
  Tag @codegpt in a new review comment at the desired location with your query.
  Examples:
  • @codegpt generate unit testing code for this code snippet.

Note: Be mindful of the bot's finite context window.
It's strongly recommended to break down tasks such as reading entire modules into smaller chunks.
For a focused discussion, use review comments to chat about specific files and their changes, instead of using the MR/PR comments.

CodeReview Documentation and Community

• Visit our Documentation
  for detailed information on how to use Starship CodeReview.