Merge pull request #264 from trdoyle81/GITOPS-10535-console-plugin-core-data-test

GITOPS-10535: add unit tests for core string and URL utilities

Author: keithchong

__mocks__/patternfly-react-core.tsx

@@ -12,15 +12,27 @@ export const Popover: React.FC<any> = ({ headerContent, bodyContent, children })
   </div>
 );
 
-export const MenuToggle = React.forwardRef<any, any>(({ children, variant, ...rest }, ref) => (
-  <button ref={ref} data-variant={variant} {...rest}>{children}</button>
-));
+export const MenuToggle = React.forwardRef<any, any>(
+  ({ children, variant, isExpanded, ...rest }, ref) => (
+    <button ref={ref} data-variant={variant} data-expanded={isExpanded} {...rest}>
+      {children}
+    </button>
+  ),
+);
 MenuToggle.displayName = 'MenuToggle';
 
 export type MenuToggleElement = HTMLButtonElement;
 export type MenuToggleProps = any;
 
-export const Dropdown: React.FC<any> = ({ children, isOpen, toggle, ...props }) => (
+export const Dropdown: React.FC<any> = ({
+  children,
+  isOpen,
+  toggle,
+  //patternfly-only props — keep off the dom to avoid react warnings in tests
+  popperProps: _popperProps,
+  onOpenChange: _onOpenChange,
+  ...props
+}) => (
   <div data-testid="dropdown" data-open={isOpen} {...props}>
     {typeof toggle === 'function' ? toggle(null) : toggle}
     {isOpen && children}
@@ -30,7 +42,14 @@ export const Dropdown: React.FC<any> = ({ children, isOpen, toggle, ...props })
 export const DropdownList: React.FC<any> = ({ children }) => <ul>{children}</ul>;
 
 export const DropdownItem: React.FC<any> = ({ children, description, isDisabled, ...props }) => (
-  <li data-disabled={isDisabled} {...props}>{children}{description && <small>{description}</small>}</li>
+  <li data-disabled={isDisabled} {...props}>
+    {children}
+    {description && <small>{description}</small>}
+  </li>
+);
+
+export const Divider: React.FC<any> = ({ component: Component = 'hr', ...props }) => (
+  <Component data-testid="divider" {...props} />
 );
 
 export const Tooltip: React.FC<any> = ({ content, children }) => (

src/gitops/utils/components/ActionDropDown/ActionDropDown.test.tsx

@@ -15,7 +15,7 @@ describe('ActionsDropdown', () => {
         />,
       ),
     ).toMatchInlineSnapshot(
-      `"<div data-testid="dropdown" data-open="false" popperProps="[object Object]"><button>Actions</button></div>"`,
+      `"<div data-testid="dropdown" data-open="false"><button data-expanded="false">Actions</button></div>"`,
     );
   });
 
@@ -28,13 +28,13 @@ describe('ActionsDropdown', () => {
         />,
       ),
     ).toMatchInlineSnapshot(
-      `"<div data-testid="dropdown" data-open="false" popperProps="[object Object]"><button data-variant="plain"><svg data-icon="EllipsisVIcon"></svg></button></div>"`,
+      `"<div data-testid="dropdown" data-open="false"><button data-variant="plain" data-expanded="false"><svg data-icon="EllipsisVIcon"></svg></button></div>"`,
     );
   });
 
   it('renders with no actions', () => {
     expect(renderToStaticMarkup(<ActionsDropdown actions={[]} />)).toMatchInlineSnapshot(
-      `"<div data-testid="dropdown" data-open="false" popperProps="[object Object]"><button>Actions</button></div>"`,
+      `"<div data-testid="dropdown" data-open="false"><button data-expanded="false">Actions</button></div>"`,
     );
   });
 });

src/gitops/utils/stringHelpers.test.ts

