Skip to content

Commit 9d985bb

Browse files
CopilotJoannaaKL
andcommitted
Address code review feedback: consolidate transport implementations
- Remove unused OnAny method - Extract executeHandler helper to eliminate code duplication - Consolidate MockHTTPClientWithHandler to use MockHTTPClientWithHandlers - Simplify transport implementation (single multiHandlerTransport) - All tests and lint checks pass Co-authored-by: JoannaaKL <[email protected]>
1 parent 7ab8bcb commit 9d985bb

File tree

1 file changed

+22
-44
lines changed

1 file changed

+22
-44
lines changed

pkg/github/helper_test.go

Lines changed: 22 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -328,13 +328,6 @@ func (m *MockRoundTripper) OnRequest(method, path string, handler http.HandlerFu
328328
return m
329329
}
330330

331-
// OnAny registers a catch-all handler
332-
func (m *MockRoundTripper) OnAny(_ http.HandlerFunc) *MockRoundTripper {
333-
// This is a simple implementation that doesn't use the handler map
334-
// It's kept for compatibility but not recommended
335-
return m
336-
}
337-
338331
// NewMockHTTPClient creates an HTTP client with a mock transport
339332
func NewMockHTTPClient() (*http.Client, *MockRoundTripper) {
340333
transport := NewMockRoundTripper()
@@ -392,29 +385,28 @@ func matchPath(pattern, path string) bool {
392385
return true
393386
}
394387

395-
// MockHTTPClientWithHandler creates an HTTP client with a single handler function
396-
func MockHTTPClientWithHandler(handler http.HandlerFunc) *http.Client {
397-
transport := &mockTransport{handler: handler}
398-
return &http.Client{Transport: transport}
399-
}
400-
401-
type mockTransport struct {
402-
handler http.HandlerFunc
403-
}
404-
405-
func (m *mockTransport) RoundTrip(req *http.Request) (*http.Response, error) {
388+
// executeHandler executes an HTTP handler and returns the response
389+
func executeHandler(handler http.HandlerFunc, req *http.Request) *http.Response {
406390
recorder := &responseRecorder{
407391
header: make(http.Header),
408392
body: &bytes.Buffer{},
409393
}
410-
m.handler(recorder, req)
394+
handler(recorder, req)
411395

412396
return &http.Response{
413397
StatusCode: recorder.statusCode,
414398
Header: recorder.header,
415399
Body: io.NopCloser(bytes.NewReader(recorder.body.Bytes())),
416400
Request: req,
417-
}, nil
401+
}
402+
}
403+
404+
// MockHTTPClientWithHandler creates an HTTP client with a single handler function
405+
func MockHTTPClientWithHandler(handler http.HandlerFunc) *http.Client {
406+
handlers := map[string]http.HandlerFunc{
407+
"": handler, // Empty key acts as catch-all
408+
}
409+
return MockHTTPClientWithHandlers(handlers)
418410
}
419411

420412
// MockHTTPClientWithHandlers creates an HTTP client with multiple handlers for different paths
@@ -428,43 +420,29 @@ type multiHandlerTransport struct {
428420
}
429421

430422
func (m *multiHandlerTransport) RoundTrip(req *http.Request) (*http.Response, error) {
423+
// Check for catch-all handler
424+
if handler, ok := m.handlers[""]; ok {
425+
return executeHandler(handler, req), nil
426+
}
427+
431428
// Try to find a handler for this request
432429
key := req.Method + " " + req.URL.Path
433430

434431
// First try exact match
435432
if handler, ok := m.handlers[key]; ok {
436-
recorder := &responseRecorder{
437-
header: make(http.Header),
438-
body: &bytes.Buffer{},
439-
}
440-
handler(recorder, req)
441-
442-
return &http.Response{
443-
StatusCode: recorder.statusCode,
444-
Header: recorder.header,
445-
Body: io.NopCloser(bytes.NewReader(recorder.body.Bytes())),
446-
Request: req,
447-
}, nil
433+
return executeHandler(handler, req), nil
448434
}
449435

450436
// Then try pattern matching
451437
for pattern, handler := range m.handlers {
438+
if pattern == "" {
439+
continue // Skip catch-all
440+
}
452441
parts := strings.SplitN(pattern, " ", 2)
453442
if len(parts) == 2 {
454443
method, pathPattern := parts[0], parts[1]
455444
if req.Method == method && matchPath(pathPattern, req.URL.Path) {
456-
recorder := &responseRecorder{
457-
header: make(http.Header),
458-
body: &bytes.Buffer{},
459-
}
460-
handler(recorder, req)
461-
462-
return &http.Response{
463-
StatusCode: recorder.statusCode,
464-
Header: recorder.header,
465-
Body: io.NopCloser(bytes.NewReader(recorder.body.Bytes())),
466-
Request: req,
467-
}, nil
445+
return executeHandler(handler, req), nil
468446
}
469447
}
470448
}

0 commit comments

Comments
 (0)