Skip to content

Commit e2a16ed

Browse files
lobsterkatielobsterkatie
committed
clean up existing request tests
1 parent 8f1a21a commit e2a16ed

File tree

1 file changed

+98
-142
lines changed

1 file changed

+98
-142
lines changed

‎packages/tracing/test/browser/request.test.ts

Lines changed: 98 additions & 142 deletions
Original file line numberDiff line numberDiff line change
@@ -1,79 +1,63 @@
11
import { BrowserClient } from '@sentry/browser';
22
import { Hub, makeMain } from '@sentry/hub';
3+
import * as utils from '@sentry/utils';
34

45
import { Span, SpanStatus, Transaction } from '../../src';
56
import { fetchCallback, FetchData, registerRequestInstrumentation } from '../../src/browser/request';
67
import { addExtensionMethods } from '../../src/hubextensions';
78

8-
declare global {
9-
// eslint-disable-next-line @typescript-eslint/no-namespace
10-
namespace NodeJS {
11-
interface Global {
12-
// Have to mock out Request because it is not defined in jest environment
13-
Request: Request;
14-
}
15-
}
16-
}
17-
189
beforeAll(() => {
1910
addExtensionMethods();
20-
// @ts-ignore need to override global Request
11+
// @ts-ignore need to override global Request because it's not in the jest environment (even with an
12+
// `@jest-environment jsdom` directive, for some reason)
2113
global.Request = {};
2214
});
2315

24-
const mockAddInstrumentationHandler = jest.fn();
25-
let mockFetchCallback = jest.fn();
26-
let mockXHRCallback = jest.fn();
27-
jest.mock('@sentry/utils', () => {
28-
const actual = jest.requireActual('@sentry/utils');
29-
return {
30-
...actual,
31-
addInstrumentationHandler: ({ callback, type }: any) => {
32-
if (type === 'fetch') {
33-
mockFetchCallback = jest.fn(callback);
34-
}
35-
if (type === 'xhr') {
36-
mockXHRCallback = jest.fn(callback);
37-
}
38-
if (typeof mockAddInstrumentationHandler === 'function') {
39-
return mockAddInstrumentationHandler({ callback, type });
40-
}
41-
},
42-
};
43-
});
16+
const addInstrumentationHandler = jest.spyOn(utils, 'addInstrumentationHandler');
4417

