fix: parsing to string and UT (tag is never used)

salesforcecli · Apr 4, 2024 · 1414853 · 1414853
1 parent 577a3af
commit 1414853
Show file tree

Hide file tree

Showing 4 changed files with 288 additions and 284 deletions.
diff --git a/src/shared/NpmName.ts b/src/shared/NpmName.ts
@@ -34,8 +34,8 @@ export const parseNpmName = (npmName: string): NpmName => {
 };
 
 /** Produces a formatted string version of the object */
-export const npmNameToString = (npmName: NpmName, includeTag = false): string =>
-  `${npmName.scope ? `@${npmName.scope}/` : ''}${npmName.name}${includeTag ? npmName.tag : ''}`;
+export const npmNameToString = (npmName: NpmName): string =>
+  `${npmName.scope ? `@${npmName.scope}/` : ''}${npmName.name}`;
 
 const validateNpmNameAndRemoveLeadingAt = (input: string): string => {
   const nameWithoutAt = input.startsWith('@') ? input.slice(1) : input;

diff --git a/src/shared/installationVerification.ts b/src/shared/installationVerification.ts
@@ -19,7 +19,7 @@ import { ux } from '@oclif/core';
 import { prompts } from '@salesforce/sf-plugins-core';
 import { maxSatisfying } from 'semver';
 import { NpmModule, NpmMeta } from './npmCommand.js';
-import { NpmName } from './NpmName.js';
+import { NpmName, npmNameToString } from './NpmName.js';
 import { setErrorName } from './errors.js';
 
 const CRYPTO_LEVEL = 'RSA-SHA256';
@@ -265,7 +265,7 @@ export class InstallationVerification implements Verifier {
     return isAllowListed({
       logger: await this.getLogger(),
       configPath: this.getConfigPath() ?? '',
-      name: this.pluginNpmName?.toString(),
+      name: this.pluginNpmName ? npmNameToString(this.pluginNpmName) : undefined,
     });
   }
 

diff --git a/test/shared/npmName.test.ts b/test/shared/npmName.test.ts
@@ -6,91 +6,110 @@
  */
 
 import { expect } from 'chai';
-import { parseNpmName } from '../../src/shared/NpmName.js';
+import { npmNameToString, parseNpmName } from '../../src/shared/NpmName.js';
 
-describe('parse', () => {
-  describe('scope without @', () => {
-    it('salesforce/foo', () => {
-      const f = parseNpmName('salesforce/foo');
-      expect(f.scope).to.equal('salesforce');
-      expect(f.name).to.equal('foo');
-      expect(f.tag).to.equal('latest');
+describe('npmName', () => {
+  describe('parse', () => {
+    describe('scope without @', () => {
+      it('salesforce/foo', () => {
+        const input = 'salesforce/foo';
+        const f = parseNpmName(input);
+        expect(f.scope).to.equal('salesforce');
+        expect(f.name).to.equal('foo');
+        expect(f.tag).to.equal('latest');
+        expect(npmNameToString(f)).to.equal('@salesforce/foo');
+      });
+      it('salesforce/foo@latest', () => {
+        const input = 'salesforce/foo@latest';
+        const f = parseNpmName(input);
+        expect(f.scope).to.equal('salesforce');
+        expect(f.name).to.equal('foo');
+        expect(f.tag).to.equal('latest');
+        expect(npmNameToString(f)).to.equal('@salesforce/foo');
+      });
+      it('salesforce/foo@rc', () => {
+        const input = 'salesforce/foo@rc';
+        const f = parseNpmName(input);
+        expect(f.scope).to.equal('salesforce');
+        expect(f.name).to.equal('foo');
+        expect(f.tag).to.equal('rc');
+        expect(npmNameToString(f)).to.equal('@salesforce/foo');
+      });
     });
-    it('salesforce/foo@latest', () => {
-      const f = parseNpmName('salesforce/foo@latest');
-      expect(f.scope).to.equal('salesforce');
-      expect(f.name).to.equal('foo');
-      expect(f.tag).to.equal('latest');
-    });
-    // it doesn't work on main
-    it('salesforce/foo@rc', () => {
-      const f = parseNpmName('salesforce/foo@rc');
-      expect(f.scope).to.equal('salesforce');
-      expect(f.name).to.equal('foo');
-      expect(f.tag).to.equal('rc');
-    });
-  });
 
-  describe('scope with @', () => {
-    it('@salesforce/foo', () => {
-      const f = parseNpmName('@salesforce/foo');
-      expect(f.scope).to.equal('salesforce');
-      expect(f.name).to.equal('foo');
-      expect(f.tag).to.equal('latest');
-    });
-    it('@salesforce/foo@latest', () => {
-      const f = parseNpmName('@salesforce/foo@latest');
-      expect(f.scope).to.equal('salesforce');
-      expect(f.name).to.equal('foo');
-      expect(f.tag).to.equal('latest');
+    describe('scope with @', () => {
+      it('@salesforce/foo', () => {
+        const input = '@salesforce/foo';
+        const f = parseNpmName(input);
+        expect(f.scope).to.equal('salesforce');
+        expect(f.name).to.equal('foo');
+        expect(f.tag).to.equal('latest');
+        expect(npmNameToString(f)).to.equal('@salesforce/foo');
+      });
+      it('@salesforce/foo@latest', () => {
+        const input = '@salesforce/foo@latest';
+        const f = parseNpmName(input);
+        expect(f.scope).to.equal('salesforce');
+        expect(f.name).to.equal('foo');
+        expect(f.tag).to.equal('latest');
+        expect(npmNameToString(f)).to.equal('@salesforce/foo');
+      });
+      it('@salesforce/foo@rc', () => {
+        const input = '@salesforce/foo@rc';
+        const f = parseNpmName(input);
+        expect(f.scope).to.equal('salesforce');
+        expect(f.name).to.equal('foo');
+        expect(f.tag).to.equal('rc');
+        expect(npmNameToString(f)).to.equal('@salesforce/foo');
+      });
     });
-    it('@salesforce/foo@rc', () => {
-      const f = parseNpmName('@salesforce/foo@rc');
-      expect(f.scope).to.equal('salesforce');
-      expect(f.name).to.equal('foo');
-      expect(f.tag).to.equal('rc');
-    });
-  });
 
-  describe('no scope', () => {
-    it('foo', () => {
-      const f = parseNpmName('foo');
-      expect(f.scope).to.be.undefined;
-      expect(f.name).to.equal('foo');
-      expect(f.tag).to.equal('latest');
-    });
-    it('foo@latest', () => {
-      const f = parseNpmName('foo@latest');
-      expect(f.scope).to.be.undefined;
-      expect(f.name).to.equal('foo');
-      expect(f.tag).to.equal('latest');
+    describe('no scope', () => {
+      it('foo', () => {
+        const input = 'foo';
+        const f = parseNpmName(input);
+        expect(f.scope).to.be.undefined;
+        expect(f.name).to.equal('foo');
+        expect(f.tag).to.equal('latest');
+        expect(npmNameToString(f)).to.equal('foo');
+      });
+      it('foo@latest', () => {
+        const input = 'foo@latest';
+        const f = parseNpmName(input);
+        expect(f.scope).to.be.undefined;
+        expect(f.name).to.equal('foo');
+        expect(f.tag).to.equal('latest');
+        expect(npmNameToString(f)).to.equal('foo');
+      });
+      it('foo@rc', () => {
+        const input = 'foo@rc';
+        const f = parseNpmName(input);
+        expect(f.scope).to.be.undefined;
+        expect(f.name).to.equal('foo');
+        expect(f.tag).to.equal('rc');
+        expect(npmNameToString(f)).to.equal('foo');
+      });
     });
-    it('foo@rc', () => {
-      const f = parseNpmName('foo@rc');
-      expect(f.scope).to.be.undefined;
-      expect(f.name).to.equal('foo');
-      expect(f.tag).to.equal('rc');
-    });
-  });
 
-  describe('invalid', () => {
-    it('empty', () => {
-      expect(() => parseNpmName('')).to.throw();
-    });
-    it('single leading @', () => {
-      expect(() => parseNpmName('@')).to.throw();
-    });
-    it('extra slashes', () => {
-      expect(() => parseNpmName('this/is/real/bad')).to.throw();
-    });
-    it('space', () => {
-      expect(() => parseNpmName('this fails')).to.throw();
-    });
-    it('extra @', () => {
-      expect(() => parseNpmName('@@')).to.throw();
-    });
-    it('extra @s', () => {
-      expect(() => parseNpmName('@foo/bar@z@f')).to.throw();
+    describe('invalid', () => {
+      it('empty', () => {
+        expect(() => parseNpmName('')).to.throw();
+      });
+      it('single leading @', () => {
+        expect(() => parseNpmName('@')).to.throw();
+      });
+      it('extra slashes', () => {
+        expect(() => parseNpmName('this/is/real/bad')).to.throw();
+      });
+      it('space', () => {
+        expect(() => parseNpmName('this fails')).to.throw();
+      });
+      it('extra @', () => {
+        expect(() => parseNpmName('@@')).to.throw();
+      });
+      it('extra @s', () => {
+        expect(() => parseNpmName('@foo/bar@z@f')).to.throw();
+      });
     });
   });
 });