From efaa3cef72f7613a1c46ab7fd132de0dee77dcca Mon Sep 17 00:00:00 2001 From: Zvonimir Pavlinovic Date: Thu, 28 Mar 2024 15:26:00 +0000 Subject: [PATCH] cmd/govulncheck: make test case config data Instead of listing test cases and their configuration in the unit test, we now make the configuration part of the test case, merging it with fixups. This allows us to enumerate test cases instead of listing them explicitly in main_test.go. Change-Id: I75448553035746cd9eb21e6cdc01bae7f13c2327 Reviewed-on: https://go-review.googlesource.com/c/vuln/+/574936 LUCI-TryBot-Result: Go LUCI Run-TryBot: Zvonimir Pavlinovic TryBot-Result: Gopher Robot Reviewed-by: Maceo Thompson --- cmd/govulncheck/main_test.go | 73 +++++++------------- cmd/govulncheck/test_utils.go | 42 +++++++---- cmd/govulncheck/testdata/common/config.json | 40 +++++++++++ cmd/govulncheck/testdata/common/fixups.json | 38 ---------- cmd/govulncheck/testdata/nogomod/config.json | 4 ++ cmd/govulncheck/testdata/stdlib/config.json | 48 +++++++++++++ cmd/govulncheck/testdata/stdlib/fixups.json | 46 ------------ cmd/govulncheck/testdata/strip/config.json | 4 ++ 8 files changed, 151 insertions(+), 144 deletions(-) create mode 100644 cmd/govulncheck/testdata/common/config.json delete mode 100644 cmd/govulncheck/testdata/common/fixups.json create mode 100644 cmd/govulncheck/testdata/nogomod/config.json create mode 100644 cmd/govulncheck/testdata/stdlib/config.json delete mode 100644 cmd/govulncheck/testdata/stdlib/fixups.json create mode 100644 cmd/govulncheck/testdata/strip/config.json diff --git a/cmd/govulncheck/main_test.go b/cmd/govulncheck/main_test.go index 05416e5..c02a448 100644 --- a/cmd/govulncheck/main_test.go +++ b/cmd/govulncheck/main_test.go @@ -32,37 +32,6 @@ import ( var update = flag.Bool("update", false, "update test files with results") -// testCase models a group of tests in cmd/govulncheck testdata. -type testCase struct { - dir string // subdirectory in testdata containing tests - skipGOOS []string // GOOS to skip, if any - copy bool // copy the folder to isolate it - skipBuild bool // skip building the test case - strip bool // indicates if binaries should be stripped -} - -func (tc testCase) skip() bool { - for _, sg := range tc.skipGOOS { - if runtime.GOOS == sg { - return true - } - } - return false -} - -// testCases contains the list of major groups of tests in -// cmd/govulncheck/testdata folder with information on how -// to run them. -var testCases = []testCase{ - {dir: "common"}, - {dir: "stdlib"}, - // Binaries are not stripped on darwin with go1.21 and earlier. See #61051. - {dir: "strip", skipGOOS: []string{"darwin"}, strip: true}, - // nogomod case has code with no go.mod and does not compile, - // so we need to copy it and skip building binaries. - {dir: "nogomod", copy: true, skipBuild: true}, -} - func TestCommand(t *testing.T) { if testing.Short() { t.Skip("skipping test that uses internet in short mode") @@ -73,23 +42,35 @@ func TestCommand(t *testing.T) { t.Fatal(err) } - for _, tc := range testCases { - t.Run(tc.dir, func(t *testing.T) { - runTestCase(t, testDir, tc) + // test all cases in testdata subdirectory + fs, err := os.ReadDir(filepath.Join(testDir, "testdata")) + if err != nil { + t.Fatalf("failed to read test cases: %v", err) + } + for _, tc := range fs { + if !tc.IsDir() { + continue + } + t.Run(tc.Name(), func(t *testing.T) { + runTestCase(t, tc.Name(), testDir) }) } } -func runTestCase(t *testing.T, testDir string, tc testCase) { - if tc.skip() { - return +func runTestCase(t *testing.T, tcName, testDir string) { + testCaseDir := filepath.Join(testDir, "testdata", tcName) + cfg, err := loadConfig(filepath.Join(testCaseDir, "config.json")) + if err != nil { + t.Fatalf("failed to load config: %v", err) } - base := tc.dir - testCaseDir := filepath.Join(testDir, "testdata", base) - if tc.copy { + if cfg.skip() { + return + } + if cfg.Copy { testCaseDir = copyTestCase(testCaseDir, t) } + vulndbDir := filepath.Join(testCaseDir, "vulndb-v1") modulesDir := filepath.Join(testCaseDir, "modules") testfilesDir := filepath.Join(testCaseDir, "testfiles") @@ -97,32 +78,28 @@ func runTestCase(t *testing.T, testDir string, tc testCase) { if err != nil { t.Fatalf("failed to create vulndb url: %v", err) } - fixups, err := loadFixups(filepath.Join(testCaseDir, "fixups.json")) - if err != nil { - t.Fatalf("failed to load fixups: %v", err) - } moduleDirs, err := filepath.Glob(filepath.Join(modulesDir, "*")) if err != nil { t.Fatal(err) } for _, md := range moduleDirs { - if tc.skipBuild { + if cfg.SkipBuild { continue } // Build test module binary. - binary, cleanup := test.GoBuild(t, md, "", tc.strip) + binary, cleanup := test.GoBuild(t, md, "", cfg.Strip) t.Cleanup(cleanup) // Set an environment variable to the path to the binary, so tests // can refer to it. The binary name is unique across all test cases. - varName := tc.dir + "_" + filepath.Base(md) + "_binary" + varName := tcName + "_" + filepath.Base(md) + "_binary" os.Setenv(varName, binary) } os.Setenv("moddir", modulesDir) os.Setenv("testdir", testfilesDir) - runTestSuite(t, testfilesDir, govulndbURI.String(), fixups, *update) + runTestSuite(t, testfilesDir, govulndbURI.String(), cfg.Fixups, *update) } // Limit the number of concurrent scans. Scanning is implemented using diff --git a/cmd/govulncheck/test_utils.go b/cmd/govulncheck/test_utils.go index d7e1df0..0caab89 100644 --- a/cmd/govulncheck/test_utils.go +++ b/cmd/govulncheck/test_utils.go @@ -6,10 +6,10 @@ package main import ( "encoding/json" - "errors" "os" "path/filepath" "regexp" + "runtime" "testing" ) @@ -68,6 +68,28 @@ func copyFile(src, dest string) error { return os.WriteFile(dest, b, 0644) } +type config struct { + // SkipGOOS is a list of GOOS to skip + SkipGOOS []string `json:"skipGOOS,omitempty"` + // Copy the folder to isolate it + Copy bool `json:"copy,omitempty"` + // SkipBuild the test case + SkipBuild bool `json:"skipBuild,omitempty"` + // Strip indicates if binaries should be stripped + Strip bool `json:"strip,omitempty"` + + Fixups []fixup `json:"fixups,omitempty"` +} + +func (c *config) skip() bool { + for _, sg := range c.SkipGOOS { + if runtime.GOOS == sg { + return true + } + } + return false +} + type fixup struct { Pattern string `json:"pattern,omitempty"` Replace string `json:"replace,omitempty"` @@ -86,22 +108,18 @@ func (f *fixup) apply(data []byte) []byte { return f.compiled.ReplaceAll(data, []byte(f.Replace)) } -// loadFixups loads and initializes fixups from path. If there is -// nothing at path, returns nil, nil. -func loadFixups(path string) ([]fixup, error) { - if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { - return nil, nil // no fixups, which is ok - } +// loadConfig loads and initializes the config from path. +func loadConfig(path string) (*config, error) { b, err := os.ReadFile(path) if err != nil { return nil, err } - var fixups []fixup - if err := json.Unmarshal(b, &fixups); err != nil { + var cfg config + if err := json.Unmarshal(b, &cfg); err != nil { return nil, err } - for i := range fixups { - fixups[i].init() + for i, _ := range cfg.Fixups { + cfg.Fixups[i].init() } - return fixups, nil + return &cfg, nil } diff --git a/cmd/govulncheck/testdata/common/config.json b/cmd/govulncheck/testdata/common/config.json new file mode 100644 index 0000000..b4eb324 --- /dev/null +++ b/cmd/govulncheck/testdata/common/config.json @@ -0,0 +1,40 @@ +{ + "fixups": [ + { + "pattern": "Scanning your code and (\\d+) packages across (\\d+)", + "replace": "Scanning your code and P packages across M" + }, + { + "pattern": "Scanner: govulncheck@v.*", + "replace": "Scanner: govulncheck@v1.0.0" + }, + { + "pattern": "\"([^\"]*\") is a file", + "replace": "govulncheck: myfile is a file" + }, + { + "pattern": "\"scanner_version\": \"[^\"]*\"", + "replace": "\"scanner_version\": \"v0.0.0-00000000000-20000101010101\"" + }, + { + "pattern": "file:///(.*)/testdata/(.*)/vulndb", + "replace": "testdata/vulndb" + }, + { + "pattern": "package (.*) is not in (GOROOT|std) (.*)", + "replace": "package foo is not in GOROOT (/tmp/foo)" + }, + { + "pattern": "modified (.*)\\)", + "replace": "modified 01 Jan 21 00:00 UTC)" + }, + { + "pattern": "Go: (go1.[\\.\\d]*|devel).*", + "replace": "Go: go1.18" + }, + { + "pattern": "\"go_version\": \"go[^\\s\"]*\"", + "replace": "\"go_version\": \"go1.18\"" + } + ] +} diff --git a/cmd/govulncheck/testdata/common/fixups.json b/cmd/govulncheck/testdata/common/fixups.json deleted file mode 100644 index cf9c40b..0000000 --- a/cmd/govulncheck/testdata/common/fixups.json +++ /dev/null @@ -1,38 +0,0 @@ -[ - { - "pattern": "Scanning your code and (\\d+) packages across (\\d+)", - "replace": "Scanning your code and P packages across M" - }, - { - "pattern": "Scanner: govulncheck@v.*", - "replace": "Scanner: govulncheck@v1.0.0" - }, - { - "pattern": "\"([^\"]*\") is a file", - "replace": "govulncheck: myfile is a file" - }, - { - "pattern": "\"scanner_version\": \"[^\"]*\"", - "replace": "\"scanner_version\": \"v0.0.0-00000000000-20000101010101\"" - }, - { - "pattern": "file:///(.*)/testdata/(.*)/vulndb", - "replace": "testdata/vulndb" - }, - { - "pattern": "package (.*) is not in (GOROOT|std) (.*)", - "replace": "package foo is not in GOROOT (/tmp/foo)" - }, - { - "pattern": "modified (.*)\\)", - "replace": "modified 01 Jan 21 00:00 UTC)" - }, - { - "pattern": "Go: (go1.[\\.\\d]*|devel).*", - "replace": "Go: go1.18" - }, - { - "pattern": "\"go_version\": \"go[^\\s\"]*\"", - "replace": "\"go_version\": \"go1.18\"" - } -] diff --git a/cmd/govulncheck/testdata/nogomod/config.json b/cmd/govulncheck/testdata/nogomod/config.json new file mode 100644 index 0000000..3b52192 --- /dev/null +++ b/cmd/govulncheck/testdata/nogomod/config.json @@ -0,0 +1,4 @@ +{ + "copy": true, + "skipBuild": true +} diff --git a/cmd/govulncheck/testdata/stdlib/config.json b/cmd/govulncheck/testdata/stdlib/config.json new file mode 100644 index 0000000..eb8ae4e --- /dev/null +++ b/cmd/govulncheck/testdata/stdlib/config.json @@ -0,0 +1,48 @@ +{ + "fixups": [ + { + "pattern": "\\.go:(\\d+):(\\d+):", + "replace": ".go:\u003cl\u003e:\u003cc\u003e:", + "comment": " mask line and column with and placeholders, resp." + }, + { + "pattern": "\\\"line\\\":(\\s)*(\\d+)", + "replace": "\"line\": \u003cl\u003e", + "comment": "modify position lines in json" + }, + { + "pattern": "\\\"column\\\":(\\s)*(\\d+)", + "replace": "\"column\": \u003cc\u003e", + "comment": "modify position columns in json" + }, + { + "pattern": "\\\"offset\\\":(\\s)*(\\d+)", + "replace": "\"offset\": \u003co\u003e", + "comment": "modify position offsets in json" + }, + { + "pattern": "Scanning your code and (\\d+) packages across (\\d+)", + "replace": "Scanning your code and P packages across M" + }, + { + "pattern": "Scanner: govulncheck@v.*", + "replace": "Scanner: govulncheck@v1.0.0" + }, + { + "pattern": "\"scanner_version\": \"[^\"]*\"", + "replace": "\"scanner_version\": \"v0.0.0-00000000000-20000101010101\"" + }, + { + "pattern": "file:///(.*)/testdata/(.*)/vulndb", + "replace": "testdata/vulndb" + }, + { + "pattern": "modified (.*)\\)", + "replace": "modified 01 Jan 21 00:00 UTC)" + }, + { + "pattern": "\"go_version\": \"go[^\\s\"]*\"", + "replace": "\"go_version\": \"go1.18\"" + } + ] +} diff --git a/cmd/govulncheck/testdata/stdlib/fixups.json b/cmd/govulncheck/testdata/stdlib/fixups.json deleted file mode 100644 index 4665914..0000000 --- a/cmd/govulncheck/testdata/stdlib/fixups.json +++ /dev/null @@ -1,46 +0,0 @@ -[ - { - "pattern": "\\.go:(\\d+):(\\d+):", - "replace": ".go:\u003cl\u003e:\u003cc\u003e:", - "comment": " mask line and column with and placeholders, resp." - }, - { - "pattern": "\\\"line\\\":(\\s)*(\\d+)", - "replace": "\"line\": \u003cl\u003e", - "comment": "modify position lines in json" - }, - { - "pattern": "\\\"column\\\":(\\s)*(\\d+)", - "replace": "\"column\": \u003cc\u003e", - "comment": "modify position columns in json" - }, - { - "pattern": "\\\"offset\\\":(\\s)*(\\d+)", - "replace": "\"offset\": \u003co\u003e", - "comment": "modify position offsets in json" - }, - { - "pattern": "Scanning your code and (\\d+) packages across (\\d+)", - "replace": "Scanning your code and P packages across M" - }, - { - "pattern": "Scanner: govulncheck@v.*", - "replace": "Scanner: govulncheck@v1.0.0" - }, - { - "pattern": "\"scanner_version\": \"[^\"]*\"", - "replace": "\"scanner_version\": \"v0.0.0-00000000000-20000101010101\"" - }, - { - "pattern": "file:///(.*)/testdata/(.*)/vulndb", - "replace": "testdata/vulndb" - }, - { - "pattern": "modified (.*)\\)", - "replace": "modified 01 Jan 21 00:00 UTC)" - }, - { - "pattern": "\"go_version\": \"go[^\\s\"]*\"", - "replace": "\"go_version\": \"go1.18\"" - } -] diff --git a/cmd/govulncheck/testdata/strip/config.json b/cmd/govulncheck/testdata/strip/config.json new file mode 100644 index 0000000..ec98bef --- /dev/null +++ b/cmd/govulncheck/testdata/strip/config.json @@ -0,0 +1,4 @@ +{ + "strip": true, + "skipGOOS": ["darwin"] +}