diff --git a/.nextchanges/cli/apps-validate-package-managers.md b/.nextchanges/cli/apps-validate-package-managers.md new file mode 100644 index 00000000000..228d7a42b8c --- /dev/null +++ b/.nextchanges/cli/apps-validate-package-managers.md @@ -0,0 +1 @@ +* Select npm or pnpm from `packageManager` declarations and lockfiles for `apps validate` and project validation during `apps deploy`. ([#6892](https://github.com/databricks/cli/pull/6892)) diff --git a/acceptance/apps/validate/app/package.json b/acceptance/apps/validate/app/package.json new file mode 100644 index 00000000000..0967ef424bc --- /dev/null +++ b/acceptance/apps/validate/app/package.json @@ -0,0 +1 @@ +{} diff --git a/acceptance/apps/validate/bin/npm b/acceptance/apps/validate/bin/npm new file mode 100755 index 00000000000..d452c009e29 --- /dev/null +++ b/acceptance/apps/validate/bin/npm @@ -0,0 +1,7 @@ +#!/bin/sh +printf '%s %s\n' "${0##*/}" "$*" >> commands.log +if [ "$*" = "${VALIDATION_TEST_FAIL:-}" ]; then + echo "validation stdout" + echo "validation stderr" >&2 + exit 17 +fi diff --git a/acceptance/apps/validate/bin/npm.cmd b/acceptance/apps/validate/bin/npm.cmd new file mode 100644 index 00000000000..1e7de72f5dd --- /dev/null +++ b/acceptance/apps/validate/bin/npm.cmd @@ -0,0 +1,7 @@ +@echo off +echo %~n0 %*>>commands.log +if "%*"=="%VALIDATION_TEST_FAIL%" ( + echo validation stdout + echo validation stderr 1>&2 + exit /b 17 +) diff --git a/acceptance/apps/validate/bin/pnpm b/acceptance/apps/validate/bin/pnpm new file mode 100755 index 00000000000..d452c009e29 --- /dev/null +++ b/acceptance/apps/validate/bin/pnpm @@ -0,0 +1,7 @@ +#!/bin/sh +printf '%s %s\n' "${0##*/}" "$*" >> commands.log +if [ "$*" = "${VALIDATION_TEST_FAIL:-}" ]; then + echo "validation stdout" + echo "validation stderr" >&2 + exit 17 +fi diff --git a/acceptance/apps/validate/bin/pnpm.cmd b/acceptance/apps/validate/bin/pnpm.cmd new file mode 100644 index 00000000000..1e7de72f5dd --- /dev/null +++ b/acceptance/apps/validate/bin/pnpm.cmd @@ -0,0 +1,7 @@ +@echo off +echo %~n0 %*>>commands.log +if "%*"=="%VALIDATION_TEST_FAIL%" ( + echo validation stdout + echo validation stderr 1>&2 + exit /b 17 +) diff --git a/acceptance/apps/validate/bom/package.json b/acceptance/apps/validate/bom/package.json new file mode 100644 index 00000000000..f54d4a2c495 --- /dev/null +++ b/acceptance/apps/validate/bom/package.json @@ -0,0 +1,5 @@ +{ + "scripts": { + "build": "node -e \"console.log('BOM build ran'); process.exit(17)\"" + } +} diff --git a/acceptance/apps/validate/conflicting/package-lock.json b/acceptance/apps/validate/conflicting/package-lock.json new file mode 100644 index 00000000000..0967ef424bc --- /dev/null +++ b/acceptance/apps/validate/conflicting/package-lock.json @@ -0,0 +1 @@ +{} diff --git a/acceptance/apps/validate/conflicting/package.json b/acceptance/apps/validate/conflicting/package.json new file mode 100644 index 00000000000..0967ef424bc --- /dev/null +++ b/acceptance/apps/validate/conflicting/package.json @@ -0,0 +1 @@ +{} diff --git a/acceptance/apps/validate/conflicting/pnpm-lock.yaml b/acceptance/apps/validate/conflicting/pnpm-lock.yaml new file mode 100644 index 00000000000..b0a073a9c13 --- /dev/null +++ b/acceptance/apps/validate/conflicting/pnpm-lock.yaml @@ -0,0 +1 @@ +lockfileVersion: '9.0' diff --git a/acceptance/apps/validate/databricks.yml b/acceptance/apps/validate/databricks.yml new file mode 100644 index 00000000000..3b058245ebe --- /dev/null +++ b/acceptance/apps/validate/databricks.yml @@ -0,0 +1,2 @@ +bundle: + name: validate-app diff --git a/acceptance/apps/validate/invalid/package.json b/acceptance/apps/validate/invalid/package.json new file mode 100644 index 00000000000..98232c64fce --- /dev/null +++ b/acceptance/apps/validate/invalid/package.json @@ -0,0 +1 @@ +{ diff --git a/acceptance/apps/validate/lifecycle/add-build/package.json b/acceptance/apps/validate/lifecycle/add-build/package.json new file mode 100644 index 00000000000..a350619357d --- /dev/null +++ b/acceptance/apps/validate/lifecycle/add-build/package.json @@ -0,0 +1,8 @@ +{ + "name": "validation-add-build", + "version": "1.0.0", + "scripts": { + "install": "node ../update-manifest.cjs", + "typegen": "node ../update-manifest.cjs" + } +} diff --git a/acceptance/apps/validate/lifecycle/invalidated/package.json b/acceptance/apps/validate/lifecycle/invalidated/package.json new file mode 100644 index 00000000000..0d35bc1fafd --- /dev/null +++ b/acceptance/apps/validate/lifecycle/invalidated/package.json @@ -0,0 +1,8 @@ +{ + "name": "validation-invalidated-manifest", + "version": "1.0.0", + "scripts": { + "install": "node ../update-manifest.cjs", + "typegen": "node ../update-manifest.cjs" + } +} diff --git a/acceptance/apps/validate/lifecycle/remove-build/package.json b/acceptance/apps/validate/lifecycle/remove-build/package.json new file mode 100644 index 00000000000..615c53f9bad --- /dev/null +++ b/acceptance/apps/validate/lifecycle/remove-build/package.json @@ -0,0 +1,9 @@ +{ + "name": "validation-remove-build", + "version": "1.0.0", + "scripts": { + "install": "node ../update-manifest.cjs", + "typegen": "node ../update-manifest.cjs", + "build": "node -e \"process.exit(17)\"" + } +} diff --git a/acceptance/apps/validate/lifecycle/update-manifest.cjs b/acceptance/apps/validate/lifecycle/update-manifest.cjs new file mode 100644 index 00000000000..9a5f12da591 --- /dev/null +++ b/acceptance/apps/validate/lifecycle/update-manifest.cjs @@ -0,0 +1,17 @@ +const fs = require("node:fs"); + +if (process.env.npm_lifecycle_event === process.env.VALIDATION_UPDATE_STEP) { + const manifest = JSON.parse(fs.readFileSync("package.json", "utf8")); + switch (process.env.VALIDATION_UPDATE_ACTION) { + case "add": + manifest.scripts.build = "node -e \"console.log('added build ran'); process.exit(17)\""; + break; + case "remove": + delete manifest.scripts.build; + break; + case "invalidate": + fs.writeFileSync("package.json", "{"); + return; + } + fs.writeFileSync("package.json", JSON.stringify(manifest)); +} diff --git a/acceptance/apps/validate/metadata-npm/databricks.yml b/acceptance/apps/validate/metadata-npm/databricks.yml new file mode 100644 index 00000000000..738d8400c6a --- /dev/null +++ b/acceptance/apps/validate/metadata-npm/databricks.yml @@ -0,0 +1,2 @@ +bundle: + name: validation-metadata-npm diff --git a/acceptance/apps/validate/metadata-npm/package.json b/acceptance/apps/validate/metadata-npm/package.json new file mode 100644 index 00000000000..6620a23a240 --- /dev/null +++ b/acceptance/apps/validate/metadata-npm/package.json @@ -0,0 +1,8 @@ +{ + "name": "validation-metadata-npm", + "version": "1.0.0", + "packageManager": "npm@10.9.2", + "scripts": { + "build": "node -e \"console.log('declared npm build ran'); process.exit(17)\"" + } +} diff --git a/acceptance/apps/validate/metadata-npm/pnpm-lock.yaml b/acceptance/apps/validate/metadata-npm/pnpm-lock.yaml new file mode 100644 index 00000000000..b0a073a9c13 --- /dev/null +++ b/acceptance/apps/validate/metadata-npm/pnpm-lock.yaml @@ -0,0 +1 @@ +lockfileVersion: '9.0' diff --git a/acceptance/apps/validate/non-runnable/package.json b/acceptance/apps/validate/non-runnable/package.json new file mode 100644 index 00000000000..cfcacb6fe6d --- /dev/null +++ b/acceptance/apps/validate/non-runnable/package.json @@ -0,0 +1,9 @@ +{ + "scripts": { + "typegen": "", + "typecheck": null, + "test": "", + "//": ["a comment"], + "build": "node -e \"console.log('non-runnable entries ignored'); process.exit(17)\"" + } +} diff --git a/acceptance/apps/validate/null/package.json b/acceptance/apps/validate/null/package.json new file mode 100644 index 00000000000..19765bd501b --- /dev/null +++ b/acceptance/apps/validate/null/package.json @@ -0,0 +1 @@ +null diff --git a/acceptance/apps/validate/out.test.toml b/acceptance/apps/validate/out.test.toml new file mode 100644 index 00000000000..0938e678987 --- /dev/null +++ b/acceptance/apps/validate/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] diff --git a/acceptance/apps/validate/output.txt b/acceptance/apps/validate/output.txt new file mode 100644 index 00000000000..cfabc47f0f1 --- /dev/null +++ b/acceptance/apps/validate/output.txt @@ -0,0 +1,196 @@ + +=== Validate the directory selected by --path +>>> withdir app [CLI] apps validate --path .. +Validating project using pnpm... +✅ Installing dependencies ([DURATION]) +✅ Generating types ([DURATION]) +✅ Running AST-grep lint ([DURATION]) +✅ Type checking ([DURATION]) +✅ Building ([DURATION]) +✅ Running tests ([DURATION]) +✅ All validation checks passed ([DURATION]) + +=== Skip tests +>>> [CLI] apps validate --skip-tests +Validating project using pnpm... +✅ Installing dependencies ([DURATION]) +✅ Generating types ([DURATION]) +✅ Running AST-grep lint ([DURATION]) +✅ Type checking ([DURATION]) +✅ Building ([DURATION]) +⏭️ Skipped Running tests +✅ All validation checks passed ([DURATION]) + +=== Report a failed script +>>> VALIDATION_TEST_FAIL=run --if-present test musterr [CLI] apps validate +Validating project using pnpm... +✅ Installing dependencies ([DURATION]) +✅ Generating types ([DURATION]) +✅ Running AST-grep lint ([DURATION]) +✅ Type checking ([DURATION]) +✅ Building ([DURATION]) +Error: tests failed (duration: [DURATION]) +❌ Running tests failed ([DURATION]) +validation failed (exit code 17) +Stdout: +validation stdout + +Stderr: +validation stderr + +Error: validation failed + +=== Deploy validates with the same package manager before uploading +>>> VALIDATION_TEST_FAIL=run --if-present build musterr [CLI] apps deploy +Validating project using pnpm... +✅ Installing dependencies ([DURATION]) +✅ Generating types ([DURATION]) +✅ Running AST-grep lint ([DURATION]) +✅ Type checking ([DURATION]) +Error: build failed (duration: [DURATION]) +❌ Building failed ([DURATION]) +validation failed (exit code 17) +Stdout: +validation stdout + +Stderr: +validation stderr + +Error: validation failed - fix errors before deploying + +=== Commands executed in the project directory +>>> cat commands.log +pnpm install +pnpm run --if-present typegen +pnpm run --if-present lint:ast-grep +pnpm run --if-present typecheck +pnpm run --if-present build +pnpm run --if-present test +pnpm install +pnpm run --if-present typegen +pnpm run --if-present lint:ast-grep +pnpm run --if-present typecheck +pnpm run --if-present build +pnpm install +pnpm run --if-present typegen +pnpm run --if-present lint:ast-grep +pnpm run --if-present typecheck +pnpm run --if-present build +pnpm run --if-present test +pnpm install +pnpm run --if-present typegen +pnpm run --if-present lint:ast-grep +pnpm run --if-present typecheck +pnpm run --if-present build + +=== Warn but continue on conflicting lockfiles +>>> [CLI] apps validate --path conflicting +Warn: found conflicting package managers; using npm from package-lock.json and ignoring pnpm-lock.yaml +Validating project using npm... +✅ Installing dependencies ([DURATION]) +✅ Generating types ([DURATION]) +✅ Running AST-grep lint ([DURATION]) +✅ Type checking ([DURATION]) +✅ Building ([DURATION]) +✅ Running tests ([DURATION]) +✅ All validation checks passed ([DURATION]) + +=== Warn but continue on unsupported lockfiles +>>> [CLI] apps validate --path unsupported +Warn: ignoring unsupported lockfile yarn.lock; apps validation uses npm or pnpm +Validating project using npm... +✅ Installing dependencies ([DURATION]) +✅ Generating types ([DURATION]) +✅ Running AST-grep lint ([DURATION]) +✅ Type checking ([DURATION]) +✅ Building ([DURATION]) +✅ Running tests ([DURATION]) +✅ All validation checks passed ([DURATION]) + +=== Validate npm workspace scripts without root scripts +>>> grep -x workspace build ran LOG.workspaces +workspace build ran + +>>> grep -F validation failed (exit code 17) LOG.workspaces +validation failed (exit code 17) + +=== Deploy rejects a failing npm workspace build +>>> grep -x workspace build ran LOG.workspaces-deploy +workspace build ran + +>>> grep -F validation failed - fix errors before deploying LOG.workspaces-deploy +Error: validation failed - fix errors before deploying + +=== Use declared npm despite a pnpm lockfile +>>> grep -x declared npm build ran LOG.metadata-npm +declared npm build ran + +>>> grep -F validation failed (exit code 17) LOG.metadata-npm +validation failed (exit code 17) + +=== Deploy honors the declared package manager +>>> grep -x declared npm build ran LOG.metadata-npm-deploy +declared npm build ran + +>>> grep -F validation failed - fix errors before deploying LOG.metadata-npm-deploy +Error: validation failed - fix errors before deploying + +=== Reject a missing package.json +>>> musterr [CLI] apps validate --path missing +Error: no supported project type detected (looking for package.json) + +=== Report native manifest errors with dependencies absent +>>> grep -x Error: validation failed LOG.invalid-absent +Error: validation failed + +=== Report native manifest errors with dependencies present +>>> grep -x Error: validation failed LOG.invalid-present +Error: validation failed + +=== Reject a null manifest through the package manager +>>> grep -x Error: validation failed LOG.null +Error: validation failed + +=== Run scripts from a manifest with a UTF-8 BOM +>>> grep -x BOM build ran LOG.bom +BOM build ran + +>>> grep -F validation failed (exit code 17) LOG.bom +validation failed (exit code 17) + +=== Let the package manager ignore non-runnable script entries +>>> grep -x non-runnable entries ignored LOG.non-runnable +non-runnable entries ignored + +>>> grep -F validation failed (exit code 17) LOG.non-runnable +validation failed (exit code 17) + +=== Run a build added by install +>>> grep -x added build ran LOG.add-build-install +added build ran + +>>> grep -F validation failed (exit code 17) LOG.add-build-install +validation failed (exit code 17) + +=== Skip a build removed by install +>>> grep -F All validation checks passed LOG.remove-build-install +✅ All validation checks passed ([DURATION]) + +=== Report native errors after install invalidates the manifest +>>> grep -x Error: validation failed LOG.invalidated-install +Error: validation failed + +=== Run a build added by typegen +>>> grep -x added build ran LOG.add-build-typegen +added build ran + +>>> grep -F validation failed (exit code 17) LOG.add-build-typegen +validation failed (exit code 17) + +=== Skip a build removed by typegen +>>> grep -F All validation checks passed LOG.remove-build-typegen +✅ All validation checks passed ([DURATION]) + +=== Report native errors after typegen invalidates the manifest +>>> grep -x Error: validation failed LOG.invalidated-typegen +Error: validation failed diff --git a/acceptance/apps/validate/package.json b/acceptance/apps/validate/package.json new file mode 100644 index 00000000000..373a51f977f --- /dev/null +++ b/acceptance/apps/validate/package.json @@ -0,0 +1,6 @@ +{ + "scripts": { + "build": "echo build", + "test": "echo test" + } +} diff --git a/acceptance/apps/validate/pnpm-lock.yaml b/acceptance/apps/validate/pnpm-lock.yaml new file mode 100644 index 00000000000..b0a073a9c13 --- /dev/null +++ b/acceptance/apps/validate/pnpm-lock.yaml @@ -0,0 +1 @@ +lockfileVersion: '9.0' diff --git a/acceptance/apps/validate/script b/acceptance/apps/validate/script new file mode 100644 index 00000000000..4c59e76e046 --- /dev/null +++ b/acceptance/apps/validate/script @@ -0,0 +1,96 @@ +validation_path="$PATH" +export PATH="$PWD/bin:$PATH" + +title "Validate the directory selected by --path" +trace withdir app $CLI apps validate --path .. + +title "Skip tests" +trace $CLI apps validate --skip-tests + +title "Report a failed script" +trace VALIDATION_TEST_FAIL="run --if-present test" musterr $CLI apps validate + +title "Deploy validates with the same package manager before uploading" +trace VALIDATION_TEST_FAIL="run --if-present build" musterr $CLI apps deploy + +title "Commands executed in the project directory" +trace cat commands.log + +title "Warn but continue on conflicting lockfiles" +trace $CLI apps validate --path conflicting + +title "Warn but continue on unsupported lockfiles" +trace $CLI apps validate --path unsupported + +title "Validate npm workspace scripts without root scripts" +export PATH="$validation_path" +mkdir -p workspaces/node_modules +trace musterr $CLI apps validate --path workspaces &> LOG.workspaces +trace grep -x "workspace build ran" LOG.workspaces +trace grep -F "validation failed (exit code 17)" LOG.workspaces + +title "Deploy rejects a failing npm workspace build" +trace withdir workspaces musterr $CLI apps deploy &> LOG.workspaces-deploy +trace grep -x "workspace build ran" LOG.workspaces-deploy +trace grep -F "validation failed - fix errors before deploying" LOG.workspaces-deploy + +title "Use declared npm despite a pnpm lockfile" +mkdir -p metadata-npm/node_modules +trace musterr $CLI apps validate --path metadata-npm &> LOG.metadata-npm +trace grep -x "declared npm build ran" LOG.metadata-npm +trace grep -F "validation failed (exit code 17)" LOG.metadata-npm + +title "Deploy honors the declared package manager" +trace withdir metadata-npm musterr $CLI apps deploy &> LOG.metadata-npm-deploy +trace grep -x "declared npm build ran" LOG.metadata-npm-deploy +trace grep -F "validation failed - fix errors before deploying" LOG.metadata-npm-deploy + +title "Reject a missing package.json" +mkdir missing +trace musterr $CLI apps validate --path missing + +for dependencies in absent present; do + title "Report native manifest errors with dependencies $dependencies" + trace musterr $CLI apps validate --path invalid &> LOG.invalid-$dependencies + contains.py "EJSONPARSE" < LOG.invalid-$dependencies > LOG.check-invalid-$dependencies + trace grep -x "Error: validation failed" LOG.invalid-$dependencies + mkdir -p invalid/node_modules +done + +title "Reject a null manifest through the package manager" +mkdir -p null/node_modules +trace musterr $CLI apps validate --path null &> LOG.null +trace grep -x "Error: validation failed" LOG.null + +title "Run scripts from a manifest with a UTF-8 BOM" +mkdir -p bom/node_modules +trace musterr $CLI apps validate --path bom &> LOG.bom +trace grep -x "BOM build ran" LOG.bom +trace grep -F "validation failed (exit code 17)" LOG.bom + +title "Let the package manager ignore non-runnable script entries" +mkdir -p non-runnable/node_modules +trace musterr $CLI apps validate --path non-runnable &> LOG.non-runnable +trace grep -x "non-runnable entries ignored" LOG.non-runnable +trace grep -F "validation failed (exit code 17)" LOG.non-runnable + +for step in install typegen; do + cp -R lifecycle lifecycle-$step + if [ "$step" = typegen ]; then + mkdir -p lifecycle-$step/{add-build,remove-build,invalidated}/node_modules + fi + + title "Run a build added by $step" + trace VALIDATION_UPDATE_STEP=$step VALIDATION_UPDATE_ACTION=add musterr $CLI apps validate --path lifecycle-$step/add-build &> LOG.add-build-$step + trace grep -x "added build ran" LOG.add-build-$step + trace grep -F "validation failed (exit code 17)" LOG.add-build-$step + + title "Skip a build removed by $step" + trace VALIDATION_UPDATE_STEP=$step VALIDATION_UPDATE_ACTION=remove $CLI apps validate --path lifecycle-$step/remove-build &> LOG.remove-build-$step + trace grep -F "All validation checks passed" LOG.remove-build-$step + + title "Report native errors after $step invalidates the manifest" + trace VALIDATION_UPDATE_STEP=$step VALIDATION_UPDATE_ACTION=invalidate musterr $CLI apps validate --path lifecycle-$step/invalidated &> LOG.invalidated-$step + contains.py "EJSONPARSE" < LOG.invalidated-$step > LOG.check-invalidated-$step + trace grep -x "Error: validation failed" LOG.invalidated-$step +done diff --git a/acceptance/apps/validate/test.toml b/acceptance/apps/validate/test.toml new file mode 100644 index 00000000000..76667680b83 --- /dev/null +++ b/acceptance/apps/validate/test.toml @@ -0,0 +1,11 @@ +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["direct"] +Env.VALIDATION_TEST_FAIL = "" +Env.npm_config_audit = "false" +Env.npm_config_fund = "false" +Env.npm_config_offline = "true" +Env.npm_config_update_notifier = "false" +Ignore = ["commands.log", "node_modules", "missing", "lifecycle-install", "lifecycle-typegen"] + +[[Repls]] +Old = '\d+\.\d+s' +New = '[DURATION]' diff --git a/acceptance/apps/validate/unsupported/package.json b/acceptance/apps/validate/unsupported/package.json new file mode 100644 index 00000000000..0967ef424bc --- /dev/null +++ b/acceptance/apps/validate/unsupported/package.json @@ -0,0 +1 @@ +{} diff --git a/acceptance/apps/validate/unsupported/yarn.lock b/acceptance/apps/validate/unsupported/yarn.lock new file mode 100644 index 00000000000..ef452491215 --- /dev/null +++ b/acceptance/apps/validate/unsupported/yarn.lock @@ -0,0 +1 @@ +# yarn lockfile v1 diff --git a/acceptance/apps/validate/workspaces/.npmrc b/acceptance/apps/validate/workspaces/.npmrc new file mode 100644 index 00000000000..76511e68b54 --- /dev/null +++ b/acceptance/apps/validate/workspaces/.npmrc @@ -0,0 +1,2 @@ +workspaces=true +update-notifier=false diff --git a/acceptance/apps/validate/workspaces/databricks.yml b/acceptance/apps/validate/workspaces/databricks.yml new file mode 100644 index 00000000000..2aecdb5c928 --- /dev/null +++ b/acceptance/apps/validate/workspaces/databricks.yml @@ -0,0 +1,2 @@ +bundle: + name: validation-workspaces diff --git a/acceptance/apps/validate/workspaces/package.json b/acceptance/apps/validate/workspaces/package.json new file mode 100644 index 00000000000..87d12a4b24e --- /dev/null +++ b/acceptance/apps/validate/workspaces/package.json @@ -0,0 +1,5 @@ +{ + "name": "validation-workspaces", + "private": true, + "workspaces": ["packages/*"] +} diff --git a/acceptance/apps/validate/workspaces/packages/app/package.json b/acceptance/apps/validate/workspaces/packages/app/package.json new file mode 100644 index 00000000000..ab8149856d5 --- /dev/null +++ b/acceptance/apps/validate/workspaces/packages/app/package.json @@ -0,0 +1,7 @@ +{ + "name": "validation-workspace-app", + "version": "1.0.0", + "scripts": { + "build": "node -e \"console.log('workspace build ran'); process.exit(17)\"" + } +} diff --git a/cmd/apps/validate.go b/cmd/apps/validate.go index 34340192798..e84a0221631 100644 --- a/cmd/apps/validate.go +++ b/cmd/apps/validate.go @@ -17,7 +17,24 @@ func newValidateCmd() *cobra.Command { Long: `Validate a Databricks App project by running build, typecheck, and lint checks. This command detects the project type and runs the appropriate validation: -- Node.js projects (package.json): runs npm install, build, typecheck, lint, and tests +- Node.js projects (package.json): installs dependencies and runs typegen, + lint:ast-grep, typecheck, build, and test scripts when present + +An npm or pnpm packageManager declaration in package.json takes precedence over +lockfiles in the project directory. Conflicting lockfiles produce a warning. +Without a supported declaration, the package manager is detected from lockfiles: +- package-lock.json or npm-shrinkwrap.json: npm +- pnpm-lock.yaml: pnpm + +Only npm and pnpm are supported. Unsupported packageManager declarations and +Yarn/Bun lockfiles produce a warning and are ignored. Without a supported +declaration, projects with both npm and pnpm lockfiles warn and use npm; +projects without a supported lockfile also use npm. +The selected package manager must be on PATH in the project directory. +Dependencies are installed when node_modules is absent. + +Scripts use the package manager's normal workspace behavior. For pnpm workspaces, +define root scripts that run the checks for the packages you want to validate. Examples: # Validate the current directory diff --git a/libs/apps/packagejson/package_json.go b/libs/apps/packagejson/package_json.go new file mode 100644 index 00000000000..214c1716ee4 --- /dev/null +++ b/libs/apps/packagejson/package_json.go @@ -0,0 +1,26 @@ +package packagejson + +import ( + "bytes" + "encoding/json" + "fmt" + "os" + "path/filepath" +) + +const FileName = "package.json" + +// Read decodes package.json in workDir without interpreting its fields. +func Read(workDir string) (map[string]json.RawMessage, error) { + data, err := os.ReadFile(filepath.Join(workDir, FileName)) + if err != nil { + return nil, fmt.Errorf("failed to read %s: %w", FileName, err) + } + // npm accepts a UTF-8 BOM: https://github.com/npm/json-parse-even-better-errors. + data = bytes.TrimPrefix(data, []byte("\xef\xbb\xbf")) + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + return nil, fmt.Errorf("failed to parse %s: %w", FileName, err) + } + return fields, nil +} diff --git a/libs/apps/packagejson/package_json_test.go b/libs/apps/packagejson/package_json_test.go new file mode 100644 index 00000000000..55d4e4fde11 --- /dev/null +++ b/libs/apps/packagejson/package_json_test.go @@ -0,0 +1,55 @@ +package packagejson_test + +import ( + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/apps/packagejson" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRead(t *testing.T) { + for _, tt := range []struct { + name string + prefix string + }{ + {name: "JSON"}, + {name: "BOM", prefix: "\xef\xbb\xbf"}, + } { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + testutil.WriteFile(t, filepath.Join(dir, packagejson.FileName), tt.prefix+`{ + "packageManager": "pnpm@10.30.3", + "Scripts": {"build": "echo build"}, + "scripts": {"typecheck": null, "//": ["a comment"]}, + "largeNumber": 9007199254740993 + }`) + fields, err := packagejson.Read(dir) + require.NoError(t, err) + assert.JSONEq(t, `"pnpm@10.30.3"`, string(fields["packageManager"])) + assert.JSONEq(t, `{"build": "echo build"}`, string(fields["Scripts"])) + assert.JSONEq(t, `{"typecheck": null, "//": ["a comment"]}`, string(fields["scripts"])) + assert.Equal(t, "9007199254740993", string(fields["largeNumber"])) + }) + } +} + +func TestReadErrors(t *testing.T) { + t.Run("missing file", func(t *testing.T) { + fields, err := packagejson.Read(t.TempDir()) + require.ErrorIs(t, err, os.ErrNotExist) + assert.Nil(t, fields) + }) + t.Run("invalid JSON", func(t *testing.T) { + dir := t.TempDir() + testutil.WriteFile(t, filepath.Join(dir, packagejson.FileName), "{") + fields, err := packagejson.Read(dir) + require.Error(t, err) + assert.ErrorAs(t, err, new(*json.SyntaxError)) + assert.Nil(t, fields) + }) +} diff --git a/libs/apps/validation/nodejs.go b/libs/apps/validation/nodejs.go index 3bd25bd5612..3fc76862aa4 100644 --- a/libs/apps/validation/nodejs.go +++ b/libs/apps/validation/nodejs.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "time" "github.com/databricks/cli/libs/cmdio" @@ -12,12 +13,18 @@ import ( "github.com/databricks/cli/libs/log" ) +const ( + packageManagerInstall = "install" + packageManagerRun = "run" + packageManagerIfPresent = "--if-present" +) + // ValidationNodeJs implements validation for Node.js-based projects. type ValidationNodeJs struct{} type validationStep struct { name string - command string + args []string errorPrefix string displayName string skipIf func(workDir string, opts ValidateOptions) bool // Optional: skip step if this returns true @@ -27,44 +34,51 @@ func (v *ValidationNodeJs) Validate(ctx context.Context, workDir string, opts Va log.Infof(ctx, "Starting Node.js validation: build + typecheck") startTime := time.Now() - cmdio.LogString(ctx, "Validating project...") + manager, err := DetectPackageManager(ctx, workDir) + if err != nil { + return nil, err + } + + cmdio.LogString(ctx, "Validating project using "+manager+"...") // TODO: these steps could be changed to npx appkit [command] instead if we can determine its an appkit project. + // Let the manager resolve scripts, including configured npm workspaces: + // https://docs.npmjs.com/cli/using-npm/workspaces#running-commands-in-the-context-of-workspaces steps := []validationStep{ { - name: "install", - command: "npm install", + name: packageManagerInstall, + args: []string{packageManagerInstall}, errorPrefix: "Failed to install dependencies", displayName: "Installing dependencies", skipIf: func(workDir string, _ ValidateOptions) bool { return hasNodeModules(workDir) }, }, { name: "generate", - command: "npm run typegen --if-present", - errorPrefix: "Failed to run npm typegen", + args: []string{packageManagerRun, packageManagerIfPresent, "typegen"}, + errorPrefix: "Failed to generate types", displayName: "Generating types", }, { name: "ast-grep-lint", - command: "npm run lint:ast-grep --if-present", + args: []string{packageManagerRun, packageManagerIfPresent, "lint:ast-grep"}, errorPrefix: "AST-grep lint found violations", displayName: "Running AST-grep lint", }, { name: "typecheck", - command: "npm run typecheck --if-present", + args: []string{packageManagerRun, packageManagerIfPresent, "typecheck"}, errorPrefix: "Failed to run client typecheck", displayName: "Type checking", }, { name: "build", - command: "npm run build --if-present", - errorPrefix: "Failed to run npm build", + args: []string{packageManagerRun, packageManagerIfPresent, "build"}, + errorPrefix: "Failed to build", displayName: "Building", }, { name: "tests", - command: "npm run test --if-present", + args: []string{packageManagerRun, packageManagerIfPresent, "test"}, errorPrefix: "Failed to run tests", displayName: "Running tests", skipIf: func(_ string, opts ValidateOptions) bool { return opts.SkipTests }, @@ -78,6 +92,7 @@ func (v *ValidationNodeJs) Validate(ctx context.Context, workDir string, opts Va cmdio.LogString(ctx, "⏭️ Skipped "+step.displayName) continue } + command := manager + " " + strings.Join(step.args, " ") log.Debugf(ctx, "running %s...", step.name) @@ -88,7 +103,7 @@ func (v *ValidationNodeJs) Validate(ctx context.Context, workDir string, opts Va spinner := cmdio.NewSpinner(ctx) spinner.Update(step.displayName + "...") - stepErr = runValidationCommand(ctx, workDir, step.command) + stepErr = runValidationCommand(ctx, workDir, command) spinner.Close() stepDuration := time.Since(stepStart) diff --git a/libs/apps/validation/nodejs_test.go b/libs/apps/validation/nodejs_test.go new file mode 100644 index 00000000000..b844bccd9e9 --- /dev/null +++ b/libs/apps/validation/nodejs_test.go @@ -0,0 +1,222 @@ +package validation_test + +import ( + "encoding/json" + "os" + osexec "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/apps/validation" + "github.com/databricks/cli/libs/cmdio" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +var validationScripts = []string{"typegen", "lint:ast-grep", "typecheck", "build", "test"} + +// These values define the protocol shared with testdata/package-manager and .cmd. +const ( + validationTestFailEnv = "VALIDATION_TEST_FAIL" + validationTestExitCode = 17 + validationTestStdout = "validation stdout" + validationTestStderr = "validation stderr" +) + +const npmValidationCommands = `npm install +npm run --if-present typegen +npm run --if-present lint:ast-grep +npm run --if-present typecheck +npm run --if-present build +npm run --if-present test +` + +// stubPackageManagers records commands without installing packages or requiring Node.js. +func stubPackageManagers(t *testing.T) { + t.Helper() + binDir := t.TempDir() + for _, manager := range []string{"npm", "pnpm"} { + for _, suffix := range []string{"", ".cmd"} { + stub := testutil.ReadFile(t, "testdata/package-manager"+suffix) + require.NoError(t, os.WriteFile(filepath.Join(binDir, manager+suffix), []byte(stub), 0o755)) + } + } + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv(validationTestFailEnv, "") +} + +// writeValidationProject creates a manifest with the requested scripts. +func writeValidationProject(t *testing.T, dir string, scripts []string) { + t.Helper() + scriptMap := make(map[string]string, len(scripts)) + for _, script := range scripts { + scriptMap[script] = "echo " + script + } + data, err := json.Marshal(map[string]any{"scripts": scriptMap}) + require.NoError(t, err) + testutil.WriteFile(t, filepath.Join(dir, "package.json"), string(data)) +} + +// isolateValidationShell keeps installed package managers out of PATH. +func isolateValidationShell(t *testing.T) { + t.Helper() + shell := "sh" + if runtime.GOOS == "windows" { + shell = "cmd.exe" + } + path, err := osexec.LookPath(shell) + require.NoError(t, err) + // Keep cmd.exe beside its localized message resources on Windows. + binDir := filepath.Dir(path) + if runtime.GOOS != "windows" { + binDir = t.TempDir() + require.NoError(t, os.Symlink(path, filepath.Join(binDir, shell))) + } + t.Setenv("PATH", binDir) + for _, manager := range []string{"npm", "pnpm"} { + _, err := osexec.LookPath(manager) + require.ErrorIs(t, err, osexec.ErrNotFound) + } +} + +func TestNodeJsValidatePackageManagers(t *testing.T) { + stubPackageManagers(t) + tests := []struct { + name string + manager string + lockfile string + manifest string + }{ + {name: "npm default", manager: "npm"}, + {name: "npm lockfile", manager: "npm", lockfile: "package-lock.json"}, + {name: "pnpm lockfile", manager: "pnpm", lockfile: "pnpm-lock.yaml"}, + {name: "pnpm declaration", manager: "pnpm", manifest: `{"packageManager":"pnpm@10.30.3"}`}, + {name: "npm declaration overrides lockfile", manager: "npm", lockfile: "pnpm-lock.yaml", manifest: `{"packageManager":"npm@10.9.2"}`}, + {name: "pnpm declaration overrides lockfile", manager: "pnpm", lockfile: "package-lock.json", manifest: `{"packageManager":"pnpm@10.30.3"}`}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + writeValidationProject(t, dir, validationScripts) + if tt.manifest != "" { + testutil.WriteFile(t, filepath.Join(dir, "package.json"), tt.manifest) + } + if tt.lockfile != "" { + testutil.Touch(t, dir, tt.lockfile) + } + validator := validation.ValidationNodeJs{} + result, err := validator.Validate(cmdio.MockDiscard(t.Context()), dir, validation.ValidateOptions{}) + require.NoError(t, err) + require.True(t, result.Success, "%+v", result.Details) + wantCommands := strings.ReplaceAll(npmValidationCommands, "npm ", tt.manager+" ") + commands := testutil.ReadFile(t, filepath.Join(dir, "commands.log")) + assert.Equal(t, wantCommands, strings.ReplaceAll(commands, "\r\n", "\n")) + }) + } +} + +func TestNodeJsValidateSkips(t *testing.T) { + stubPackageManagers(t) + commands := strings.ReplaceAll(npmValidationCommands, "npm ", "pnpm ") + tests := []struct { + name string + scripts []string + nodeModules bool + skipTests bool + want string + }{ + {name: "missing scripts", want: commands}, + {name: "optional scripts", scripts: []string{"build"}, want: commands}, + {name: "installed dependencies", scripts: []string{"build"}, nodeModules: true, want: strings.TrimPrefix(commands, "pnpm install\n")}, + {name: "skip tests", scripts: []string{"build", "test"}, skipTests: true, want: strings.TrimSuffix(commands, "pnpm run --if-present test\n")}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + writeValidationProject(t, dir, tt.scripts) + testutil.Touch(t, dir, "pnpm-lock.yaml") + if tt.nodeModules { + require.NoError(t, os.Mkdir(filepath.Join(dir, "node_modules"), 0o755)) + } + validator := validation.ValidationNodeJs{} + result, err := validator.Validate(cmdio.MockDiscard(t.Context()), dir, validation.ValidateOptions{SkipTests: tt.skipTests}) + require.NoError(t, err) + require.True(t, result.Success, "%+v", result.Details) + commands := testutil.ReadFile(t, filepath.Join(dir, "commands.log")) + assert.Equal(t, tt.want, strings.ReplaceAll(commands, "\r\n", "\n")) + }) + } +} + +func TestNodeJsValidateCommandFailure(t *testing.T) { + stubPackageManagers(t) + for _, command := range []string{"install", "run --if-present typecheck", "run --if-present test"} { + t.Run(command, func(t *testing.T) { + dir := t.TempDir() + writeValidationProject(t, dir, validationScripts) + testutil.Touch(t, dir, "pnpm-lock.yaml") + t.Setenv(validationTestFailEnv, command) + validator := validation.ValidationNodeJs{} + result, err := validator.Validate(cmdio.MockDiscard(t.Context()), dir, validation.ValidateOptions{}) + require.NoError(t, err) + require.False(t, result.Success) + require.NotNil(t, result.Details) + assert.Equal(t, validationTestExitCode, result.Details.ExitCode) + assert.Contains(t, result.Details.Stdout, validationTestStdout) + assert.Contains(t, result.Details.Stderr, validationTestStderr) + commands := testutil.ReadFile(t, filepath.Join(dir, "commands.log")) + assert.True(t, strings.HasSuffix(strings.ReplaceAll(commands, "\r\n", "\n"), "pnpm "+command+"\n"), commands) + }) + } +} + +func TestNodeJsValidateManagerFromRelativePath(t *testing.T) { + // Read the stub before chdir so the relative testdata path still resolves. + stub := testutil.ReadFile(t, "testdata/package-manager") + stubCmd := testutil.ReadFile(t, "testdata/package-manager.cmd") + for _, workDir := range []string{".", "app"} { + t.Run(workDir, func(t *testing.T) { + isolateValidationShell(t) + dir := t.TempDir() + projectDir := filepath.Join(dir, workDir) + // The manager is only available relative to the project's directory. + relBin := filepath.Join("node_modules", ".bin") + require.NoError(t, os.MkdirAll(filepath.Join(projectDir, relBin), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(projectDir, relBin, "pnpm"), []byte(stub), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(projectDir, relBin, "pnpm.cmd"), []byte(stubCmd), 0o755)) + writeValidationProject(t, projectDir, []string{"build"}) + testutil.Touch(t, projectDir, "pnpm-lock.yaml") + + t.Chdir(dir) + t.Setenv("PATH", relBin+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv(validationTestFailEnv, "") + + validator := validation.ValidationNodeJs{} + result, err := validator.Validate(cmdio.MockDiscard(t.Context()), workDir, validation.ValidateOptions{}) + require.NoError(t, err) + require.True(t, result.Success, "%+v", result.Details) + commands := testutil.ReadFile(t, filepath.Join(projectDir, "commands.log")) + // node_modules exists, so install is skipped and scripts run via the relative pnpm. + want := strings.ReplaceAll(strings.TrimPrefix(npmValidationCommands, "npm install\n"), "npm ", "pnpm ") + assert.Equal(t, want, strings.ReplaceAll(commands, "\r\n", "\n")) + }) + } +} + +func TestNodeJsValidateMissingPackageManager(t *testing.T) { + isolateValidationShell(t) + dir := t.TempDir() + writeValidationProject(t, dir, validationScripts) + testutil.Touch(t, dir, "pnpm-lock.yaml") + validator := validation.ValidationNodeJs{} + result, err := validator.Validate(cmdio.MockDiscard(t.Context()), dir, validation.ValidateOptions{}) + require.NoError(t, err) + require.False(t, result.Success) + require.NotNil(t, result.Details) + assert.NotZero(t, result.Details.ExitCode) + assert.Contains(t, result.Details.Stderr, "pnpm") + assert.NoFileExists(t, filepath.Join(dir, "commands.log")) +} diff --git a/libs/apps/validation/package_manager.go b/libs/apps/validation/package_manager.go new file mode 100644 index 00000000000..3d392b1e1b3 --- /dev/null +++ b/libs/apps/validation/package_manager.go @@ -0,0 +1,112 @@ +package validation + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/databricks/cli/libs/apps/packagejson" + "github.com/databricks/cli/libs/log" +) + +const ( + packageManagerNpm = "npm" + packageManagerPnpm = "pnpm" + packageManagerYarn = "yarn" + packageManagerBun = "bun" + packageManagerKey = "packageManager" +) + +// DetectPackageManager prefers package.json's packageManager declaration over lockfiles +// in workDir. Unsupported declarations and lockfiles are logged and ignored. Without a +// supported declaration, npm lockfiles take precedence over pnpm, with npm as the default. +// The selected manager must be installed to run validation. +func DetectPackageManager(ctx context.Context, workDir string) (string, error) { + lockfiles := []struct { + name string + manager string + supported bool + }{ + // https://docs.npmjs.com/cli/configuring-npm/package-lock-json + // https://docs.npmjs.com/cli/configuring-npm/npm-shrinkwrap-json + {"package-lock.json", packageManagerNpm, true}, + {"npm-shrinkwrap.json", packageManagerNpm, true}, + // https://pnpm.io/git + {"pnpm-lock.yaml", packageManagerPnpm, true}, + // https://classic.yarnpkg.com/lang/en/docs/yarn-lock/ + {"yarn.lock", packageManagerYarn, false}, + // https://bun.sh/docs/install/lockfile + {"bun.lock", packageManagerBun, false}, + {"bun.lockb", packageManagerBun, false}, + } + + // Lockfiles are listed npm-first, so the first supported one found wins and keeps + // npm's precedence when there is no supported packageManager declaration. + manager := readDeclaredPackageManager(ctx, workDir) + chosen := packagejson.FileName + for _, lockfile := range lockfiles { + info, err := os.Stat(filepath.Join(workDir, lockfile.name)) + if errors.Is(err, os.ErrNotExist) { + continue + } + if !lockfile.supported { + log.Warnf(ctx, "ignoring unsupported lockfile %s; apps validation uses %s or %s", lockfile.name, packageManagerNpm, packageManagerPnpm) + continue + } + if err != nil { + return "", fmt.Errorf("failed to inspect %s: %w", lockfile.name, err) + } + if !info.Mode().IsRegular() { + return "", fmt.Errorf("lockfile %s must be a regular file", lockfile.name) + } + if manager == "" { + manager, chosen = lockfile.manager, lockfile.name + continue + } + if lockfile.manager != manager { + log.Warnf(ctx, "found conflicting package managers; using %s from %s and ignoring %s", manager, chosen, lockfile.name) + } + } + + if manager == "" { + return packageManagerNpm, nil + } + return manager, nil +} + +// readDeclaredPackageManager leaves manifest validation to the selected package manager. +func readDeclaredPackageManager(ctx context.Context, workDir string) string { + fields, err := packagejson.Read(workDir) + if err != nil { + // Preserve native manifest diagnostics instead of failing during detection. + log.Debugf(ctx, "cannot read package manager metadata: %v", err) + return "" + } + value, ok := fields[packageManagerKey] + if !ok { + return "" + } + var declaration string + if err := json.Unmarshal(value, &declaration); err != nil { + log.Warnf(ctx, "ignoring invalid %s in %s; expected a string", packageManagerKey, packagejson.FileName) + return "" + } + if declaration == "" { + return "" + } + + // Only select the tool; its version policy remains native: + // https://nodejs.org/api/packages.html#packagemanager. + manager, _, _ := strings.Cut(declaration, "@") + switch manager { + case packageManagerNpm, packageManagerPnpm: + return manager + default: + log.Warnf(ctx, "ignoring unsupported %s %q in %s; apps validation uses %s or %s", packageManagerKey, declaration, packagejson.FileName, packageManagerNpm, packageManagerPnpm) + return "" + } +} diff --git a/libs/apps/validation/package_manager_test.go b/libs/apps/validation/package_manager_test.go new file mode 100644 index 00000000000..ff3233ba7a5 --- /dev/null +++ b/libs/apps/validation/package_manager_test.go @@ -0,0 +1,219 @@ +package validation_test + +import ( + "bytes" + "log/slog" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/apps/packagejson" + "github.com/databricks/cli/libs/apps/validation" + "github.com/databricks/cli/libs/log" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDetectPackageManager(t *testing.T) { + tests := []struct { + name string + lockfiles []string + want string + }{ + {name: "no lockfile", want: "npm"}, + {name: "npm", lockfiles: []string{"package-lock.json"}, want: "npm"}, + {name: "npm shrinkwrap", lockfiles: []string{"npm-shrinkwrap.json"}, want: "npm"}, + {name: "pnpm", lockfiles: []string{"pnpm-lock.yaml"}, want: "pnpm"}, + {name: "npm aliases", lockfiles: []string{"package-lock.json", "npm-shrinkwrap.json"}, want: "npm"}, + // Unsupported and conflicting lockfiles are ignored (with a warning), never fatal. + {name: "unsupported yarn", lockfiles: []string{"yarn.lock"}, want: "npm"}, + {name: "unsupported bun", lockfiles: []string{"bun.lock"}, want: "npm"}, + {name: "unsupported bun binary", lockfiles: []string{"bun.lockb"}, want: "npm"}, + {name: "unsupported manager alongside pnpm", lockfiles: []string{"pnpm-lock.yaml", "yarn.lock"}, want: "pnpm"}, + {name: "conflicting managers prefer npm", lockfiles: []string{"pnpm-lock.yaml", "package-lock.json"}, want: "npm"}, + {name: "conflict with npm aliases", lockfiles: []string{"pnpm-lock.yaml", "package-lock.json", "npm-shrinkwrap.json"}, want: "npm"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + for _, lockfile := range tt.lockfiles { + testutil.Touch(t, dir, lockfile) + } + manager, err := validation.DetectPackageManager(t.Context(), dir) + require.NoError(t, err) + assert.Equal(t, tt.want, manager) + }) + } +} + +func TestDetectPackageManagerInvalidLockfile(t *testing.T) { + for _, lockfile := range []string{"package-lock.json", "npm-shrinkwrap.json", "pnpm-lock.yaml"} { + t.Run(lockfile, func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, lockfile), 0o755)) + _, err := validation.DetectPackageManager(t.Context(), dir) + require.EqualError(t, err, "lockfile "+lockfile+" must be a regular file") + }) + } +} + +func TestDetectPackageManagerDeclaration(t *testing.T) { + tests := []struct { + name string + manifest string + lockfiles []string + want string + warnings []string + }{ + {name: "npm without lockfile", manifest: `{"packageManager":"npm@10.9.2"}`, want: "npm"}, + {name: "pnpm without lockfile", manifest: `{"packageManager":"pnpm@10.30.3"}`, want: "pnpm"}, + {name: "name without version", manifest: `{"packageManager":"pnpm"}`, want: "pnpm"}, + {name: "version remains native", manifest: `{"packageManager":"pnpm@invalid-version"}`, want: "pnpm"}, + { + name: "npm overrides pnpm lockfile", + manifest: `{"packageManager":"npm@10.9.2"}`, + lockfiles: []string{"pnpm-lock.yaml"}, + want: "npm", + warnings: []string{"using npm from package.json and ignoring pnpm-lock.yaml"}, + }, + { + name: "pnpm overrides npm lockfiles", + manifest: `{"packageManager":"pnpm@10.30.3"}`, + lockfiles: []string{"package-lock.json", "npm-shrinkwrap.json", "pnpm-lock.yaml"}, + want: "pnpm", + warnings: []string{ + "using pnpm from package.json and ignoring package-lock.json", + "using pnpm from package.json and ignoring npm-shrinkwrap.json", + }, + }, + {name: "matching lockfile", manifest: `{"packageManager":"pnpm@10.30.3"}`, lockfiles: []string{"pnpm-lock.yaml"}, want: "pnpm"}, + {name: "missing field", manifest: `{}`, lockfiles: []string{"pnpm-lock.yaml"}, want: "pnpm"}, + {name: "empty field", manifest: `{"packageManager":""}`, lockfiles: []string{"pnpm-lock.yaml"}, want: "pnpm"}, + {name: "null field", manifest: `{"packageManager":null}`, lockfiles: []string{"pnpm-lock.yaml"}, want: "pnpm"}, + {name: "exact field name", manifest: `{"PackageManager":"pnpm@10.30.3"}`, want: "npm"}, + {name: "devEngines is not used", manifest: `{"devEngines":{"packageManager":{"name":"pnpm"}}}`, want: "npm"}, + {name: "scripts are not interpreted", manifest: `{"packageManager":"pnpm@10.30.3","scripts":{"typecheck":null,"//":["a comment"]}}`, want: "pnpm"}, + {name: "BOM", manifest: "\xef\xbb\xbf" + `{"packageManager":"pnpm@10.30.3"}`, want: "pnpm"}, + { + name: "unsupported declaration uses lockfiles", + manifest: `{"packageManager":"yarn@4.0.0"}`, + lockfiles: []string{"pnpm-lock.yaml"}, + want: "pnpm", + warnings: []string{"ignoring unsupported packageManager"}, + }, + { + name: "unsupported declaration uses npm default", + manifest: `{"packageManager":"bun@1.0.0"}`, + want: "npm", + warnings: []string{"ignoring unsupported packageManager"}, + }, + { + name: "exact manager name", + manifest: `{"packageManager":"PNPM@10.30.3"}`, + want: "npm", + warnings: []string{"ignoring unsupported packageManager"}, + }, + { + name: "non-string declaration uses lockfiles", + manifest: `{"packageManager":{"name":"npm"}}`, + lockfiles: []string{"pnpm-lock.yaml"}, + want: "pnpm", + warnings: []string{"ignoring invalid packageManager"}, + }, + {name: "malformed manifest uses npm default", manifest: `{"packageManager":"pnpm@10.30.3",}`, want: "npm"}, + {name: "malformed manifest uses lockfiles", manifest: `{"packageManager":"npm@10.9.2",}`, lockfiles: []string{"pnpm-lock.yaml"}, want: "pnpm"}, + {name: "null manifest stays native", manifest: `null`, lockfiles: []string{"pnpm-lock.yaml"}, want: "pnpm"}, + {name: "non-object manifest stays native", manifest: `[]`, lockfiles: []string{"pnpm-lock.yaml"}, want: "pnpm"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + testutil.WriteFile(t, filepath.Join(dir, packagejson.FileName), tt.manifest) + for _, lockfile := range tt.lockfiles { + testutil.Touch(t, dir, lockfile) + } + var warnings bytes.Buffer + ctx := log.NewContext(t.Context(), slog.New(slog.NewTextHandler(&warnings, nil))) + manager, err := validation.DetectPackageManager(ctx, dir) + require.NoError(t, err) + assert.Equal(t, tt.want, manager) + assert.Equal(t, len(tt.warnings), strings.Count(warnings.String(), "level=WARN"), warnings.String()) + for _, warning := range tt.warnings { + assert.Contains(t, warnings.String(), warning) + } + }) + } +} + +func TestDetectPackageManagerDeclarationInvalidLockfile(t *testing.T) { + dir := t.TempDir() + testutil.WriteFile(t, filepath.Join(dir, packagejson.FileName), `{"packageManager":"npm@10.9.2"}`) + require.NoError(t, os.Mkdir(filepath.Join(dir, "pnpm-lock.yaml"), 0o755)) + _, err := validation.DetectPackageManager(t.Context(), dir) + require.EqualError(t, err, "lockfile pnpm-lock.yaml must be a regular file") +} + +func TestDetectPackageManagerDeclarationReadError(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, packagejson.FileName), 0o755)) + testutil.Touch(t, dir, "pnpm-lock.yaml") + manager, err := validation.DetectPackageManager(t.Context(), dir) + require.NoError(t, err) + assert.Equal(t, "pnpm", manager) +} + +func TestDetectPackageManagerDeclarationProjectDirectory(t *testing.T) { + dir := t.TempDir() + testutil.WriteFile(t, filepath.Join(dir, packagejson.FileName), `{"packageManager":"pnpm@10.30.3"}`) + t.Chdir(dir) + for _, tt := range []struct { + name string + manifest string + lockfile string + }{ + {name: "declared", manifest: `{"packageManager":"npm@10.9.2"}`, lockfile: "pnpm-lock.yaml"}, + {name: "no-parent-inheritance", manifest: `{}`}, + } { + t.Run(tt.name, func(t *testing.T) { + testutil.WriteFile(t, filepath.Join(tt.name, packagejson.FileName), tt.manifest) + if tt.lockfile != "" { + testutil.Touch(t, tt.name, tt.lockfile) + } + manager, err := validation.DetectPackageManager(t.Context(), tt.name) + require.NoError(t, err) + assert.Equal(t, "npm", manager) + }) + } +} + +func TestDetectPackageManagerUnsupportedLockfileDirectory(t *testing.T) { + for _, lockfile := range []string{"yarn.lock", "bun.lock", "bun.lockb"} { + for _, manager := range []string{"npm", "pnpm"} { + t.Run(lockfile+"/"+manager, func(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, lockfile), 0o755)) + if manager == "pnpm" { + testutil.Touch(t, dir, "pnpm-lock.yaml") + } + var warnings bytes.Buffer + ctx := log.NewContext(t.Context(), slog.New(slog.NewTextHandler(&warnings, nil))) + actual, err := validation.DetectPackageManager(ctx, dir) + require.NoError(t, err) + assert.Equal(t, manager, actual) + assert.Contains(t, warnings.String(), "ignoring unsupported lockfile "+lockfile) + }) + } + } +} + +func TestDetectPackageManagerProjectDirectory(t *testing.T) { + dir := t.TempDir() + testutil.Touch(t, dir, "pnpm-lock.yaml") + projectDir := filepath.Join(dir, "app") + testutil.Touch(t, projectDir, "package-lock.json") + manager, err := validation.DetectPackageManager(t.Context(), projectDir) + require.NoError(t, err) + assert.Equal(t, "npm", manager) +} diff --git a/libs/apps/validation/testdata/package-manager b/libs/apps/validation/testdata/package-manager new file mode 100644 index 00000000000..d452c009e29 --- /dev/null +++ b/libs/apps/validation/testdata/package-manager @@ -0,0 +1,7 @@ +#!/bin/sh +printf '%s %s\n' "${0##*/}" "$*" >> commands.log +if [ "$*" = "${VALIDATION_TEST_FAIL:-}" ]; then + echo "validation stdout" + echo "validation stderr" >&2 + exit 17 +fi diff --git a/libs/apps/validation/testdata/package-manager.cmd b/libs/apps/validation/testdata/package-manager.cmd new file mode 100644 index 00000000000..1e7de72f5dd --- /dev/null +++ b/libs/apps/validation/testdata/package-manager.cmd @@ -0,0 +1,7 @@ +@echo off +echo %~n0 %*>>commands.log +if "%*"=="%VALIDATION_TEST_FAIL%" ( + echo validation stdout + echo validation stderr 1>&2 + exit /b 17 +) diff --git a/libs/apps/validation/validation.go b/libs/apps/validation/validation.go index 7438653d2a0..7eefff387e7 100644 --- a/libs/apps/validation/validation.go +++ b/libs/apps/validation/validation.go @@ -6,6 +6,8 @@ import ( "os" "path/filepath" "strings" + + "github.com/databricks/cli/libs/apps/packagejson" ) // ValidationDetail contains detailed output from a failed validation. @@ -69,7 +71,7 @@ type Validation interface { // Returns nil if no validator is applicable. func GetProjectValidator(workDir string) Validation { // Check for Node.js project (package.json exists) - packageJSON := filepath.Join(workDir, "package.json") + packageJSON := filepath.Join(workDir, packagejson.FileName) if _, err := os.Stat(packageJSON); err == nil { return &ValidationNodeJs{} }