Enhanced : Test for pkg/gofr/websocket module #2290
Conversation
There was a problem hiding this comment.
Here are some areas for improvement in the tests:
-
Error Handling Coverage:
- Some tests, like
TestConnection_Bind_ErrorPaths, could include more diverse scenarios for error handling, such as network interruptions or unexpected server responses.
- Some tests, like
-
Mock Usage:
- Some unrelated tests use real WebSocket connections unnecessarily. Mocking could improve test speed and isolation, especially in
TestConnection_BindandTestConnection_ReadMessage. Please note not necessarily for all tests.
- Some unrelated tests use real WebSocket connections unnecessarily. Mocking could improve test speed and isolation, especially in
-
Test Duplication:
- There is some duplication in setup code for WebSocket servers. Extracting this into reusable helper functions could improve maintainability. Add T.Helper for them.
-
Unimplemented Methods:
- Tests for unimplemented methods like
ParamandPathParamare redundant unless these methods are expected to be implemented in the future.
- Tests for unimplemented methods like
-
Test Coverage for Options:
- Ensure all combinations of options in
NewWSUpgraderare tested, especially for conflicting or invalid configurations. - Conflicting options should give error. (this will require real websocket connection)
Other than this, @thzgajendra, Good work !
- Ensure all combinations of options in
Test Improvements in WebSocket PackageThis PR addresses multiple gaps and improvements in the WebSocket test suite:
Overall, this PR improves coverage, maintainability, and reliability of the WebSocket test suite while removing unnecessary or redundant tests. |
There was a problem hiding this comment.
- All the Mock Tests, Constants Tests and Performance Tests can be removed as :
- Mock Tests are not contributing to anything in coverage.
- Performance tests are already set to skip in short mode, indicating they're not core tests
- Constants Tests just verifies constant values
| @@ -1,9 +1,7 @@ | |||
| // Code generated by MockGen. DO NOT EDIT. | |||
| // Source: interfaces.go | |||
There was a problem hiding this comment.
Why they are being removed??
pkg/gofr/websocket/websocket_test.go
Outdated
| data = new(string) | ||
| default: | ||
| data = &map[string]any{} | ||
| if tt.expectError { |
There was a problem hiding this comment.
Test methods should not contains if else statements like this. If required we can break the method into two methods one for success case and other for error.
pkg/gofr/websocket/websocket_test.go
Outdated
| defer wg.Done() | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| conn, err := upgrader.Upgrade(w, r, nil) | ||
| if tt.expectError { |
There was a problem hiding this comment.
Test methods should not contains if else statements like this. If required we can break the method into two methods one for success case and other for error.
pkg/gofr/websocket/websocket_test.go
Outdated
| dialer := websocket.DefaultDialer | ||
| conn, resp, err := dialer.Dial(url, nil) | ||
|
|
||
| if tt.expectError { |
There was a problem hiding this comment.
Remove this if else also please
pkg/gofr/websocket/websocket_test.go
Outdated
| connections := manager.ListConnections() | ||
| server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| conn, err := upgrader.Upgrade(w, r, nil) | ||
| if tt.expectError { |
There was a problem hiding this comment.
Here also we can break it into two method with and without error but no if else like this.
pkg/gofr/websocket/websocket_test.go
Outdated
|
|
||
| mockConn := &Connection{Conn: &websocket.Conn{}} | ||
| manager.AddWebsocketConnection("testService", mockConn) | ||
| if tt.expectError { |
pkg/gofr/websocket/websocket_test.go
Outdated
| wsConn := &Connection{Conn: conn} | ||
| err = wsConn.Bind(tt.targetType) | ||
|
|
||
| if tt.expectError { |
|
Sure , I'll Update these recommended changes. |
thzgajendra
force-pushed
the
add/test
branch
from
6a4960e to
28531ee
Compare
WebSocket Package: 100% Test Coverage & Linter Fixes
📋 Overview
This PR implements comprehensive test coverage for the
pkg/gofr/websocketpackage, achieving 100% test coverage while fixing all linter issues. The changes include extensive test cases covering all functionality, edge cases, performance scenarios, and proper error handling.🎯 Goals Achieved
📊 Test Coverage Results
Coverage Breakdown:
🔧 Changes Made
1. Consolidated Test File
pkg/gofr/websocket/websocket_test.go2. Core Functionality Tests
3. Edge Case Coverage
🚀 Key Features Tested
Thread Safety
Error Handling
Performance
WebSocket Protocol
🧪 Test Execution
Run All Tests
Run with Coverage
go test -cover -coverprofile=coverage.out go tool cover -func=coverage.outRun Performance Tests
go test -v -short=falseRun Linter
golangci-lint run .📈 Metrics
🔍 Code Quality Improvements
Error Handling
Code Organization
Performance
✅ Checklist
Ready for Review ✅