Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(shared): Improve test coverage #1925

Merged
merged 3 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/grumpy-swans-taste.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@clerk/shared": patch
---

Improve internal test coverage and fix small bug inside `callWithRetry`
59 changes: 58 additions & 1 deletion packages/shared/src/__tests__/browser.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,66 @@
import { inBrowser, isValidBrowserOnline, userAgentIsRobot } from '../browser';
import { inBrowser, isValidBrowser, isValidBrowserOnline, userAgentIsRobot } from '../browser';

describe('inBrowser()', () => {
afterEach(() => {
jest.restoreAllMocks();
});

it('returns true if window is defined', () => {
expect(inBrowser()).toBe(true);
});
it('returns false if window is undefined', () => {
const windowSpy = jest.spyOn(global, 'window', 'get');
// @ts-ignore - Test
windowSpy.mockReturnValue(undefined);
expect(inBrowser()).toBe(false);
});
});

describe('isValidBrowser', () => {
let userAgentGetter: any;
let webdriverGetter: any;

beforeEach(() => {
userAgentGetter = jest.spyOn(window.navigator, 'userAgent', 'get');
webdriverGetter = jest.spyOn(window.navigator, 'webdriver', 'get');
});

afterEach(() => {
jest.restoreAllMocks();
});

it('returns false if not in browser', () => {
const windowSpy = jest.spyOn(global, 'window', 'get');
// @ts-ignore - Test
windowSpy.mockReturnValue(undefined);

expect(isValidBrowser()).toBe(false);
});

it('returns true if in browser, navigator is not a bot, and webdriver is not enabled', () => {
userAgentGetter.mockReturnValue(
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:109.0) Gecko/20100101 Firefox/109.0',
);
webdriverGetter.mockReturnValue(false);

expect(isValidBrowser()).toBe(true);
});

it('returns false if navigator is a bot', () => {
userAgentGetter.mockReturnValue('msnbot-NewsBlogs/2.0b (+http://search.msn.com/msnbot.htm)');
webdriverGetter.mockReturnValue(false);

expect(isValidBrowser()).toBe(false);
});

it('returns false if webdriver is enabled', () => {
userAgentGetter.mockReturnValue(
'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:109.0) Gecko/20100101 Firefox/109.0',
);
webdriverGetter.mockReturnValue(true);

expect(isValidBrowser()).toBe(false);
});
});

describe('detectUserAgentRobot', () => {
Expand Down
23 changes: 23 additions & 0 deletions packages/shared/src/__tests__/callWithRetry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { callWithRetry } from '../callWithRetry';

describe('callWithRetry', () => {
test('should return the result of the function if it succeeds', async () => {
const fn = jest.fn().mockResolvedValue('result');
const result = await callWithRetry(fn);
expect(result).toBe('result');
expect(fn).toHaveBeenCalledTimes(1);
});

test('should retry the function if it fails', async () => {
const fn = jest.fn().mockRejectedValueOnce(new Error('error')).mockResolvedValueOnce('result');
const result = await callWithRetry(fn, 1, 2);
expect(result).toBe('result');
expect(fn).toHaveBeenCalledTimes(2);
});

test('should throw an error if the function fails too many times', async () => {
const fn = jest.fn().mockRejectedValue(new Error('error'));
await expect(callWithRetry(fn, 1, 2)).rejects.toThrow('error');
expect(fn).toHaveBeenCalledTimes(2);
});
});
30 changes: 30 additions & 0 deletions packages/shared/src/__tests__/keys.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import {
buildPublishableKey,
createDevOrStagingUrlCache,
isDevelopmentFromApiKey,
isLegacyFrontendApiKey,
isProductionFromApiKey,
isPublishableKey,
parsePublishableKey,
} from '../keys';
Expand Down Expand Up @@ -90,3 +92,31 @@ describe('isDevOrStagingUrl(url)', () => {
expect(isDevOrStagingUrl(a)).toBe(expected);
});
});

describe('isDevelopmentFromApiKey(key)', () => {
const cases: Array<[string, boolean]> = [
['sk_live_Y2xlcmsuY2xlcmsuZGV2JA==', false],
['sk_test_Y2xlcmsuY2xlcmsuZGV2JA==', true],
['live_Y2xlcmsuY2xlcmsuZGV2JA==', false],
['test_Y2xlcmsuY2xlcmsuZGV2JA==', true],
];

test.each(cases)('given %p as a publishable key string, returns %p', (publishableKeyStr, expected) => {
const result = isDevelopmentFromApiKey(publishableKeyStr);
expect(result).toEqual(expected);
});
});

describe('isProductionFromApiKey(key)', () => {
const cases: Array<[string, boolean]> = [
['sk_live_Y2xlcmsuY2xlcmsuZGV2JA==', true],
['sk_test_Y2xlcmsuY2xlcmsuZGV2JA==', false],
['live_Y2xlcmsuY2xlcmsuZGV2JA==', true],
['test_Y2xlcmsuY2xlcmsuZGV2JA==', false],
];

test.each(cases)('given %p as a publishable key string, returns %p', (publishableKeyStr, expected) => {
const result = isProductionFromApiKey(publishableKeyStr);
expect(result).toEqual(expected);
});
});
58 changes: 57 additions & 1 deletion packages/shared/src/__tests__/url.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { addClerkPrefix, parseSearchParams, stripScheme } from '../url';
import { addClerkPrefix, getClerkJsMajorVersionOrTag, getScriptUrl, parseSearchParams, stripScheme } from '../url';

