Let a listener's teardown run last and answer the sends the tests left hanging
This commit is contained in:
@@ -155,10 +155,14 @@ exactly 140.
|
||||
- `message_id` values the library generates are UUID v7.
|
||||
- A test that needs a dummy peer must `resume()` its sockets. An unread socket never processes the
|
||||
peer's FIN, so `server.close()` hangs forever — that is a test bug, not a library one.
|
||||
- Everything a test opens is handed to `test/teardown.ts` as it is opened, never closed on the test's
|
||||
- Everything a test opens gets its teardown registered as it is opened, never closed on the test's
|
||||
last line: an assertion that throws skips that line, and the listener it leaves behind keeps
|
||||
`node --test` alive until CI's ten-minute cap. The teardown aborts rather than drains, so a test
|
||||
that fails holding the send window still ends.
|
||||
`node --test` alive until CI's ten-minute cap. `test/teardown.ts` covers a session, a server and a
|
||||
listener; anything else takes a bare `t.after`. Its close aborts rather than drains, so a test that
|
||||
fails holding the send window still ends.
|
||||
- `t.after` hooks run in registration order, so registering at creation tears the outermost resource
|
||||
down first — the wrong way round for a listener, which blocks until every connection on it is gone.
|
||||
A listener's own hook goes last, after the hooks that close what is connected to it.
|
||||
- `assert.equal` from `node:assert/strict` narrows its first argument, so a following `?.` on the
|
||||
same value is flagged as unnecessary. Assert once with `assert.ok(x)` and use plain access after.
|
||||
|
||||
|
||||
@@ -227,7 +227,6 @@ describe('a live session against the reference implementation', () => {
|
||||
});
|
||||
});
|
||||
|
||||
t.after(() => new Promise<void>(resolve => { refServer.close(() => { resolve(); }); }));
|
||||
await new Promise<void>(resolve => { refServer.listen(0, () => { resolve(); }); });
|
||||
|
||||
const port = refServer.address()?.port ?? 0;
|
||||
@@ -236,6 +235,7 @@ describe('a live session against the reference implementation', () => {
|
||||
assert.equal(err, undefined);
|
||||
assert.ok(session);
|
||||
closeAfter(t, session);
|
||||
t.after(() => new Promise<void>(resolve => { refServer.close(() => { resolve(); }); }));
|
||||
|
||||
const sent = await session.sendSms({
|
||||
from: 'MyBrand',
|
||||
|
||||
+2
-2
@@ -173,6 +173,7 @@ describe('README: Server', () => {
|
||||
|
||||
assert.equal(clientErr, undefined);
|
||||
assert.ok(session);
|
||||
closeAfter(t, session);
|
||||
|
||||
await session.sendSms({ from: '46701113311', message: 'Hello world', to: '46709771337' });
|
||||
await session.unbind();
|
||||
@@ -212,6 +213,7 @@ describe('README: Server', () => {
|
||||
|
||||
assert.equal(clientErr, undefined);
|
||||
assert.ok(session);
|
||||
closeAfter(t, session);
|
||||
|
||||
const reported = once<Dlr>(resolve => { session.on('dlr', resolve); });
|
||||
const sent = await session.sendSms({
|
||||
@@ -223,8 +225,6 @@ describe('README: Server', () => {
|
||||
|
||||
assert.equal(sent.err, undefined);
|
||||
assert.equal((await reported).statusMsg, 'DELIVERED');
|
||||
|
||||
await session.unbind();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -16,7 +16,7 @@ import { Reassembler, decodeSegments } from '../src/reassembly.ts';
|
||||
import { Session } from '../src/session.ts';
|
||||
import { DlrMerger } from '../src/dlr-merger.ts';
|
||||
import { client } from '../src/client.ts';
|
||||
import { closeAfter, endListenerAfter } from './teardown.ts';
|
||||
import { closeAfter, closeListenerAfter } from './teardown.ts';
|
||||
import { consts } from '../src/defs/constants.ts';
|
||||
import { errors } from '../src/defs/errors.ts';
|
||||
import { objToPdu } from '../src/pdu.ts';
|
||||
@@ -575,7 +575,7 @@ describe('AbortSignal on a send', () => {
|
||||
});
|
||||
|
||||
await new Promise<void>(resolve => silent.listen(0, resolve));
|
||||
endListenerAfter(t, silent, accepted);
|
||||
closeListenerAfter(t, silent, accepted);
|
||||
|
||||
const address = silent.address();
|
||||
const port = typeof address === 'object' && address !== null ? address.port : 0;
|
||||
@@ -711,7 +711,7 @@ describe('graceful shutdown', () => {
|
||||
});
|
||||
|
||||
test('stops accepting the moment close() is called, not when the drain ends', async t => {
|
||||
const smpp = await startServer(t, { shutdownTimeout: 30_000 });
|
||||
const smpp = await startServer(t, { shutdownTimeout: 500 });
|
||||
const arrived = once<Session>(resolve => { smpp.on('session', resolve); });
|
||||
const silent = net.connect({ port: smpp.port });
|
||||
|
||||
@@ -758,7 +758,8 @@ describe('graceful shutdown', () => {
|
||||
});
|
||||
|
||||
test('does not report a reconnect on a session closed while it was coming back up', async t => {
|
||||
const listener = net.createServer(sock => { sock.resume(); });
|
||||
const accepted: net.Socket[] = [];
|
||||
const listener = net.createServer(sock => { accepted.push(sock); sock.resume(); });
|
||||
|
||||
await new Promise<void>(resolve => { listener.listen(0, resolve); });
|
||||
|
||||
@@ -793,7 +794,8 @@ describe('graceful shutdown', () => {
|
||||
const reported: string[] = [];
|
||||
|
||||
closeAfter(t, session);
|
||||
endListenerAfter(t, listener, opened);
|
||||
t.after(() => { for (const sock of opened) sock.destroy(); });
|
||||
closeListenerAfter(t, listener, accepted);
|
||||
session.on('reconnected', () => reported.push('reconnected'));
|
||||
first.sock.destroy();
|
||||
|
||||
|
||||
+14
-13
@@ -12,7 +12,7 @@ import { PduFramer } from '../src/pdu-framer.ts';
|
||||
import { ReconnectLoop } from '../src/reconnect-loop.ts';
|
||||
import { Session, bindCommands } from '../src/session.ts';
|
||||
import { client } from '../src/client.ts';
|
||||
import { closeAfter, endListenerAfter } from './teardown.ts';
|
||||
import { closeAfter, closeListenerAfter } from './teardown.ts';
|
||||
import { consts } from '../src/defs/constants.ts';
|
||||
import { isCommand, objToPdu, pduReturn, pduToObj } from '../src/pdu.ts';
|
||||
import { paramText } from '../src/defs/types.ts';
|
||||
@@ -316,9 +316,6 @@ describe('bind', () => {
|
||||
assert.ok(byDefault);
|
||||
assert.ok(asFive);
|
||||
assert.deepEqual(declared, [0x34, 0x50]);
|
||||
|
||||
await byDefault.unbind();
|
||||
await asFive.unbind();
|
||||
});
|
||||
|
||||
test('tells a 3.4 peer the version it supports in the bind response', async t => {
|
||||
@@ -472,12 +469,13 @@ describe('bind direction', () => {
|
||||
assert.ok(session);
|
||||
|
||||
const [sms] = await Promise.all([
|
||||
incoming,
|
||||
incoming.then(async received => {
|
||||
await received.sendResp();
|
||||
|
||||
return received;
|
||||
}),
|
||||
session.sendSms({ dlr: true, from: '46701113311', message: 'one way', to: '46709771337' }),
|
||||
]);
|
||||
|
||||
await sms.sendResp();
|
||||
|
||||
const report = await sms.sendDlr();
|
||||
|
||||
assert.ok(report.err instanceof Error);
|
||||
@@ -606,7 +604,11 @@ describe('sending', () => {
|
||||
assert.ok(session);
|
||||
|
||||
const [sms] = await Promise.all([
|
||||
incoming,
|
||||
incoming.then(async received => {
|
||||
await received.sendResp();
|
||||
|
||||
return received;
|
||||
}),
|
||||
session.sendSms({
|
||||
destinationAddrNpi: consts.NPI.ISDN,
|
||||
destinationAddrTon: consts.TON.NATIONAL,
|
||||
@@ -929,7 +931,7 @@ describe('robustness', () => {
|
||||
const silent = net.createServer(sock => { accepted.push(sock); sock.resume(); });
|
||||
|
||||
await new Promise<void>(resolve => silent.listen(0, resolve));
|
||||
endListenerAfter(t, silent, accepted);
|
||||
closeListenerAfter(t, silent, accepted);
|
||||
|
||||
const address = silent.address();
|
||||
const port = typeof address === 'object' && address !== null ? address.port : 0;
|
||||
@@ -1269,13 +1271,12 @@ describe('application hooks that throw or reject', () => {
|
||||
|
||||
smpp.on('session', () => Promise.reject(new Error('session listener rejected')));
|
||||
|
||||
const { session } = await connect(t, smpp);
|
||||
await connect(t, smpp);
|
||||
|
||||
const reported = await raceWithin(500, failed);
|
||||
|
||||
assert.ok(reported instanceof Error, 'a rejecting session listener should reach the server');
|
||||
assert.equal(reported.message, 'session listener rejected');
|
||||
|
||||
await session?.close();
|
||||
});
|
||||
|
||||
test('closes even when an application close listener throws', async t => {
|
||||
|
||||
+10
-4
@@ -9,13 +9,19 @@ export function closeAfter(t: TestContext, closable: Closable): void {
|
||||
t.after(() => closable.close({ signal: AbortSignal.abort() }));
|
||||
}
|
||||
|
||||
/** A listener with a socket still on it never closes, so the sockets go first. */
|
||||
export function endListenerAfter(t: TestContext, listener: Server, sockets: Socket[]): void {
|
||||
/**
|
||||
* Stops a listener, given every socket it accepted — one still open and it never closes. The wait is
|
||||
* bounded because a caller that misses one must not hang the net itself.
|
||||
*/
|
||||
export function closeListenerAfter(t: TestContext, listener: Server, accepted: Socket[]): void {
|
||||
t.after(async () => {
|
||||
for (const sock of sockets) {
|
||||
for (const sock of accepted) {
|
||||
sock.destroy();
|
||||
}
|
||||
|
||||
await new Promise<void>(resolve => { listener.close(() => { resolve(); }); });
|
||||
await Promise.race([
|
||||
new Promise<void>(resolve => { listener.close(() => { resolve(); }); }),
|
||||
new Promise<void>(resolve => { setTimeout(resolve, 1000).unref(); }),
|
||||
]);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -57,7 +57,6 @@ Rules the API follows:
|
||||
| Merged multipart DLRs including across a reconnect, reassembly bounds, per-send abort, the segment cap | `test/session-extras.test.ts` |
|
||||
| A draining `close()` and `unbind()`, bounded by `shutdownTimeout` or an abort | `test/session-extras.test.ts` |
|
||||
| Every runnable README example | `test/readme.test.ts` |
|
||||
| Teardown that survives a failing assertion, so a broken test reports rather than hangs | `test/teardown.ts` |
|
||||
| Receipt-versus-message classification by `esm_class` | `test/dlr.test.ts`, `test/session.test.ts` |
|
||||
| A listener that throws, or rejects, reaching `sessionError`/`serverError` rather than the process | `test/session.test.ts`, `test/error-from.test.ts` |
|
||||
| Cross-checked against node-smpp both ways and over a live session | `test/interop.test.ts` |
|
||||
|
||||
Reference in New Issue
Block a user