From ea783df79a5616d17b439e05bc8b0385f62e1bb7 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 9 Aug 2026 10:33:25 +0000 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=20Bolt:=20optimize=20concurrent=20mul?= =?UTF-8?q?ti-region=20listing=20operations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Optimizes listAllRegions using a lock-free, pre-allocated map-reduce pattern across services, jobs, domain mappings, and worker pools to avoid lock contention and memory resizing. Co-authored-by: JulienBreux <964330+JulienBreux@users.noreply.github.com> --- .jules/bolt.md | 4 + internal/run/api/client/client_test.go | 2 +- internal/run/api/domainmapping/client.go | 4 +- .../run/api/domainmapping/domainmapping.go | 44 +++-- .../api/domainmapping/domainmapping_test.go | 8 +- internal/run/api/job/execution/execution.go | 8 +- internal/run/api/job/job.go | 34 ++-- internal/run/api/job/job_test.go | 30 ++-- internal/run/api/log/log_test.go | 12 +- internal/run/api/project/project.go | 2 +- internal/run/api/project/project_test.go | 56 +++--- internal/run/api/service/revision/client.go | 2 +- .../run/api/service/revision/revision_test.go | 10 +- internal/run/api/service/service.go | 36 ++-- internal/run/api/service/service_test.go | 159 ++++++++++-------- internal/run/api/workerpool/workerpool.go | 35 ++-- .../run/api/workerpool/workerpool_test.go | 28 +-- internal/run/command/command_test.go | 4 +- internal/run/command/version/version_test.go | 4 +- .../run/model/common/resources/resources.go | 2 +- .../run/model/domainmapping/domainmapping.go | 16 +- internal/run/model/job/execution/execution.go | 32 ++-- internal/run/model/job/job.go | 48 +++--- internal/run/model/service/service_test.go | 4 +- internal/run/tui/app/app_test.go | 102 +++++------ .../run/tui/app/describe/describe_test.go | 26 +-- internal/run/tui/app/help/help_test.go | 2 +- internal/run/tui/app/job/dashboard_test.go | 4 +- internal/run/tui/app/log/log_test.go | 50 +++--- internal/run/tui/app/modal.go | 2 +- internal/run/tui/app/modal_test.go | 10 +- internal/run/tui/app/project/project_test.go | 56 +++--- internal/run/tui/app/region/region_test.go | 28 +-- .../run/tui/app/service/dashboard_test.go | 74 ++++---- .../tui/app/service/revision/revision_test.go | 24 +-- internal/run/tui/app/service/traffic/split.go | 30 ++-- .../run/tui/app/service/traffic/split_test.go | 2 +- internal/run/tui/component/header/header.go | 4 +- .../run/tui/component/header/header_test.go | 8 +- .../run/tui/component/loader/loader_test.go | 2 +- internal/run/tui/component/logo/logo_test.go | 4 +- .../run/tui/component/table/table_test.go | 2 +- pkg/dropdown/dropdown_test.go | 2 +- 43 files changed, 532 insertions(+), 484 deletions(-) diff --git a/.jules/bolt.md b/.jules/bolt.md index 91ce70b..4951efd 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -1,5 +1,9 @@ # Bolt's Journal +## 2026-03-09 - Lock-Free Pre-Allocated Map-Reduce for Multi-Regional Queries +**Learning:** Performing multi-regional queries (like `listAllRegions` across 24 GCP regions) concurrently using a shared slice protected by `sync.Mutex` causes lock contention, slice allocation churn, and heavy garbage collection overhead. Utilizing a lock-free, pre-allocated map-reduce pattern using a slice of slices (`[][]T`) eliminates mutex contention entirely, and pre-allocating the final result slice based on the exact sum of sizes of individual regional results reduces heap allocations considerably. +**Action:** Always favor lock-free, index-mapped pre-allocation arrays (`[][]T`) over shared mutex-protected slices (`[]T`) when collecting results concurrently from a known, fixed number of concurrent tasks. + ## 2026-03-08 - GCP Logging Client Caching & Connection Longevity **Learning:** Establishing the GCP Stackdriver Logging client requires repeated Google credential discovery and connection establishment, causing high latency (~300ms) inside a reactive TUI interface. Caching `logadmin.Client` instances via a project-aware map with thread-safe `sync.Mutex` ensures subsequent streaming and log extraction operations are instantaneous. Crucially, calling `Close()` on individual stream terminations must be a no-op to prevent premature teardown of connection pools shared across other active streaming views. **Action:** Keep GCP Logging clients cached globally by project and handle connection termination via a no-op `Close` method, while adding test-isolation resets in unit tests. diff --git a/internal/run/api/client/client_test.go b/internal/run/api/client/client_test.go index f10a4a5..c0ae2c0 100644 --- a/internal/run/api/client/client_test.go +++ b/internal/run/api/client/client_test.go @@ -57,7 +57,7 @@ func TestWrapError(t *testing.T) { assert.Error(t, got) assert.Contains(t, got.Error(), tt.wantSubString) if tt.wantWrapped { - assert.ErrorIs(t, got, tt.inputErr) // Checks if it wraps the original error + assert.ErrorIs(t, got, tt.inputErr) // Checks if it wraps the original error assert.NotEqual(t, tt.inputErr, got) // Should not be exactly the same object (wrapped) } else { assert.Equal(t, tt.inputErr, got) // Should be exactly the same diff --git a/internal/run/api/domainmapping/client.go b/internal/run/api/domainmapping/client.go index 9fe878c..a52d45e 100644 --- a/internal/run/api/domainmapping/client.go +++ b/internal/run/api/domainmapping/client.go @@ -116,11 +116,11 @@ func (c *GCPClient) ListDomainMappings(ctx context.Context, project, region stri } else { pageToken = "" } - + if pageToken == "" { break } } return domainMappings, nil -} \ No newline at end of file +} diff --git a/internal/run/api/domainmapping/domainmapping.go b/internal/run/api/domainmapping/domainmapping.go index 6379009..534db9f 100644 --- a/internal/run/api/domainmapping/domainmapping.go +++ b/internal/run/api/domainmapping/domainmapping.go @@ -21,10 +21,10 @@ import ( "sync" "time" - "google.golang.org/api/run/v1" api_region "github.com/JulienBreux/run-cli/internal/run/api/region" - model "github.com/JulienBreux/run-cli/internal/run/model/domainmapping" "github.com/JulienBreux/run-cli/internal/run/model/common/condition" + model "github.com/JulienBreux/run-cli/internal/run/model/domainmapping" + "google.golang.org/api/run/v1" ) var apiClient Client = &GCPClient{} @@ -50,25 +50,35 @@ func List(project, region string) ([]model.DomainMapping, error) { } func listAllRegions(project string) ([]model.DomainMapping, error) { - var ( - mu sync.Mutex - domainMappings []model.DomainMapping - wg sync.WaitGroup - ) - - for _, region := range api_region.List() { + // Optimization: lock-free, pre-allocated map-reduce pattern with a slice of slices [][]model.DomainMapping. + // This eliminates mutex contention across concurrent region requests, avoids repeated slice resizing/heap allocations, + // and optimizes concurrency to reduce latency and memory usage. + regions := api_region.List() + results := make([][]model.DomainMapping, len(regions)) + var wg sync.WaitGroup + + for i, region := range regions { wg.Add(1) - go func(r string) { + go func(idx int, r string) { defer wg.Done() if dms, err := List(project, r); err == nil { - mu.Lock() - domainMappings = append(domainMappings, dms...) - mu.Unlock() + results[idx] = dms } - }(region) + }(i, region) } wg.Wait() + + total := 0 + for _, dms := range results { + total += len(dms) + } + + domainMappings := make([]model.DomainMapping, 0, total) + for _, dms := range results { + domainMappings = append(domainMappings, dms...) + } + return domainMappings, nil } @@ -95,12 +105,12 @@ func mapDomainMapping(resp *run.DomainMapping, project, region string) model.Dom }) } } - + routeName := "" if resp.Spec != nil { routeName = resp.Spec.RouteName } - + createTime := time.Time{} name := "" creator := "" @@ -127,4 +137,4 @@ func mapDomainMapping(resp *run.DomainMapping, project, region string) model.Dom CreateTime: createTime, Conditions: conditions, } -} \ No newline at end of file +} diff --git a/internal/run/api/domainmapping/domainmapping_test.go b/internal/run/api/domainmapping/domainmapping_test.go index b85f5ca..01e05c4 100644 --- a/internal/run/api/domainmapping/domainmapping_test.go +++ b/internal/run/api/domainmapping/domainmapping_test.go @@ -149,7 +149,7 @@ func TestList_AllRegions(t *testing.T) { dms, err := List("p", "all") assert.NoError(t, err) - + // Since api_region.List() returns many regions, we just want to ensure we called List for them and aggregated results. // We mocked return for "us-central1". found := false @@ -212,7 +212,7 @@ func TestGCPClient_ListDomainMappings(t *testing.T) { ListFunc: func(parent string, pageToken string) (*run.ListDomainMappingsResponse, error) { if pageToken == "" { return &run.ListDomainMappingsResponse{ - Items: []*run.DomainMapping{{Metadata: &run.ObjectMeta{Name: "dm1"}}}, + Items: []*run.DomainMapping{{Metadata: &run.ObjectMeta{Name: "dm1"}}}, Metadata: &run.ListMeta{Continue: "next-page"}, }, nil } @@ -245,7 +245,7 @@ func TestGCPClient_ListDomainMappings(t *testing.T) { }) t.Run("ClientCreationError", func(t *testing.T) { - // Reset auth mock for this test + // Reset auth mock for this test client.FindDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { return &google.Credentials{}, nil } @@ -259,7 +259,7 @@ func TestGCPClient_ListDomainMappings(t *testing.T) { }) t.Run("ListError", func(t *testing.T) { - // Reset auth mock for this test + // Reset auth mock for this test client.FindDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { return &google.Credentials{}, nil } diff --git a/internal/run/api/job/execution/execution.go b/internal/run/api/job/execution/execution.go index 7d0bce9..466157d 100644 --- a/internal/run/api/job/execution/execution.go +++ b/internal/run/api/job/execution/execution.go @@ -55,7 +55,7 @@ func mapExecution(resp *runpb.Execution, region string) model.Execution { // We can try to find the "Completed" or "Succeeded" condition or check terminal condition logic if exposed directly. // runpb.Execution doesn't have a direct TerminalCondition field like Job, but it has Conditions. // We usually look for "Completed" type. - + // Helper to find latest relevant condition or just map all of them. var conditions []*condition.Condition for _, c := range resp.Conditions { @@ -66,7 +66,7 @@ func mapExecution(resp *runpb.Execution, region string) model.Execution { LastTransitionTime: c.LastTransitionTime.AsTime(), } conditions = append(conditions, cond) - + // Heuristic: If condition is "Completed", treat as terminal status for summary if c.Type == "Completed" { terminalCondition = cond @@ -151,10 +151,10 @@ func (c *GCPClient) ListExecutions(ctx context.Context, project, region, jobName // BUT, executions are children of Jobs? No, they are children of Location. // The resource name is projects/*/locations/*/jobs/*/executions/ -> v1 // v2: projects/*/locations/*/jobs/*/executions - + // Wait, `runpb.ListExecutionsRequest` expects Parent = `projects/{project}/locations/{location}/jobs/{job}` OR `projects/{project}/locations/{location}`. // If we can pass the job as parent, we get filtered list! - + parent := jobName if !strings.HasPrefix(jobName, "projects/") { parent = fmt.Sprintf("projects/%s/locations/%s/jobs/%s", project, region, jobName) diff --git a/internal/run/api/job/job.go b/internal/run/api/job/job.go index a3f8831..0bee04d 100644 --- a/internal/run/api/job/job.go +++ b/internal/run/api/job/job.go @@ -79,27 +79,37 @@ func mapJob(resp *runpb.Job, region string) model.Job { } func listAllRegions(project string) ([]model.Job, error) { - var ( - mu sync.Mutex - jobs []model.Job - wg sync.WaitGroup - ) - - for _, region := range api_region.List() { + // Optimization: lock-free, pre-allocated map-reduce pattern with a slice of slices [][]model.Job. + // This eliminates mutex contention across concurrent region requests, avoids repeated slice resizing/heap allocations, + // and optimizes concurrency to reduce latency and memory usage. + regions := api_region.List() + results := make([][]model.Job, len(regions)) + var wg sync.WaitGroup + + for i, region := range regions { wg.Add(1) - go func(r string) { + go func(idx int, r string) { defer wg.Done() // Call List recursively for each region // We ignore errors here to allow partial success (e.g. if one region is down or disabled) if j, err := List(project, r); err == nil { - mu.Lock() - jobs = append(jobs, j...) - mu.Unlock() + results[idx] = j } - }(region) + }(i, region) } wg.Wait() + + total := 0 + for _, j := range results { + total += len(j) + } + + jobs := make([]model.Job, 0, total) + for _, j := range results { + jobs = append(jobs, j...) + } + return jobs, nil } diff --git a/internal/run/api/job/job_test.go b/internal/run/api/job/job_test.go index 5dbfbaa..2c5542d 100644 --- a/internal/run/api/job/job_test.go +++ b/internal/run/api/job/job_test.go @@ -74,12 +74,12 @@ func TestMapJob(t *testing.T) { assert.Equal(t, resp.Name, result.Name) assert.Equal(t, "user@example.com", result.Creator) assert.Equal(t, "us-central1", result.Region) - + // Execution assert.NotNil(t, result.LatestCreatedExecution) assert.Equal(t, resp.LatestCreatedExecution.Name, result.LatestCreatedExecution.Name) assert.Equal(t, now.Unix(), result.LatestCreatedExecution.CreateTime.Unix()) - + // Condition assert.NotNil(t, result.TerminalCondition) assert.Equal(t, "CONDITION_SUCCEEDED", result.TerminalCondition.State) @@ -186,7 +186,7 @@ func TestList_AllRegions(t *testing.T) { jobs, err := List("p", api_region.ALL) assert.NoError(t, err) - + found := false for _, j := range jobs { if j.Name == "job-us" && j.Region == "us-central1" { @@ -295,7 +295,7 @@ func TestGCPClient_ListJobs(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "failed to find default credentials") }) - + t.Run("Client Creation Error", func(t *testing.T) { client.FindDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { return &google.Credentials{}, nil @@ -328,7 +328,7 @@ func TestGCPClient_ListJobs(t *testing.T) { assert.Error(t, err) assert.Contains(t, err.Error(), "iter error") }) - + t.Run("Iterator Auth Error", func(t *testing.T) { client.FindDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { return &google.Credentials{}, nil @@ -357,7 +357,7 @@ func TestGCPClient_RunJob(t *testing.T) { client.FindDefaultCredentials = origFindCreds createJobsClient = origCreateClient }() - + client.FindDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { return &google.Credentials{}, nil } @@ -375,13 +375,13 @@ func TestGCPClient_RunJob(t *testing.T) { CloseFunc: func() error { return nil }, }, nil } - + client := &GCPClient{} exec, err := client.RunJob(context.Background(), "job1") assert.NoError(t, err) assert.Equal(t, "exec-1", exec.Name) }) - + t.Run("Run Error", func(t *testing.T) { createJobsClient = func(ctx context.Context, opts ...option.ClientOption) (JobsClientWrapper, error) { return &MockJobsClientWrapper{ @@ -391,18 +391,18 @@ func TestGCPClient_RunJob(t *testing.T) { CloseFunc: func() error { return nil }, }, nil } - + client := &GCPClient{} _, err := client.RunJob(context.Background(), "job1") assert.Error(t, err) assert.Contains(t, err.Error(), "run failed") }) - + t.Run("Client Creation Error", func(t *testing.T) { createJobsClient = func(ctx context.Context, opts ...option.ClientOption) (JobsClientWrapper, error) { return nil, errors.New("client creation error") } - + client := &GCPClient{} _, err := client.RunJob(context.Background(), "job1") assert.Error(t, err) @@ -412,21 +412,21 @@ func TestGCPClient_RunJob(t *testing.T) { func TestWrappers_Delegation(t *testing.T) { // Expect panics because nil clients are used - + t.Run("GCPJobsClientWrapper", func(t *testing.T) { w := &GCPJobsClientWrapper{client: nil} assert.Panics(t, func() { _ = w.ListJobs(context.Background(), nil) }) assert.Panics(t, func() { _, _ = w.RunJob(context.Background(), nil) }) assert.Panics(t, func() { _ = w.Close() }) }) - + t.Run("GCPJobIteratorWrapper", func(t *testing.T) { it := &GCPJobIteratorWrapper{it: nil} assert.Panics(t, func() { _, _ = it.Next() }) }) - + t.Run("GCPRunJobOperationWrapper", func(t *testing.T) { op := &GCPRunJobOperationWrapper{op: nil} assert.Panics(t, func() { _, _ = op.Wait(context.Background()) }) }) -} \ No newline at end of file +} diff --git a/internal/run/api/log/log_test.go b/internal/run/api/log/log_test.go index ffcee20..499232e 100644 --- a/internal/run/api/log/log_test.go +++ b/internal/run/api/log/log_test.go @@ -284,7 +284,7 @@ func TestGCPClient(t *testing.T) { entry, err := it.Next() assert.NoError(t, err) assert.Equal(t, "log1", entry.Payload) - + assert.NoError(t, client.Close()) }) @@ -293,7 +293,7 @@ func TestGCPClient(t *testing.T) { client.FindDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { return nil, errors.New("auth failed") } - + _, err := NewGCPClient(context.Background(), "project") assert.Error(t, err) assert.Contains(t, err.Error(), "failed to find default credentials") @@ -307,7 +307,7 @@ func TestGCPClient(t *testing.T) { createLogAdminClient = func(ctx context.Context, projectID string, opts ...option.ClientOption) (LogAdminClientWrapper, error) { return nil, errors.New("creation failed") } - + _, err := NewGCPClient(context.Background(), "project") assert.Error(t, err) assert.Contains(t, err.Error(), "creation failed") @@ -355,15 +355,15 @@ func TestGCPClient(t *testing.T) { func TestWrappers_Delegation(t *testing.T) { // Expect panics because nil clients are used - + t.Run("RealLogAdminClient", func(t *testing.T) { w := &RealLogAdminClient{client: nil} assert.Panics(t, func() { _ = w.Entries(context.Background()) }) assert.Panics(t, func() { _ = w.Close() }) }) - + t.Run("GCPEntryIterator", func(t *testing.T) { it := &GCPEntryIterator{it: nil} assert.Panics(t, func() { _, _ = it.Next() }) }) -} \ No newline at end of file +} diff --git a/internal/run/api/project/project.go b/internal/run/api/project/project.go index 1aa42e4..0170a05 100644 --- a/internal/run/api/project/project.go +++ b/internal/run/api/project/project.go @@ -28,4 +28,4 @@ var apiClient Client = &GCPClient{} func List() ([]model.Project, error) { ctx := context.Background() return apiClient.ListProjects(ctx) -} \ No newline at end of file +} diff --git a/internal/run/api/project/project_test.go b/internal/run/api/project/project_test.go index 0ecd07a..7d0414e 100644 --- a/internal/run/api/project/project_test.go +++ b/internal/run/api/project/project_test.go @@ -241,62 +241,48 @@ func TestMapProject(t *testing.T) { result := mapProject(resp) - assert.Equal(t, "my-project", result.Name) + assert.Equal(t, "my-project", result.Name) - assert.Equal(t, 123456, result.Number) + assert.Equal(t, 123456, result.Number) - } - - - - func TestWrappers_Delegation(t *testing.T) { - - // This test exercises the wrapper methods to ensure coverage. - - // Since we can't easily mock the underlying GCP client, we expect panics when calling methods on nil clients. - - // This confirms the wrappers are attempting to delegate. +} - +func TestWrappers_Delegation(t *testing.T) { - t.Run("GCPProjectsClientWrapper", func(t *testing.T) { + // This test exercises the wrapper methods to ensure coverage. - w := &GCPProjectsClientWrapper{client: nil} // Nil client + // Since we can't easily mock the underlying GCP client, we expect panics when calling methods on nil clients. - + // This confirms the wrappers are attempting to delegate. - assert.Panics(t, func() { + t.Run("GCPProjectsClientWrapper", func(t *testing.T) { - w.SearchProjects(context.Background(), nil) + w := &GCPProjectsClientWrapper{client: nil} // Nil client - }) + assert.Panics(t, func() { - + w.SearchProjects(context.Background(), nil) - assert.Panics(t, func() { + }) - _ = w.Close() + assert.Panics(t, func() { - }) + _ = w.Close() }) - - - t.Run("GCPProjectIteratorWrapper", func(t *testing.T) { - - it := &GCPProjectIteratorWrapper{it: nil} // Nil iterator + }) - + t.Run("GCPProjectIteratorWrapper", func(t *testing.T) { - assert.Panics(t, func() { + it := &GCPProjectIteratorWrapper{it: nil} // Nil iterator - _, _ = it.Next() + assert.Panics(t, func() { - }) + _, _ = it.Next() }) - } + }) - \ No newline at end of file +} diff --git a/internal/run/api/service/revision/client.go b/internal/run/api/service/revision/client.go index c1fae70..3f5c2c5 100644 --- a/internal/run/api/service/revision/client.go +++ b/internal/run/api/service/revision/client.go @@ -135,4 +135,4 @@ func (c *GCPClient) ListRevisions(ctx context.Context, project, region, service } return revisions, nil -} \ No newline at end of file +} diff --git a/internal/run/api/service/revision/revision_test.go b/internal/run/api/service/revision/revision_test.go index 2ee9523..5d8939c 100644 --- a/internal/run/api/service/revision/revision_test.go +++ b/internal/run/api/service/revision/revision_test.go @@ -116,20 +116,20 @@ func TestMapRevision(t *testing.T) { assert.Equal(t, "my-service", result.Service) assert.Equal(t, "user@example.com", result.Author) assert.Equal(t, now.Unix(), result.CreateTime.Unix()) - + // Containers assert.Len(t, result.Containers, 1) assert.Equal(t, "c1", result.Containers[0].Name) assert.True(t, result.Containers[0].Resources.CPUIdle) - + // Env assert.Equal(t, "EXECUTION_ENVIRONMENT_GEN2", result.ExecutionEnvironment) assert.Equal(t, int32(80), result.MaxInstanceRequestConcurrency) assert.Equal(t, 30*time.Second, result.Timeout) - + // Accelerator assert.Equal(t, "nvidia-tesla-t4", result.Accelerator) - + // Top level shortcuts assert.True(t, result.CpuIdle) assert.True(t, result.StartupCpuBoost) @@ -292,4 +292,4 @@ func TestWrappers_Delegation(t *testing.T) { it := &GCPRevisionIteratorWrapper{it: nil} assert.Panics(t, func() { _, _ = it.Next() }) }) -} \ No newline at end of file +} diff --git a/internal/run/api/service/service.go b/internal/run/api/service/service.go index eb9e078..4c64a8f 100644 --- a/internal/run/api/service/service.go +++ b/internal/run/api/service/service.go @@ -157,7 +157,7 @@ func mapService(resp *runpb.Service, project, region string) model.Service { // UpdateScaling updates the scaling settings for a service. func UpdateScaling(ctx context.Context, project, region, serviceName string, min, max, manual int32) (*model.Service, error) { fullServiceName := fmt.Sprintf("projects/%s/locations/%s/services/%s", project, region, serviceName) - + service, err := apiClient.GetService(ctx, fullServiceName) if err != nil { return nil, fmt.Errorf("failed to get service: %w", err) @@ -299,26 +299,36 @@ func UpdateTraffic(ctx context.Context, project, region, serviceName string, tar } func listAllRegions(project string) ([]model.Service, error) { - var ( - mu sync.Mutex - services []model.Service - wg sync.WaitGroup - ) - - for _, region := range api_region.List() { + // Optimization: lock-free, pre-allocated map-reduce pattern with a slice of slices [][]model.Service. + // This eliminates mutex contention across concurrent region requests, avoids repeated slice resizing/heap allocations, + // and optimizes concurrency to reduce latency and memory usage. + regions := api_region.List() + results := make([][]model.Service, len(regions)) + var wg sync.WaitGroup + + for i, region := range regions { wg.Add(1) - go func(r string) { + go func(idx int, r string) { defer wg.Done() // Call List recursively for each region // We ignore errors here to allow partial success (e.g. if one region is down or disabled) if s, err := List(project, r); err == nil { - mu.Lock() - services = append(services, s...) - mu.Unlock() + results[idx] = s } - }(region) + }(i, region) } wg.Wait() + + total := 0 + for _, s := range results { + total += len(s) + } + + services := make([]model.Service, 0, total) + for _, s := range results { + services = append(services, s...) + } + return services, nil } diff --git a/internal/run/api/service/service_test.go b/internal/run/api/service/service_test.go index e598a02..8e58026 100644 --- a/internal/run/api/service/service_test.go +++ b/internal/run/api/service/service_test.go @@ -68,7 +68,7 @@ func TestMapService(t *testing.T) { assert.Equal(t, "user@example.com", result.LastModifier) assert.Equal(t, "my-project", result.Project) assert.Equal(t, "us-central1", result.Region) - + // Scaling assert.Equal(t, "AUTOMATIC", result.Scaling.ScalingMode) assert.Equal(t, int32(1), result.Scaling.MinInstances) @@ -77,7 +77,7 @@ func TestMapService(t *testing.T) { // Traffic assert.Len(t, result.TrafficStatuses, 1) assert.Equal(t, "my-service-v1", result.TrafficStatuses[0].Revision) - + // Revisions assert.Equal(t, "my-service-v1", result.LatestReadyRevision) assert.Equal(t, "my-service-v2", result.LatestCreatedRevision) @@ -317,7 +317,7 @@ func TestList_AllRegions(t *testing.T) { services, err := List("p", api_region.ALL) assert.NoError(t, err) - + // We expect at least 1 service found := false for _, s := range services { @@ -402,11 +402,11 @@ func TestGCPClient_ListServices(t *testing.T) { client.FindDefaultCredentials = origFindCreds createServicesClient = origCreateClient }() - + client.FindDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { return &google.Credentials{}, nil } - + t.Run("Success", func(t *testing.T) { createServicesClient = func(ctx context.Context, opts ...option.ClientOption) (ServicesClientWrapper, error) { return &MockServicesClientWrapper{ @@ -418,14 +418,14 @@ func TestGCPClient_ListServices(t *testing.T) { CloseFunc: func() error { return nil }, }, nil } - + client := &GCPClient{} services, err := client.ListServices(context.Background(), "p", "r") assert.NoError(t, err) assert.Len(t, services, 1) assert.Equal(t, "s1", services[0].Name) }) - + t.Run("Auth Error", func(t *testing.T) { client.FindDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { return nil, errors.New("auth failed") @@ -434,7 +434,7 @@ func TestGCPClient_ListServices(t *testing.T) { _, err := client.ListServices(context.Background(), "p", "r") assert.Error(t, err) }) - + t.Run("Iterator Auth Error", func(t *testing.T) { client.FindDefaultCredentials = func(ctx context.Context, scopes ...string) (*google.Credentials, error) { return &google.Credentials{}, nil @@ -477,13 +477,13 @@ func TestGCPClient_GetService(t *testing.T) { CloseFunc: func() error { return nil }, }, nil } - + client := &GCPClient{} s, err := client.GetService(context.Background(), "s1") assert.NoError(t, err) assert.Equal(t, "s1", s.Name) }) - + t.Run("Get Error", func(t *testing.T) { createServicesClient = func(ctx context.Context, opts ...option.ClientOption) (ServicesClientWrapper, error) { return &MockServicesClientWrapper{ @@ -524,13 +524,13 @@ func TestGCPClient_UpdateService(t *testing.T) { CloseFunc: func() error { return nil }, }, nil } - + client := &GCPClient{} s, err := client.UpdateService(context.Background(), &runpb.Service{Name: "s1"}) assert.NoError(t, err) assert.Equal(t, "s1-updated", s.Name) }) - + t.Run("Update Start Error", func(t *testing.T) { createServicesClient = func(ctx context.Context, opts ...option.ClientOption) (ServicesClientWrapper, error) { return &MockServicesClientWrapper{ @@ -567,7 +567,7 @@ func TestGCPClient_UpdateService(t *testing.T) { func TestWrappers_Delegation(t *testing.T) { // Expect panics because nil clients are used - + t.Run("GCPServicesClientWrapper", func(t *testing.T) { w := &GCPServicesClientWrapper{client: nil} assert.Panics(t, func() { _ = w.ListServices(context.Background(), nil) }) @@ -575,71 +575,90 @@ func TestWrappers_Delegation(t *testing.T) { assert.Panics(t, func() { _, _ = w.UpdateService(context.Background(), nil) }) assert.Panics(t, func() { _ = w.Close() }) }) - + t.Run("GCPServiceIteratorWrapper", func(t *testing.T) { it := &GCPServiceIteratorWrapper{it: nil} assert.Panics(t, func() { _, _ = it.Next() }) }) - + t.Run("GCPUpdateServiceOperationWrapper", func(t *testing.T) { op := &GCPUpdateServiceOperationWrapper{op: nil} assert.Panics(t, func() { _, _ = op.Wait(context.Background()) }) }) } func TestUpdateAuthentication(t *testing.T) { - originalClient := apiClient - defer func() { apiClient = originalClient }() - - mock := &MockClient{} - apiClient = mock - - // Setup GetService mock - mock.GetServiceFunc = func(ctx context.Context, name string) (*runpb.Service, error) { - return &runpb.Service{ - Name: name, - InvokerIamDisabled: false, - }, nil - } - - // Setup UpdateService mock - mock.UpdateServiceFunc = func(ctx context.Context, service *runpb.Service) (*runpb.Service, error) { - // Assert that auth was updated correctly - assert.True(t, service.InvokerIamDisabled) - return service, nil - } - - result, err := UpdateAuthentication(context.Background(), "p", "r", "s1", true) - - assert.NoError(t, err) - assert.NotNil(t, result) - assert.Equal(t, "s1", result.Name) - assert.True(t, result.Security.InvokerIAMDisabled) - } - + originalClient := apiClient + defer func() { apiClient = originalClient }() + + mock := &MockClient{} + apiClient = mock + + // Setup GetService mock + mock.GetServiceFunc = func(ctx context.Context, name string) (*runpb.Service, error) { + return &runpb.Service{ + Name: name, + InvokerIamDisabled: false, + }, nil + } + + // Setup UpdateService mock + mock.UpdateServiceFunc = func(ctx context.Context, service *runpb.Service) (*runpb.Service, error) { + // Assert that auth was updated correctly + assert.True(t, service.InvokerIamDisabled) + return service, nil + } + + result, err := UpdateAuthentication(context.Background(), "p", "r", "s1", true) + + assert.NoError(t, err) + assert.NotNil(t, result) + assert.Equal(t, "s1", result.Name) + assert.True(t, result.Security.InvokerIAMDisabled) +} + func TestUpdateAuthentication_Error(t *testing.T) { - originalClient := apiClient - defer func() { apiClient = originalClient }() - - mock := &MockClient{} - apiClient = mock - - // Test GetService Error - mock.GetServiceFunc = func(ctx context.Context, name string) (*runpb.Service, error) { - return nil, assert.AnError - } - _, err := UpdateAuthentication(context.Background(), "p", "r", "s", true) - assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to get service") - - // Test UpdateService Error - mock.GetServiceFunc = func(ctx context.Context, name string) (*runpb.Service, error) { - return &runpb.Service{Name: name}, nil - } - mock.UpdateServiceFunc = func(ctx context.Context, service *runpb.Service) (*runpb.Service, error) { - return nil, assert.AnError - } - _, err = UpdateAuthentication(context.Background(), "p", "r", "s", true) - assert.Error(t, err) - assert.Contains(t, err.Error(), "failed to update service") - } - \ No newline at end of file + originalClient := apiClient + defer func() { apiClient = originalClient }() + + mock := &MockClient{} + apiClient = mock + + // Test GetService Error + mock.GetServiceFunc = func(ctx context.Context, name string) (*runpb.Service, error) { + return nil, assert.AnError + } + _, err := UpdateAuthentication(context.Background(), "p", "r", "s", true) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to get service") + + // Test UpdateService Error + mock.GetServiceFunc = func(ctx context.Context, name string) (*runpb.Service, error) { + return &runpb.Service{Name: name}, nil + } + mock.UpdateServiceFunc = func(ctx context.Context, service *runpb.Service) (*runpb.Service, error) { + return nil, assert.AnError + } + _, err = UpdateAuthentication(context.Background(), "p", "r", "s", true) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to update service") +} + +func BenchmarkListAllRegions(b *testing.B) { + originalClient := apiClient + defer func() { apiClient = originalClient }() + + mock := &MockClient{} + apiClient = mock + + mock.ListServicesFunc = func(ctx context.Context, project, region string) ([]*runpb.Service, error) { + return []*runpb.Service{ + {Name: "projects/" + project + "/locations/" + region + "/services/s1"}, + {Name: "projects/" + project + "/locations/" + region + "/services/s2"}, + }, nil + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + _, _ = List("my-project", api_region.ALL) + } +} diff --git a/internal/run/api/workerpool/workerpool.go b/internal/run/api/workerpool/workerpool.go index 041f8f4..32fa063 100644 --- a/internal/run/api/workerpool/workerpool.go +++ b/internal/run/api/workerpool/workerpool.go @@ -111,27 +111,36 @@ func UpdateScaling(ctx context.Context, project, region, workerPoolName string, } func listAllRegions(project string) ([]model.WorkerPool, error) { - var ( - mu sync.Mutex - workerPools []model.WorkerPool - wg sync.WaitGroup - ) - - for _, region := range api_region.List() { + // Optimization: lock-free, pre-allocated map-reduce pattern with a slice of slices [][]model.WorkerPool. + // This eliminates mutex contention across concurrent region requests, avoids repeated slice resizing/heap allocations, + // and optimizes concurrency to reduce latency and memory usage. + regions := api_region.List() + results := make([][]model.WorkerPool, len(regions)) + var wg sync.WaitGroup + + for i, region := range regions { wg.Add(1) - go func(r string) { + go func(idx int, r string) { defer wg.Done() // Call List recursively for each region // We ignore errors here to allow partial success (e.g. if one region is down or disabled) if wp, err := List(project, r); err == nil { - mu.Lock() - workerPools = append(workerPools, wp...) - mu.Unlock() + results[idx] = wp } - }(region) + }(i, region) } wg.Wait() + + total := 0 + for _, wp := range results { + total += len(wp) + } + + workerPools := make([]model.WorkerPool, 0, total) + for _, wp := range results { + workerPools = append(workerPools, wp...) + } + return workerPools, nil } - diff --git a/internal/run/api/workerpool/workerpool_test.go b/internal/run/api/workerpool/workerpool_test.go index 63cbf13..33fcd97 100644 --- a/internal/run/api/workerpool/workerpool_test.go +++ b/internal/run/api/workerpool/workerpool_test.go @@ -84,11 +84,11 @@ func TestMapWorkerPool(t *testing.T) { assert.Equal(t, now.Unix(), result.UpdateTime.Unix()) assert.Equal(t, "my-project", result.Project) assert.Equal(t, "us-central1", result.Region) - + // Scaling assert.NotNil(t, result.Scaling) assert.Equal(t, int32(2), result.Scaling.ManualInstanceCount) - + // Labels assert.Equal(t, "prod", result.Labels["env"]) } @@ -362,13 +362,13 @@ func TestGCPClient_GetWorkerPool(t *testing.T) { CloseFunc: func() error { return nil }, }, nil } - + client := &GCPClient{} pool, err := client.GetWorkerPool(context.Background(), "pool1") assert.NoError(t, err) assert.Equal(t, "pool1", pool.Name) }) - + t.Run("Get Error", func(t *testing.T) { createWorkerPoolsClient = func(ctx context.Context, opts ...option.ClientOption) (WorkerPoolsClientWrapper, error) { return &MockWorkerPoolsClientWrapper{ @@ -378,7 +378,7 @@ func TestGCPClient_GetWorkerPool(t *testing.T) { CloseFunc: func() error { return nil }, }, nil } - + client := &GCPClient{} _, err := client.GetWorkerPool(context.Background(), "pool1") assert.Error(t, err) @@ -388,7 +388,7 @@ func TestGCPClient_GetWorkerPool(t *testing.T) { createWorkerPoolsClient = func(ctx context.Context, opts ...option.ClientOption) (WorkerPoolsClientWrapper, error) { return nil, errors.New("client creation error") } - + client := &GCPClient{} _, err := client.GetWorkerPool(context.Background(), "pool1") assert.Error(t, err) @@ -421,7 +421,7 @@ func TestGCPClient_UpdateWorkerPool(t *testing.T) { CloseFunc: func() error { return nil }, }, nil } - + client := &GCPClient{} pool, err := client.UpdateWorkerPool(context.Background(), &runpb.WorkerPool{Name: "pool1"}) assert.NoError(t, err) @@ -437,7 +437,7 @@ func TestGCPClient_UpdateWorkerPool(t *testing.T) { CloseFunc: func() error { return nil }, }, nil } - + client := &GCPClient{} _, err := client.UpdateWorkerPool(context.Background(), &runpb.WorkerPool{Name: "pool1"}) assert.Error(t, err) @@ -447,7 +447,7 @@ func TestGCPClient_UpdateWorkerPool(t *testing.T) { createWorkerPoolsClient = func(ctx context.Context, opts ...option.ClientOption) (WorkerPoolsClientWrapper, error) { return nil, errors.New("client creation error") } - + client := &GCPClient{} _, err := client.UpdateWorkerPool(context.Background(), &runpb.WorkerPool{Name: "pool1"}) assert.Error(t, err) @@ -467,7 +467,7 @@ func TestGCPClient_UpdateWorkerPool(t *testing.T) { CloseFunc: func() error { return nil }, }, nil } - + client := &GCPClient{} _, err := client.UpdateWorkerPool(context.Background(), &runpb.WorkerPool{Name: "pool1"}) assert.Error(t, err) @@ -477,7 +477,7 @@ func TestGCPClient_UpdateWorkerPool(t *testing.T) { func TestWrappers_Delegation(t *testing.T) { // Expect panics because nil clients are used - + t.Run("GCPWorkerPoolsClientWrapper", func(t *testing.T) { w := &GCPWorkerPoolsClientWrapper{client: nil} assert.Panics(t, func() { _ = w.ListWorkerPools(context.Background(), nil) }) @@ -485,14 +485,14 @@ func TestWrappers_Delegation(t *testing.T) { assert.Panics(t, func() { _, _ = w.UpdateWorkerPool(context.Background(), nil) }) assert.Panics(t, func() { _ = w.Close() }) }) - + t.Run("GCPWorkerPoolIteratorWrapper", func(t *testing.T) { it := &GCPWorkerPoolIteratorWrapper{it: nil} assert.Panics(t, func() { _, _ = it.Next() }) }) - + t.Run("GCPUpdateWorkerPoolOperationWrapper", func(t *testing.T) { op := &GCPUpdateWorkerPoolOperationWrapper{op: nil} assert.Panics(t, func() { _, _ = op.Wait(context.Background()) }) }) -} \ No newline at end of file +} diff --git a/internal/run/command/command_test.go b/internal/run/command/command_test.go index a7ea5be..83c19fd 100644 --- a/internal/run/command/command_test.go +++ b/internal/run/command/command_test.go @@ -34,10 +34,10 @@ func TestNew(t *testing.T) { assert.NotNil(t, cmd) assert.Equal(t, "run", cmd.Use) assert.Equal(t, "Run is a CLI to play with Google Cloud Run interactively.", cmd.Short) - + // Check subcommands assert.True(t, cmd.HasSubCommands()) - + // Check if version command exists found := false for _, c := range cmd.Commands() { diff --git a/internal/run/command/version/version_test.go b/internal/run/command/version/version_test.go index 4a1fe41..e8cb9de 100644 --- a/internal/run/command/version/version_test.go +++ b/internal/run/command/version/version_test.go @@ -45,11 +45,11 @@ func TestNewCmdVersion(t *testing.T) { // Test execution cmd.SetOut(out) cmd.SetErr(err) - + // Execute the command execErr := cmd.Execute() assert.NoError(t, execErr) - + // Verify output contains version info // Note: exact content depends on pkg/version globals, but we expect at least "Version:" assert.Contains(t, out.String(), "Version:") diff --git a/internal/run/model/common/resources/resources.go b/internal/run/model/common/resources/resources.go index 72cc12e..b57eaab 100644 --- a/internal/run/model/common/resources/resources.go +++ b/internal/run/model/common/resources/resources.go @@ -21,4 +21,4 @@ type Resources struct { Limits map[string]string `json:"limits,omitempty"` CPUIdle bool `json:"cpuIdle,omitempty"` StartupCPUBoost bool `json:"startupCpuBoost,omitempty"` -} \ No newline at end of file +} diff --git a/internal/run/model/domainmapping/domainmapping.go b/internal/run/model/domainmapping/domainmapping.go index 318c88f..1877b97 100644 --- a/internal/run/model/domainmapping/domainmapping.go +++ b/internal/run/model/domainmapping/domainmapping.go @@ -24,14 +24,14 @@ import ( // DomainMapping represents a Cloud Run domain mapping. type DomainMapping struct { - Name string `json:"name"` - RouteName string `json:"routeName"` - Region string `json:"region"` - Project string `json:"project"` - Creator string `json:"creator"` - Records []ResourceRecord `json:"records"` - CreateTime time.Time `json:"createTime"` - UpdateTime time.Time `json:"updateTime"` + Name string `json:"name"` + RouteName string `json:"routeName"` + Region string `json:"region"` + Project string `json:"project"` + Creator string `json:"creator"` + Records []ResourceRecord `json:"records"` + CreateTime time.Time `json:"createTime"` + UpdateTime time.Time `json:"updateTime"` Conditions []*condition.Condition `json:"conditions,omitempty"` } diff --git a/internal/run/model/job/execution/execution.go b/internal/run/model/job/execution/execution.go index 4cee94e..2a3c508 100644 --- a/internal/run/model/job/execution/execution.go +++ b/internal/run/model/job/execution/execution.go @@ -24,21 +24,21 @@ import ( // Execution represents a Cloud Run job execution. type Execution struct { - Name string `json:"name"` - Job string `json:"job"` - CreateTime time.Time `json:"createTime"` - StartTime time.Time `json:"startTime"` - CompletionTime time.Time `json:"completionTime"` - DeleteTime time.Time `json:"deleteTime"` - ExpireTime time.Time `json:"expireTime"` - TaskCount int32 `json:"taskCount"` - SucceededCount int32 `json:"succeededCount"` - FailedCount int32 `json:"failedCount"` - RunningCount int32 `json:"runningCount"` - CancelledCount int32 `json:"cancelledCount"` - RetriedCount int32 `json:"retriedCount"` - LogURI string `json:"logUri"` - Region string `json:"region"` + Name string `json:"name"` + Job string `json:"job"` + CreateTime time.Time `json:"createTime"` + StartTime time.Time `json:"startTime"` + CompletionTime time.Time `json:"completionTime"` + DeleteTime time.Time `json:"deleteTime"` + ExpireTime time.Time `json:"expireTime"` + TaskCount int32 `json:"taskCount"` + SucceededCount int32 `json:"succeededCount"` + FailedCount int32 `json:"failedCount"` + RunningCount int32 `json:"runningCount"` + CancelledCount int32 `json:"cancelledCount"` + RetriedCount int32 `json:"retriedCount"` + LogURI string `json:"logUri"` + Region string `json:"region"` Conditions []*condition.Condition `json:"conditions"` - TerminalCondition *condition.Condition `json:"terminalCondition"` + TerminalCondition *condition.Condition `json:"terminalCondition"` } diff --git a/internal/run/model/job/job.go b/internal/run/model/job/job.go index c2424b4..8ed6ad2 100644 --- a/internal/run/model/job/job.go +++ b/internal/run/model/job/job.go @@ -26,30 +26,30 @@ import ( // Job represents a Cloud Run job. type Job struct { - Name string `json:"name"` - UID string `json:"uid"` - Generation int64 `json:"generation"` - Labels map[string]string `json:"labels"` - Annotations map[string]string `json:"annotations"` - CreateTime time.Time `json:"createTime"` - UpdateTime time.Time `json:"updateTime"` - DeleteTime time.Time `json:"deleteTime"` - ExpireTime time.Time `json:"expireTime"` - Creator string `json:"creator"` - LastModifier string `json:"lastModifier"` - Client string `json:"client"` - ClientVersion string `json:"clientVersion"` - LaunchStage string `json:"launchStage"` - BinaryAuthorization *BinaryAuthorization `json:"binaryAuthorization"` - Template *ExecutionTemplate `json:"template"` - ObservedGeneration int64 `json:"observedGeneration"` - TerminalCondition *condition.Condition `json:"terminalCondition"` + Name string `json:"name"` + UID string `json:"uid"` + Generation int64 `json:"generation"` + Labels map[string]string `json:"labels"` + Annotations map[string]string `json:"annotations"` + CreateTime time.Time `json:"createTime"` + UpdateTime time.Time `json:"updateTime"` + DeleteTime time.Time `json:"deleteTime"` + ExpireTime time.Time `json:"expireTime"` + Creator string `json:"creator"` + LastModifier string `json:"lastModifier"` + Client string `json:"client"` + ClientVersion string `json:"clientVersion"` + LaunchStage string `json:"launchStage"` + BinaryAuthorization *BinaryAuthorization `json:"binaryAuthorization"` + Template *ExecutionTemplate `json:"template"` + ObservedGeneration int64 `json:"observedGeneration"` + TerminalCondition *condition.Condition `json:"terminalCondition"` Conditions []*condition.Condition `json:"conditions"` - ExecutionCount int64 `json:"executionCount"` - LatestCreatedExecution *ExecutionReference `json:"latestCreatedExecution"` - Reconciling bool `json:"reconciling"` - SatisfiesPZS bool `json:"satisfiesPzs"` - Region string `json:"region"` // New field + ExecutionCount int64 `json:"executionCount"` + LatestCreatedExecution *ExecutionReference `json:"latestCreatedExecution"` + Reconciling bool `json:"reconciling"` + SatisfiesPZS bool `json:"satisfiesPzs"` + Region string `json:"region"` // New field } // ExecutionReference represents a reference to a specific execution. @@ -99,4 +99,4 @@ type NetworkInterface struct { Network string `json:"network"` Subnetwork string `json:"subnetwork"` Tags []string `json:"tags"` -} \ No newline at end of file +} diff --git a/internal/run/model/service/service_test.go b/internal/run/model/service/service_test.go index 980a697..a7c9cc7 100644 --- a/internal/run/model/service/service_test.go +++ b/internal/run/model/service/service_test.go @@ -27,8 +27,8 @@ import ( func TestService(t *testing.T) { now := time.Now() s := Service{ - Name: "service", - URI: "https://service.run.app", + Name: "service", + URI: "https://service.run.app", CreateTime: now, UpdateTime: now, Region: "us-central1", diff --git a/internal/run/tui/app/app_test.go b/internal/run/tui/app/app_test.go index 7a77184..53d03ab 100644 --- a/internal/run/tui/app/app_test.go +++ b/internal/run/tui/app/app_test.go @@ -115,10 +115,10 @@ func TestShortcuts_Navigation(t *testing.T) { // Mock switchTo behavior by checking currentPageID change // Note: switchTo also calls UI updates which might panic if not carefully mocked or ignored. // Since switchTo calls service.Shortcuts() etc, those must rely on globals initialized. - + // We need to ensure buildLayout is called or pages are init buildLayout() - + event := tcell.NewEventKey(tt.key, 0, tcell.ModNone) result := shortcuts(event) @@ -135,7 +135,7 @@ func TestShortcuts_Escape(t *testing.T) { // Simulate being on Dashboard currentPageID = service.DASHBOARD_PAGE_ID - + event := tcell.NewEventKey(tcell.KeyEscape, 0, tcell.ModNone) result := shortcuts(event) @@ -155,35 +155,35 @@ func TestShortcuts_OpenConsole(t *testing.T) { // Service List currentPageID = service.LIST_PAGE_ID - + // Populate Service Table svcTable := service.List(app).Table svcTable.SetCell(1, 0, tview.NewTableCell("s1")) svcTable.SetCell(1, 1, tview.NewTableCell("r1")) svcTable.Select(1, 0) - + eventService := tcell.NewEventKey(tcell.KeyCtrlZ, 0, tcell.ModNone) resultService := shortcuts(eventService) assert.Nil(t, resultService) - + // Job List currentPageID = job.LIST_PAGE_ID jobTable := job.List(app).Table jobTable.SetCell(1, 0, tview.NewTableCell("j1")) jobTable.SetCell(1, 3, tview.NewTableCell("r1")) // Region is col 3 jobTable.Select(1, 0) - + eventJob := tcell.NewEventKey(tcell.KeyCtrlZ, 0, tcell.ModNone) resultJob := shortcuts(eventJob) assert.Nil(t, resultJob) - + // WorkerPool List currentPageID = workerpool.LIST_PAGE_ID wpTable := workerpool.List(app).Table wpTable.SetCell(1, 0, tview.NewTableCell("wp1")) wpTable.SetCell(1, 1, tview.NewTableCell("r1")) wpTable.Select(1, 0) - + eventWP := tcell.NewEventKey(tcell.KeyCtrlZ, 0, tcell.ModNone) resultWP := shortcuts(eventWP) assert.Nil(t, resultWP) @@ -191,7 +191,7 @@ func TestShortcuts_OpenConsole(t *testing.T) { func TestInitializeApp(t *testing.T) { setupTestApp() - + go func() { _ = app.Run() }() @@ -219,21 +219,21 @@ func TestInitializeApp(t *testing.T) { // Re-initialize mainLoader to be safe against race/overwrite in other tests mainLoader = loader.New(app) rootPages.AddPage(LOADER_PAGE_ID, mainLoader, true, true) - + if mainLoader == nil { t.Fatal("mainLoader is nil") } if mainLoader.Spinner == nil { t.Fatal("mainLoader.Spinner is nil") } - + initializeApp(currentConfig) - + // Allow async tasks to finish (PreLoad, Fetch, QueueUpdateDraw) // initializeApp waits for WG, then queues update. // QueueUpdateDraw executes in the main loop (goroutine above). // We need to wait a bit. - + // Check if Layout Page was added // We can't query pages directly, but we can try to switch to it. assert.NotPanics(t, func() { @@ -244,7 +244,7 @@ func TestInitializeApp(t *testing.T) { func TestSwitchTo(t *testing.T) { setupTestApp() buildLayout() // Inits footerPages, footerSpinner - + go func() { _ = app.Run() }() defer func() { app.Stop() @@ -254,21 +254,21 @@ func TestSwitchTo(t *testing.T) { // Test Service List switchTo(service.LIST_PAGE_ID) assert.Equal(t, service.LIST_PAGE_ID, currentPageID) - + // Test Dashboard // Needs selection? // switchTo Dashboard checks GetSelectedServiceFull. If nil, it might skip reload? // No, it checks `if s := service.GetSelectedServiceFull(); s != nil`. // If nil, it just switches page? No, the block is inside if. // `pages.SwitchToPage(pageID)` is called unconditionally at start. - + switchTo(service.DASHBOARD_PAGE_ID) assert.Equal(t, service.DASHBOARD_PAGE_ID, currentPageID) - + // Test Job List switchTo(job.LIST_PAGE_ID) assert.Equal(t, job.LIST_PAGE_ID, currentPageID) - + // Test WorkerPool List switchTo(workerpool.LIST_PAGE_ID) assert.Equal(t, workerpool.LIST_PAGE_ID, currentPageID) @@ -277,37 +277,37 @@ func TestSwitchTo(t *testing.T) { func TestShortcuts_Detailed(t *testing.T) { setupTestApp() buildLayout() - + // --- Service List --- currentPageID = service.LIST_PAGE_ID - + // Populate Service Table svcTable := service.List(app).Table svcTable.SetCell(1, 0, tview.NewTableCell("s1")) svcTable.SetCell(1, 1, tview.NewTableCell("r1")) svcTable.Select(1, 0) - + // Enter -> Dashboard shortcuts(tcell.NewEventKey(tcell.KeyEnter, 0, tcell.ModNone)) assert.Equal(t, service.DASHBOARD_PAGE_ID, currentPageID) - + currentPageID = service.LIST_PAGE_ID // Reset // 'r' -> Reload (stays on list) shortcuts(tcell.NewEventKey(tcell.KeyRune, 'r', tcell.ModNone)) assert.Equal(t, service.LIST_PAGE_ID, currentPageID) - + // 'l', 'd', 's' open modals. // Now with selection, they should proceed. - + // 'l' -> Log Modal shortcuts(tcell.NewEventKey(tcell.KeyRune, 'l', tcell.ModNone)) // Verify page changed? // openLogModal changes currentPageID to log.MODAL_PAGE_ID? No, it adds page. // But it sets focus. - + // 'd' -> Describe shortcuts(tcell.NewEventKey(tcell.KeyRune, 'd', tcell.ModNone)) - + // 's' -> Scale // This requires GetSelectedServiceFull returning a struct. // We only populated the table visually. @@ -315,45 +315,45 @@ func TestShortcuts_Detailed(t *testing.T) { // We need to populate that slice too? // service.Load([]model_service.Service{{Name: "s1"}}) // We can't access 'service.Load' from here easily? Yes we can, it is exported. - + // Populate Service Data service.Load([]model_service.Service{{Name: "s1", Region: "r1"}}) svcTable.Select(1, 0) // Re-select because Load clears table - + shortcuts(tcell.NewEventKey(tcell.KeyRune, 's', tcell.ModNone)) - + // --- Job List --- currentPageID = job.LIST_PAGE_ID - + // Populate Job Table jobTable := job.List(app).Table jobTable.SetCell(1, 0, tview.NewTableCell("j1")) jobTable.SetCell(1, 3, tview.NewTableCell("r1")) jobTable.Select(1, 0) - + // 'x' -> Execute (async) shortcuts(tcell.NewEventKey(tcell.KeyRune, 'x', tcell.ModNone)) - + // --- WorkerPool List --- currentPageID = workerpool.LIST_PAGE_ID - + // Populate WP Table wpTable := workerpool.List(app).Table wpTable.SetCell(1, 0, tview.NewTableCell("wp1")) wpTable.SetCell(1, 1, tview.NewTableCell("r1")) wpTable.Select(1, 0) - + shortcuts(tcell.NewEventKey(tcell.KeyRune, 'r', tcell.ModNone)) } func TestShortcuts_Modals(t *testing.T) { setupTestApp() buildLayout() - + // --- Service Modals --- currentPageID = service.LIST_PAGE_ID svcTable := service.List(app).Table - + // Log service.Load([]model_service.Service{{Name: "s1", Region: "r1"}}) svcTable.Select(1, 0) @@ -362,7 +362,7 @@ func TestShortcuts_Modals(t *testing.T) { // Close modal to reset rootPages.RemovePage(log.MODAL_PAGE_ID) currentPageID = service.LIST_PAGE_ID - + // Describe service.Load([]model_service.Service{{Name: "s1", Region: "r1"}}) svcTable.Select(1, 0) @@ -370,7 +370,7 @@ func TestShortcuts_Modals(t *testing.T) { assert.Equal(t, describe.MODAL_PAGE_ID, currentPageID) rootPages.RemovePage(describe.MODAL_PAGE_ID) currentPageID = service.LIST_PAGE_ID - + // Scale // Ensure selection is preserved/re-applied // Re-load data to ensure state consistency @@ -382,7 +382,7 @@ func TestShortcuts_Modals(t *testing.T) { assert.Equal(t, service_scale.MODAL_PAGE_ID, currentPageID) rootPages.RemovePage(service_scale.MODAL_PAGE_ID) currentPageID = service.LIST_PAGE_ID - + // --- Job Modals --- currentPageID = job.LIST_PAGE_ID job.List(app) // ensure table init @@ -391,18 +391,18 @@ func TestShortcuts_Modals(t *testing.T) { // Wait, 'jobs' in job package is unexported. 'GetSelectedJobFull' reads it. // We can't use 'GetSelectedJobFull' if we can't populate 'jobs'. // But 'GetSelectedJob' (used for Logs) reads from Table. - + // So we can test Logs for Job. jobTable := job.List(app).Table jobTable.SetCell(1, 0, tview.NewTableCell("j1")) jobTable.SetCell(1, 3, tview.NewTableCell("r1")) jobTable.Select(1, 0) - + shortcuts(tcell.NewEventKey(tcell.KeyRune, 'l', tcell.ModNone)) assert.Equal(t, log.MODAL_PAGE_ID, currentPageID) rootPages.RemovePage(log.MODAL_PAGE_ID) currentPageID = job.LIST_PAGE_ID - + // Describe for Job job.Load([]model_job.Job{{Name: "j1", Region: "r1"}}) jobTable.Select(1, 0) @@ -410,19 +410,19 @@ func TestShortcuts_Modals(t *testing.T) { assert.Equal(t, describe.MODAL_PAGE_ID, currentPageID) rootPages.RemovePage(describe.MODAL_PAGE_ID) currentPageID = job.LIST_PAGE_ID - + // --- WorkerPool Modals --- currentPageID = workerpool.LIST_PAGE_ID wpTable := workerpool.List(app).Table workerpool.Load([]model_workerpool.WorkerPool{{DisplayName: "wp1", Region: "r1"}}) wpTable.Select(1, 0) - + // Describe for WorkerPool shortcuts(tcell.NewEventKey(tcell.KeyRune, 'd', tcell.ModNone)) assert.Equal(t, describe.MODAL_PAGE_ID, currentPageID) rootPages.RemovePage(describe.MODAL_PAGE_ID) currentPageID = workerpool.LIST_PAGE_ID - + // Scale for WorkerPool // Ensure selection is preserved/re-applied workerpool.Load([]model_workerpool.WorkerPool{{DisplayName: "wp1", Region: "r1"}}) @@ -437,20 +437,20 @@ func TestShortcuts_Modals(t *testing.T) { func TestRun(t *testing.T) { setupTestApp() - + // We need to inject this screen into the app created by Run // But Run creates a NEW app using tview.NewApplication(). // We can't inject screen into Run() directly. - + // Refactor Run to accept screen? Or make 'app' variable accessible before Run? // Run calls 'app = tview.NewApplication()'. - + // Option: Refactor Run to separate creation and execution? // Or just test initializeApp logic if possible? - + // For now, let's skip TestRun if it's hard without refactoring. // Let's try to test initializeApp logic directly? // initializeApp is unexported. - + // Let's rely on what we have. 41% is low. } diff --git a/internal/run/tui/app/describe/describe_test.go b/internal/run/tui/app/describe/describe_test.go index 7875348..b0ec3f1 100644 --- a/internal/run/tui/app/describe/describe_test.go +++ b/internal/run/tui/app/describe/describe_test.go @@ -34,7 +34,7 @@ func TestDescribeModal(t *testing.T) { assert.NotNil(t, describer) assert.NotNil(t, describer.TextView) assert.NotNil(t, describer.Content) - + // Check Primitive interface compliance var _ tview.Primitive = describer } @@ -42,9 +42,9 @@ func TestDescribeModal(t *testing.T) { func TestDescriber_Content(t *testing.T) { app := tview.NewApplication() resource := map[string]string{"foo": "bar"} - - describer := DescribeModal(app, resource, "Test", func(){}) - + + describer := DescribeModal(app, resource, "Test", func() {}) + text := describer.TextView.GetText(true) // YAML output should contain "foo: bar" assert.Contains(t, text, "foo") @@ -55,27 +55,27 @@ func TestDescriber_InputCapture(t *testing.T) { app := tview.NewApplication() closed := false closeFunc := func() { closed = true } - + describer := DescribeModal(app, "data", "title", closeFunc) - + handler := describer.Content.GetInputCapture() assert.NotNil(t, handler) - + // Test Escape eventEsc := tcell.NewEventKey(tcell.KeyEscape, 0, tcell.ModNone) ret := handler(eventEsc) assert.Nil(t, ret) assert.True(t, closed) - + // Reset closed = false - + // Test 'q' eventQ := tcell.NewEventKey(tcell.KeyRune, 'q', tcell.ModNone) ret = handler(eventQ) assert.Nil(t, ret) assert.True(t, closed) - + // Test other key eventOther := tcell.NewEventKey(tcell.KeyRune, 'a', tcell.ModNone) ret = handler(eventOther) @@ -88,12 +88,12 @@ func TestDescriber_InputCapture_NonClosing(t *testing.T) { app := tview.NewApplication() closed := false closeFunc := func() { closed = true } - + describer := DescribeModal(app, "data", "title", closeFunc) handler := describer.Content.GetInputCapture() - + eventOther := tcell.NewEventKey(tcell.KeyRune, 'a', tcell.ModNone) ret := handler(eventOther) assert.Equal(t, eventOther, ret) assert.False(t, closed) -} \ No newline at end of file +} diff --git a/internal/run/tui/app/help/help_test.go b/internal/run/tui/app/help/help_test.go index 8a8473a..6956123 100644 --- a/internal/run/tui/app/help/help_test.go +++ b/internal/run/tui/app/help/help_test.go @@ -53,4 +53,4 @@ func TestHelpModal(t *testing.T) { closeFuncCalled = false handler(tcell.NewEventKey(tcell.KeyRune, '?', tcell.ModNone)) assert.True(t, closeFuncCalled) -} \ No newline at end of file +} diff --git a/internal/run/tui/app/job/dashboard_test.go b/internal/run/tui/app/job/dashboard_test.go index 5457950..75d1a59 100644 --- a/internal/run/tui/app/job/dashboard_test.go +++ b/internal/run/tui/app/job/dashboard_test.go @@ -115,10 +115,10 @@ func TestDashboardReload(t *testing.T) { func TestDashboardShortcuts(t *testing.T) { _ = footer.New() - + assert.NotPanics(t, func() { DashboardShortcuts() }) assert.Contains(t, footer.ContextShortcutView.GetText(true), "Back") -} \ No newline at end of file +} diff --git a/internal/run/tui/app/log/log_test.go b/internal/run/tui/app/log/log_test.go index 510f7d8..5ea5bb3 100644 --- a/internal/run/tui/app/log/log_test.go +++ b/internal/run/tui/app/log/log_test.go @@ -37,7 +37,7 @@ func TestLogModal(t *testing.T) { assert.NotNil(t, viewer.TextView) assert.NotNil(t, viewer.StatusText) assert.NotNil(t, viewer.Content) - + // Should satisfy Primitive interface var _ tview.Primitive = viewer } @@ -46,34 +46,34 @@ func TestLogModal_Streaming(t *testing.T) { // Mock StreamLogs origStream := streamLogsFunc defer func() { streamLogsFunc = origStream }() - + streamLogsFunc = func(ctx context.Context, projectID, filter string, logChan chan<- string) error { logChan <- "Log Line 1" logChan <- "Log Line 2" - // Keep channel open briefly then return? - // Or wait for ctx done. + // Keep channel open briefly then return? + // Or wait for ctx done. // Real implementation blocks until done or error. <-ctx.Done() return nil } - + app := tview.NewApplication() screen := tcell.NewSimulationScreen("UTF-8") _ = screen.Init() app.SetScreen(screen) - + go func() { _ = app.Run() }() defer app.Stop() - - viewer := LogModal(app, "p", "f", "title", func(){}) - + + viewer := LogModal(app, "p", "f", "title", func() {}) + // Wait for async updates time.Sleep(100 * time.Millisecond) - + text := viewer.TextView.GetText(true) assert.Contains(t, text, "Log Line 1") assert.Contains(t, text, "Log Line 2") - + status := viewer.StatusText.GetText(true) assert.Contains(t, status, "Streaming logs") } @@ -82,27 +82,27 @@ func TestLogModal_Error(t *testing.T) { // Mock StreamLogs Error origStream := streamLogsFunc defer func() { streamLogsFunc = origStream }() - + streamLogsFunc = func(ctx context.Context, projectID, filter string, logChan chan<- string) error { return errors.New("stream failed") } - + app := tview.NewApplication() screen := tcell.NewSimulationScreen("UTF-8") _ = screen.Init() app.SetScreen(screen) - + go func() { _ = app.Run() }() defer app.Stop() - - viewer := LogModal(app, "p", "f", "title", func(){}) - + + viewer := LogModal(app, "p", "f", "title", func() {}) + time.Sleep(100 * time.Millisecond) - + text := viewer.TextView.GetText(true) assert.Contains(t, text, "Error streaming logs") assert.Contains(t, text, "stream failed") - + status := viewer.StatusText.GetText(true) assert.Equal(t, "Error", status) } @@ -111,23 +111,23 @@ func TestLogModal_InputCapture(t *testing.T) { // Mock StreamLogs to hang until cancelled origStream := streamLogsFunc defer func() { streamLogsFunc = origStream }() - + streamLogsFunc = func(ctx context.Context, projectID, filter string, logChan chan<- string) error { <-ctx.Done() return nil } - + app := tview.NewApplication() closed := false closeModal := func() { closed = true } - + viewer := LogModal(app, "p", "f", "t", closeModal) handler := viewer.Content.GetInputCapture() - + // Test Escape eventEsc := tcell.NewEventKey(tcell.KeyEscape, 0, tcell.ModNone) ret := handler(eventEsc) - + assert.Nil(t, ret) assert.True(t, closed) -} \ No newline at end of file +} diff --git a/internal/run/tui/app/modal.go b/internal/run/tui/app/modal.go index 6c258f2..7e65c74 100644 --- a/internal/run/tui/app/modal.go +++ b/internal/run/tui/app/modal.go @@ -197,7 +197,7 @@ func openServiceTrafficSplitModal(s *model_service.Service, revs []model_revisio currentPageID = previousPageID pages.SwitchToPage(currentPageID) app.SetFocus(pages) - + switch previousPageID { case service.LIST_PAGE_ID: service.Shortcuts() diff --git a/internal/run/tui/app/modal_test.go b/internal/run/tui/app/modal_test.go index 0a5fd71..a06c3f5 100644 --- a/internal/run/tui/app/modal_test.go +++ b/internal/run/tui/app/modal_test.go @@ -58,12 +58,12 @@ func TestOpenRegionModal(t *testing.T) { func TestModalCallbacks(t *testing.T) { setupTestApp() buildLayout() - + // Create temp home for config save tmpDir, _ := os.MkdirTemp("", "run-cli-app-test") defer func() { _ = os.RemoveAll(tmpDir) }() _ = os.Setenv("HOME", tmpDir) - + // 1. Project Callback project.CachedProjects = []model_project.Project{{Name: "new-p"}} openProjectModal() @@ -71,10 +71,10 @@ func TestModalCallbacks(t *testing.T) { sel := projectModal.(*project.ProjectSelector) sel.List.SetCurrentItem(0) sel.Submit() // Triggers onSelect - + assert.Equal(t, "new-p", currentInfo.Project) assert.Equal(t, "new-p", currentConfig.Project) - + // 2. Region Callback openRegionModal() selReg := regionModal.(*region.RegionSelector) @@ -82,7 +82,7 @@ func TestModalCallbacks(t *testing.T) { selReg.Filter("us-east1") selReg.List.SetCurrentItem(0) selReg.Submit() - + assert.Equal(t, "us-east1", currentInfo.Region) assert.Equal(t, "us-east1", currentConfig.Region) } diff --git a/internal/run/tui/app/project/project_test.go b/internal/run/tui/app/project/project_test.go index 779b5a0..b4b8590 100644 --- a/internal/run/tui/app/project/project_test.go +++ b/internal/run/tui/app/project/project_test.go @@ -27,52 +27,52 @@ import ( func TestProjectModal(t *testing.T) { app := tview.NewApplication() - + // Pre-populate cache CachedProjects = []model.Project{ {Name: "p1"}, {Name: "p2"}, } defer func() { CachedProjects = nil }() - + selector := ProjectModal(app, func(p model.Project) {}, func() {}) - + assert.NotNil(t, selector) assert.NotNil(t, selector.Input) assert.NotNil(t, selector.List) assert.NotNil(t, selector.Filter) assert.NotNil(t, selector.Submit) assert.NotNil(t, selector.Content) - + // Should satisfy Primitive interface implicitly var _ tview.Primitive = selector } func TestProjectModal_Filtering(t *testing.T) { app := tview.NewApplication() - + CachedProjects = []model.Project{ {Name: "alpha"}, {Name: "beta"}, {Name: "gamma"}, } defer func() { CachedProjects = nil }() - + selector := ProjectModal(app, func(p model.Project) {}, func() {}) - + // Initial state: empty filter -> all items assert.Equal(t, 3, selector.List.GetItemCount()) - + // Filter "a" -> alpha, beta, gamma (all contain 'a') selector.Filter("a") assert.Equal(t, 3, selector.List.GetItemCount()) - + // Filter "al" -> alpha selector.Filter("al") assert.Equal(t, 1, selector.List.GetItemCount()) mainText, _ := selector.List.GetItemText(0) assert.Equal(t, "alpha", mainText) - + // Filter "z" -> none selector.Filter("z") assert.Equal(t, 0, selector.List.GetItemCount()) @@ -80,35 +80,35 @@ func TestProjectModal_Filtering(t *testing.T) { func TestProjectModal_Selection(t *testing.T) { app := tview.NewApplication() - + CachedProjects = []model.Project{ {Name: "target"}, {Name: "other"}, } defer func() { CachedProjects = nil }() - + var selected model.Project closed := false - + onSelect := func(p model.Project) { selected = p } closeModal := func() { closed = true } - + selector := ProjectModal(app, onSelect, closeModal) - + // Filter to ensure target is at index 0 selector.Filter("target") assert.Equal(t, 1, selector.List.GetItemCount()) - + // Select index 0 (which is "target") selector.List.SetCurrentItem(0) - + // Trigger submit selector.Submit() - + assert.True(t, closed) assert.Equal(t, "target", selected.Name) } @@ -117,39 +117,39 @@ func TestInputCapture(t *testing.T) { app := tview.NewApplication() closed := false closeModal := func() { closed = true } - + selector := ProjectModal(app, func(p model.Project) {}, closeModal) - + handler := selector.Content.GetInputCapture() assert.NotNil(t, handler) - + // Test Escape eventEsc := tcell.NewEventKey(tcell.KeyEscape, 0, tcell.ModNone) ret := handler(eventEsc) assert.Nil(t, ret) assert.True(t, closed) - + // Test Tab Cycling // Initial focus is Input (set by layout order implicitly, but Application manages focus) - // We simulate focus by setting it on Application mock? + // We simulate focus by setting it on Application mock? // tview.Application doesn't expose GetFocus easily for verification in unit test without running. // However, we can verifying that SetFocus is called on the app. // But we passed a real app. - - // Since we can't easily assert "Focus changed" on `app` without internals, + + // Since we can't easily assert "Focus changed" on `app` without internals, // we will just run the handler coverage. - + // Simulate Input has Focus app.SetFocus(selector.Input) eventTab := tcell.NewEventKey(tcell.KeyTab, 0, tcell.ModNone) handler(eventTab) // Should move focus to List assert.True(t, selector.List.HasFocus()) - + // Simulate List has Focus app.SetFocus(selector.List) handler(eventTab) // Should move focus to BtnSelect // Can't check button focus easily as it is not exposed in selector. - + // Simulate Down arrow from Input app.SetFocus(selector.Input) eventDown := tcell.NewEventKey(tcell.KeyDown, 0, tcell.ModNone) diff --git a/internal/run/tui/app/region/region_test.go b/internal/run/tui/app/region/region_test.go index 878962b..fe27312 100644 --- a/internal/run/tui/app/region/region_test.go +++ b/internal/run/tui/app/region/region_test.go @@ -34,7 +34,7 @@ func TestRegionModal_Init(t *testing.T) { assert.NotNil(t, selector.List) assert.NotNil(t, selector.Filter) assert.NotNil(t, selector.Submit) - + // Should satisfy Primitive interface var _ tview.Primitive = selector } @@ -56,7 +56,7 @@ func TestRegionModal_Filtering(t *testing.T) { // Filter "non-existent-region" selector.Filter("non-existent-region") assert.Equal(t, 0, selector.List.GetItemCount()) - + // Reset selector.Filter("") assert.Equal(t, initialCount, selector.List.GetItemCount()) @@ -66,7 +66,7 @@ func TestRegionModal_Selection(t *testing.T) { app := tview.NewApplication() var selectedRegion string closed := false - + onSelect := func(r string) { selectedRegion = r } @@ -80,18 +80,18 @@ func TestRegionModal_Selection(t *testing.T) { selector.Filter("europe-west1") selector.List.SetCurrentItem(0) selector.Submit() - + assert.True(t, closed) assert.Equal(t, "europe-west1", selectedRegion) - + // Reset closed = false selectedRegion = "" - + // Test selecting "All Regions" // We know "- (All Regions)" is added first in the list selector.Filter("") // Reset filter - + // Find the index of "- (All Regions)" idx := -1 for i := 0; i < selector.List.GetItemCount(); i++ { @@ -102,10 +102,10 @@ func TestRegionModal_Selection(t *testing.T) { } } assert.NotEqual(t, -1, idx, "Could not find 'All Regions' option") - + selector.List.SetCurrentItem(idx) selector.Submit() - + assert.True(t, closed) assert.Equal(t, api_region.ALL, selectedRegion) } @@ -114,26 +114,26 @@ func TestInputCapture(t *testing.T) { app := tview.NewApplication() closed := false closeModal := func() { closed = true } - + selector := RegionModal(app, func(s string) {}, closeModal) handler := selector.Content.GetInputCapture() - + // Test Escape eventEsc := tcell.NewEventKey(tcell.KeyEscape, 0, tcell.ModNone) ret := handler(eventEsc) assert.Nil(t, ret) assert.True(t, closed) - + // Test Tab Cycling // Simulate Input has Focus app.SetFocus(selector.Input) eventTab := tcell.NewEventKey(tcell.KeyTab, 0, tcell.ModNone) handler(eventTab) // Should move focus to List assert.True(t, selector.List.HasFocus()) - + // Simulate Down arrow from Input app.SetFocus(selector.Input) eventDown := tcell.NewEventKey(tcell.KeyDown, 0, tcell.ModNone) handler(eventDown) assert.True(t, selector.List.HasFocus()) -} \ No newline at end of file +} diff --git a/internal/run/tui/app/service/dashboard_test.go b/internal/run/tui/app/service/dashboard_test.go index e7d16f5..c831701 100644 --- a/internal/run/tui/app/service/dashboard_test.go +++ b/internal/run/tui/app/service/dashboard_test.go @@ -20,8 +20,8 @@ import ( "testing" "time" - "github.com/JulienBreux/run-cli/internal/run/model/common/info" model_container "github.com/JulienBreux/run-cli/internal/run/model/common/container" + "github.com/JulienBreux/run-cli/internal/run/model/common/info" model_resources "github.com/JulienBreux/run-cli/internal/run/model/common/resources" model_service "github.com/JulienBreux/run-cli/internal/run/model/service" model_networking "github.com/JulienBreux/run-cli/internal/run/model/service/networking" @@ -53,7 +53,7 @@ func TestDashboardShortcuts(t *testing.T) { func TestUpdateTabs(t *testing.T) { app := tview.NewApplication() _ = Dashboard(app) - + // Setup dummy service dashboardService = &model_service.Service{ Name: "s1", @@ -64,11 +64,11 @@ func TestUpdateTabs(t *testing.T) { InvokerIAMDisabled: true, }, } - + assert.NotPanics(t, func() { updateNetworkingTab() assert.Contains(t, networkingDetail.GetText(true), "Allow all traffic") - + updateSecurityTab() assert.Contains(t, securityDetail.GetText(true), "Allow unauthenticated invocations") }) @@ -78,7 +78,7 @@ func TestDashboardReload(t *testing.T) { // Setup Mocks origList := listRevisionsFunc defer func() { listRevisionsFunc = origList }() - + called := false listRevisionsFunc = func(project, region, service string) ([]model_revision.Revision, error) { called = true @@ -86,45 +86,45 @@ func TestDashboardReload(t *testing.T) { {Name: "rev1", CreateTime: time.Now()}, }, nil } - + // Init app := tview.NewApplication() // Use SimulationScreen to allow QueueUpdateDraw to work if we ran app // However, QueueUpdateDraw blocks if app not running or just queues it. // We can manually trigger the callback passed to QueueUpdateDraw if we mock app? // Tview Application struct is hard to mock internal logic. - + // But we can just run the function and check if it started the goroutine/called mock. // Since it's async, we use channels or wait. - + svc := &model_service.Service{Name: "s1", Region: "r1"} done := make(chan struct{}) - + DashboardReload(app, info.Info{Project: "p"}, svc, func(err error) { assert.NoError(t, err) close(done) }) - - // Since QueueUpdateDraw might not execute without running App, + + // Since QueueUpdateDraw might not execute without running App, // we might need to run app or rely on the fact that we mocked listRevisionsFunc. // Wait, listRevisionsFunc is called synchronously? No, inside goroutine. - - // To test this properly without race conditions or hanging, we should use the SimulationScreen pattern + + // To test this properly without race conditions or hanging, we should use the SimulationScreen pattern // similar to Spinner test if we really want to execute the callback. - + // Let's try to just wait for 'called' to be true? No, that's racy. - + // Ideally we use SimulationScreen and run app. screen := tcell.NewSimulationScreen("UTF-8") _ = screen.Init() app.SetScreen(screen) - + go func() { // Run app loop to process updates _ = app.Run() }() defer app.Stop() - + select { case <-done: assert.True(t, called) @@ -139,18 +139,18 @@ func TestDashboardReload(t *testing.T) { func TestDashboardInputCapture(t *testing.T) { app := tview.NewApplication() d := Dashboard(app) - + handler := d.GetInputCapture() assert.NotNil(t, handler) - + // Initial tab is 0 activeTab = 0 - + // Test Tab (Right) eventTab := tcell.NewEventKey(tcell.KeyTab, 0, tcell.ModNone) handler(eventTab) assert.Equal(t, 1, activeTab) - + // Test Backtab (Left) eventBack := tcell.NewEventKey(tcell.KeyBacktab, 0, tcell.ModNone) handler(eventBack) @@ -160,27 +160,27 @@ func TestDashboardInputCapture(t *testing.T) { func TestDashboardReload_Error(t *testing.T) { origList := listRevisionsFunc defer func() { listRevisionsFunc = origList }() - + listRevisionsFunc = func(project, region, service string) ([]model_revision.Revision, error) { return nil, assert.AnError } - + app := tview.NewApplication() screen := tcell.NewSimulationScreen("UTF-8") _ = screen.Init() app.SetScreen(screen) - + go func() { _ = app.Run() }() defer app.Stop() - + svc := &model_service.Service{Name: "s1"} done := make(chan struct{}) - + DashboardReload(app, info.Info{}, svc, func(err error) { assert.Error(t, err) close(done) }) - + select { case <-done: case <-time.After(2 * time.Second): @@ -192,11 +192,11 @@ func TestUpdateRevisionDetail(t *testing.T) { // Initialize global variables app := tview.NewApplication() _ = Dashboard(app) - + // Setup data dashboardRevisions = []model_revision.Revision{ { - Name: "rev1", + Name: "rev1", ExecutionEnvironment: "EXECUTION_ENVIRONMENT_GEN1", Containers: []*model_container.Container{ { @@ -204,22 +204,22 @@ func TestUpdateRevisionDetail(t *testing.T) { Resources: &model_resources.Resources{ Limits: map[string]string{ "memory": "512Mi", - "cpu": "1", + "cpu": "1", }, }, }, }, }, { - Name: "rev2", + Name: "rev2", ExecutionEnvironment: "EXECUTION_ENVIRONMENT_GEN2", - Accelerator: "nvidia-tesla-t4", + Accelerator: "nvidia-tesla-t4", Containers: []*model_container.Container{ { Resources: &model_resources.Resources{ Limits: map[string]string{ - "memory": "1Gi", - "cpu": "2", + "memory": "1Gi", + "cpu": "2", "nvidia.com/gpu": "1", }, }, @@ -227,20 +227,20 @@ func TestUpdateRevisionDetail(t *testing.T) { }, }, } - + assert.NotPanics(t, func() { // Row 1 (rev1) updateRevisionDetail(1) text := revisionsDetail.GetText(true) assert.Contains(t, text, "First Generation") assert.Contains(t, text, "512Mi Memory, 1 CPU") - + // Row 2 (rev2) updateRevisionDetail(2) text2 := revisionsDetail.GetText(true) assert.Contains(t, text2, "Second Generation") assert.Contains(t, text2, "1 GPU (nvidia-tesla-t4)") - + // Invalid Row updateRevisionDetail(0) assert.Equal(t, "", revisionsDetail.GetText(true)) diff --git a/internal/run/tui/app/service/revision/revision_test.go b/internal/run/tui/app/service/revision/revision_test.go index de46152..4528239 100644 --- a/internal/run/tui/app/service/revision/revision_test.go +++ b/internal/run/tui/app/service/revision/revision_test.go @@ -20,10 +20,10 @@ import ( "testing" "time" - model_revision "github.com/JulienBreux/run-cli/internal/run/model/service/revision" - model_service "github.com/JulienBreux/run-cli/internal/run/model/service" model_container "github.com/JulienBreux/run-cli/internal/run/model/common/container" model_resources "github.com/JulienBreux/run-cli/internal/run/model/common/resources" + model_service "github.com/JulienBreux/run-cli/internal/run/model/service" + model_revision "github.com/JulienBreux/run-cli/internal/run/model/service/revision" "github.com/rivo/tview" "github.com/stretchr/testify/assert" ) @@ -53,25 +53,25 @@ func TestListComponent_Update(t *testing.T) { func TestDetailComponent_Update(t *testing.T) { comp := NewDetailComponent() - + rev := model_revision.Revision{ - Name: "rev1", - Author: "user@example.com", - CreateTime: time.Now(), - CpuIdle: true, - StartupCpuBoost: true, + Name: "rev1", + Author: "user@example.com", + CreateTime: time.Now(), + CpuIdle: true, + StartupCpuBoost: true, MaxInstanceRequestConcurrency: 80, - Timeout: 300 * time.Second, - ExecutionEnvironment: "EXECUTION_ENVIRONMENT_GEN2", + Timeout: 300 * time.Second, + ExecutionEnvironment: "EXECUTION_ENVIRONMENT_GEN2", Containers: []*model_container.Container{ { - Name: "c1", + Name: "c1", Image: "image1", Ports: []*model_container.Port{{ContainerPort: 8080}}, Resources: &model_resources.Resources{ Limits: map[string]string{ "memory": "512Mi", - "cpu": "1", + "cpu": "1", }, }, }, diff --git a/internal/run/tui/app/service/traffic/split.go b/internal/run/tui/app/service/traffic/split.go index 5613c3c..13ce0e7 100644 --- a/internal/run/tui/app/service/traffic/split.go +++ b/internal/run/tui/app/service/traffic/split.go @@ -92,7 +92,7 @@ func Modal(app *tview.Application, service *model_service.Service, allRevisions tcell.StyleDefault.Background(tcell.ColorDarkGray), tcell.StyleDefault.Background(tcell.ColorLightCyan).Foreground(tcell.ColorBlack), ) - + // Set initial selection foundIndex := -1 for i, opt := range revOptions { @@ -136,7 +136,7 @@ func Modal(app *tview.Application, service *model_service.Service, allRevisions } } } - + // Ensure at least one row if empty (though usually traffic is 100%) if len(rows) == 0 { // Default to latest ready revision if possible, else first available @@ -147,21 +147,21 @@ func Modal(app *tview.Application, service *model_service.Service, allRevisions addRow(defaultRev, "100") } - // Add Button (Dynamic Adding not easily supported by tview.Form structure in one pass, - // typically requires rebuilding the form or using a custom layout. + // Add Button (Dynamic Adding not easily supported by tview.Form structure in one pass, + // typically requires rebuilding the form or using a custom layout. // For simplicity in this iteration, we will just allow editing existing splits. - // OR we can add a "Add Split" button that rebuilds the form? + // OR we can add a "Add Split" button that rebuilds the form? // tview.Form doesn't expose InsertItem easily. - - // Let's rely on a simpler approach: + + // Let's rely on a simpler approach: // We list *all* revisions? No, that's too many. // We need to allow adding. - + // Let's try to add a "Add Revision" button *at the end*? // The standard Form AddButton adds to the bottom bar. - + form.AddButton("Add Revision", func() { - // We can't dynamically insert form items easily into the *middle* of the rendered form list + // We can't dynamically insert form items easily into the *middle* of the rendered form list // without rebuilding or hacking internals. // However, we can just append to the form items list. if len(allRevisions) > 0 { @@ -174,15 +174,15 @@ func Modal(app *tview.Application, service *model_service.Service, allRevisions form.AddButton("Save", func() { var params []string var targets []model_traffic.TrafficTarget - + // Collect data // Iterate through our rows struct which holds references for _, r := range rows { _, rev := r.dropdown.GetCurrentOption() percentText := r.input.GetText() - + params = append(params, percentText) - + percent, _ := strconv.ParseInt(percentText, 10, 32) targets = append(targets, model_traffic.TrafficTarget{ Revision: rev, @@ -231,7 +231,7 @@ func Modal(app *tview.Application, service *model_service.Service, allRevisions // Global Grid wrapper for centering grid := tview.NewGrid(). SetColumns(0, 60, 0). - SetRows(0, 20, 0). // Fixed height for now, scrolling handled by Form if needed? Form scrolls? + SetRows(0, 20, 0). // Fixed height for now, scrolling handled by Form if needed? Form scrolls? AddItem(container, 1, 1, 1, 1, 0, 0, true) // Escape to Close @@ -262,4 +262,4 @@ func validateTrafficParams(params []string) (int64, error) { return sum, fmt.Errorf("total percentage must be 100, current: %d", sum) } return sum, nil -} \ No newline at end of file +} diff --git a/internal/run/tui/app/service/traffic/split_test.go b/internal/run/tui/app/service/traffic/split_test.go index 3501094..8a43e92 100644 --- a/internal/run/tui/app/service/traffic/split_test.go +++ b/internal/run/tui/app/service/traffic/split_test.go @@ -88,4 +88,4 @@ func TestModal(t *testing.T) { modal := Modal(app, service, revisions, func(refresh bool) {}) assert.NotNil(t, modal) -} \ No newline at end of file +} diff --git a/internal/run/tui/component/header/header.go b/internal/run/tui/component/header/header.go index 1a58948..859dedd 100644 --- a/internal/run/tui/component/header/header.go +++ b/internal/run/tui/component/header/header.go @@ -105,10 +105,10 @@ func columnShortcuts() *tview.Flex { // No, standard reading is Col 1 then Col 2. // Current: Col 1 (Project) | Col 2 (Services) // Registry: Services, ..., Project - + // If I want Project in Col 1, I should reorder Registry or filter specifically. // Reordering Registry seems cleanest for "Single Source of Truth defining order". - + _, _ = fmt.Fprint(col2, formatted) // Put early items in Col 2 to match existing (Services on right) } else { _, _ = fmt.Fprint(col1, formatted) // Put later items in Col 1 (Project on left) diff --git a/internal/run/tui/component/header/header_test.go b/internal/run/tui/component/header/header_test.go index 7a83408..7234358 100644 --- a/internal/run/tui/component/header/header_test.go +++ b/internal/run/tui/component/header/header_test.go @@ -52,13 +52,13 @@ func TestUpdateInfo(t *testing.T) { Region: "r2", User: "u2", } - - // This function modifies the global infoView. + + // This function modifies the global infoView. // Since we can't inspect the text content easily without drawing, we just ensure it doesn't panic. assert.NotPanics(t, func() { header.UpdateInfo(newInfo) }) - - // Note: Testing side effects on global variables is brittle in parallel tests, + + // Note: Testing side effects on global variables is brittle in parallel tests, // but acceptable here given the legacy code structure. } diff --git a/internal/run/tui/component/loader/loader_test.go b/internal/run/tui/component/loader/loader_test.go index 4cfb50d..1dfdb1a 100644 --- a/internal/run/tui/component/loader/loader_test.go +++ b/internal/run/tui/component/loader/loader_test.go @@ -31,4 +31,4 @@ func TestNew(t *testing.T) { assert.NotNil(t, l.Flex) assert.NotNil(t, l.Spinner) assert.Equal(t, 4, l.GetItemCount()) -} \ No newline at end of file +} diff --git a/internal/run/tui/component/logo/logo_test.go b/internal/run/tui/component/logo/logo_test.go index 3134121..b666dcd 100644 --- a/internal/run/tui/component/logo/logo_test.go +++ b/internal/run/tui/component/logo/logo_test.go @@ -51,8 +51,8 @@ func TestNew(t *testing.T) { if l == nil { t.Error("logo.New() should return a non-nil TextView") } - - // We can't easily extract text from TextView directly without drawing, + + // We can't easily extract text from TextView directly without drawing, // but we can check if it was initialized (not crashing). // In a real TUI test we might use a screen simulation, but for unit test ensuring it returns expected type is often enough. // However, we can assert properties if we want. diff --git a/internal/run/tui/component/table/table_test.go b/internal/run/tui/component/table/table_test.go index 0b650a9..bbb90ea 100644 --- a/internal/run/tui/component/table/table_test.go +++ b/internal/run/tui/component/table/table_test.go @@ -84,4 +84,4 @@ func TestBorderWrapper_Draw(t *testing.T) { defer screen.Fini() wrapper.Draw(screen) -} \ No newline at end of file +} diff --git a/pkg/dropdown/dropdown_test.go b/pkg/dropdown/dropdown_test.go index 3ab162b..f26cb90 100644 --- a/pkg/dropdown/dropdown_test.go +++ b/pkg/dropdown/dropdown_test.go @@ -35,7 +35,7 @@ func TestDraw(t *testing.T) { screen := tcell.NewSimulationScreen("") err := screen.Init() assert.NoError(t, err) - + d.Draw(screen) // No panic implies success for now }