gopls/internal/regtest: avoid race in TestSwitchFromGOPATHToModuleMode

findleyr · findleyr · commit 1e0dff28f4f9 · 2023-01-03T18:27:19.000Z
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>
diff --git a/gopls/internal/regtest/watch/watch_test.go b/gopls/internal/regtest/watch/watch_test.go
@@ -596,9 +596,16 @@ func main() {
 		env.AfterChange(
 			EmptyDiagnostics("main.go"),
 		)
-		if err := env.Sandbox.RunGoCommand(env.Ctx, "foo", "mod", []string{"init", "mod.com"}, true); err != nil {
+		if err := env.Sandbox.RunGoCommand(env.Ctx, "", "mod", []string{"init", "mod.com"}, true); err != nil {
 			t.Fatal(err)
 		}
+
+		// TODO(golang/go#57558, golang/go#57512): file watching is asynchronous,
+		// and we must wait for the view to be reconstructed before touching
+		// main.go, so that the new view "knows" about main.go. This is a bug, but
+		// awaiting the change here avoids it.
+		env.AfterChange()
+
 		env.RegexpReplace("main.go", `"foo/blah"`, `"mod.com/foo/blah"`)
 		env.AfterChange(
 			EmptyDiagnostics("main.go"),
diff --git a/gopls/internal/regtest/workspace/workspace_test.go b/gopls/internal/regtest/workspace/workspace_test.go
@@ -1199,12 +1199,10 @@ use (
 		// Removing the go.work file should put us back where we started.
 		env.RemoveWorkspaceFile("go.work")
 
-		// TODO(golang/go#57508): because file watching is asynchronous, we must
-		// ensure that the go.work change is seen before other changes, in order
-		// for the snapshot to "know" about the orphaned b/main.go below.
-		//
-		// This is a bug, plain and simple, but we await here to avoid test flakes
-		// while the underlying cause is fixed.
+		// TODO(golang/go#57558, golang/go#57508): file watching is asynchronous,
+		// and we must wait for the view to be reconstructed before touching
+		// b/main.go, so that the new view "knows" about b/main.go. This is simply
+		// a bug, but awaiting the change here avoids it.
 		env.Await(env.DoneWithChangeWatchedFiles())
 
 		// TODO(rfindley): fix this bug: reopening b/main.go is necessary here
