-
Notifications
You must be signed in to change notification settings - Fork 34
🐛 Fix npm package ID to be uniq across directories #5523
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 all commits
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,98 @@ | ||
| // Copyright (c) Mondoo, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
|
|
||
| package resources | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "testing" | ||
|
|
||
| "github.com/spf13/afero" | ||
| "github.com/stretchr/testify/require" | ||
| "go.mondoo.com/cnquery/v11/llx" | ||
| "go.mondoo.com/cnquery/v11/providers-sdk/v1/inventory" | ||
| "go.mondoo.com/cnquery/v11/providers-sdk/v1/plugin" | ||
| "go.mondoo.com/cnquery/v11/providers/os/connection/fs" | ||
| "go.mondoo.com/cnquery/v11/types" | ||
| "go.mondoo.com/cnquery/v11/utils/syncx" | ||
| ) | ||
|
|
||
| func TestNpmPackage_unique(t *testing.T) { | ||
| mockFS := afero.NewMemMapFs() | ||
| // create test files and directories | ||
| err := mockFS.MkdirAll("/usr/local/lib/node_modules/generator-code", 0o755) | ||
| require.NoError(t, err) | ||
| err = mockFS.MkdirAll("/usr/local/lib/node_modules/yo", 0o755) | ||
| require.NoError(t, err) | ||
|
|
||
| // Read package.json files from testdata | ||
| yoPkg, err := os.ReadFile(filepath.Join("packages", "testdata", "yo_package.json")) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, yoPkg) | ||
|
|
||
| gcPkg, err := os.ReadFile(filepath.Join("packages", "testdata", "gc_package.json")) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, gcPkg) | ||
|
|
||
| err = afero.WriteFile(mockFS, "/usr/local/lib/node_modules/generator-code/package.json", gcPkg, 0o644) | ||
| require.NoError(t, err) | ||
| err = afero.WriteFile(mockFS, "/usr/local/lib/node_modules/yo/package.json", yoPkg, 0o644) | ||
| require.NoError(t, err) | ||
|
|
||
| conn, err := fs.NewFileSystemConnectionWithFs(0, &inventory.Config{}, &inventory.Asset{}, "", nil, mockFS) | ||
| require.NoError(t, err) | ||
|
|
||
| r := &plugin.Runtime{ | ||
| Resources: &syncx.Map[plugin.Resource]{}, | ||
| Connection: conn, | ||
| Callback: &providerCallbacks{}, | ||
| } | ||
| mqlNpm := &mqlNpmPackages{ | ||
| MqlRuntime: r, | ||
| } | ||
|
|
||
| // Create resources from filesystem | ||
| err = mqlNpm.gatherData() | ||
| require.NoError(t, err) | ||
|
|
||
| // Check that we have 4 packages | ||
| require.Equal(t, 4, len(mqlNpm.List.Data)) | ||
|
|
||
| // Check that the first package is yosay | ||
| pkg2 := mqlNpm.List.Data[2].(*mqlNpmPackage) | ||
| require.Equal(t, "yosay", pkg2.Name.Data) | ||
| require.Equal(t, "^2.0.2", pkg2.Version.Data) | ||
|
|
||
| // Check that the third package is also yosay, but with a different version | ||
| pkg3 := mqlNpm.List.Data[3].(*mqlNpmPackage) | ||
| require.Equal(t, "yosay", pkg3.Name.Data) | ||
| require.Equal(t, "^3.0.0", pkg3.Version.Data) | ||
|
|
||
| // To get the correct data, we need distinct IDs | ||
| require.NotEqual(t, pkg2.MqlID(), pkg3.MqlID()) | ||
| require.Equal(t, "yosay/usr/local/lib/node_modules/yo/package.json", pkg2.MqlID()) | ||
| require.Equal(t, "yosay/usr/local/lib/node_modules/generator-code/package.json", pkg3.MqlID()) | ||
| } | ||
|
|
||
| // Mock callbacks for testing | ||
| // These are needed during calls to CreateSharedResource | ||
| type providerCallbacks struct{} | ||
|
Contributor
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. can we extract this in a test util file so it can be used elsewhere too?
Contributor
Author
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. I gave it a try. Seem like they are to specific to the tests. |
||
|
|
||
| func (p *providerCallbacks) GetData(req *plugin.DataReq) (*plugin.DataRes, error) { | ||
| return &plugin.DataRes{ | ||
| Data: &llx.Primitive{ | ||
| Type: string(types.Resource(req.Resource)), | ||
| Value: []byte("not of interest"), | ||
| }, | ||
| }, nil | ||
| } | ||
|
|
||
| func (p *providerCallbacks) GetRecording(req *plugin.DataReq) (*plugin.ResourceData, error) { | ||
| res := plugin.ResourceData{} | ||
| return &res, nil | ||
| } | ||
|
|
||
| func (p *providerCallbacks) Collect(req *plugin.DataRes) error { | ||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| { | ||
| "name": "generator-code", | ||
| "version": "1.11.9", | ||
| "description": "Yeoman generator for Visual Studio Code extensions.", | ||
| "keywords": [ | ||
| "yeoman-generator", | ||
| "vscode", | ||
| "visual studio", | ||
| "visual studio code", | ||
| "vs code", | ||
| "extensions" | ||
| ], | ||
| "type": "module", | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/Microsoft/vscode-generator-code.git" | ||
| }, | ||
| "bugs": { | ||
| "url": "https://github.com/Microsoft/vscode-generator-code/issues" | ||
| }, | ||
| "main": "./generators/app/index.js", | ||
| "homepage": "http://code.visualstudio.com", | ||
| "license": "MIT", | ||
| "author": { | ||
| "name": "VS Code Team", | ||
| "url": "https://github.com/Microsoft" | ||
| }, | ||
| "engines": { | ||
| "node": "^18.17.0 || >=20.5.0" | ||
| }, | ||
| "scripts": { | ||
| "test": "mocha", | ||
| "prepublishOnly": "npm test", | ||
| "preversion": "npm test", | ||
| "postversion": "git push && git push --tags" | ||
| }, | ||
| "dependencies": { | ||
| "yosay": "^3.0.0" | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| { | ||
| "name": "yo", | ||
| "version": "5.1.0", | ||
| "description": "CLI tool for running Yeoman generators", | ||
| "license": "BSD-2-Clause", | ||
| "repository": "yeoman/yo", | ||
| "homepage": "http://yeoman.io", | ||
| "author": "Yeoman", | ||
| "main": "lib", | ||
| "bin": { | ||
| "yo": "lib/cli.js", | ||
| "yo-complete": "lib/completion/index.js" | ||
| }, | ||
| "engines": { | ||
| "node": "^18.17.0 || >=20.5.0" | ||
| }, | ||
| "scripts": { | ||
| "postinstall": "yodoctor", | ||
| "postupdate": "yodoctor", | ||
| "pretest": "xo", | ||
| "test": "nyc mocha --timeout=30000", | ||
| "coverage": "nyc report --reporter=text-lcov | coveralls" | ||
| }, | ||
| "files": [ | ||
| "lib" | ||
| ], | ||
| "keywords": [ | ||
| "cli-app", | ||
| "cli", | ||
| "front-end", | ||
| "development", | ||
| "dev", | ||
| "build", | ||
| "web", | ||
| "tool", | ||
| "scaffold", | ||
| "stack", | ||
| "yeoman", | ||
| "generator", | ||
| "generate", | ||
| "app", | ||
| "boilerplate" | ||
| ], | ||
| "dependencies": { | ||
| "yosay": "^2.0.2" | ||
| }, | ||
| "resolutions": { | ||
| "natives": "1.1.3" | ||
| }, | ||
| "tabtab": { | ||
| "yo": [ | ||
| "-f", | ||
| "--force", | ||
| "--version", | ||
| "--no-color", | ||
| "--generators", | ||
| "--local-only" | ||
| ] | ||
| }, | ||
| "xo": { | ||
| "space": true, | ||
| "overrides": [ | ||
| { | ||
| "files": "test/**", | ||
| "envs": [ | ||
| "node", | ||
| "mocha" | ||
| ] | ||
| } | ||
| ], | ||
| "rules": { | ||
| "promise/prefer-await-to-then": 0, | ||
| "unicorn/no-array-reduce": "off" | ||
| } | ||
| } | ||
| } |
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.
can we make
mqlFilesto be a[]*mqlPkgFileInfo{}list? that way we avoid the casting. not sure if this causes a problem later in the codeThere 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.
No,
L335:
This requires
[]interfac{}.