Skip to content

Commit

Permalink
gopls/internal/regtest: avoid race in TestSwitchFromGOPATHToModuleMode
Browse files Browse the repository at this point in the history
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 <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>
  • Loading branch information
findleyr committed Jan 3, 2023
1 parent 0441b43 commit 1e0dff2
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
9 changes: 8 additions & 1 deletion gopls/internal/regtest/watch/watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
10 changes: 4 additions & 6 deletions gopls/internal/regtest/workspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 1e0dff2

Please sign in to comment.