From c510fea7f620c9b406f1b3f655729ad83ed32c2b Mon Sep 17 00:00:00 2001 From: Alessandro Pio Ardizio Date: Thu, 18 Feb 2021 11:00:39 +0100 Subject: [PATCH] Fix sonar code smells (#184) * Fix sonar code smells * Add coverage thresholds * Coverage thresholds * Update src/pools/abstract-pool.ts Co-authored-by: Shinigami * Update src/pools/abstract-pool.ts Co-authored-by: Shinigami * Update src/worker/abstract-worker.ts Co-authored-by: Shinigami * Update src/worker/abstract-worker.ts Co-authored-by: Shinigami Co-authored-by: Shinigami --- .nycrc.json | 2 +- src/pools/abstract-pool.ts | 24 +++++++++++++++------- src/worker/abstract-worker.ts | 11 +++++++++- tests/pools/abstract/abstract-pool.test.js | 9 ++++++++ tests/worker/abstract-worker.test.js | 10 +++++++++ 5 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 tests/worker/abstract-worker.test.js diff --git a/.nycrc.json b/.nycrc.json index 0d2fa013..b6bd046a 100644 --- a/.nycrc.json +++ b/.nycrc.json @@ -2,6 +2,6 @@ "check-coverage": true, "lines": 90, "statements": 90, - "functions": 89, + "functions": 90, "branches": 85 } diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts index 4f467bf4..c951c6f2 100644 --- a/src/pools/abstract-pool.ts +++ b/src/pools/abstract-pool.ts @@ -2,6 +2,13 @@ import EventEmitter from 'events' import type { MessageValue } from '../utility-types' import type { IPool } from './pool' +/** + * An intentional empty function. + */ +function emptyFunction () { + // intentionally left blank +} + /** * Callback invoked if the worker raised an error. */ @@ -142,10 +149,7 @@ export abstract class AbstractPool< if (!this.isMain()) { throw new Error('Cannot start a pool from a worker!') } - // TODO christopher 2021-02-07: Improve this check e.g. with a pattern or blank check - if (!this.filePath) { - throw new Error('Please specify a file with a worker implementation') - } + this.checkFilePath(this.filePath) this.setupHook() for (let i = 1; i <= this.numberOfWorkers; i++) { @@ -155,6 +159,12 @@ export abstract class AbstractPool< this.emitter = new PoolEmitter() } + private checkFilePath (filePath: string) { + if (!filePath) { + throw new Error('Please specify a file with a worker implementation') + } + } + /** * Perform the task specified in the constructor with the data parameter. * @@ -317,9 +327,9 @@ export abstract class AbstractPool< protected createAndSetupWorker (): Worker { const worker: Worker = this.createWorker() - worker.on('error', this.opts.errorHandler ?? (() => {})) - worker.on('online', this.opts.onlineHandler ?? (() => {})) - worker.on('exit', this.opts.exitHandler ?? (() => {})) + worker.on('error', this.opts.errorHandler ?? emptyFunction) + worker.on('online', this.opts.onlineHandler ?? emptyFunction) + worker.on('exit', this.opts.exitHandler ?? emptyFunction) worker.once('exit', () => this.removeWorker(worker)) this.workers.push(worker) diff --git a/src/worker/abstract-worker.ts b/src/worker/abstract-worker.ts index 6aa34e8e..03d954fe 100644 --- a/src/worker/abstract-worker.ts +++ b/src/worker/abstract-worker.ts @@ -66,7 +66,7 @@ export abstract class AbstractWorker< this.opts.maxInactiveTime ?? DEFAULT_MAX_INACTIVE_TIME this.async = !!this.opts.async this.lastTask = Date.now() - if (!fn) throw new Error('fn parameter is mandatory') + this.checkFunctionInput(fn) // Keep the worker active if (!isMain) { this.interval = setInterval( @@ -96,6 +96,15 @@ export abstract class AbstractWorker< }) } + /** + * Check if the `fn` parameter is passed to the constructor. + * + * @param fn The function that should be defined. + */ + private checkFunctionInput (fn: (data: Data) => Response) { + if (!fn) throw new Error('fn parameter is mandatory') + } + /** * Returns the main worker. * diff --git a/tests/pools/abstract/abstract-pool.test.js b/tests/pools/abstract/abstract-pool.test.js index 65d667f1..bc2385f4 100644 --- a/tests/pools/abstract/abstract-pool.test.js +++ b/tests/pools/abstract/abstract-pool.test.js @@ -52,4 +52,13 @@ describe('Abstract pool test suite ', () => { ) }).toThrowError() }) + + it('Verify that filePath is checked', () => { + expect(() => { + const pool = new StubPoolWithIsMainMethod(1).toThrowError() + }) + expect(() => { + const pool = new StubPoolWithIsMainMethod(1, '').toThrowError() + }) + }) }) diff --git a/tests/worker/abstract-worker.test.js b/tests/worker/abstract-worker.test.js new file mode 100644 index 00000000..ddb8427b --- /dev/null +++ b/tests/worker/abstract-worker.test.js @@ -0,0 +1,10 @@ +const expect = require('expect') +const { ThreadWorker } = require('../../lib') + +describe('Abstract worker test suite', () => { + it('Verify that fn function is mandatory', () => { + expect(() => { + const worker = new ThreadWorker() + }).toThrowError() + }) +}) -- 2.34.1