4518
describe('registerRequestInstrumentation', () => {
4619
beforeEach(() => {
47-
mockFetchCallback.mockReset();
48-
mockXHRCallback.mockReset();
49-
mockAddInstrumentationHandler.mockReset();
20+
jest.clearAllMocks();
5021
});
5122

52-
it('tracks fetch and xhr requests', () => {
23+
it('instruments fetch and xhr requests', () => {
5324
registerRequestInstrumentation();
54-
expect(mockAddInstrumentationHandler).toHaveBeenCalledTimes(2);
55-
// fetch
56-
expect(mockAddInstrumentationHandler).toHaveBeenNthCalledWith(1, { callback: expect.any(Function), type: 'fetch' });
57-
// xhr
58-
expect(mockAddInstrumentationHandler).toHaveBeenNthCalledWith(2, { callback: expect.any(Function), type: 'xhr' });
25+
26+
expect(addInstrumentationHandler).toHaveBeenCalledWith({
27+
callback: expect.any(Function),
28+
type: 'fetch',
29+
});
30+
expect(addInstrumentationHandler).toHaveBeenCalledWith({
31+
callback: expect.any(Function),
32+
type: 'xhr',
33+
});
5934
});
6035

61-
it('does not add fetch requests spans if traceFetch is false', () => {
36+
it('does not instrument fetch requests if traceFetch is false', () => {
6237
registerRequestInstrumentation({ traceFetch: false });
63-
expect(mockAddInstrumentationHandler).toHaveBeenCalledTimes(1);
64-
expect(mockFetchCallback()).toBe(undefined);
38+
39+
expect(addInstrumentationHandler).not.toHaveBeenCalledWith({ callback: expect.any(Function), type: 'fetch' });
6540
});
6641

67-
it('does not add xhr requests spans if traceXHR is false', () => {
42+
it('does not instrument xhr requests if traceXHR is false', () => {
6843
registerRequestInstrumentation({ traceXHR: false });
69-
expect(mockAddInstrumentationHandler).toHaveBeenCalledTimes(1);
70-
expect(mockXHRCallback()).toBe(undefined);
44+
45+
expect(addInstrumentationHandler).not.toHaveBeenCalledWith({ callback: expect.any(Function), type: 'xhr' });
7146
});
7247
});
7348

74-
describe('_fetchCallback()', () => {
49+
describe('callbacks', () => {
7550
let hub: Hub;
7651
let transaction: Transaction;
52+
const alwaysCreateSpan = jest.fn().mockReturnValue(true);
53+
const neverCreateSpan = jest.fn().mockReturnValue(false);
54+
const fetchHandlerData: FetchData = {
55+
args: ['http://dogs.are.great/', {}],
56+
fetchData: { url: 'http://dogs.are.great/', method: 'GET' },
57+
startTimestamp: 1356996072000,
58+
};
59+
const endTimestamp = 1356996072000;
60+
7761
beforeAll(() => {
7862
hub = new Hub(new BrowserClient({ tracesSampleRate: 1 }));
7963
makeMain(hub);
@@ -84,106 +68,78 @@ describe('_fetchCallback()', () => {
8468
hub.configureScope(scope => scope.setSpan(transaction));
8569
});
8670

87-
it('does not create span if it should not be created', () => {
88-
const shouldCreateSpan = (url: string): boolean => url === '/organizations';
89-
const data: FetchData = {
90-
args: ['/users'],
91-
fetchData: {
92-
method: 'GET',
93-
url: '/users',
94-
},
95-
startTimestamp: 1595509730275,
96-
};
97-
const spans = {};
98-
99-
fetchCallback(data, shouldCreateSpan, spans);
100-
expect(spans).toEqual({});
101-
});
71+
describe('fetchCallback()', () => {
72+
it('does not create span if shouldCreateSpan returns false', () => {
73+
const spans = {};
10274

103-
it('does not create span if there is no fetch data', () => {
104-
const shouldCreateSpan = (_: string): boolean => true;
105-
const data: FetchData = {
106-
args: [],
107-
startTimestamp: 1595509730275,
108-
};
109-
const spans = {};
75+
fetchCallback(fetchHandlerData, neverCreateSpan, spans);
11076

111-
fetchCallback(data, shouldCreateSpan, spans);
112-
expect(spans).toEqual({});
113-
});
77+
expect(spans).toEqual({});
78+
});
11479

115-
it('creates and finishes fetch span on active transaction', () => {
116-
const shouldCreateSpan = (_: string): boolean => true;
117-
const data: FetchData = {
118-
args: ['/users'],
119-
fetchData: {
120-
method: 'GET',
121-
url: '/users',
122-
},
123-
startTimestamp: 1595509730275,
124-
};
125-
const spans: Record<string, Span> = {};
126-
127-
// Start fetch request
128-
fetchCallback(data, shouldCreateSpan, spans);
129-
const spanKey = Object.keys(spans)[0];
130-
131-
const fetchSpan = spans[spanKey];
132-
expect(fetchSpan).toBeInstanceOf(Span);
133-
expect(fetchSpan.data).toEqual({
134-
method: 'GET',
135-
type: 'fetch',
136-
url: '/users',
80+
it('does not create span if there is no fetch data in handler data', () => {
81+
const noFetchData = { args: fetchHandlerData.args, startTimestamp: fetchHandlerData.startTimestamp };
82+
const spans = {};
83+
84+
fetchCallback(noFetchData, alwaysCreateSpan, spans);
85+
expect(spans).toEqual({});
13786
});
138-
expect(fetchSpan.description).toBe('GET /users');
139-
expect(fetchSpan.op).toBe('http');
140-
if (data.fetchData) {
141-
expect(data.fetchData.__span).toBeDefined();
142-
} else {
143-
fail('Fetch data does not exist');
144-
}
145-
146-
const newData = {
147-
...data,
148-
endTimestamp: data.startTimestamp + 12343234,
149-
};
150-
151-
// End fetch request
152-
fetchCallback(newData, shouldCreateSpan, spans);
153-
expect(spans).toEqual({});
154-
if (transaction.spanRecorder) {
155-
expect(transaction.spanRecorder.spans[1].endTimestamp).toBeDefined();
156-
} else {
157-
fail('Transaction does not have span recorder');
158-
}
159-
});
16087

161-
it('sets response status on finish', () => {
162-
const shouldCreateSpan = (_: string): boolean => true;
163-
const data: FetchData = {
164-
args: ['/users'],
165-
fetchData: {
88+
89+
it('creates and finishes fetch span on active transaction', () => {
90+
const spans = {};
91+
92+
// triggered by request being sent
93+
fetchCallback(fetchHandlerData, alwaysCreateSpan, spans);
94+
95+
const newSpan = transaction.spanRecorder?.spans[1];
96+
97+
expect(newSpan).toBeDefined();
98+
expect(newSpan).toBeInstanceOf(Span);
99+
expect(newSpan!.data).toEqual({
166100
method: 'GET',
167-
url: '/users',
168-
},
169-
startTimestamp: 1595509730275,
170-
};
171-
const spans: Record<string, Span> = {};
172-
173-
// Start fetch request
174-
fetchCallback(data, shouldCreateSpan, spans);
175-
176-
const newData = {
177-
...data,
178-
endTimestamp: data.startTimestamp + 12343234,
179-
response: { status: 404 } as Response,
180-
};
181-
// End fetch request
182-
fetchCallback(newData, shouldCreateSpan, spans);
183-
if (transaction.spanRecorder) {
184-
expect(transaction.spanRecorder.spans[1].status).toBe(SpanStatus.fromHttpCode(404));
185-
} else {
186-
fail('Transaction does not have span recorder');
187-
}
101+
type: 'fetch',
102+
url: 'http://dogs.are.great/',
103+
});
104+
expect(newSpan!.description).toBe('GET http://dogs.are.great/');
105+
expect(newSpan!.op).toBe('http');
106+
expect(fetchHandlerData.fetchData?.__span).toBeDefined();
107+
108+
const postRequestFetchHandlerData = {
109+
...fetchHandlerData,
110+
endTimestamp,
111+
};
112+
113+
// triggered by response coming back
114+
fetchCallback(postRequestFetchHandlerData, alwaysCreateSpan, spans);
115+
116+
expect(newSpan!.endTimestamp).toBeDefined();
117+
});
118+
119+
it('sets response status on finish', () => {
120+
const spans: Record<string, Span> = {};
121+
122+
// triggered by request being sent
123+
fetchCallback(fetchHandlerData, alwaysCreateSpan, spans);
124+
125+
const newSpan = transaction.spanRecorder?.spans[1];
126+
127+
expect(newSpan).toBeDefined();
128+
129+
const postRequestFetchHandlerData = {
130+
...fetchHandlerData,
131+
endTimestamp,
132+
response: { status: 404 } as Response,
133+
};
134+
135+
// triggered by response coming back
136+
fetchCallback(postRequestFetchHandlerData, alwaysCreateSpan, spans);
137+
138+
expect(newSpan!.status).toBe(SpanStatus.fromHttpCode(404));
139+
});
140+
141+
it('adds sentry-trace header to fetch requests', () => {
142+
// TODO
143+
});
188144
});
189145
});

0 commit comments

Comments
 (0)