Skip to content

Commit 451180c

Browse files
ethanndicksonethanndickson
committed
WIP
1 parent fa5e1c0 commit 451180c

File tree

5 files changed

+98
-33
lines changed

5 files changed

+98
-33
lines changed

‎cli/cliui/output.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"reflect"
88
"strings"
99

10+
"github.com/jedib0t/go-pretty/v6/table"
1011
"golang.org/x/xerrors"
1112

1213
"github.com/coder/serpent"
@@ -143,7 +144,11 @@ func (f *tableFormat) AttachOptions(opts *serpent.OptionSet) {
143144

144145
// Format implements OutputFormat.
145146
func (f *tableFormat) Format(_ context.Context, data any) (string, error) {
146-
return renderTable(data, f.sort, f.columns, nil)
147+
headers := make(table.Row, len(f.allColumns))
148+
for i, header := range f.allColumns {
149+
headers[i] = header
150+
}
151+
return renderTable(data, f.sort, headers, f.columns)
147152
}
148153

149154
type jsonFormat struct{}

‎cli/cliui/table.go

+27-20
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func filterTableColumns(header table.Row, columns []string) []table.ColumnConfig
5757
// DisplayTable renders a table as a string. The input argument can be:
5858
// - a struct slice.
5959
// - an interface slice, where the first element is a struct,
60-
// and all other elements are of the same type, or a TableSeperator.
60+
// and all other elements are of the same type, or a TableSeparator.
6161
//
6262
// At least one field in the struct must have a `table:""` tag
6363
// containing the name of the column in the outputted table.
@@ -77,12 +77,12 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error)
7777
v := reflect.Indirect(reflect.ValueOf(out))
7878

7979
if v.Kind() != reflect.Slice {
80-
return "", xerrors.Errorf("DisplayTable called with a non-slice type")
80+
return "", xerrors.New("DisplayTable called with a non-slice type")
8181
}
8282
var tableType reflect.Type
8383
if v.Type().Elem().Kind() == reflect.Interface {
8484
if v.Len() == 0 {
85-
return "", xerrors.Errorf("DisplayTable called with empty interface slice")
85+
return "", xerrors.New("DisplayTable called with empty interface slice")
8686
}
8787
tableType = reflect.Indirect(reflect.ValueOf(v.Index(0).Interface())).Type()
8888
} else {
@@ -100,6 +100,10 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error)
100100
if sort == "" {
101101
sort = defaultSort
102102
}
103+
headers := make(table.Row, len(headersRaw))
104+
for i, header := range headersRaw {
105+
headers[i] = strings.ReplaceAll(header, "_", " ")
106+
}
103107
// Verify that the given sort column and filter columns are valid.
104108
if sort != "" || len(filterColumns) != 0 {
105109
headersMap := make(map[string]string, len(headersRaw))
@@ -145,16 +149,12 @@ func DisplayTable(out any, sort string, filterColumns []string) (string, error)
145149
return "", xerrors.Errorf("specified sort column %q not found in table headers, available columns are %q", sort, strings.Join(headersRaw, `", "`))
146150
}
147151
}
148-
return renderTable(out, sort, headersRaw, filterColumns)
152+
return renderTable(out, sort, headers, filterColumns)
149153
}
150154

