Skip to content

Commit

Permalink
fix(core): Check that functions have been called in beforeSend and …
Browse files Browse the repository at this point in the history 
…`beforeSendTransaction` tests (#6150)

Per @Lms24's suggestion[1], this adds a check to make sure the relevant function is being called in our `beforeSend` and `beforeSendTransaction` tests.

[1] #6121 (comment)
  • Loading branch information
@lobsterkatie
lobsterkatie committed Nov 8, 2022
1 parent 1e23e90 commit 1f3fc98
Showing 1 changed file with 45 additions and 24 deletions.
69 changes: 45 additions & 24 deletions packages/core/test/lib/base.test.ts
View file
Expand Up @@ -888,31 +888,33 @@ describe('BaseClient', () => {
});

test('calls `beforeSend` and uses original event without any changes', () => {
expect.assertions(1);
expect.assertions(2);

const beforeSend = jest.fn(event => event);
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend });
const client = new TestClient(options);

client.captureEvent({ message: 'hello' });

expect(beforeSend).toHaveBeenCalled();
expect(TestClient.instance!.event!.message).toEqual('hello');
});

test('calls `beforeSendTransaction` and uses original event without any changes', () => {
expect.assertions(1);
expect.assertions(2);

const beforeSendTransaction = jest.fn(event => event);
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction });
const client = new TestClient(options);

client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' });

expect(beforeSendTransaction).toHaveBeenCalled();
expect(TestClient.instance!.event!.transaction).toBe('/dogs/are/great');
});

