Skip to content
This repository has been archived by the owner on Nov 15, 2021. It is now read-only.

Commit

Permalink
Fixes to tests for iojs/node 0.12
Browse files Browse the repository at this point in the history 
Move to built-in agent instead of agentkeepalive.

Tests now handle the big change in node.js/cluster - when no workers are available, connection requests are outright refused. In node 0.10.36 and previous, requests would hang around until a worker was able to service them. The tests leveraged this heavily, making requests immediately after causing an error - on node 10.36 this would result in a delay and a response from the next worker. Now it results in an ECONNREFUSED.
  • Loading branch information
@scottnonnenberg
scottnonnenberg committed Mar 19, 2015
1 parent 5201573 commit 9991818
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 70 deletions.
1 change: 0 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
"thehelp-log-shim": "1.x"
},
"devDependencies": {
"agentkeepalive": "0.2.3",
"express": "4.12.3",
"grunt": "0.4.5",
"morgan": "1.5.2",
Expand Down
2 changes: 1 addition & 1 deletion test/integration/server/test_development_scenario.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ describe('development scenarios: in-process testing', function() {

it('does not shut down gracefully', function() {
expect(child.result).to.contain(' logger.info(result.toString());');
expect(child.result).to.contain(' ^');
expect(child.result).to.contain(' ^');

expect(child.result).not.to.match(/gracefully shutting down/);
expect(child.result).not.to.match(/pre-exit/);
Expand Down
81 changes: 39 additions & 42 deletions test/integration/server/test_end_to_end.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,26 @@
'use strict';

var path = require('path');
var http = require('http');

var core = require('thehelp-core');
var supertest = require('supertest');
var expect = require('thehelp-test').expect;
var util = require('./util');
var Pool = require('agentkeepalive');
var serverUtil = require('../../../src/server/util');

var logShim = require('thehelp-log-shim');
var logger = logShim('end-to-end:test');

var WORKER_STARTUP = 750; // milliseconds


describe('end-to-end', function() {
var agent, child, pool;

before(function(done) {
// https://github.com/node-modules/agentkeepalive#new-agentoptions
pool = new Pool({
keepAliveMsecs: 10000
pool = new http.Agent({
keepAlive: true
});

agent = supertest.agent('http://localhost:3000');
Expand Down Expand Up @@ -62,6 +64,17 @@ describe('end-to-end', function() {
.expect(500, done);
});

it('server does not accept a new connection after crash', function(done) {
this.timeout(5000);

agent
.get('/')
.expect(200, function(err) {
expect(err).to.have.property('code', 'ECONNREFUSED');
setTimeout(done, WORKER_STARTUP);
});
});

it('starts up another node', function(done) {
this.timeout(5000);

Expand All @@ -79,15 +92,18 @@ describe('end-to-end', function() {
done = serverUtil.once(done);
var delayComplete = false;

var second = new Pool({
keepAliveMsecs: 10000
var secondPool = new http.Agent({
keepAlive: true
});
var thirdPool = new http.Agent({
keepAlive: true
});

// long-running task still gets connection:close, even though error comes after
// relies on GracefulExpress._closeConnAfterResponses
agent
.get('/longDelay')
.agent(second)
.agent(secondPool)
.expect('X-Worker', '2')
.expect('Connection', 'close') // relies on patchResMethods = true
.expect(200, function(err) {
Expand All @@ -96,13 +112,17 @@ describe('end-to-end', function() {
logger.error(core.breadcrumbs.toString(err));
return done(err);
}

expect(delayComplete).to.equal(true);

setTimeout(done, WORKER_STARTUP);
});

// long-running task still gets connection:close, even though error comes after
// relies on GracefulExpress._closeConnAfterResponses
agent
.get('/delay')
.agent(second)
.agent(secondPool)
.expect('X-Worker', '2')
.expect('Connection', 'close') // relies on patchResMethods = true
.expect(200, function(err) {
Expand All @@ -114,67 +134,44 @@ describe('end-to-end', function() {

delayComplete = true;

// we use the same pool as
// we use the same pool as above
setImmediate(function() {
agent
.get('/')
.agent(second)
.agent(secondPool)
.expect('X-Worker', '3')
.expect('Connection', 'keep-alive')
.expect(200, function(err) {
if (err) {
err.message += ' - / request, on second pool';
logger.error(core.breadcrumbs.toString(err));
return done(err);
}
expect(err).to.have.property('code', 'ECONNREFUSED');
});
});
});

agent
.get('/error')
.agent(thirdPool)
.expect('Connection', 'close')
.expect('X-Worker', '2')
.expect(500, function(err) {
if (err) {
err.message += ' - /error request';
logger.error(core.breadcrumbs.toString(err));
return done(err);
}

expect(delayComplete).to.equal(false);

// this request sneaks in on an idle keepalive connection
// we don't want this request to sneak in on an idle keepalive connection
// this relies on GracefulExpress._closeInactiveSockets
agent
.get('/')
.agent(pool)
.expect('X-Worker', '3')
.expect('Connection', 'keep-alive')
.expect(200, function(err) {
if (err) {
err.message += ' - / keepalive request to worker 3';
logger.error(core.breadcrumbs.toString(err));
return done(err);
}

expect(delayComplete).to.equal(true);
expect(err).to.have.property('code', 'ECONNREFUSED');
});

// this is a new client, should definitely hit the new worker
// this is a new socket, request should definitely fail
agent
.get('/')
.expect('X-Worker', '3')
.expect(200, function(err) {
if (err) {
err.message += ' - / new request on worker 3';
logger.error(core.breadcrumbs.toString(err));
return done(err);
}

expect(delayComplete).to.equal(true);

done();
expect(err).to.have.property('code', 'ECONNREFUSED');
});
});
});
Expand All @@ -196,7 +193,7 @@ describe('end-to-end', function() {
return done(err);
}

done();
setTimeout(done, WORKER_STARTUP);
});

agent
Expand Down Expand Up @@ -229,7 +226,7 @@ describe('end-to-end', function() {
return done(err);
}

done();
setTimeout(done, WORKER_STARTUP);
});

agent
Expand All @@ -250,7 +247,7 @@ describe('end-to-end', function() {

done = serverUtil.once(done);

// results in a keepalive because headers are writen before the error happens
// results in a keepalive because headers are written before the error happens
// then we don't make another request on the socket, so it sticks around
agent
.get('/writeHeadAndDelay')
Expand Down
10 changes: 9 additions & 1 deletion test/integration/server/test_logging.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ var supertest = require('supertest');
var expect = require('thehelp-test').expect;
var util = require('./util');

var WORKER_STARTUP = 750; // milliseconds

var winston;

try {
Expand Down Expand Up @@ -86,7 +88,13 @@ describe('winston creates expected log files', function() {
.expect('Content-Type', /text\/plain/)
.expect('X-Worker', '1')
.expect(/^error\!/)
.expect(500, done);
.expect(500, function(err) {
if (err) {
throw err;
}

setTimeout(done, WORKER_STARTUP);
});
});

it('starts up another node', function(done) {
Expand Down
34 changes: 16 additions & 18 deletions test/integration/server/test_no_close_sockets.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
'use strict';

var path = require('path');
var http = require('http');

var core = require('thehelp-core');
var supertest = require('supertest');
var expect = require('thehelp-test').expect;
var util = require('./util');
var Pool = require('agentkeepalive');
var serverUtil = require('../../../src/server/util');

var logShim = require('thehelp-log-shim');
Expand All @@ -18,9 +18,8 @@ describe('keepalive sockets not closed', function() {
var agent, child, pool;

before(function(done) {
// https://github.com/node-modules/agentkeepalive#new-agentoptions
pool = new Pool({
keepAliveMsecs: 10000
pool = new http.Agent({
keepAlive: true
});

agent = supertest.agent('http://localhost:3000');
Expand All @@ -43,9 +42,6 @@ describe('keepalive sockets not closed', function() {
it('socket not closed, so keepalive gets 503', function(done) {
this.timeout(5000);

done = serverUtil.once(done);
var delayComplete = false;

// long-running task still gets connection:close, even though error comes after
agent
.get('/delay')
Expand All @@ -55,10 +51,9 @@ describe('keepalive sockets not closed', function() {
if (err) {
err.message += ' - /delay request';
logger.error(core.breadcrumbs.toString(err));
return done(err);
}

delayComplete = true;
done();
});

agent
Expand All @@ -69,11 +64,9 @@ describe('keepalive sockets not closed', function() {
if (err) {
err.message += ' - /error request';
logger.error(core.breadcrumbs.toString(err));
return done(err);
return;
}

expect(delayComplete).to.equal(false);

// this request sneaks in on a keepalive connection
agent
.get('/')
Expand All @@ -85,17 +78,23 @@ describe('keepalive sockets not closed', function() {
if (err) {
err.message += ' - / keepalive request to worker 3';
logger.error(core.breadcrumbs.toString(err));
return done(err);
}

done();
});
});
});

it('second worker starts up, starts keepalive connection', function(done) {
it('server does not accept a new connection after crash', function(done) {
this.timeout(5000);

agent
.get('/')
.expect(200, function(err) {
expect(err).to.have.property('code', 'ECONNREFUSED');
setTimeout(done, 3000);
});
});

it('second worker starts up, starts keepalive connection', function(done) {
agent
.get('/')
.agent(pool)
Expand Down Expand Up @@ -130,7 +129,7 @@ describe('keepalive sockets not closed', function() {
if (err) {
err.message += ' - /delay request';
logger.error(core.breadcrumbs.toString(err));
return done(err);
done(err);
}

done();
Expand All @@ -144,7 +143,6 @@ describe('keepalive sockets not closed', function() {
if (err) {
err.message += ' - /error request';
logger.error(core.breadcrumbs.toString(err));
return done(err);
}
});
});
Expand Down
Loading

0 comments on commit 9991818

Please sign in to comment.