gopls/internal/lsp/regtest: delete TestWatchReplaceTargets

We don't actually watch replace targets anymore. The way to specify if a
module is being used is by including it in a go.work file.

Looking back on the flakiness, I am pretty sure it was due simply to
type-checking on slow builders, back when we limited each regtest to
20s. This module imports some standard library packages that used to be
slow to type check. I am pretty sure this test would no longer be flaky,
if we still supported the functionality.

While porting over the assertions from this test to TestUseGoWork, I
discovered golang/go#60340, a bug in the order of our file watcher
evaluation.

Fixes golang/go#50748

Change-Id: I26c10ac659d0f195da18b6181b54d7c373cc984b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/496879
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

gopls/internal/lsp/regtest/expectation.go

@@ -576,50 +576,6 @@ func jsonProperty(obj interface{}, path ...string) interface{} {
 	return jsonProperty(m[path[0]], path[1:]...)
 }
 
-// RegistrationMatching asserts that the client has received a capability
-// registration matching the given regexp.
-//
-// TODO(rfindley): remove this once TestWatchReplaceTargets has been revisited.
-//
-// Deprecated: use (No)FileWatchMatching
-func RegistrationMatching(re string) Expectation {
-	rec := regexp.MustCompile(re)
-	check := func(s State) Verdict {
-		for _, p := range s.registrations {
-			for _, r := range p.Registrations {
-				if rec.Match([]byte(r.Method)) {
-					return Met
-				}
-			}
-		}
-		return Unmet
-	}
-	return Expectation{
-		Check:       check,
-		Description: fmt.Sprintf("registration matching %q", re),
-	}
-}
-
-// UnregistrationMatching asserts that the client has received an
-// unregistration whose ID matches the given regexp.
-func UnregistrationMatching(re string) Expectation {
-	rec := regexp.MustCompile(re)
-	check := func(s State) Verdict {
-		for _, p := range s.unregistrations {
-			for _, r := range p.Unregisterations {
-				if rec.Match([]byte(r.Method)) {
-					return Met
-				}
-			}
-		}
-		return Unmet
-	}
-	return Expectation{
-		Check:       check,
-		Description: fmt.Sprintf("unregistration matching %q", re),
-	}
-}
-
 // Diagnostics asserts that there is at least one diagnostic matching the given
 // filters.
 func Diagnostics(filters ...DiagnosticFilter) Expectation {

gopls/internal/regtest/workspace/workspace_test.go

@@ -183,29 +183,6 @@ replace random.org => %s
 	})
 }
 
-// This test checks that gopls updates the set of files it watches when a
-// replace target is added to the go.mod.
-func TestWatchReplaceTargets(t *testing.T) {
-	t.Skipf("skipping known-flaky test: see https://go.dev/issue/50748")
-
-	WithOptions(
-		ProxyFiles(workspaceProxy),
-		WorkspaceFolders("pkg"),
-	).Run(t, workspaceModule, func(t *testing.T, env *Env) {
-		// Add a replace directive and expect the files that gopls is watching
-		// to change.
-		dir := env.Sandbox.Workdir.URI("goodbye").SpanURI().Filename()
-		goModWithReplace := fmt.Sprintf(`%s
-replace random.org => %s
-`, env.ReadWorkspaceFile("pkg/go.mod"), dir)
-		env.WriteWorkspaceFile("pkg/go.mod", goModWithReplace)
-		env.AfterChange(
-			UnregistrationMatching("didChangeWatchedFiles"),
-			RegistrationMatching("didChangeWatchedFiles"),
-		)
-	})
-}
-
 const workspaceModuleProxy = `
 -- example.com@v1.2.3/go.mod --
 module example.com
@@ -575,10 +552,18 @@ use (
 `
 	WithOptions(
 		ProxyFiles(workspaceModuleProxy),
+		Settings{
+			"subdirWatchPatterns": "on",
+		},
 	).Run(t, multiModule, func(t *testing.T, env *Env) {
-		// Initially, the go.work should cause only the a.com module to be
-		// loaded. Validate this by jumping to a definition in b.com and ensuring
-		// that we go to the module cache.
+		// Initially, the go.work should cause only the a.com module to be loaded,
+		// so we shouldn't get any file watches for modb. Further validate this by
+		// jumping to a definition in b.com and ensuring that we go to the module
+		// cache.
+		env.OnceMet(
+			InitialWorkspaceLoad,
+			NoFileWatchMatching("modb"),
+		)
 		env.OpenFile("moda/a/a.go")
 		env.Await(env.DoneWithOpen())
 
@@ -610,9 +595,13 @@ use (
 `)
 
 		// As of golang/go#54069, writing go.work to the workspace triggers a
-		// workspace reload.
+		// workspace reload, and new file watches.
 		env.AfterChange(
 			Diagnostics(env.AtRegexp("modb/b/b.go", "x")),
+			// TODO(golang/go#60340): we don't get a file watch yet, because
+			// updateWatchedDirectories runs before snapshot.load. Instead, we get it
+			// after the next change (the didOpen below).
+			// FileWatchMatching("modb"),
 		)
 
 		// Jumping to definition should now go to b.com in the workspace.
@@ -623,7 +612,13 @@ use (
 		// Now, let's modify the go.work *overlay* (not on disk), and verify that
 		// this change is only picked up once it is saved.
 		env.OpenFile("go.work")
-		env.AfterChange()
+		env.AfterChange(
+			// TODO(golang/go#60340): delete this expectation in favor of
+			// the commented-out expectation above, once we fix the evaluation order
+			// of file watches. We should not have to wait for a second change to get
+			// the correct watches.
+			FileWatchMatching("modb"),
+		)
 		env.SetBufferContent("go.work", `go 1.17
 
 use (