Skip to content

Commit 53826a8

Browse files
findleyrgopherbot
authored andcommitted
internal/test/integration/workspace: fix TestUseGoWork
Fix a tricky race in TestUseGoWork, where a view recalculation causes diagnostics in modb/go.mod to be cleared. Prior to CL 675016, we'd expect a diagnostic in modb/go.mod because it had an open file, and so zero config would cause it to be diagnosed. Subsequent to CL 675016, there is no file, so the diagnostic should be cleared, but because we *weren't* guarding the assertion in an 'AfterChange' (for obsolete historical reasons, I think), the 'Await' expression was immediately satisfied except in the rare cases when the rediagnosis won the race. This is one of the reasons why an unguarded 'Await' is almost always problematic, not least because its failure mode is to just hang. Invert the sense of the assertion, and guard the problematic await (and others) in this test. Fixes golang/go#74165 Change-Id: Ie5b9ab6b87ded41afe68830136196ef3588422a7 Reviewed-on: https://go-review.googlesource.com/c/tools/+/684656 LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Robert Findley <[email protected]> Reviewed-by: Alan Donovan <[email protected]>
1 parent 3b3100c commit 53826a8

File tree

1 file changed

+5
-8
lines changed

1 file changed

+5
-8
lines changed

gopls/internal/test/integration/workspace/workspace_test.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -645,7 +645,7 @@ use (
645645
//
646646
// TODO: should editing the go.work above cause modb diagnostics to be
647647
// suppressed?
648-
env.Await(env.DoneWithChange())
648+
env.AfterChange()
649649
if err := checkHelloLocation("modb/b/b.go"); err != nil {
650650
t.Fatal(err)
651651
}
@@ -656,12 +656,9 @@ use (
656656
t.Fatal(err)
657657
}
658658

659-
// This fails if guarded with a OnceMet(DoneWithSave(), ...), because it is
660-
// delayed (and therefore not synchronous with the change).
661-
//
662-
// Note: this check used to assert on NoDiagnostics, but with zero-config
663-
// gopls we still have diagnostics.
664-
env.Await(Diagnostics(ForFile("modb/go.mod"), WithMessage("example.com is not used")))
659+
// Since no file in modb is open, there should be no view containing
660+
// modb/go.mod, and we should clear its diagnostics.
661+
env.AfterChange(NoDiagnostics(ForFile("modb/go.mod")))
665662

666663
// Test Formatting.
667664
env.SetBufferContent("go.work", `go 1.18
@@ -673,7 +670,7 @@ use (
673670
)
674671
`) // TODO(matloob): For some reason there's a "start position 7:0 is out of bounds" error when the ")" is on the last character/line in the file. Rob probably knows what's going on.
675672
env.SaveBuffer("go.work")
676-
env.Await(env.DoneWithSave())
673+
env.AfterChange()
677674
gotWorkContents := env.ReadWorkspaceFile("go.work")
678675
wantWorkContents := `go 1.18
679676

0 commit comments

Comments
 (0)