From 2917a1dede0b7a2fc1f5caf0225793b77891cabb Mon Sep 17 00:00:00 2001 From: Peter Hale Date: Wed, 22 Sep 2021 12:01:33 -0600 Subject: [PATCH 01/20] fix: verify fails if node not installed @W-9930707@ --- src/commands/plugins/trust/verify.ts | 1 + src/hooks/verifyInstallSignature.ts | 1 + src/lib/installationVerification.ts | 7 ++- src/lib/npmCommand.ts | 77 +++++++++++++++++++++-- test/lib/installationVerification.test.ts | 15 +++++ test/lib/npmCommand.test.ts | 62 +++++++++++++----- 6 files changed, 139 insertions(+), 24 deletions(-) diff --git a/src/commands/plugins/trust/verify.ts b/src/commands/plugins/trust/verify.ts index 59b1d18c..e4cee5be 100644 --- a/src/commands/plugins/trust/verify.ts +++ b/src/commands/plugins/trust/verify.ts @@ -50,6 +50,7 @@ export class Verify extends SfdxCommand { cacheDir: get(this.config, 'configDir') as string, configDir: get(this.config, 'cacheDir') as string, dataDir: get(this.config, 'dataDir') as string, + rootDir: get(this.config, 'root') as string, }; this.logger.debug(`cacheDir: ${configContext.cacheDir}`); diff --git a/src/hooks/verifyInstallSignature.ts b/src/hooks/verifyInstallSignature.ts index 4faede6d..c2339bc3 100644 --- a/src/hooks/verifyInstallSignature.ts +++ b/src/hooks/verifyInstallSignature.ts @@ -58,6 +58,7 @@ export const hook: Hook.PluginsPreinstall = async function (options) { cacheDir: options.config.cacheDir, configDir: options.config.configDir, dataDir: options.config.dataDir, + rootDir: options.config.root, }; const vConfig = VerificationConfigBuilder.build(npmName, configContext); diff --git a/src/lib/installationVerification.ts b/src/lib/installationVerification.ts index 06aabb73..405a4765 100644 --- a/src/lib/installationVerification.ts +++ b/src/lib/installationVerification.ts @@ -29,6 +29,7 @@ export interface ConfigContext { configDir?: string; cacheDir?: string; dataDir?: string; + rootDir?: string; } export interface Verifier { verify(): Promise; @@ -306,7 +307,9 @@ export class InstallationVerification implements Verifier { // Make sure the cache path exists. try { await fs.mkdirp(this.getCachePath()); - new NpmModule(npmMeta.moduleName, npmMeta.version).pack(getNpmRegistry().href, { cwd: this.getCachePath() }); + new NpmModule(npmMeta.moduleName, npmMeta.version, this.config.rootDir).pack(getNpmRegistry().href, { + cwd: this.getCachePath(), + }); const tarBallFile = fs .readdirSync(this.getCachePath(), { withFileTypes: true }) .find((entry) => entry.isFile() && entry.name.includes(npmMeta.version)); @@ -345,7 +348,7 @@ export class InstallationVerification implements Verifier { ? `@${this.pluginNpmName.scope}/${this.pluginNpmName.name}` : this.pluginNpmName.name; - const npmModule = new NpmModule(npmShowModule, this.pluginNpmName.tag); + const npmModule = new NpmModule(npmShowModule, this.pluginNpmName.tag, this.config.rootDir); const npmMetadata = npmModule.show(npmRegistry.href); logger.debug('retrieveNpmMeta | Found npm meta information.'); if (!npmMetadata.versions) { diff --git a/src/lib/npmCommand.ts b/src/lib/npmCommand.ts index 33bf0389..c5858443 100644 --- a/src/lib/npmCommand.ts +++ b/src/lib/npmCommand.ts @@ -39,6 +39,7 @@ export type NpmShowResults = { type NpmCommandOptions = shelljs.ExecOptions & { json?: boolean; registry?: string; + root?: string; }; type NpmCommandResult = NpmShowResults & { @@ -55,7 +56,7 @@ export class NpmCommand { private static npmPkgPath = require.resolve('npm/package.json'); public static runNpmCmd(cmd: string, options = {} as NpmCommandOptions): NpmCommandResult { - const npmCli = NpmCommand.npmCli(); + const npmCli = NpmCommand.npmCli(options.root); const exec = `${npmCli} ${cmd} --registry=${options.registry} --json`; const npmShowResult = shelljs.exec(exec, { ...options, @@ -78,28 +79,92 @@ export class NpmCommand { return this.npmPkgPath; } - private static npmCli(): string { + /** + * Return a executable path to this modules reference to npm as + * + * + * @private + */ + private static npmCli(root: string = undefined): string { + const nodeBinPath = NpmCommand.findNodeBin(root); const pkgPath = NpmCommand.npmPackagePath(); const pkgJson = fs.readJsonSync(pkgPath) as NpmPackage; const prjPath = pkgPath.substring(0, pkgPath.lastIndexOf(path.sep)); - return path.join(prjPath, pkgJson.bin['npm']); + return `"${nodeBinPath}" "${path.join(prjPath, pkgJson.bin['npm'])}"`; + } + + /** + * Locate node executable and return its full path. + * First see if node is on the path, if so, use unqualified name. + * If not on the path, try to locate the node version installed with sfdx. + * If found, return full path to the executable + * If sfdx or node executable cannot be found, an exception is thrown + * + * @private + */ + private static findNodeBin(root: string = undefined): string { + let sfdxPath; + if (root) { + sfdxPath = root; + } else { + throw new SfdxError('Plugin root dir is not set', 'PluginRootNotSet'); + } + // find node within sfdx installation + const sfdxBinDirPaths = NpmCommand.getSfdxBinDirs(sfdxPath); + if (sfdxBinDirPaths && sfdxBinDirPaths.length > 0) { + const nodeBinPath = shelljs + .find(sfdxBinDirPaths) + .filter((file) => { + const fileName = path.basename(file); + const stat = fs.statSync(file); + const isExecutable = !stat.isDirectory(); + return isExecutable && (process.platform === 'win32' ? fileName === 'node.exe' : fileName === 'node'); + }) + .find((file) => file); + if (nodeBinPath) { + return fs.realpathSync(nodeBinPath); + } + } + // check to see if node is installed + const whichNode = shelljs.exec('which node', { + silent: true, + fatal: false, + async: false, + env: process.env, + }); + if (whichNode.code === 0) { + return 'node'; + } + throw new SfdxError('Cannot locate node executable within sfdx installation.', 'CannotFindNodeExecutable'); + } + + /** + * Test each potential directory exists used for sfdx installation + * + * @param sfdxPath + * @private + */ + private static getSfdxBinDirs(sfdxPath: string): string[] { + return sfdxPath + ? [path.join(sfdxPath, 'bin'), path.join(sfdxPath, 'config,', 'bin')].filter((p) => fs.existsSync(p)) + : []; } } export class NpmModule { public npmMeta: NpmMeta; - public constructor(private module: string, private version: string = 'latest') { + public constructor(private module: string, private version: string = 'latest', private root: string = undefined) { this.npmMeta = { moduleName: module, }; } public show(registry: string): NpmShowResults { - return NpmCommand.runNpmCmd(`show ${this.module}@${this.version}`, { registry }); + return NpmCommand.runNpmCmd(`show ${this.module}@${this.version}`, { registry, root: this.root }); } public pack(registry: string, options?: shelljs.ExecOptions): void { - NpmCommand.runNpmCmd(`pack ${this.module}@${this.version}`, { ...options, registry }); + NpmCommand.runNpmCmd(`pack ${this.module}@${this.version}`, { ...options, registry, root: this.root }); return; } } diff --git a/test/lib/installationVerification.test.ts b/test/lib/installationVerification.test.ts index 7a020826..6254c100 100644 --- a/test/lib/installationVerification.test.ts +++ b/test/lib/installationVerification.test.ts @@ -68,6 +68,18 @@ const getShelljsExecStub = ( stderr, stdout: JSON.stringify(PACK_RESULT), }; + } else if (cmd.includes('node')) { + return { + code: 0, + stderr, + stdout: 'node', + }; + } else if (cmd.includes('sfdx')) { + return { + code: 0, + stderr, + stdout: 'sfdx', + }; } else { throw new Error(`Unexpected test cmd - ${cmd}`); } @@ -105,6 +117,9 @@ describe('InstallationVerification Tests', () => { get configDir() { return 'configDir'; }, + get rootDir() { + return __dirname; + }, }; const currentRegistry = process.env.SFDX_NPM_REGISTRY; let plugin: NpmName; diff --git a/test/lib/npmCommand.test.ts b/test/lib/npmCommand.test.ts index 4ec7df56..5da83a62 100644 --- a/test/lib/npmCommand.test.ts +++ b/test/lib/npmCommand.test.ts @@ -67,6 +67,16 @@ describe('should run npm commands', () => { code: 0, stdout: JSON.stringify(PACK_RESULT), }; + } else if (cmd.includes('node')) { + return { + code: 0, + stdout: 'node', + }; + } else if (cmd.includes('sfdx')) { + return { + code: 0, + stdout: 'sfdx', + }; } else { throw new Error(`Unexpected test cmd - ${cmd}`); } @@ -78,26 +88,26 @@ describe('should run npm commands', () => { }); it('Runs the show command', () => { - const npmMetadata = new NpmModule(MODULE_NAME).show(DEFAULT_REGISTRY); - expect(shelljsExecStub.callCount).to.equal(1); - expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@latest`); - expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); + const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); + expect(shelljsExecStub.callCount).to.equal(2); + expect(shelljsExecStub.secondCall.args[0]).to.include(`show ${MODULE_NAME}@latest`); + expect(shelljsExecStub.secondCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); expect(npmMetadata).to.deep.equal(SHOW_RESULT); }); it('Runs the show command with specified version', () => { - const npmMetadata = new NpmModule(MODULE_NAME, MODULE_VERSION).show(DEFAULT_REGISTRY); - expect(shelljsExecStub.callCount).to.equal(1); - expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@${MODULE_VERSION}`); - expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); + const npmMetadata = new NpmModule(MODULE_NAME, MODULE_VERSION, __dirname).show(DEFAULT_REGISTRY); + expect(shelljsExecStub.callCount).to.equal(2); + expect(shelljsExecStub.secondCall.args[0]).to.include(`show ${MODULE_NAME}@${MODULE_VERSION}`); + expect(shelljsExecStub.secondCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); expect(npmMetadata).to.deep.equal(SHOW_RESULT); }); it('Runs the pack command', () => { - new NpmModule(MODULE_NAME, MODULE_VERSION).pack(DEFAULT_REGISTRY, { cwd: CACHE_PATH }); - expect(shelljsExecStub.callCount).to.equal(1); - expect(shelljsExecStub.firstCall.args[0]).to.include(`pack ${MODULE_NAME}@${MODULE_VERSION}`); - expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); + new NpmModule(MODULE_NAME, MODULE_VERSION, __dirname).pack(DEFAULT_REGISTRY, { cwd: CACHE_PATH }); + expect(shelljsExecStub.callCount).to.equal(2); + expect(shelljsExecStub.secondCall.args[0]).to.include(`pack ${MODULE_NAME}@${MODULE_VERSION}`); + expect(shelljsExecStub.secondCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); }); }); @@ -120,6 +130,16 @@ describe('should run npm commands with execution errors', () => { stderr: 'command execution error', stdout: '', }; + } else if (cmd.includes('node')) { + return { + code: 0, + stdout: 'node', + }; + } else if (cmd.includes('sfdx')) { + return { + code: 0, + stdout: 'sfdx', + }; } else { throw new Error(`Unexpected test cmd - ${cmd}`); } @@ -132,7 +152,7 @@ describe('should run npm commands with execution errors', () => { it('show command throws error', () => { try { - const npmMetadata = new NpmModule(MODULE_NAME).show(DEFAULT_REGISTRY); + const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); expect(npmMetadata).to.be.undefined; fail('Error'); } catch (error) { @@ -142,7 +162,7 @@ describe('should run npm commands with execution errors', () => { it('Runs the pack command', () => { try { - new NpmModule(MODULE_NAME, MODULE_VERSION).pack(DEFAULT_REGISTRY, { cwd: CACHE_PATH }); + new NpmModule(MODULE_NAME, MODULE_VERSION, __dirname).pack(DEFAULT_REGISTRY, { cwd: CACHE_PATH }); fail('Error'); } catch (error) { expect(error.code).to.equal('ShellExecError'); @@ -167,6 +187,16 @@ describe('should run npm commands with parse errors', () => { code: 0, stdout: 'not a json string', }; + } else if (cmd.includes('node')) { + return { + code: 0, + stdout: 'node', + }; + } else if (cmd.includes('sfdx')) { + return { + code: 0, + stdout: 'sfdx', + }; } else { throw new Error(`Unexpected test cmd - ${cmd}`); } @@ -179,7 +209,7 @@ describe('should run npm commands with parse errors', () => { it('show command throws error', () => { try { - const npmMetadata = new NpmModule(MODULE_NAME).show(DEFAULT_REGISTRY); + const npmMetadata = new NpmModule(MODULE_NAME, MODULE_VERSION, __dirname).show(DEFAULT_REGISTRY); expect(npmMetadata).to.be.undefined; fail('Error'); } catch (error) { @@ -189,7 +219,7 @@ describe('should run npm commands with parse errors', () => { it('Runs the pack command', () => { try { - new NpmModule(MODULE_NAME, MODULE_VERSION).pack(DEFAULT_REGISTRY, { cwd: CACHE_PATH }); + new NpmModule(MODULE_NAME, MODULE_VERSION, __dirname).pack(DEFAULT_REGISTRY, { cwd: CACHE_PATH }); fail('Error'); } catch (error) { expect(error.code).to.equal('ShellParseError'); From b317549079c4de2f6c6e0f21879ddbc9b6f04a43 Mon Sep 17 00:00:00 2001 From: Benjamin Maggi Date: Wed, 22 Sep 2021 19:56:26 -0300 Subject: [PATCH 02/20] chore: code cleanup --- src/commands/plugins/trust/verify.ts | 6 +++--- src/hooks/verifyInstallSignature.ts | 2 +- src/lib/installationVerification.ts | 6 +++--- src/lib/npmCommand.ts | 8 +------- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/commands/plugins/trust/verify.ts b/src/commands/plugins/trust/verify.ts index e4cee5be..0aa51c26 100644 --- a/src/commands/plugins/trust/verify.ts +++ b/src/commands/plugins/trust/verify.ts @@ -47,10 +47,10 @@ export class Verify extends SfdxCommand { const vConfig = new VerificationConfig(); const configContext: ConfigContext = { - cacheDir: get(this.config, 'configDir') as string, - configDir: get(this.config, 'cacheDir') as string, + cacheDir: get(this.config, 'cacheDir') as string, + configDir: get(this.config, 'configDir') as string, dataDir: get(this.config, 'dataDir') as string, - rootDir: get(this.config, 'root') as string, + cliRoot: get(this.config, 'root') as string, }; this.logger.debug(`cacheDir: ${configContext.cacheDir}`); diff --git a/src/hooks/verifyInstallSignature.ts b/src/hooks/verifyInstallSignature.ts index c2339bc3..08ccf64e 100644 --- a/src/hooks/verifyInstallSignature.ts +++ b/src/hooks/verifyInstallSignature.ts @@ -58,7 +58,7 @@ export const hook: Hook.PluginsPreinstall = async function (options) { cacheDir: options.config.cacheDir, configDir: options.config.configDir, dataDir: options.config.dataDir, - rootDir: options.config.root, + cliRoot: options.config.root, }; const vConfig = VerificationConfigBuilder.build(npmName, configContext); diff --git a/src/lib/installationVerification.ts b/src/lib/installationVerification.ts index 405a4765..8b8d3ad3 100644 --- a/src/lib/installationVerification.ts +++ b/src/lib/installationVerification.ts @@ -29,7 +29,7 @@ export interface ConfigContext { configDir?: string; cacheDir?: string; dataDir?: string; - rootDir?: string; + cliRoot?: string; } export interface Verifier { verify(): Promise; @@ -307,7 +307,7 @@ export class InstallationVerification implements Verifier { // Make sure the cache path exists. try { await fs.mkdirp(this.getCachePath()); - new NpmModule(npmMeta.moduleName, npmMeta.version, this.config.rootDir).pack(getNpmRegistry().href, { + new NpmModule(npmMeta.moduleName, npmMeta.version, this.config.cliRoot).pack(getNpmRegistry().href, { cwd: this.getCachePath(), }); const tarBallFile = fs @@ -348,7 +348,7 @@ export class InstallationVerification implements Verifier { ? `@${this.pluginNpmName.scope}/${this.pluginNpmName.name}` : this.pluginNpmName.name; - const npmModule = new NpmModule(npmShowModule, this.pluginNpmName.tag, this.config.rootDir); + const npmModule = new NpmModule(npmShowModule, this.pluginNpmName.tag, this.config.cliRoot); const npmMetadata = npmModule.show(npmRegistry.href); logger.debug('retrieveNpmMeta | Found npm meta information.'); if (!npmMetadata.versions) { diff --git a/src/lib/npmCommand.ts b/src/lib/npmCommand.ts index c5858443..57f2a11d 100644 --- a/src/lib/npmCommand.ts +++ b/src/lib/npmCommand.ts @@ -126,13 +126,7 @@ export class NpmCommand { } } // check to see if node is installed - const whichNode = shelljs.exec('which node', { - silent: true, - fatal: false, - async: false, - env: process.env, - }); - if (whichNode.code === 0) { + if (shelljs.which('node')) { return 'node'; } throw new SfdxError('Cannot locate node executable within sfdx installation.', 'CannotFindNodeExecutable'); From 4b4815dacb7e7f00c23a3c421a9dfcce98159586 Mon Sep 17 00:00:00 2001 From: Benjamin Maggi Date: Wed, 22 Sep 2021 20:00:50 -0300 Subject: [PATCH 03/20] chore: code cleanup --- test/lib/installationVerification.test.ts | 2 +- test/lib/npmCommand.test.ts | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/test/lib/installationVerification.test.ts b/test/lib/installationVerification.test.ts index 6254c100..f6c07806 100644 --- a/test/lib/installationVerification.test.ts +++ b/test/lib/installationVerification.test.ts @@ -117,7 +117,7 @@ describe('InstallationVerification Tests', () => { get configDir() { return 'configDir'; }, - get rootDir() { + get cliRoot() { return __dirname; }, }; diff --git a/test/lib/npmCommand.test.ts b/test/lib/npmCommand.test.ts index 5da83a62..d24b090c 100644 --- a/test/lib/npmCommand.test.ts +++ b/test/lib/npmCommand.test.ts @@ -89,25 +89,25 @@ describe('should run npm commands', () => { it('Runs the show command', () => { const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); - expect(shelljsExecStub.callCount).to.equal(2); - expect(shelljsExecStub.secondCall.args[0]).to.include(`show ${MODULE_NAME}@latest`); - expect(shelljsExecStub.secondCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); + expect(shelljsExecStub.callCount).to.equal(1); + expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@latest`); + expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); expect(npmMetadata).to.deep.equal(SHOW_RESULT); }); it('Runs the show command with specified version', () => { const npmMetadata = new NpmModule(MODULE_NAME, MODULE_VERSION, __dirname).show(DEFAULT_REGISTRY); - expect(shelljsExecStub.callCount).to.equal(2); - expect(shelljsExecStub.secondCall.args[0]).to.include(`show ${MODULE_NAME}@${MODULE_VERSION}`); - expect(shelljsExecStub.secondCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); + expect(shelljsExecStub.callCount).to.equal(1); + expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@${MODULE_VERSION}`); + expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); expect(npmMetadata).to.deep.equal(SHOW_RESULT); }); it('Runs the pack command', () => { new NpmModule(MODULE_NAME, MODULE_VERSION, __dirname).pack(DEFAULT_REGISTRY, { cwd: CACHE_PATH }); - expect(shelljsExecStub.callCount).to.equal(2); - expect(shelljsExecStub.secondCall.args[0]).to.include(`pack ${MODULE_NAME}@${MODULE_VERSION}`); - expect(shelljsExecStub.secondCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); + expect(shelljsExecStub.callCount).to.equal(1); + expect(shelljsExecStub.firstCall.args[0]).to.include(`pack ${MODULE_NAME}@${MODULE_VERSION}`); + expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); }); }); From 7ae1c5e529001ef6ca193c78a8f539d94ac9aecb Mon Sep 17 00:00:00 2001 From: Benjamin Maggi Date: Wed, 22 Sep 2021 22:51:51 -0300 Subject: [PATCH 04/20] test: node in slim an global --- test/lib/npmCommand.test.ts | 122 ++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/test/lib/npmCommand.test.ts b/test/lib/npmCommand.test.ts index d24b090c..67d20bf0 100644 --- a/test/lib/npmCommand.test.ts +++ b/test/lib/npmCommand.test.ts @@ -10,6 +10,7 @@ import { expect } from 'chai'; import Sinon = require('sinon'); import * as shelljs from 'shelljs'; import { stubMethod } from '@salesforce/ts-sinon'; +import { fs } from '@salesforce/core'; import { NpmModule } from '../../src/lib/npmCommand'; const DEFAULT_REGISTRY = 'https://registry.npmjs.org/'; @@ -48,6 +49,8 @@ const PACK_RESULT = [ ], }, ]; +const NODE_NAME = 'node'; +const NODE_PATH = `/usr/local/sfdx/bin/${NODE_NAME}`; describe('should run npm commands', () => { let sandbox: sinon.SinonSandbox; @@ -111,6 +114,125 @@ describe('should run npm commands', () => { }); }); +describe('should find the node executable', () => { + let sandbox: sinon.SinonSandbox; + let shelljsExecStub: Sinon.SinonStub; + let shelljsFindStub: Sinon.SinonStub; + let shelljsWhichStub: Sinon.SinonStub; + let existsSyncStub: Sinon.SinonStub; + let statSyncStub: Sinon.SinonStub; + let realpathSyncStub: Sinon.SinonStub; + + beforeEach(() => { + sandbox = Sinon.createSandbox(); + shelljsExecStub = stubMethod(sandbox, shelljs, 'exec').callsFake((cmd: string) => { + expect(cmd).to.be.a('string').and.not.to.be.empty; + if (cmd.includes('show')) { + return { + code: 0, + stdout: JSON.stringify(SHOW_RESULT), + }; + } else if (cmd.includes('pack')) { + return { + code: 0, + stdout: JSON.stringify(PACK_RESULT), + }; + } else if (cmd.includes('node')) { + return { + code: 0, + stdout: 'node', + }; + } else if (cmd.includes('sfdx')) { + return { + code: 0, + stdout: 'sfdx', + }; + } else { + throw new Error(`Unexpected test cmd - ${cmd}`); + } + }); + shelljsFindStub = stubMethod(sandbox, shelljs, 'find').callsFake((filePaths: string[]) => { + expect(filePaths).to.be.a('array').and.to.have.length.greaterThan(0); + return [NODE_PATH]; + }); + statSyncStub = stubMethod(sandbox, fs, 'statSync').callsFake((filePath: string) => { + expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0); + return { + isDirectory() { + return false; + }, + }; + }); + realpathSyncStub = stubMethod(sandbox, fs, 'realpathSync').callsFake((filePath: string) => { + expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0); + return NODE_PATH; + }); + existsSyncStub = stubMethod(sandbox, fs, 'existsSync').callsFake((filePath: string) => { + expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0); + return true; + }); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it('finds node binary inside sfdx bin folder and runs npm show command', () => { + const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); + expect(existsSyncStub.callCount).to.equal(2); + expect(shelljsFindStub.callCount).to.equal(1); + expect(statSyncStub.callCount).to.equal(1); + expect(realpathSyncStub.callCount).to.equal(1); + expect(shelljsExecStub.callCount).to.equal(1); + expect(shelljsExecStub.firstCall.args[0]).to.include(NODE_PATH); + expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@latest`); + expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); + expect(npmMetadata).to.deep.equal(SHOW_RESULT); + }); + + it('fails to find node binary inside sfdx bin folder and falls back to global node and runs npm show command', () => { + existsSyncStub.restore(); + existsSyncStub = stubMethod(sandbox, fs, 'existsSync').callsFake((filePath: string) => { + expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0); + return false; + }); + shelljsWhichStub = stubMethod(sandbox, shelljs, 'which').callsFake((filePath: string) => { + expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0).and.to.be.equal('node'); + return NODE_PATH; + }); + const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); + expect(existsSyncStub.callCount).to.equal(2); + expect(shelljsFindStub.callCount).to.equal(0); + expect(statSyncStub.callCount).to.equal(0); + expect(realpathSyncStub.callCount).to.equal(0); + expect(shelljsWhichStub.callCount).to.equal(1); + expect(shelljsExecStub.callCount).to.equal(1); + expect(shelljsExecStub.firstCall.args[0]).to.include(NODE_NAME); + expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@latest`); + expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); + expect(npmMetadata).to.deep.equal(SHOW_RESULT); + }); + + it('fails to find node binary and throws', () => { + existsSyncStub.restore(); + existsSyncStub = stubMethod(sandbox, fs, 'existsSync').callsFake((filePath: string) => { + expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0); + return false; + }); + shelljsWhichStub = stubMethod(sandbox, shelljs, 'which').callsFake((filePath: string) => { + expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0).and.to.be.equal('node'); + return null; + }); + try { + const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); + expect(npmMetadata).to.be.undefined; + fail('Error'); + } catch (error) { + expect(error.code).to.equal('CannotFindNodeExecutable'); + } + }); +}); + describe('should run npm commands with execution errors', () => { let sandbox: sinon.SinonSandbox; From b0563f513cf3a7bbf9f7f9b1af2431b0addaf8d1 Mon Sep 17 00:00:00 2001 From: Benjamin Maggi Date: Wed, 22 Sep 2021 22:53:18 -0300 Subject: [PATCH 05/20] chore: code clean + path for win --- src/lib/npmCommand.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib/npmCommand.ts b/src/lib/npmCommand.ts index 57f2a11d..b5e57ccc 100644 --- a/src/lib/npmCommand.ts +++ b/src/lib/npmCommand.ts @@ -39,7 +39,7 @@ export type NpmShowResults = { type NpmCommandOptions = shelljs.ExecOptions & { json?: boolean; registry?: string; - root?: string; + cliRoot?: string; }; type NpmCommandResult = NpmShowResults & { @@ -56,7 +56,7 @@ export class NpmCommand { private static npmPkgPath = require.resolve('npm/package.json'); public static runNpmCmd(cmd: string, options = {} as NpmCommandOptions): NpmCommandResult { - const npmCli = NpmCommand.npmCli(options.root); + const npmCli = NpmCommand.npmCli(options.cliRoot); const exec = `${npmCli} ${cmd} --registry=${options.registry} --json`; const npmShowResult = shelljs.exec(exec, { ...options, @@ -140,25 +140,25 @@ export class NpmCommand { */ private static getSfdxBinDirs(sfdxPath: string): string[] { return sfdxPath - ? [path.join(sfdxPath, 'bin'), path.join(sfdxPath, 'config,', 'bin')].filter((p) => fs.existsSync(p)) + ? [path.join(sfdxPath, 'bin'), path.join(sfdxPath, 'client', 'bin')].filter((p) => fs.existsSync(p)) : []; } } export class NpmModule { public npmMeta: NpmMeta; - public constructor(private module: string, private version: string = 'latest', private root: string = undefined) { + public constructor(private module: string, private version: string = 'latest', private cliRoot: string = undefined) { this.npmMeta = { moduleName: module, }; } public show(registry: string): NpmShowResults { - return NpmCommand.runNpmCmd(`show ${this.module}@${this.version}`, { registry, root: this.root }); + return NpmCommand.runNpmCmd(`show ${this.module}@${this.version}`, { registry, cliRoot: this.cliRoot }); } public pack(registry: string, options?: shelljs.ExecOptions): void { - NpmCommand.runNpmCmd(`pack ${this.module}@${this.version}`, { ...options, registry, root: this.root }); + NpmCommand.runNpmCmd(`pack ${this.module}@${this.version}`, { ...options, registry, cliRoot: this.cliRoot }); return; } } From 8e2e7ed147797f0f6b20cd1fa82637846395230e Mon Sep 17 00:00:00 2001 From: Benjamin Date: Thu, 23 Sep 2021 17:50:00 -0300 Subject: [PATCH 06/20] Update src/lib/npmCommand.ts Co-authored-by: Shane McLaughlin --- src/lib/npmCommand.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/npmCommand.ts b/src/lib/npmCommand.ts index b5e57ccc..c0e44f51 100644 --- a/src/lib/npmCommand.ts +++ b/src/lib/npmCommand.ts @@ -111,7 +111,7 @@ export class NpmCommand { } // find node within sfdx installation const sfdxBinDirPaths = NpmCommand.getSfdxBinDirs(sfdxPath); - if (sfdxBinDirPaths && sfdxBinDirPaths.length > 0) { + if (sfdxBinDirPaths?.length > 0) { const nodeBinPath = shelljs .find(sfdxBinDirPaths) .filter((file) => { From d8cd5879078fb81903a2dc1a5f22b699eb3f46bf Mon Sep 17 00:00:00 2001 From: Benjamin Date: Thu, 23 Sep 2021 19:06:17 -0300 Subject: [PATCH 07/20] Update src/lib/npmCommand.ts Co-authored-by: Shane McLaughlin --- src/lib/npmCommand.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/lib/npmCommand.ts b/src/lib/npmCommand.ts index c0e44f51..cbffcc25 100644 --- a/src/lib/npmCommand.ts +++ b/src/lib/npmCommand.ts @@ -103,12 +103,9 @@ export class NpmCommand { * @private */ private static findNodeBin(root: string = undefined): string { - let sfdxPath; - if (root) { - sfdxPath = root; - } else { + if (!root) { throw new SfdxError('Plugin root dir is not set', 'PluginRootNotSet'); - } + } // find node within sfdx installation const sfdxBinDirPaths = NpmCommand.getSfdxBinDirs(sfdxPath); if (sfdxBinDirPaths?.length > 0) { From 68b6965a7934d1d9fbd5c5e429d696b36c1de7a7 Mon Sep 17 00:00:00 2001 From: Rodrigo Espinosa de los Monteros <1084688+RodEsp@users.noreply.github.com> Date: Thu, 23 Sep 2021 20:09:34 -0400 Subject: [PATCH 08/20] chore: streamline code & update some comments --- src/lib/npmCommand.ts | 67 ++++++++++------------- test/lib/installationVerification.test.ts | 8 ++- test/lib/npmCommand.test.ts | 24 ++++---- 3 files changed, 48 insertions(+), 51 deletions(-) diff --git a/src/lib/npmCommand.ts b/src/lib/npmCommand.ts index cbffcc25..0da158ba 100644 --- a/src/lib/npmCommand.ts +++ b/src/lib/npmCommand.ts @@ -56,9 +56,10 @@ export class NpmCommand { private static npmPkgPath = require.resolve('npm/package.json'); public static runNpmCmd(cmd: string, options = {} as NpmCommandOptions): NpmCommandResult { - const npmCli = NpmCommand.npmCli(options.cliRoot); - const exec = `${npmCli} ${cmd} --registry=${options.registry} --json`; - const npmShowResult = shelljs.exec(exec, { + const nodeExecutable = NpmCommand.findNode(options.cliRoot); + const npmCli = NpmCommand.npmCli(); + const command = `"${nodeExecutable}" "${npmCli}" ${cmd} --registry=${options.registry} --json`; + const npmShowResult = shelljs.exec(command, { ...options, silent: true, fatal: true, @@ -80,62 +81,52 @@ export class NpmCommand { } /** - * Return a executable path to this modules reference to npm as - * + * Returns the path to the npm-cli.js file in this package's node_modules * * @private */ - private static npmCli(root: string = undefined): string { - const nodeBinPath = NpmCommand.findNodeBin(root); + private static npmCli(): string { const pkgPath = NpmCommand.npmPackagePath(); const pkgJson = fs.readJsonSync(pkgPath) as NpmPackage; const prjPath = pkgPath.substring(0, pkgPath.lastIndexOf(path.sep)); - return `"${nodeBinPath}" "${path.join(prjPath, pkgJson.bin['npm'])}"`; + return `${path.join(prjPath, pkgJson.bin['npm'])}`; } /** - * Locate node executable and return its full path. - * First see if node is on the path, if so, use unqualified name. - * If not on the path, try to locate the node version installed with sfdx. - * If found, return full path to the executable - * If sfdx or node executable cannot be found, an exception is thrown + * Locate node executable and return its absolute path + * First it tries to locate the node executable on the root path passed in + * If not found then tries to use whatver 'node' resolves to on the user's PATH + * If found return absolute path to the executable + * If the node executable cannot be found, an error is thrown * * @private */ - private static findNodeBin(root: string = undefined): string { - if (!root) { - throw new SfdxError('Plugin root dir is not set', 'PluginRootNotSet'); - } - // find node within sfdx installation - const sfdxBinDirPaths = NpmCommand.getSfdxBinDirs(sfdxPath); - if (sfdxBinDirPaths?.length > 0) { - const nodeBinPath = shelljs - .find(sfdxBinDirPaths) - .filter((file) => { - const fileName = path.basename(file); - const stat = fs.statSync(file); - const isExecutable = !stat.isDirectory(); - return isExecutable && (process.platform === 'win32' ? fileName === 'node.exe' : fileName === 'node'); - }) - .find((file) => file); - if (nodeBinPath) { - return fs.realpathSync(nodeBinPath); + private static findNode(root: string = undefined): string { + if (root) { + const sfdxBinDirs = NpmCommand.findSfdxBinDirs(root); + if (sfdxBinDirs.length > 0) { + // Find the node executable + const node = shelljs.find(sfdxBinDirs).find((file) => file.includes('node')); + if (node) { + return fs.realpathSync(node); + } } } - // check to see if node is installed - if (shelljs.which('node')) { - return 'node'; - } - throw new SfdxError('Cannot locate node executable within sfdx installation.', 'CannotFindNodeExecutable'); + + // Check to see if node is installed + const nodeShellString: shelljs.ShellString = shelljs.which('node'); + if (nodeShellString?.code === 0 && nodeShellString?.stdout) return nodeShellString.stdout; + + throw new SfdxError('Cannot locate node executable.', 'CannotFindNodeExecutable'); } /** - * Test each potential directory exists used for sfdx installation + * Finds the bin directory in the sfdx installation root path * * @param sfdxPath * @private */ - private static getSfdxBinDirs(sfdxPath: string): string[] { + private static findSfdxBinDirs(sfdxPath: string): string[] { return sfdxPath ? [path.join(sfdxPath, 'bin'), path.join(sfdxPath, 'client', 'bin')].filter((p) => fs.existsSync(p)) : []; diff --git a/test/lib/installationVerification.test.ts b/test/lib/installationVerification.test.ts index f6c07806..39c12443 100644 --- a/test/lib/installationVerification.test.ts +++ b/test/lib/installationVerification.test.ts @@ -122,10 +122,12 @@ describe('InstallationVerification Tests', () => { }, }; const currentRegistry = process.env.SFDX_NPM_REGISTRY; + let fsReaddirSyncStub: Sinon.SinonStub; let plugin: NpmName; + let realpathSyncStub: Sinon.SinonStub; let sandbox: sinon.SinonSandbox; let shelljsExecStub: Sinon.SinonStub; - let fsReaddirSyncStub: Sinon.SinonStub; + let shelljsFindStub: Sinon.SinonStub; beforeEach(() => { sandbox = Sinon.createSandbox(); @@ -137,11 +139,15 @@ describe('InstallationVerification Tests', () => { }, }, ]); + realpathSyncStub = stubMethod(sandbox, fs, 'realpathSync').returns('node.exe'); + shelljsFindStub = stubMethod(sandbox, shelljs, 'find').returns(['node.exe']); plugin = NpmName.parse('foo'); }); afterEach(() => { fsReaddirSyncStub.restore(); + realpathSyncStub.restore(); + shelljsFindStub.restore(); if (shelljsExecStub) { shelljsExecStub.restore(); } diff --git a/test/lib/npmCommand.test.ts b/test/lib/npmCommand.test.ts index 67d20bf0..3bcd5924 100644 --- a/test/lib/npmCommand.test.ts +++ b/test/lib/npmCommand.test.ts @@ -54,10 +54,14 @@ const NODE_PATH = `/usr/local/sfdx/bin/${NODE_NAME}`; describe('should run npm commands', () => { let sandbox: sinon.SinonSandbox; + let realpathSyncStub: Sinon.SinonStub; let shelljsExecStub: Sinon.SinonStub; + let shelljsFindStub: Sinon.SinonStub; beforeEach(() => { sandbox = Sinon.createSandbox(); + realpathSyncStub = stubMethod(sandbox, fs, 'realpathSync').returns('node.exe'); + shelljsFindStub = stubMethod(sandbox, shelljs, 'find').returns(['node.exe']); shelljsExecStub = stubMethod(sandbox, shelljs, 'exec').callsFake((cmd: string) => { expect(cmd).to.be.a('string').and.not.to.be.empty; if (cmd.includes('show')) { @@ -87,6 +91,9 @@ describe('should run npm commands', () => { }); afterEach(() => { + realpathSyncStub.restore(); + shelljsFindStub.restore(); + shelljsExecStub.restore(); sandbox.restore(); }); @@ -120,7 +127,6 @@ describe('should find the node executable', () => { let shelljsFindStub: Sinon.SinonStub; let shelljsWhichStub: Sinon.SinonStub; let existsSyncStub: Sinon.SinonStub; - let statSyncStub: Sinon.SinonStub; let realpathSyncStub: Sinon.SinonStub; beforeEach(() => { @@ -155,14 +161,6 @@ describe('should find the node executable', () => { expect(filePaths).to.be.a('array').and.to.have.length.greaterThan(0); return [NODE_PATH]; }); - statSyncStub = stubMethod(sandbox, fs, 'statSync').callsFake((filePath: string) => { - expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0); - return { - isDirectory() { - return false; - }, - }; - }); realpathSyncStub = stubMethod(sandbox, fs, 'realpathSync').callsFake((filePath: string) => { expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0); return NODE_PATH; @@ -181,7 +179,6 @@ describe('should find the node executable', () => { const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); expect(existsSyncStub.callCount).to.equal(2); expect(shelljsFindStub.callCount).to.equal(1); - expect(statSyncStub.callCount).to.equal(1); expect(realpathSyncStub.callCount).to.equal(1); expect(shelljsExecStub.callCount).to.equal(1); expect(shelljsExecStub.firstCall.args[0]).to.include(NODE_PATH); @@ -198,12 +195,14 @@ describe('should find the node executable', () => { }); shelljsWhichStub = stubMethod(sandbox, shelljs, 'which').callsFake((filePath: string) => { expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0).and.to.be.equal('node'); - return NODE_PATH; + return { + stdout: NODE_PATH, + code: 0, + } as shelljs.ShellString; }); const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); expect(existsSyncStub.callCount).to.equal(2); expect(shelljsFindStub.callCount).to.equal(0); - expect(statSyncStub.callCount).to.equal(0); expect(realpathSyncStub.callCount).to.equal(0); expect(shelljsWhichStub.callCount).to.equal(1); expect(shelljsExecStub.callCount).to.equal(1); @@ -219,6 +218,7 @@ describe('should find the node executable', () => { expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0); return false; }); + shelljsWhichStub.restore(); shelljsWhichStub = stubMethod(sandbox, shelljs, 'which').callsFake((filePath: string) => { expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0).and.to.be.equal('node'); return null; From 8f6cd2d43b7ce3675eb3a7722ec9740235e139e9 Mon Sep 17 00:00:00 2001 From: Rodrigo Espinosa de los Monteros <1084688+RodEsp@users.noreply.github.com> Date: Thu, 23 Sep 2021 20:14:31 -0400 Subject: [PATCH 09/20] fix: remmove unnecessary string template --- src/lib/npmCommand.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/npmCommand.ts b/src/lib/npmCommand.ts index 0da158ba..acf7c76e 100644 --- a/src/lib/npmCommand.ts +++ b/src/lib/npmCommand.ts @@ -89,7 +89,7 @@ export class NpmCommand { const pkgPath = NpmCommand.npmPackagePath(); const pkgJson = fs.readJsonSync(pkgPath) as NpmPackage; const prjPath = pkgPath.substring(0, pkgPath.lastIndexOf(path.sep)); - return `${path.join(prjPath, pkgJson.bin['npm'])}`; + return path.join(prjPath, pkgJson.bin['npm']); } /** From 7a48f87db91741dd2735d6e85919ea0a2945c9f1 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Fri, 24 Sep 2021 10:12:49 -0500 Subject: [PATCH 10/20] test: run nuts from CI --- .circleci/config.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8729ec07..3575d1ca 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -60,6 +60,17 @@ workflows: node_version: lts - os: windows node_version: maintenance + - release-management/test-nut: + name: nuts-on-linux + sfdx_version: latest + requires: + - release-management/test-package + - release-management/test-nut: + name: nuts-on-windows + sfdx_version: latest + os: windows + requires: + - release-management/test-package - release-management/release-package: sign: true github-release: true From 6cc997edc99c4152e3414869767d7c2b6e8a64b0 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Fri, 24 Sep 2021 10:27:15 -0500 Subject: [PATCH 11/20] test: can't use answers to prompts on windows --- .circleci/config.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3575d1ca..45f184c8 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -65,12 +65,6 @@ workflows: sfdx_version: latest requires: - release-management/test-package - - release-management/test-nut: - name: nuts-on-windows - sfdx_version: latest - os: windows - requires: - - release-management/test-package - release-management/release-package: sign: true github-release: true From ee3a86e1456d1192bdf4923f62974d7be7df001c Mon Sep 17 00:00:00 2001 From: Rodrigo Espinosa de los Monteros <1084688+RodEsp@users.noreply.github.com> Date: Fri, 24 Sep 2021 14:01:42 -0400 Subject: [PATCH 12/20] fix: make sure node filepath is executable --- package.json | 2 ++ src/lib/npmCommand.ts | 22 +++++++++++++++++-- test/lib/npmCommand.test.ts | 42 +++++++++++++++++++++++++++++++------ yarn.lock | 13 ++++++++++++ 4 files changed, 71 insertions(+), 8 deletions(-) diff --git a/package.json b/package.json index 0e9aae4e..c68fb76d 100644 --- a/package.json +++ b/package.json @@ -26,6 +26,7 @@ "@salesforce/prettier-config": "^0.0.2", "@salesforce/ts-sinon": "1.3.21", "@types/shelljs": "^0.8.9", + "@types/sinon-chai": "^3.2.5", "@typescript-eslint/eslint-plugin": "^4.2.0", "@typescript-eslint/parser": "^4.2.0", "chai": "^4.2.0", @@ -47,6 +48,7 @@ "pretty-quick": "^3.1.0", "shx": "0.3.3", "sinon": "10.0.0", + "sinon-chai": "^3.7.0", "ts-node": "^10.0.0", "typescript": "^4.1.3" }, diff --git a/src/lib/npmCommand.ts b/src/lib/npmCommand.ts index acf7c76e..aa9890f0 100644 --- a/src/lib/npmCommand.ts +++ b/src/lib/npmCommand.ts @@ -6,9 +6,12 @@ */ /* eslint-disable @typescript-eslint/no-unused-vars */ +import { type as osType } from 'os'; import * as path from 'path'; -import * as shelljs from 'shelljs'; + import npmRunPath from 'npm-run-path'; +import * as shelljs from 'shelljs'; + import { SfdxError, fs } from '@salesforce/core'; export type NpmMeta = { @@ -102,11 +105,26 @@ export class NpmCommand { * @private */ private static findNode(root: string = undefined): string { + const isExecutable = (filepath: string): boolean => { + if (osType() === 'Windows_NT') return filepath.endsWith('node.exe'); + + try { + if (filepath.endsWith('node')) { + // This checks if the filepath is executable on Mac or Linux, if it is not it errors. + fs.accessSync(filepath, fs.constants.X_OK); + return true; + } + } catch { + return false; + } + return false; + }; + if (root) { const sfdxBinDirs = NpmCommand.findSfdxBinDirs(root); if (sfdxBinDirs.length > 0) { // Find the node executable - const node = shelljs.find(sfdxBinDirs).find((file) => file.includes('node')); + const node = shelljs.find(sfdxBinDirs).filter((file) => isExecutable(file))[0]; if (node) { return fs.realpathSync(node); } diff --git a/test/lib/npmCommand.test.ts b/test/lib/npmCommand.test.ts index 3bcd5924..dc18f0a7 100644 --- a/test/lib/npmCommand.test.ts +++ b/test/lib/npmCommand.test.ts @@ -6,13 +6,17 @@ */ import { fail } from 'assert'; -import { expect } from 'chai'; -import Sinon = require('sinon'); +import * as os from 'os'; +import { expect, use as chaiUse } from 'chai'; +import * as Sinon from 'sinon'; +import * as SinonChai from 'sinon-chai'; import * as shelljs from 'shelljs'; import { stubMethod } from '@salesforce/ts-sinon'; import { fs } from '@salesforce/core'; import { NpmModule } from '../../src/lib/npmCommand'; +chaiUse(SinonChai); + const DEFAULT_REGISTRY = 'https://registry.npmjs.org/'; const MODULE_NAME = '@salesforce/plugin-source'; const MODULE_VERSION = '1.0.0'; @@ -128,6 +132,8 @@ describe('should find the node executable', () => { let shelljsWhichStub: Sinon.SinonStub; let existsSyncStub: Sinon.SinonStub; let realpathSyncStub: Sinon.SinonStub; + let osTypeStub: Sinon.SinonStub; + let accessSyncStub: Sinon.SinonStub; beforeEach(() => { sandbox = Sinon.createSandbox(); @@ -169,6 +175,11 @@ describe('should find the node executable', () => { expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0); return true; }); + accessSyncStub = stubMethod(sandbox, fs, 'accessSync').callsFake((filePath: string) => { + expect(filePath).to.be.a('string').and.to.have.length.greaterThan(0); + return undefined; + }); + osTypeStub = stubMethod(sandbox, os, 'type').callsFake(() => 'Linux'); }); afterEach(() => { @@ -177,10 +188,29 @@ describe('should find the node executable', () => { it('finds node binary inside sfdx bin folder and runs npm show command', () => { const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); - expect(existsSyncStub.callCount).to.equal(2); - expect(shelljsFindStub.callCount).to.equal(1); - expect(realpathSyncStub.callCount).to.equal(1); - expect(shelljsExecStub.callCount).to.equal(1); + expect(accessSyncStub).to.have.been.calledOnce; + expect(existsSyncStub).to.have.been.calledTwice; + expect(osTypeStub).to.have.been.calledOnce; + expect(realpathSyncStub).to.have.been.calledOnce; + expect(shelljsExecStub).to.have.been.calledOnce; + expect(shelljsFindStub).to.have.been.calledOnce; + expect(shelljsExecStub.firstCall.args[0]).to.include(NODE_PATH); + expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@latest`); + expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); + expect(npmMetadata).to.deep.equal(SHOW_RESULT); + }); + + it('finds node binary inside sfdx bin folder on windows and runs npm show command', () => { + shelljsFindStub.returns(['C:\\Program Files\\sfdx\\client\\bin\\node.exe']); + osTypeStub.returns('Windows_NT'); + + const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); + expect(accessSyncStub).to.not.have.been.called; + expect(existsSyncStub).to.have.been.calledTwice; + expect(osTypeStub).to.have.been.calledOnce; + expect(realpathSyncStub).to.have.been.calledOnce; + expect(shelljsExecStub).to.have.been.calledOnce; + expect(shelljsFindStub).to.have.been.calledOnce; expect(shelljsExecStub.firstCall.args[0]).to.include(NODE_PATH); expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@latest`); expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); diff --git a/yarn.lock b/yarn.lock index 1e844854..413ea59d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1050,6 +1050,14 @@ "@types/glob" "*" "@types/node" "*" +"@types/sinon-chai@^3.2.5": + version "3.2.5" + resolved "https://registry.yarnpkg.com/@types/sinon-chai/-/sinon-chai-3.2.5.tgz#df21ae57b10757da0b26f512145c065f2ad45c48" + integrity sha512-bKQqIpew7mmIGNRlxW6Zli/QVyc3zikpGzCa797B/tRnD9OtHvZ/ts8sYXV+Ilj9u3QRaUEM8xrjgd1gwm1BpQ== + dependencies: + "@types/chai" "*" + "@types/sinon" "*" + "@types/sinon@*", "@types/sinon@10.0.0": version "10.0.0" resolved "https://registry.yarnpkg.com/@types/sinon/-/sinon-10.0.0.tgz#eecc3847af03d45ffe53d55aaaaf6ecb28b5e584" @@ -6913,6 +6921,11 @@ signal-exit@^3.0.0, signal-exit@^3.0.2, signal-exit@^3.0.3: resolved "https://registry.npmjs.org/signal-exit/-/signal-exit-3.0.3.tgz#a1410c2edd8f077b08b4e253c8eacfcaf057461c" integrity sha512-VUJ49FC8U1OxwZLxIbTTrDvLnf/6TDgxZcK8wxR8zs13xpx7xbG60ndBlhNrFi2EMuFRoeDoJO7wthSLq42EjA== +sinon-chai@^3.7.0: + version "3.7.0" + resolved "https://registry.yarnpkg.com/sinon-chai/-/sinon-chai-3.7.0.tgz#cfb7dec1c50990ed18c153f1840721cf13139783" + integrity sha512-mf5NURdUaSdnatJx3uhoBOrY9dtL19fiOtAdT1Azxg3+lNJFiuN0uzaU3xX1LeAfL17kHQhTAJgpsfhbMJMY2g== + sinon@10.0.0: version "10.0.0" resolved "https://registry.yarnpkg.com/sinon/-/sinon-10.0.0.tgz#52279f97e35646ff73d23207d0307977c9b81430" From 5463ea28dbb99708cea6681a647ce13663ec20aa Mon Sep 17 00:00:00 2001 From: Rodrigo Espinosa de los Monteros <1084688+RodEsp@users.noreply.github.com> Date: Fri, 24 Sep 2021 14:13:50 -0400 Subject: [PATCH 13/20] chore: update unit tests so error messages are more descriptive --- test/lib/npmCommand.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/lib/npmCommand.test.ts b/test/lib/npmCommand.test.ts index dc18f0a7..b55d314d 100644 --- a/test/lib/npmCommand.test.ts +++ b/test/lib/npmCommand.test.ts @@ -103,7 +103,7 @@ describe('should run npm commands', () => { it('Runs the show command', () => { const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); - expect(shelljsExecStub.callCount).to.equal(1); + expect(shelljsExecStub).to.have.been.calledOnce; expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@latest`); expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); expect(npmMetadata).to.deep.equal(SHOW_RESULT); @@ -111,7 +111,7 @@ describe('should run npm commands', () => { it('Runs the show command with specified version', () => { const npmMetadata = new NpmModule(MODULE_NAME, MODULE_VERSION, __dirname).show(DEFAULT_REGISTRY); - expect(shelljsExecStub.callCount).to.equal(1); + expect(shelljsExecStub).to.have.been.calledOnce; expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@${MODULE_VERSION}`); expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); expect(npmMetadata).to.deep.equal(SHOW_RESULT); @@ -119,7 +119,7 @@ describe('should run npm commands', () => { it('Runs the pack command', () => { new NpmModule(MODULE_NAME, MODULE_VERSION, __dirname).pack(DEFAULT_REGISTRY, { cwd: CACHE_PATH }); - expect(shelljsExecStub.callCount).to.equal(1); + expect(shelljsExecStub).to.have.been.calledOnce; expect(shelljsExecStub.firstCall.args[0]).to.include(`pack ${MODULE_NAME}@${MODULE_VERSION}`); expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); }); @@ -231,11 +231,11 @@ describe('should find the node executable', () => { } as shelljs.ShellString; }); const npmMetadata = new NpmModule(MODULE_NAME, undefined, __dirname).show(DEFAULT_REGISTRY); - expect(existsSyncStub.callCount).to.equal(2); - expect(shelljsFindStub.callCount).to.equal(0); - expect(realpathSyncStub.callCount).to.equal(0); - expect(shelljsWhichStub.callCount).to.equal(1); - expect(shelljsExecStub.callCount).to.equal(1); + expect(existsSyncStub).to.have.been.calledTwice; + expect(shelljsFindStub).to.not.have.been.called; + expect(realpathSyncStub).to.not.have.been.called; + expect(shelljsWhichStub).to.have.been.calledOnce; + expect(shelljsExecStub).to.have.been.calledOnce; expect(shelljsExecStub.firstCall.args[0]).to.include(NODE_NAME); expect(shelljsExecStub.firstCall.args[0]).to.include(`show ${MODULE_NAME}@latest`); expect(shelljsExecStub.firstCall.args[0]).to.include(`--registry=${DEFAULT_REGISTRY}`); From 60444392cb905333a712b793cedb723fcab8671c Mon Sep 17 00:00:00 2001 From: Shane McLaughlin Date: Fri, 24 Sep 2021 13:20:55 -0500 Subject: [PATCH 14/20] test: run nuts on branches, partial nuts on windows (#168) --- .circleci/config.yml | 6 +++++ test/nuts/plugin-install.nut.ts | 43 +++++++++++++++++++-------------- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 45f184c8..3575d1ca 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -65,6 +65,12 @@ workflows: sfdx_version: latest requires: - release-management/test-package + - release-management/test-nut: + name: nuts-on-windows + sfdx_version: latest + os: windows + requires: + - release-management/test-package - release-management/release-package: sign: true github-release: true diff --git a/test/nuts/plugin-install.nut.ts b/test/nuts/plugin-install.nut.ts index b08f72c9..927097e1 100644 --- a/test/nuts/plugin-install.nut.ts +++ b/test/nuts/plugin-install.nut.ts @@ -6,6 +6,7 @@ */ import * as path from 'path'; +import * as os from 'os'; import { expect } from 'chai'; import { TestSession, execCmd } from '@salesforce/cli-plugins-testkit'; import { fs } from '@salesforce/core'; @@ -33,27 +34,33 @@ describe('plugins:install commands', () => { }); it('plugins:install prompts on unsigned plugin (denies)', () => { - process.env.TESTKIT_EXECUTABLE_PATH = 'sfdx'; - const result = execCmd(`plugins:install ${UNSIGNED_MODULE_NAME}`, { - ensureExitCode: 2, // code 2 is the output code for the NO answer - answers: ['N'], - }); - expect(result.shellOutput.stderr).to.contain( - 'This plugin is not digitally signed and its authenticity cannot be verified. Continue installation y/n?:' - ); - expect(result.shellOutput.stderr).to.contain('The user canceled the plugin installation'); + // windows does not support answering the prompt + if (os.type() !== 'Windows_NT') { + process.env.TESTKIT_EXECUTABLE_PATH = 'sfdx'; + const result = execCmd(`plugins:install ${UNSIGNED_MODULE_NAME}`, { + ensureExitCode: 2, // code 2 is the output code for the NO answer + answers: ['N'], + }); + expect(result.shellOutput.stderr).to.contain( + 'This plugin is not digitally signed and its authenticity cannot be verified. Continue installation y/n?:' + ); + expect(result.shellOutput.stderr).to.contain('The user canceled the plugin installation'); + } }); it('plugins:install prompts on unsigned plugin (accepts)', () => { - process.env.TESTKIT_EXECUTABLE_PATH = 'sfdx'; - const result = execCmd(`plugins:install ${UNSIGNED_MODULE_NAME}`, { - ensureExitCode: 0, - answers: ['Y'], - }); - expect(result.shellOutput.stderr).to.contain( - 'This plugin is not digitally signed and its authenticity cannot be verified. Continue installation y/n?:' - ); - expect(result.shellOutput.stdout).to.contain('Finished digital signature check'); + // windows does not support answering the prompt + if (os.type() !== 'Windows_NT') { + process.env.TESTKIT_EXECUTABLE_PATH = 'sfdx'; + const result = execCmd(`plugins:install ${UNSIGNED_MODULE_NAME}`, { + ensureExitCode: 0, + answers: ['Y'], + }); + expect(result.shellOutput.stderr).to.contain( + 'This plugin is not digitally signed and its authenticity cannot be verified. Continue installation y/n?:' + ); + expect(result.shellOutput.stdout).to.contain('Finished digital signature check'); + } }); }); From c290d41a04dc74218328418a7f46daee35a88c3b Mon Sep 17 00:00:00 2001 From: mshanemc Date: Fri, 24 Sep 2021 14:00:10 -0500 Subject: [PATCH 15/20] test: add telemetry ack file --- test/nuts/plugin-install.nut.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/nuts/plugin-install.nut.ts b/test/nuts/plugin-install.nut.ts index 927097e1..93d3940b 100644 --- a/test/nuts/plugin-install.nut.ts +++ b/test/nuts/plugin-install.nut.ts @@ -18,6 +18,12 @@ let session: TestSession; describe('plugins:install commands', () => { before(async () => { session = await TestSession.create(); + await fs.writeJson( + path.join(session.homeDir, '.sfdx'), + JSON.stringify({ + acknowledged: true, + }) + ); }); after(async () => { @@ -67,6 +73,12 @@ describe('plugins:install commands', () => { describe('plugins:install commands', () => { before(async () => { session = await TestSession.create(); + await fs.writeJson( + path.join(session.homeDir, '.sfdx'), + JSON.stringify({ + acknowledged: true, + }) + ); const configDir = path.join(session.homeDir, '.config', 'sfdx'); fs.mkdirSync(configDir, { recursive: true }); fs.writeJsonSync(path.join(configDir, 'unsignedPluginAllowList.json'), [UNSIGNED_MODULE_NAME]); From 47c6f23aa8dd1dfbab3fbef4c34cb624e98085a3 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Fri, 24 Sep 2021 14:08:23 -0500 Subject: [PATCH 16/20] test: ignore errors on deleting testProjects --- test/nuts/plugin-install.nut.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/nuts/plugin-install.nut.ts b/test/nuts/plugin-install.nut.ts index 93d3940b..7850be49 100644 --- a/test/nuts/plugin-install.nut.ts +++ b/test/nuts/plugin-install.nut.ts @@ -28,7 +28,11 @@ describe('plugins:install commands', () => { after(async () => { await session?.zip(undefined, 'artifacts'); - await session?.clean(); + try { + await session?.clean(); + } catch (error) { + // ignore + } }); it('plugins:install signed plugin', () => { @@ -91,7 +95,11 @@ describe('plugins:install commands', () => { after(async () => { await session?.zip(undefined, 'artifacts'); - await session?.clean(); + try { + await session?.clean(); + } catch (error) { + // ignore + } }); it('plugins:install unsigned plugin in the allow list', () => { From 8d07d3b6663044af3106fcb66cc24a6881d26169 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Fri, 24 Sep 2021 14:31:43 -0500 Subject: [PATCH 17/20] test: store correct filename --- package.json | 2 +- test/nuts/plugin-install.nut.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index c68fb76d..41913bc7 100644 --- a/package.json +++ b/package.json @@ -116,7 +116,7 @@ "test": "sf-test", "test:command-reference": "./bin/run commandreference:generate --erroronwarnings", "test:deprecation-policy": "./bin/run snapshot:compare", - "test:nuts": "nyc mocha \"**/*.nut.ts\" --slow 4500 --timeout 600000 --parallel", + "test:nuts": "nyc mocha \"**/*.nut.ts\" --slow 4500 --timeout 600000", "version": "oclif-dev readme" }, "husky": { diff --git a/test/nuts/plugin-install.nut.ts b/test/nuts/plugin-install.nut.ts index 7850be49..7a48b7f6 100644 --- a/test/nuts/plugin-install.nut.ts +++ b/test/nuts/plugin-install.nut.ts @@ -19,7 +19,7 @@ describe('plugins:install commands', () => { before(async () => { session = await TestSession.create(); await fs.writeJson( - path.join(session.homeDir, '.sfdx'), + path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), JSON.stringify({ acknowledged: true, }) @@ -78,7 +78,7 @@ describe('plugins:install commands', () => { before(async () => { session = await TestSession.create(); await fs.writeJson( - path.join(session.homeDir, '.sfdx'), + path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), JSON.stringify({ acknowledged: true, }) From 861de03a79ee9841920359e99d9d50c34b6e3b4c Mon Sep 17 00:00:00 2001 From: mshanemc Date: Fri, 24 Sep 2021 14:44:41 -0500 Subject: [PATCH 18/20] test: can't write to non-existent dir --- test/nuts/plugin-install.nut.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/nuts/plugin-install.nut.ts b/test/nuts/plugin-install.nut.ts index 7a48b7f6..be77b094 100644 --- a/test/nuts/plugin-install.nut.ts +++ b/test/nuts/plugin-install.nut.ts @@ -18,6 +18,7 @@ let session: TestSession; describe('plugins:install commands', () => { before(async () => { session = await TestSession.create(); + await fs.mkdirp(path.join(session.homeDir, '.sfdx')); await fs.writeJson( path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), JSON.stringify({ @@ -77,6 +78,7 @@ describe('plugins:install commands', () => { describe('plugins:install commands', () => { before(async () => { session = await TestSession.create(); + await fs.mkdirp(path.join(session.homeDir, '.sfdx')); await fs.writeJson( path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), JSON.stringify({ From 48d6a064cab912c58cdf4c3d142f98c0cf10272d Mon Sep 17 00:00:00 2001 From: mshanemc Date: Fri, 24 Sep 2021 14:51:43 -0500 Subject: [PATCH 19/20] test: smaller test plugin, json in ack file --- test/nuts/plugin-install.nut.ts | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/test/nuts/plugin-install.nut.ts b/test/nuts/plugin-install.nut.ts index be77b094..f42a7cf1 100644 --- a/test/nuts/plugin-install.nut.ts +++ b/test/nuts/plugin-install.nut.ts @@ -12,19 +12,16 @@ import { TestSession, execCmd } from '@salesforce/cli-plugins-testkit'; import { fs } from '@salesforce/core'; const SIGNED_MODULE_NAME = '@salesforce/plugin-user'; -const UNSIGNED_MODULE_NAME = 'sfdx-jayree'; +const UNSIGNED_MODULE_NAME = '@mshanemc/plugin-streaming'; let session: TestSession; describe('plugins:install commands', () => { before(async () => { session = await TestSession.create(); await fs.mkdirp(path.join(session.homeDir, '.sfdx')); - await fs.writeJson( - path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), - JSON.stringify({ - acknowledged: true, - }) - ); + await fs.writeJson(path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), { + acknowledged: true, + }); }); after(async () => { @@ -79,12 +76,9 @@ describe('plugins:install commands', () => { before(async () => { session = await TestSession.create(); await fs.mkdirp(path.join(session.homeDir, '.sfdx')); - await fs.writeJson( - path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), - JSON.stringify({ - acknowledged: true, - }) - ); + await fs.writeJson(path.join(session.homeDir, '.sfdx', 'acknowledgedUsageCollection.json'), { + acknowledged: true, + }); const configDir = path.join(session.homeDir, '.config', 'sfdx'); fs.mkdirSync(configDir, { recursive: true }); fs.writeJsonSync(path.join(configDir, 'unsignedPluginAllowList.json'), [UNSIGNED_MODULE_NAME]); From 971809de17665fe37657effde3139fac820ac597 Mon Sep 17 00:00:00 2001 From: mshanemc Date: Fri, 24 Sep 2021 15:14:28 -0500 Subject: [PATCH 20/20] test: no nuts for windows --- .circleci/config.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 3575d1ca..72318c1a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -65,12 +65,12 @@ workflows: sfdx_version: latest requires: - release-management/test-package - - release-management/test-nut: - name: nuts-on-windows - sfdx_version: latest - os: windows - requires: - - release-management/test-package + # - release-management/test-nut: + # name: nuts-on-windows + # sfdx_version: latest + # os: windows + # requires: + # - release-management/test-package - release-management/release-package: sign: true github-release: true