From 459e2b88fc869374454543f70b377f13599756f7 Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Tue, 12 Jul 2022 17:13:32 -0400 Subject: [PATCH] internal/lsp/progress: actually close over Context in WorkDoneWriter 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 TryBot-Result: Gopher Robot Run-TryBot: Robert Findley Auto-Submit: Robert Findley Reviewed-by: Alan Donovan --- gopls/internal/regtest/misc/generate_test.go | 2 -- internal/lsp/command.go | 4 ++-- internal/lsp/progress/progress.go | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/gopls/internal/regtest/misc/generate_test.go b/gopls/internal/regtest/misc/generate_test.go index 1dc22d737b..44789514f4 100644 --- a/gopls/internal/regtest/misc/generate_test.go +++ b/gopls/internal/regtest/misc/generate_test.go @@ -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 diff --git a/internal/lsp/command.go b/internal/lsp/command.go index cd4c727310..c173ef2354 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -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 @@ -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 } diff --git a/internal/lsp/progress/progress.go b/internal/lsp/progress/progress.go index d6794cf338..8b0d1c6a22 100644 --- a/internal/lsp/progress/progress.go +++ b/internal/lsp/progress/progress.go @@ -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) {