Skip to content

Commit

Permalink
gopls/internal/regtest: simplify OnceMet expressions with an env helper
Browse files Browse the repository at this point in the history
Simplify env.Await(OnceMet(...)) to env.OnceMet(...). Aside from
avoiding boilerplate, this makes it easier to identify where we're still
using Await.

Updates golang/go#39384

Change-Id: I57a18242ce6b48e371e5ce4876ef01a6774fe15c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/461917
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
  • Loading branch information
findleyr committed Jan 13, 2023
1 parent ab7b5b2 commit 672a036
Show file tree
Hide file tree
Showing 20 changed files with 263 additions and 457 deletions.
12 changes: 12 additions & 0 deletions gopls/internal/lsp/regtest/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,13 +302,25 @@ func checkExpectations(s State, expectations []Expectation) (Verdict, string) {
return finalVerdict, summary.String()
}

// Await blocks until the given expectations are all simultaneously met.
//
// Generally speaking Await should be avoided because it can block indefinitely
// if gopls ends up in a state where the expectations are never going to be
// met. Use AfterChange or OnceMet instead, so that the runner knows when to
// stop waiting.
func (e *Env) Await(expectations ...Expectation) {
e.T.Helper()
if err := e.Awaiter.Await(e.Ctx, expectations...); err != nil {
e.T.Fatal(err)
}
}

// OnceMet blocks until precondition is met or unmeetable; if the precondition
// is met, it atomically checks that all expectations in mustMeets are met.
func (e *Env) OnceMet(precondition Expectation, mustMeets ...Expectation) {
e.Await(OnceMet(precondition, mustMeets...))
}

// Await waits for all expectations to simultaneously be met. It should only be
// called from the main test goroutine.
func (a *Awaiter) Await(ctx context.Context, expectations ...Expectation) error {
Expand Down
8 changes: 3 additions & 5 deletions gopls/internal/lsp/regtest/expectation.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,11 +291,9 @@ func (e *Env) DoneDiagnosingChanges() Expectation {
// expectations.
func (e *Env) AfterChange(expectations ...Expectation) {
e.T.Helper()
e.Await(
OnceMet(
e.DoneDiagnosingChanges(),
expectations...,
),
e.OnceMet(
e.DoneDiagnosingChanges(),
expectations...,
)
}

Expand Down
18 changes: 8 additions & 10 deletions gopls/internal/regtest/codelens/codelens_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,16 +208,14 @@ require golang.org/x/hello v1.2.3
env.OpenFile("b/go.mod")
env.ExecuteCodeLensCommand("a/go.mod", command.CheckUpgrades, nil)
d := &protocol.PublishDiagnosticsParams{}
env.Await(
OnceMet(
env.DiagnosticAtRegexpWithMessage("a/go.mod", `require`, "can be upgraded"),
ReadDiagnostics("a/go.mod", d),
// We do not want there to be a diagnostic for b/go.mod,
// but there may be some subtlety in timing here, where this
// should always succeed, but may not actually test the correct
// behavior.
NoMatchingDiagnostics(env.AtRegexp("b/go.mod", `require`)),
),
env.OnceMet(
env.DiagnosticAtRegexpWithMessage("a/go.mod", `require`, "can be upgraded"),
ReadDiagnostics("a/go.mod", d),
// We do not want there to be a diagnostic for b/go.mod,
// but there may be some subtlety in timing here, where this
// should always succeed, but may not actually test the correct
// behavior.
NoMatchingDiagnostics(env.AtRegexp("b/go.mod", `require`)),
)
// Check for upgrades in b/go.mod and then clear them.
env.ExecuteCodeLensCommand("b/go.mod", command.CheckUpgrades, nil)
Expand Down
8 changes: 3 additions & 5 deletions gopls/internal/regtest/codelens/gcdetails_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,9 @@ func main() {
env.OpenFile("main.go")
env.ExecuteCodeLensCommand("main.go", command.GCDetails, nil)
d := &protocol.PublishDiagnosticsParams{}
env.Await(
OnceMet(
DiagnosticAt("main.go", 5, 13),
ReadDiagnostics("main.go", d),
),
env.OnceMet(
DiagnosticAt("main.go", 5, 13),
ReadDiagnostics("main.go", d),
)
// Confirm that the diagnostics come from the gc details code lens.
var found bool
Expand Down
5 changes: 1 addition & 4 deletions gopls/internal/regtest/diagnostics/builtin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ const (
if !strings.HasSuffix(name, "builtin.go") {
t.Fatalf("jumped to %q, want builtin.go", name)
}
env.Await(OnceMet(
env.DoneWithOpen(),
NoDiagnostics("builtin.go"),
))
env.AfterChange(NoDiagnostics("builtin.go"))
})
}
Loading

0 comments on commit 672a036

Please sign in to comment.