refactor: consolidate test files and remove internal package dependencies#4
refactor: consolidate test files and remove internal package dependencies#4
Conversation
…cies This commit consolidates test files across examples and removes dependencies on internal packages: Test File Consolidation: - Merged counter example tests into counter_test.go (E2E + WebSocket) - Merged todos example tests into todos_test.go (E2E + WebSocket) - Merged avatar-upload example tests into avatar-upload_test.go (E2E + WebSocket) - Deleted original separate test files after merging Internal Package Fixes: - observability: replaced internal/observe with standard slog - production/single-host: replaced internal/observe with local trace middleware - trace-correlation: replaced internal/observe with local trace middleware - Added local TraceMiddleware and LoggerWithTraceID implementations All 8 examples now pass their tests successfully. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR consolidates test files across example projects and removes dependencies on internal packages to improve maintainability. The changes successfully merge multiple test files into single, well-organized test files per example, and replace internal package dependencies with local implementations or standard library equivalents.
Key changes:
- Consolidated test files in counter, todos, and avatar-upload examples into single
*_test.gofiles per example - Replaced
internal/observepackage usage with standardslogand local middleware implementations - Updated
e2etest.StopDockerChrome()API calls to use new signature (removed*exec.Cmdparameter)
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| counter/counter_test.go | Consolidates E2E and WebSocket tests with TestMain setup |
| counter/test_main_test.go | Deleted - merged into counter_test.go |
| counter/ws_test.go | Deleted - merged into counter_test.go |
| todos/todos_test.go | Consolidates E2E and WebSocket tests with TestMain setup |
| todos/test_main_test.go | Deleted - merged into todos_test.go |
| todos/ws_test.go | Deleted - merged into todos_test.go |
| avatar-upload/avatar-upload_test.go | Consolidates E2E and WebSocket upload tests |
| avatar-upload/upload_ws_e2e_test.go | Deleted - merged into avatar-upload_test.go |
| observability/main.go | Replaces internal/observe with standard slog |
| production/single-host/main.go | Adds local TraceMiddleware and LoggerWithTraceID implementations |
| trace-correlation/main.go | Adds local trace middleware implementations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Context key for trace ID | ||
| type contextKey string | ||
|
|
||
| const traceIDKey contextKey = "trace_id" | ||
|
|
||
| // TraceMiddleware adds a trace ID to the request context | ||
| func TraceMiddleware(next http.Handler) http.Handler { | ||
| return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
| // Generate or extract trace ID | ||
| traceID := r.Header.Get("X-Trace-ID") | ||
| if traceID == "" { | ||
| traceID = uuid.New().String() | ||
| } | ||
|
|
||
| // Add trace ID to response header | ||
| w.Header().Set("X-Trace-ID", traceID) | ||
|
|
||
| // Add trace ID to context | ||
| ctx := context.WithValue(r.Context(), traceIDKey, traceID) | ||
| next.ServeHTTP(w, r.WithContext(ctx)) | ||
| }) | ||
| } | ||
|
|
||
| // LoggerWithTraceID returns a logger with trace ID from context | ||
| func LoggerWithTraceID(logger *slog.Logger, ctx context.Context) *slog.Logger { | ||
| if traceID, ok := ctx.Value(traceIDKey).(string); ok { | ||
| return logger.With("trace_id", traceID) | ||
| } | ||
| return logger | ||
| } | ||
|
|
||
| // GetTraceID extracts the trace ID from context | ||
| func GetTraceID(ctx context.Context) string { | ||
| if traceID, ok := ctx.Value(traceIDKey).(string); ok { | ||
| return traceID | ||
| } | ||
| return "" | ||
| } |
There was a problem hiding this comment.
The TraceMiddleware, LoggerWithTraceID, and GetTraceID implementations are duplicated across trace-correlation/main.go and production/single-host/main.go. Consider extracting this common tracing logic into a shared package or utility file to maintain consistency and reduce duplication. This would make future updates to the tracing implementation easier to manage.
| // Note: For production metrics, integrate with your preferred metrics system | ||
| // (Prometheus, StatsD, DataDog, etc.) by instrumenting the Change() method | ||
| if envConfig.MetricsEnabled { | ||
| go metrics.EmitPeriodically(30 * time.Second) | ||
| logger.Info("Metrics collection enabled - integrate with your metrics backend") | ||
| } |
There was a problem hiding this comment.
The comment and log message indicate metrics are 'enabled' but no actual metrics collection is implemented. This could be misleading for users expecting functional metrics collection. Consider clarifying that this is a placeholder requiring user implementation, or update the message to say 'metrics flag is set' rather than 'enabled'.
CI was failing because tests were calling StopDockerChrome with 2 parameters, but the remote lvt module (v0.0.0-20251103195948-fbcd6dfae2d0) expects 3 parameters: - StopDockerChrome(t *testing.T, cmd *exec.Cmd, debugPort int) Updated all test files to capture and pass the chromeCmd from StartDockerChrome. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary
This PR consolidates test files across examples and removes dependencies on internal packages to improve maintainability and fix import errors.
Test File Consolidation
test_main_test.go,counter_e2e_test.go, andws_test.gointocounter_test.gotest_main_test.go,todos_e2e_test.go, andws_test.gointotodos_test.goavatar_upload_e2e_test.goandupload_ws_e2e_test.gointoavatar-upload_test.go[example]_test.gonaming conventionInternal Package Dependency Removal
internal/observewith standardslogpackageTraceMiddlewareandLoggerWithTraceIDimplementationsGetTraceIDgithub.com/google/uuidimport for trace ID generationTest Plan
./test-all.shto verify all 8 examples passAll tests passing ✓
🤖 Generated with Claude Code