Skip to content

Commit

Permalink
server: call server.address after listening event
Browse files Browse the repository at this point in the history
As per the Node.js API docs (https://nodejs.org/api/http.html)
the `server.address` function should only be called *after*
the "listening" event is triggered.
  • Loading branch information
Kevin Delisle committed Jun 15, 2017
1 parent f5310e9 commit 37614f0
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 29 deletions.
22 changes: 15 additions & 7 deletions packages/core/src/server.ts
Expand Up @@ -20,19 +20,27 @@ export class Server extends Context {
this.state = ServerState.cold;
}

async start() {
async start() : Promise<void> {
this.state = ServerState.starting;

const server = createServer((req, res) => this._handleRequest(req, res));
server.listen(this.config.port);

// FIXME(bajtos) The updated port number should be part of "status" object,
// we shouldn't be changing original config IMO.
// Consider exposing full base URL including http/https scheme prefix
this.config.port = server.address().port;

await new Promise(resolve => server.once('listening', resolve));
this.state = ServerState.listening;
return new Promise<void>((resolve, reject) => {
server.once('listening', () => {
// FIXME(bajtos) The updated port number should be part of "status"
// object, we shouldn't be changing original config IMO.
// Consider exposing full base URL including http/https scheme prefix
try {
this.config.port = server.address().port;
this.state = ServerState.listening;
resolve();
} catch (err) {
reject(err);
}
});
});
}

protected _handleRequest(req: ServerRequest, res: ServerResponse) {
Expand Down
46 changes: 31 additions & 15 deletions packages/core/test/acceptance/routing/routing.acceptance.ts
Expand Up @@ -61,12 +61,11 @@ describe('Routing', () => {
}
givenControllerInApp(app, EchoController);

return (
whenIMakeRequestTo(app)
.get('/echo?msg=hello%20world')
// Then I get the result `hello world` from the `Method`
.expect('hello world')
);
return whenIMakeRequestTo(app).then(client => {
return client.get('/echo?msg=hello%20world')
// Then I get the result `hello world` from the `Method`
.expect('hello world');
});
});

it('injects controller constructor arguments', () => {
Expand Down Expand Up @@ -94,7 +93,10 @@ describe('Routing', () => {
}
givenControllerInApp(app, InfoController);

return whenIMakeRequestTo(app).get('/name').expect('TestApp');
return whenIMakeRequestTo(app).then(client => {
return client.get('/name')
.expect('TestApp');
});
});

it('creates a new child context for each request', async () => {
Expand Down Expand Up @@ -127,10 +129,16 @@ describe('Routing', () => {
// Rebind "flag" to "modified". Since we are modifying
// the per-request child context, the change should
// be discarded after the request is done.
await whenIMakeRequestTo(app).put('/flag');
await whenIMakeRequestTo(app).then(client => {
return client.put('/flag');
// Get the value "flag" is bound to.
// This should return the original value.
await whenIMakeRequestTo(app).get('/flag').expect('original');

});
await whenIMakeRequestTo(app).then(client => {
return client.get('/flag')
.expect('original');
});
});

it('binds request and response objects', () => {
Expand All @@ -154,7 +162,10 @@ describe('Routing', () => {
}
givenControllerInApp(app, StatusController);

return whenIMakeRequestTo(app).get('/status').expect(202, 'GET');
return whenIMakeRequestTo(app).then(client => {
return client.get('/status')
.expect(202, 'GET');
});
});

it('binds controller constructor object and operation', () => {
Expand Down Expand Up @@ -182,9 +193,13 @@ describe('Routing', () => {
}
givenControllerInApp(app, GetCurrentController);

return whenIMakeRequestTo(app).get('/name').expect({
ctor: 'GetCurrentController',
operation: 'getControllerName',
return whenIMakeRequestTo(app).then(client => {
return client.get('/name')
.expect({
ctor: 'GetCurrentController',
operation: 'getControllerName',
});

});
});

Expand All @@ -201,8 +216,9 @@ describe('Routing', () => {
app.controller(controller);
}

function whenIMakeRequestTo(app: Application): Client {
function whenIMakeRequestTo(app: Application): Promise<Client> {
const server = new Server(app, {port: 0});
return createClientForServer(server);
const result = createClientForServer(server);
return result;
}
});
17 changes: 12 additions & 5 deletions packages/core/test/acceptance/sequence/sequence.acceptance.ts
Expand Up @@ -32,7 +32,10 @@ describe('Sequence - ', () => {
beforeEach(givenAppWithController);

it('default sequence', () => {
return whenIMakeRequestTo().get('/name').expect('SequenceApp');
return whenIMakeRequestTo(app).then(client => {
return client.get('/name')
.expect('SequenceApp');
});
});

it('user defined sequence', () => {
Expand All @@ -59,7 +62,10 @@ describe('Sequence - ', () => {
// bind user defined sequence
app.bind('sequence').toClass(MySequence);

return whenIMakeRequestTo().get('/name').expect('MySequence SequenceApp');
return whenIMakeRequestTo(app).then(client => {
return client.get('/name')
.expect('MySequence SequenceApp');
});
});

function givenAppWithController() {
Expand Down Expand Up @@ -97,8 +103,9 @@ describe('Sequence - ', () => {
app.controller(controller);
}

function whenIMakeRequestTo(): Client {
const server = new Server(app, {port: 0});
return createClientForServer(server);
function whenIMakeRequestTo(application: Application): Promise<Client> {
const server = new Server(application, {port: 0});
const result = createClientForServer(server);
return result;
}
});
4 changes: 2 additions & 2 deletions packages/testlab/src/client.ts
Expand Up @@ -31,8 +31,8 @@ export interface Server {
start(): Promise<void>;
}

export function createClientForServer(server: Server): Client {
const dontWaitForListening = server.start();
export async function createClientForServer(server: Server): Promise<Client> {
await server.start();
const url = `http://127.0.0.1:${server.config.port}`;
// TODO(bajtos) Find a way how to stop the server after all tests are done
return supertest(url);
Expand Down

0 comments on commit 37614f0

Please sign in to comment.