Skip to content

Commit 94602ac

Browse files
mtibbenmtibben
authored
fix: keep diagnostics off stdout for structured output (#786)
* fix: keep diagnostics off stdout for structured output * fix: restore stdio before failing validation test
1 parent a41c0fb commit 94602ac

9 files changed

Lines changed: 149 additions & 13 deletions

File tree

‎cmd/artifacts/list.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,7 @@ func (c *ListCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error {
9393
return err
9494
}
9595
if bld == nil {
96-
fmt.Println("No build found.")
97-
return nil
96+
return output.WriteTextOrStructured(os.Stdout, format, []buildkite.Artifact{}, "No build found.")
9897
}
9998

10099
var buildArtifacts []buildkite.Artifact

‎cmd/build/view.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ func (c *ViewCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error {
8787
}
8888

8989
ctx := context.Background()
90+
format := output.ResolveFormat(c.Output, f.Config.OutputFormat())
9091

9192
var opts view.ViewOptions
9293
opts.Pipeline = c.Pipeline
@@ -126,8 +127,7 @@ func (c *ViewCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error {
126127
return err
127128
}
128129
if bld == nil {
129-
fmt.Println("No build found.")
130-
return nil
130+
return output.WriteTextOrStructured(os.Stdout, format, nil, "No build found.")
131131
}
132132

133133
opts.Organization = bld.Organization
@@ -229,7 +229,6 @@ func (c *ViewCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error {
229229
},
230230
}
231231

232-
format := output.ResolveFormat(c.Output, f.Config.OutputFormat())
233232
if format == output.FormatText {
234233
writer, cleanup := bkIO.Pager(f.NoPager, f.Config.Pager())
235234
defer func() { _ = cleanup() }()

‎cmd/organization/list.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,10 @@ func (c *ListCmd) Run(globals cli.GlobalFlags) error {
4141

4242
f.NoPager = f.NoPager || globals.DisablePager()
4343

44+
format := output.ResolveFormat(c.Output, f.Config.OutputFormat())
4445
orgs := f.Config.ConfiguredOrganizations()
4546
if len(orgs) == 0 {
46-
fmt.Println("No organizations configured. Run `bk configure` to add one.")
47-
return nil
47+
return output.WriteTextOrStructured(os.Stdout, format, []Organization{}, "No organizations configured. Run `bk configure` to add one.")
4848
}
4949

5050
slices.Sort(orgs)
@@ -58,8 +58,6 @@ func (c *ListCmd) Run(globals cli.GlobalFlags) error {
5858
}
5959
}
6060

61-
format := output.ResolveFormat(c.Output, f.Config.OutputFormat())
62-
6361
if format != output.FormatText {
6462
return output.Write(os.Stdout, organizations, format)
6563
}

‎cmd/pipeline/list.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,9 +99,9 @@ func (c *ListCmd) runPipelineList(ctx context.Context, f *factory.Factory) error
9999
return fmt.Errorf("failed to list pipelines: %w", err)
100100
}
101101

102+
format := output.ResolveFormat(c.Output, f.Config.OutputFormat())
102103
if len(pipelines) == 0 {
103-
fmt.Fprintln(os.Stdout, "No pipelines found matching the specified criteria.")
104-
return nil
104+
return output.WriteTextOrStructured(os.Stdout, format, []buildkite.Pipeline{}, "No pipelines found matching the specified criteria.")
105105
}
106106

107107
return c.displayPipelines(pipelines, f)

‎internal/util/util.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package util
33
import (
44
"encoding/base64"
55
"fmt"
6+
"os"
67
"strings"
78

89
"github.com/pkg/browser"
@@ -21,7 +22,7 @@ func OpenInWebBrowser(openInWeb bool, webUrl string) error {
2122
if openInWeb {
2223
err := browser.OpenURL(webUrl)
2324
if err != nil {
24-
fmt.Println("Error opening browser: ", err)
25+
fmt.Fprintf(os.Stderr, "Error opening browser: %v\n", err)
2526
return err
2627
}
2728
}

‎pkg/cmd/validation/config.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package validation
33
import (
44
"errors"
55
"fmt"
6+
"os"
67
"strings"
78

89
"github.com/buildkite/cli/v3/internal/config"
@@ -58,7 +59,7 @@ func validateConfiguration(conf *config.Config, commandPath, orgOverride string)
5859
return errors.New("you are not authenticated. Run bk auth login to authenticate")
5960
// an organization may not be present if the user is only viewing public resources
6061
case missingOrg:
61-
fmt.Println("Warning: no organization set, only public pipelines will be visible. Run bk auth login, or bk use, to set an organization")
62+
fmt.Fprintln(os.Stderr, "Warning: no organization set, only public pipelines will be visible. Run bk auth login, or bk use, to set an organization")
6263
return nil
6364
}
6465

‎pkg/cmd/validation/config_test.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package validation
22

33
import (
4+
"io"
5+
"os"
6+
"strings"
47
"testing"
58

69
"github.com/buildkite/cli/v3/internal/config"
@@ -52,6 +55,29 @@ func TestValidateConfiguration_MissingValues(t *testing.T) {
5255
t.Fatalf("expected no error when token and org are set, got %v", err)
5356
}
5457
})
58+
59+
t.Run("missing org warning is written to stderr", func(t *testing.T) {
60+
t.Setenv("BUILDKITE_API_TOKEN", "token")
61+
t.Setenv("BUILDKITE_ORGANIZATION_SLUG", "")
62+
conf := newTestConfig(t)
63+
64+
var validationErr error
65+
stdout, stderr := captureStandardStreams(t, func() {
66+
validationErr = ValidateConfiguration(conf, "pipeline view")
67+
})
68+
69+
if validationErr != nil {
70+
t.Fatalf("expected no error when only org is missing, got %v", validationErr)
71+
}
72+
73+
if stdout != "" {
74+
t.Fatalf("expected stdout to remain empty, got %q", stdout)
75+
}
76+
77+
if !strings.Contains(stderr, "Warning: no organization set") {
78+
t.Fatalf("expected stderr warning, got %q", stderr)
79+
}
80+
})
5581
}
5682

5783
func newTestConfig(t *testing.T) *config.Config {
@@ -61,3 +87,54 @@ func newTestConfig(t *testing.T) *config.Config {
6187
bkKeyring.MockForTesting()
6288
return config.New(nil, nil)
6389
}
90+
91+
func captureStandardStreams(t *testing.T, fn func()) (stdout, stderr string) {
92+
t.Helper()
93+
94+
oldStdout := os.Stdout
95+
oldStderr := os.Stderr
96+
97+
stdoutR, stdoutW, err := os.Pipe()
98+
if err != nil {
99+
t.Fatalf("os.Pipe() stdout error = %v", err)
100+
}
101+
stderrR, stderrW, err := os.Pipe()
102+
if err != nil {
103+
t.Fatalf("os.Pipe() stderr error = %v", err)
104+
}
105+
106+
os.Stdout = stdoutW
107+
os.Stderr = stderrW
108+
109+
defer func() {
110+
os.Stdout = oldStdout
111+
os.Stderr = oldStderr
112+
}()
113+
114+
fn()
115+
116+
if err := stdoutW.Close(); err != nil {
117+
t.Fatalf("stdout close error = %v", err)
118+
}
119+
if err := stderrW.Close(); err != nil {
120+
t.Fatalf("stderr close error = %v", err)
121+
}
122+
123+
stdoutBytes, err := io.ReadAll(stdoutR)
124+
if err != nil {
125+
t.Fatalf("stdout read error = %v", err)
126+
}
127+
stderrBytes, err := io.ReadAll(stderrR)
128+
if err != nil {
129+
t.Fatalf("stderr read error = %v", err)
130+
}
131+
132+
if err := stdoutR.Close(); err != nil {
133+
t.Fatalf("stdout reader close error = %v", err)
134+
}
135+
if err := stderrR.Close(); err != nil {
136+
t.Fatalf("stderr reader close error = %v", err)
137+
}
138+
139+
return string(stdoutBytes), string(stderrBytes)
140+
}

‎pkg/output/output.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,17 @@ func Write(w io.Writer, v interface{}, format Format) error {
5353
}
5454
}
5555

56+
// WriteTextOrStructured writes a human-readable text line for text output and
57+
// structured output for JSON or YAML formats.
58+
func WriteTextOrStructured(w io.Writer, format Format, structuredValue interface{}, text string) error {
59+
if format == FormatText {
60+
_, err := fmt.Fprintln(w, text)
61+
return err
62+
}
63+
64+
return Write(w, structuredValue, format)
65+
}
66+
5667
func writeJSON(w io.Writer, v interface{}) error {
5768
encoder := json.NewEncoder(w)
5869
encoder.SetIndent("", " ")

‎pkg/output/output_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package output
2+
3+
import (
4+
"bytes"
5+
"strings"
6+
"testing"
7+
)
8+
9+
func TestWriteTextOrStructured(t *testing.T) {
10+
t.Parallel()
11+
12+
t.Run("writes text for text output", func(t *testing.T) {
13+
t.Parallel()
14+
15+
var buf bytes.Buffer
16+
if err := WriteTextOrStructured(&buf, FormatText, []string{}, "No pipelines found."); err != nil {
17+
t.Fatalf("WriteTextOrStructured() error = %v", err)
18+
}
19+
20+
if got := strings.TrimSpace(buf.String()); got != "No pipelines found." {
21+
t.Fatalf("WriteTextOrStructured() = %q, want %q", got, "No pipelines found.")
22+
}
23+
})
24+
25+
t.Run("writes structured empty collections for json output", func(t *testing.T) {
26+
t.Parallel()
27+
28+
var buf bytes.Buffer
29+
if err := WriteTextOrStructured(&buf, FormatJSON, []string{}, "ignored"); err != nil {
30+
t.Fatalf("WriteTextOrStructured() error = %v", err)
31+
}
32+
33+
if got := strings.TrimSpace(buf.String()); got != "[]" {
34+
t.Fatalf("WriteTextOrStructured() = %q, want %q", got, "[]")
35+
}
36+
})
37+
38+
t.Run("writes structured null values for json output", func(t *testing.T) {
39+
t.Parallel()
40+
41+
var buf bytes.Buffer
42+
if err := WriteTextOrStructured(&buf, FormatJSON, nil, "ignored"); err != nil {
43+
t.Fatalf("WriteTextOrStructured() error = %v", err)
44+
}
45+
46+
if got := strings.TrimSpace(buf.String()); got != "null" {
47+
t.Fatalf("WriteTextOrStructured() = %q, want %q", got, "null")
48+
}
49+
})
50+
}

0 commit comments

Comments
 (0)