Skip to content

Commit 8b2544f

Browse files
mirkoCrobumirkoCrobu
authored andcommitted
feat: review feedback
1 parent e6a4e25 commit 8b2544f

File tree

3 files changed

+43
-68
lines changed

3 files changed

+43
-68
lines changed

cmd/arduino-app-cli/board/board.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -81,18 +81,18 @@ func newSyncAppCmd() *cobra.Command {
8181
}
8282
defer s.Close()
8383
s.OnPull = func(name, path string) {
84-
feedback.Printf(" ⬆️ Pulled app %q to folder %q", name, path)
84+
feedback.Info(fmt.Sprintf(" ⬆️ Pulled app %q to folder %q", name, path))
8585
}
8686
s.OnPush = func(name string) {
87-
feedback.Printf(" ⬇️ Pushed app %q to the board", name)
87+
feedback.Info(fmt.Sprintf(" ⬇️ Pushed app %q to the board", name))
8888
}
8989

9090
tmp, err := s.EnableSyncApp(remote)
9191
if err != nil {
9292
return fmt.Errorf("failed to enable sync for app %q: %w", remote, err)
9393
}
9494

95-
feedback.Printf("Enable sync of %q at %q", remote, tmp)
95+
feedback.Info(fmt.Sprintf("Enable sync of %q at %q", remote, tmp))
9696

9797
<-cmd.Context().Done()
9898
_ = s.DisableSyncApp(remote)
@@ -119,7 +119,7 @@ func newBoardListCmd() *cobra.Command {
119119
default:
120120
panic("unreachable")
121121
}
122-
feedback.Printf("%s (%s) - Connection: %s [%s]\n", b.BoardName, b.CustomName, b.Protocol, address)
122+
feedback.Info(fmt.Sprintf("%s (%s) - Connection: %s [%s]\n", b.BoardName, b.CustomName, b.Protocol, address))
123123
}
124124
return nil
125125
},
@@ -140,7 +140,7 @@ func newBoardSetName() *cobra.Command {
140140
if err := board.SetCustomName(cmd.Context(), conn, name); err != nil {
141141
return fmt.Errorf("failed to set custom name: %w", err)
142142
}
143-
feedback.Printf("Custom name set to %q\n", name)
143+
feedback.Info(fmt.Sprintf("Custom name set to %q\n", name))
144144
return nil
145145
},
146146
}

cmd/feedback/feedback.go

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -127,16 +127,6 @@ func GetFormat() OutputFormat {
127127
return format
128128
}
129129

