[Workspace CLI] better logs streaming for validate (#20238)

* [Workspace CLI] better logs streaming for `validate`

* add tests, handle `\b`

Author: filiptronicek

components/gitpod-cli/cmd/validate.go

@@ -6,6 +6,7 @@ package cmd
 
 import (
 	"bufio"
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -16,6 +17,7 @@ import (
 	"path/filepath"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	"github.com/gitpod-io/gitpod/common-go/log"
 	"github.com/gitpod-io/gitpod/gitpod-cli/pkg/supervisor"
@@ -564,59 +566,85 @@ func pipeTask(ctx context.Context, task *api.TaskStatus, supervisor *supervisor.
 	}
 }
 
-func listenTerminal(ctx context.Context, task *api.TaskStatus, supervisor *supervisor.SupervisorClient, runLog *logrus.Entry) error {
-	listen, err := supervisor.Terminal.Listen(ctx, &api.ListenTerminalRequest{
-		Alias: task.Terminal,
-	})
-	if err != nil {
-		return err
-	}
+// TerminalReader is an interface for anything that can receive terminal data (this is abstracted for use in testing)
+type TerminalReader interface {
+	Recv() ([]byte, error)
+}
 
-	pr, pw := io.Pipe()
-	defer pr.Close()
-	defer pw.Close()
+type LinePrinter func(string)
 
-	scanner := bufio.NewScanner(pr)
-	const maxTokenSize = 1 * 1024 * 1024 // 1 MB
-	buf := make([]byte, maxTokenSize)
-	scanner.Buffer(buf, maxTokenSize)
+// processTerminalOutput reads from a TerminalReader, processes the output, and calls the provided LinePrinter for each complete line.
+// It handles UTF-8 decoding of characters split across chunks and control characters (\n \r \b).
+func processTerminalOutput(reader TerminalReader, printLine LinePrinter) error {
+	var buffer, line bytes.Buffer
 
-	go func() {
-		defer pw.Close()
-		for {
-			resp, err := listen.Recv()
-			if err != nil {
-				_ = pw.CloseWithError(err)
-				return
-			}
+	flushLine := func() {
+		if line.Len() > 0 {
+			printLine(line.String())
+			line.Reset()
+		}
+	}
 
-			title := resp.GetTitle()
-			if title != "" {
-				task.Presentation.Name = title
+	for {
+		data, err := reader.Recv()
+		if err != nil {
+			if err == io.EOF {
+				flushLine()
+				return nil
 			}
+			return err
+		}
+
+		buffer.Write(data)
 
-			exitCode := resp.GetExitCode()
-			if exitCode != 0 {
-				runLog.Infof("%s: exited with code %d", task.Presentation.Name, exitCode)
+		for {
+			r, size := utf8.DecodeRune(buffer.Bytes())
+			if r == utf8.RuneError && size == 0 {
+				break // incomplete character at the end
 			}
 
-			data := resp.GetData()
-			if len(data) > 0 {
-				_, err := pw.Write(data)
-				if err != nil {
-					_ = pw.CloseWithError(err)
-					return
+			char := buffer.Next(size)
+
+			switch r {
+			case '\r':
+				flushLine()
+			case '\n':
+				flushLine()
+			case '\b':
+				if line.Len() > 0 {
+					line.Truncate(line.Len() - 1)
 				}
+			default:
+				line.Write(char)
 			}
 		}
-	}()
+	}
+}
 
-	for scanner.Scan() {
-		line := scanner.Text()
+func listenTerminal(ctx context.Context, task *api.TaskStatus, supervisor *supervisor.SupervisorClient, runLog *logrus.Entry) error {
+	listen, err := supervisor.Terminal.Listen(ctx, &api.ListenTerminalRequest{Alias: task.Terminal})
+	if err != nil {
+		return err
+	}
+
+	terminalReader := &TerminalReaderAdapter{listen}
+	printLine := func(line string) {
 		runLog.Infof("%s: %s", task.Presentation.Name, line)
 	}
 
-	return scanner.Err()
+	return processTerminalOutput(terminalReader, printLine)
+}
+
+type TerminalReaderAdapter struct {
+	client api.TerminalService_ListenClient
+}
+
+func (t *TerminalReaderAdapter) Recv() ([]byte, error) {
+	resp, err := t.client.Recv()
+	if err != nil {
+		return nil, err
+	}
+	return resp.GetData(), nil
 }
 
 var validateOpts struct {

components/gitpod-cli/cmd/validate_test.go

@@ -0,0 +1,103 @@
+// Copyright (c) 2024 Gitpod GmbH. All rights reserved.
+// Licensed under the GNU Affero General Public License (AGPL).
+// See License.AGPL.txt in the project root for license information.
+
+package cmd
+
+import (
+	"io"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
+)
+
+type MockTerminalReader struct {
+	Data   [][]byte
+	Index  int
+	Errors []error
+}
+
+func (m *MockTerminalReader) Recv() ([]byte, error) {
+	if m.Index >= len(m.Data) {
+		return nil, io.EOF
+	}
+	data := m.Data[m.Index]
+	err := m.Errors[m.Index]
+	m.Index++
+	return data, err
+}
+
+func TestProcessTerminalOutput(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    [][]byte
+		expected []string
+	}{
+		{
+			name:     "Simple line",
+			input:    [][]byte{[]byte("Hello, World!\n")},
+			expected: []string{"Hello, World!"},
+		},
+		{
+			name:     "Windows line ending",
+			input:    [][]byte{[]byte("Hello\r\nWorld\r\n")},
+			expected: []string{"Hello", "World"},
+		},
+		{
+			name: "Updating line",
+			input: [][]byte{
+				[]byte("Hello, World!\r"),
+				[]byte("Hello, World 2!\r"),
+				[]byte("Hello, World 3!\n"),
+			},
+			expected: []string{"Hello, World!", "Hello, World 2!", "Hello, World 3!"},
+		},
+		{
+			name:     "Backspace",
+			input:    [][]byte{[]byte("Helloo\bWorld\n")},
+			expected: []string{"HelloWorld"},
+		},
+		{
+			name:     "Partial UTF-8",
+			input:    [][]byte{[]byte("Hello, 世"), []byte("界\n")},
+			expected: []string{"Hello, 世界"},
+		},
+		{
+			name: "Partial emoji",
+			input: [][]byte{
+				[]byte("Hello "),
+				{240, 159},
+				{145, 141},
+				[]byte("!\n"),
+			},
+			expected: []string{"Hello 👍!"},
+		},
+		{
+			name:     "Multiple lines in one receive",
+			input:    [][]byte{[]byte("Line1\nLine2\nLine3\n")},
+			expected: []string{"Line1", "Line2", "Line3"},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			reader := &MockTerminalReader{
+				Data:   test.input,
+				Errors: make([]error, len(test.input)),
+			}
+
+			var actual []string
+			printLine := func(line string) {
+				actual = append(actual, line)
+			}
+
+			err := processTerminalOutput(reader, printLine)
+			assert.NoError(t, err)
+
+			if diff := cmp.Diff(test.expected, actual); diff != "" {
+				t.Errorf("processTerminalOutput() mismatch (-want +got):\n%s", diff)
+			}
+		})
+	}
+}

components/gitpod-cli/go.mod

@@ -23,6 +23,7 @@ require (
 	github.com/sirupsen/logrus v1.9.3
 	github.com/sourcegraph/jsonrpc2 v0.0.0-20200429184054-15c2290dcb37
 	github.com/spf13/cobra v1.6.1
+	github.com/stretchr/testify v1.8.4
 	golang.org/x/sync v0.2.0
 	golang.org/x/term v0.15.0
 	golang.org/x/xerrors v0.0.0-20220609144429-65e65417b02f
@@ -33,6 +34,7 @@ require (
 require (
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/gitpod-io/gitpod/components/scrubber v0.0.0-00010101000000-000000000000 // indirect
 	github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 // indirect
 	github.com/hashicorp/golang-lru v1.0.2 // indirect
@@ -43,12 +45,14 @@ require (
 	github.com/mitchellh/reflectwalk v1.0.2 // indirect
 	github.com/onsi/ginkgo v1.16.5 // indirect
 	github.com/onsi/gomega v1.24.2 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_golang v1.16.0 // indirect
 	github.com/prometheus/client_model v0.3.0 // indirect
 	github.com/prometheus/common v0.42.0 // indirect
 	golang.org/x/crypto v0.16.0 // indirect
 	golang.org/x/sys v0.15.0 // indirect
 	google.golang.org/genproto v0.0.0-20221118155620-16455021b5e6 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 )
 
 require (