From 2a00eb980c22c4ab128b36fa6970d290416a5a05 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Tue, 1 Dec 2020 15:05:43 +0100 Subject: [PATCH 01/13] Hacky integration test for CampaignProgressPrinter --- cmd/src/campaign_progress_printer.go | 11 +- cmd/src/campaign_progress_printer_test.go | 151 ++++++++++++++++++ internal/output/output.go | 8 +- internal/output/progress.go | 5 + .../output/progress_with_status_bars_tty.go | 4 + 5 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 cmd/src/campaign_progress_printer_test.go diff --git a/cmd/src/campaign_progress_printer.go b/cmd/src/campaign_progress_printer.go index b780dbe942..2d78605672 100644 --- a/cmd/src/campaign_progress_printer.go +++ b/cmd/src/campaign_progress_printer.go @@ -29,6 +29,9 @@ func newCampaignProgressPrinter(out *output.Output, verbose bool, numParallelism } type campaignProgressPrinter struct { + // Used in tests only + forceNoSpinner bool + out *output.Output sem *semaphore.Weighted @@ -62,7 +65,13 @@ func (p *campaignProgressPrinter) initProgressBar(statuses []*campaigns.TaskStat p.progress = p.out.ProgressWithStatusBars([]output.ProgressBar{{ Label: fmt.Sprintf("Executing ... (0/%d, 0 errored)", len(statuses)), Max: float64(len(statuses)), - }}, statusBars, nil) + }}, statusBars, &output.ProgressOpts{ + // TODO: Hacky, only for testing + ForceNoSpinner: p.forceNoSpinner, + SuccessEmoji: "\u2705", + SuccessStyle: output.StyleSuccess, + PendingStyle: output.StylePending, + }) return numStatusBars } diff --git a/cmd/src/campaign_progress_printer_test.go b/cmd/src/campaign_progress_printer_test.go new file mode 100644 index 0000000000..cda5864466 --- /dev/null +++ b/cmd/src/campaign_progress_printer_test.go @@ -0,0 +1,151 @@ +package main + +import ( + "strconv" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/sourcegraph/src-cli/internal/campaigns" + "github.com/sourcegraph/src-cli/internal/output" +) + +func TestCampaignProgressPrinterIntegration(t *testing.T) { + buf := &ttyBuf{} + + out := output.NewOutput(buf, output.OutputOpts{ + ForceTTY: true, + ForceColor: true, + Verbose: true, + }) + + printer := newCampaignProgressPrinter(out, true, 4) + printer.forceNoSpinner = true + + printer.PrintStatuses([]*campaigns.TaskStatus{ + { + RepoName: "github.com/sourcegraph/sourcegraph", + StartedAt: time.Now(), + CurrentlyExecuting: "echo Hello World > README.md", + }, + { + RepoName: "github.com/sourcegraph/src-cli", + StartedAt: time.Now().Add(time.Duration(-5) * time.Second), + CurrentlyExecuting: "Downloading archive", + }, + }) + + have := buf.Lines() + want := []string{ + "⠋ Executing... (0/2, 0 errored) ", + "│ ", + "├── github.com/sourcegraph/sourcegraph echo Hello World > README.md 0s", + "└── github.com/sourcegraph/src-cli Downloading archive 0s", + } + + if !cmp.Equal(want, have) { + t.Fatalf("wrong output:\n%s", cmp.Diff(want, have)) + } + +} + +type ttyBuf struct { + lines [][]byte + + line int + column int +} + +func (t *ttyBuf) Write(b []byte) (int, error) { + var cur int + + for cur < len(b) { + switch b[cur] { + case '\n': + t.line++ + t.column = 0 + case '\x1b': + // First of all: forgive me. + // + // Now. Looks like we ran into a VT100 escape code. + // They follow this structure: + // + // \x1b [ + // + // So we jump over the \x1b[ and try to parse the digit. + + digitStart := cur + 2 // cur == '\x1b', cur + 1 == '[' + digitIndex := cur + 2 + for isDigit(b[digitIndex]) { + digitIndex++ + } + + rawDigit := string(b[digitStart:digitIndex]) + digit, err := strconv.ParseInt(rawDigit, 0, 64) + if err != nil { + return 0, err + } + + command := b[digitIndex] + + switch command { + case 'K': + // reset current line + if len(t.lines) > t.line { + t.lines[t.line] = []byte{} + t.column = 0 + } + case 'A': + // move line up by + t.line = t.line - int(digit) + + case 'D': + // *d*elete cursor by amount + t.column = t.column - int(digit) + + case 'm': + // noop + + case ';': + // color, skip over until end of color command + for b[digitIndex] != 'm' { + digitIndex++ + } + } + cur = digitIndex + 1 + continue + + default: + t.writeToCurrentLine(b[cur]) + } + + cur++ + } + + return len(b), nil +} + +func (t *ttyBuf) writeToCurrentLine(b byte) { + if len(t.lines) == t.line { + t.lines = append(t.lines, []byte{}) + } + + if len(t.lines[t.line]) <= t.column { + t.lines[t.line] = append(t.lines[t.line], b) + } else { + t.lines[t.line][t.column] = b + } + t.column++ +} + +func (t *ttyBuf) Lines() []string { + var lines []string + for _, l := range t.lines { + lines = append(lines, string(l)) + } + return lines +} + +func isDigit(ch byte) bool { + return '0' <= ch && ch <= '9' +} diff --git a/internal/output/output.go b/internal/output/output.go index ffb4c415be..6930c51221 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -50,7 +50,10 @@ type Output struct { type OutputOpts struct { // ForceColor ignores all terminal detection and enabled coloured output. ForceColor bool - Verbose bool + // ForceTTY ignores all terminal detection and enables TTY output. + ForceTTY bool + + Verbose bool } // newOutputPlatformQuirks provides a way for conditionally compiled code to @@ -62,6 +65,9 @@ func NewOutput(w io.Writer, opts OutputOpts) *Output { if opts.ForceColor { caps.Color = true } + if opts.ForceTTY { + caps.Isatty = true + } o := &Output{caps: caps, opts: opts, w: w} if newOutputPlatformQuirks != nil { diff --git a/internal/output/progress.go b/internal/output/progress.go index 96b70dc20a..5355dec0f6 100644 --- a/internal/output/progress.go +++ b/internal/output/progress.go @@ -33,6 +33,11 @@ type ProgressOpts struct { PendingStyle Style SuccessEmoji string SuccessStyle Style + + // ForceNoSpinner turns of the automatic updating of the progress bar and + // spinner in a background goroutine. + // Used for testing only! + ForceNoSpinner bool } func newProgress(bars []ProgressBar, o *Output, opts *ProgressOpts) Progress { diff --git a/internal/output/progress_with_status_bars_tty.go b/internal/output/progress_with_status_bars_tty.go index dbbeba8b56..7faad980ac 100644 --- a/internal/output/progress_with_status_bars_tty.go +++ b/internal/output/progress_with_status_bars_tty.go @@ -34,6 +34,10 @@ func newProgressWithStatusBarsTTY(bars []*ProgressBar, statusBars []*StatusBar, p.draw() + if opts.ForceNoSpinner { + return p + } + go func() { for s := range p.spinner.C { func() { From 3bc1440aa77c8d2d2dd3da2598836a1b86e1195c Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Tue, 1 Dec 2020 15:19:02 +0100 Subject: [PATCH 02/13] Clean up ttyBuf --- cmd/src/campaign_progress_printer_test.go | 25 ++++++++++++++--------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/cmd/src/campaign_progress_printer_test.go b/cmd/src/campaign_progress_printer_test.go index cda5864466..a28890a3e9 100644 --- a/cmd/src/campaign_progress_printer_test.go +++ b/cmd/src/campaign_progress_printer_test.go @@ -74,19 +74,23 @@ func (t *ttyBuf) Write(b []byte) (int, error) { // // So we jump over the \x1b[ and try to parse the digit. - digitStart := cur + 2 // cur == '\x1b', cur + 1 == '[' - digitIndex := cur + 2 - for isDigit(b[digitIndex]) { - digitIndex++ + cur = cur + 2 // cur == '\x1b', cur + 1 == '[' + + digitStart := cur + for isDigit(b[cur]) { + cur++ } - rawDigit := string(b[digitStart:digitIndex]) + rawDigit := string(b[digitStart:cur]) digit, err := strconv.ParseInt(rawDigit, 0, 64) if err != nil { return 0, err } - command := b[digitIndex] + command := b[cur] + + // Debug helper: + // fmt.Printf("command=%q, digit=%d (t.line=%d, t.column=%d)\n", command, digit, t.line, t.column) switch command { case 'K': @@ -102,18 +106,19 @@ func (t *ttyBuf) Write(b []byte) (int, error) { case 'D': // *d*elete cursor by amount t.column = t.column - int(digit) + if t.column < 0 { + t.column = 0 + } case 'm': // noop case ';': // color, skip over until end of color command - for b[digitIndex] != 'm' { - digitIndex++ + for b[cur] != 'm' { + cur++ } } - cur = digitIndex + 1 - continue default: t.writeToCurrentLine(b[cur]) From f0e5b410e4b9b1aceaf0148912160b24ac6cccae Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Tue, 1 Dec 2020 15:28:21 +0100 Subject: [PATCH 03/13] Force width and height in integration test --- cmd/src/campaign_progress_printer_test.go | 14 ++++++++------ internal/output/output.go | 11 +++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/cmd/src/campaign_progress_printer_test.go b/cmd/src/campaign_progress_printer_test.go index a28890a3e9..01b998fe9a 100644 --- a/cmd/src/campaign_progress_printer_test.go +++ b/cmd/src/campaign_progress_printer_test.go @@ -14,9 +14,11 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { buf := &ttyBuf{} out := output.NewOutput(buf, output.OutputOpts{ - ForceTTY: true, - ForceColor: true, - Verbose: true, + ForceTTY: true, + ForceColor: true, + ForceHeight: 25, + ForceWidth: 80, + Verbose: true, }) printer := newCampaignProgressPrinter(out, true, 4) @@ -38,9 +40,9 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { have := buf.Lines() want := []string{ "⠋ Executing... (0/2, 0 errored) ", - "│ ", - "├── github.com/sourcegraph/sourcegraph echo Hello World > README.md 0s", - "└── github.com/sourcegraph/src-cli Downloading archive 0s", + "│ ", + "├── github.com/sourcegraph/sourcegraph echo Hello World > README.md 0s", + "└── github.com/sourcegraph/src-cli Downloading archive 0s", } if !cmp.Equal(want, have) { diff --git a/internal/output/output.go b/internal/output/output.go index 6930c51221..f7326a3c25 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -53,6 +53,11 @@ type OutputOpts struct { // ForceTTY ignores all terminal detection and enables TTY output. ForceTTY bool + // ForceHeight ignores all terminal detection and sets the height to this value. + ForceHeight int + // ForceWidth ignores all terminal detection and sets the width to this value. + ForceWidth int + Verbose bool } @@ -68,6 +73,12 @@ func NewOutput(w io.Writer, opts OutputOpts) *Output { if opts.ForceTTY { caps.Isatty = true } + if opts.ForceHeight != 0 { + caps.Height = opts.ForceHeight + } + if opts.ForceWidth != 0 { + caps.Width = opts.ForceWidth + } o := &Output{caps: caps, opts: opts, w: w} if newOutputPlatformQuirks != nil { From 67fe8324988300e0b0f2cf291e578472bb4d27a2 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Tue, 1 Dec 2020 15:33:00 +0100 Subject: [PATCH 04/13] For now: skip on Windows --- cmd/src/campaign_progress_printer_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/src/campaign_progress_printer_test.go b/cmd/src/campaign_progress_printer_test.go index 01b998fe9a..d1bdf702a2 100644 --- a/cmd/src/campaign_progress_printer_test.go +++ b/cmd/src/campaign_progress_printer_test.go @@ -1,6 +1,7 @@ package main import ( + "runtime" "strconv" "testing" "time" @@ -11,6 +12,10 @@ import ( ) func TestCampaignProgressPrinterIntegration(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Something emits different escape codes on windows.") + } + buf := &ttyBuf{} out := output.NewOutput(buf, output.OutputOpts{ From b0eeaba7eddcaeff86c53e7baf2a7d4d05352388 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Tue, 1 Dec 2020 17:20:54 +0100 Subject: [PATCH 05/13] Add completed diff to progress printer test --- cmd/src/campaign_progress_printer.go | 17 ++++- cmd/src/campaign_progress_printer_test.go | 93 +++++++++++++++++++++-- 2 files changed, 99 insertions(+), 11 deletions(-) diff --git a/cmd/src/campaign_progress_printer.go b/cmd/src/campaign_progress_printer.go index 2d78605672..6ec4378701 100644 --- a/cmd/src/campaign_progress_printer.go +++ b/cmd/src/campaign_progress_printer.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "sort" "strings" "github.com/sourcegraph/go-diff/diff" @@ -305,14 +306,14 @@ func verboseDiffSummary(fileDiffs []*diff.FileDiff) ([]string, error) { sumInsertions int sumDeletions int ) + sort.Slice(fileDiffs, func(i, j int) bool { + return diffDisplayName(fileDiffs[i]) < diffDisplayName(fileDiffs[j]) + }) fileStats := make(map[string]string, len(fileDiffs)) for _, f := range fileDiffs { - name := f.NewName - if name == "/dev/null" { - name = f.OrigName - } + name := diffDisplayName(f) if len(name) > maxFilenameLen { maxFilenameLen = len(name) @@ -350,3 +351,11 @@ func verboseDiffSummary(fileDiffs []*diff.FileDiff) ([]string, error) { return lines, nil } + +func diffDisplayName(f *diff.FileDiff) string { + name := f.NewName + if name == "/dev/null" { + name = f.OrigName + } + return name +} diff --git a/cmd/src/campaign_progress_printer_test.go b/cmd/src/campaign_progress_printer_test.go index d1bdf702a2..9ad5f69e24 100644 --- a/cmd/src/campaign_progress_printer_test.go +++ b/cmd/src/campaign_progress_printer_test.go @@ -11,6 +11,31 @@ import ( "github.com/sourcegraph/src-cli/internal/output" ) +const progressPrinterDiff = `diff --git README.md README.md +new file mode 100644 +index 0000000..3363c39 +--- /dev/null ++++ README.md +@@ -0,0 +1,3 @@ ++# README ++ ++This is the readme +diff --git a/b/c/c.txt a/b/c/c.txt +deleted file mode 100644 +index 5da75cf..0000000 +--- a/b/c/c.txt ++++ /dev/null +@@ -1 +0,0 @@ +-this is c +diff --git x/x.txt x/x.txt +index 627c2ae..88f1836 100644 +--- x/x.txt ++++ x/x.txt +@@ -1 +1 @@ +-this is x ++this is x (or is it?) +` + func TestCampaignProgressPrinterIntegration(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("Something emits different escape codes on windows.") @@ -26,10 +51,7 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { Verbose: true, }) - printer := newCampaignProgressPrinter(out, true, 4) - printer.forceNoSpinner = true - - printer.PrintStatuses([]*campaigns.TaskStatus{ + statuses := []*campaigns.TaskStatus{ { RepoName: "github.com/sourcegraph/sourcegraph", StartedAt: time.Now(), @@ -40,16 +62,73 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { StartedAt: time.Now().Add(time.Duration(-5) * time.Second), CurrentlyExecuting: "Downloading archive", }, - }) + { + RepoName: "github.com/sourcegraph/automation-testing", + StartedAt: time.Now().Add(time.Duration(-5) * time.Second), + CurrentlyExecuting: "echo Hello World > README.md", + }, + } + printer := newCampaignProgressPrinter(out, true, 4) + printer.forceNoSpinner = true + + // Print with all three tasks running + printer.PrintStatuses(statuses) have := buf.Lines() want := []string{ - "⠋ Executing... (0/2, 0 errored) ", + "⠋ Executing... (0/3, 0 errored) ", "│ ", "├── github.com/sourcegraph/sourcegraph echo Hello World > README.md 0s", - "└── github.com/sourcegraph/src-cli Downloading archive 0s", + "├── github.com/sourcegraph/src-cli Downloading archive 0s", + "└── github.com/sourcegraph/automati... echo Hello World > README.md 0s", + } + if !cmp.Equal(want, have) { + t.Fatalf("wrong output:\n%s", cmp.Diff(want, have)) } + // Now mark the last task as completed + statuses[len(statuses)-1] = &campaigns.TaskStatus{ + RepoName: "github.com/sourcegraph/automation-testing", + StartedAt: time.Now().Add(time.Duration(-5) * time.Second), + FinishedAt: time.Now().Add(time.Duration(5) * time.Second), + CurrentlyExecuting: "", + Err: nil, + ChangesetSpec: &campaigns.ChangesetSpec{ + BaseRepository: "graphql-id", + CreatedChangeset: &campaigns.CreatedChangeset{ + BaseRef: "refs/heads/main", + BaseRev: "d34db33f", + HeadRepository: "graphql-id", + HeadRef: "refs/heads/my-campaign", + Title: "This is my campaign", + Body: "This is my campaign", + Commits: []campaigns.GitCommitDescription{ + { + Message: "This is my campaign", + Diff: progressPrinterDiff, + }, + }, + Published: false, + }, + }, + } + + printer.PrintStatuses(statuses) + have = buf.Lines() + want = []string{ + "github.com/sourcegraph/automation-testing", + "\tREADME.md | 3 +++", + "\ta/b/c/c.txt | 1 -", + "\tx/x.txt | 2 +-", + " 3 files changed, 4 insertions, 2 deletions", + " Execution took 10s", + "", + "⠋ Executing... (1/3, 0 errored) ███████████████▍", + "│ 0s", // Not sure why this is here, bug? + "├── github.com/sourcegraph/sourcegraph echo Hello World > README.md 0s", + "├── github.com/sourcegraph/src-cli Downloading archive 0s", + "└── github.com/sourcegraph/automati... 3 files changed ++++ 0s", + } if !cmp.Equal(want, have) { t.Fatalf("wrong output:\n%s", cmp.Diff(want, have)) } From b5bf3c2b59b69810b42bb31fe5031a2f6d13ee2a Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Tue, 1 Dec 2020 17:24:10 +0100 Subject: [PATCH 06/13] Print again to make sure that the output is the same --- cmd/src/campaign_progress_printer_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cmd/src/campaign_progress_printer_test.go b/cmd/src/campaign_progress_printer_test.go index 9ad5f69e24..687c8fbe23 100644 --- a/cmd/src/campaign_progress_printer_test.go +++ b/cmd/src/campaign_progress_printer_test.go @@ -133,6 +133,13 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { t.Fatalf("wrong output:\n%s", cmp.Diff(want, have)) } + // Print again to make sure we get the same result + printer.PrintStatuses(statuses) + have = buf.Lines() + if !cmp.Equal(want, have) { + t.Fatalf("wrong output:\n%s", cmp.Diff(want, have)) + } + } type ttyBuf struct { From 2e300965f7b727c6dd7e8f7d3dab80cc4e687cdf Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 2 Dec 2020 11:19:34 +0100 Subject: [PATCH 07/13] Sort filenames before printing diff summary --- cmd/src/campaign_progress_printer.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/cmd/src/campaign_progress_printer.go b/cmd/src/campaign_progress_printer.go index 6ec4378701..46828535a9 100644 --- a/cmd/src/campaign_progress_printer.go +++ b/cmd/src/campaign_progress_printer.go @@ -306,15 +306,15 @@ func verboseDiffSummary(fileDiffs []*diff.FileDiff) ([]string, error) { sumInsertions int sumDeletions int ) - sort.Slice(fileDiffs, func(i, j int) bool { - return diffDisplayName(fileDiffs[i]) < diffDisplayName(fileDiffs[j]) - }) fileStats := make(map[string]string, len(fileDiffs)) + fileNames := make([]string, len(fileDiffs)) - for _, f := range fileDiffs { + for i, f := range fileDiffs { name := diffDisplayName(f) + fileNames[i] = name + if len(name) > maxFilenameLen { maxFilenameLen = len(name) } @@ -329,8 +329,11 @@ func verboseDiffSummary(fileDiffs []*diff.FileDiff) ([]string, error) { fileStats[name] = fmt.Sprintf("%d %s", num, diffStatDiagram(stat)) } - for file, stats := range fileStats { - lines = append(lines, fmt.Sprintf("\t%-*s | %s", maxFilenameLen, file, stats)) + sort.Slice(fileNames, func(i, j int) bool { return fileNames[i] < fileNames[j] }) + + for _, name := range fileNames { + stats := fileStats[name] + lines = append(lines, fmt.Sprintf("\t%-*s | %s", maxFilenameLen, name, stats)) } var insertionsPlural string From 550dafa230af0b68eb2152e7da09ef0e87dfe2e5 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 2 Dec 2020 11:21:27 +0100 Subject: [PATCH 08/13] Fix missing clearCurrentLine call --- cmd/src/campaign_progress_printer_test.go | 2 +- internal/output/progress_with_status_bars_tty.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/src/campaign_progress_printer_test.go b/cmd/src/campaign_progress_printer_test.go index 687c8fbe23..58c3c4a9b9 100644 --- a/cmd/src/campaign_progress_printer_test.go +++ b/cmd/src/campaign_progress_printer_test.go @@ -124,7 +124,7 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { " Execution took 10s", "", "⠋ Executing... (1/3, 0 errored) ███████████████▍", - "│ 0s", // Not sure why this is here, bug? + "│ ", "├── github.com/sourcegraph/sourcegraph echo Hello World > README.md 0s", "├── github.com/sourcegraph/src-cli Downloading archive 0s", "└── github.com/sourcegraph/automati... 3 files changed ++++ 0s", diff --git a/internal/output/progress_with_status_bars_tty.go b/internal/output/progress_with_status_bars_tty.go index 7faad980ac..b5a8b1cbda 100644 --- a/internal/output/progress_with_status_bars_tty.go +++ b/internal/output/progress_with_status_bars_tty.go @@ -174,6 +174,7 @@ func (p *progressWithStatusBarsTTY) draw() { } if len(p.statusBars) > 0 { + p.o.clearCurrentLine() fmt.Fprint(p.o.w, StylePending, "│", runewidth.FillLeft("\n", p.o.caps.Width-1)) } From f68526d171b6d2ae6b90d1af48731b3311c67fe7 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 2 Dec 2020 11:29:20 +0100 Subject: [PATCH 09/13] Always create a new line on newline character --- cmd/src/campaign_progress_printer_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cmd/src/campaign_progress_printer_test.go b/cmd/src/campaign_progress_printer_test.go index 58c3c4a9b9..8682a029da 100644 --- a/cmd/src/campaign_progress_printer_test.go +++ b/cmd/src/campaign_progress_printer_test.go @@ -81,6 +81,7 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { "├── github.com/sourcegraph/sourcegraph echo Hello World > README.md 0s", "├── github.com/sourcegraph/src-cli Downloading archive 0s", "└── github.com/sourcegraph/automati... echo Hello World > README.md 0s", + "", } if !cmp.Equal(want, have) { t.Fatalf("wrong output:\n%s", cmp.Diff(want, have)) @@ -128,6 +129,7 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { "├── github.com/sourcegraph/sourcegraph echo Hello World > README.md 0s", "├── github.com/sourcegraph/src-cli Downloading archive 0s", "└── github.com/sourcegraph/automati... 3 files changed ++++ 0s", + "", } if !cmp.Equal(want, have) { t.Fatalf("wrong output:\n%s", cmp.Diff(want, have)) @@ -157,6 +159,11 @@ func (t *ttyBuf) Write(b []byte) (int, error) { case '\n': t.line++ t.column = 0 + + if len(t.lines) == t.line { + t.lines = append(t.lines, []byte{}) + } + case '\x1b': // First of all: forgive me. // From 48cab3266264efd3524e9c52ff90803cb78828c8 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 2 Dec 2020 11:33:07 +0100 Subject: [PATCH 10/13] Be more defensive in ttyBuf --- cmd/src/campaign_progress_printer_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cmd/src/campaign_progress_printer_test.go b/cmd/src/campaign_progress_printer_test.go index 8682a029da..6efa63651a 100644 --- a/cmd/src/campaign_progress_printer_test.go +++ b/cmd/src/campaign_progress_printer_test.go @@ -165,6 +165,13 @@ func (t *ttyBuf) Write(b []byte) (int, error) { } case '\x1b': + // Check if we're looking at a VT100 escape code. + if len(b) <= cur || b[cur+1] != '[' { + t.writeToCurrentLine(b[cur]) + cur++ + continue + } + // First of all: forgive me. // // Now. Looks like we ran into a VT100 escape code. From c7f017d581e4d55bfa085a78deae78f41ec466d1 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 2 Dec 2020 11:36:52 +0100 Subject: [PATCH 11/13] Use stable timestamps in test --- cmd/src/campaign_progress_printer_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/src/campaign_progress_printer_test.go b/cmd/src/campaign_progress_printer_test.go index 6efa63651a..eda4cad1d9 100644 --- a/cmd/src/campaign_progress_printer_test.go +++ b/cmd/src/campaign_progress_printer_test.go @@ -51,20 +51,21 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { Verbose: true, }) + now := time.Now() statuses := []*campaigns.TaskStatus{ { RepoName: "github.com/sourcegraph/sourcegraph", - StartedAt: time.Now(), + StartedAt: now, CurrentlyExecuting: "echo Hello World > README.md", }, { RepoName: "github.com/sourcegraph/src-cli", - StartedAt: time.Now().Add(time.Duration(-5) * time.Second), + StartedAt: now.Add(time.Duration(-5) * time.Second), CurrentlyExecuting: "Downloading archive", }, { RepoName: "github.com/sourcegraph/automation-testing", - StartedAt: time.Now().Add(time.Duration(-5) * time.Second), + StartedAt: now.Add(time.Duration(-5) * time.Second), CurrentlyExecuting: "echo Hello World > README.md", }, } @@ -90,8 +91,8 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { // Now mark the last task as completed statuses[len(statuses)-1] = &campaigns.TaskStatus{ RepoName: "github.com/sourcegraph/automation-testing", - StartedAt: time.Now().Add(time.Duration(-5) * time.Second), - FinishedAt: time.Now().Add(time.Duration(5) * time.Second), + StartedAt: now.Add(time.Duration(-5) * time.Second), + FinishedAt: now.Add(time.Duration(5) * time.Second), CurrentlyExecuting: "", Err: nil, ChangesetSpec: &campaigns.ChangesetSpec{ @@ -141,7 +142,6 @@ func TestCampaignProgressPrinterIntegration(t *testing.T) { if !cmp.Equal(want, have) { t.Fatalf("wrong output:\n%s", cmp.Diff(want, have)) } - } type ttyBuf struct { From 5f45b8a6b6c9649ed31fd4ab5723ff9bef2cabdc Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 2 Dec 2020 11:44:02 +0100 Subject: [PATCH 12/13] Clean up duplicated default opts --- cmd/src/campaign_progress_printer.go | 19 +++++++++---------- internal/output/progress.go | 6 ++++++ internal/output/progress_tty.go | 4 ++-- .../output/progress_with_status_bars_tty.go | 2 +- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/cmd/src/campaign_progress_printer.go b/cmd/src/campaign_progress_printer.go index 46828535a9..260172aa65 100644 --- a/cmd/src/campaign_progress_printer.go +++ b/cmd/src/campaign_progress_printer.go @@ -63,16 +63,15 @@ func (p *campaignProgressPrinter) initProgressBar(statuses []*campaigns.TaskStat statusBars = append(statusBars, output.NewStatusBar()) } - p.progress = p.out.ProgressWithStatusBars([]output.ProgressBar{{ - Label: fmt.Sprintf("Executing ... (0/%d, 0 errored)", len(statuses)), - Max: float64(len(statuses)), - }}, statusBars, &output.ProgressOpts{ - // TODO: Hacky, only for testing - ForceNoSpinner: p.forceNoSpinner, - SuccessEmoji: "\u2705", - SuccessStyle: output.StyleSuccess, - PendingStyle: output.StylePending, - }) + progressBars := []output.ProgressBar{ + { + Label: fmt.Sprintf("Executing ... (0/%d, 0 errored)", len(statuses)), + Max: float64(len(statuses)), + }, + } + + opts := output.DefaultProgressTTYOpts.WithForceNoSpinner(p.forceNoSpinner) + p.progress = p.out.ProgressWithStatusBars(progressBars, statusBars, opts) return numStatusBars } diff --git a/internal/output/progress.go b/internal/output/progress.go index 5355dec0f6..bae5a27a2f 100644 --- a/internal/output/progress.go +++ b/internal/output/progress.go @@ -40,6 +40,12 @@ type ProgressOpts struct { ForceNoSpinner bool } +func (opt *ProgressOpts) WithForceNoSpinner(forceNoSpinner bool) *ProgressOpts { + c := *opt + c.ForceNoSpinner = forceNoSpinner + return &c +} + func newProgress(bars []ProgressBar, o *Output, opts *ProgressOpts) Progress { barPtrs := make([]*ProgressBar, len(bars)) for i := range bars { diff --git a/internal/output/progress_tty.go b/internal/output/progress_tty.go index cc09e2dd4b..9939e9ccfc 100644 --- a/internal/output/progress_tty.go +++ b/internal/output/progress_tty.go @@ -9,7 +9,7 @@ import ( "github.com/mattn/go-runewidth" ) -var defaultProgressTTYOpts = ProgressOpts{ +var DefaultProgressTTYOpts = &ProgressOpts{ SuccessEmoji: "\u2705", SuccessStyle: StyleSuccess, PendingStyle: StylePending, @@ -145,7 +145,7 @@ func newProgressTTY(bars []*ProgressBar, o *Output, opts *ProgressOpts) *progres if opts != nil { p.opts = *opts } else { - p.opts = defaultProgressTTYOpts + p.opts = *DefaultProgressTTYOpts } p.determineEmojiWidth() diff --git a/internal/output/progress_with_status_bars_tty.go b/internal/output/progress_with_status_bars_tty.go index b5a8b1cbda..8222d72b29 100644 --- a/internal/output/progress_with_status_bars_tty.go +++ b/internal/output/progress_with_status_bars_tty.go @@ -22,7 +22,7 @@ func newProgressWithStatusBarsTTY(bars []*ProgressBar, statusBars []*StatusBar, if opts != nil { p.opts = *opts } else { - p.opts = defaultProgressTTYOpts + p.opts = *DefaultProgressTTYOpts } p.determineEmojiWidth() From 4ce51135d75f17de0d7e86c5d32eaeb19a800b30 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 2 Dec 2020 11:52:32 +0100 Subject: [PATCH 13/13] Make handling of no-spinner consistent --- cmd/src/campaign_progress_printer.go | 2 +- internal/output/progress.go | 10 +++++----- internal/output/progress_simple.go | 9 ++++++++- internal/output/progress_tty.go | 4 ++++ internal/output/progress_with_status_bars.go | 2 +- internal/output/progress_with_status_bars_simple.go | 10 +++++++++- internal/output/progress_with_status_bars_tty.go | 2 +- 7 files changed, 29 insertions(+), 10 deletions(-) diff --git a/cmd/src/campaign_progress_printer.go b/cmd/src/campaign_progress_printer.go index 260172aa65..88229a5d3c 100644 --- a/cmd/src/campaign_progress_printer.go +++ b/cmd/src/campaign_progress_printer.go @@ -70,7 +70,7 @@ func (p *campaignProgressPrinter) initProgressBar(statuses []*campaigns.TaskStat }, } - opts := output.DefaultProgressTTYOpts.WithForceNoSpinner(p.forceNoSpinner) + opts := output.DefaultProgressTTYOpts.WithoutSpinner() p.progress = p.out.ProgressWithStatusBars(progressBars, statusBars, opts) return numStatusBars diff --git a/internal/output/progress.go b/internal/output/progress.go index bae5a27a2f..566949c4bb 100644 --- a/internal/output/progress.go +++ b/internal/output/progress.go @@ -34,15 +34,15 @@ type ProgressOpts struct { SuccessEmoji string SuccessStyle Style - // ForceNoSpinner turns of the automatic updating of the progress bar and + // NoSpinner turns of the automatic updating of the progress bar and // spinner in a background goroutine. // Used for testing only! - ForceNoSpinner bool + NoSpinner bool } -func (opt *ProgressOpts) WithForceNoSpinner(forceNoSpinner bool) *ProgressOpts { +func (opt *ProgressOpts) WithoutSpinner() *ProgressOpts { c := *opt - c.ForceNoSpinner = forceNoSpinner + c.NoSpinner = true return &c } @@ -53,7 +53,7 @@ func newProgress(bars []ProgressBar, o *Output, opts *ProgressOpts) Progress { } if !o.caps.Isatty { - return newProgressSimple(barPtrs, o) + return newProgressSimple(barPtrs, o, opts) } return newProgressTTY(barPtrs, o, opts) diff --git a/internal/output/progress_simple.go b/internal/output/progress_simple.go index 48fb8f26f1..e68377903b 100644 --- a/internal/output/progress_simple.go +++ b/internal/output/progress_simple.go @@ -38,13 +38,20 @@ func (p *progressSimple) stop() { <-c } -func newProgressSimple(bars []*ProgressBar, o *Output) *progressSimple { +func newProgressSimple(bars []*ProgressBar, o *Output, opts *ProgressOpts) *progressSimple { p := &progressSimple{ Output: o, bars: bars, done: make(chan chan struct{}), } + if opts.NoSpinner { + if p.Output.opts.Verbose { + writeBars(p.Output, p.bars) + } + return p + } + go func() { ticker := time.NewTicker(1 * time.Second) defer ticker.Stop() diff --git a/internal/output/progress_tty.go b/internal/output/progress_tty.go index 9939e9ccfc..c8069b307b 100644 --- a/internal/output/progress_tty.go +++ b/internal/output/progress_tty.go @@ -156,6 +156,10 @@ func newProgressTTY(bars []*ProgressBar, o *Output, opts *ProgressOpts) *progres p.draw() + if opts.NoSpinner { + return p + } + go func() { for s := range p.spinner.C { func() { diff --git a/internal/output/progress_with_status_bars.go b/internal/output/progress_with_status_bars.go index 43ba899cb1..969b695347 100644 --- a/internal/output/progress_with_status_bars.go +++ b/internal/output/progress_with_status_bars.go @@ -16,7 +16,7 @@ func newProgressWithStatusBars(bars []ProgressBar, statusBars []*StatusBar, o *O } if !o.caps.Isatty { - return newProgressWithStatusBarsSimple(barPtrs, statusBars, o) + return newProgressWithStatusBarsSimple(barPtrs, statusBars, o, opts) } return newProgressWithStatusBarsTTY(barPtrs, statusBars, o, opts) diff --git a/internal/output/progress_with_status_bars_simple.go b/internal/output/progress_with_status_bars_simple.go index 5a2c7df278..8552859e9b 100644 --- a/internal/output/progress_with_status_bars_simple.go +++ b/internal/output/progress_with_status_bars_simple.go @@ -48,7 +48,7 @@ func (p *progressWithStatusBarsSimple) StatusBarResetf(i int, label, format stri } } -func newProgressWithStatusBarsSimple(bars []*ProgressBar, statusBars []*StatusBar, o *Output) *progressWithStatusBarsSimple { +func newProgressWithStatusBarsSimple(bars []*ProgressBar, statusBars []*StatusBar, o *Output, opts *ProgressOpts) *progressWithStatusBarsSimple { p := &progressWithStatusBarsSimple{ progressSimple: &progressSimple{ Output: o, @@ -58,6 +58,14 @@ func newProgressWithStatusBarsSimple(bars []*ProgressBar, statusBars []*StatusBa statusBars: statusBars, } + if opts.NoSpinner { + if p.Output.opts.Verbose { + writeBars(p.Output, p.bars) + writeStatusBars(p.Output, p.statusBars) + } + return p + } + go func() { ticker := time.NewTicker(1 * time.Second) defer ticker.Stop() diff --git a/internal/output/progress_with_status_bars_tty.go b/internal/output/progress_with_status_bars_tty.go index 8222d72b29..012f5de48d 100644 --- a/internal/output/progress_with_status_bars_tty.go +++ b/internal/output/progress_with_status_bars_tty.go @@ -34,7 +34,7 @@ func newProgressWithStatusBarsTTY(bars []*ProgressBar, statusBars []*StatusBar, p.draw() - if opts.ForceNoSpinner { + if opts.NoSpinner { return p }