151-
func renderTable(out any, sort string, headersRaw []string, filterColumns []string) (string, error) {
155+
func renderTable(out any, sort string, headers table.Row, filterColumns []string) (string, error) {
152156
v := reflect.Indirect(reflect.ValueOf(out))
153157

154-
headers := make(table.Row, len(headersRaw))
155-
for i, header := range headersRaw {
156-
headers[i] = header
157-
}
158158
// Setup the table formatter.
159159
tw := Table()
160160
tw.AppendHeader(headers)
@@ -181,8 +181,8 @@ func renderTable(out any, sort string, headersRaw []string, filterColumns []stri
181181
}
182182

183183
rowSlice := make([]any, len(headers))
184-
for i, h := range headersRaw {
185-
v, ok := rowMap[h]
184+
for i, h := range headers {
185+
v, ok := rowMap[h.(string)]
186186
if !ok {
187187
v = nil
188188
}
@@ -219,25 +219,28 @@ func renderTable(out any, sort string, headersRaw []string, filterColumns []stri
219219
// returned. If the table tag is malformed, an error is returned.
220220
//
221221
// The returned name is transformed from "snake_case" to "normal text".
222-
func parseTableStructTag(field reflect.StructField) (name string, defaultSort, recursive bool, skipParentName bool, err error) {
222+
func parseTableStructTag(field reflect.StructField) (name string, defaultSort, noSortOpt, recursive, skipParentName bool, err error) {
223223
tags, err := structtag.Parse(string(field.Tag))
224224
if err != nil {
225-
return "", false, false, false, xerrors.Errorf("parse struct field tag %q: %w", string(field.Tag), err)
225+
return "", false, false, false, false, xerrors.Errorf("parse struct field tag %q: %w", string(field.Tag), err)
226226
}
227227

228228
tag, err := tags.Get("table")
229229
if err != nil || tag.Name == "-" {
230230
// tags.Get only returns an error if the tag is not found.
231-
return "", false, false, false, nil
231+
return "", false, false, false, false, nil
232232
}
233233

234234
defaultSortOpt := false
235+
noSortOpt = false
235236
recursiveOpt := false
236237
skipParentNameOpt := false
237238
for _, opt := range tag.Options {
238239
switch opt {
239240
case "default_sort":
240241
defaultSortOpt = true
242+
case "nosort":
243+
noSortOpt = true
241244
case "recursive":
242245
recursiveOpt = true
243246
case "recursive_inline":
@@ -247,11 +250,11 @@ func parseTableStructTag(field reflect.StructField) (name string, defaultSort, r
247250
recursiveOpt = true
248251
skipParentNameOpt = true
249252
default:
250-
return "", false, false, false, xerrors.Errorf("unknown option %q in struct field tag", opt)
253+
return "", false, false, false, false, xerrors.Errorf("unknown option %q in struct field tag", opt)
251254
}
252255
}
253256

254-
return strings.ReplaceAll(tag.Name, "_", " "), defaultSortOpt, recursiveOpt, skipParentNameOpt, nil
257+
return strings.ReplaceAll(tag.Name, "_", " "), defaultSortOpt, noSortOpt, recursiveOpt, skipParentNameOpt, nil
255258
}
256259

257260
func isStructOrStructPointer(t reflect.Type) bool {
@@ -275,12 +278,16 @@ func typeToTableHeaders(t reflect.Type, requireDefault bool) ([]string, string,
275278

276279
headers := []string{}
277280
defaultSortName := ""
281+
noSortOpt := false
278282
for i := 0; i < t.NumField(); i++ {
279283
field := t.Field(i)
280-
name, defaultSort, recursive, skip, err := parseTableStructTag(field)
284+
name, defaultSort, noSort, recursive, skip, err := parseTableStructTag(field)
281285
if err != nil {
282286
return nil, "", xerrors.Errorf("parse struct tags for field %q in type %q: %w", field.Name, t.String(), err)
283287
}
288+
if requireDefault && noSort {
289+
noSortOpt = true
290+
}
284291

285292
if name == "" && (recursive && skip) {
286293
return nil, "", xerrors.Errorf("a name is required for the field %q. "+
@@ -323,8 +330,8 @@ func typeToTableHeaders(t reflect.Type, requireDefault bool) ([]string, string,
323330
headers = append(headers, name)
324331
}
325332

326-
if defaultSortName == "" && requireDefault {
327-
return nil, "", xerrors.Errorf("no field marked as default_sort in type %q", t.String())
333+
if defaultSortName == "" && requireDefault && !noSortOpt {
334+
return nil, "", xerrors.Errorf("no field marked as default_sort or nosort in type %q", t.String())
328335
}
329336

330337
return headers, defaultSortName, nil
@@ -351,7 +358,7 @@ func valueToTableMap(val reflect.Value) (map[string]any, error) {
351358
for i := 0; i < val.NumField(); i++ {
352359
field := val.Type().Field(i)
353360
fieldVal := val.Field(i)
354-
name, _, recursive, skip, err := parseTableStructTag(field)
361+
name, _, _, recursive, skip, err := parseTableStructTag(field)
355362
if err != nil {
356363
return nil, xerrors.Errorf("parse struct tags for field %q in type %T: %w", field.Name, val, err)
357364
}

‎cli/speedtest.go

+13-12
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,19 @@ import (
1818
"github.com/coder/serpent"
1919
)
2020

21-
type speedtestResult struct {
22-
Overall speedtestResultInterval `json:"overall"`
23-
Intervals []speedtestResultInterval `json:"intervals"`
21+
type SpeedtestResult struct {
22+
Overall SpeedtestResultInterval `json:"overall"`
23+
Intervals []SpeedtestResultInterval `json:"intervals"`
2424
}
2525

26-
type speedtestResultInterval struct {
26+
type SpeedtestResultInterval struct {
2727
StartTimeSeconds float64 `json:"start_time_seconds"`
2828
EndTimeSeconds float64 `json:"end_time_seconds"`
2929
ThroughputMbits float64 `json:"throughput_mbits"`
3030
}
3131

3232
type speedtestTableItem struct {
33-
Interval string `table:"Interval,default_sort"`
33+
Interval string `table:"Interval,nosort"`
3434
Throughput string `table:"Throughput"`
3535
}
3636

@@ -42,7 +42,7 @@ func (r *RootCmd) speedtest() *serpent.Command {
4242
pcapFile string
4343
formatter = cliui.NewOutputFormatter(
4444
cliui.ChangeFormatterData(cliui.TableFormat([]speedtestTableItem{}, []string{"Interval", "Throughput"}), func(data any) (any, error) {
45-
res, ok := data.(speedtestResult)
45+
res, ok := data.(SpeedtestResult)
4646
if !ok {
4747
// This should never happen
4848
return "", xerrors.Errorf("expected speedtestResult, got %T", data)
@@ -56,7 +56,7 @@ func (r *RootCmd) speedtest() *serpent.Command {
5656
}
5757
tableRows[len(res.Intervals)] = cliui.TableSeparator{}
5858
tableRows[len(res.Intervals)+1] = speedtestTableItem{
59-
Interval: "Total",
59+
Interval: fmt.Sprintf("%.2f-%.2f sec", res.Overall.StartTimeSeconds, res.Overall.EndTimeSeconds),
6060
Throughput: fmt.Sprintf("%.4f Mbits/sec", res.Overall.ThroughputMbits),
6161
}
6262
return tableRows, nil
@@ -167,19 +167,20 @@ func (r *RootCmd) speedtest() *serpent.Command {
167167
if err != nil {
168168
return err
169169
}
170-
var outputResult speedtestResult
170+
var outputResult SpeedtestResult
171171
startTime := results[0].IntervalStart
172-
outputResult.Intervals = make([]speedtestResultInterval, len(results)-1)
172+
outputResult.Intervals = make([]SpeedtestResultInterval, len(results)-1)
173173
for i, r := range results {
174-
tmp := speedtestResultInterval{
174+
interval := SpeedtestResultInterval{
175175
StartTimeSeconds: r.IntervalStart.Sub(startTime).Seconds(),
176176
EndTimeSeconds: r.IntervalEnd.Sub(startTime).Seconds(),
177177
ThroughputMbits: r.MBitsPerSecond(),
178178
}
179179
if r.Total {
180-
outputResult.Overall = tmp
180+
interval.StartTimeSeconds = 0
181+
outputResult.Overall = interval
181182
} else {
182-
outputResult.Intervals[i] = tmp
183+
outputResult.Intervals[i] = interval
183184
}
184185
}
185186
out, err := formatter.Format(inv.Context(), outputResult)

‎cli/speedtest_test.go

+45
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package cli_test
22

33
import (
4+
"bytes"
45
"context"
6+
"encoding/json"
57
"testing"
68

79
"github.com/stretchr/testify/assert"
@@ -10,6 +12,7 @@ import (
1012
"cdr.dev/slog"
1113
"cdr.dev/slog/sloggers/slogtest"
1214
"github.com/coder/coder/v2/agent/agenttest"
15+
"github.com/coder/coder/v2/cli"
1316
"github.com/coder/coder/v2/cli/clitest"
1417
"github.com/coder/coder/v2/coderd/coderdtest"
1518
"github.com/coder/coder/v2/codersdk"
@@ -56,3 +59,45 @@ func TestSpeedtest(t *testing.T) {
5659
})
5760
<-cmdDone
5861
}
62+
63+
func TestSpeedtestJson(t *testing.T) {
64+
t.Parallel()
65+
t.Skip("Potentially flaky test - see https://github.com/coder/coder/issues/6321")
66+
if testing.Short() {
67+
t.Skip("This test takes a minimum of 5ms per a hardcoded value in Tailscale!")
68+
}
69+
client, workspace, agentToken := setupWorkspaceForAgent(t)
70+
_ = agenttest.New(t, client.URL, agentToken)
71+
coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
72+
73+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
74+
defer cancel()
75+
76+
require.Eventually(t, func() bool {
77+
ws, err := client.Workspace(ctx, workspace.ID)
78+
if !assert.NoError(t, err) {
79+
return false
80+
}
81+
a := ws.LatestBuild.Resources[0].Agents[0]
82+
return a.Status == codersdk.WorkspaceAgentConnected &&
83+
a.LifecycleState == codersdk.WorkspaceAgentLifecycleReady
84+
}, testutil.WaitLong, testutil.IntervalFast, "agent is not ready")
85+
86+
inv, root := clitest.New(t, "speedtest", "--output=json", workspace.Name)
87+
clitest.SetupConfig(t, client, root)
88+
out := bytes.NewBuffer(nil)
89+
inv.Stdout = out
90+
ctx, cancel = context.WithTimeout(context.Background(), testutil.WaitLong)
91+
defer cancel()
92+
93+
inv.Logger = slogtest.Make(t, nil).Named("speedtest").Leveled(slog.LevelDebug)
94+
cmdDone := tGo(t, func() {
95+
err := inv.WithContext(ctx).Run()
96+
assert.NoError(t, err)
97+
})
98+
<-cmdDone
99+
100+
var result cli.SpeedtestResult
101+
require.NoError(t, json.Unmarshal(out.Bytes(), &result))
102+
require.Len(t, result.Intervals, 5)
103+
}

‎cli/testdata/coder_speedtest_--help.golden

+7
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ USAGE:
66
Run upload and download tests from your machine to a workspace
77

88
OPTIONS:
9+
-c, --column string-array (default: Interval,Throughput)
10+
Columns to display in table output. Available columns: Interval,
11+
Throughput.
12+
913
-d, --direct bool
1014
Specifies whether to wait for a direct connection before testing
1115
speed.
@@ -14,6 +18,9 @@ OPTIONS:
1418
Specifies whether to run in reverse mode where the client receives and
1519
the server sends.
1620

21+
-o, --output string (default: table)
22+
Output format. Available formats: table, json.
23+
1724
--pcap-file string
1825
Specifies a file to write a network capture to.
1926

0 commit comments

Comments
 (0)