Skip to content

Commit 1e0dff2

Browse files
committed
gopls/internal/regtest: avoid race in TestSwitchFromGOPATHToModuleMode
Fix two bugs in TestSwitchFromGOPATHToModuleMode: - `go mod init` was run in the wrong directory. - on-disk change notifications raced with the main.go edit, causing us to only encounter the problem of the previous bullet in rare cases where the on-disk notification won the race I've filed golang/go#57558 to track fixing the fundamental raciness of view changes. Fixes golang/go#57512 Change-Id: I2b21f944377e0ba45ee7be019a28f18a334f3516 Reviewed-on: https://go-review.googlesource.com/c/tools/+/459560 gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
1 parent 0441b43 commit 1e0dff2

File tree

2 files changed

+12
-7
lines changed

2 files changed

+12
-7
lines changed

gopls/internal/regtest/watch/watch_test.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,9 +596,16 @@ func main() {
596596
env.AfterChange(
597597
EmptyDiagnostics("main.go"),
598598
)
599-
if err := env.Sandbox.RunGoCommand(env.Ctx, "foo", "mod", []string{"init", "mod.com"}, true); err != nil {
599+
if err := env.Sandbox.RunGoCommand(env.Ctx, "", "mod", []string{"init", "mod.com"}, true); err != nil {
600600
t.Fatal(err)
601601
}
602+
603+
// TODO(golang/go#57558, golang/go#57512): file watching is asynchronous,
604+
// and we must wait for the view to be reconstructed before touching
605+
// main.go, so that the new view "knows" about main.go. This is a bug, but
606+
// awaiting the change here avoids it.
607+
env.AfterChange()
608+
602609
env.RegexpReplace("main.go", `"foo/blah"`, `"mod.com/foo/blah"`)
603610
env.AfterChange(
604611
EmptyDiagnostics("main.go"),

gopls/internal/regtest/workspace/workspace_test.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1199,12 +1199,10 @@ use (
11991199
// Removing the go.work file should put us back where we started.
12001200
env.RemoveWorkspaceFile("go.work")
12011201

1202-
// TODO(golang/go#57508): because file watching is asynchronous, we must
1203-
// ensure that the go.work change is seen before other changes, in order
1204-
// for the snapshot to "know" about the orphaned b/main.go below.
1205-
//
1206-
// This is a bug, plain and simple, but we await here to avoid test flakes
1207-
// while the underlying cause is fixed.
1202+
// TODO(golang/go#57558, golang/go#57508): file watching is asynchronous,
1203+
// and we must wait for the view to be reconstructed before touching
1204+
// b/main.go, so that the new view "knows" about b/main.go. This is simply
1205+
// a bug, but awaiting the change here avoids it.
12081206
env.Await(env.DoneWithChangeWatchedFiles())
12091207

12101208
// TODO(rfindley): fix this bug: reopening b/main.go is necessary here

0 commit comments

Comments
 (0)