Skip to content

Commit 57a5158

Browse files
committed
improve tests
1 parent cb35f42 commit 57a5158

File tree

2 files changed

+55
-23
lines changed

2 files changed

+55
-23
lines changed

packages/browser/test/unit/transports/setup.test.ts

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,17 @@
11
import { NoopTransport } from '@sentry/core';
22

3-
import { FetchTransport, setupBrowserTransport, XHRTransport } from '../../../src/transports';
3+
import {
4+
FetchTransport,
5+
makeNewFetchTransport,
6+
makeNewXHRTransport,
7+
setupBrowserTransport,
8+
XHRTransport,
9+
} from '../../../src/transports';
410
import { SimpleTransport } from '../mocks/simpletransport';
511

612
const DSN = 'https://username@domain/123';
713

814
let fetchSupported = true;
9-
let getNativeFetchImplCalled = false;
1015

1116
jest.mock('@sentry/utils', () => {
1217
const original = jest.requireActual('@sentry/utils');
@@ -23,23 +28,32 @@ jest.mock('@sentry/utils', () => {
2328
};
2429
});
2530

26-
jest.mock('@sentry/browser/src/transports/utils', () => {
27-
const original = jest.requireActual('@sentry/browser/src/transports/utils');
31+
jest.mock('../../../src/transports/new-fetch', () => {
32+
const original = jest.requireActual('../../../src/transports/new-fetch');
2833
return {
2934
...original,
30-
getNativeFetchImplementation() {
31-
getNativeFetchImplCalled = true;
32-
return {
33-
fetch: () => {},
34-
};
35-
},
35+
makeNewFetchTransport: jest.fn(() => ({
36+
send: () => Promise.resolve({ status: 'success' }),
37+
flush: () => Promise.resolve(true),
38+
})),
39+
};
40+
});
41+
42+
jest.mock('../../../src/transports/new-xhr', () => {
43+
const original = jest.requireActual('../../../src/transports/new-xhr');
44+
return {
45+
...original,
46+
makeNewXHRTransport: jest.fn(() => ({
47+
send: () => Promise.resolve({ status: 'success' }),
48+
flush: () => Promise.resolve(true),
49+
})),
3650
};
3751
});
3852

3953
describe('setupBrowserTransport', () => {
40-
beforeEach(() => {
41-
getNativeFetchImplCalled = false;
42-
});
54+
afterEach(() => jest.clearAllMocks());
55+
56+
afterAll(() => jest.resetAllMocks());
4357

4458
it('returns NoopTransport if no dsn is passed', () => {
4559
const { transport, newTransport } = setupBrowserTransport({});
@@ -65,10 +79,8 @@ describe('setupBrowserTransport', () => {
6579
expect(transport).toBeDefined();
6680
expect(transport).toBeInstanceOf(FetchTransport);
6781
expect(newTransport).toBeDefined();
68-
// This is a weird way of testing that `newTransport` is using fetch but it works.
69-
// Given that the new transports are functions, we cannot test their instance.
70-
// Totally open for suggestions how to test this better here
71-
expect(getNativeFetchImplCalled).toBe(true);
82+
expect(makeNewFetchTransport).toHaveBeenCalledTimes(1);
83+
expect(makeNewXHRTransport).toHaveBeenCalledTimes(0);
7284
});
7385

7486
it('returns xhrTransports if fetch is not supported', () => {
@@ -80,9 +92,7 @@ describe('setupBrowserTransport', () => {
8092
expect(transport).toBeDefined();
8193
expect(transport).toBeInstanceOf(XHRTransport);
8294
expect(newTransport).toBeDefined();
83-
// This is a weird way of testing that `newTransport` is using fetch but it works.
84-
// Given that the new transports are functions, we cannot test their instance.
85-
// Totally open for suggestions how to test this better here
86-
expect(getNativeFetchImplCalled).toBe(false);
95+
expect(makeNewFetchTransport).toHaveBeenCalledTimes(0);
96+
expect(makeNewXHRTransport).toHaveBeenCalledTimes(1);
8797
});
8898
});

packages/node/test/transports/setup.test.ts

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,26 @@ import { NoopTransport } from '@sentry/core';
22
import { FakeTransport } from '@sentry/core/test/mocks/transport';
33
import { HTTPSTransport, HTTPTransport, setupNodeTransport } from '@sentry/node/src/transports';
44

5+
import { makeNodeTransport } from '../../src/transports/new';
6+
7+
jest.mock('../../src/transports/new', () => {
8+
const original = jest.requireActual('../../src/transports/new');
9+
return {
10+
...original,
11+
makeNodeTransport: jest.fn(() => ({
12+
send: () => Promise.resolve({ status: 'success' }),
13+
flush: () => Promise.resolve(true),
14+
})),
15+
};
16+
});
17+
518
const DSN = 'https://username@domain/123';
619

720
describe('setupNodeTransport', () => {
21+
afterEach(() => jest.clearAllMocks());
22+
23+
afterAll(() => jest.resetAllMocks());
24+
825
it('returns NoopTransport if no dsn is passed', () => {
926
const { transport, newTransport } = setupNodeTransport({});
1027

@@ -23,22 +40,27 @@ describe('setupNodeTransport', () => {
2340
});
2441

2542
it('returns HTTPS transport as a default', () => {
43+
// jest.spyOn(nodeTransportCreation, 'makeNodeTransport').mockImplementation(() => ({
44+
// send: (request: Envelope) => Promise.resolve({ status: 'success' }),
45+
// flush: (timeout: number) => Promise.resolve(true),
46+
// }));
47+
2648
const options = { dsn: DSN };
2749
const { transport, newTransport } = setupNodeTransport(options);
2850

2951
expect(transport).toBeDefined();
3052
expect(transport).toBeInstanceOf(HTTPSTransport);
3153
expect(newTransport).toBeDefined();
54+
expect(makeNodeTransport).toHaveBeenCalledTimes(1);
3255
});
3356

3457
it('returns HTTP transport if specified in the dsn', () => {
35-
// fetchSupported = false;
36-
3758
const options = { dsn: 'http://username@domain/123' };
3859
const { transport, newTransport } = setupNodeTransport(options);
3960

4061
expect(transport).toBeDefined();
4162
expect(transport).toBeInstanceOf(HTTPTransport);
4263
expect(newTransport).toBeDefined();
64+
expect(makeNodeTransport).toHaveBeenCalledTimes(1);
4365
});
4466
});

0 commit comments

Comments
 (0)