Add comprehensive Go tests for agent and supervisor - #49
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds extensive unit tests across Agent and Supervisor services, plus a new TEST_IMPLEMENTATION_SUMMARY.md document and multiple documentation edits. Also includes one architectural API change (StartEnvironment request model documented) and numerous non-functional formatting/content updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTP_Handler as StartEnvironmentHandler
participant Service as EnvironmentService
participant Azure as ACI/Cloud
Note over HTTP_Handler,Service: New structured request flow (StartEnvironmentRequest)
Client->>HTTP_Handler: POST /environments/{id}/start { cloudRegion, aciContainerGroupId }
HTTP_Handler->>Service: StartEnvironment(ctx, StartEnvironmentRequest)
Service->>Azure: provision/start container group (async/SDK)
Azure-->>Service: provision result / errors
Service-->>HTTP_Handler: success / error
HTTP_Handler-->>Client: 200/4xx/5xx JSON response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (35)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/agent/internal/middleware/cors_test.go (1)
64-73: Tighten checks for disallowed origins.Right now the disallowed-origin case never asserts that
Access-Control-Allow-Originstays unset, so a regression that mistakenly whitelists every origin would pass unnoticed. Please assert the negative path explicitly.if tt.wantAllowed { if allowOrigin != tt.origin && allowOrigin != "*" { t.Errorf("CORS header not set for allowed origin %s", tt.origin) } + } else if allowOrigin != "" { + t.Errorf("CORS header should be empty for disallowed origin %s, got %q", tt.origin, allowOrigin) }TEST_IMPLEMENTATION_SUMMARY.md (1)
14-37: Fix markdown lint warnings.Markdownlint is flagging (a) missing blank lines around your tables, (b) bare URLs, and (c) code fences without a language. Adding the blank lines keeps the tables rendering consistently, wrapping bare URLs in link text prevents MD034, and tagging the directory listings as
textclears MD040.## Test Coverage Summary ### Agent Service (apps/agent) + | Package | Coverage | Test File | Key Tests | |---------|----------|-----------|-----------| ... ### Supervisor Service (apps/supervisor) + | Package | Coverage | Test File | Key Tests |-- **PR**: https://github.com/VAIBHAVSING/Dev8.dev/pull/49 +- **PR**: [VAIBHAVSING/Dev8.dev#49](https://github.com/VAIBHAVSING/Dev8.dev/pull/49)-``` +```text apps/agent/internal/ ... -``` +```Repeat the
textfence tag for the supervisor tree as well to silence MD040. Based on static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
TEST_IMPLEMENTATION_SUMMARY.md(1 hunks)apps/agent/internal/azure/client_test.go(1 hunks)apps/agent/internal/azure/storage_test.go(1 hunks)apps/agent/internal/config/config_test.go(1 hunks)apps/agent/internal/handlers/environment_test.go(1 hunks)apps/agent/internal/handlers/health_test.go(1 hunks)apps/agent/internal/middleware/cors_test.go(1 hunks)apps/agent/internal/middleware/logging_test.go(1 hunks)apps/agent/internal/models/environment_test.go(1 hunks)apps/agent/internal/services/environment_test.go(1 hunks)apps/supervisor/internal/backup/manager_test.go(1 hunks)apps/supervisor/internal/config/config_test.go(1 hunks)apps/supervisor/internal/monitor/monitor_test.go(1 hunks)apps/supervisor/internal/monitor/state_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-30T18:07:52.326Z
Learnt from: CR
PR: VAIBHAVSING/Dev8.dev#0
File: agent/AGENT.md:0-0
Timestamp: 2025-09-30T18:07:52.326Z
Learning: Applies to agent/apps/agent/**/*_test.go : Include unit tests for new Go HTTP handlers
Applied to files:
apps/agent/internal/middleware/logging_test.goapps/agent/internal/handlers/health_test.goapps/agent/internal/middleware/cors_test.goapps/agent/internal/handlers/environment_test.go
🧬 Code graph analysis (13)
apps/agent/internal/services/environment_test.go (2)
apps/agent/internal/services/environment.go (5)
EnvironmentService(17-22)s(64-158)s(238-276)s(168-200)generateContainerGroupName(336-338)apps/agent/internal/config/config.go (1)
AzureConfig(32-42)
apps/agent/internal/middleware/logging_test.go (1)
apps/agent/internal/middleware/logging.go (2)
LoggingMiddleware(10-31)rw(40-43)
apps/agent/internal/azure/client_test.go (2)
apps/agent/internal/azure/client.go (3)
ContainerGroupSpec(229-240)NewClient(22-49)Client(15-19)apps/agent/internal/config/config.go (2)
AzureConfig(32-42)RegionConfig(45-51)
apps/agent/internal/azure/storage_test.go (1)
apps/agent/internal/azure/storage.go (2)
NewStorageClient(23-44)StorageClient(16-20)
apps/agent/internal/models/environment_test.go (1)
apps/agent/internal/models/environment.go (17)
ActivityReport(90-94)ActivitySnapshot(82-87)StatusCreating(9-9)StatusStarting(10-10)StatusRunning(11-11)StatusStopping(12-12)StatusStopped(13-13)StatusError(14-14)StatusDeleting(15-15)ProviderAzure(22-22)ProviderAWS(23-23)ProviderGCP(24-24)ErrInvalidRequest(162-164)ErrNotFound(166-168)ErrInternalServer(170-172)ErrUnauthorized(174-176)AppError(152-155)
apps/agent/internal/handlers/health_test.go (1)
apps/agent/internal/handlers/health.go (1)
NewHealthHandler(14-18)
apps/supervisor/internal/backup/manager_test.go (2)
apps/supervisor/internal/config/config.go (1)
BackupConfig(23-30)apps/supervisor/internal/monitor/state.go (1)
State(9-17)
apps/supervisor/internal/monitor/state_test.go (1)
apps/supervisor/internal/monitor/state.go (2)
State(9-17)Snapshot(42-47)
apps/agent/internal/middleware/cors_test.go (1)
apps/agent/internal/middleware/cors.go (1)
CORSMiddleware(9-44)
apps/supervisor/internal/config/config_test.go (1)
apps/supervisor/internal/config/config.go (3)
BackupConfig(23-30)MountConfig(33-44)AgentConfig(53-60)
apps/agent/internal/handlers/environment_test.go (2)
apps/agent/internal/models/environment.go (1)
AppError(152-155)apps/agent/internal/handlers/environment.go (1)
EnvironmentHandler(14-16)
apps/supervisor/internal/monitor/monitor_test.go (1)
apps/supervisor/internal/monitor/state.go (2)
State(9-17)Snapshot(42-47)
apps/agent/internal/config/config_test.go (1)
apps/agent/internal/config/config.go (2)
AzureConfig(32-42)RegionConfig(45-51)
🪛 LanguageTool
TEST_IMPLEMENTATION_SUMMARY.md
[grammar] ~3-~3: There might be a mistake here.
Context: ...o Test Suite Implementation ## Overview This document summarizes the comprehensi...
(QB_NEW_EN)
[grammar] ~6-~6: There might be a mistake here.
Context: ...and supervisor). ## Implementation Date October 16, 2025 ## Branch & PR - **Bra...
(QB_NEW_EN)
[grammar] ~9-~9: There might be a mistake here.
Context: ...on Date October 16, 2025 ## Branch & PR - Branch: add-comprehensive-go-tests -...
(QB_NEW_EN)
[grammar] ~10-~10: There might be a mistake here.
Context: ... 16, 2025 ## Branch & PR - Branch: add-comprehensive-go-tests - PR: https://github.com/VAIBHAVSING/Dev...
(QB_NEW_EN)
[grammar] ~11-~11: There might be a mistake here.
Context: .../github.com//pull/49 - Commit: Add comprehensive tests for ag...
(QB_NEW_EN)
[grammar] ~26-~26: There might be a mistake here.
Context: ...tainer images | Total Test Files: 9 Total Test Cases: 50+ ### Superviso...
(QB_NEW_EN)
[grammar] ~29-~29: There might be a mistake here.
Context: ...### Supervisor Service (apps/supervisor) | Package | Coverage | Test File | Key T...
(QB_NEW_EN)
[grammar] ~30-~30: There might be a mistake here.
Context: ...age | Coverage | Test File | Key Tests | |---------|----------|-----------|------...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...----|----------|-----------|-----------| | monitor | 77.6% | monitor_test.go, sta...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...hot immutability, reporter integration | | config | 71.8% | config_test.go | Conf...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ... credential masking, backup exclusions | | backup | 65.6% | manager_test.go | Bac...
(QB_NEW_EN)
[grammar] ~36-~36: There might be a mistake here.
Context: ...y calculation | Total Test Files: 4 Total Test Cases: 40+ ## Test Desig...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ...gn Principles ### 1. Table-Driven Tests All tests use Go's table-driven pattern ...
(QB_NEW_EN)
[grammar] ~54-~54: There might be a mistake here.
Context: ...``` ### 2. Edge Cases & Error Scenarios - Empty/nil values - Invalid formats - Mis...
(QB_NEW_EN)
[grammar] ~55-~55: There might be a mistake here.
Context: ...ses & Error Scenarios - Empty/nil values - Invalid formats - Missing required field...
(QB_NEW_EN)
[grammar] ~56-~56: There might be a mistake here.
Context: ...ios - Empty/nil values - Invalid formats - Missing required fields - Boundary condi...
(QB_NEW_EN)
[grammar] ~57-~57: There might be a mistake here.
Context: ...nvalid formats - Missing required fields - Boundary conditions - Concurrent access ...
(QB_NEW_EN)
[grammar] ~58-~58: There might be a mistake here.
Context: ...ng required fields - Boundary conditions - Concurrent access patterns ### 3. Azure...
(QB_NEW_EN)
[grammar] ~61-~61: There might be a mistake here.
Context: ...ccess patterns ### 3. Azure SDK Mocking Tests avoid requiring real Azure credent...
(QB_NEW_EN)
[grammar] ~62-~62: There might be a mistake here.
Context: ...oid requiring real Azure credentials by: - Testing method signatures without actual...
(QB_NEW_EN)
[grammar] ~63-~63: There might be a mistake here.
Context: ...thod signatures without actual API calls - Using skip directives for integration te...
(QB_NEW_EN)
[grammar] ~64-~64: There might be a mistake here.
Context: ...ng skip directives for integration tests - Validating error handling and data struc...
(QB_NEW_EN)
[grammar] ~67-~67: There might be a mistake here.
Context: ...d data structures ### 4. Docker Context Supervisor tests account for container e...
(QB_NEW_EN)
[grammar] ~68-~68: There might be a mistake here.
Context: ...tests account for container environment: - Proper logger initialization - Context c...
(QB_NEW_EN)
[grammar] ~69-~69: There might be a mistake here.
Context: ...ironment: - Proper logger initialization - Context cancellation handling - File sys...
(QB_NEW_EN)
[grammar] ~70-~70: There might be a mistake here.
Context: ...lization - Context cancellation handling - File system operations in temp directori...
(QB_NEW_EN)
[grammar] ~75-~75: There might be a mistake here.
Context: ... ## Key Test Highlights ### Agent Tests 1. Config Multi-Region Support: Tests par...
(QB_NEW_EN)
[grammar] ~82-~82: There might be a mistake here.
Context: ...t, NotFound, etc.) ### Supervisor Tests 1. Concurrent State Access: Tests thread-...
(QB_NEW_EN)
[grammar] ~86-~86: Use a hyphen to join words.
Context: ...ic**: Tests activity-based sync decision making 5. Credential Masking: Valida...
(QB_NEW_EN_HYPHEN)
[grammar] ~178-~178: There might be a mistake here.
Context: ...g - ✅ Cleanup with t.TempDir() and defer - ✅ Clear test failure messages - ✅ Separa...
(QB_NEW_EN)
[grammar] ~179-~179: There might be a mistake here.
Context: ...nd defer - ✅ Clear test failure messages - ✅ Separate integration tests with skip d...
(QB_NEW_EN)
[grammar] ~187-~187: There might be a mistake here.
Context: .../github.com//pull/49
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.18.1)
TEST_IMPLEMENTATION_SUMMARY.md
11-11: Bare URL used
(MD034, no-bare-urls)
17-17: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
30-30: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
120-120: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
140-140: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
154-154: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
187-187: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (14)
apps/supervisor/internal/config/config_test.go (3)
110-114: Validate the test logic for empty value handling.The test sets the environment variable to an empty string (
tt.value != ""), but then expectsgetEnvto return the fallback. Verify thatgetEnvtreats an empty string as missing and returns the fallback, as this behavior is not standard in all implementations.
260-307: LGTM! Excellent security-focused testing.The tests for
EffectiveMountCredentialsinclude explicit checks to ensure sensitive data (account keys and SAS tokens) are not leaked. This is a critical security practice for credential handling.
100-105: ****The
getEnvfunction at line 124 ofconfig.goexplicitly trims whitespace:if value := strings.TrimSpace(os.Getenv(key)); value != "" {. The test case "whitespace value" correctly expects the fallback when the environment variable contains only whitespace, and this behavior is properly implemented. No action needed.Likely an incorrect or invalid review comment.
apps/agent/internal/models/environment_test.go (1)
8-118: LGTM! Comprehensive validation test coverage.The test cases thoroughly cover required fields, boundary conditions (CPU cores, memory, storage), and default value handling. The table-driven approach makes the tests easy to understand and maintain.
apps/agent/internal/handlers/environment_test.go (1)
14-52: LGTM! Solid JSON response testing.The tests verify correct status codes, Content-Type headers, and valid JSON body structure for both success and created responses.
apps/agent/internal/config/config_test.go (2)
124-175: LGTM! Comprehensive region configuration testing.The tests cover multi-region configurations, single regions, empty input (default behavior), and malformed input. The edge case handling for malformed regions is particularly good.
177-213: LGTM! CORS origin parsing well-tested.The tests verify parsing of multiple origins, single origin, and the default fallback behavior when the environment variable is empty.
apps/agent/internal/azure/client_test.go (2)
37-61: LGTM! Appropriate handling of credential-dependent tests.The test correctly acknowledges that Azure credential creation will fail in test environments without real credentials. The logging approach (Lines 56-60) documents the expected behavior in both scenarios without causing test failures.
10-35: LGTM! Clear struct validation testing.The test verifies that
ContainerGroupSpecfields are correctly assigned, covering key fields like container name, CPU, and memory.apps/supervisor/internal/monitor/state_test.go (2)
67-95: LGTM! Excellent concurrency safety testing.The test launches 200 goroutines (100 each for IDE and SSH updates) plus 100 concurrent readers to verify thread-safety. This is essential for a component that will be accessed from multiple goroutines in production.
97-126: LGTM! Strong immutability verification.The test verifies that snapshots remain unchanged after state updates, which is critical for preventing unintended side effects when snapshots are shared across goroutines or stored.
apps/agent/internal/azure/storage_test.go (2)
52-97: LGTM! Comprehensive error classification testing.The tests cover multiple error scenarios for
isNotFoundError: nil errors, AzureResponseErrorwith 404/500 status codes, and string-based error detection. This ensures robust error handling across different error types.
99-127: LGTM! Appropriate use of skipped tests for credential-dependent methods.The test correctly uses
t.Skip()to document the method signatures without attempting actual Azure API calls, which would require credentials. This is a pragmatic approach for testing in environments without Azure access.apps/supervisor/internal/monitor/monitor_test.go (1)
10-30: LGTM! Comprehensive monitor initialization testing.The test verifies all configuration parameters (interval, IDE port, SSH port) are correctly set during monitor creation.
| func TestEnvironmentHandler_Routes(t *testing.T) { | ||
| // Create a mock environment service (would need proper mocking in production) | ||
| handler := &EnvironmentHandler{} | ||
|
|
||
| tests := []struct { | ||
| name string | ||
| method string | ||
| path string | ||
| body interface{} | ||
| setupVars func(*http.Request) *http.Request | ||
| }{ | ||
| { | ||
| name: "list environments", | ||
| method: "GET", | ||
| path: "/api/v1/environments", | ||
| body: nil, | ||
| }, | ||
| { | ||
| name: "create environment", | ||
| method: "POST", | ||
| path: "/api/v1/environments", | ||
| body: models.CreateEnvironmentRequest{ | ||
| Name: "test", | ||
| CloudRegion: "eastus", | ||
| CPUCores: 2, | ||
| MemoryGB: 4, | ||
| StorageGB: 100, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| var body []byte | ||
| var err error | ||
| if tt.body != nil { | ||
| body, err = json.Marshal(tt.body) | ||
| if err != nil { | ||
| t.Fatalf("Failed to marshal body: %v", err) | ||
| } | ||
| } | ||
|
|
||
| req := httptest.NewRequest(tt.method, tt.path, bytes.NewReader(body)) | ||
| req.Header.Set("Content-Type", "application/json") | ||
|
|
||
| if tt.setupVars != nil { | ||
| req = tt.setupVars(req) | ||
| } | ||
|
|
||
| w := httptest.NewRecorder() | ||
|
|
||
| // Note: This is a basic structure test | ||
| // In a real test, you'd call the actual handler methods | ||
| if tt.method == "GET" && tt.path == "/api/v1/environments" { | ||
| handler.ListEnvironments(w, req) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Prevent potential nil pointer panic in handler tests.
The test creates an EnvironmentHandler with a nil service at Line 124 and then calls handler.ListEnvironments(w, req) at Line 176. This will likely cause a nil pointer panic if the handler method attempts to use the service.
Consider one of these approaches:
- Mock the service properly:
// Create a mock service
mockService := &mockEnvironmentService{} // Define this type
handler := &EnvironmentHandler{service: mockService}- Or skip the actual handler call:
// Note: This is a basic structure test
// In a real test, you'd call the actual handler methods
if tt.method == "GET" && tt.path == "/api/v1/environments" {
// handler.ListEnvironments(w, req) // Skip for now
t.Skip("Handler requires mocked service")
}🤖 Prompt for AI Agents
In apps/agent/internal/handlers/environment_test.go around lines 122 to 180, the
test instantiates EnvironmentHandler with a nil service and then calls
handler.ListEnvironments which can panic; fix by providing a non-nil mock
service or avoiding the call: either construct a test mock that implements the
environment service interface and set handler := &EnvironmentHandler{service:
mockService} (create minimal methods used by ListEnvironments) before invoking
the handler, or change the test to skip calling handler.ListEnvironments (e.g.,
replace the call with a t.Skip) so no nil service is accessed.
| // Check default base image is set | ||
| if !tt.wantErr && tt.req.BaseImage == "" { | ||
| if tt.req.BaseImage != "node" { | ||
| t.Error("Validate() should set default base image to 'node'") | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix inverted logic in default base image check.
The test checks if tt.req.BaseImage == "" (empty) and then expects it to NOT equal "node". This logic is inverted. After validation, if the base image was empty, it should be set to the default "node". The condition should be:
if tt.req.BaseImage == "" {
t.Error("Validate() should set default base image to 'node'")
}Apply this diff to fix the logic:
// Check default base image is set
-if !tt.wantErr && tt.req.BaseImage == "" {
- if tt.req.BaseImage != "node" {
- t.Error("Validate() should set default base image to 'node'")
- }
-}
+if !tt.wantErr && tt.req.BaseImage == "" {
+ t.Error("Validate() should set default base image to 'node'")
+}🤖 Prompt for AI Agents
In apps/agent/internal/models/environment_test.go around lines 110 to 115, the
test's default base image assertion is inverted: it currently checks if
tt.req.BaseImage == "" and then fails if it is not "node". Change the logic so
that after validation, if tt.req.BaseImage == "" then call t.Error("Validate()
should set default base image to 'node'"); otherwise the test should pass when
BaseImage equals "node". Ensure the condition directly flags the empty value as
an error, reflecting that Validate() must populate the default.
| func TestMonitor_WithReporter(t *testing.T) { | ||
| logger := slog.Default() | ||
| state := &State{} | ||
| reporter := &mockReporter{} | ||
| monitor := New(logger, state, 100*time.Millisecond, reporter) | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) | ||
| defer cancel() | ||
|
|
||
| go monitor.Run(ctx) | ||
|
|
||
| // Give it time to sample a few times | ||
| time.Sleep(300 * time.Millisecond) | ||
|
|
||
| // Note: In a real environment, the reporter would be called | ||
| // Here we just verify the structure works | ||
| if reporter.reportCount < 0 { | ||
| t.Log("Reporter structure verified") | ||
| } | ||
| } |
There was a problem hiding this comment.
Fix reporter verification logic and timing issues.
The test has two problems:
-
Line 103: The sleep duration (300ms) exceeds the context timeout (250ms), so the monitor will have stopped before the sleep completes. This makes the reporter call timing unpredictable.
-
Line 107: The condition
reporter.reportCount < 0will always be false for an integer initialized to 0. This check doesn't actually verify the reporter was called.
Apply this diff to fix the test:
func TestMonitor_WithReporter(t *testing.T) {
logger := slog.Default()
state := &State{}
reporter := &mockReporter{}
- monitor := New(logger, state, 100*time.Millisecond, reporter)
+ monitor := New(logger, state, 50*time.Millisecond, reporter)
- ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond)
+ ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
defer cancel()
go monitor.Run(ctx)
- // Give it time to sample a few times
- time.Sleep(300 * time.Millisecond)
+ // Wait for context to complete
+ time.Sleep(250 * time.Millisecond)
- // Note: In a real environment, the reporter would be called
- // Here we just verify the structure works
- if reporter.reportCount < 0 {
- t.Log("Reporter structure verified")
+ // Verify reporter was called at least once
+ if reporter.reportCount == 0 {
+ t.Error("Reporter was not called during monitoring")
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestMonitor_WithReporter(t *testing.T) { | |
| logger := slog.Default() | |
| state := &State{} | |
| reporter := &mockReporter{} | |
| monitor := New(logger, state, 100*time.Millisecond, reporter) | |
| ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) | |
| defer cancel() | |
| go monitor.Run(ctx) | |
| // Give it time to sample a few times | |
| time.Sleep(300 * time.Millisecond) | |
| // Note: In a real environment, the reporter would be called | |
| // Here we just verify the structure works | |
| if reporter.reportCount < 0 { | |
| t.Log("Reporter structure verified") | |
| } | |
| } | |
| func TestMonitor_WithReporter(t *testing.T) { | |
| logger := slog.Default() | |
| state := &State{} | |
| reporter := &mockReporter{} | |
| monitor := New(logger, state, 50*time.Millisecond, reporter) | |
| ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) | |
| defer cancel() | |
| go monitor.Run(ctx) | |
| // Wait for context to complete | |
| time.Sleep(250 * time.Millisecond) | |
| // Verify reporter was called at least once | |
| if reporter.reportCount == 0 { | |
| t.Error("Reporter was not called during monitoring") | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/supervisor/internal/monitor/monitor_test.go around lines 91 to 110, the
test sleeps longer than the context timeout and uses an always-false check for
the reporter call; reduce the sleep to be shorter than the context timeout
(e.g., 200ms or less) so the monitor is still running during the assertion, and
replace the meaningless condition `reporter.reportCount < 0` with a proper
assertion that the reporter was invoked (e.g., `if reporter.reportCount == 0 {
t.Fatalf("expected reporter to be called, got 0") }`), or use a small loop/wait
with a deadline to poll until reportCount > 0 to avoid flakiness.
Summary
Adds comprehensive test coverage for both the agent and supervisor Go services.
Changes
Agent Tests (apps/agent)
Supervisor Tests (apps/supervisor)
Test Details
All tests:
Azure Integration
Tests for Azure SDK operations use mocks to avoid requiring real Azure credentials during testing. Integration tests with actual Azure resources should be run separately in CI/CD pipeline.
Running Tests
Coverage Summary
Notes
Summary by CodeRabbit
Documentation
Tests
Refactor