test('calls `beforeSend` and uses the modified event', () => {
expect.assertions(1);
expect.assertions(2);

const beforeSend = jest.fn(event => {
event.message = 'changed1';
Expand All @@ -923,11 +925,12 @@ describe('BaseClient', () => {

client.captureEvent({ message: 'hello' });

expect(beforeSend).toHaveBeenCalled();
expect(TestClient.instance!.event!.message).toEqual('changed1');
});

test('calls `beforeSendTransaction` and uses the modified event', () => {
expect.assertions(1);
expect.assertions(2);

const beforeSendTransaction = jest.fn(event => {
event.transaction = '/adopt/dont/shop';
Expand All @@ -938,11 +941,12 @@ describe('BaseClient', () => {

client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' });

expect(beforeSendTransaction).toHaveBeenCalled();
expect(TestClient.instance!.event!.transaction).toBe('/adopt/dont/shop');
});

test('calls `beforeSend` and discards the event', () => {
expect.assertions(3);
expect.assertions(4);

const beforeSend = jest.fn(() => null);
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend });
Expand All @@ -952,6 +956,7 @@ describe('BaseClient', () => {

client.captureEvent({ message: 'hello' });

expect(beforeSend).toHaveBeenCalled();
expect(TestClient.instance!.event).toBeUndefined();
// This proves that the reason the event didn't send/didn't get set on the test client is not because there was an
// error, but because `beforeSend` returned `null`
Expand All @@ -960,7 +965,7 @@ describe('BaseClient', () => {
});

test('calls `beforeSendTransaction` and discards the event', () => {
expect.assertions(3);
expect.assertions(4);

const beforeSendTransaction = jest.fn(() => null);
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction });
Expand All @@ -970,6 +975,7 @@ describe('BaseClient', () => {

client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' });

expect(beforeSendTransaction).toHaveBeenCalled();
expect(TestClient.instance!.event).toBeUndefined();
// This proves that the reason the event didn't send/didn't get set on the test client is not because there was an
// error, but because `beforeSendTransaction` returned `null`
Expand All @@ -979,7 +985,7 @@ describe('BaseClient', () => {

test('calls `beforeSend` and logs info about invalid return value', () => {
const invalidValues = [undefined, false, true, [], 1];
expect.assertions(invalidValues.length * 2);
expect.assertions(invalidValues.length * 3);

for (const val of invalidValues) {
const beforeSend = jest.fn(() => val);
Expand All @@ -990,14 +996,15 @@ describe('BaseClient', () => {

client.captureEvent({ message: 'hello' });

expect(beforeSend).toHaveBeenCalled();
expect(TestClient.instance!.event).toBeUndefined();
expect(loggerWarnSpy).toBeCalledWith(new SentryError('`beforeSend` must return `null` or a valid event.'));
}
});

test('calls `beforeSendTransaction` and logs info about invalid return value', () => {
const invalidValues = [undefined, false, true, [], 1];
expect.assertions(invalidValues.length * 2);
expect.assertions(invalidValues.length * 3);

for (const val of invalidValues) {
const beforeSendTransaction = jest.fn(() => val);
Expand All @@ -1008,6 +1015,7 @@ describe('BaseClient', () => {

client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' });

expect(beforeSendTransaction).toHaveBeenCalled();
expect(TestClient.instance!.event).toBeUndefined();
expect(loggerWarnSpy).toBeCalledWith(
new SentryError('`beforeSendTransaction` must return `null` or a valid event.'),
Expand All @@ -1017,7 +1025,7 @@ describe('BaseClient', () => {

test('calls async `beforeSend` and uses original event without any changes', done => {
jest.useFakeTimers();
expect.assertions(1);
expect.assertions(2);

const beforeSend = jest.fn(
async event =>
Expand All @@ -1034,6 +1042,7 @@ describe('BaseClient', () => {
jest.runOnlyPendingTimers();

TestClient.sendEventCalled = (event: Event) => {
expect(beforeSend).toHaveBeenCalled();
expect(event.message).toEqual('hello');
};

Expand All @@ -1046,7 +1055,7 @@ describe('BaseClient', () => {

test('calls async `beforeSendTransaction` and uses original event without any changes', done => {
jest.useFakeTimers();
expect.assertions(1);
expect.assertions(2);

const beforeSendTransaction = jest.fn(
async event =>
Expand All @@ -1063,6 +1072,7 @@ describe('BaseClient', () => {
jest.runOnlyPendingTimers();

TestClient.sendEventCalled = (event: Event) => {
expect(beforeSendTransaction).toHaveBeenCalled();
expect(event.transaction).toBe('/dogs/are/great');
};

Expand All @@ -1075,7 +1085,7 @@ describe('BaseClient', () => {

test('calls async `beforeSend` and uses the modified event', done => {
jest.useFakeTimers();
expect.assertions(1);
expect.assertions(2);

const beforeSend = jest.fn(async event => {
event.message = 'changed2';
Expand All @@ -1092,6 +1102,7 @@ describe('BaseClient', () => {
jest.runOnlyPendingTimers();

TestClient.sendEventCalled = (event: Event) => {
expect(beforeSend).toHaveBeenCalled();
expect(event.message).toEqual('changed2');
};

Expand All @@ -1104,7 +1115,7 @@ describe('BaseClient', () => {

test('calls async `beforeSendTransaction` and uses the modified event', done => {
jest.useFakeTimers();
expect.assertions(1);
expect.assertions(2);

const beforeSendTransaction = jest.fn(async event => {
event.transaction = '/adopt/dont/shop';
Expand All @@ -1121,6 +1132,7 @@ describe('BaseClient', () => {
jest.runOnlyPendingTimers();

TestClient.sendEventCalled = (event: Event) => {
expect(beforeSendTransaction).toHaveBeenCalled();
expect(event.transaction).toBe('/adopt/dont/shop');
};

Expand All @@ -1133,7 +1145,7 @@ describe('BaseClient', () => {

test('calls async `beforeSend` and discards the event', () => {
jest.useFakeTimers();
expect.assertions(1);
expect.assertions(2);

const beforeSend = jest.fn(
async () =>
Expand All @@ -1149,12 +1161,13 @@ describe('BaseClient', () => {
client.captureEvent({ message: 'hello' });
jest.runAllTimers();

expect(beforeSend).toHaveBeenCalled();
expect(TestClient.instance!.event).toBeUndefined();
});

test('calls async `beforeSendTransaction` and discards the event', () => {
jest.useFakeTimers();
expect.assertions(1);
expect.assertions(2);

const beforeSendTransaction = jest.fn(
async () =>
Expand All @@ -1170,24 +1183,26 @@ describe('BaseClient', () => {
client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' });
jest.runAllTimers();

expect(beforeSendTransaction).toHaveBeenCalled();
expect(TestClient.instance!.event).toBeUndefined();
});

test('`beforeSend` gets access to a hint as a second argument', () => {
expect.assertions(2);
expect.assertions(3);

const beforeSend = jest.fn((event, hint) => ({ ...event, data: hint.data }));
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSend });
const client = new TestClient(options);

client.captureEvent({ message: 'hello' }, { data: 'someRandomThing' });

expect(beforeSend).toHaveBeenCalledWith(expect.any(Object), expect.objectContaining({ data: 'someRandomThing' }));
expect(TestClient.instance!.event!.message).toEqual('hello');
expect((TestClient.instance!.event! as any).data).toEqual('someRandomThing');
});

test('`beforeSendTransaction` gets access to a hint as a second argument', () => {
expect.assertions(2);
expect.assertions(3);

const beforeSendTransaction = jest.fn((event, hint) => ({ ...event, data: hint.data }));
const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN, beforeSendTransaction });
Expand All @@ -1198,45 +1213,50 @@ describe('BaseClient', () => {
{ data: { dogs: 'yes', cats: 'maybe' } },
);

expect(beforeSendTransaction).toHaveBeenCalledWith(
expect.any(Object),
expect.objectContaining({ data: { dogs: 'yes', cats: 'maybe' } }),
);
expect(TestClient.instance!.event!.transaction).toBe('/dogs/are/great');
expect((TestClient.instance!.event! as any).data).toEqual({ dogs: 'yes', cats: 'maybe' });
});

test('`beforeSend` records dropped events', () => {
expect.assertions(1);
expect.assertions(2);

const beforeSend = jest.fn(() => null);
const client = new TestClient(
getDefaultTestClientOptions({
dsn: PUBLIC_DSN,
beforeSend() {
return null;
},
beforeSend,
}),
);

const recordLostEventSpy = jest.spyOn(client, 'recordDroppedEvent');

client.captureEvent({ message: 'hello' }, {});

expect(beforeSend).toHaveBeenCalled();
expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'error');
});

test('`beforeSendTransaction` records dropped events', () => {
expect.assertions(1);
expect.assertions(2);

const beforeSendTransaction = jest.fn(() => null);

const client = new TestClient(
getDefaultTestClientOptions({
dsn: PUBLIC_DSN,
beforeSendTransaction() {
return null;
},
beforeSendTransaction,
}),
);

const recordLostEventSpy = jest.spyOn(client, 'recordDroppedEvent');

client.captureEvent({ transaction: '/dogs/are/great', type: 'transaction' });

expect(beforeSendTransaction).toHaveBeenCalled();
expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'transaction');
});

Expand Down Expand Up @@ -1361,6 +1381,7 @@ describe('BaseClient', () => {
},
});

expect(beforeSendTransaction).toHaveBeenCalled();
expect(TestClient.instance!.event!.transaction).toBe('/adopt/dont/shop');
expect(TestClient.instance!.event!.transaction_info).toEqual({
source: 'custom',
Expand Down

0 comments on commit 1f3fc98

Please sign in to comment.