Skip to content

Commit aec3278

Browse files
timfishtimfish
committed
remove the timeout overrides and add some more comments
1 parent af66eb4 commit aec3278

File tree

3 files changed

+120
-170
lines changed

3 files changed

+120
-170
lines changed

‎dev-packages/node-integration-tests/suites/anr/test.ts

Lines changed: 77 additions & 111 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import { conditionalTest } from '../../utils';
22
import { assertSentryEvent, assertSentrySession, createRunner } from '../../utils/runner';
33

4-
const ANR_TEST_TIMEOUT = 10_000;
5-
64
const EXPECTED_ANR_EVENT = {
75
// Ensure we have context
86
contexts: {
@@ -54,123 +52,91 @@ const EXPECTED_ANR_EVENT = {
5452
};
5553

5654
conditionalTest({ min: 16 })('should report ANR when event loop blocked', () => {
57-
test(
58-
'CJS',
59-
done => {
60-
createRunner(__dirname, 'basic.js')
61-
.expect({
62-
event: event => {
63-
assertSentryEvent(event, EXPECTED_ANR_EVENT);
64-
},
65-
})
66-
.start(done);
67-
},
68-
ANR_TEST_TIMEOUT,
69-
);
55+
test('CJS', done => {
56+
createRunner(__dirname, 'basic.js')
57+
.expect({
58+
event: event => {
59+
assertSentryEvent(event, EXPECTED_ANR_EVENT);
60+
},
61+
})
62+
.start(done);
63+
});
7064

7165
// TODO (v8): Remove this old API and this test
72-
test(
73-
'Legacy API',
74-
done => {
75-
createRunner(__dirname, 'legacy.js')
76-
.expect({
77-
event: event => {
78-
assertSentryEvent(event, EXPECTED_ANR_EVENT);
79-
},
80-
})
81-
.start(done);
82-
},
83-
ANR_TEST_TIMEOUT,
84-
);
66+
test('Legacy API', done => {
67+
createRunner(__dirname, 'legacy.js')
68+
.expect({
69+
event: event => {
70+
assertSentryEvent(event, EXPECTED_ANR_EVENT);
71+
},
72+
})
73+
.start(done);
74+
});
8575

86-
test(
87-
'ESM',
88-
done => {
89-
createRunner(__dirname, 'basic.mjs')
90-
.expect({
91-
event: event => {
92-
assertSentryEvent(event, EXPECTED_ANR_EVENT);
93-
},
94-
})
95-
.start(done);
96-
},
97-
ANR_TEST_TIMEOUT,
98-
);
76+
test('ESM', done => {
77+
createRunner(__dirname, 'basic.mjs')
78+
.expect({
79+
event: event => {
80+
assertSentryEvent(event, EXPECTED_ANR_EVENT);
81+
},
82+
})
83+
.start(done);
84+
});
9985

100-
test(
101-
'With --inspect',
102-
done => {
103-
createRunner(__dirname, 'basic.mjs')
104-
.withFlags('--inspect')
105-
.expect({
106-
event: event => {
107-
assertSentryEvent(event, EXPECTED_ANR_EVENT);
108-
},
109-
})
110-
.start(done);
111-
},
112-
ANR_TEST_TIMEOUT,
113-
);
86+
test('With --inspect', done => {
87+
createRunner(__dirname, 'basic.mjs')
88+
.withFlags('--inspect')
89+
.expect({
90+
event: event => {
91+
assertSentryEvent(event, EXPECTED_ANR_EVENT);
92+
},
93+
})
94+
.start(done);
95+
});
11496

115-
test(
116-
'should exit',
117-
done => {
118-
const runner = createRunner(__dirname, 'should-exit.js').start();
97+
test('should exit', done => {
98+
const runner = createRunner(__dirname, 'should-exit.js').start();
11999

120-
setTimeout(() => {
121-
expect(runner.childHasExited()).toBe(true);
122-
done();
123-
}, 5_000);
124-
},
125-
ANR_TEST_TIMEOUT,
126-
);
100+
setTimeout(() => {
101+
expect(runner.childHasExited()).toBe(true);
102+
done();
103+
}, 5_000);
104+
});
127105

128-
test(
129-
'should exit forced',
130-
done => {
131-
const runner = createRunner(__dirname, 'should-exit-forced.js').start();
106+
test('should exit forced', done => {
107+
const runner = createRunner(__dirname, 'should-exit-forced.js').start();
132108

133-
setTimeout(() => {
134-
expect(runner.childHasExited()).toBe(true);
135-
done();
136-
}, 5_000);
137-
},
138-
ANR_TEST_TIMEOUT,
139-
);
109+
setTimeout(() => {
110+
expect(runner.childHasExited()).toBe(true);
111+
done();
112+
}, 5_000);
113+
});
140114

141-
test(
142-
'With session',
143-
done => {
144-
createRunner(__dirname, 'basic-session.js')
145-
.expect({
146-
session: session => {
147-
assertSentrySession(session, {
148-
status: 'abnormal',
149-
abnormal_mechanism: 'anr_foreground',
150-
});
151-
},
152-
})
153-
.expect({
154-
event: event => {
155-
assertSentryEvent(event, EXPECTED_ANR_EVENT);
156-
},
157-
})
158-
.start(done);
159-
},
160-
ANR_TEST_TIMEOUT,
161-
);
115+
test('With session', done => {
116+
createRunner(__dirname, 'basic-session.js')
117+
.expect({
118+
session: session => {
119+
assertSentrySession(session, {
120+
status: 'abnormal',
121+
abnormal_mechanism: 'anr_foreground',
122+
});
123+
},
124+
})
125+
.expect({
126+
event: event => {
127+
assertSentryEvent(event, EXPECTED_ANR_EVENT);
128+
},
129+
})
130+
.start(done);
131+
});
162132

163-
test(
164-
'from forked process',
165-
done => {
166-
createRunner(__dirname, 'forker.js')
167-
.expect({
168-
event: event => {
169-
assertSentryEvent(event, EXPECTED_ANR_EVENT);
170-
},
171-
})
172-
.start(done);
173-
},
174-
ANR_TEST_TIMEOUT,
175-
);
133+
test('from forked process', done => {
134+
createRunner(__dirname, 'forker.js')
135+
.expect({
136+
event: event => {
137+
assertSentryEvent(event, EXPECTED_ANR_EVENT);
138+
},
139+
})
140+
.start(done);
141+
});
176142
});

‎dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts

Lines changed: 34 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import * as path from 'path';
33
import { conditionalTest } from '../../../utils';
44
import { assertSentryEvent, createRunner } from '../../../utils/runner';
55

6-
const LOCAL_VARIABLES_TEST_TIMEOUT = 20_000;
76
const EXPECTED_LOCAL_VARIABLES_EVENT = {
87
exception: {
98
values: [
@@ -31,57 +30,45 @@ const EXPECTED_LOCAL_VARIABLES_EVENT = {
3130
};
3231

3332
conditionalTest({ min: 18 })('LocalVariables integration', () => {
34-
test(
35-
'Should not include local variables by default',
36-
done => {
37-
createRunner(__dirname, 'no-local-variables.js')
38-
.ignore('session')
39-
.expect({
40-
event: event => {
41-
const frames = event.exception?.values?.[0].stacktrace?.frames || [];
42-
const lastFrame = frames[frames.length - 1];
33+
test('Should not include local variables by default', done => {
34+
createRunner(__dirname, 'no-local-variables.js')
35+
.ignore('session')
36+
.expect({
37+
event: event => {
38+
const frames = event.exception?.values?.[0].stacktrace?.frames || [];
39+
const lastFrame = frames[frames.length - 1];
4340

44-
expect(lastFrame.vars).toBeUndefined();
41+
expect(lastFrame.vars).toBeUndefined();
4542

46-
const penultimateFrame = frames[frames.length - 2];
43+
const penultimateFrame = frames[frames.length - 2];
4744

48-
expect(penultimateFrame.vars).toBeUndefined();
49-
},
50-
})
51-
.start(done);
52-
},
53-
LOCAL_VARIABLES_TEST_TIMEOUT,
54-
);
45+
expect(penultimateFrame.vars).toBeUndefined();
46+
},
47+
})
48+
.start(done);
49+
});
5550

56-
test(
57-
'Should include local variables when enabled',
58-
done => {
59-
createRunner(__dirname, 'local-variables.js')
60-
.ignore('session')
61-
.expect({
62-
event: event => {
63-
assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT);
64-
},
65-
})
66-
.start(done);
67-
},
68-
LOCAL_VARIABLES_TEST_TIMEOUT,
69-
);
51+
test('Should include local variables when enabled', done => {
52+
createRunner(__dirname, 'local-variables.js')
53+
.ignore('session')
54+
.expect({
55+
event: event => {
56+
assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT);
57+
},
58+
})
59+
.start(done);
60+
});
7061

71-
test(
72-
'Should include local variables with ESM',
73-
done => {
74-
createRunner(__dirname, 'local-variables-caught.mjs')
75-
.ignore('session')
76-
.expect({
77-
event: event => {
78-
assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT);
79-
},
80-
})
81-
.start(done);
82-
},
83-
LOCAL_VARIABLES_TEST_TIMEOUT,
84-
);
62+
test('Should include local variables with ESM', done => {
63+
createRunner(__dirname, 'local-variables-caught.mjs')
64+
.ignore('session')
65+
.expect({
66+
event: event => {
67+
assertSentryEvent(event, EXPECTED_LOCAL_VARIABLES_EVENT);
68+
},
69+
})
70+
.start(done);
71+
});
8572

8673
test('Includes local variables for caught exceptions when enabled', done => {
8774
createRunner(__dirname, 'local-variables-caught.js')

‎dev-packages/node-integration-tests/utils/runner.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@ export function createRunner(...paths: string[]) {
9393
}
9494
}
9595

96-
function checkDone(): void {
96+
/** Called after each expect callback to check if we're complete */
97+
function expectCallbackCalled(): void {
9798
envelopeCount++;
9899
if (envelopeCount === expectedEnvelopeCount) {
99100
child.kill();
@@ -105,12 +106,14 @@ export function createRunner(...paths: string[]) {
105106
// Lines can have leading '[something] [{' which we need to remove
106107
const cleanedLine = line.replace(/^.*?] \[{"/, '[{"');
107108

109+
// See if we have a port message
108110
if (cleanedLine.startsWith('{"port":')) {
109111
const { port } = JSON.parse(cleanedLine) as { port: number };
110112
serverPort = port;
111113
return;
112114
}
113115

116+
// Skip any lines that don't start with envelope JSON
114117
if (!cleanedLine.startsWith('[{')) {
115118
return;
116119
}
@@ -119,10 +122,6 @@ export function createRunner(...paths: string[]) {
119122
try {
120123
envelope = JSON.parse(cleanedLine) as Envelope;
121124
} catch (_) {
122-
//
123-
}
124-
125-
if (!envelope) {
126125
return;
127126
}
128127

@@ -135,7 +134,7 @@ export function createRunner(...paths: string[]) {
135134

136135
const expected = expectedEnvelopes.shift();
137136

138-
// Catch any error or failed assertions and pass them to done
137+
// Catch any error or failed assertions and pass them to done to end the test quickly
139138
try {
140139
if (!expected) {
141140
throw new Error(`No more expected envelope items but we received a '${envelopeItemType}' item`);
@@ -149,17 +148,17 @@ export function createRunner(...paths: string[]) {
149148

150149
if ('event' in expected) {
151150
expected.event(item[1] as Event);
152-
checkDone();
151+
expectCallbackCalled();
153152
}
154153

155154
if ('transaction' in expected) {
156155
expected.transaction(item[1] as Event);
157-
checkDone();
156+
expectCallbackCalled();
158157
}
159158

160159
if ('session' in expected) {
161160
expected.session(item[1] as SerializedSession);
162-
checkDone();
161+
expectCallbackCalled();
163162
}
164163
} catch (e) {
165164
done?.(e);
@@ -168,16 +167,14 @@ export function createRunner(...paths: string[]) {
168167
}
169168

170169
let buffer = Buffer.alloc(0);
171-
172170
child.stdout.on('data', (data: Buffer) => {
171+
// This is horribly memory inefficient but it's only for tests
173172
buffer = Buffer.concat([buffer, data]);
174173

175174
let splitIndex = -1;
176175
while ((splitIndex = buffer.indexOf(0xa)) >= 0) {
177176
const line = buffer.subarray(0, splitIndex).toString();
178-
179177
buffer = Buffer.from(buffer.subarray(splitIndex + 1));
180-
181178
tryParseLine(line);
182179
}
183180
});

0 commit comments

Comments
 (0)