-
Notifications
You must be signed in to change notification settings - Fork 2
test(lockfile): add invariant — LocationRole must be set when BlockLocation is valid #149
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.
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package lockfile_test | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/DataDog/datadog-sbom-generator/internal/utility/fileposition" | ||
| "github.com/DataDog/datadog-sbom-generator/pkg/lockfile" | ||
| "github.com/DataDog/datadog-sbom-generator/pkg/lockfile/internal/testutil" | ||
| _ "github.com/DataDog/datadog-sbom-generator/pkg/lockfile/parsers" // Register all extractors | ||
| ) | ||
|
|
||
| // TestLocationRoleSetWhenBlockLocationIsValid verifies that every extractor sets | ||
| // LocationRole whenever it also sets a valid BlockLocation. | ||
| // | ||
| // This is an invariant that the reporter relies on: vulnerability_result.go uses | ||
| // LocationRole to populate the "role" field in the emitted PackageLocation. If an | ||
| // extractor populates BlockLocation but leaves LocationRole empty, the role will be | ||
| // blank in the SBOM output even though the location is meaningful. | ||
| // | ||
| // The test walks pkg/lockfile/fixtures/, runs the registered extractor for each | ||
| // fixture file, and fails if any returned PackageDetails has a valid BlockLocation | ||
| // but an empty LocationRole. | ||
| func TestLocationRoleSetWhenBlockLocationIsValid(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| fixturesRoot := "fixtures" | ||
| context := testutil.GetTestContext() | ||
|
|
||
| err := filepath.WalkDir(fixturesRoot, func(path string, d os.DirEntry, err error) error { | ||
| if err != nil || d.IsDir() { | ||
| return err | ||
| } | ||
|
|
||
| extractor, _ := lockfile.FindExtractor(path, map[string]bool{}) | ||
| if extractor == nil { | ||
| return nil | ||
| } | ||
|
|
||
| f, err := lockfile.OpenLocalDepFile(path) | ||
| if err != nil { | ||
| return nil // skip unreadable files | ||
| } | ||
| defer f.Close() | ||
|
|
||
| packages, err := extractor.Extract(f, context) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For extractors that add source-file locations via matchers, this check is bypassed because it calls Useful? React with 👍 / 👎. |
||
| if err != nil { | ||
| return nil // skip files that fail to parse (invalid fixtures are intentional) | ||
| } | ||
|
|
||
| for _, pkg := range packages { | ||
| if fileposition.IsFilePositionExtractedSuccessfully(pkg.BlockLocation) && pkg.LocationRole == "" { | ||
| t.Errorf( | ||
| "extractor for %q returned package %s@%s with a valid BlockLocation but empty LocationRole", | ||
| path, pkg.Name, pkg.Version, | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| }) | ||
|
|
||
| if err != nil { | ||
| t.Fatalf("error walking fixtures: %v", err) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this test,
FindExtractoronly returns an extractor whenenabledParsers[name]is true (seepkg/lockfile/extract.go), so passing an empty map disables every registered parser. As a resultextractoris nil for every fixture and the new invariant test passes without exercising any extractor, including the futurepackage.jsoncase it is meant to catch. Populate the map from the registered extractor names before walking fixtures, or use a helper that matches all registered extractors.Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4be1fa3. The test now populates
allParsersfromlockfile.ListSupportedExtractors()before passing it toFindExtractor, so every registered extractor is exercised against its fixture files.