Skip to content

Commit e8e31dc

Browse files
mafredrimafredri
authored
fix(cli): use correct task status in list/status output (coder#20453)
1 parent 40e1784 commit e8e31dc

File tree

7 files changed

+127
-44
lines changed

7 files changed

+127
-44
lines changed

‎cli/cliui/table.go‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -296,22 +296,23 @@ func renderTable(out any, sort string, headers table.Row, filterColumns []string
296296
// returned. If the table tag is malformed, an error is returned.
297297
//
298298
// The returned name is transformed from "snake_case" to "normal text".
299-
func parseTableStructTag(field reflect.StructField) (name string, defaultSort, noSortOpt, recursive, skipParentName bool, err error) {
299+
func parseTableStructTag(field reflect.StructField) (name string, defaultSort, noSortOpt, recursive, skipParentName, emptyNil bool, err error) {
300300
tags, err := structtag.Parse(string(field.Tag))
301301
if err != nil {
302-
return "", false, false, false, false, xerrors.Errorf("parse struct field tag %q: %w", string(field.Tag), err)
302+
return "", false, false, false, false, false, xerrors.Errorf("parse struct field tag %q: %w", string(field.Tag), err)
303303
}
304304

305305
tag, err := tags.Get("table")
306306
if err != nil || tag.Name == "-" {
307307
// tags.Get only returns an error if the tag is not found.
308-
return "", false, false, false, false, nil
308+
return "", false, false, false, false, false, nil
309309
}
310310

311311
defaultSortOpt := false
312312
noSortOpt = false
313313
recursiveOpt := false
314314
skipParentNameOpt := false
315+
emptyNilOpt := false
315316
for _, opt := range tag.Options {
316317
switch opt {
317318
case "default_sort":
@@ -326,12 +327,14 @@ func parseTableStructTag(field reflect.StructField) (name string, defaultSort, n
326327
// make sure the child name is unique across all nested structs in the parent.
327328
recursiveOpt = true
328329
skipParentNameOpt = true
330+
case "empty_nil":
331+
emptyNilOpt = true
329332
default:
330-
return "", false, false, false, false, xerrors.Errorf("unknown option %q in struct field tag", opt)
333+
return "", false, false, false, false, false, xerrors.Errorf("unknown option %q in struct field tag", opt)
331334
}
332335
}
333336

334-
return strings.ReplaceAll(tag.Name, "_", " "), defaultSortOpt, noSortOpt, recursiveOpt, skipParentNameOpt, nil
337+
return strings.ReplaceAll(tag.Name, "_", " "), defaultSortOpt, noSortOpt, recursiveOpt, skipParentNameOpt, emptyNilOpt, nil
335338
}
336339

337340
func isStructOrStructPointer(t reflect.Type) bool {
@@ -358,7 +361,7 @@ func typeToTableHeaders(t reflect.Type, requireDefault bool) ([]string, string,
358361
noSortOpt := false
359362
for i := 0; i < t.NumField(); i++ {
360363
field := t.Field(i)
361-
name, defaultSort, noSort, recursive, skip, err := parseTableStructTag(field)
364+
name, defaultSort, noSort, recursive, skip, _, err := parseTableStructTag(field)
362365
if err != nil {
363366
return nil, "", xerrors.Errorf("parse struct tags for field %q in type %q: %w", field.Name, t.String(), err)
364367
}
@@ -435,16 +438,22 @@ func valueToTableMap(val reflect.Value) (map[string]any, error) {
435438
for i := 0; i < val.NumField(); i++ {
436439
field := val.Type().Field(i)
437440
fieldVal := val.Field(i)
438-
name, _, _, recursive, skip, err := parseTableStructTag(field)
441+
name, _, _, recursive, skip, emptyNil, err := parseTableStructTag(field)
439442
if err != nil {
440443
return nil, xerrors.Errorf("parse struct tags for field %q in type %T: %w", field.Name, val, err)
441444
}
442445
if name == "" {
443446
continue
444447
}
445448

446-
// Recurse if it's a struct.
447449
fieldType := field.Type
450+
451+
// If empty_nil is set and this is a nil pointer, use a zero value.
452+
if emptyNil && fieldVal.Kind() == reflect.Pointer && fieldVal.IsNil() {
453+
fieldVal = reflect.New(fieldType.Elem())
454+
}
455+
456+
// Recurse if it's a struct.
448457
if recursive {
449458
if !isStructOrStructPointer(fieldType) {
450459
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) {
467476
}
468477

469478
// Otherwise, we just use the field value.
470-
row[name] = val.Field(i).Interface()
479+
row[name] = fieldVal.Interface()
471480
}
472481

473482
return row, nil

‎cli/cliui/table_test.go‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,78 @@ foo <nil> 10 [a, b, c] foo1 11 foo2 12 fo
400400
})
401401
})
402402
})
403+
404+
t.Run("EmptyNil", func(t *testing.T) {
405+
t.Parallel()
406+
407+
type emptyNilTest struct {
408+
Name string `table:"name,default_sort"`
409+
EmptyOnNil *string `table:"empty_on_nil,empty_nil"`
410+
NormalBehavior *string `table:"normal_behavior"`
411+
}
412+
413+
value := "value"
414+
in := []emptyNilTest{
415+
{
416+
Name: "has_value",
417+
EmptyOnNil: &value,
418+
NormalBehavior: &value,
419+
},
420+
{
421+
Name: "has_nil",
422+
EmptyOnNil: nil,
423+
NormalBehavior: nil,
424+
},
425+
}
426+
427+
expected := `
428+
NAME EMPTY ON NIL NORMAL BEHAVIOR
429+
has_nil <nil>
430+
has_value value value
431+
`
432+
433+
out, err := cliui.DisplayTable(in, "", nil)
434+
log.Println("rendered table:\n" + out)
435+
require.NoError(t, err)
436+
compareTables(t, expected, out)
437+
})
438+
439+
t.Run("EmptyNilWithRecursiveInline", func(t *testing.T) {
440+
t.Parallel()
441+
442+
type nestedData struct {
443+
Name string `table:"name"`
444+
}
445+
446+
type inlineTest struct {
447+
Nested *nestedData `table:"ignored,recursive_inline,empty_nil"`
448+
Count int `table:"count,default_sort"`
449+
}
450+
451+
in := []inlineTest{
452+
{
453+
Nested: &nestedData{
454+
Name: "alice",
455+
},
456+
Count: 1,
457+
},
458+
{
459+
Nested: nil,
460+
Count: 2,
461+
},
462+
}
463+
464+
expected := `
465+
NAME COUNT
466+
alice 1
467+
2
468+
`
469+
470+
out, err := cliui.DisplayTable(in, "", nil)
471+
log.Println("rendered table:\n" + out)
472+
require.NoError(t, err)
473+
compareTables(t, expected, out)
474+
})
403475
}
404476

405477
// compareTables normalizes the incoming table lines

‎cli/exp_task_list_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ func TestExpTaskList(t *testing.T) {
162162

163163
// Validate the table includes the task and status.
164164
pty.ExpectMatch(task.Name)
165-
pty.ExpectMatch("running")
165+
pty.ExpectMatch("initializing")
166166
pty.ExpectMatch(wantPrompt)
167167
})
168168

@@ -175,9 +175,9 @@ func TestExpTaskList(t *testing.T) {
175175
owner := coderdtest.CreateFirstUser(t, client)
176176
memberClient, memberUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
177177

178-
// Create two AI tasks: one running, one stopped.
179-
runningTask := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStart, "keep me running")
180-
stoppedTask := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStop, "stop me please")
178+
// Create two AI tasks: one initializing, one paused.
179+
initializingTask := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStart, "keep me initializing")
180+
pausedTask := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStop, "stop me please")
181181

