diff --git a/cli/cliui/table.go b/cli/cliui/table.go index 478bbe2260..c828548022 100644 --- a/cli/cliui/table.go +++ b/cli/cliui/table.go @@ -296,22 +296,23 @@ func renderTable(out any, sort string, headers table.Row, filterColumns []string // returned. If the table tag is malformed, an error is returned. // // The returned name is transformed from "snake_case" to "normal text". -func parseTableStructTag(field reflect.StructField) (name string, defaultSort, noSortOpt, recursive, skipParentName bool, err error) { +func parseTableStructTag(field reflect.StructField) (name string, defaultSort, noSortOpt, recursive, skipParentName, emptyNil bool, err error) { tags, err := structtag.Parse(string(field.Tag)) if err != nil { - return "", false, false, false, false, xerrors.Errorf("parse struct field tag %q: %w", string(field.Tag), err) + return "", false, false, false, false, false, xerrors.Errorf("parse struct field tag %q: %w", string(field.Tag), err) } tag, err := tags.Get("table") if err != nil || tag.Name == "-" { // tags.Get only returns an error if the tag is not found. - return "", false, false, false, false, nil + return "", false, false, false, false, false, nil } defaultSortOpt := false noSortOpt = false recursiveOpt := false skipParentNameOpt := false + emptyNilOpt := false for _, opt := range tag.Options { switch opt { case "default_sort": @@ -326,12 +327,14 @@ func parseTableStructTag(field reflect.StructField) (name string, defaultSort, n // make sure the child name is unique across all nested structs in the parent. recursiveOpt = true skipParentNameOpt = true + case "empty_nil": + emptyNilOpt = true default: - return "", false, false, false, false, xerrors.Errorf("unknown option %q in struct field tag", opt) + return "", false, false, false, false, false, xerrors.Errorf("unknown option %q in struct field tag", opt) } } - return strings.ReplaceAll(tag.Name, "_", " "), defaultSortOpt, noSortOpt, recursiveOpt, skipParentNameOpt, nil + return strings.ReplaceAll(tag.Name, "_", " "), defaultSortOpt, noSortOpt, recursiveOpt, skipParentNameOpt, emptyNilOpt, nil } func isStructOrStructPointer(t reflect.Type) bool { @@ -358,7 +361,7 @@ func typeToTableHeaders(t reflect.Type, requireDefault bool) ([]string, string, noSortOpt := false for i := 0; i < t.NumField(); i++ { field := t.Field(i) - name, defaultSort, noSort, recursive, skip, err := parseTableStructTag(field) + name, defaultSort, noSort, recursive, skip, _, err := parseTableStructTag(field) if err != nil { return nil, "", xerrors.Errorf("parse struct tags for field %q in type %q: %w", field.Name, t.String(), err) } @@ -435,7 +438,7 @@ func valueToTableMap(val reflect.Value) (map[string]any, error) { for i := 0; i < val.NumField(); i++ { field := val.Type().Field(i) fieldVal := val.Field(i) - name, _, _, recursive, skip, err := parseTableStructTag(field) + name, _, _, recursive, skip, emptyNil, err := parseTableStructTag(field) if err != nil { return nil, xerrors.Errorf("parse struct tags for field %q in type %T: %w", field.Name, val, err) } @@ -443,8 +446,14 @@ func valueToTableMap(val reflect.Value) (map[string]any, error) { continue } - // Recurse if it's a struct. fieldType := field.Type + + // If empty_nil is set and this is a nil pointer, use a zero value. + if emptyNil && fieldVal.Kind() == reflect.Pointer && fieldVal.IsNil() { + fieldVal = reflect.New(fieldType.Elem()) + } + + // Recurse if it's a struct. if recursive { if !isStructOrStructPointer(fieldType) { return nil, xerrors.Errorf("field %q in type %q is marked as recursive but does not contain a struct or a pointer to a struct", field.Name, fieldType.String()) @@ -467,7 +476,7 @@ func valueToTableMap(val reflect.Value) (map[string]any, error) { } // Otherwise, we just use the field value. - row[name] = val.Field(i).Interface() + row[name] = fieldVal.Interface() } return row, nil diff --git a/cli/cliui/table_test.go b/cli/cliui/table_test.go index 4e82707f3f..424b9c9a7d 100644 --- a/cli/cliui/table_test.go +++ b/cli/cliui/table_test.go @@ -400,6 +400,78 @@ foo 10 [a, b, c] foo1 11 foo2 12 fo }) }) }) + + t.Run("EmptyNil", func(t *testing.T) { + t.Parallel() + + type emptyNilTest struct { + Name string `table:"name,default_sort"` + EmptyOnNil *string `table:"empty_on_nil,empty_nil"` + NormalBehavior *string `table:"normal_behavior"` + } + + value := "value" + in := []emptyNilTest{ + { + Name: "has_value", + EmptyOnNil: &value, + NormalBehavior: &value, + }, + { + Name: "has_nil", + EmptyOnNil: nil, + NormalBehavior: nil, + }, + } + + expected := ` +NAME EMPTY ON NIL NORMAL BEHAVIOR +has_nil +has_value value value + ` + + out, err := cliui.DisplayTable(in, "", nil) + log.Println("rendered table:\n" + out) + require.NoError(t, err) + compareTables(t, expected, out) + }) + + t.Run("EmptyNilWithRecursiveInline", func(t *testing.T) { + t.Parallel() + + type nestedData struct { + Name string `table:"name"` + } + + type inlineTest struct { + Nested *nestedData `table:"ignored,recursive_inline,empty_nil"` + Count int `table:"count,default_sort"` + } + + in := []inlineTest{ + { + Nested: &nestedData{ + Name: "alice", + }, + Count: 1, + }, + { + Nested: nil, + Count: 2, + }, + } + + expected := ` +NAME COUNT +alice 1 + 2 + ` + + out, err := cliui.DisplayTable(in, "", nil) + log.Println("rendered table:\n" + out) + require.NoError(t, err) + compareTables(t, expected, out) + }) } // compareTables normalizes the incoming table lines diff --git a/cli/exp_task_list_test.go b/cli/exp_task_list_test.go index d1e0862dc7..d297310dc4 100644 --- a/cli/exp_task_list_test.go +++ b/cli/exp_task_list_test.go @@ -162,7 +162,7 @@ func TestExpTaskList(t *testing.T) { // Validate the table includes the task and status. pty.ExpectMatch(task.Name) - pty.ExpectMatch("running") + pty.ExpectMatch("initializing") pty.ExpectMatch(wantPrompt) }) @@ -175,9 +175,9 @@ func TestExpTaskList(t *testing.T) { owner := coderdtest.CreateFirstUser(t, client) memberClient, memberUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) - // Create two AI tasks: one running, one stopped. - runningTask := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStart, "keep me running") - stoppedTask := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStop, "stop me please") + // Create two AI tasks: one initializing, one paused. + initializingTask := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStart, "keep me initializing") + pausedTask := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStop, "stop me please") // Use JSON output to reliably validate filtering. inv, root := clitest.New(t, "exp", "task", "list", "--status=paused", "--output=json") @@ -194,10 +194,10 @@ func TestExpTaskList(t *testing.T) { var tasks []codersdk.Task require.NoError(t, json.Unmarshal(stdout.Bytes(), &tasks)) - // Only the stopped task is returned. + // Only the paused task is returned. require.Len(t, tasks, 1, "expected one task after filtering") - require.Equal(t, stoppedTask.ID, tasks[0].ID) - require.NotEqual(t, runningTask.ID, tasks[0].ID) + require.Equal(t, pausedTask.ID, tasks[0].ID) + require.NotEqual(t, initializingTask.ID, tasks[0].ID) }) t.Run("UserFlag_Me_Table", func(t *testing.T) { @@ -234,7 +234,7 @@ func TestExpTaskList(t *testing.T) { memberClient, memberUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) // Given: We have two tasks - task1 := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStart, "keep me running") + task1 := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStart, "keep me active") task2 := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStop, "stop me please") // Given: We add the `--quiet` flag diff --git a/cli/exp_task_status.go b/cli/exp_task_status.go index f34812657a..1bd77f5f7f 100644 --- a/cli/exp_task_status.go +++ b/cli/exp_task_status.go @@ -156,28 +156,21 @@ func taskWatchIsEnded(task codersdk.Task) bool { } type taskStatusRow struct { - codersdk.Task `table:"-"` - ChangedAgo string `json:"-" table:"state changed,default_sort"` - Timestamp time.Time `json:"-" table:"-"` - TaskStatus string `json:"-" table:"status"` - Healthy bool `json:"-" table:"healthy"` - TaskState string `json:"-" table:"state"` - Message string `json:"-" table:"message"` + codersdk.Task `table:"r,recursive_inline"` + ChangedAgo string `json:"-" table:"state changed"` + Healthy bool `json:"-" table:"healthy"` } func taskStatusRowEqual(r1, r2 taskStatusRow) bool { - return r1.TaskStatus == r2.TaskStatus && + return r1.Status == r2.Status && r1.Healthy == r2.Healthy && - r1.TaskState == r2.TaskState && - r1.Message == r2.Message + taskStateEqual(r1.CurrentState, r2.CurrentState) } func toStatusRow(task codersdk.Task) taskStatusRow { tsr := taskStatusRow{ Task: task, ChangedAgo: time.Since(task.UpdatedAt).Truncate(time.Second).String() + " ago", - Timestamp: task.UpdatedAt, - TaskStatus: string(task.WorkspaceStatus), } tsr.Healthy = task.WorkspaceAgentHealth != nil && task.WorkspaceAgentHealth.Healthy && @@ -187,9 +180,19 @@ func toStatusRow(task codersdk.Task) taskStatusRow { if task.CurrentState != nil { tsr.ChangedAgo = time.Since(task.CurrentState.Timestamp).Truncate(time.Second).String() + " ago" - tsr.Timestamp = task.CurrentState.Timestamp - tsr.TaskState = string(task.CurrentState.State) - tsr.Message = task.CurrentState.Message } return tsr } + +func taskStateEqual(se1, se2 *codersdk.TaskStateEntry) bool { + var s1, m1, s2, m2 string + if se1 != nil { + s1 = string(se1.State) + m1 = se1.Message + } + if se2 != nil { + s2 = string(se2.State) + m2 = se2.Message + } + return s1 == s2 && m1 == m2 +} diff --git a/cli/exp_task_status_test.go b/cli/exp_task_status_test.go index 01b2a8379d..ae6f1db29d 100644 --- a/cli/exp_task_status_test.go +++ b/cli/exp_task_status_test.go @@ -55,8 +55,8 @@ func Test_TaskStatus(t *testing.T) { }, { args: []string{"exists"}, - expectOutput: `STATE CHANGED STATUS HEALTHY STATE MESSAGE -0s ago running true working Thinking furiously...`, + expectOutput: `STATE CHANGED STATUS HEALTHY STATE MESSAGE +0s ago active true working Thinking furiously...`, hf: func(ctx context.Context, now time.Time) func(w http.ResponseWriter, r *http.Request) { return func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { @@ -114,12 +114,12 @@ func Test_TaskStatus(t *testing.T) { }, { args: []string{"exists", "--watch"}, - expectOutput: ` -STATE CHANGED STATUS HEALTHY STATE MESSAGE + expectOutput: `STATE CHANGED STATUS HEALTHY STATE MESSAGE 5s ago pending true -4s ago running true -3s ago running true working Reticulating splines... -2s ago running true complete Splines reticulated successfully!`, +4s ago initializing true +4s ago active true +3s ago active true working Reticulating splines... +2s ago active true complete Splines reticulated successfully!`, hf: func(ctx context.Context, now time.Time) func(http.ResponseWriter, *http.Request) { var calls atomic.Int64 return func(w http.ResponseWriter, r *http.Request) { diff --git a/cli/exp_task_test.go b/cli/exp_task_test.go index 82457ec748..d2d3728aeb 100644 --- a/cli/exp_task_test.go +++ b/cli/exp_task_test.go @@ -94,8 +94,7 @@ func Test_Tasks(t *testing.T) { var task codersdk.Task require.NoError(t, json.NewDecoder(strings.NewReader(stdout)).Decode(&task), "should unmarshal task status") require.Equal(t, task.Name, taskName, "task name should match") - // NOTE: task status changes type, this is so this test works with both old and new model - require.Contains(t, []string{"active", "running"}, string(task.Status), "task should be active") + require.Equal(t, codersdk.TaskStatusActive, task.Status, "task should be active") }, }, { diff --git a/codersdk/aitasks.go b/codersdk/aitasks.go index 318caa3d17..bf55cd6eda 100644 --- a/codersdk/aitasks.go +++ b/codersdk/aitasks.go @@ -163,15 +163,15 @@ type Task struct { TemplateDisplayName string `json:"template_display_name" table:"template display name"` TemplateIcon string `json:"template_icon" table:"template icon"` WorkspaceID uuid.NullUUID `json:"workspace_id" format:"uuid" table:"workspace id"` - WorkspaceStatus WorkspaceStatus `json:"workspace_status,omitempty" enums:"pending,starting,running,stopping,stopped,failed,canceling,canceled,deleting,deleted" table:"status"` + WorkspaceStatus WorkspaceStatus `json:"workspace_status,omitempty" enums:"pending,starting,running,stopping,stopped,failed,canceling,canceled,deleting,deleted" table:"workspace status"` WorkspaceBuildNumber int32 `json:"workspace_build_number,omitempty" table:"workspace build number"` WorkspaceAgentID uuid.NullUUID `json:"workspace_agent_id" format:"uuid" table:"workspace agent id"` WorkspaceAgentLifecycle *WorkspaceAgentLifecycle `json:"workspace_agent_lifecycle" table:"workspace agent lifecycle"` WorkspaceAgentHealth *WorkspaceAgentHealth `json:"workspace_agent_health" table:"workspace agent health"` WorkspaceAppID uuid.NullUUID `json:"workspace_app_id" format:"uuid" table:"workspace app id"` InitialPrompt string `json:"initial_prompt" table:"initial prompt"` - Status TaskStatus `json:"status" enums:"pending,initializing,active,paused,unknown,error" table:"task status"` - CurrentState *TaskStateEntry `json:"current_state" table:"cs,recursive_inline"` + Status TaskStatus `json:"status" enums:"pending,initializing,active,paused,unknown,error" table:"status"` + CurrentState *TaskStateEntry `json:"current_state" table:"cs,recursive_inline,empty_nil"` CreatedAt time.Time `json:"created_at" format:"date-time" table:"created at"` UpdatedAt time.Time `json:"updated_at" format:"date-time" table:"updated at"` }