Skip to content

Commit 8442065

Browse files
lobsterkatieAbhiPrasad
authored andcommitted
fix(tests): Fix type errors in tests (#4908)
In the process of updating our jest dependencies, a significant number of type errors in tests suddenly appeared. (It seems newer versions of `jest` et al are stricter than the ones we had been using.) This fixes those errors, and changes our jest config so that it will throw an error on any types problems in a test, so we'll know to fix them locally. (Still to do: Fix our configuration so that the linter will catch the errors, so we don't have to actually run the tests to find them.)
1 parent 28537d6 commit 8442065

35 files changed

+243
-169
lines changed

jest.config.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ module.exports = {
1111
globals: {
1212
'ts-jest': {
1313
tsconfig: '<rootDir>/tsconfig.test.json',
14-
diagnostics: false,
1514
},
1615
},
1716
testPathIgnorePatterns: ['<rootDir>/build/', '<rootDir>/node_modules/'],

packages/browser/test/unit/integrations/linkederrors.test.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { ExtendedError } from '@sentry/types';
1+
import { Event as SentryEvent, Exception, ExtendedError } from '@sentry/types';
22
import { createStackParser } from '@sentry/utils';
33

44
import { BrowserClient } from '../../../src/client';
@@ -8,6 +8,12 @@ import { setupBrowserTransport } from '../../../src/transports';
88

99
const parser = createStackParser(...defaultStackParsers);
1010

11+
type EventWithException = SentryEvent & {
12+
exception: {
13+
values: Exception[];
14+
};
15+
};
16+
1117
describe('LinkedErrors', () => {
1218
describe('handler', () => {
1319
it('should bail out if event does not contain exception', () => {
@@ -44,7 +50,7 @@ describe('LinkedErrors', () => {
4450
return client.eventFromException(originalException).then(event => {
4551
const result = LinkedErrorsModule._handler(parser, 'cause', 5, event, {
4652
originalException,
47-
});
53+
}) as EventWithException;
4854

4955
// It shouldn't include root exception, as it's already processed in the event by the main error handler
5056
expect(result.exception.values.length).toBe(3);
@@ -75,7 +81,7 @@ describe('LinkedErrors', () => {
7581
return client.eventFromException(originalException).then(event => {
7682
const result = LinkedErrorsModule._handler(parser, 'reason', 5, event, {
7783
originalException,
78-
});
84+
}) as EventWithException;
7985

8086
expect(result.exception.values.length).toBe(3);
8187
expect(result.exception.values[0].type).toBe('SyntaxError');
@@ -103,7 +109,7 @@ describe('LinkedErrors', () => {
103109
return client.eventFromException(originalException).then(event => {
104110
const result = LinkedErrorsModule._handler(parser, 'cause', 2, event, {
105111
originalException,
106-
});
112+
}) as EventWithException;
107113

108114
expect(result.exception.values.length).toBe(2);
109115
expect(result.exception.values[0].type).toBe('TypeError');

packages/browser/test/unit/mocks/simpletransport.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { eventStatusFromHttpCode, resolvedSyncPromise } from '@sentry/utils';
33
import { Event, Response } from '../../../src';
44
import { BaseTransport } from '../../../src/transports';
55

6+
// @ts-ignore It's okay that we're not implementing the `_sendRequest()` method because we don't use it in our tests
67
export class SimpleTransport extends BaseTransport {
78
public sendEvent(_: Event): PromiseLike<Response> {
89
return this._buffer.add(() =>

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import { BaseTransport } from '../../../src/transports/base';
33
const testDsn = 'https://[email protected]/42';
44
const envelopeEndpoint = 'https://sentry.io/api/42/envelope/?sentry_key=123&sentry_version=7';
55

6+
// @ts-ignore We're purposely not implementing the methods of the abstract `BaseTransport` class in order to be able to
7+
// assert on what the class provides and what it leaves to the concrete class to implement
68
class SimpleTransport extends BaseTransport {}
79

810
describe('BaseTransport', () => {
@@ -111,12 +113,12 @@ describe('BaseTransport', () => {
111113
});
112114
});
113115

114-
it('doesnt provide sendEvent() implementation', () => {
116+
it('doesnt provide sendEvent() implementation', async () => {
115117
expect.assertions(1);
116118
const transport = new SimpleTransport({ dsn: testDsn });
117119

118120
try {
119-
void transport.sendEvent({});
121+
await transport.sendEvent({});
120122
} catch (e) {
121123
expect(e).toBeDefined();
122124
}

packages/browser/test/unit/transports/new-xhr.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ function createXHRMock() {
2727
case 'Retry-After':
2828
return '10';
2929
case `${retryAfterSeconds}`:
30-
return;
30+
return null;
3131
default:
3232
return `${retryAfterSeconds}:error:scope`;
3333
}
@@ -57,7 +57,7 @@ describe('NewXHRTransport', () => {
5757
expect(xhrMock.setRequestHeader).toHaveBeenCalledTimes(0);
5858
expect(xhrMock.send).toHaveBeenCalledTimes(0);
5959

60-
await Promise.all([transport.send(ERROR_ENVELOPE), (xhrMock as XMLHttpRequest).onreadystatechange(null)]);
60+
await Promise.all([transport.send(ERROR_ENVELOPE), (xhrMock as XMLHttpRequest).onreadystatechange!({} as Event)]);
6161

6262
expect(xhrMock.open).toHaveBeenCalledTimes(1);
6363
expect(xhrMock.open).toHaveBeenCalledWith('POST', DEFAULT_XHR_TRANSPORT_OPTIONS.url);
@@ -70,7 +70,7 @@ describe('NewXHRTransport', () => {
7070

7171
const [res] = await Promise.all([
7272
transport.send(ERROR_ENVELOPE),
73-
(xhrMock as XMLHttpRequest).onreadystatechange(null),
73+
(xhrMock as XMLHttpRequest).onreadystatechange!({} as Event),
7474
]);
7575

7676
expect(res).toBeDefined();
@@ -80,7 +80,7 @@ describe('NewXHRTransport', () => {
8080
it('sets rate limit response headers', async () => {
8181
const transport = makeNewXHRTransport(DEFAULT_XHR_TRANSPORT_OPTIONS);
8282

83-
await Promise.all([transport.send(ERROR_ENVELOPE), (xhrMock as XMLHttpRequest).onreadystatechange(null)]);
83+
await Promise.all([transport.send(ERROR_ENVELOPE), (xhrMock as XMLHttpRequest).onreadystatechange!({} as Event)]);
8484

8585
expect(xhrMock.getResponseHeader).toHaveBeenCalledTimes(2);
8686
expect(xhrMock.getResponseHeader).toHaveBeenCalledWith('X-Sentry-Rate-Limits');
@@ -99,7 +99,7 @@ describe('NewXHRTransport', () => {
9999
};
100100

101101
const transport = makeNewXHRTransport(options);
102-
await Promise.all([transport.send(ERROR_ENVELOPE), (xhrMock as XMLHttpRequest).onreadystatechange(null)]);
102+
await Promise.all([transport.send(ERROR_ENVELOPE), (xhrMock as XMLHttpRequest).onreadystatechange!({} as Event)]);
103103

104104
expect(xhrMock.setRequestHeader).toHaveBeenCalledTimes(3);
105105
expect(xhrMock.setRequestHeader).toHaveBeenCalledWith('referrerPolicy', headers.referrerPolicy);

packages/core/test/lib/base.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ describe('BaseClient', () => {
6969

7070
const options = { dsn: PUBLIC_DSN };
7171
const client = new TestClient(options, setupTestTransport(options).transport);
72-
expect(dsnToString(client.getDsn())).toBe(PUBLIC_DSN);
72+
expect(dsnToString(client.getDsn()!)).toBe(PUBLIC_DSN);
7373
});
7474

7575
test('allows missing Dsn', () => {

packages/core/test/mocks/client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export function setupTestTransport(options: TestOptions): { transport: Transport
7979
const transportOptions = options.transportOptions ? options.transportOptions : { dsn: options.dsn };
8080

8181
if (options.transport) {
82-
return { transport: new this._options.transport(transportOptions) };
82+
return { transport: new options.transport(transportOptions) };
8383
}
8484

8585
return noop;

packages/gatsby/test/gatsby-browser.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/* eslint-disable @typescript-eslint/no-var-requires */
22
/* eslint-disable @typescript-eslint/no-explicit-any */
33

4-
const { onClientEntry } = require('../gatsby-browser');
4+
import { onClientEntry } from '../gatsby-browser';
55

66
(global as any).__SENTRY_RELEASE__ = '683f3a6ab819d47d23abfca9a914c81f0524d35b';
77
(global as any).__SENTRY_DSN__ = 'https://[email protected]/0';
@@ -153,7 +153,7 @@ describe('onClientEntry', () => {
153153

154154
// Run this last to check for any test side effects
155155
it('does not run if plugin params are undefined', () => {
156-
onClientEntry();
156+
onClientEntry(undefined, undefined);
157157
expect(sentryInit).toHaveBeenCalledTimes(0);
158158
expect(tracingAddExtensionMethods).toHaveBeenCalledTimes(0);
159159
});

packages/gatsby/test/gatsby-node.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* eslint-disable @typescript-eslint/no-var-requires */
22
/* eslint-disable @typescript-eslint/no-explicit-any */
3-
const { onCreateWebpackConfig } = require('../gatsby-node');
3+
import { onCreateWebpackConfig } from '../gatsby-node';
44

55
describe('onCreateWebpackConfig', () => {
66
it('sets a webpack config', () => {
@@ -12,7 +12,9 @@ describe('onCreateWebpackConfig', () => {
1212
setWebpackConfig: jest.fn(),
1313
};
1414

15-
onCreateWebpackConfig({ plugins, actions });
15+
const getConfig = jest.fn();
16+
17+
onCreateWebpackConfig({ plugins, actions, getConfig });
1618

1719
expect(plugins.define).toHaveBeenCalledTimes(1);
1820
expect(plugins.define).toHaveBeenLastCalledWith({

packages/gatsby/test/setEnvVars.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,6 @@
11
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access
22
process.env.SENTRY_RELEASE = '14abbb1678a2eb59d1a171ea33d630dd6c6eee70';
3+
4+
// This file needs to have an import or an export to count as a module, which is necessary when using
5+
// the `isolatedModules` tsconfig option.
6+
export const _ = '';

packages/hub/test/global.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
1+
import { getGlobalObject } from '@sentry/utils';
2+
13
import { getCurrentHub, getHubFromCarrier, Hub } from '../src';
24

5+
const global = getGlobalObject();
6+
37
describe('global', () => {
48
test('getGlobalHub', () => {
59
expect(getCurrentHub()).toBeTruthy();

0 commit comments

Comments
 (0)