182182
// Use JSON output to reliably validate filtering.
183183
inv, root := clitest.New(t, "exp", "task", "list", "--status=paused", "--output=json")
@@ -194,10 +194,10 @@ func TestExpTaskList(t *testing.T) {
194194
var tasks []codersdk.Task
195195
require.NoError(t, json.Unmarshal(stdout.Bytes(), &tasks))
196196

197-
// Only the stopped task is returned.
197+
// Only the paused task is returned.
198198
require.Len(t, tasks, 1, "expected one task after filtering")
199-
require.Equal(t, stoppedTask.ID, tasks[0].ID)
200-
require.NotEqual(t, runningTask.ID, tasks[0].ID)
199+
require.Equal(t, pausedTask.ID, tasks[0].ID)
200+
require.NotEqual(t, initializingTask.ID, tasks[0].ID)
201201
})
202202

203203
t.Run("UserFlag_Me_Table", func(t *testing.T) {
@@ -234,7 +234,7 @@ func TestExpTaskList(t *testing.T) {
234234
memberClient, memberUser := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
235235

236236
// Given: We have two tasks
237-
task1 := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStart, "keep me running")
237+
task1 := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStart, "keep me active")
238238
task2 := makeAITask(t, db, owner.OrganizationID, owner.UserID, memberUser.ID, database.WorkspaceTransitionStop, "stop me please")
239239

240240
// Given: We add the `--quiet` flag

‎cli/exp_task_status.go‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -156,28 +156,21 @@ func taskWatchIsEnded(task codersdk.Task) bool {
156156
}
157157

158158
type taskStatusRow struct {
159-
codersdk.Task `table:"-"`
160-
ChangedAgo string `json:"-" table:"state changed,default_sort"`
161-
Timestamp time.Time `json:"-" table:"-"`
162-
TaskStatus string `json:"-" table:"status"`
163-
Healthy bool `json:"-" table:"healthy"`
164-
TaskState string `json:"-" table:"state"`
165-
Message string `json:"-" table:"message"`
159+
codersdk.Task `table:"r,recursive_inline"`
160+
ChangedAgo string `json:"-" table:"state changed"`
161+
Healthy bool `json:"-" table:"healthy"`
166162
}
167163

168164
func taskStatusRowEqual(r1, r2 taskStatusRow) bool {
169-
return r1.TaskStatus == r2.TaskStatus &&
165+
return r1.Status == r2.Status &&
170166
r1.Healthy == r2.Healthy &&
171-
r1.TaskState == r2.TaskState &&
172-
r1.Message == r2.Message
167+
taskStateEqual(r1.CurrentState, r2.CurrentState)
173168
}
174169

175170
func toStatusRow(task codersdk.Task) taskStatusRow {
176171
tsr := taskStatusRow{
177172
Task: task,
178173
ChangedAgo: time.Since(task.UpdatedAt).Truncate(time.Second).String() + " ago",
179-
Timestamp: task.UpdatedAt,
180-
TaskStatus: string(task.WorkspaceStatus),
181174
}
182175
tsr.Healthy = task.WorkspaceAgentHealth != nil &&
183176
task.WorkspaceAgentHealth.Healthy &&
@@ -187,9 +180,19 @@ func toStatusRow(task codersdk.Task) taskStatusRow {
187180

188181
if task.CurrentState != nil {
189182
tsr.ChangedAgo = time.Since(task.CurrentState.Timestamp).Truncate(time.Second).String() + " ago"
190-
tsr.Timestamp = task.CurrentState.Timestamp
191-
tsr.TaskState = string(task.CurrentState.State)
192-
tsr.Message = task.CurrentState.Message
193183
}
194184
return tsr
195185
}
186+
187+
func taskStateEqual(se1, se2 *codersdk.TaskStateEntry) bool {
188+
var s1, m1, s2, m2 string
189+
if se1 != nil {
190+
s1 = string(se1.State)
191+
m1 = se1.Message
192+
}
193+
if se2 != nil {
194+
s2 = string(se2.State)
195+
m2 = se2.Message
196+
}
197+
return s1 == s2 && m1 == m2
198+
}

‎cli/exp_task_status_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,8 @@ func Test_TaskStatus(t *testing.T) {
5555
},
5656
{
5757
args: []string{"exists"},
58-
expectOutput: `STATE CHANGED STATUS HEALTHY STATE MESSAGE
59-
0s ago running true working Thinking furiously...`,
58+
expectOutput: `STATE CHANGED STATUS HEALTHY STATE MESSAGE
59+
0s ago active true working Thinking furiously...`,
6060
hf: func(ctx context.Context, now time.Time) func(w http.ResponseWriter, r *http.Request) {
6161
return func(w http.ResponseWriter, r *http.Request) {
6262
switch r.URL.Path {
@@ -114,12 +114,12 @@ func Test_TaskStatus(t *testing.T) {
114114
},
115115
{
116116
args: []string{"exists", "--watch"},
117-
expectOutput: `
118-
STATE CHANGED STATUS HEALTHY STATE MESSAGE
117+
expectOutput: `STATE CHANGED STATUS HEALTHY STATE MESSAGE
119118
5s ago pending true
120-
4s ago running true
121-
3s ago running true working Reticulating splines...
122-
2s ago running true complete Splines reticulated successfully!`,
119+
4s ago initializing true
120+
4s ago active true
121+
3s ago active true working Reticulating splines...
122+
2s ago active true complete Splines reticulated successfully!`,
123123
hf: func(ctx context.Context, now time.Time) func(http.ResponseWriter, *http.Request) {
124124
var calls atomic.Int64
125125
return func(w http.ResponseWriter, r *http.Request) {

‎cli/exp_task_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,7 @@ func Test_Tasks(t *testing.T) {
9494
var task codersdk.Task
9595
require.NoError(t, json.NewDecoder(strings.NewReader(stdout)).Decode(&task), "should unmarshal task status")
9696
require.Equal(t, task.Name, taskName, "task name should match")
97-
// NOTE: task status changes type, this is so this test works with both old and new model
98-
require.Contains(t, []string{"active", "running"}, string(task.Status), "task should be active")
97+
require.Equal(t, codersdk.TaskStatusActive, task.Status, "task should be active")
9998
},
10099
},
101100
{

‎codersdk/aitasks.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,15 +163,15 @@ type Task struct {
163163
TemplateDisplayName string `json:"template_display_name" table:"template display name"`
164164
TemplateIcon string `json:"template_icon" table:"template icon"`
165165
WorkspaceID uuid.NullUUID `json:"workspace_id" format:"uuid" table:"workspace id"`
166-
WorkspaceStatus WorkspaceStatus `json:"workspace_status,omitempty" enums:"pending,starting,running,stopping,stopped,failed,canceling,canceled,deleting,deleted" table:"status"`
166+
WorkspaceStatus WorkspaceStatus `json:"workspace_status,omitempty" enums:"pending,starting,running,stopping,stopped,failed,canceling,canceled,deleting,deleted" table:"workspace status"`
167167
WorkspaceBuildNumber int32 `json:"workspace_build_number,omitempty" table:"workspace build number"`
168168
WorkspaceAgentID uuid.NullUUID `json:"workspace_agent_id" format:"uuid" table:"workspace agent id"`
169169
WorkspaceAgentLifecycle *WorkspaceAgentLifecycle `json:"workspace_agent_lifecycle" table:"workspace agent lifecycle"`
170170
WorkspaceAgentHealth *WorkspaceAgentHealth `json:"workspace_agent_health" table:"workspace agent health"`
171171
WorkspaceAppID uuid.NullUUID `json:"workspace_app_id" format:"uuid" table:"workspace app id"`
172172
InitialPrompt string `json:"initial_prompt" table:"initial prompt"`
173-
Status TaskStatus `json:"status" enums:"pending,initializing,active,paused,unknown,error" table:"task status"`
174-
CurrentState *TaskStateEntry `json:"current_state" table:"cs,recursive_inline"`
173+
Status TaskStatus `json:"status" enums:"pending,initializing,active,paused,unknown,error" table:"status"`
174+
CurrentState *TaskStateEntry `json:"current_state" table:"cs,recursive_inline,empty_nil"`
175175
CreatedAt time.Time `json:"created_at" format:"date-time" table:"created at"`
176176
UpdatedAt time.Time `json:"updated_at" format:"date-time" table:"updated at"`
177177
}

0 commit comments

Comments
 (0)