Skip to content

Commit 26c99b8

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 dd602ce commit 26c99b8

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
@@ -168,19 +168,24 @@ describe.each([
168168
});
169169

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

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

186191
try {
@@ -189,38 +194,29 @@ describe('worker message handling', () => {
189194
value: true,
190195
writable: true,
191196
});
192-
Object.defineProperty(process, 'send', {
193-
configurable: true,
194-
value: send,
195-
});
196197

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

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

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

0 commit comments

Comments
 (0)