@@ -1,23 +1,74 @@
 import { detectGitType, gitUrlRegex } from './stringHelpers';
 
 describe('gitUrlRegex', () => {
-  it('matches valid git URLs', () => {
+  it('matches valid https and git URLs', () => {
     expect(gitUrlRegex.test('https://github.com/foo/bar')).toMatchInlineSnapshot(`true`);
     expect(gitUrlRegex.test('https://github.com/foo/bar.git')).toMatchInlineSnapshot(`true`);
+    expect(gitUrlRegex.test('https://www.github.com/foo/bar')).toMatchInlineSnapshot(`true`);
+    expect(gitUrlRegex.test('http://github.com/foo/bar')).toMatchInlineSnapshot(`true`);
+    expect(gitUrlRegex.test('git://github.com/foo/bar.git')).toMatchInlineSnapshot(`true`);
+    expect(gitUrlRegex.test('https://gitlab.com/group/sub/project.git')).toMatchInlineSnapshot(
+      `true`,
+    );
+  });
+
+  it('matches valid ssh and scp-style URLs', () => {
     expect(gitUrlRegex.test('git@github.com:foo/bar.git')).toMatchInlineSnapshot(`true`);
     expect(gitUrlRegex.test('ssh://git@github.com/foo/bar')).toMatchInlineSnapshot(`true`);
-    expect(gitUrlRegex.test('not a url')).toMatchInlineSnapshot(`false`);
+    expect(gitUrlRegex.test('ssh://git@gitlab.com/foo/bar.git')).toMatchInlineSnapshot(`true`);
+    expect(gitUrlRegex.test('git@bitbucket.org:team/repo.git')).toMatchInlineSnapshot(`true`);
+  });
+
+  it('rejects empty, blank, and malformed URLs', () => {
     expect(gitUrlRegex.test('')).toMatchInlineSnapshot(`false`);
+    expect(gitUrlRegex.test(' ')).toMatchInlineSnapshot(`false`);
+    expect(gitUrlRegex.test('not a url')).toMatchInlineSnapshot(`false`);
+    expect(gitUrlRegex.test('not-a-url')).toMatchInlineSnapshot(`false`);
+    expect(gitUrlRegex.test('https://')).toMatchInlineSnapshot(`false`);
+    expect(gitUrlRegex.test('ftp://github.com/foo/bar')).toMatchInlineSnapshot(`false`);
+  });
+
+  it('matches non-standard but syntactically valid git hosts', () => {
+    expect(gitUrlRegex.test('https://gitea.example.com/foo/bar.git')).toMatchInlineSnapshot(`true`);
+    expect(gitUrlRegex.test('https://github.enterprise.example.com/foo/bar')).toMatchInlineSnapshot(
+      `true`,
+    );
+    //host alone is still considered a url shape by this regex
+    expect(gitUrlRegex.test('https://github.com')).toMatchInlineSnapshot(`true`);
   });
 });
 
 describe('detectGitType', () => {
-  it('detects git providers', () => {
+  it('detects known providers from https URLs', () => {
     expect(detectGitType('https://github.com/foo/bar')).toMatchInlineSnapshot(`"github"`);
+    expect(detectGitType('https://www.github.com/foo/bar')).toMatchInlineSnapshot(`"github"`);
     expect(detectGitType('https://gitlab.com/foo/bar')).toMatchInlineSnapshot(`"gitlab"`);
+    expect(detectGitType('https://www.gitlab.com/foo/bar')).toMatchInlineSnapshot(`"gitlab"`);
     expect(detectGitType('https://bitbucket.org/foo/bar')).toMatchInlineSnapshot(`"bitbucket"`);
-    expect(detectGitType('https://example.com/foo/bar')).toMatchInlineSnapshot(`"other"`);
-    expect(detectGitType('not a url')).toMatchInlineSnapshot(`""`);
+    expect(detectGitType('https://www.bitbucket.org/foo/bar')).toMatchInlineSnapshot(`"bitbucket"`);
+  });
+
+  it('detects known providers from scp-style SSH URLs', () => {
     expect(detectGitType('git@github.com:foo/bar.git')).toMatchInlineSnapshot(`"github"`);
+    expect(detectGitType('git@gitlab.com:foo/bar.git')).toMatchInlineSnapshot(`"gitlab"`);
+    expect(detectGitType('git@bitbucket.org:team/repo.git')).toMatchInlineSnapshot(`"bitbucket"`);
+  });
+
+  it('returns empty string for invalid or empty input', () => {
+    expect(detectGitType('')).toMatchInlineSnapshot(`""`);
+    expect(detectGitType('not a url')).toMatchInlineSnapshot(`""`);
+    expect(detectGitType('https://')).toMatchInlineSnapshot(`""`);
+  });
+
+  it('returns other for unrecognized or non-standard formats', () => {
+    //valid git url shape, but not a known public provider
+    expect(detectGitType('https://example.com/foo/bar')).toMatchInlineSnapshot(`"other"`);
+    expect(detectGitType('https://gitea.example.com/foo/bar.git')).toMatchInlineSnapshot(`"other"`);
+    expect(detectGitType('https://github.enterprise.example.com/foo/bar')).toMatchInlineSnapshot(
+      `"other"`,
+    );
+    //http (not https) and ssh:// do not match hasDomain checks, so provider is unsure
+    expect(detectGitType('http://github.com/foo/bar')).toMatchInlineSnapshot(`"other"`);
+    expect(detectGitType('ssh://git@github.com/foo/bar')).toMatchInlineSnapshot(`"other"`);
   });
 });

src/gitops/utils/urls.test.ts

@@ -1,18 +1,44 @@
 import { isSHA, repoUrl, revisionUrl } from './urls';
 
 describe('isSHA', () => {
-  it('identifies SHA hashes', () => {
+  it('identifies short and full hex SHAs', () => {
+    expect(isSHA('abcde')).toMatchInlineSnapshot(`true`);
     expect(isSHA('abc123def')).toMatchInlineSnapshot(`true`);
     expect(isSHA('abc123def456789012345678901234567890abcd')).toMatchInlineSnapshot(`true`);
+    expect(isSHA('1234567890123456789012345678901234567890')).toMatchInlineSnapshot(`true`);
+  });
+
+  it('identifies sha256-prefixed hashes', () => {
+    expect(isSHA('sha256:abc123de')).toMatchInlineSnapshot(`true`);
     expect(isSHA('sha256:abc123def456789012345678901234567890abcd')).toMatchInlineSnapshot(`true`);
+  });
+
+  it('rejects empty strings, branches, tags, and missing SHAs', () => {
+    expect(isSHA('')).toMatchInlineSnapshot(`false`);
+    expect(isSHA('HEAD')).toMatchInlineSnapshot(`false`);
     expect(isSHA('main')).toMatchInlineSnapshot(`false`);
+    expect(isSHA('develop')).toMatchInlineSnapshot(`false`);
     expect(isSHA('v1.0.0')).toMatchInlineSnapshot(`false`);
+    expect(isSHA('v1.2.3')).toMatchInlineSnapshot(`false`);
+  });
+
+  it('rejects values outside the supported hex length and charset', () => {
+    //too short for plain sha (needs 5–40 hex chars)
     expect(isSHA('abc')).toMatchInlineSnapshot(`false`);
+    expect(isSHA('abcd')).toMatchInlineSnapshot(`false`);
+    //too long for plain sha (>40)
+    expect(isSHA('12345678901234567890123456789012345678901')).toMatchInlineSnapshot(`false`);
+    //uppercase is not matched by the lowercase-only regex
+    expect(isSHA('ABCDEF')).toMatchInlineSnapshot(`false`);
+    expect(isSHA('abc123DEF')).toMatchInlineSnapshot(`false`);
+    //sha256 prefix with empty or too-short hash
+    expect(isSHA('sha256:')).toMatchInlineSnapshot(`false`);
+    expect(isSHA('sha256:abc')).toMatchInlineSnapshot(`false`);
   });
 });
 
 describe('repoUrl', () => {
-  it('extracts repo URLs from various formats', () => {
+  it('extracts canonical https repo paths from common formats', () => {
     expect(repoUrl('https://github.com/argoproj/argo-cd.git')).toMatchInlineSnapshot(
       `"https://github.com/argoproj/argo-cd"`,
     );
@@ -22,24 +48,30 @@ describe('repoUrl', () => {
     expect(repoUrl('git@github.com:argoproj/argo-cd.git')).toMatchInlineSnapshot(
       `"https://github.com/argoproj/argo-cd"`,
     );
+    expect(repoUrl('ssh://git@github.com/foo/bar.git')).toMatchInlineSnapshot(
+      `"https://github.com/foo/bar"`,
+    );
     expect(repoUrl('https://gitlab.com/group/project.git')).toMatchInlineSnapshot(
       `"https://gitlab.com/group/project"`,
     );
     expect(repoUrl('https://bitbucket.org/team/repo.git')).toMatchInlineSnapshot(
       `"https://bitbucket.org/team/repo"`,
     );
+  });
+
+  it('returns null for empty, malformed, or unsupported providers', () => {
+    expect(repoUrl('')).toMatchInlineSnapshot(`null`);
+    expect(repoUrl('not a url')).toMatchInlineSnapshot(`null`);
     expect(repoUrl('https://internal.example.com/repo.git')).toMatchInlineSnapshot(`null`);
+    expect(repoUrl('https://gitea.io/foo/bar')).toMatchInlineSnapshot(`null`);
   });
 });
 
 describe('revisionUrl', () => {
-  it('builds revision URLs for different providers', () => {
+  it('builds commit URLs for SHA revisions', () => {
     expect(revisionUrl('https://github.com/foo/bar.git', 'abc123def', false)).toMatchInlineSnapshot(
       `"https://github.com/foo/bar/commit/abc123def"`,
     );
-    expect(revisionUrl('https://github.com/foo/bar.git', 'main', false)).toMatchInlineSnapshot(
-      `"https://github.com/foo/bar/tree/main"`,
-    );
     expect(revisionUrl('https://gitlab.com/foo/bar.git', 'abc123def', false)).toMatchInlineSnapshot(
       `"https://gitlab.com/foo/bar/-/commit/abc123def"`,
     );
@@ -49,8 +81,37 @@ describe('revisionUrl', () => {
     expect(
       revisionUrl('https://bitbucket.org/foo/bar.git', 'abc123def', true),
     ).toMatchInlineSnapshot(`"https://bitbucket.org/foo/bar/src/abc123def"`);
+  });
+
+  it('builds tree/src URLs for branch names', () => {
+    expect(revisionUrl('https://github.com/foo/bar.git', 'main', false)).toMatchInlineSnapshot(
+      `"https://github.com/foo/bar/tree/main"`,
+    );
+    expect(revisionUrl('https://gitlab.com/foo/bar.git', 'main', false)).toMatchInlineSnapshot(
+      `"https://gitlab.com/foo/bar/-/tree/main"`,
+    );
+    expect(revisionUrl('https://bitbucket.org/foo/bar.git', 'main', false)).toMatchInlineSnapshot(
+      `"https://bitbucket.org/foo/bar/src/main"`,
+    );
+    expect(revisionUrl('https://bitbucket.org/foo/bar.git', 'main', true)).toMatchInlineSnapshot(
+      `"https://bitbucket.org/foo/bar/src/main"`,
+    );
+  });
+
+  it('defaults missing revision to HEAD', () => {
     expect(revisionUrl('https://github.com/foo/bar.git', '', false)).toMatchInlineSnapshot(
       `"https://github.com/foo/bar/tree/HEAD"`,
     );
+    expect(revisionUrl('https://github.com/foo/bar.git', null as any, false)).toMatchInlineSnapshot(
+      `"https://github.com/foo/bar/tree/HEAD"`,
+    );
+  });
+
+  it('returns null for empty, malformed, or unsupported repo URLs', () => {
+    expect(revisionUrl('', 'abc123', false)).toMatchInlineSnapshot(`null`);
+    expect(revisionUrl('not a url', 'abc123', false)).toMatchInlineSnapshot(`null`);
+    expect(revisionUrl('https://gitea.io/foo/bar.git', 'abc123', false)).toMatchInlineSnapshot(
+      `null`,
+    );
   });
 });

src/gitops/utils/urls.ts

@@ -3,7 +3,8 @@
  * https://github.com/argoproj/argo-cd/blob/4bd8b07c514e26c6b7837f30d52afd1a3cdedcfd/ui/src/app/shared/components/urls.ts
  */
 
-import * as GitUrlParse from 'git-url-parse';
+//cjs package — default import needs esmoduleinterop or jest can't call it
+import GitUrlParse from 'git-url-parse';
 import { GitUrl } from 'git-url-parse';
 
 export const isSHA = (revision: string) => {

tsconfig.json

@@ -8,8 +8,11 @@
     "allowJs": true,
     "strict": false,
     "allowSyntheticDefaultImports": true,
+    //so cjs default imports (git-url-parse) work in jest
+    "esModuleInterop": true,
     "noUnusedLocals": true,
     "lib": ["dom", "es2017"],
+    "types": ["jest"],
     "paths": {
       "@gitops/*": ["src/gitops/*"],
       "@gitops-models/*": ["src/gitops/models/*"],