From d08f5dc9fbfc126ed1a4386b2ef120822334924f Mon Sep 17 00:00:00 2001 From: Robert Findley Date: Fri, 22 Jul 2022 17:55:31 -0400 Subject: [PATCH] gopls/internal/regtest: unskip all of TestModFileModification I believe the races described in the issue have been fixed: we should invalidate mod tidy results on any metadata change. If this invalidation doesn't work due to a race, we want to know about it. Update the test to wait for file-related events to complete before removing files, in an attempt to avoid windows file-locking issues. For golang/go#40269 For golang/go#53878 Change-Id: I91f0cb4969851010b34904a0b78ab9bd2808f92e Reviewed-on: https://go-review.googlesource.com/c/tools/+/420718 Run-TryBot: Robert Findley gopls-CI: kokoro TryBot-Result: Gopher Robot Reviewed-by: Suzy Mueller --- .../internal/regtest/modfile/modfile_test.go | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/gopls/internal/regtest/modfile/modfile_test.go b/gopls/internal/regtest/modfile/modfile_test.go index 79441c1fe7..91c8005be9 100644 --- a/gopls/internal/regtest/modfile/modfile_test.go +++ b/gopls/internal/regtest/modfile/modfile_test.go @@ -95,7 +95,10 @@ func main() { goModContent := env.ReadWorkspaceFile("a/go.mod") env.OpenFile("a/main.go") env.Await( - env.DiagnosticAtRegexp("a/main.go", "\"example.com/blah\""), + OnceMet( + env.DoneWithOpen(), + env.DiagnosticAtRegexp("a/main.go", "\"example.com/blah\""), + ), ) if got := env.ReadWorkspaceFile("a/go.mod"); got != goModContent { t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(t, goModContent, got)) @@ -114,26 +117,43 @@ func main() { // Reproduce golang/go#40269 by deleting and recreating main.go. t.Run("delete main.go", func(t *testing.T) { - t.Skip("This test will be flaky until golang/go#40269 is resolved.") - runner.Run(t, untidyModule, func(t *testing.T, env *Env) { goModContent := env.ReadWorkspaceFile("a/go.mod") mainContent := env.ReadWorkspaceFile("a/main.go") env.OpenFile("a/main.go") env.SaveBuffer("a/main.go") + // Ensure that we're done processing all the changes caused by opening + // and saving above. If not, we may run into a file locking issue on + // windows. + // + // If this proves insufficient, env.RemoveWorkspaceFile can be updated to + // retry file lock errors on windows. + env.Await( + env.DoneWithOpen(), + env.DoneWithSave(), + env.DoneWithChangeWatchedFiles(), + ) env.RemoveWorkspaceFile("a/main.go") + + // TODO(rfindley): awaiting here shouldn't really be necessary. We should + // be consistent eventually. + // + // Probably this was meant to exercise a race with the change below. env.Await( env.DoneWithOpen(), env.DoneWithSave(), env.DoneWithChangeWatchedFiles(), ) - env.WriteWorkspaceFile("main.go", mainContent) + env.WriteWorkspaceFile("a/main.go", mainContent) env.Await( - env.DiagnosticAtRegexp("main.go", "\"example.com/blah\""), + OnceMet( + env.DoneWithChangeWatchedFiles(), + env.DiagnosticAtRegexp("a/main.go", "\"example.com/blah\""), + ), ) - if got := env.ReadWorkspaceFile("go.mod"); got != goModContent { + if got := env.ReadWorkspaceFile("a/go.mod"); got != goModContent { t.Fatalf("go.mod changed on disk:\n%s", tests.Diff(t, goModContent, got)) } })