130-
// Printf behaves like fmt.Printf but writes on the out writer and adds a newline.
131-
func Printf(format string, v ...interface{}) {
132-
Print(fmt.Sprintf(format, v...))
133-
}
134-
135-
// Print behaves like fmt.Print but writes on the out writer and adds a newline.
136-
func Print(v string) {
137-
fmt.Fprintln(feedbackOut, v)
138-
}
139-
140130
// FatalError outputs the error and exits with status exitCode.
141131
func FatalError(err error, exitCode ExitCode) {
142132
Fatal(err.Error(), exitCode)
@@ -264,41 +254,40 @@ func Minimal(message string) {
264254
if logLevel < LogLevelMinimal {
265255
return
266256
}
267-
printLogMessage("step", message)
257+
printLogMessage(message)
268258
}
269259

270260
func Info(message string) {
271261
if logLevel < LogLevelInfo {
272262
return
273263
}
274-
printLogMessage("info", message)
264+
printLogMessage(message)
275265
}
276266

277267
func Debug(message string) {
278268
if logLevel < LogLevelDebug {
279269
return
280270
}
281-
printLogMessage("debug", message)
271+
printLogMessage(message)
282272
}
283273

284274
func Warning(message string) {
285275
if logLevel < LogLevelInfo {
286276
return
287277
}
288-
printLogMessage("warning", message)
278+
printLogMessage(message)
289279
}
290280

291-
func printLogMessage(level string, message string) {
281+
func printLogMessage(message string) {
292282
switch format {
293283
case JSON, MinifiedJSON:
294284
type logMsg struct {
295-
Level string `json:"level"`
296285
Message string `json:"message"`
297286
}
298-
output, _ := json.Marshal(logMsg{Level: level, Message: message})
299-
fmt.Fprintln(feedbackOut, string(output))
287+
output, _ := json.Marshal(logMsg{Message: message})
288+
fmt.Fprintln(stdOut, string(output))
300289
default: // Text
301-
fmt.Fprintf(feedbackErr, "%s %s\n", "["+strings.ToUpper(level)+"]", message)
290+
fmt.Fprintf(feedbackErr, "%s\n", message)
302291
}
303292
}
304293

cmd/feedback/feedback_test.go

Lines changed: 30 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -31,17 +31,20 @@ func TestOutputSelection(t *testing.T) {
3131
myOut := new(bytes.Buffer)
3232
SetOut(myOut)
3333
SetErr(myErr)
34+
35+
// Set the log level to minimal to ensure the message is printed
36+
SetLogLevel(LogLevelMinimal)
3437
SetFormat(Text)
3538

36-
// Could not change output stream after format has been set
3739
require.Panics(t, func() { SetOut(nil) })
3840
require.Panics(t, func() { SetErr(nil) })
39-
40-
// Coule not change output format twice
4141
require.Panics(t, func() { SetFormat(JSON) })
4242

43-
Print("Hello")
44-
require.Equal(t, myOut.String(), "Hello\n")
43+
Minimal("Hello")
44+
45+
require.Equal(t, "Hello\n", myErr.String())
46+
// The standard output stream should be empty
47+
require.Empty(t, myOut.String())
4548
}
4649

4750
func TestJSONOutputStream(t *testing.T) {
@@ -65,59 +68,42 @@ func TestJSONOutputStream(t *testing.T) {
6568
require.JSONEq(t, string(d), `{"stdout":"Hello\ufffdA","stderr":"Hello ERR"}`)
6669
}
6770

68-
func TestJsonOutputOnCustomStreams(t *testing.T) {
71+
func TestJsonOutput(t *testing.T) {
6972
reset()
7073

74+
// Setup custom buffers for stdout and stderr
7175
myErr := new(bytes.Buffer)
7276
myOut := new(bytes.Buffer)
7377
SetOut(myOut)
7478
SetErr(myErr)
75-
SetFormat(JSON)
7679

77-
// Could not change output stream after format has been set
78-
require.Panics(t, func() { SetOut(nil) })
79-
require.Panics(t, func() { SetErr(nil) })
80-
// Could not change output format twice
81-
require.Panics(t, func() { SetFormat(JSON) })
80+
// Set the state for the test
81+
SetLogLevel(LogLevelMinimal)
82+
SetFormat(JSON)
8283

83-
Print("Hello") // Output interactive data
84+
// --- Test 1: Verify a log message ---
85+
Minimal("Hello")
8486

85-
require.Equal(t, "", myOut.String())
86-
require.Equal(t, "", myErr.String())
87-
require.Equal(t, "Hello\n", bufferOut.String())
87+
// In JSON mode, logs are JSON Lines sent to stdout (myOut).
88+
expectedLog := `{"message": "Hello"}` // Assuming your printLogMessage does this
89+
require.JSONEq(t, expectedLog, myOut.String())
90+
require.Empty(t, myErr.String(), "Stderr should be empty for a log message")
91+
myOut.Reset() // Reset the buffer for the next test
8892

93+
// --- Test 2: Verify a result message ---
8994
PrintResult(&testResult{Success: true})
9095

91-
require.JSONEq(t, myOut.String(), `{ "success": true }`)
92-
require.Equal(t, myErr.String(), "")
93-
myOut.Reset()
94-
95-
_, _, res := OutputStreams()
96-
PrintResult(&testResult{Success: false, Output: res()})
97-
98-
require.JSONEq(t, `
99-
{
100-
"success": false,
101-
"output": {
102-
"stdout": "Hello\n",
103-
"stderr": ""
104-
}
105-
}`, myOut.String())
106-
require.Equal(t, myErr.String(), "")
96+
// PrintResult should output a well-formed JSON object to stdout (myOut).
97+
expectedResult := `{
98+
"success": true
99+
}`
100+
require.JSONEq(t, expectedResult, myOut.String())
101+
require.Empty(t, myErr.String(), "Stderr should be empty for a success result")
107102
}
108103

109104
type testResult struct {
110-
Success bool `json:"success"`
111-
Output *OutputStreamsResult `json:"output,omitempty"`
105+
Success bool `json:"success"`
112106
}
113107

114-
func (r *testResult) Data() interface{} {
115-
return r
116-
}
117-
118-
func (r *testResult) String() string {
119-
if r.Success {
120-
return "Success"
121-
}
122-
return "Failure"
123-
}
108+
func (r testResult) String() string { return fmt.Sprintf("Success: %t", r.Success) }
109+
func (r testResult) Data() interface{} { return r }

0 commit comments

Comments
 (0)