Skip to content

Commit

Permalink
internal/lsp/progress: actually close over Context in WorkDoneWriter
Browse files Browse the repository at this point in the history
CL 409936 eliminated cases where we close over a Context during progress
reporting, except in one instance where it wasn't possible: the
WorkDoneWriter that must implement the io.Writer interface.

Unfortunately it contained a glaring bug that the ctx field was never
set, and the regression test for progress reporting during `go generate`
was disabled due to flakiness (golang/go#49901).

Incidentally, the fundamental problem that CL 409936 addressed may also
fix the flakiness of TestGenerateProgress.

Fix the bug, and re-enable the test.

Fixes golang/go#53781

Change-Id: Ideb99a5525667e45d2e41fcc5078699ba1e0f1a3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/417115
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
  • Loading branch information
findleyr authored and gopherbot committed Jul 12, 2022
1 parent 7c06b01 commit 459e2b8
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 6 deletions.
2 changes: 0 additions & 2 deletions gopls/internal/regtest/misc/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import (
)

func TestGenerateProgress(t *testing.T) {
t.Skipf("skipping flaky test: https://golang.org/issue/49901")

const generatedWorkspace = `
-- go.mod --
module fake.test
Expand Down
4 changes: 2 additions & 2 deletions internal/lsp/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ func (c *commandHandler) runTests(ctx context.Context, snapshot source.Snapshot,
// create output
buf := &bytes.Buffer{}
ew := progress.NewEventWriter(ctx, "test")
out := io.MultiWriter(ew, progress.NewWorkDoneWriter(work), buf)
out := io.MultiWriter(ew, progress.NewWorkDoneWriter(ctx, work), buf)

// Run `go test -run Func` on each test.
var failedTests int
Expand Down Expand Up @@ -487,7 +487,7 @@ func (c *commandHandler) Generate(ctx context.Context, args command.GenerateArgs
Args: []string{"-x", pattern},
WorkingDir: args.Dir.SpanURI().Filename(),
}
stderr := io.MultiWriter(er, progress.NewWorkDoneWriter(deps.work))
stderr := io.MultiWriter(er, progress.NewWorkDoneWriter(ctx, deps.work))
if err := deps.snapshot.RunGoCommandPiped(ctx, source.Normal, inv, er, stderr); err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions internal/lsp/progress/progress.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ type WorkDoneWriter struct {
wd *WorkDone
}

func NewWorkDoneWriter(wd *WorkDone) *WorkDoneWriter {
return &WorkDoneWriter{wd: wd}
func NewWorkDoneWriter(ctx context.Context, wd *WorkDone) *WorkDoneWriter {
return &WorkDoneWriter{ctx: ctx, wd: wd}
}

func (wdw *WorkDoneWriter) Write(p []byte) (n int, err error) {
Expand Down

0 comments on commit 459e2b8

Please sign in to comment.