Skip to content

Commit

Permalink
test: Parallelize package tests in high-cost packages. (#7126)
Browse files Browse the repository at this point in the history 
* test: Parallelize package level tests in high-cost packages.

This commit adds `t.Parallel()` calls to the beginning of many tests
across several Go packages in OPA. The slowest packages (taking ~10s or
more) have been instrumented where possible as a proof-of-concept. On a
machine with many cores, the tests now will complete as fast as the
slowest test per package, instead of the sum of all the tests in a
particular package.

* server/server_test: Remove 3x tests from parallel set.

This commit fixes a data race that could occur in the `server` package
tests, because 3x tests were modifying package variables under
`internal/version`. These tests now run sequentially, and are not
included in the parallel test set.

* plugins/bundle/plugin_test: Remove 2x tests from the parallel set.

Two tests in this package modified a package variable directly, and as
such cannot be safely run in parallel with each other or any other tests
in the package.

* topdown/*_test: t.Parallel refactors.

This commit wraps up a large batch of fairly mechanical refactorings to
add t.Parallel() annotations to almost every test under `topdown`. The
tests that could not be safely parallelized now have explicit warning
comments on them describing why they are not safe to run in parallel.

* storage/disk: t.Parallel refactors.

This commit bundles up test parallelization changes for the
`storage/disk` package, dramatically reducing its execution time.

* topdown/net_test: Remove sub-test parallelization.

* rego: t.Parallel refactors.

This commit includes a bundle of t.Parallel refactoring changes for the
`rego` package, including a timer-related bugfix, and a slight change on
a cancellation test to reduce its overall cost during test runs (the
logic is preserved, but the mandatory timeouts are lower now).

* test: Fixes for sporadic test breakages.

---------

Signed-off-by: Philip Conrad <[email protected]>
  • Loading branch information
@philipaconrad
philipaconrad authored Nov 12, 2024
1 parent c0fe200 commit d2c0459
Show file tree
Hide file tree
Showing 47 changed files with 1,103 additions and 63 deletions.
70 changes: 69 additions & 1 deletion cmd/bench_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
// Minimize the number of tests that *actually* run the benchmarks, they are pretty slow.
// Have one test that exercises the whole flow.
func TestRunBenchmark(t *testing.T) {
t.Parallel()

params := testBenchParams()

args := []string{"1 + 1"}
Expand Down Expand Up @@ -61,6 +63,8 @@ func TestRunBenchmark(t *testing.T) {
}

func TestRunBenchmarkWithQueryImport(t *testing.T) {
t.Parallel()

params := testBenchParams()
// We add the rego.v1 import ..
params.imports = newrepeatedStringFlag([]string{"rego.v1"})
Expand Down Expand Up @@ -99,6 +103,8 @@ func TestRunBenchmarkWithQueryImport(t *testing.T) {
}

func TestRunBenchmarkE2E(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.e2e = true

Expand Down Expand Up @@ -143,6 +149,7 @@ func TestRunBenchmarkE2E(t *testing.T) {
}

func TestRunBenchmarkE2EWithOPAConfigFile(t *testing.T) {
t.Parallel()

fs := map[string]string{
"/config.yaml": `{"decision_logs": {"console": true}}`,
Expand Down Expand Up @@ -196,6 +203,8 @@ func TestRunBenchmarkE2EWithOPAConfigFile(t *testing.T) {
}

func TestRunBenchmarkFailFastE2E(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.fail = true // configured to fail on undefined results
params.e2e = true
Expand Down Expand Up @@ -225,6 +234,8 @@ func TestRunBenchmarkFailFastE2E(t *testing.T) {
}

func TestBenchPartialE2E(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.partial = true
params.fail = true
Expand Down Expand Up @@ -270,6 +281,8 @@ func TestBenchPartialE2E(t *testing.T) {
}

func TestRunBenchmarkPartialFailFastE2E(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.partial = true
params.unknowns = []string{}
Expand Down Expand Up @@ -300,10 +313,11 @@ func TestRunBenchmarkPartialFailFastE2E(t *testing.T) {
if actual != expected {
t.Fatalf("\nExpected:\n%s\n\nGot:\n%s\n", expected, actual)
}

}

func TestRunBenchmarkFailFast(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.fail = true // configured to fail on undefined results

Expand Down Expand Up @@ -345,6 +359,8 @@ func (r *mockBenchRunner) run(ctx context.Context, ectx *evalContext, params ben
}

func TestBenchPartial(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.partial = true
params.fail = true
Expand All @@ -362,6 +378,8 @@ func TestBenchPartial(t *testing.T) {
}

func TestBenchMainErrPreparing(t *testing.T) {
t.Parallel()

params := testBenchParams()
args := []string{"???"} // query compile error
var buf bytes.Buffer
Expand All @@ -377,6 +395,8 @@ func TestBenchMainErrPreparing(t *testing.T) {
}

func TestBenchMainErrRunningBenchmark(t *testing.T) {
t.Parallel()

params := testBenchParams()
args := []string{"1+1"}
var buf bytes.Buffer
Expand All @@ -397,6 +417,8 @@ func TestBenchMainErrRunningBenchmark(t *testing.T) {
}

func TestBenchMainWithCount(t *testing.T) {
t.Parallel()

params := testBenchParams()
args := []string{"1+1"}
var buf bytes.Buffer
Expand Down Expand Up @@ -425,6 +447,8 @@ func TestBenchMainWithCount(t *testing.T) {
}

func TestBenchMainWithNegativeCount(t *testing.T) {
t.Parallel()

params := testBenchParams()
args := []string{"1+1"}
var buf bytes.Buffer
Expand Down Expand Up @@ -489,6 +513,8 @@ func validateBenchMainPrep(t *testing.T, args []string, params benchmarkCommandP
}

func TestBenchMainWithJSONInputFile(t *testing.T) {
t.Parallel()

params := testBenchParams()
files := map[string]string{
"/input.json": `{"x": 42}`,
Expand All @@ -502,6 +528,8 @@ func TestBenchMainWithJSONInputFile(t *testing.T) {
}

func TestBenchMainWithYAMLInputFile(t *testing.T) {
t.Parallel()

params := testBenchParams()
files := map[string]string{
"/input.yaml": `x: 42`,
Expand All @@ -515,6 +543,8 @@ func TestBenchMainWithYAMLInputFile(t *testing.T) {
}

func TestBenchMainInvalidInputFile(t *testing.T) {
t.Parallel()

params := testBenchParams()
files := map[string]string{
"/input.yaml": `x: 42`,
Expand All @@ -536,6 +566,8 @@ func TestBenchMainInvalidInputFile(t *testing.T) {
}

func TestBenchMainWithJSONInputFileE2E(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.e2e = true
files := map[string]string{
Expand All @@ -559,6 +591,8 @@ func TestBenchMainWithJSONInputFileE2E(t *testing.T) {
}

func TestBenchMainWithYAMLInputFileE2E(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.e2e = true
files := map[string]string{
Expand All @@ -582,6 +616,8 @@ func TestBenchMainWithYAMLInputFileE2E(t *testing.T) {
}

func TestBenchMainInvalidInputFileE2E(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.e2e = true
files := map[string]string{
Expand All @@ -605,6 +641,8 @@ func TestBenchMainInvalidInputFileE2E(t *testing.T) {
}

func TestBenchMainWithBundleData(t *testing.T) {
t.Parallel()

params := testBenchParams()

b := testBundle()
Expand Down Expand Up @@ -637,6 +675,8 @@ func TestBenchMainWithBundleData(t *testing.T) {
}

func TestBenchMainWithBundleDataE2E(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.e2e = true

Expand Down Expand Up @@ -679,6 +719,8 @@ func TestBenchMainWithBundleDataE2E(t *testing.T) {
}

func TestBenchMainWithDataE2E(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.e2e = true

Expand Down Expand Up @@ -716,6 +758,8 @@ func TestBenchMainWithDataE2E(t *testing.T) {
}

func TestBenchMainBadQueryE2E(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.e2e = true
args := []string{"foo.bar"}
Expand All @@ -733,6 +777,8 @@ func TestBenchMainBadQueryE2E(t *testing.T) {
}

func TestBenchMainCompatibleFlags(t *testing.T) {
t.Parallel()

tests := []struct {
note string
v0Compatible bool
Expand Down Expand Up @@ -813,7 +859,10 @@ a[4] {

for _, mode := range modes {
for _, tc := range tests {
tc := tc // Reassignment prevents loop var scoping issue. (See: https://go.dev/blog/loopvar-preview)
t.Run(fmt.Sprintf("%s, %s", tc.note, mode.name), func(t *testing.T) {
t.Parallel()

files := map[string]string{
"mod.rego": tc.module,
}
Expand Down Expand Up @@ -863,6 +912,8 @@ a[4] {
}

func TestBenchMainWithBundleRegoVersion(t *testing.T) {
t.Parallel()

tests := []struct {
note string
bundleRegoVersion int
Expand Down Expand Up @@ -972,7 +1023,10 @@ a contains 4 if {
for _, bundleType := range bundleTypeCases {
for _, mode := range modes {
for _, tc := range tests {
tc := tc // Reassignment prevents loop var scoping issue. (See: https://go.dev/blog/loopvar-preview)
t.Run(fmt.Sprintf("%s, %s, %s", bundleType.note, tc.note, mode.name), func(t *testing.T) {
t.Parallel()

files := map[string]string{}

if bundleType.tar {
Expand Down Expand Up @@ -1062,6 +1116,8 @@ a contains 4 if {
}

func TestRenderBenchmarkResultJSONOutput(t *testing.T) {
t.Parallel()

params := testBenchParams()
err := params.outputFormat.Set(evalJSONOutput)
if err != nil {
Expand Down Expand Up @@ -1103,6 +1159,8 @@ func TestRenderBenchmarkResultJSONOutput(t *testing.T) {
}

func TestRenderBenchmarkResultPrettyOutput(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.benchMem = false
err := params.outputFormat.Set(evalPrettyOutput)
Expand Down Expand Up @@ -1140,6 +1198,8 @@ func TestRenderBenchmarkResultPrettyOutput(t *testing.T) {
}

func TestRenderBenchmarkResultPrettyOutputShowAllocs(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.benchMem = true
err := params.outputFormat.Set(evalPrettyOutput)
Expand Down Expand Up @@ -1179,6 +1239,8 @@ func TestRenderBenchmarkResultPrettyOutputShowAllocs(t *testing.T) {
}

func TestRenderBenchmarkResultGoBenchOutputShowAllocs(t *testing.T) {
t.Parallel()

params := testBenchParams()
params.benchMem = true
err := params.outputFormat.Set(benchmarkGoBenchOutput)
Expand All @@ -1203,6 +1265,8 @@ func TestRenderBenchmarkResultGoBenchOutputShowAllocs(t *testing.T) {
}

func TestRenderBenchmarkErrorJSONOutput(t *testing.T) {
t.Parallel()

params := testBenchParams()
err := params.outputFormat.Set(evalJSONOutput)
if err != nil {
Expand Down Expand Up @@ -1244,6 +1308,8 @@ func TestRenderBenchmarkErrorJSONOutput(t *testing.T) {
}

func TestRenderBenchmarkErrorPrettyOutput(t *testing.T) {
t.Parallel()

params := testBenchParams()
err := params.outputFormat.Set(evalPrettyOutput)
if err != nil {
Expand All @@ -1254,6 +1320,8 @@ func TestRenderBenchmarkErrorPrettyOutput(t *testing.T) {
}

func TestRenderBenchmarkErrorGoBenchOutput(t *testing.T) {
t.Parallel()

params := testBenchParams()
err := params.outputFormat.Set(benchmarkGoBenchOutput)
if err != nil {
Expand Down
Loading

0 comments on commit d2c0459

Please sign in to comment.