Skip to content

Commit fb635fb

Browse files
committed
tests: simplify dummy socket timeout servers
The dummy socket servers used in functional tests for checking the connection timeout, in particular, for the multi-host scenario, have a somewhat complicated implementation. The resulting setup is overwhelming and seems to be causing some synchronicity issues in the test harness. These issues are, at times, observable when checking if the elapsed time whilst trying to connect to the server is, in fact, above or equal to the sum of all the reported connection timeouts. This patch simplifies the dummy server setup and inverts the timestamp-based checks to compare the elapsed time during each test to the timeout itself, instead of simply comparing raw timestamps, which are not easy to understand when the assertions fail. Change-Id: I96ed9a4ab06eca9545a6f1b2ce4a922c42892537
1 parent 4072365 commit fb635fb

File tree

3 files changed

+69
-65
lines changed

3 files changed

+69
-65
lines changed
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
* Copyright (c) 2022, Oracle and/or its affiliates.
3+
*
4+
* This program is free software; you can redistribute it and/or modify
5+
* it under the terms of the GNU General Public License, version 2.0, as
6+
* published by the Free Software Foundation.
7+
*
8+
* This program is also distributed with certain software (including
9+
* but not limited to OpenSSL) that is licensed under separate terms,
10+
* as designated in a particular file or component or in included license
11+
* documentation. The authors of MySQL hereby grant you an
12+
* additional permission to link the program and your derivative works
13+
* with the separately licensed software that they have included with
14+
* MySQL.
15+
*
16+
* Without limiting anything contained in the foregoing, this file,
17+
* which is part of MySQL Connector/Node.js, is also subject to the
18+
* Universal FOSS Exception, version 1.0, a copy of which can be found at
19+
* http://oss.oracle.com/licenses/universal-foss-exception.
20+
*
21+
* This program is distributed in the hope that it will be useful, but
22+
* WITHOUT ANY WARRANTY; without even the implied warranty of
23+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
24+
* See the GNU General Public License, version 2.0, for more details.
25+
*
26+
* You should have received a copy of the GNU General Public License
27+
* along with this program; if not, write to the Free Software Foundation, Inc.,
28+
* 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
29+
*/
30+
31+
'use strict';
32+
33+
module.exports = function (server) {
34+
return new Promise((resolve, reject) => {
35+
// If the server is not listening, there is nothing to do.
36+
if (!server.listening) {
37+
return resolve();
38+
}
39+
40+
// Make sure the server does not accept any new connections.
41+
server.close(err => {
42+
if (err) {
43+
return reject(err);
44+
}
45+
46+
return resolve();
47+
});
48+
49+
// The callback on "close" is only called after all
50+
// connections are closed, so we need to make sure that
51+
// happens.
52+
server.emit('close');
53+
});
54+
};

