Skip to content

Commit

Permalink
chore: review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
WillieRuemmele committed Oct 30, 2023
1 parent 9c6e0ea commit 55cf679
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 11 deletions.
2 changes: 0 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,3 @@ node_modules
oclif.manifest.json

oclif.lock

oclif.lock
5 changes: 2 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@
"/lib",
"/messages",
"/oclif.manifest.json",
"/oclif.lock",
"/oclif.lock"
],
"homepage": "https://github.com/salesforcecli/plugin-trust",
Expand Down Expand Up @@ -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",
Expand Down
8 changes: 4 additions & 4 deletions src/shared/npmCommand.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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);
Expand Down Expand Up @@ -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'),
Expand Down
14 changes: 12 additions & 2 deletions test/shared/npmCommand.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down

0 comments on commit 55cf679

Please sign in to comment.