Skip to content

Commit ecdbb7a

Browse files
authored
feat: Parallel tests with global logger (google#1798)
Proof of concept of Parallel testing with a global logger via black magic (inspecting stack traces). Changes in this PR: - Refactor `cmdlogger.CmdLogger` to be an interface. - Move `cmdlogger.CmdLogger` implementation to be `cmdlogger.Handler` - Create a new logging package caled `testlogger`, and an implementation of `cmdlogger.CmdLogger` called `testlogger.Handler` - `testlogger.Handler` passes all logging queries to a map of underlying `cmdlogger.CmdLogger`s, with the key being the memory address value of `testing.T` passed into each test. - I believe this address should be unique for the lifespan of each test, even if after the test the same memory address gets reused. - Move static function into `static.go` This results in much faster test runtimes again. On my local machine, it takes the time taken down from over 5 minutes to <30 seconds. Obviously this is not the most stable approach, as internal function names and variables can change any time go is updated, but since this is only used in tests, and breakages would be quite loud, I believe this is acceptable.
1 parent 154d178 commit ecdbb7a

File tree

15 files changed

+314
-115
lines changed

15 files changed

+314
-115
lines changed

cmd/osv-reporter/main.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func splitLastArg(args []string) []string {
3737
func run(args []string, stdout, stderr io.Writer) int {
3838
logger := cmdlogger.New(stdout, stderr)
3939

40-
slog.SetDefault(slog.New(&logger))
40+
slog.SetDefault(slog.New(logger))
4141

4242
// Allow multiple arguments to be defined by github actions by splitting the last argument
4343
// by new lines.

cmd/osv-scanner/fix/command_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ func matchFile(t *testing.T, file string) {
2222
}
2323

2424
func TestCommand(t *testing.T) {
25+
t.Parallel()
26+
2527
tests := []struct {
2628
name string
2729
args []string
@@ -68,6 +70,8 @@ func TestCommand(t *testing.T) {
6870
}
6971
for _, tt := range tests {
7072
t.Run(tt.name, func(t *testing.T) {
73+
t.Parallel()
74+
7175
tc := testcmd.Case{
7276
Name: tt.name,
7377
Args: slices.Clone(tt.args),

cmd/osv-scanner/fix/testmain_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package fix
22

33
import (
4+
"log/slog"
45
"testing"
56

7+
"github.com/google/osv-scanner/v2/internal/testlogger"
68
"github.com/google/osv-scanner/v2/internal/testutility"
79
)
810

911
func TestMain(m *testing.M) {
12+
slog.SetDefault(slog.New(testlogger.New()))
1013
m.Run()
1114

1215
testutility.CleanSnapshots(m)

cmd/osv-scanner/internal/cmd/helpers_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ func Test_insertDefaultCommand(t *testing.T) {
6969

7070
logger := cmdlogger.New(stdout, stderr)
7171

72-
slog.SetDefault(slog.New(&logger))
72+
slog.SetDefault(slog.New(logger))
7373

7474
argsActual := insertDefaultCommand(tt.originalArgs, commands, defaultCommand, stderr)
7575
if !reflect.DeepEqual(argsActual, tt.wantArgs) {

cmd/osv-scanner/internal/cmd/run.go

+19-3
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ import (
55
"fmt"
66
"io"
77
"log/slog"
8+
"testing"
89

910
"github.com/google/osv-scanner/v2/cmd/osv-scanner/fix"
1011
"github.com/google/osv-scanner/v2/cmd/osv-scanner/scan"
1112
"github.com/google/osv-scanner/v2/cmd/osv-scanner/update"
1213
"github.com/google/osv-scanner/v2/internal/cmdlogger"
14+
"github.com/google/osv-scanner/v2/internal/testlogger"
1315
"github.com/google/osv-scanner/v2/internal/version"
1416
"github.com/google/osv-scanner/v2/pkg/osvscanner"
1517
"github.com/urfave/cli/v2"
@@ -21,9 +23,23 @@ var (
2123
)
2224

2325
func Run(args []string, stdout, stderr io.Writer) int {
24-
logger := cmdlogger.New(stdout, stderr)
26+
// --- Setup Logger ---
27+
logHandler := cmdlogger.New(stdout, stderr)
2528

26-
slog.SetDefault(slog.New(&logger))
29+
// If in testing mode, set logger via Handler
30+
// Otherwise, set default global logger
31+
if testing.Testing() {
32+
handler, ok := slog.Default().Handler().(*testlogger.Handler)
33+
if !ok {
34+
panic("Test failed to initialize default logger with Handler")
35+
}
36+
37+
handler.AddInstance(logHandler)
38+
defer handler.Delete()
39+
} else {
40+
slog.SetDefault(slog.New(logHandler))
41+
}
42+
// ---
2743

2844
cli.VersionPrinter = func(ctx *cli.Context) {
2945
slog.Info("osv-scanner version: " + ctx.App.Version)
@@ -77,7 +93,7 @@ func Run(args []string, stdout, stderr io.Writer) int {
7793

7894
// if we've been told to print an error, and not already exited with
7995
// a specific error code, then exit with a generic non-zero code
80-
if logger.HasErrored() {
96+
if logHandler.HasErrored() {
8197
return 127
8298
}
8399

cmd/osv-scanner/scan/command_test.go

+33
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
)
1010

1111
func TestCommand(t *testing.T) {
12+
t.Parallel()
13+
1214
tests := []testcmd.Case{
1315
{
1416
Name: "",
@@ -241,12 +243,15 @@ func TestCommand(t *testing.T) {
241243
}
242244
for _, tt := range tests {
243245
t.Run(tt.Name, func(t *testing.T) {
246+
t.Parallel()
244247
testcmd.RunAndMatchSnapshots(t, tt)
245248
})
246249
}
247250
}
248251

249252
func TestCommand_CallAnalysis(t *testing.T) {
253+
t.Parallel()
254+
250255
// Switch to acceptance test if this takes too long, or when we add rust tests
251256
// testutility.SkipIfNotAcceptanceTesting(t, "Takes a while to run")
252257

@@ -262,12 +267,15 @@ func TestCommand_CallAnalysis(t *testing.T) {
262267
}
263268
for _, tt := range tests {
264269
t.Run(tt.Name, func(t *testing.T) {
270+
t.Parallel()
265271
testcmd.RunAndMatchSnapshots(t, tt)
266272
})
267273
}
268274
}
269275

270276
func TestCommand_LockfileWithExplicitParseAs(t *testing.T) {
277+
t.Parallel()
278+
271279
tests := []testcmd.Case{
272280
{
273281
Name: "unsupported parse-as",
@@ -380,13 +388,16 @@ func TestCommand_LockfileWithExplicitParseAs(t *testing.T) {
380388
}
381389
for _, tt := range tests {
382390
t.Run(tt.Name, func(t *testing.T) {
391+
t.Parallel()
383392
testcmd.RunAndMatchSnapshots(t, tt)
384393
})
385394
}
386395
}
387396

388397
// TestCommand_GithubActions tests common actions the github actions reusable workflow will run
389398
func TestCommand_GithubActions(t *testing.T) {
399+
t.Parallel()
400+
390401
tests := []testcmd.Case{
391402
{
392403
Name: "scanning osv-scanner custom format",
@@ -401,12 +412,15 @@ func TestCommand_GithubActions(t *testing.T) {
401412
}
402413
for _, tt := range tests {
403414
t.Run(tt.Name, func(t *testing.T) {
415+
t.Parallel()
404416
testcmd.RunAndMatchSnapshots(t, tt)
405417
})
406418
}
407419
}
408420

409421
func TestCommand_LocalDatabases(t *testing.T) {
422+
t.Parallel()
423+
410424
tests := []testcmd.Case{
411425
{
412426
Name: "one specific supported lockfile",
@@ -472,6 +486,7 @@ func TestCommand_LocalDatabases(t *testing.T) {
472486

473487
for _, tt := range tests {
474488
t.Run(tt.Name, func(t *testing.T) {
489+
t.Parallel()
475490
if testutility.IsAcceptanceTesting() {
476491
testDir := testutility.CreateTestDir(t)
477492
old := tt.Args
@@ -488,6 +503,8 @@ func TestCommand_LocalDatabases(t *testing.T) {
488503
}
489504

490505
func TestCommand_LocalDatabases_AlwaysOffline(t *testing.T) {
506+
t.Parallel()
507+
491508
tests := []testcmd.Case{
492509
{
493510
Name: "a bunch of different lockfiles and ecosystem",
@@ -498,6 +515,7 @@ func TestCommand_LocalDatabases_AlwaysOffline(t *testing.T) {
498515

499516
for _, tt := range tests {
500517
t.Run(tt.Name, func(t *testing.T) {
518+
t.Parallel()
501519
testDir := testutility.CreateTestDir(t)
502520
old := tt.Args
503521
tt.Args = []string{"", "--local-db-path", testDir}
@@ -512,6 +530,8 @@ func TestCommand_LocalDatabases_AlwaysOffline(t *testing.T) {
512530
}
513531

514532
func TestCommand_Licenses(t *testing.T) {
533+
t.Parallel()
534+
515535
tests := []testcmd.Case{
516536
{
517537
Name: "No vulnerabilities with license summary",
@@ -579,15 +599,19 @@ func TestCommand_Licenses(t *testing.T) {
579599
Exit: 1,
580600
},
581601
}
602+
582603
for _, tt := range tests {
583604
t.Run(tt.Name, func(t *testing.T) {
605+
t.Parallel()
584606
testcmd.RunAndMatchSnapshots(t, tt)
585607
})
586608
}
587609
}
588610

589611
// Tests all subcommands here.
590612
func TestCommand_SubCommands(t *testing.T) {
613+
t.Parallel()
614+
591615
tests := []testcmd.Case{
592616
// without subcommands
593617
{
@@ -609,14 +633,18 @@ func TestCommand_SubCommands(t *testing.T) {
609633
},
610634
// TODO: add tests for other future subcommands
611635
}
636+
612637
for _, tt := range tests {
613638
t.Run(tt.Name, func(t *testing.T) {
639+
t.Parallel()
614640
testcmd.RunAndMatchSnapshots(t, tt)
615641
})
616642
}
617643
}
618644

619645
func TestCommand_MavenTransitive(t *testing.T) {
646+
t.Parallel()
647+
620648
tests := []testcmd.Case{
621649
{
622650
Name: "scans transitive dependencies for pom.xml by default",
@@ -659,12 +687,15 @@ func TestCommand_MavenTransitive(t *testing.T) {
659687

660688
for _, tt := range tests {
661689
t.Run(tt.Name, func(t *testing.T) {
690+
t.Parallel()
662691
testcmd.RunAndMatchSnapshots(t, tt)
663692
})
664693
}
665694
}
666695

667696
func TestCommand_MoreLockfiles(t *testing.T) {
697+
t.Parallel()
698+
668699
tests := []testcmd.Case{
669700
{
670701
Name: "uv.lock",
@@ -704,8 +735,10 @@ func TestCommand_MoreLockfiles(t *testing.T) {
704735
},
705736
*/
706737
}
738+
707739
for _, tt := range tests {
708740
t.Run(tt.Name, func(t *testing.T) {
741+
t.Parallel()
709742
testcmd.RunAndMatchSnapshots(t, tt)
710743
})
711744
}

cmd/osv-scanner/scan/image/testmain_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package image_test
22

33
import (
4+
"log/slog"
45
"testing"
56

7+
"github.com/google/osv-scanner/v2/internal/testlogger"
68
"github.com/google/osv-scanner/v2/internal/testutility"
79
)
810

911
func TestMain(m *testing.M) {
12+
slog.SetDefault(slog.New(testlogger.New()))
1013
m.Run()
1114

1215
testutility.CleanSnapshots(m)

cmd/osv-scanner/scan/testmain_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package scan_test
22

33
import (
4+
"log/slog"
45
"testing"
56

7+
"github.com/google/osv-scanner/v2/internal/testlogger"
68
"github.com/google/osv-scanner/v2/internal/testutility"
79
)
810

911
func TestMain(m *testing.M) {
12+
slog.SetDefault(slog.New(testlogger.New()))
1013
m.Run()
1114

1215
testutility.CleanSnapshots(m)

cmd/osv-scanner/testmain_test.go

+4
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
11
package main
22

33
import (
4+
"log/slog"
45
"os"
56
"testing"
67

78
"github.com/go-git/go-git/v5"
9+
"github.com/google/osv-scanner/v2/internal/testlogger"
810
"github.com/google/osv-scanner/v2/internal/testutility"
911
)
1012

@@ -18,6 +20,8 @@ func TestMain(m *testing.M) {
1820
if err != nil {
1921
panic(err)
2022
}
23+
24+
slog.SetDefault(slog.New(testlogger.New()))
2125
m.Run()
2226

2327
testutility.CleanSnapshots(m)

cmd/osv-scanner/update/testmain_test.go

+3
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package update_test
22

33
import (
4+
"log/slog"
45
"testing"
56

7+
"github.com/google/osv-scanner/v2/internal/testlogger"
68
"github.com/google/osv-scanner/v2/internal/testutility"
79
)
810

911
func TestMain(m *testing.M) {
12+
slog.SetDefault(slog.New(testlogger.New()))
1013
m.Run()
1114

1215
testutility.CleanSnapshots(m)

0 commit comments

Comments
 (0)