mirror of
https://github.com/sourcegraph/sourcegraph.git
synced 2026-02-06 14:51:44 +00:00
Fixes DINF-82; This was very much a rabbithole. A few things: - The race that @bobheadxi mentioned here https://github.com/sourcegraph/sourcegraph/pull/63405#discussion_r1648180713 wasn't from `*output.Output` being unsafe, but `outputtest.Buffer` as it happened again (see [DINF-82](https://linear.app/sourcegraph/issue/DINF-82/devsgsg-test-failed-with-a-detected-race-condition)) - There something messed up with `cmds.start()`, which sometimes ends up printing the command output _after_ the exit message instead of before. - The crude `sort.Strings(want|have)` that was there already fixes that. - And without the sleep, it's possible to read the output from the `outputtest.Buffer` before the command outputs get written to it. - The `time.Sleep(300 * time.Milliseconds)` _mitigates/hides_ that problem. At least, this shouldn't blow up in CI and buys us time to fix the whole thing. We're tracking this in DINF-104. And out of 200 runs, I also stumbled on a race in `progress_tty`, tracked in DINF-105 (that packages is originally meant to be used by `src-cli` and was re-used for `sg` 3 years ago). I'm pretty unhappy about the solution, but a bandage is better than nothing. While ideally, we should really reconsider dropping `std.Output` entirely in `sg` and use the good stuff from github.com/charmbracelet instead because we don't want to spend too much time on arcane terminal things ourselves, I'm much more about concerned the concurrency issues mentioned above. ## Test plan CI + `sg bazel test //dev/sg:sg_test --runs_per_test=100`
142 lines
2.7 KiB
Go
142 lines
2.7 KiB
Go
package outputtest
|
|
|
|
import (
|
|
"strconv"
|
|
"sync"
|
|
)
|
|
|
|
// Buffer is used to test code that uses the `output` library to produce
|
|
// output. It implements io.Writer and can be passed to output.NewOutput
|
|
// instead of stdout/stderr. See tests for Buffer for examples.
|
|
//
|
|
// Buffer parses *most* of the escape codes used by `output` and keeps the
|
|
// produced output accessible through its `Lines()` method.
|
|
//
|
|
// NOTE: Buffer is *not* complete and probably can't parse everything that
|
|
// output produces. It should be extended as needed.
|
|
type Buffer struct {
|
|
sync.Mutex
|
|
lines [][]byte
|
|
|
|
line int
|
|
column int
|
|
}
|
|
|
|
func (t *Buffer) Write(b []byte) (int, error) {
|
|
t.Lock()
|
|
defer t.Unlock()
|
|
|
|
cur := 0
|
|
|
|
// Debug helper:
|
|
// fmt.Printf("b: %q\n", string(b))
|
|
|
|
for cur < len(b) {
|
|
switch b[cur] {
|
|
case '\n':
|
|
t.line++
|
|
t.column = 0
|
|
|
|
if len(t.lines) < t.line {
|
|
t.lines = append(t.lines, []byte{})
|
|
}
|
|
|
|
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.
|
|
// They follow this structure:
|
|
//
|
|
// \x1b [ <digit> <command>
|
|
//
|
|
// So we jump over the \x1b[ and try to parse the digit.
|
|
|
|
cur = cur + 2 // cur == '\x1b', cur + 1 == '['
|
|
|
|
digitStart := cur
|
|
for isDigit(b[cur]) {
|
|
cur++
|
|
}
|
|
|
|
rawDigit := string(b[digitStart:cur])
|
|
digit, err := strconv.ParseInt(rawDigit, 0, 64)
|
|
if err != nil {
|
|
return 0, err
|
|
}
|
|
|
|
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':
|
|
// reset current line
|
|
if len(t.lines) > t.line {
|
|
t.lines[t.line] = []byte{}
|
|
t.column = 0
|
|
}
|
|
case 'A':
|
|
// move line up by <digit>
|
|
t.line = t.line - int(digit)
|
|
|
|
case 'D':
|
|
// *d*elete cursor by <digit> 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[cur] != 'm' {
|
|
cur++
|
|
}
|
|
}
|
|
|
|
default:
|
|
t.writeToCurrentLine(b[cur])
|
|
}
|
|
|
|
cur++
|
|
}
|
|
|
|
return len(b), nil
|
|
}
|
|
|
|
func (t *Buffer) 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 *Buffer) Lines() []string {
|
|
t.Lock()
|
|
defer t.Unlock()
|
|
|
|
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' }
|