From 5502c07c81463257e1dd3015ee5a828846ece0b2 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Tue, 4 Apr 2023 00:04:09 +0200 Subject: [PATCH] refactor: remove deprecated getter in worker selection strategies code MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Jérôme Benoit --- .../worker-choice-strategy-context.ts | 10 - .../selection-strategies.test.js | 171 ++++++++---------- .../worker-choice-strategy-context.test.js | 74 ++++---- 3 files changed, 115 insertions(+), 140 deletions(-) diff --git a/src/pools/selection-strategies/worker-choice-strategy-context.ts b/src/pools/selection-strategies/worker-choice-strategy-context.ts index 011e00db..bd616a54 100644 --- a/src/pools/selection-strategies/worker-choice-strategy-context.ts +++ b/src/pools/selection-strategies/worker-choice-strategy-context.ts @@ -58,16 +58,6 @@ export class WorkerChoiceStrategyContext< return getWorkerChoiceStrategy(this.pool, workerChoiceStrategy) } - /** - * Gets the worker choice strategy used in the context. - * - * @returns The worker choice strategy. - * @deprecated Scheduled removal. - */ - public getWorkerChoiceStrategy (): IWorkerChoiceStrategy { - return this.workerChoiceStrategy - } - /** * Gets the worker choice strategy required statistics. * diff --git a/tests/pools/selection-strategies/selection-strategies.test.js b/tests/pools/selection-strategies/selection-strategies.test.js index d09dde6f..dd420c7f 100644 --- a/tests/pools/selection-strategies/selection-strategies.test.js +++ b/tests/pools/selection-strategies/selection-strategies.test.js @@ -43,7 +43,7 @@ describe('Selection strategies test suite', () => { WorkerChoiceStrategies.ROUND_ROBIN ) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy().nextWorkerId + pool.workerChoiceStrategyContext.workerChoiceStrategy.nextWorkerId ).toBe(0) // We need to clean up the resources after our test await pool.destroy() @@ -152,11 +152,11 @@ describe('Selection strategies test suite', () => { { workerChoiceStrategy: WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN } ) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy().nextWorkerId + pool.workerChoiceStrategyContext.workerChoiceStrategy.nextWorkerId ).toBeUndefined() pool.setWorkerChoiceStrategy(WorkerChoiceStrategies.ROUND_ROBIN) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy().nextWorkerId + pool.workerChoiceStrategyContext.workerChoiceStrategy.nextWorkerId ).toBe(0) await pool.destroy() pool = new DynamicThreadPool( @@ -166,13 +166,13 @@ describe('Selection strategies test suite', () => { { workerChoiceStrategy: WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN } ) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workerChoiceStrategy.nextWorkerId + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .nextWorkerId ).toBeUndefined() pool.setWorkerChoiceStrategy(WorkerChoiceStrategies.ROUND_ROBIN) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workerChoiceStrategy.nextWorkerId + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .nextWorkerId ).toBe(0) // We need to clean up the resources after our test await pool.destroy() @@ -351,18 +351,16 @@ describe('Selection strategies test suite', () => { expect(pool.opts.workerChoiceStrategy).toBe( WorkerChoiceStrategies.FAIR_SHARE ) - for (const workerKey of pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerLastVirtualTaskTimestamp.keys()) { + for (const workerKey of pool.workerChoiceStrategyContext.workerChoiceStrategy.workerLastVirtualTaskTimestamp.keys()) { expect( - pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerLastVirtualTaskTimestamp.get(workerKey).start + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerLastVirtualTaskTimestamp.get( + workerKey + ).start ).toBe(0) expect( - pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerLastVirtualTaskTimestamp.get(workerKey).end + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerLastVirtualTaskTimestamp.get( + workerKey + ).end ).toBe(0) } // We need to clean up the resources after our test @@ -418,7 +416,7 @@ describe('Selection strategies test suite', () => { } await Promise.all(promises) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() + pool.workerChoiceStrategyContext.workerChoiceStrategy .workerLastVirtualTaskTimestamp.size ).toBe(pool.workers.length) // We need to clean up the resources after our test @@ -438,10 +436,10 @@ describe('Selection strategies test suite', () => { promises.push(pool.execute()) } await Promise.all(promises) - // expect( - // pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - // .workerChoiceStrategy.workerLastVirtualTaskTimestamp.size - // ).toBe(pool.workers.length) + expect( + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .workerLastVirtualTaskTimestamp.size + ).toBe(pool.workers.length) // We need to clean up the resources after our test await pool.destroy() }) @@ -452,22 +450,20 @@ describe('Selection strategies test suite', () => { './tests/worker-files/thread/testWorker.js' ) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() + pool.workerChoiceStrategyContext.workerChoiceStrategy .workerLastVirtualTaskTimestamp ).toBeUndefined() pool.setWorkerChoiceStrategy(WorkerChoiceStrategies.FAIR_SHARE) - for (const workerKey of pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerLastVirtualTaskTimestamp.keys()) { + for (const workerKey of pool.workerChoiceStrategyContext.workerChoiceStrategy.workerLastVirtualTaskTimestamp.keys()) { expect( - pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerLastVirtualTaskTimestamp.get(workerKey).start + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerLastVirtualTaskTimestamp.get( + workerKey + ).start ).toBe(0) expect( - pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerLastVirtualTaskTimestamp.get(workerKey).end + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerLastVirtualTaskTimestamp.get( + workerKey + ).end ).toBe(0) } await pool.destroy() @@ -477,24 +473,20 @@ describe('Selection strategies test suite', () => { './tests/worker-files/thread/testWorker.js' ) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workerChoiceStrategy.workerLastVirtualTaskTimestamp + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .workerLastVirtualTaskTimestamp ).toBeUndefined() pool.setWorkerChoiceStrategy(WorkerChoiceStrategies.FAIR_SHARE) - for (const workerKey of pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerChoiceStrategy.workerLastVirtualTaskTimestamp.keys()) { + for (const workerKey of pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy.workerLastVirtualTaskTimestamp.keys()) { expect( - pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerChoiceStrategy.workerLastVirtualTaskTimestamp.get(workerKey) - .start + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy.workerLastVirtualTaskTimestamp.get( + workerKey + ).start ).toBe(0) expect( - pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerChoiceStrategy.workerLastVirtualTaskTimestamp.get(workerKey) - .end + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy.workerLastVirtualTaskTimestamp.get( + workerKey + ).end ).toBe(0) } // We need to clean up the resources after our test @@ -511,24 +503,21 @@ describe('Selection strategies test suite', () => { WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN ) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy().currentWorkerId + pool.workerChoiceStrategyContext.workerChoiceStrategy.currentWorkerId ).toBe(0) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .defaultWorkerWeight + pool.workerChoiceStrategyContext.workerChoiceStrategy.defaultWorkerWeight ).toBeGreaterThan(0) - for (const workerKey of pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workersTaskRunTime.keys()) { + for (const workerKey of pool.workerChoiceStrategyContext.workerChoiceStrategy.workersTaskRunTime.keys()) { expect( - pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workersTaskRunTime.get(workerKey).weight + pool.workerChoiceStrategyContext.workerChoiceStrategy.workersTaskRunTime.get( + workerKey + ).weight ).toBeGreaterThan(0) expect( - pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workersTaskRunTime.get(workerKey).runTime + pool.workerChoiceStrategyContext.workerChoiceStrategy.workersTaskRunTime.get( + workerKey + ).runTime ).toBe(0) } // We need to clean up the resources after our test @@ -584,8 +573,8 @@ describe('Selection strategies test suite', () => { } await Promise.all(promises) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workersTaskRunTime.size + pool.workerChoiceStrategyContext.workerChoiceStrategy.workersTaskRunTime + .size ).toBe(pool.workers.length) // We need to clean up the resources after our test await pool.destroy() @@ -600,14 +589,17 @@ describe('Selection strategies test suite', () => { ) // TODO: Create a better test to cover `WeightedRoundRobinWorkerChoiceStrategy#choose` const promises = [] - for (let i = 0; i < max * 2; i++) { + const maxMultiplier = + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .defaultWorkerWeight + for (let i = 0; i < max * maxMultiplier; i++) { promises.push(pool.execute()) } await Promise.all(promises) - // expect( - // pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - // .workerChoiceStrategy.workersTaskRunTime.size - // ).toBe(pool.workers.length) + expect( + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .workersTaskRunTime.size + ).toBe(pool.workers.length) // We need to clean up the resources after our test await pool.destroy() }) @@ -618,31 +610,26 @@ describe('Selection strategies test suite', () => { './tests/worker-files/thread/testWorker.js' ) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy().currentWorkerId + pool.workerChoiceStrategyContext.workerChoiceStrategy.currentWorkerId ).toBeUndefined() expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .defaultWorkerWeight + pool.workerChoiceStrategyContext.workerChoiceStrategy.defaultWorkerWeight ).toBeUndefined() expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workersTaskRunTime + pool.workerChoiceStrategyContext.workerChoiceStrategy.workersTaskRunTime ).toBeUndefined() pool.setWorkerChoiceStrategy(WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy().currentWorkerId + pool.workerChoiceStrategyContext.workerChoiceStrategy.currentWorkerId ).toBe(0) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .defaultWorkerWeight + pool.workerChoiceStrategyContext.workerChoiceStrategy.defaultWorkerWeight ).toBeGreaterThan(0) - for (const workerKey of pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workersTaskRunTime.keys()) { + for (const workerKey of pool.workerChoiceStrategyContext.workerChoiceStrategy.workersTaskRunTime.keys()) { expect( - pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workersTaskRunTime.get(workerKey).runTime + pool.workerChoiceStrategyContext.workerChoiceStrategy.workersTaskRunTime.get( + workerKey + ).runTime ).toBe(0) } await pool.destroy() @@ -652,33 +639,31 @@ describe('Selection strategies test suite', () => { './tests/worker-files/thread/testWorker.js' ) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workerChoiceStrategy.currentWorkerId + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .currentWorkerId ).toBeUndefined() expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workerChoiceStrategy.defaultWorkerWeight + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .defaultWorkerWeight ).toBeUndefined() expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workerChoiceStrategy.workersTaskRunTime + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .workersTaskRunTime ).toBeUndefined() pool.setWorkerChoiceStrategy(WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workerChoiceStrategy.currentWorkerId + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .currentWorkerId ).toBe(0) expect( - pool.workerChoiceStrategyContext.getWorkerChoiceStrategy() - .workerChoiceStrategy.defaultWorkerWeight + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy + .defaultWorkerWeight ).toBeGreaterThan(0) - for (const workerKey of pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerChoiceStrategy.workersTaskRunTime.keys()) { + for (const workerKey of pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy.workersTaskRunTime.keys()) { expect( - pool.workerChoiceStrategyContext - .getWorkerChoiceStrategy() - .workerChoiceStrategy.workersTaskRunTime.get(workerKey).runTime + pool.workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy.workersTaskRunTime.get( + workerKey + ).runTime ).toBe(0) } // We need to clean up the resources after our test diff --git a/tests/pools/selection-strategies/worker-choice-strategy-context.test.js b/tests/pools/selection-strategies/worker-choice-strategy-context.test.js index 9ab2babf..1f4b51cb 100644 --- a/tests/pools/selection-strategies/worker-choice-strategy-context.test.js +++ b/tests/pools/selection-strategies/worker-choice-strategy-context.test.js @@ -66,7 +66,7 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.workerChoiceStrategy = WorkerChoiceStrategyStub const chosenWorkerKey = workerChoiceStrategyContext.execute() expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy().choose.calledOnce + workerChoiceStrategyContext.workerChoiceStrategy.choose.calledOnce ).toBe(true) expect(chosenWorkerKey).toBe(0) }) @@ -84,7 +84,7 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.workerChoiceStrategy = WorkerChoiceStrategyStub const chosenWorkerKey = workerChoiceStrategyContext.execute() expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy().choose.calledOnce + workerChoiceStrategyContext.workerChoiceStrategy.choose.calledOnce ).toBe(true) expect(chosenWorkerKey).toBe(0) }) @@ -96,9 +96,9 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.setWorkerChoiceStrategy( WorkerChoiceStrategies.ROUND_ROBIN ) - expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy() - ).toBeInstanceOf(RoundRobinWorkerChoiceStrategy) + expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( + RoundRobinWorkerChoiceStrategy + ) }) it('Verify that setWorkerChoiceStrategy() works with ROUND_ROBIN and dynamic pool', () => { @@ -108,11 +108,11 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.setWorkerChoiceStrategy( WorkerChoiceStrategies.ROUND_ROBIN ) + expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( + DynamicPoolWorkerChoiceStrategy + ) expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy() - ).toBeInstanceOf(DynamicPoolWorkerChoiceStrategy) - expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy().workerChoiceStrategy + workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy ).toBeInstanceOf(RoundRobinWorkerChoiceStrategy) }) @@ -123,9 +123,9 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.setWorkerChoiceStrategy( WorkerChoiceStrategies.LESS_USED ) - expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy() - ).toBeInstanceOf(LessUsedWorkerChoiceStrategy) + expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( + LessUsedWorkerChoiceStrategy + ) }) it('Verify that setWorkerChoiceStrategy() works with LESS_USED and dynamic pool', () => { @@ -135,11 +135,11 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.setWorkerChoiceStrategy( WorkerChoiceStrategies.LESS_USED ) + expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( + DynamicPoolWorkerChoiceStrategy + ) expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy() - ).toBeInstanceOf(DynamicPoolWorkerChoiceStrategy) - expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy().workerChoiceStrategy + workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy ).toBeInstanceOf(LessUsedWorkerChoiceStrategy) }) @@ -150,9 +150,9 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.setWorkerChoiceStrategy( WorkerChoiceStrategies.LESS_BUSY ) - expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy() - ).toBeInstanceOf(LessBusyWorkerChoiceStrategy) + expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( + LessBusyWorkerChoiceStrategy + ) }) it('Verify that setWorkerChoiceStrategy() works with LESS_BUSY and dynamic pool', () => { @@ -162,11 +162,11 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.setWorkerChoiceStrategy( WorkerChoiceStrategies.LESS_BUSY ) + expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( + DynamicPoolWorkerChoiceStrategy + ) expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy() - ).toBeInstanceOf(DynamicPoolWorkerChoiceStrategy) - expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy().workerChoiceStrategy + workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy ).toBeInstanceOf(LessBusyWorkerChoiceStrategy) }) @@ -177,9 +177,9 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.setWorkerChoiceStrategy( WorkerChoiceStrategies.FAIR_SHARE ) - expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy() - ).toBeInstanceOf(FairShareWorkerChoiceStrategy) + expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( + FairShareWorkerChoiceStrategy + ) }) it('Verify that setWorkerChoiceStrategy() works with FAIR_SHARE and dynamic pool', () => { @@ -189,11 +189,11 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.setWorkerChoiceStrategy( WorkerChoiceStrategies.FAIR_SHARE ) + expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( + DynamicPoolWorkerChoiceStrategy + ) expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy() - ).toBeInstanceOf(DynamicPoolWorkerChoiceStrategy) - expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy().workerChoiceStrategy + workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy ).toBeInstanceOf(FairShareWorkerChoiceStrategy) }) @@ -204,9 +204,9 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.setWorkerChoiceStrategy( WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN ) - expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy() - ).toBeInstanceOf(WeightedRoundRobinWorkerChoiceStrategy) + expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( + WeightedRoundRobinWorkerChoiceStrategy + ) }) it('Verify that setWorkerChoiceStrategy() works with WEIGHTED_ROUND_ROBIN and dynamic pool', () => { @@ -216,11 +216,11 @@ describe('Worker choice strategy context test suite', () => { workerChoiceStrategyContext.setWorkerChoiceStrategy( WorkerChoiceStrategies.WEIGHTED_ROUND_ROBIN ) + expect(workerChoiceStrategyContext.workerChoiceStrategy).toBeInstanceOf( + DynamicPoolWorkerChoiceStrategy + ) expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy() - ).toBeInstanceOf(DynamicPoolWorkerChoiceStrategy) - expect( - workerChoiceStrategyContext.getWorkerChoiceStrategy().workerChoiceStrategy + workerChoiceStrategyContext.workerChoiceStrategy.workerChoiceStrategy ).toBeInstanceOf(WeightedRoundRobinWorkerChoiceStrategy) }) }) -- 2.34.1