describe('parseSearchParams(queryString)', () => {
it('parses query string and returns a URLSearchParams object', () => {
Expand Down Expand Up @@ -56,3 +56,59 @@ describe('addClerkPrefix(str)', () => {
expect(addClerkPrefix(urlInput)).toBe(urlOutput);
});
});

describe('getClerkJsMajorVersionOrTag', () => {
const stagingFrontendApi = 'foobar.lclstage.dev';

it('returns staging if pkgVersion is not provided and frontendApi is staging', () => {
expect(getClerkJsMajorVersionOrTag(stagingFrontendApi)).toBe('staging');
});

it('returns latest if pkgVersion is not provided and frontendApi is not staging', () => {
expect(getClerkJsMajorVersionOrTag('foobar.dev')).toBe('latest');
});

it('returns next if pkgVersion contains next', () => {
expect(getClerkJsMajorVersionOrTag('foobar.dev', '1.2.3-next.4')).toBe('next');
});

it('returns the major version if pkgVersion is provided', () => {
expect(getClerkJsMajorVersionOrTag('foobar.dev', '1.2.3')).toBe('1');
});

it('returns latest if pkgVersion is empty string', () => {
expect(getClerkJsMajorVersionOrTag('foobar.dev', '')).toBe('latest');
});
});

describe('getScriptUrl', () => {
const frontendApi = 'https://foobar.dev';

it('returns URL using the clerkJSVersion if provided', () => {
expect(getScriptUrl(frontendApi, { clerkJSVersion: '1.2.3' })).toBe(
'https://foobar.dev/npm/@clerk/[email protected]/dist/clerk.browser.js',
);
});

it('returns URL using the latest version if clerkJSVersion & pkgVersion is not provided + frontendApi is not staging', () => {
expect(getScriptUrl(frontendApi, {})).toBe('https://foobar.dev/npm/@clerk/clerk-js@latest/dist/clerk.browser.js');
});

it('returns URL using the major version if only pkgVersion is provided', () => {
expect(getScriptUrl(frontendApi, { pkgVersion: '1.2.3' })).toBe(
'https://foobar.dev/npm/@clerk/clerk-js@1/dist/clerk.browser.js',
);
});

it('returns URL using the major version if only pkgVersion contains next', () => {
expect(getScriptUrl(frontendApi, { pkgVersion: '1.2.3-next.4' })).toBe(
'https://foobar.dev/npm/@clerk/clerk-js@next/dist/clerk.browser.js',
);
});

it('returns URL using the staging tag if frontendApi is staging', () => {
expect(getScriptUrl('https://foobar.lclstage.dev', {})).toBe(
'https://foobar.lclstage.dev/npm/@clerk/clerk-js@staging/dist/clerk.browser.js',
);
});
});
2 changes: 1 addition & 1 deletion packages/shared/src/callWithRetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,6 @@ export async function callWithRetry<T>(
}
await wait(2 ** attempt * 100);

return callWithRetry(fn, attempt + 1);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests actually uncovered this small bug here

return callWithRetry(fn, attempt + 1, maxAttempts);
}
}
22 changes: 22 additions & 0 deletions packages/shared/src/utils/__tests__/createDeferredPromise.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { createDeferredPromise } from '../createDeferredPromise';

describe('createDeferredPromise', () => {
test('resolves with correct value', async () => {
const { promise, resolve } = createDeferredPromise();
const expectedValue = 'hello world';
resolve(expectedValue);
const result = await promise;
expect(result).toBe(expectedValue);
});

test('rejects with correct error', async () => {
const { promise, reject } = createDeferredPromise();
const expectedError = new Error('something went wrong');
reject(expectedError);
try {
await promise;
} catch (error) {
expect(error).toBe(expectedError);
}
});
});
20 changes: 20 additions & 0 deletions packages/shared/src/utils/__tests__/instance.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { isStaging } from '../instance';

describe('isStaging', () => {
it.each([
['clerk', false],
['clerk.com', false],
['whatever.com', false],
['clerk.abcef', false],
['clerk.abcef.12345', false],
['clerk.abcef.12345.lcl', false],
['clerk.abcef.12345.lcl.dev', false],
['clerk.abcef.12345.stg.dev', false],
['clerk.abcef.12345.lclstage.dev', true],
['clerk.abcef.12345.stgstage.dev', true],
['clerk.abcef.12345.clerkstage.dev', true],
['clerk.abcef.12345.accountsstage.dev', true],
])('validates the frontendApi format', (str, expected) => {
expect(isStaging(str)).toBe(expected);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { runWithExponentialBackOff } from '../runWithExponentialBackOff';

describe('runWithExponentialBackOff', () => {
test('resolves with the result of the callback', async () => {
const result = await runWithExponentialBackOff(() => Promise.resolve('success'));
expect(result).toBe('success');
});

test('retries the callback until it succeeds', async () => {
let attempts = 0;
const result = await runWithExponentialBackOff(() => {
attempts++;
if (attempts < 3) {
throw new Error('failed');
}
return Promise.resolve('success');
});
expect(result).toBe('success');
expect(attempts).toBe(3);
});
});
3 changes: 3 additions & 0 deletions packages/shared/src/utils/instance.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
/**
* Check if the frontendApi ends with a staging domain
*/
export function isStaging(frontendApi: string): boolean {
return (
frontendApi.endsWith('.lclstage.dev') ||
Expand Down
Loading