Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

internal/lsp/*_test.go: replace for+success pattern #1248

Merged
merged 1 commit into from
Nov 8, 2024
Merged

internal/lsp/*_test.go: replace for+success pattern #1248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 8 additions & 50 deletions internal/lsp/server_aggregates_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ import rego.v1
defer timeout.Stop()

// no unresolved-imports at this stage
for {
var success bool
for success := false; !success; {
select {
case violations := <-messages["foo.rego"]:
if slices.Contains(violations, "unresolved-import") {
Expand All @@ -71,10 +70,6 @@ import rego.v1
case <-timeout.C:
t.Fatalf("timed out waiting for expected foo.rego diagnostics")
}

if success {
break
}
}

barURI := fileURIScheme + filepath.Join(tempDir, "bar.rego")
Expand All @@ -99,8 +94,7 @@ import rego.v1
// unresolved-imports is now expected
timeout.Reset(determineTimeout())

for {
var success bool
for success := false; !success; {
select {
case violations := <-messages["foo.rego"]:
if !slices.Contains(violations, "unresolved-import") {
Expand All @@ -113,10 +107,6 @@ import rego.v1
case <-timeout.C:
t.Fatalf("timed out waiting for expected foo.rego diagnostics")
}

if success {
break
}
}

fooURI := fileURIScheme + filepath.Join(tempDir, "foo.rego")
Expand Down Expand Up @@ -144,8 +134,7 @@ import data.qux # new name for bar.rego package
// unresolved-imports is again not expected
timeout.Reset(determineTimeout())

for {
var success bool
for success := false; !success; {
select {
case violations := <-messages["foo.rego"]:
if slices.Contains(violations, "unresolved-import") {
Expand All @@ -158,10 +147,6 @@ import data.qux # new name for bar.rego package
case <-timeout.C:
t.Fatalf("timed out waiting for expected foo.rego diagnostics")
}

if success {
break
}
}
}

Expand Down Expand Up @@ -206,9 +191,7 @@ import data.quz
ticker := time.NewTicker(500 * time.Millisecond)
defer ticker.Stop()

for {
success := false

for success := false; !success; {
select {
case <-ticker.C:
aggs := ls.cache.GetFileAggregates()
Expand All @@ -222,10 +205,6 @@ import data.quz
case <-timeout.C:
t.Fatalf("timed out waiting for file aggregates to be set")
}

if success {
break
}
}

determineImports := func(aggs map[string][]report.Aggregate) []string {
Expand Down Expand Up @@ -292,9 +271,7 @@ import data.wow # new

timeout.Reset(determineTimeout())

for {
success := false

for success := false; !success; {
select {
case <-ticker.C:
imports = determineImports(ls.cache.GetFileAggregates())
Expand All @@ -309,10 +286,6 @@ import data.wow # new
case <-timeout.C:
t.Fatalf("timed out waiting for file aggregates to be set")
}

if success {
break
}
}
}

Expand Down Expand Up @@ -358,8 +331,7 @@ import rego.v1
timeout := time.NewTimer(determineTimeout())
defer timeout.Stop()

for {
var success bool
for success := true; !success; {
select {
case violations := <-messages["foo.rego"]:
if !slices.Contains(violations, "unresolved-import") {
Expand All @@ -378,10 +350,6 @@ import rego.v1
case <-timeout.C:
t.Fatalf("timed out waiting for foo.rego diagnostics")
}

if success {
break
}
}

// update the contents of the bar.rego file to address the unresolved-import
Expand All @@ -407,8 +375,7 @@ import rego.v1
// wait for foo.rego to have the correct violations
timeout.Reset(determineTimeout())

for {
var success bool
for success := false; !success; {
select {
case violations := <-messages["foo.rego"]:
if slices.Contains(violations, "unresolved-import") {
Expand All @@ -427,10 +394,6 @@ import rego.v1
case <-timeout.C:
t.Fatalf("timed out waiting for foo.rego diagnostics")
}

if success {
break
}
}

// update the contents of the bar.rego to bring back the violation
Expand All @@ -454,8 +417,7 @@ import rego.v1
// check the violation is back
timeout.Reset(determineTimeout())

for {
var success bool
for success := false; !success; {
select {
case violations := <-messages["foo.rego"]:
if !slices.Contains(violations, "unresolved-import") {
Expand All @@ -474,9 +436,5 @@ import rego.v1
case <-timeout.C:
t.Fatalf("timed out waiting for foo.rego diagnostics")
}

if success {
break
}
}
}
35 changes: 5 additions & 30 deletions internal/lsp/server_config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,18 +87,13 @@ allow := true
timeout := time.NewTimer(determineTimeout())
defer timeout.Stop()

for {
var success bool
for success := false; !success; {
select {
case requestData := <-receivedMessages:
success = testRequestDataCodes(t, requestData, mainRegoFileURI, []string{"opa-fmt"})
case <-timeout.C:
t.Fatalf("timed out waiting for file diagnostics to be sent")
}

if success {
break
}
}

// User updates config file contents in parent directory that is not
Expand All @@ -120,18 +115,13 @@ allow := true
// validate that the client received a new, empty diagnostics notification for the file
timeout.Reset(determineTimeout())

for {
var success bool
for success := false; !success; {
select {
case requestData := <-receivedMessages:
success = testRequestDataCodes(t, requestData, mainRegoFileURI, []string{})
case <-timeout.C:
t.Fatalf("timed out waiting for file diagnostics to be sent")
}

if success {
break
}
}
}

Expand Down Expand Up @@ -164,8 +154,7 @@ func TestLanguageServerCachesEnabledRulesAndUsesDefaultConfig(t *testing.T) {
timeout := time.NewTimer(3 * time.Second)
ticker := time.NewTicker(500 * time.Millisecond)

for {
var success bool
for success := false; !success; {
select {
case <-ticker.C:
enabledRules := ls.getEnabledNonAggregateRules()
Expand All @@ -181,10 +170,6 @@ func TestLanguageServerCachesEnabledRulesAndUsesDefaultConfig(t *testing.T) {
case <-timeout.C:
t.Fatalf("timed out waiting for enabled rules to be correct")
}

if success {
break
}
}

err = os.MkdirAll(filepath.Join(tempDir, ".regal"), 0o755)
Expand Down Expand Up @@ -221,8 +206,7 @@ rules:

timeout.Reset(determineTimeout())

for {
var success bool
for success := false; !success; {
select {
case <-ticker.C:
enabledRules := ls.getEnabledNonAggregateRules()
Expand All @@ -244,10 +228,6 @@ rules:
case <-timeout.C:
t.Fatalf("timed out waiting for enabled rules to be correct")
}

if success {
break
}
}

configContents2 := `
Expand All @@ -270,8 +250,7 @@ rules:

timeout.Reset(determineTimeout())

for {
var success bool
for success := false; !success; {
select {
case <-ticker.C:
enabledRules := ls.getEnabledNonAggregateRules()
Expand Down Expand Up @@ -299,9 +278,5 @@ rules:
case <-timeout.C:
t.Fatalf("timed out waiting for enabled rules to be correct")
}

if success {
break
}
}
}
30 changes: 6 additions & 24 deletions internal/lsp/server_multi_file_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,32 +74,28 @@ ignore:

// wait for the aggregate data to be set, required for correct lint in next
// step
for {
var success bool
for success := false; !success; {
select {
default:
uri := "file://" + filepath.Join(tempDir, "admins.rego")

aggs := ls.cache.GetFileAggregates(uri)
if len(aggs) > 0 {
success = true

break // don't sleep
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's the only non-automatic change here

}

time.Sleep(500 * time.Millisecond)
case <-timeout.C:
t.Fatalf("timed out waiting admin aggregates to be set")
}

if success {
break
}
}

timeout.Reset(determineTimeout())

// validate that the client received a diagnostics notification for authz.rego
for {
var success bool
for success := false; !success; {
select {
case violations := <-messages["authz.rego"]:
if !slices.Contains(violations, "prefer-package-imports") {
Expand All @@ -112,17 +108,12 @@ ignore:
case <-timeout.C:
t.Fatalf("timed out waiting for authz.rego diagnostics to be sent")
}

if success {
break
}
}

// validate that the client received a diagnostics notification for admins.rego
timeout.Reset(determineTimeout())

for {
var success bool
for success := false; !success; {
select {
case violations := <-messages["admins.rego"]:
if !slices.Contains(violations, "use-assignment-operator") {
Expand All @@ -135,10 +126,6 @@ ignore:
case <-timeout.C:
t.Fatalf("timed out waiting for admins.rego diagnostics to be sent")
}

if success {
break
}
}

// 3. Client sends textDocument/didChange notification with new contents
Expand Down Expand Up @@ -171,8 +158,7 @@ allow if input.user in admins.users
// authz.rego should now have no violations
timeout.Reset(determineTimeout())

for {
var success bool
for success := false; !success; {
select {
case violations := <-messages["authz.rego"]:
if len(violations) > 0 {
Expand All @@ -185,9 +171,5 @@ allow if input.user in admins.users
case <-timeout.C:
t.Fatalf("timed out waiting for authz.rego diagnostics to be sent")
}

if success {
break
}
}
}
Loading