test/fixtures/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ module.exports = {
3838
createSchema: require('./createSchema'),
3939
createUser: require('./createUser'),
4040
deregisterService: require('./deregisterService'),
41+
destroyServerSocket: require('./destroyServerSocket'),
4142
dropSchema: require('./dropSchema'),
4243
dropUser: require('./dropUser'),
4344
getIPv4Address: require('./getIPv4Address'),

test/functional/default/connection/timeout.js

Lines changed: 14 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2021, Oracle and/or its affiliates.
2+
* Copyright (c) 2020, 2022, Oracle and/or its affiliates.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License, version 2.0, as
@@ -35,6 +35,7 @@
3535
const config = require('../../../config');
3636
const errors = require('../../../../lib/constants/errors');
3737
const expect = require('chai').expect;
38+
const fixtures = require('../../../fixtures');
3839
const mysqlx = require('../../../../');
3940
const net = require('net');
4041
const os = require('os');
@@ -93,25 +94,10 @@ describe('connecting to servers with a given timeout', () => {
9394

9495
beforeEach('create fake server', () => {
9596
server = net.createServer();
96-
97-
server.on('connection', socket => {
98-
server.on('close', () => socket.destroy());
99-
socket.pause();
100-
});
10197
});
10298

103-
afterEach('close fake server', done => {
104-
// If the server is not listening, there is nothing to do.
105-
if (!server.listening) {
106-
return done();
107-
}
108-
109-
// Make sure the server does not accept any new connections.
110-
server.close(done);
111-
// The callback on "close" is only called after all
112-
// connections are closed, so we need to make sure that
113-
// happens.
114-
server.emit('close');
99+
afterEach('close fake server', () => {
100+
return fixtures.destroyServerSocket(server);
115101
});
116102

117103
context('using a Unix socket', () => {
@@ -255,51 +241,10 @@ describe('connecting to servers with a given timeout', () => {
255241
beforeEach('create fake servers', () => {
256242
primary = net.createServer();
257243
secondary = net.createServer();
258-
259-
[primary, secondary].forEach(server => {
260-
server.on('connection', socket => {
261-
primary.on('close', () => socket.destroy());
262-
socket.pause();
263-
});
264-
});
265244
});
266245

267-
afterEach('close fake servers', done => {
268-
// If the servers are not listening, there is nothing to do.
269-
if (!secondary.listening && !primary.listening) {
270-
return done();
271-
}
272-
273-
// If only the primary is listening.
274-
if (!secondary.listening) {
275-
// We prevent it from accepting new connections.
276-
primary.close(done);
277-
// And close the existing ones.
278-
primary.emit('close');
279-
return;
280-
}
281-
282-
// By this point both the primary and secondary should be
283-
// listening.
284-
secondary.close(err => {
285-
if (!err) {
286-
return primary.close(done);
287-
}
288-
289-
// Even if there is an error, we should try to close
290-
// the primary as well.
291-
return primary.close(err => {
292-
if (err) {
293-
return done(err);
294-
}
295-
296-
return done(err);
297-
});
298-
});
299-
300-
[primary, secondary].forEach(server => {
301-
server.emit('close');
302-
});
246+
afterEach('close fake servers', () => {
247+
return Promise.all([fixtures.destroyServerSocket(primary), fixtures.destroyServerSocket(secondary)]);
303248
});
304249

305250
context('using a Unix socket', () => {
@@ -331,8 +276,9 @@ describe('connecting to servers with a given timeout', () => {
331276
return expect.fail();
332277
})
333278
.catch(err => {
279+
const elapsedTime = Date.now() - beforeTest;
334280
// The timeout check restarts for each endpoint.
335-
expect(Date.now()).to.be.at.least(beforeTest + connectTimeout * 2);
281+
expect(elapsedTime).to.be.at.least(connectTimeout * timeoutConfig.endpoints.length);
336282
return expect(err.message).to.equal(error);
337283
});
338284
});
@@ -354,8 +300,9 @@ describe('connecting to servers with a given timeout', () => {
354300
return expect.fail();
355301
})
356302
.catch(err => {
303+
const elapsedTime = Date.now() - beforeTest;
357304
// The timeout check restarts for each endpoint.
358-
expect(Date.now()).to.be.at.least(beforeTest + connectTimeout * 2);
305+
expect(elapsedTime).to.be.at.least(connectTimeout * timeoutConfig.endpoints.length);
359306
return expect(err.message).to.equal(error);
360307
});
361308
});
@@ -421,8 +368,9 @@ describe('connecting to servers with a given timeout', () => {
421368
return expect.fail();
422369
})
423370
.catch(err => {
371+
const elapsedTime = Date.now() - beforeTest;
424372
// The timeout check restarts for each endpoint.
425-
expect(Date.now()).to.be.at.least(beforeTest + connectTimeout * 2);
373+
expect(elapsedTime).to.be.at.least(connectTimeout * timeoutConfig.endpoints.length);
426374
return expect(err.message).to.equal(error);
427375
});
428376
});
@@ -440,8 +388,9 @@ describe('connecting to servers with a given timeout', () => {
440388
return expect.fail();
441389
})
442390
.catch(err => {
391+
const elapsedTime = Date.now() - beforeTest;
443392
// The timeout check restarts for each endpoint.
444-
expect(Date.now()).to.be.at.least(beforeTest + connectTimeout * 2);
393+
expect(elapsedTime).to.be.at.least(connectTimeout * timeoutConfig.endpoints.length);
445394
return expect(err.message).to.equal(error);
446395
});
447396
});

0 commit comments

Comments
 (0)