write diags to stderr

ethanndickson · ethanndickson · commit 0efd6ed5e482 · 2024-09-24T14:47:24.000Z
diff --git a/cli/cliui/agent.go b/cli/cliui/agent.go
@@ -406,12 +406,6 @@ func (d ConnDiags) splitDiagnostics() (general, client, agent []string) {
 		}
 	}
 
-	if d.PingP2P {
-		general = append(general, "✔ You are connected directly (p2p)")
-	} else {
-		general = append(general, fmt.Sprintf("❗ You are connected via a DERP relay, not directly (p2p)\n%s#common-problems-with-direct-connections", d.TroubleshootingURL))
-	}
-
 	if d.PingP2P && !d.Verbose {
 		return general, client, agent
 	}
diff --git a/cli/ping.go b/cli/ping.go
@@ -62,8 +62,7 @@ func (s *pingSummary) addResult(r *ipnstate.PingResult) {
 }
 
 // Write finalizes the summary and writes it
-func (s *pingSummary) Write(wsname string, w io.Writer) {
-	s.Workspace = wsname
+func (s *pingSummary) Write(w io.Writer) {
 	if s.Successful > 0 {
 		s.Avg = ptr.Ref(time.Duration(s.latencySum / float64(s.Successful) * float64(time.Second)))
 	}
@@ -140,7 +139,7 @@ func (r *RootCmd) ping() *serpent.Command {
 			diagCtx, diagCancel := context.WithTimeout(inv.Context(), 30*time.Second)
 			defer diagCancel()
 			diags := conn.GetPeerDiagnostics()
-			cliui.PeerDiagnostics(inv.Stdout, diags)
+			cliui.PeerDiagnostics(inv.Stderr, diags)
 
 			// Silent ping to determine whether we should show diags
 			_, didP2p, _, _ := conn.Ping(ctx)
@@ -186,8 +185,10 @@ func (r *RootCmd) ping() *serpent.Command {
 				}
 			}
 
-			connDiags.Write(inv.Stdout)
-			results := &pingSummary{}
+			connDiags.Write(inv.Stderr)
+			results := &pingSummary{
+				Workspace: workspaceName,
+			}
 			n := 0
 			start := time.Now()
 		pingLoop:
@@ -266,7 +267,13 @@ func (r *RootCmd) ping() *serpent.Command {
 				}
 			}
 
-			results.Write(workspaceName, inv.Stdout)
+			if didP2p {
+				_, _ = fmt.Fprintf(inv.Stderr, "✔ You are connected directly (p2p)\n")
+			} else {
+				_, _ = fmt.Fprintf(inv.Stderr, "❗ You are connected via a DERP relay, not directly (p2p)\n%s#common-problems-with-direct-connections\n", connDiags.TroubleshootingURL)
+			}
+
+			results.Write(inv.Stdout)
 
 			return nil
 		},
diff --git a/cli/ping_internal_test.go b/cli/ping_internal_test.go
@@ -33,11 +33,13 @@ func TestBuildSummary(t *testing.T) {
 			},
 		}
 
-		actual := pingSummary{}
+		actual := pingSummary{
+			Workspace: "test",
+		}
 		for _, r := range input {
 			actual.addResult(r)
 		}
-		actual.Write("test", io.Discard)
+		actual.Write(io.Discard)
 		require.Equal(t, time.Duration(0.1*float64(time.Second)), *actual.Min)
 		require.Equal(t, time.Duration(0.2*float64(time.Second)), *actual.Avg)
 		require.Equal(t, time.Duration(0.3*float64(time.Second)), *actual.Max)
@@ -53,11 +55,13 @@ func TestBuildSummary(t *testing.T) {
 			},
 		}
 
-		actual := &pingSummary{}
+		actual := &pingSummary{
+			Workspace: "test",
+		}
 		for _, r := range input {
 			actual.addResult(r)
 		}
-		actual.Write("test", io.Discard)
+		actual.Write(io.Discard)
 		require.Equal(t, actual.Successful, 1)
 		require.Equal(t, time.Duration(0.2*float64(time.Second)), *actual.Min)
 		require.Equal(t, time.Duration(0.2*float64(time.Second)), *actual.Avg)
@@ -90,11 +94,13 @@ func TestBuildSummary(t *testing.T) {
 			m2:         0,
 		}
 
-		actual := &pingSummary{}
+		actual := &pingSummary{
+			Workspace: "test",
+		}
 		for _, r := range input {
 			actual.addResult(r)
 		}
-		actual.Write("test", io.Discard)
+		actual.Write(io.Discard)
 		require.Equal(t, expected, actual)
 	})
 }
diff --git a/cli/ping_test.go b/cli/ping_test.go
@@ -65,8 +65,6 @@ func TestPing(t *testing.T) {
 			assert.NoError(t, err)
 		})
 
-		pty.ExpectMatch("✔ received remote agent data from Coder networking coordinator")
-		pty.ExpectMatch("You are connected")
 		pty.ExpectMatch("pong from " + workspace.Name)
 		cancel()
 		<-cmdDone

Original file line number	Diff line number	Diff line change
`@@ -406,12 +406,6 @@ func (d ConnDiags) splitDiagnostics() (general, client, agent []string) {`
`406`	`406`	`}`
`407`	`407`	`}`
`408`	`408`
`409`		`- if d.PingP2P {`
`410`		`- general = append(general, "✔ You are connected directly (p2p)")`
`411`		`- } else {`
`412`		`- general = append(general, fmt.Sprintf("❗ You are connected via a DERP relay, not directly (p2p)\n%s#common-problems-with-direct-connections", d.TroubleshootingURL))`
`413`		`- }`
`414`		`-`
`415`	`409`	`if d.PingP2P && !d.Verbose {`
`416`	`410`	`return general, client, agent`
`417`	`411`	`}`