From 55cf6795e886d0445671436b5ad57b6f42fb8302 Mon Sep 17 00:00:00 2001 From: Willie Ruemmele Date: Mon, 30 Oct 2023 10:17:03 -0600 Subject: [PATCH] chore: review comments --- .gitignore | 2 -- package.json | 5 ++--- src/shared/npmCommand.ts | 8 ++++---- test/shared/npmCommand.test.ts | 14 ++++++++++++-- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 9b08d8e3..4843d1fa 100644 --- a/.gitignore +++ b/.gitignore @@ -46,5 +46,3 @@ node_modules oclif.manifest.json oclif.lock - -oclif.lock diff --git a/package.json b/package.json index d77051da..7d471034 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,6 @@ "/lib", "/messages", "/oclif.manifest.json", - "/oclif.lock", "/oclif.lock" ], "homepage": "https://github.com/salesforcecli/plugin-trust", @@ -106,12 +105,12 @@ "build": "wireit", "clean": "sf-clean", "clean-all": "sf-clean all", - "clean:lib": "shx rm -rf lib && shx rm -rf coverage && shx rm -rf .nyc_output && shx rm -f oclif.manifest.json", + "clean:lib": "shx rm -rf lib && shx rm -rf coverage && shx rm -rf .nyc_output && shx rm -f oclif.manifest.json oclif.lock", "compile": "wireit", "docs": "sf-docs", "format": "wireit", "lint": "wireit", - "postpack": "shx rm -f oclif.manifest.json", + "postpack": "shx rm -f oclif.manifest.json oclif.lock", "prepack": "sf-prepack", "prepare": "sf-install", "test": "wireit", diff --git a/src/shared/npmCommand.ts b/src/shared/npmCommand.ts index 1197c4d9..fb59832f 100644 --- a/src/shared/npmCommand.ts +++ b/src/shared/npmCommand.ts @@ -105,7 +105,7 @@ class NpmCommand { try { if (filepath.endsWith('node')) { - // This checks if the filepath is executable on Mac or Linux, if it is not it errors. + // 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; } @@ -118,7 +118,7 @@ class NpmCommand { if (root) { const sfdxBinDirs = NpmCommand.findSfdxBinDirs(root); if (sfdxBinDirs.length > 0) { - // Find the node executable + // Find the node executable const node = shelljs.find(sfdxBinDirs).filter((file) => isExecutable(file))[0]; if (node) { return fs.realpathSync(node); @@ -163,8 +163,8 @@ export class NpmModule { cliRoot: this.cliRoot, }); - // `npm show` doesn't return exit code 1 when it fails to get a specific package version - // If `s tdout` is empty then no info was found in the registry. + // `npm show` doesn't return exit code 1 when it fails to get a specific package version + // If `stdout` is empty then no info was found in the registry. if (showCmd.stdout === '') { throw setErrorName( new SfError(`Failed to find ${this.module}@${this.version} in the registry`, 'NpmError'), diff --git a/test/shared/npmCommand.test.ts b/test/shared/npmCommand.test.ts index 52155cfe..cf3e99a2 100644 --- a/test/shared/npmCommand.test.ts +++ b/test/shared/npmCommand.test.ts @@ -205,7 +205,12 @@ describe('should find the node executable', () => { 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(realpathSyncStub).to.have.been.calledTwice; + // when switching to ESM, realpathSyncStub started to be called twice + // realpathSync('C:\\Program Files\\sfdx\\client\\bin\\node.exe') + // realpathSync('/Users/william.ruemmele/projects/oss/plugin-trust/node_modules/npm/package.json' + // when placing a breakpoint at NpmCommand~124 - the only realPathSync method usage, it was only hit once while debugging this UT + expect(realpathSyncStub.firstCall.args[0]).to.include(NODE_NAME); expect(shelljsExecStub).to.have.been.calledOnce; expect(shelljsFindStub).to.have.been.calledOnce; expect(shelljsExecStub.firstCall.args[0]).to.include(NODE_PATH); @@ -224,7 +229,12 @@ describe('should find the node executable', () => { 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(realpathSyncStub).to.have.been.calledTwice; + // when switching to ESM, realpathSyncStub started to be called twice + // realpathSync('C:\\Program Files\\sfdx\\client\\bin\\node.exe') + // realpathSync('/Users/william.ruemmele/projects/oss/plugin-trust/node_modules/npm/package.json' + // when placing a breakpoint at NpmCommand~124 - the only realPathSync method usage, it was only hit once while debugging this UT + expect(realpathSyncStub.firstCall.args[0]).to.include(NODE_NAME); expect(shelljsExecStub).to.have.been.calledOnce; expect(shelljsFindStub).to.have.been.calledOnce; expect(shelljsExecStub.firstCall.args[0]).to.include(NODE_PATH);