From 85a3f8a7b3087e7240c1d307ba6dd78c05883f83 Mon Sep 17 00:00:00 2001 From: Alessandro Pio Ardizio Date: Wed, 17 Feb 2021 10:54:49 +0100 Subject: [PATCH] Tests enhancement (#172) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit * Increase coverage and remove deprecated methods * Add utility test function instead of use setTimeout to wait workers scale down * Decrease a little bit the required nyc percentages * Add a TestUtils class. Signed-off-by: Jérôme Benoit * More sleep usage. Signed-off-by: Jérôme Benoit * Run npm format. Signed-off-by: Jérôme Benoit * Remove deprecated methods and update changelog, increase require coverage * Check coverage change Co-authored-by: Jérôme Benoit --- .nycrc.json | 7 +++ CHANGELOG.md | 6 +-- benchmarks/bench.js | 6 +-- package.json | 4 +- src/pools/abstract-pool.ts | 21 -------- tests/pools/cluster/dynamic.test.js | 49 ++++++------------- tests/pools/cluster/fixed.test.js | 24 ++------- tests/pools/thread/dynamic.test.js | 35 +++++-------- tests/pools/thread/fixed.test.js | 23 ++------- tests/test-utils.js | 21 ++++++++ tests/worker-files/cluster/echoWorker.js | 1 - .../cluster/longRunningWorkerHardBehavior.js | 0 .../cluster/longRunningWorkerSoftBehavior.js | 0 tests/worker-files/thread/echoWorker.js | 1 - .../thread/longRunningWorkerHardBehavior.js | 0 .../thread/longRunningWorkerSoftBehavior.js | 0 16 files changed, 71 insertions(+), 127 deletions(-) create mode 100644 .nycrc.json create mode 100644 tests/test-utils.js rename tests/{worker => worker-files}/cluster/longRunningWorkerHardBehavior.js (100%) rename tests/{worker => worker-files}/cluster/longRunningWorkerSoftBehavior.js (100%) rename tests/{worker => worker-files}/thread/longRunningWorkerHardBehavior.js (100%) rename tests/{worker => worker-files}/thread/longRunningWorkerSoftBehavior.js (100%) diff --git a/.nycrc.json b/.nycrc.json new file mode 100644 index 00000000..0d2fa013 --- /dev/null +++ b/.nycrc.json @@ -0,0 +1,7 @@ +{ + "check-coverage": true, + "lines": 90, + "statements": 90, + "functions": 89, + "branches": 85 +} diff --git a/CHANGELOG.md b/CHANGELOG.md index d23919ef..4915f5e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,10 +38,10 @@ const { DynamicThreadPool } = require('poolifier') For cluster and thread pools, you can now only send and receive serializable `JSON` data. _This is not a limitation by poolifier but NodeJS._ -#### Public properties renaming +### Public methods removed -- Thread Pool's `numWorkers` is now `numberOfWorkers` -- Thread Pool's `nextWorker` is now `nextWorkerIndex` +`numWorkers` method removed +`nextWorker` method removed #### Internal (protected) properties and methods renaming diff --git a/benchmarks/bench.js b/benchmarks/bench.js index f8d93381..2ad7f22c 100644 --- a/benchmarks/bench.js +++ b/benchmarks/bench.js @@ -11,13 +11,13 @@ const LIST_FORMATTER = new Intl.ListFormat('en-US', { type: 'conjunction' }) -// wait some seconds before start, my pools need to load threads !!! +// Wait some seconds before start, pools need to load threads !!! setTimeout(async () => { test() }, 3000) async function test () { - // add tests + // Add tests suite .add('Pioardi:Static:ThreadPool', async function () { await fixedThreadTest() @@ -31,7 +31,7 @@ async function test () { .add('Pioardi:Dynamic:ClusterPool', async function () { await dynamicClusterTest() }) - // add listeners + // Add listeners .on('cycle', function (event) { console.log(event.target.toString()) }) diff --git a/package.json b/package.json index 22e5a630..ceef04b1 100644 --- a/package.json +++ b/package.json @@ -13,8 +13,8 @@ "test:debug": "npm run build && mocha -r source-map-support/register --inspect --exit --timeout 20000 'tests/**/*.test.js'", "test:prod": "npm run build:prod && nyc mocha -r source-map-support/register --exit --timeout 20000 'tests/**/*.test.js'", "sonar": "sonar-scanner", - "coverage": "nyc report --reporter=lcov --check-coverage --lines 80", - "coverage:html": "nyc report --reporter=html --check-coverage --lines 80", + "coverage": "nyc report --reporter=lcov", + "coverage:html": "nyc report --reporter=html", "format": "prettier --loglevel silent --write .; prettierx --write .", "lint": "eslint .", "lint:fix": "eslint . --fix" diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 5ef2eed9..4a6ba72f 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -155,27 +155,6 @@ export abstract class AbstractPool< this.emitter = new PoolEmitter() } - /** - * Number of workers that this pool should manage. - * - * @returns Number of workers that this pool manages. - * @deprecated Only here for backward compatibility. - */ - // eslint-disable-next-line spellcheck/spell-checker - public get numWorkers (): number { - return this.numberOfWorkers - } - - /** - * Index for the next worker. - * - * @returns Index for the next worker. - * @deprecated Only here for backward compatibility. - */ - public get nextWorker (): number { - return this.nextWorkerIndex - } - /** * Perform the task specified in the constructor with the data parameter. * diff --git a/tests/pools/cluster/dynamic.test.js b/tests/pools/cluster/dynamic.test.js index 01b72072..fbeb77cf 100644 --- a/tests/pools/cluster/dynamic.test.js +++ b/tests/pools/cluster/dynamic.test.js @@ -1,5 +1,6 @@ const expect = require('expect') const { DynamicClusterPool } = require('../../../lib/index') +const TestUtils = require('../../test-utils') const min = 1 const max = 3 const pool = new DynamicClusterPool( @@ -20,7 +21,6 @@ describe('Dynamic cluster pool test suite ', () => { it('Verify that new workers are created when required, max size is not exceeded and that after a while new workers will die', async () => { const promises = [] - let closedWorkers = 0 let fullPool = 0 pool.emitter.on('FullPool', () => fullPool++) for (let i = 0; i < max * 2; i++) { @@ -28,14 +28,9 @@ describe('Dynamic cluster pool test suite ', () => { } expect(pool.workers.length).toBeLessThanOrEqual(max) expect(pool.workers.length).toBeGreaterThan(min) - pool.workers.forEach(w => { - w.on('exit', () => { - closedWorkers++ - }) - }) expect(fullPool > 1).toBeTruthy() - await new Promise(resolve => setTimeout(resolve, 5000)) - expect(closedWorkers).toBe(max - min) + const numberOfExitEvents = await TestUtils.waitExits(pool, max - min) + expect(numberOfExitEvents).toBe(max - min) }) it('Verify scale worker up and down is working', async () => { @@ -44,37 +39,21 @@ describe('Dynamic cluster pool test suite ', () => { pool.execute({ test: 'test' }) } expect(pool.workers.length).toBeGreaterThan(min) - await new Promise(resolve => setTimeout(resolve, 3000)) + await TestUtils.waitExits(pool, max - min) expect(pool.workers.length).toBe(min) for (let i = 0; i < max * 10; i++) { pool.execute({ test: 'test' }) } expect(pool.workers.length).toBeGreaterThan(min) - await new Promise(resolve => setTimeout(resolve, 3000)) + await TestUtils.waitExits(pool, max - min) expect(pool.workers.length).toBe(min) }) - it('Shutdown test', async () => { - let closedWorkers = 0 - pool.workers.forEach(w => { - w.on('exit', () => { - closedWorkers++ - }) - }) - pool.destroy() - await new Promise(resolve => setTimeout(resolve, 2000)) - expect(closedWorkers).toBe(min) - }) - it('Validations test', () => { - let error - try { - const pool1 = new DynamicClusterPool() - console.log(pool1) - } catch (e) { - error = e - } - expect(error).toBeTruthy() - expect(error.message).toBeTruthy() + it('Shutdown test', async () => { + const exitPromise = TestUtils.waitExits(pool, min) + await pool.destroy() + const res = await exitPromise + expect(res).toBe(min) }) it('Should work even without opts in input', async () => { @@ -91,14 +70,14 @@ describe('Dynamic cluster pool test suite ', () => { const longRunningPool = new DynamicClusterPool( min, max, - './tests/worker/cluster/longRunningWorkerHardBehavior.js' + './tests/worker-files/cluster/longRunningWorkerHardBehavior.js' ) expect(longRunningPool.workers.length).toBe(min) for (let i = 0; i < max * 10; i++) { longRunningPool.execute({ test: 'test' }) } expect(longRunningPool.workers.length).toBe(max) - await new Promise(resolve => setTimeout(resolve, 3000)) + await TestUtils.waitExits(longRunningPool, max - min) // Here we expect the workers to be at the max size since that the task is still running expect(longRunningPool.workers.length).toBe(min) }) @@ -107,14 +86,14 @@ describe('Dynamic cluster pool test suite ', () => { const longRunningPool = new DynamicClusterPool( min, max, - './tests/worker/cluster/longRunningWorkerSoftBehavior.js' + './tests/worker-files/cluster/longRunningWorkerSoftBehavior.js' ) expect(longRunningPool.workers.length).toBe(min) for (let i = 0; i < max * 10; i++) { longRunningPool.execute({ test: 'test' }) } expect(longRunningPool.workers.length).toBe(max) - await new Promise(resolve => setTimeout(resolve, 3000)) + await TestUtils.sleep(1500) // Here we expect the workers to be at the max size since that the task is still running expect(longRunningPool.workers.length).toBe(max) }) diff --git a/tests/pools/cluster/fixed.test.js b/tests/pools/cluster/fixed.test.js index f5300ecc..08476e09 100644 --- a/tests/pools/cluster/fixed.test.js +++ b/tests/pools/cluster/fixed.test.js @@ -1,5 +1,6 @@ const expect = require('expect') const { FixedClusterPool } = require('../../../lib/index') +const TestUtils = require('../../test-utils') const numberOfWorkers = 10 const maxTasks = 500 const pool = new FixedClusterPool( @@ -115,27 +116,10 @@ describe('Fixed cluster pool test suite ', () => { }) it('Shutdown test', async () => { - let closedWorkers = 0 - pool.workers.forEach(w => { - w.on('exit', () => { - closedWorkers++ - }) - }) + const exitPromise = TestUtils.waitExits(pool, numberOfWorkers) await pool.destroy() - await new Promise(resolve => setTimeout(resolve, 500)) - expect(closedWorkers).toBe(numberOfWorkers) - }) - - it('Validations test', () => { - let error - try { - const pool1 = new FixedClusterPool() - console.log(pool1) - } catch (e) { - error = e - } - expect(error).toBeTruthy() - expect(error.message).toBeTruthy() + const res = await exitPromise + expect(res).toBe(numberOfWorkers) }) it('Should work even without opts in input', async () => { diff --git a/tests/pools/thread/dynamic.test.js b/tests/pools/thread/dynamic.test.js index 4efb0f4b..66aeaa3a 100644 --- a/tests/pools/thread/dynamic.test.js +++ b/tests/pools/thread/dynamic.test.js @@ -1,5 +1,6 @@ const expect = require('expect') const { DynamicThreadPool } = require('../../../lib/index') +const TestUtils = require('../../test-utils') const min = 1 const max = 3 const pool = new DynamicThreadPool( @@ -20,21 +21,15 @@ describe('Dynamic thread pool test suite ', () => { it('Verify that new workers are created when required, max size is not exceeded and that after a while new workers will die', async () => { const promises = [] - let closedThreads = 0 let fullPool = 0 pool.emitter.on('FullPool', () => fullPool++) for (let i = 0; i < max * 2; i++) { promises.push(pool.execute({ test: 'test' })) } expect(pool.workers.length).toBe(max) - pool.workers.forEach(w => { - w.on('exit', () => { - closedThreads++ - }) - }) expect(fullPool > 1).toBeTruthy() - await new Promise(resolve => setTimeout(resolve, 2000)) - expect(closedThreads).toBe(max - min) + const res = await TestUtils.waitExits(pool, max - min) + expect(res).toBe(max - min) }) it('Verify scale thread up and down is working', async () => { @@ -43,13 +38,13 @@ describe('Dynamic thread pool test suite ', () => { pool.execute({ test: 'test' }) } expect(pool.workers.length).toBe(max) - await new Promise(resolve => setTimeout(resolve, 1000)) + await TestUtils.waitExits(pool, max - min) expect(pool.workers.length).toBe(min) for (let i = 0; i < max * 10; i++) { pool.execute({ test: 'test' }) } expect(pool.workers.length).toBe(max) - await new Promise(resolve => setTimeout(resolve, 1500)) + await TestUtils.waitExits(pool, max - min) expect(pool.workers.length).toBe(min) }) @@ -65,15 +60,11 @@ describe('Dynamic thread pool test suite ', () => { }) it('Validations test', () => { - let error - try { + expect(() => { const pool1 = new DynamicThreadPool() - console.log(pool1) - } catch (e) { - error = e - } - expect(error).toBeTruthy() - expect(error.message).toBeTruthy() + }).toThrowError( + new Error('Please specify a file with a worker implementation') + ) }) it('Should work even without opts in input', async () => { @@ -90,7 +81,7 @@ describe('Dynamic thread pool test suite ', () => { const longRunningPool = new DynamicThreadPool( min, max, - './tests/worker/thread/longRunningWorkerHardBehavior.js', + './tests/worker-files/thread/longRunningWorkerHardBehavior.js', { errorHandler: e => console.error(e), onlineHandler: () => console.log('worker is online') @@ -101,7 +92,7 @@ describe('Dynamic thread pool test suite ', () => { longRunningPool.execute({ test: 'test' }) } expect(longRunningPool.workers.length).toBe(max) - await new Promise(resolve => setTimeout(resolve, 1500)) + await TestUtils.waitExits(longRunningPool, max - min) expect(longRunningPool.workers.length).toBe(min) }) @@ -109,7 +100,7 @@ describe('Dynamic thread pool test suite ', () => { const longRunningPool = new DynamicThreadPool( min, max, - './tests/worker/thread/longRunningWorkerSoftBehavior.js', + './tests/worker-files/thread/longRunningWorkerSoftBehavior.js', { errorHandler: e => console.error(e), onlineHandler: () => console.log('worker is online') @@ -120,7 +111,7 @@ describe('Dynamic thread pool test suite ', () => { longRunningPool.execute({ test: 'test' }) } expect(longRunningPool.workers.length).toBe(max) - await new Promise(resolve => setTimeout(resolve, 1500)) + await TestUtils.sleep(1500) // Here we expect the workers to be at the max size since that the task is still running expect(longRunningPool.workers.length).toBe(max) }) diff --git a/tests/pools/thread/fixed.test.js b/tests/pools/thread/fixed.test.js index 755ccf4d..2a7d005b 100644 --- a/tests/pools/thread/fixed.test.js +++ b/tests/pools/thread/fixed.test.js @@ -1,5 +1,6 @@ const expect = require('expect') const { FixedThreadPool } = require('../../../lib/index') +const TestUtils = require('../../test-utils') const numberOfThreads = 10 const maxTasks = 400 const pool = new FixedThreadPool( @@ -93,26 +94,10 @@ describe('Fixed thread pool test suite ', () => { }) it('Shutdown test', async () => { - let closedThreads = 0 - pool.workers.forEach(w => { - w.on('exit', () => { - closedThreads++ - }) - }) + const exitPromise = TestUtils.waitExits(pool, numberOfThreads) await pool.destroy() - expect(closedThreads).toBe(numberOfThreads) - }) - - it('Validations test', () => { - let error - try { - const pool1 = new FixedThreadPool() - console.log(pool1) - } catch (e) { - error = e - } - expect(error).toBeTruthy() - expect(error.message).toBeTruthy() + const res = await exitPromise + expect(res).toBe(numberOfThreads) }) it('Should work even without opts in input', async () => { diff --git a/tests/test-utils.js b/tests/test-utils.js new file mode 100644 index 00000000..690f4b37 --- /dev/null +++ b/tests/test-utils.js @@ -0,0 +1,21 @@ +class TestUtils { + static async waitExits (pool, numberOfExitEventsToWait) { + let exitEvents = 0 + return new Promise(function (resolve, reject) { + pool.workers.forEach(w => { + w.on('exit', () => { + exitEvents++ + if (exitEvents === numberOfExitEventsToWait) { + resolve(exitEvents) + } + }) + }) + }) + } + + static async sleep (ms) { + return new Promise(resolve => setTimeout(resolve, ms)) + } +} + +module.exports = TestUtils diff --git a/tests/worker-files/cluster/echoWorker.js b/tests/worker-files/cluster/echoWorker.js index 054c4bb3..9bcae281 100644 --- a/tests/worker-files/cluster/echoWorker.js +++ b/tests/worker-files/cluster/echoWorker.js @@ -6,6 +6,5 @@ function echo (data) { } module.exports = new ClusterWorker(echo, { - maxInactiveTime: 500, killBehavior: KillBehaviors.HARD }) diff --git a/tests/worker/cluster/longRunningWorkerHardBehavior.js b/tests/worker-files/cluster/longRunningWorkerHardBehavior.js similarity index 100% rename from tests/worker/cluster/longRunningWorkerHardBehavior.js rename to tests/worker-files/cluster/longRunningWorkerHardBehavior.js diff --git a/tests/worker/cluster/longRunningWorkerSoftBehavior.js b/tests/worker-files/cluster/longRunningWorkerSoftBehavior.js similarity index 100% rename from tests/worker/cluster/longRunningWorkerSoftBehavior.js rename to tests/worker-files/cluster/longRunningWorkerSoftBehavior.js diff --git a/tests/worker-files/thread/echoWorker.js b/tests/worker-files/thread/echoWorker.js index 071428c5..cfe9427b 100644 --- a/tests/worker-files/thread/echoWorker.js +++ b/tests/worker-files/thread/echoWorker.js @@ -6,6 +6,5 @@ function echo (data) { } module.exports = new ThreadWorker(echo, { - maxInactiveTime: 500, killBehavior: KillBehaviors.HARD }) diff --git a/tests/worker/thread/longRunningWorkerHardBehavior.js b/tests/worker-files/thread/longRunningWorkerHardBehavior.js similarity index 100% rename from tests/worker/thread/longRunningWorkerHardBehavior.js rename to tests/worker-files/thread/longRunningWorkerHardBehavior.js diff --git a/tests/worker/thread/longRunningWorkerSoftBehavior.js b/tests/worker-files/thread/longRunningWorkerSoftBehavior.js similarity index 100% rename from tests/worker/thread/longRunningWorkerSoftBehavior.js rename to tests/worker-files/thread/longRunningWorkerSoftBehavior.js -- 2.34.1