Skip to content

Commit 328ed00

Browse files
committed
Simplify process.send sanity checks.
99.9% of the time process.send() is going to work. We don't need to guard it when it's already inside of a try block. Just guard the retry send. Also reduces the amount of excessive mocking going on in the tests by using jest more instead of creating our own mocks. Signed-off-by: Jason Marshall <jdmarshall@users.noreply.github.com>
1 parent 722b90f commit 328ed00

1 file changed

Lines changed: 19 additions & 23 deletions

File tree

test/clusterTest.js

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -167,19 +167,24 @@ describe.each([
167167
});
168168

169169
describe('worker message handling', () => {
170-
it('does not send metrics after the IPC channel disconnects', async () => {
170+
beforeEach(() => {
171171
jest.resetModules();
172+
});
173+
174+
it('does not send metrics after the IPC channel disconnects', async () => {
172175
jest.doMock('cluster', () => {
173176
return { isPrimary: false };
174177
});
175178

176-
const messageListeners = new Set(process.listeners('message'));
177179
const connectedDescriptor = Object.getOwnPropertyDescriptor(
178180
process,
179181
'connected',
180182
);
181-
const sendDescriptor = Object.getOwnPropertyDescriptor(process, 'send');
182-
const send = jest.fn();
183+
184+
const AggregatorRegistry = require('../lib/cluster');
185+
new AggregatorRegistry();
186+
187+
const send = jest.spyOn(process, 'send');
183188
let listener;
184189

185190
try {
@@ -188,38 +193,29 @@ describe('worker message handling', () => {
188193
value: true,
189194
writable: true,
190195
});
191-
Object.defineProperty(process, 'send', {
192-
configurable: true,
193-
value: send,
194-
});
195196

196-
const AggregatorRegistry = require('../lib/cluster');
197-
new AggregatorRegistry();
198-
199-
listener = process
200-
.listeners('message')
201-
.find(candidate => !messageListeners.has(candidate));
197+
listener = process.listeners('message').at(-1);
202198
expect(listener).toBeDefined();
203199

204-
listener({ type: GET_METRICS_REQ, requestId: 1 });
200+
send.mockImplementationOnce(() => {
201+
throw new Error('disconnected');
202+
});
203+
205204
process.connected = false;
205+
206+
listener({ type: GET_METRICS_REQ, requestId: 1 });
206207
await new Promise(resolve => setImmediate(resolve));
207208

208-
expect(send).toHaveBeenCalledTimes(1); // Announcement
209+
expect(send).not.toHaveBeenCalled();
209210
} finally {
210-
if (listener) process.removeListener('message', listener);
211+
process.removeListener('message', listener);
211212
if (connectedDescriptor) {
212213
Object.defineProperty(process, 'connected', connectedDescriptor);
213214
} else {
214215
delete process.connected;
215216
}
216-
if (sendDescriptor) {
217-
Object.defineProperty(process, 'send', sendDescriptor);
218-
} else {
219-
delete process.send;
220-
}
221-
jest.dontMock('cluster');
222217
jest.resetModules();
218+
jest.clearAllMocks();
223219
}
224220
});
225221
});

0 commit comments

Comments
 (0)