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.

Sign up for GitHub

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

refactor: 테스트 리팩토링 #99

Merged
merged 4 commits into from
Jul 1, 2024
Merged

refactor: 테스트 리팩토링 #99

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d294d42
refactor: script element mock 유틸 활용
moresoo Jul 1, 2024
6ce9a1e
refactor: SDK load 테스트에서 NamespaceNotAvailableError 에러 관련 검증을 하지 않는다
moresoo Jul 1, 2024
c25749c
chore: end of line
moresoo Jul 1, 2024
3b1bae4
chore: end of line
moresoo Jul 1, 2024
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
10 changes: 2 additions & 8 deletions packages/brandpay-sdk/src/loadBrandPay.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,15 @@ describe('loadBrandPay', () => {

test('src를 지정하면 주어진 URL로 script를 로드한다', async () => {
const { loadBrandPay } = await import('./loadBrandPay');
const { NamespaceNotAvailableError } = await import('@tosspayments/sdk-loader');

const testSource = `https://js.tosspayments.com/v1/payment-widget`;

try {
await loadBrandPay('test_key', 'customer_key', {}, {
src: testSource,
});
} catch (error) {
if (error instanceof NamespaceNotAvailableError) {
// NOTE: SDK에서 namespace에 인스턴스를 꽂아주는 동작이 테스트 환경에서는 일어나지 않아 발생하는 에러를 무시합니다
return;
}

throw error;
} catch {
// NOTE: SDK에서 namespace에 인스턴스를 꽂아주는 동작이 테스트 환경에서는 일어나지 않아 발생하는 에러를 무시합니다
}

const script = document.querySelector(`script[src="${testSource}"]`);
Expand Down
8 changes: 1 addition & 7 deletions packages/payment-sdk/src/loadTossPayments.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ describe('loadTossPayments', () => {

test('src를 지정하면 주어진 URL로 script를 로드한다', async () => {
const { loadTossPayments } = await import('./loadTossPayments');
const { NamespaceNotAvailableError } = await import('@tosspayments/sdk-loader');

const testSource = `https://js.tosspayments.com/v1/brandpay`;

Expand All @@ -40,12 +39,7 @@ describe('loadTossPayments', () => {
src: testSource,
});
} catch (error) {
if (error instanceof NamespaceNotAvailableError) {
// NOTE: SDK에서 namespace에 인스턴스를 꽂아주는 동작이 테스트 환경에서는 일어나지 않아 발생하는 에러를 무시합니다
return;
}

throw error;
// NOTE: SDK에서 namespace에 인스턴스를 꽂아주는 동작이 테스트 환경에서는 일어나지 않아 발생하는 에러를 무시합니다.
}

const script = document.querySelector(`script[src="${testSource}"]`);
Expand Down
10 changes: 2 additions & 8 deletions packages/payment-widget-sdk/src/loadPaymentWidget.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,15 @@ describe('loadPaymentWidget', () => {

test('src를 지정하면 주어진 URL로 script를 로드한다', async () => {
const { loadPaymentWidget } = await import('./loadPaymentWidget');
const { NamespaceNotAvailableError } = await import('@tosspayments/sdk-loader');

const testSource = `https://js.tosspayments.com/v1/brandpay`;

try {
await loadPaymentWidget('test_key', 'customer_key', {}, {
src: testSource,
});
} catch (error) {
if (error instanceof NamespaceNotAvailableError) {
// NOTE: SDK에서 namespace에 인스턴스를 꽂아주는 동작이 테스트 환경에서는 일어나지 않아 발생하는 에러를 무시합니다
return;
}

throw error;
} catch {
// NOTE: SDK에서 namespace에 인스턴스를 꽂아주는 동작이 테스트 환경에서는 일어나지 않아 발생하는 에러를 무시합니다.
}

const script = document.querySelector(`script[src="${testSource}"]`);
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk-loader/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export { loadScript, NamespaceNotAvailableError, ScriptLoadFailedError } from './loadScript'
export { loadScript } from './loadScript'
96 changes: 59 additions & 37 deletions packages/sdk-loader/src/loadScript.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { beforeEach, describe, expect, test, vi } from 'vitest';
import { afterEach, describe, expect, test, vi } from 'vitest';

declare global {
interface Window {
Expand All @@ -7,57 +7,44 @@ declare global {
}

describe('loadScript', () => {
// NOTE: load, error 이벤트를 임의로 발생시키기 위해 이벤트 리스터를 모킹합니다
let eventListeners1: { [key: string]: EventListener };
let eventListeners2: { [key: string]: EventListener };

let script1: HTMLScriptElement;
let script2: HTMLScriptElement;

beforeEach(() => {
afterEach(() => {
vi.resetModules();

document.head.innerHTML = '';
document.head.appendChild = vi.fn(); // NOTE: 테스트 환경에서 script inject 방지

delete window.TossPayments;

eventListeners1 = {};
eventListeners2 = {};

script1 = document.createElement('script');
script2 = document.createElement('script');

script1.addEventListener = (event: string, listener: EventListener) => {
eventListeners1[event] = listener;
};

script2.addEventListener = (event: string, listener: EventListener) => {
eventListeners2[event] = listener;
};

vi.spyOn(document, 'createElement')
.mockReturnValueOnce(script1)
.mockReturnValueOnce(script2);
document.head.innerHTML = '';
document.head.appendChild = vi.fn(); // NOTE: 테스트 환경에서 script inject 방지
});

describe('기본 동작', () => {
test('script 로드가 완료되면, 주어진 namespace에 생성된 인스턴스와 동일한 인스턴스를 resolve 해야한다', async () => {
// given
const { loadScript } = await import('./loadScript');

const { script, eventListeners } = mockScriptElement();

vi.spyOn(document, 'createElement')
.mockReturnValueOnce(script)

// when
Comment on lines +24 to +28
Copy link
Collaborator

Choose a reason for hiding this comment

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

mockScriptElement 라는 이름이 document.createElement 를 mocking 한다는걸 드러내는게 아닐까요?
그래서 이 함수 내부에서 아래 로직도 함께 처리해줄거라고 기대할 것 같아요~

vi.spyOn(document, 'createElement')
  .mockReturnValueOnce(script)

그리고 eventListeners 는 없애도 되지 않을까요? 두가지 역할을 하고 있는데, 이벤트 핸들러 hijacking 없이도 trigger 를 할 수 있는 방법이 있어서요~

  • load script 가 등록하는 이벤트 핸들러 hijacking
  • 해당 이벤트 핸들러를 테스트에서 직접 trigger
// load 이벤트를 script 태그에 전달
script.dispatchEvent(new Event('load'))

// error 이벤트를 script 태그에 전달
script.dispatchEvent(new Event('error'))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mockScriptElement 라는 이름이 document.createElement 를 mocking 한다는걸 드러내는게 아닐까요?
그래서 이 함수 내부에서 아래 로직도 함께 처리해줄거라고 기대할 것 같아요~

mocking을 유틸 밖으로 뺀 이유는 아래처럼 script 생성이 여러번 될 때의 테스트를 작성하기 위함이었는데, 성현님 말씀대로 mocking 동작이 mockScriptElement 밖에서 처리되는 게 많이 이상하긴 하네요~ 현재는 위 요구사항이 필요한 테스트는 없지만, 생긴다고 해도 mockScriptElement 유틸 내부에서 독립적으로 처리할 수 있는 방법을 찾아 적용해야 할 것 같네요! 감사합니다~ 👍 b357b7e

const { script: script1 } = mockScriptElement();
const { script: script2 } = mockScriptElement();

vi.spyOn(document, 'createElement')
  .mockReturnValueOnce(script1)
  .mockReturnValueOnce(script2);

그리고 eventListeners 는 없애도 되지 않을까요?

요건 dispatchEvent 메소드 생각을 못했습니다ㅋㅋ.. 훨씬 좋네요! f557119

Copy link
Collaborator

Choose a reason for hiding this comment

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

mocking을 유틸 밖으로 뺀 이유는 아래처럼 script 생성이 여러번 될 때의 테스트를 작성하기 위함이었는데, 성현님 말씀대로 mocking 동작이 mockScriptElement 밖에서 처리되는 게 많이 이상하긴 하네요~

아하, 전 요것도 시나리오를 나눠보면, 아래처럼 mocking 시점을 분리해서 해결할 수 있지 않을까? 생각하긴 했어요~ 만약 안된다면 다른 방법을 찾아보긴 해야겠네요 😄

const failed = mockScriptElement();
failed.script.dispatchEvent(...)

// retry 전에 setup
const success = mockScriptElement();
success.script.dispatchEvent(...)

const promise = loadScript('http://example.com/example.js', 'TossPayments');
window.TossPayments = vi.fn(); // SDK는 주어진 namespace에 인스턴스를 생성함
eventListeners1.load(new Event('load')); // script 로드가 완료됨
eventListeners.load(new Event('load')); // script 로드가 완료됨

// then
expect(promise).resolves.toBe(window.TossPayments);
});
test('script 로드를 실패하면, cachedPromise가 null로 설정되고 에러를 throw 해야한다', async () => {
// given
const { loadScript } = await import('./loadScript');
const { script, eventListeners } = mockScriptElement();

vi.spyOn(document, 'createElement')
.mockReturnValueOnce(script)

// when
const promise = loadScript('http://example.com/example.js', 'TossPayments');
eventListeners1.error(new Event('error')); // script 로드가 실패함
eventListeners.error(new Event('error')); // script 로드가 실패함

// then
await expect(promise).rejects.toThrowError('[TossPayments SDK] Failed to load script: [http://example.com/example.js]');
Expand All @@ -68,34 +55,54 @@ describe('loadScript', () => {
});

test('script 로드를 성공했지만 namespace에 인스턴스가 존재하지 않으면, 에러를 throw 해야한다', async () => {
// given
const { loadScript } = await import('./loadScript');
const { script, eventListeners } = mockScriptElement();

vi.spyOn(document, 'createElement')
.mockReturnValueOnce(script)

// when
const promise = loadScript('http://example.com/example.js', 'TossPayments');
eventListeners1.load(new Event('load')); // script 로드가 완료됨
eventListeners.load(new Event('load')); // script 로드가 완료됨

// then
expect(promise).rejects.toThrowError('[TossPayments SDK] TossPayments is not available');
});
test('priority 옵션을 설정하면, script element의 fetchPriority 속성이 설정되어야 한다', async () => {
// given
const { loadScript } = await import('./loadScript');
const { script, eventListeners } = mockScriptElement();

vi.spyOn(document, 'createElement')
.mockReturnValueOnce(script)


// when
const promise = loadScript('http://example.com/example.js', 'TossPayments', { priority: 'high' });
window.TossPayments = vi.fn(); // SDK는 주어진 namespace에 인스턴스를 생성함
eventListeners1.load(new Event('load')); // script 로드가 완료됨
eventListeners.load(new Event('load')); // script 로드가 완료됨

// then
expect((script1 as any).fetchPriority).toBe('high');
expect((script as any).fetchPriority).toBe('high');
await expect(promise).resolves.toBe(window.TossPayments);
});
});

describe('캐시된 script 로더 Promise가 존재하면', () => {
test('해당 Promise를 반환해야 한다', async () => {
// given
const { loadScript } = await import('./loadScript');

const { script, eventListeners } = mockScriptElement();

vi.spyOn(document, 'createElement')
.mockReturnValueOnce(script)

// when
const promise1 = loadScript('http://example.com/script.js', 'TossPayments');
window.TossPayments = vi.fn(); // SDK는 주어진 namespace에 인스턴스를 생성함
eventListeners1.load(new Event('load')); // script 로드가 완료됨
eventListeners.load(new Event('load')); // script 로드가 완료됨

const promise2 = loadScript('http://example.com/script.js', 'TossPayments');

Expand All @@ -106,8 +113,9 @@ describe('loadScript', () => {

describe('캐시된 script 로더 Promise가 존재하지 않으면', () => {
test('SSR 환경이면 null을 resolve 해야한다', async () => {
const { loadScript } = await import('./loadScript');
// given
const { loadScript } = await import('./loadScript');

const originalWindow = window;
const originalDocument = document;
window = undefined as any;
Expand All @@ -123,7 +131,9 @@ describe('loadScript', () => {
document = originalDocument;
});
test('주어진 namespace에 인스턴스가 존재하면, 해당 인스턴스를 resolve 해야한다', async () => {
// given
const { loadScript } = await import('./loadScript');

// when
window.TossPayments = vi.fn();
const promise = loadScript('http://example.com/script.js', 'TossPayments');
Expand All @@ -133,4 +143,16 @@ describe('loadScript', () => {
});
test.todo('기존 src를 가진 script가 존재하면, 기존 script의 이벤트를 모두 제거하고 스크립트도 제거한 후 새로 만들어야 한다',);
});
});
});

function mockScriptElement() {
// NOTE: script의 load, error 이벤트를 임의로 발생시키기 위해 이벤트 리스너를 mocking 합니다
const eventListeners = {} as { [key: string]: EventListener };
const script = document.createElement('script');

script.addEventListener = (event: string, listener: EventListener) => {
eventListeners[event] = listener;
};

return { script, eventListeners };
}
4 changes: 2 additions & 2 deletions packages/sdk-loader/src/loadScript.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,14 @@ function getNamespace<Namespace>(name: string) {
return (window[name as any] as any) as Namespace | undefined;
}

export class NamespaceNotAvailableError extends Error {
class NamespaceNotAvailableError extends Error {
constructor(namespace: string) {
super(`[TossPayments SDK] ${namespace} is not available`);
this.name = 'NamespaceNotAvailableError';
}
}

export class ScriptLoadFailedError extends Error {
class ScriptLoadFailedError extends Error {
constructor(src: string) {
super(`[TossPayments SDK] Failed to load script: [${src}]`);
this.name = 'ScriptLoadFailedError';
Expand Down
10 changes: 2 additions & 8 deletions packages/tosspayments-sdk/src/loadTossPayments.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,15 @@ describe('loadTossPayments', () => {

test('src를 지정하면 주어진 URL로 script를 로드한다', async () => {
const { loadTossPayments } = await import('./loadTossPayments');
const { NamespaceNotAvailableError } = await import('@tosspayments/sdk-loader');

const testSource = `https://js.tosspayments.com/v1/brandpay`;

try {
await loadTossPayments('test_key', {
src: testSource,
});
} catch (error) {
if (error instanceof NamespaceNotAvailableError) {
// NOTE: SDK에서 namespace에 인스턴스를 꽂아주는 동작이 테스트 환경에서는 일어나지 않아 발생하는 에러를 무시합니다
return;
}

throw error;
} catch {
// NOTE: SDK에서 namespace에 인스턴스를 꽂아주는 동작이 테스트 환경에서는 일어나지 않아 발생하는 에러를 무시합니다.
}

const script = document.querySelector(`script[src="${testSource}"]`);
Expand Down
Loading