diff --git a/internal/lsp/race_off.go b/internal/lsp/race_off.go new file mode 100644 index 00000000..e7d4a886 --- /dev/null +++ b/internal/lsp/race_off.go @@ -0,0 +1,8 @@ +//go:build !race +// +build !race + +package lsp + +func isRaceEnabled() bool { + return false +} diff --git a/internal/lsp/race_on.go b/internal/lsp/race_on.go new file mode 100644 index 00000000..3d9ba5d0 --- /dev/null +++ b/internal/lsp/race_on.go @@ -0,0 +1,8 @@ +//go:build race +// +build race + +package lsp + +func isRaceEnabled() bool { + return true +} diff --git a/internal/lsp/server.go b/internal/lsp/server.go index 0d605980..7bb0ef45 100644 --- a/internal/lsp/server.go +++ b/internal/lsp/server.go @@ -237,7 +237,6 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { case <-ctx.Done(): return case job := <-l.lintFileJobs: - fmt.Fprintln(l.errorLog, "file", filepath.Base(job.URI), job.Reason) bis := l.builtinsForCurrentCapabilities() // updateParse will not return an error when the parsing failed, @@ -321,7 +320,6 @@ func (l *LanguageServer) StartDiagnosticsWorker(ctx context.Context) { case <-ctx.Done(): return case job := <-workspaceLintRuns: - fmt.Fprintln(l.errorLog, "workspace", job.Reason) err := updateAllDiagnostics( ctx, l.cache, diff --git a/internal/lsp/server_aggregates_test.go b/internal/lsp/server_aggregates_test.go index f1c6bfde..bb6fba84 100644 --- a/internal/lsp/server_aggregates_test.go +++ b/internal/lsp/server_aggregates_test.go @@ -76,18 +76,12 @@ import rego.v1 tempDir := t.TempDir() - ls, connClient, err := createAndInitServer(ctx, newTestLogger(t), tempDir, files, clientHandler) + _, connClient, err := createAndInitServer(ctx, newTestLogger(t), tempDir, files, clientHandler) if err != nil { t.Fatalf("failed to create and init language server: %s", err) } - for f := range ls.cache.GetAllFiles() { - t.Log("file loaded: ", f) - } - - t.Log("queue", len(ls.lintWorkspaceJobs)) - - timeout := time.NewTimer(defaultTimeout) + timeout := time.NewTimer(determineTimeout()) defer timeout.Stop() // no unresolved-imports at this stage @@ -111,22 +105,18 @@ import rego.v1 } } - t.Log("queue", len(ls.lintWorkspaceJobs)) - barURI := fileURIScheme + filepath.Join(tempDir, "bar.rego") - contentUpdate1 := `package qux - -import rego.v1 -` - err = connClient.Call(ctx, "textDocument/didChange", types.TextDocumentDidChangeParams{ TextDocument: types.TextDocumentIdentifier{ URI: barURI, }, ContentChanges: []types.TextDocumentContentChangeEvent{ { - Text: contentUpdate1, + Text: `package qux + +import rego.v1 +`, }, }, }, nil) @@ -134,42 +124,8 @@ import rego.v1 t.Fatalf("failed to send didChange notification: %s", err) } - t.Log("queue", len(ls.lintWorkspaceJobs)) - - // wait for content to have been updated - timeout.Reset(defaultTimeout) - - ticker := time.NewTicker(500 * time.Millisecond) - - for { - var success bool - select { - case <-ticker.C: - contents, ok := ls.cache.GetFileContents(barURI) - if !ok { - t.Log("waiting, bar.rego contents missing") - - continue - } - - if contents != contentUpdate1 { - t.Log("waiting, bar.rego contents not yet updated") - - continue - } - - success = true - case <-timeout.C: - t.Fatalf("timed out waiting bar.rego content to have been updated") - } - - if success { - break - } - } - // unresolved-imports is now expected - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool @@ -193,21 +149,19 @@ import rego.v1 fooURI := fileURIScheme + filepath.Join(tempDir, "foo.rego") - contentUpdate2 := `package foo - -import rego.v1 - -import data.baz -import data.qux # new name for bar.rego package -` - err = connClient.Call(ctx, "textDocument/didChange", types.TextDocumentDidChangeParams{ TextDocument: types.TextDocumentIdentifier{ URI: fooURI, }, ContentChanges: []types.TextDocumentContentChangeEvent{ { - Text: contentUpdate2, + Text: `package foo + +import rego.v1 + +import data.baz +import data.qux # new name for bar.rego package +`, }, }, }, nil) @@ -215,38 +169,8 @@ import data.qux # new name for bar.rego package t.Fatalf("failed to send didChange notification: %s", err) } - // wait for content to have been updated - timeout.Reset(defaultTimeout) - - for { - var success bool - select { - case <-ticker.C: - contents, ok := ls.cache.GetFileContents(fooURI) - if !ok { - t.Log("waiting, foo.rego contents missing") - - continue - } - - if contents != contentUpdate2 { - t.Log("waiting, foo.rego contents not yet updated") - - continue - } - - success = true - case <-timeout.C: - t.Fatalf("timed out waiting foo.rego content to have been updated") - } - - if success { - break - } - } - // unresolved-imports is again not expected - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool @@ -309,7 +233,7 @@ import data.quz } // 1. check the Aggregates are set at start up - timeout := time.NewTimer(defaultTimeout) + timeout := time.NewTimer(determineTimeout()) ticker := time.NewTicker(500 * time.Millisecond) defer ticker.Stop() @@ -398,7 +322,7 @@ import data.wow # new t.Fatalf("failed to send didChange notification: %s", err) } - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { success := false @@ -490,7 +414,7 @@ import rego.v1 } // wait for foo.rego to have the correct violations - timeout := time.NewTimer(defaultTimeout) + timeout := time.NewTimer(determineTimeout()) defer timeout.Stop() for { @@ -515,10 +439,6 @@ import rego.v1 // update the contents of the bar.rego file to address the unresolved-import barURI := fileURIScheme + filepath.Join(tempDir, "bar.rego") - contentUpdate1 := `package bax # package imported in foo.rego - -import rego.v1 -` err = connClient.Call(ctx, "textDocument/didChange", types.TextDocumentDidChangeParams{ TextDocument: types.TextDocumentIdentifier{ @@ -526,7 +446,10 @@ import rego.v1 }, ContentChanges: []types.TextDocumentContentChangeEvent{ { - Text: contentUpdate1, + Text: `package bax # package imported in foo.rego + +import rego.v1 +`, }, }, }, nil) @@ -534,40 +457,8 @@ import rego.v1 t.Fatalf("failed to send didChange notification: %s", err) } - // wait for bar.rego to have the correct content - timeout.Reset(defaultTimeout) - - ticker := time.NewTicker(500 * time.Millisecond) - - for { - var success bool - select { - case <-ticker.C: - contents, ok := ls.cache.GetFileContents(barURI) - if !ok { - t.Log("waiting, bar.rego contents missing") - - continue - } - - if contents != contentUpdate1 { - t.Log("waiting, bar.rego contents not yet updated") - - continue - } - - success = true - case <-timeout.C: - t.Fatalf("timed out waiting bar.rego content to have been updated") - } - - if success { - break - } - } - // wait for foo.rego to have the correct violations - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool @@ -589,11 +480,6 @@ import rego.v1 } } - contentUpdate2 := `package bar # original package to bring back the violation - -import rego.v1 -` - // update the contents of the bar.rego to bring back the violation err = connClient.Call(ctx, "textDocument/didChange", types.TextDocumentDidChangeParams{ TextDocument: types.TextDocumentIdentifier{ @@ -601,7 +487,10 @@ import rego.v1 }, ContentChanges: []types.TextDocumentContentChangeEvent{ { - Text: contentUpdate2, + Text: `package bar # original package to bring back the violation + +import rego.v1 +`, }, }, }, nil) @@ -609,38 +498,8 @@ import rego.v1 t.Fatalf("failed to send didChange notification: %s", err) } - // wait for bar.rego to have the correct content - timeout.Reset(defaultTimeout) - - for { - var success bool - select { - case <-ticker.C: - contents, ok := ls.cache.GetFileContents(barURI) - if !ok { - t.Log("waiting, bar.rego contents missing") - - continue - } - - if contents != contentUpdate2 { - t.Log("waiting, bar.rego contents not yet updated") - - continue - } - - success = true - case <-timeout.C: - t.Fatalf("timed out waiting bar.rego content to have been updated") - } - - if success { - break - } - } - // check the violation is back - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool diff --git a/internal/lsp/server_config_test.go b/internal/lsp/server_config_test.go index 34a847ea..4051ea4b 100644 --- a/internal/lsp/server_config_test.go +++ b/internal/lsp/server_config_test.go @@ -83,7 +83,7 @@ allow := true t.Fatalf("expected client root URI to be %s, got %s", exp, got) } - timeout := time.NewTimer(defaultTimeout) + timeout := time.NewTimer(determineTimeout()) defer timeout.Stop() for { @@ -117,7 +117,7 @@ allow := true } // validate that the client received a new, empty diagnostics notification for the file - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool diff --git a/internal/lsp/server_multi_file_test.go b/internal/lsp/server_multi_file_test.go index d5c81339..00bcedd1 100644 --- a/internal/lsp/server_multi_file_test.go +++ b/internal/lsp/server_multi_file_test.go @@ -99,7 +99,7 @@ ignore: } // validate that the client received a diagnostics notification for authz.rego - timeout := time.NewTimer(defaultTimeout) + timeout := time.NewTimer(determineTimeout()) defer timeout.Stop() for { @@ -123,7 +123,7 @@ ignore: } // validate that the client received a diagnostics notification admins.rego - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool @@ -173,7 +173,7 @@ allow if input.user in admins.users } // authz.rego should now have no violations - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool diff --git a/internal/lsp/server_single_file_test.go b/internal/lsp/server_single_file_test.go index 98258f90..c79ba849 100644 --- a/internal/lsp/server_single_file_test.go +++ b/internal/lsp/server_single_file_test.go @@ -81,7 +81,7 @@ rules: } // validate that the client received a diagnostics notification for the file - timeout := time.NewTimer(defaultTimeout) + timeout := time.NewTimer(determineTimeout()) defer timeout.Stop() for { @@ -117,7 +117,7 @@ allow := true } // validate that the client received a new diagnostics notification for the file - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool @@ -149,7 +149,7 @@ rules: } // validate that the client received a new, empty diagnostics notification for the file - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool @@ -197,7 +197,7 @@ capabilities: } // validate that the client received a new, empty diagnostics notification for the file - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool @@ -250,7 +250,7 @@ allow := neo4j.q } // validate that the client received a new diagnostics notification for the file - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) for { var success bool @@ -282,7 +282,7 @@ allow := neo4j.q // LSP for a completion. We expect to see neo4j.query show up. Since // neo4j.query is an EOPA-specific builtin, it should never appear if // we're using the normal OPA capabilities file. - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) ticker := time.NewTicker(500 * time.Millisecond) defer ticker.Stop() diff --git a/internal/lsp/server_template_test.go b/internal/lsp/server_template_test.go index 22d24d09..bd9bd24f 100644 --- a/internal/lsp/server_template_test.go +++ b/internal/lsp/server_template_test.go @@ -163,7 +163,7 @@ func TestNewFileTemplating(t *testing.T) { go ls.StartTemplateWorker(ctx) // wait for the server to load it's config - timeout := time.NewTimer(defaultTimeout) + timeout := time.NewTimer(determineTimeout()) select { case <-timeout.C: t.Fatalf("timed out waiting for server to load config") @@ -200,7 +200,7 @@ func TestNewFileTemplating(t *testing.T) { } // Validate that the client received a workspace edit - timeout.Reset(defaultTimeout) + timeout.Reset(determineTimeout()) expectedMessage := fmt.Sprintf(`{ "edit": { diff --git a/internal/lsp/server_test.go b/internal/lsp/server_test.go index 7170f836..82fd2f7f 100644 --- a/internal/lsp/server_test.go +++ b/internal/lsp/server_test.go @@ -29,6 +29,19 @@ const defaultTimeout = 20 * time.Second const defaultBufferedChannelSize = 5 +// determineTimeout returns a timeout duration based on whether +// the test suite is running with race detection, if so, a more permissive +// timeout is used. +func determineTimeout() time.Duration { + if isRaceEnabled() { + // based on the upper bound here, 20x slower + // https://go.dev/doc/articles/race_detector#Runtime_Overheads + return defaultTimeout * 20 + } + + return defaultTimeout +} + const fileURIScheme = "file://" // NewTestLogger returns an io.Writer that logs to the given testing.T.