From d4d02be72f0123706956579b92948dd697231582 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 18 Sep 2026 09:10:07 +0200 Subject: [PATCH 1/3] fix(table output): sort tags so table output is deterministic The flow and environment table printers rendered tags by iterating the tags map directly, so the order of the rendered pairs changed between runs. Against a flow tagged app, env and team, `kosli get flow` returned three distinct orderings across 20 runs. Tag rendering now goes through one sorted helper, shared by the six places that had their own copy: get flow, get environment, list flows, list environments, get repo and get control. Both existing output formats are preserved, so ordering is the only user-visible change: bracketed pairs for the flow and environment tables, unbracketed for repo and control. The helper also guards a missing or non-map tags value, which get environment asserted unchecked, and formats values with %v rather than %s. Neither is reachable through the API as it behaves today, which sends "tags": {} for an untagged resource and string values for tags set via `kosli tag`; both are kept because the sibling printers already guarded this way. Every existing assertion on tag output used a single tag or none, which is why the ordering was never caught. The new tests use three keys. --- cmd/kosli/getControl.go | 11 +--- cmd/kosli/getEnvironment.go | 8 +-- cmd/kosli/getFlow.go | 9 +-- cmd/kosli/getRepo.go | 20 +----- cmd/kosli/listEnvironments.go | 9 +-- cmd/kosli/listFlows.go | 8 +-- cmd/kosli/tableHelpers.go | 41 ++++++++++++ cmd/kosli/tableHelpers_test.go | 111 +++++++++++++++++++++++++++++++++ 8 files changed, 160 insertions(+), 57 deletions(-) create mode 100644 cmd/kosli/tableHelpers.go create mode 100644 cmd/kosli/tableHelpers_test.go diff --git a/cmd/kosli/getControl.go b/cmd/kosli/getControl.go index bce6f47c2..17a8bb340 100644 --- a/cmd/kosli/getControl.go +++ b/cmd/kosli/getControl.go @@ -105,16 +105,7 @@ func printControlAsTable(raw string, out io.Writer, page int) error { rows = append(rows, fmt.Sprintf("Created at:\t%s", createdAtFormatted)) } - if tags, ok := control["tags"].(map[string]any); ok && len(tags) > 0 { - tagKeys := make([]string, 0, len(tags)) - for key := range tags { - tagKeys = append(tagKeys, key) - } - sort.Strings(tagKeys) - tagPairs := make([]string, 0, len(tags)) - for _, key := range tagKeys { - tagPairs = append(tagPairs, fmt.Sprintf("%s=%s", key, tags[key])) - } + if tagPairs := sortedTagPairs(control["tags"]); len(tagPairs) > 0 { rows = append(rows, fmt.Sprintf("Tags:\t%s", strings.Join(tagPairs, ", "))) } diff --git a/cmd/kosli/getEnvironment.go b/cmd/kosli/getEnvironment.go index b381600d7..ef37d0686 100644 --- a/cmd/kosli/getEnvironment.go +++ b/cmd/kosli/getEnvironment.go @@ -6,7 +6,6 @@ import ( "io" "net/http" "net/url" - "strings" "github.com/kosli-dev/cli/internal/output" "github.com/kosli-dev/cli/internal/requests" @@ -86,12 +85,7 @@ func printEnvironmentAsTable(raw string, out io.Writer, page int) error { state = "NON-COMPLIANT" } - tags := env["tags"].(map[string]any) - tagsOutput := "" - for key, value := range tags { - tagsOutput += fmt.Sprintf("[%s=%s], ", key, value) - } - tagsOutput = strings.TrimSuffix(tagsOutput, ", ") + tagsOutput := formatTags(env["tags"]) if tagsOutput == "" { tagsOutput = "None" } diff --git a/cmd/kosli/getFlow.go b/cmd/kosli/getFlow.go index a8e5867ae..7f78fe752 100644 --- a/cmd/kosli/getFlow.go +++ b/cmd/kosli/getFlow.go @@ -93,14 +93,7 @@ func printFlowAsTable(raw string, out io.Writer, page int) error { template = strings.ReplaceAll(template, " ", ", ") } - tagsOutput := "" - if flow["tags"] != nil { - tags := flow["tags"].(map[string]any) - for key, value := range tags { - tagsOutput += fmt.Sprintf("[%s=%s], ", key, value) - } - } - tagsOutput = strings.TrimSuffix(tagsOutput, ", ") + tagsOutput := formatTags(flow["tags"]) if tagsOutput == "" { tagsOutput = "None" } diff --git a/cmd/kosli/getRepo.go b/cmd/kosli/getRepo.go index 5d993b081..d82de00a1 100644 --- a/cmd/kosli/getRepo.go +++ b/cmd/kosli/getRepo.go @@ -6,7 +6,6 @@ import ( "io" "net/http" neturl "net/url" - "sort" "strings" "github.com/kosli-dev/cli/internal/output" @@ -180,21 +179,8 @@ func printRepoAsTable(raw string, out io.Writer, page int) error { } // formatRepoTags renders a repo's tags map as sorted "key=value" pairs, or "" -// when there are no tags. Following the flow/env table conventions, list -// columns show the empty string while the get repo detail view shows "None". +// when there are no tags. Repo and control tables use unbracketed pairs, unlike +// the flow and environment tables, which use formatTags. func formatRepoTags(rawTags any) string { - tags, ok := rawTags.(map[string]any) - if !ok || len(tags) == 0 { - return "" - } - keys := make([]string, 0, len(tags)) - for key := range tags { - keys = append(keys, key) - } - sort.Strings(keys) - pairs := make([]string, 0, len(tags)) - for _, key := range keys { - pairs = append(pairs, fmt.Sprintf("%s=%v", key, tags[key])) - } - return strings.Join(pairs, ", ") + return strings.Join(sortedTagPairs(rawTags), ", ") } diff --git a/cmd/kosli/listEnvironments.go b/cmd/kosli/listEnvironments.go index f2b1350f2..bc4b6a024 100644 --- a/cmd/kosli/listEnvironments.go +++ b/cmd/kosli/listEnvironments.go @@ -7,7 +7,6 @@ import ( "net/http" "net/url" "strconv" - "strings" "time" "github.com/kosli-dev/cli/internal/output" @@ -202,13 +201,7 @@ func printEnvListAsTable(raw string, out io.Writer, page int) error { last_modified_str = time.Unix(int64(last_modified_at.(float64)), 0).Format(time.RFC3339) } - tagsOutput := "" - if tags, ok := env["tags"].(map[string]any); ok { - for key, value := range tags { - tagsOutput += fmt.Sprintf("[%s=%s], ", key, value) - } - tagsOutput = strings.TrimSuffix(tagsOutput, ", ") - } + tagsOutput := formatTags(env["tags"]) var policies []any if env["policies"] != nil { diff --git a/cmd/kosli/listFlows.go b/cmd/kosli/listFlows.go index e85d7796c..1800bc105 100644 --- a/cmd/kosli/listFlows.go +++ b/cmd/kosli/listFlows.go @@ -164,13 +164,7 @@ func printFlowsListAsTable(raw string, out io.Writer, page int) error { header := []string{"NAME", "DESCRIPTION", "TAGS"} rows := []string{} for _, flow := range flows { - tagsOutput := "" - if tags, ok := flow["tags"].(map[string]any); ok { - for key, value := range tags { - tagsOutput += fmt.Sprintf("[%s=%s], ", key, value) - } - } - tagsOutput = strings.TrimSuffix(tagsOutput, ", ") + tagsOutput := formatTags(flow["tags"]) row := fmt.Sprintf("%s\t%s\t%s", flow["name"], flow["description"], tagsOutput) rows = append(rows, row) } diff --git a/cmd/kosli/tableHelpers.go b/cmd/kosli/tableHelpers.go new file mode 100644 index 000000000..26dcfb821 --- /dev/null +++ b/cmd/kosli/tableHelpers.go @@ -0,0 +1,41 @@ +package main + +import ( + "fmt" + "slices" + "strings" +) + +// sortedTagPairs renders a tags map as "key=value" pairs ordered by key. The +// order matters because tags reach the printers as a map, whose iteration order +// would otherwise leak into the table output. A missing or non-map value yields +// no pairs, since the API omits "tags" entirely when there are none. +func sortedTagPairs(rawTags any) []string { + tags, ok := rawTags.(map[string]any) + if !ok || len(tags) == 0 { + return nil + } + keys := make([]string, 0, len(tags)) + for key := range tags { + keys = append(keys, key) + } + slices.Sort(keys) + pairs := make([]string, 0, len(tags)) + for _, key := range keys { + pairs = append(pairs, fmt.Sprintf("%s=%v", key, tags[key])) + } + return pairs +} + +// formatTags renders a tags map as "[key=value], [key=value]" ordered by key, +// or "" when there are no tags. Detail views substitute "None" for the empty +// string; list columns leave it blank. Repo and control tables use the +// unbracketed pairs from sortedTagPairs instead. +func formatTags(rawTags any) string { + pairs := sortedTagPairs(rawTags) + bracketed := make([]string, 0, len(pairs)) + for _, pair := range pairs { + bracketed = append(bracketed, "["+pair+"]") + } + return strings.Join(bracketed, ", ") +} diff --git a/cmd/kosli/tableHelpers_test.go b/cmd/kosli/tableHelpers_test.go new file mode 100644 index 000000000..7437be3bd --- /dev/null +++ b/cmd/kosli/tableHelpers_test.go @@ -0,0 +1,111 @@ +package main + +import ( + "bytes" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSortedTagPairs(t *testing.T) { + for _, tc := range []struct { + name string + tags any + want []string + }{ + { + name: "pairs are ordered by key, not by map iteration order", + tags: map[string]any{"team": "platform", "env": "prod", "app": "api"}, + want: []string{"app=api", "env=prod", "team=platform"}, + }, + { + name: "non-string values are rendered as values, not as %!s verbs", + tags: map[string]any{"replicas": float64(3), "critical": true}, + want: []string{"critical=true", "replicas=3"}, + }, + { + name: "nil tags yield no pairs", + tags: nil, + want: nil, + }, + { + name: "a non-map yields no pairs", + tags: "team=platform", + want: nil, + }, + { + name: "an empty map yields no pairs", + tags: map[string]any{}, + want: nil, + }, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, sortedTagPairs(tc.tags)) + }) + } +} + +func TestFormatTags(t *testing.T) { + for _, tc := range []struct { + name string + tags any + want string + }{ + { + name: "tags are bracketed, comma separated and ordered by key", + tags: map[string]any{"team": "platform", "env": "prod"}, + want: "[env=prod], [team=platform]", + }, + { + name: "no tags yields the empty string", + tags: nil, + want: "", + }, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, formatTags(tc.tags)) + }) + } +} + +// The table printers receive tags as a map, so without an explicit sort their +// output ordering was whatever Go's map iteration gave that run. +func TestTagRenderingIsDeterministicAcrossPrinters(t *testing.T) { + t.Run("printFlowAsTable orders tags by key", func(t *testing.T) { + raw := `{"name":"backend","description":"Backend service","template":"artifact","last_deployment_at":null,"tags":{"team":"platform","env":"prod","app":"api"}}` + var buf bytes.Buffer + require.NoError(t, printFlowAsTable(raw, &buf, 0)) + require.Contains(t, buf.String(), "Tags: [app=api], [env=prod], [team=platform]\n") + }) + + t.Run("printEnvironmentAsTable orders tags by key", func(t *testing.T) { + raw := `{"name":"prod","type":"K8S","description":"","state":true,"last_reported_at":null,"tags":{"team":"platform","env":"prod","app":"api"}}` + var buf bytes.Buffer + require.NoError(t, printEnvironmentAsTable(raw, &buf, 0)) + require.Contains(t, buf.String(), "[app=api], [env=prod], [team=platform]") + }) + + t.Run("printFlowsListAsTable orders tags by key", func(t *testing.T) { + raw := `[{"name":"backend","description":"Backend service","tags":{"team":"platform","env":"prod","app":"api"}}]` + var buf bytes.Buffer + require.NoError(t, printFlowsListAsTable(raw, &buf, 1)) + require.Contains(t, buf.String(), "[app=api], [env=prod], [team=platform]") + }) + + t.Run("printEnvListAsTable orders tags by key", func(t *testing.T) { + raw := `[{"name":"prod","type":"K8S","last_reported_at":null,"last_modified_at":null,"tags":{"team":"platform","env":"prod","app":"api"}}]` + var buf bytes.Buffer + require.NoError(t, printEnvListAsTable(raw, &buf, 1)) + require.Contains(t, buf.String(), "[app=api], [env=prod], [team=platform]") + }) +} + +// The API omits "tags" entirely for an environment without tags, which used to +// panic on an unchecked type assertion. +func TestPrintEnvironmentAsTableWithoutTags(t *testing.T) { + raw := `{"name":"prod","type":"K8S","description":"","state":true,"last_reported_at":null}` + var buf bytes.Buffer + require.NoError(t, printEnvironmentAsTable(raw, &buf, 0)) + require.Contains(t, buf.String(), "Tags:") + require.Contains(t, buf.String(), "None") +} From 1a1dbdea6d0254c4a431c9c0c62c257f57fc1e6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 18 Sep 2026 09:26:07 +0200 Subject: [PATCH 2/3] refactor(table output): one tag helper per output format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on #1207. formatPlainTags replaces both formatRepoTags and the inline strings.Join in get control, so the unbracketed format has one implementation instead of two plus a wrapper named after one of its callers. sortedTagPairs is now reached only through the two formatters. The sortedTagPairs doc no longer claims the API omits "tags" when there are none — it sends an empty object, as the PR description says — so the guard is described as defensive instead. The same wrong claim is corrected in the test. TestTagRenderingIsDeterministicAcrossPrinters becomes TestTagRenderingIsSortedAcrossPrinters: each subtest renders once, so what it asserts is sorted order, not determinism. All four now pin the full rendering, tab padding included, so a column-width change cannot pass silently. TestFormatRepoTags moves to tableHelpers_test.go as TestFormatPlainTags. --- cmd/kosli/getControl.go | 4 +-- cmd/kosli/getRepo.go | 10 +----- cmd/kosli/getRepo_test.go | 7 ---- cmd/kosli/listRepos.go | 2 +- cmd/kosli/tableHelpers.go | 12 +++++-- cmd/kosli/tableHelpers_test.go | 60 ++++++++++++++++++++++++---------- 6 files changed, 55 insertions(+), 40 deletions(-) diff --git a/cmd/kosli/getControl.go b/cmd/kosli/getControl.go index 17a8bb340..8c00ee610 100644 --- a/cmd/kosli/getControl.go +++ b/cmd/kosli/getControl.go @@ -105,8 +105,8 @@ func printControlAsTable(raw string, out io.Writer, page int) error { rows = append(rows, fmt.Sprintf("Created at:\t%s", createdAtFormatted)) } - if tagPairs := sortedTagPairs(control["tags"]); len(tagPairs) > 0 { - rows = append(rows, fmt.Sprintf("Tags:\t%s", strings.Join(tagPairs, ", "))) + if tagsOutput := formatPlainTags(control["tags"]); tagsOutput != "" { + rows = append(rows, fmt.Sprintf("Tags:\t%s", tagsOutput)) } if links, ok := control["links"].(map[string]any); ok && len(links) > 0 { diff --git a/cmd/kosli/getRepo.go b/cmd/kosli/getRepo.go index d82de00a1..fc1a10172 100644 --- a/cmd/kosli/getRepo.go +++ b/cmd/kosli/getRepo.go @@ -6,7 +6,6 @@ import ( "io" "net/http" neturl "net/url" - "strings" "github.com/kosli-dev/cli/internal/output" "github.com/kosli-dev/cli/internal/requests" @@ -162,7 +161,7 @@ func printRepoAsTable(raw string, out io.Writer, page int) error { return err } - tagsOutput := formatRepoTags(repo["tags"]) + tagsOutput := formatPlainTags(repo["tags"]) if tagsOutput == "" { tagsOutput = "None" } @@ -177,10 +176,3 @@ func printRepoAsTable(raw string, out io.Writer, page int) error { tabFormattedPrint(out, []string{}, rows) return nil } - -// formatRepoTags renders a repo's tags map as sorted "key=value" pairs, or "" -// when there are no tags. Repo and control tables use unbracketed pairs, unlike -// the flow and environment tables, which use formatTags. -func formatRepoTags(rawTags any) string { - return strings.Join(sortedTagPairs(rawTags), ", ") -} diff --git a/cmd/kosli/getRepo_test.go b/cmd/kosli/getRepo_test.go index 9d5ff20a4..783cb6e54 100644 --- a/cmd/kosli/getRepo_test.go +++ b/cmd/kosli/getRepo_test.go @@ -170,13 +170,6 @@ func TestGetRepoCommandTestSuite(t *testing.T) { suite.Run(t, new(GetRepoCommandTestSuite)) } -func TestFormatRepoTags(t *testing.T) { - require.Equal(t, "", formatRepoTags(nil)) - require.Equal(t, "", formatRepoTags(map[string]any{})) - require.Equal(t, "", formatRepoTags("not-a-map")) - require.Equal(t, "a=1, b=x", formatRepoTags(map[string]any{"b": "x", "a": float64(1)})) -} - func TestPrintRepoAsTableRendersNonStringValues(t *testing.T) { // ids and tag values are rendered with %v so a numeric id from the // server prints as a number instead of a %!s(float64=...) artifact diff --git a/cmd/kosli/listRepos.go b/cmd/kosli/listRepos.go index 21a35114c..99a757c78 100644 --- a/cmd/kosli/listRepos.go +++ b/cmd/kosli/listRepos.go @@ -182,7 +182,7 @@ func printReposListAsTable(raw string, out io.Writer, page int) error { header := []string{"NAME", "URL", "PROVIDER", "TAGS"} rows := []string{} for _, repo := range response.Repos { - row := fmt.Sprintf("%v\t%v\t%v\t%s", repo["name"], repo["url"], repo["provider"], formatRepoTags(repo["tags"])) + row := fmt.Sprintf("%v\t%v\t%v\t%s", repo["name"], repo["url"], repo["provider"], formatPlainTags(repo["tags"])) rows = append(rows, row) } diff --git a/cmd/kosli/tableHelpers.go b/cmd/kosli/tableHelpers.go index 26dcfb821..cf12b564b 100644 --- a/cmd/kosli/tableHelpers.go +++ b/cmd/kosli/tableHelpers.go @@ -9,7 +9,8 @@ import ( // sortedTagPairs renders a tags map as "key=value" pairs ordered by key. The // order matters because tags reach the printers as a map, whose iteration order // would otherwise leak into the table output. A missing or non-map value yields -// no pairs, since the API omits "tags" entirely when there are none. +// no pairs: responses carry an empty object today, but the printers have always +// tolerated both shapes, so the shared helper does too. func sortedTagPairs(rawTags any) []string { tags, ok := rawTags.(map[string]any) if !ok || len(tags) == 0 { @@ -29,8 +30,7 @@ func sortedTagPairs(rawTags any) []string { // formatTags renders a tags map as "[key=value], [key=value]" ordered by key, // or "" when there are no tags. Detail views substitute "None" for the empty -// string; list columns leave it blank. Repo and control tables use the -// unbracketed pairs from sortedTagPairs instead. +// string; list columns leave it blank. func formatTags(rawTags any) string { pairs := sortedTagPairs(rawTags) bracketed := make([]string, 0, len(pairs)) @@ -39,3 +39,9 @@ func formatTags(rawTags any) string { } return strings.Join(bracketed, ", ") } + +// formatPlainTags renders a tags map as "key=value, key=value" ordered by key, +// or "" when there are no tags. +func formatPlainTags(rawTags any) string { + return strings.Join(sortedTagPairs(rawTags), ", ") +} diff --git a/cmd/kosli/tableHelpers_test.go b/cmd/kosli/tableHelpers_test.go index 7437be3bd..51c0ec9d5 100644 --- a/cmd/kosli/tableHelpers_test.go +++ b/cmd/kosli/tableHelpers_test.go @@ -69,43 +69,67 @@ func TestFormatTags(t *testing.T) { } // The table printers receive tags as a map, so without an explicit sort their -// output ordering was whatever Go's map iteration gave that run. -func TestTagRenderingIsDeterministicAcrossPrinters(t *testing.T) { - t.Run("printFlowAsTable orders tags by key", func(t *testing.T) { - raw := `{"name":"backend","description":"Backend service","template":"artifact","last_deployment_at":null,"tags":{"team":"platform","env":"prod","app":"api"}}` +// output ordering was whatever Go's map iteration gave that run: the released +// CLI returned three distinct orderings for these three keys across 20 runs. +// Each subtest pins the full rendering, tab padding included, so a column-width +// change cannot pass silently. +func TestTagRenderingIsSortedAcrossPrinters(t *testing.T) { + const tags = `{"team":"platform","env":"prod","app":"api"}` + + t.Run("printFlowAsTable", func(t *testing.T) { + raw := `{"name":"backend","description":"Backend service","template":"artifact","last_deployment_at":null,"tags":` + tags + `}` var buf bytes.Buffer require.NoError(t, printFlowAsTable(raw, &buf, 0)) - require.Contains(t, buf.String(), "Tags: [app=api], [env=prod], [team=platform]\n") + require.Equal(t, "Name: backend\n"+ + "Description: Backend service\n"+ + "Template: artifact\n"+ + "Last Deployment At: N/A\n"+ + "Tags: [app=api], [env=prod], [team=platform]\n", buf.String()) }) - t.Run("printEnvironmentAsTable orders tags by key", func(t *testing.T) { - raw := `{"name":"prod","type":"K8S","description":"","state":true,"last_reported_at":null,"tags":{"team":"platform","env":"prod","app":"api"}}` + t.Run("printEnvironmentAsTable", func(t *testing.T) { + raw := `{"name":"prod","type":"K8S","description":"","state":true,"last_reported_at":null,"tags":` + tags + `}` var buf bytes.Buffer require.NoError(t, printEnvironmentAsTable(raw, &buf, 0)) - require.Contains(t, buf.String(), "[app=api], [env=prod], [team=platform]") + require.Equal(t, "Name: prod\n"+ + "Type: K8S\n"+ + "Description: \n"+ + "State: COMPLIANT\n"+ + "Last Reported At: N/A\n"+ + "Tags: [app=api], [env=prod], [team=platform]\n"+ + "Policies: []\n", buf.String()) }) - t.Run("printFlowsListAsTable orders tags by key", func(t *testing.T) { - raw := `[{"name":"backend","description":"Backend service","tags":{"team":"platform","env":"prod","app":"api"}}]` + t.Run("printFlowsListAsTable", func(t *testing.T) { + raw := `[{"name":"backend","description":"Backend service","tags":` + tags + `}]` var buf bytes.Buffer require.NoError(t, printFlowsListAsTable(raw, &buf, 1)) - require.Contains(t, buf.String(), "[app=api], [env=prod], [team=platform]") + require.Equal(t, "NAME DESCRIPTION TAGS\n"+ + "backend Backend service [app=api], [env=prod], [team=platform]\n", buf.String()) }) - t.Run("printEnvListAsTable orders tags by key", func(t *testing.T) { - raw := `[{"name":"prod","type":"K8S","last_reported_at":null,"last_modified_at":null,"tags":{"team":"platform","env":"prod","app":"api"}}]` + t.Run("printEnvListAsTable", func(t *testing.T) { + raw := `[{"name":"prod","type":"K8S","last_reported_at":null,"last_modified_at":null,"tags":` + tags + `}]` var buf bytes.Buffer require.NoError(t, printEnvListAsTable(raw, &buf, 1)) - require.Contains(t, buf.String(), "[app=api], [env=prod], [team=platform]") + require.Equal(t, "NAME TYPE LAST REPORT LAST MODIFIED TAGS POLICIES\n"+ + "prod K8S [app=api], [env=prod], [team=platform] []\n", buf.String()) }) } -// The API omits "tags" entirely for an environment without tags, which used to -// panic on an unchecked type assertion. +// Responses carry "tags": {} for an untagged resource, but get environment +// asserted the value to a map unchecked, so an absent or null key would have +// panicked. The other printers already tolerated both shapes. func TestPrintEnvironmentAsTableWithoutTags(t *testing.T) { raw := `{"name":"prod","type":"K8S","description":"","state":true,"last_reported_at":null}` var buf bytes.Buffer require.NoError(t, printEnvironmentAsTable(raw, &buf, 0)) - require.Contains(t, buf.String(), "Tags:") - require.Contains(t, buf.String(), "None") + require.Contains(t, buf.String(), "Tags: None\n") +} + +func TestFormatPlainTags(t *testing.T) { + require.Equal(t, "", formatPlainTags(nil)) + require.Equal(t, "", formatPlainTags(map[string]any{})) + require.Equal(t, "", formatPlainTags("not-a-map")) + require.Equal(t, "a=1, b=x", formatPlainTags(map[string]any{"b": "x", "a": float64(1)})) } From 3ffbec8415fc189fe2e36b14f189aeab6b30a043 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dan=20Gr=C3=B8ndahl?= Date: Fri, 18 Sep 2026 09:33:44 +0200 Subject: [PATCH 3/3] docs(table output): keep only the durable why in tag comments The comments narrated the bug instead of the constraint: that the released CLI returned three distinct orderings across 20 runs, that get environment used to assert the tags value unchecked, and that the other printers already tolerated both shapes. None of that is useful to a reader a year from now, and it belongs here in the history rather than in the source. What is left states the constraint the code cannot express on its own: map iteration order must not reach table output, an absent tags value must render as "None" rather than panic, and the printer tests assert full renderings so a column-width change cannot pass silently. --- cmd/kosli/tableHelpers.go | 8 +++----- cmd/kosli/tableHelpers_test.go | 11 +++-------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/cmd/kosli/tableHelpers.go b/cmd/kosli/tableHelpers.go index cf12b564b..e9e70bf82 100644 --- a/cmd/kosli/tableHelpers.go +++ b/cmd/kosli/tableHelpers.go @@ -6,11 +6,9 @@ import ( "strings" ) -// sortedTagPairs renders a tags map as "key=value" pairs ordered by key. The -// order matters because tags reach the printers as a map, whose iteration order -// would otherwise leak into the table output. A missing or non-map value yields -// no pairs: responses carry an empty object today, but the printers have always -// tolerated both shapes, so the shared helper does too. +// sortedTagPairs renders a tags map as "key=value" pairs ordered by key, so map +// iteration order cannot leak into table output. A missing or non-map value +// yields no pairs. func sortedTagPairs(rawTags any) []string { tags, ok := rawTags.(map[string]any) if !ok || len(tags) == 0 { diff --git a/cmd/kosli/tableHelpers_test.go b/cmd/kosli/tableHelpers_test.go index 51c0ec9d5..1fca471b2 100644 --- a/cmd/kosli/tableHelpers_test.go +++ b/cmd/kosli/tableHelpers_test.go @@ -68,11 +68,8 @@ func TestFormatTags(t *testing.T) { } } -// The table printers receive tags as a map, so without an explicit sort their -// output ordering was whatever Go's map iteration gave that run: the released -// CLI returned three distinct orderings for these three keys across 20 runs. -// Each subtest pins the full rendering, tab padding included, so a column-width -// change cannot pass silently. +// Each subtest asserts the full rendering, tab padding included, so a change in +// column width cannot pass silently. func TestTagRenderingIsSortedAcrossPrinters(t *testing.T) { const tags = `{"team":"platform","env":"prod","app":"api"}` @@ -117,9 +114,7 @@ func TestTagRenderingIsSortedAcrossPrinters(t *testing.T) { }) } -// Responses carry "tags": {} for an untagged resource, but get environment -// asserted the value to a map unchecked, so an absent or null key would have -// panicked. The other printers already tolerated both shapes. +// An absent or null tags value must render as "None" rather than panic. func TestPrintEnvironmentAsTableWithoutTags(t *testing.T) { raw := `{"name":"prod","type":"K8S","description":"","state":true,"last_reported_at":null}` var buf bytes.Buffer