From 3dfc1ca213cc53f8396fd55620ab5164849ba6d8 Mon Sep 17 00:00:00 2001 From: Mikunj Date: Fri, 22 May 2020 14:04:10 +1000 Subject: [PATCH] Improved JobQueue. Added tests. --- package.json | 3 +- ts/session/utils/JobQueue.ts | 21 +++-- ts/test/session/utils/JobQueue_test.ts | 109 +++++++++++++++++++++++++ ts/test/tslint.json | 6 +- ts/test/utils/timeout.ts | 4 + yarn.lock | 31 +++++++ 6 files changed, 165 insertions(+), 9 deletions(-) create mode 100644 ts/test/session/utils/JobQueue_test.ts create mode 100644 ts/test/utils/timeout.ts diff --git a/package.json b/package.json index 4603dedd2..4e78cb491 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "test-electron": "yarn grunt test", "test-integration": "ELECTRON_DISABLE_SANDBOX=1 mocha --exit --timeout 10000 integration_test/integration_test.js", "test-integration-parts": "ELECTRON_DISABLE_SANDBOX=1 mocha --exit --timeout 10000 integration_test/integration_test.js --grep 'registration' && ELECTRON_DISABLE_SANDBOX=1 mocha --exit --timeout 10000 integration_test/integration_test.js --grep 'openGroup' && ELECTRON_DISABLE_SANDBOX=1 mocha --exit --timeout 10000 integration_test/integration_test.js --grep 'addFriends' && ELECTRON_DISABLE_SANDBOX=1 mocha --exit --timeout 10000 integration_test/integration_test.js --grep 'linkDevice' && ELECTRON_DISABLE_SANDBOX=1 mocha --exit --timeout 10000 integration_test/integration_test.js --grep 'closedGroup'", - "test-node": "mocha --recursive --exit test/app test/modules ts/test libloki/test/node", + "test-node": "mocha --recursive --exit test/app test/modules ts/test libloki/test/node --timeout 10000", "eslint": "eslint --cache .", "eslint-fix": "eslint --fix .", "eslint-full": "eslint .", @@ -120,6 +120,7 @@ }, "devDependencies": { "@types/chai": "4.1.2", + "@types/chai-as-promised": "^7.1.2", "@types/classnames": "2.2.3", "@types/color": "^3.0.0", "@types/config": "0.0.34", diff --git a/ts/session/utils/JobQueue.ts b/ts/session/utils/JobQueue.ts index d9a58d909..fa5082836 100644 --- a/ts/session/utils/JobQueue.ts +++ b/ts/session/utils/JobQueue.ts @@ -1,37 +1,44 @@ import { v4 as uuid } from 'uuid'; +type Job = (() => PromiseLike) | (() => ResultType); + // TODO: This needs to replace js/modules/job_queue.js export class JobQueue { private pending: Promise = Promise.resolve(); - private readonly jobs: Map> = new Map(); + private readonly jobs: Map> = new Map(); public has(id: string): boolean { return this.jobs.has(id); } - public async add(job: () => any): Promise { + public async add(job: Job): Promise { const id = uuid(); return this.addWithId(id, job); } - public async addWithId(id: string, job: () => any): Promise { + public async addWithId( + id: string, + job: Job + ): Promise { if (this.jobs.has(id)) { - return this.jobs.get(id); + return this.jobs.get(id) as Promise; } const previous = this.pending || Promise.resolve(); this.pending = previous.then(job, job); const current = this.pending; - current + void current + .catch(() => { + // This is done to avoid UnhandledPromiseError + }) .finally(() => { if (this.pending === current) { delete this.pending; } this.jobs.delete(id); - }) - .ignore(); + }); this.jobs.set(id, current); diff --git a/ts/test/session/utils/JobQueue_test.ts b/ts/test/session/utils/JobQueue_test.ts new file mode 100644 index 000000000..f431b455d --- /dev/null +++ b/ts/test/session/utils/JobQueue_test.ts @@ -0,0 +1,109 @@ +import chai from 'chai'; +import { v4 as uuid } from 'uuid'; +import { JobQueue } from '../../../session/utils/JobQueue'; +import { delay } from '../../utils/delay'; + +// tslint:disable-next-line: no-require-imports no-var-requires +const chaiAsPromised = require('chai-as-promised'); +chai.use(chaiAsPromised); + +const { assert } = chai; + +describe('JobQueue', () => { + describe('has', () => { + it('should return the correct value', async () => { + const queue = new JobQueue(); + const id = 'jobId'; + + assert.isFalse(queue.has(id)); + const promise = queue.addWithId(id, async () => delay(100)); + assert.isTrue(queue.has(id)); + await promise; + assert.isFalse(queue.has(id)); + }); + }); + + describe('addWithId', () => { + it('should run the jobs concurrently', async () => { + const input = [[10, 300], [20, 200], [30, 100]]; + const queue = new JobQueue(); + const mapper = async ([value, ms]: Array): Promise => + queue.addWithId(uuid(), async () => { + await delay(ms); + + return value; + }); + + const start = Date.now(); + assert.deepEqual(await Promise.all(input.map(mapper)), [10, 20, 30]); + const timeTaken = Date.now() - start; + assert.closeTo(timeTaken, 600, 50, 'Queue was delayed'); + }); + + it('should return the result of the job', async () => { + const queue = new JobQueue(); + const success = queue.addWithId(uuid(), async () => { + await delay(100); + + return 'success'; + }); + const failure = queue.addWithId(uuid(), async () => { + await delay(100); + throw new Error('failed'); + }); + + assert.equal(await success, 'success'); + await assert.isRejected(failure, /failed/); + }); + + it('should handle sync and async tasks', async () => { + const queue = new JobQueue(); + const first = queue.addWithId(uuid(), () => 'first'); + const second = queue.addWithId(uuid(), async () => { + await delay(100); + + return 'second'; + }); + const third = queue.addWithId(uuid(), () => 'third'); + + assert.deepEqual(await Promise.all([first, second, third]), [ + 'first', + 'second', + 'third', + ]); + }); + + it('should return the previous job if same id was passed', async () => { + const queue = new JobQueue(); + const id = uuid(); + const job = async () => { + await delay(100); + + return 'job1'; + }; + + const promise = queue.addWithId(id, job); + const otherPromise = queue.addWithId(id, () => 'job2'); + assert.equal(await promise, await otherPromise); + await promise; + }); + + it('should remove completed jobs', async () => { + const queue = new JobQueue(); + const id = uuid(); + + const successfullJob = queue.addWithId(id, async () => delay(100)); + assert.isTrue(queue.has(id)); + await successfullJob; + assert.isFalse(queue.has(id)); + + const failJob = queue.addWithId(id, async () => { + await delay(100); + throw new Error('failed'); + }); + assert.isTrue(queue.has(id)); + await assert.isRejected(failJob, /failed/); + assert.isFalse(queue.has(id)); + }); + }); +}); diff --git a/ts/test/tslint.json b/ts/test/tslint.json index 4645335d0..21571a6db 100644 --- a/ts/test/tslint.json +++ b/ts/test/tslint.json @@ -6,6 +6,10 @@ "no-implicit-dependencies": false, // All tests use arrow functions, and they can be long - "max-func-body-length": false + "max-func-body-length": false, + + "no-unused-expression": false, + + "await-promise": [true, "PromiseLike"] } } diff --git a/ts/test/utils/timeout.ts b/ts/test/utils/timeout.ts new file mode 100644 index 000000000..cafd9cf55 --- /dev/null +++ b/ts/test/utils/timeout.ts @@ -0,0 +1,4 @@ +export async function timeout(ms: number): Promise { + // tslint:disable-next-line no-string-based-set-timeout + return new Promise(resolve => setTimeout(resolve, ms)); +} diff --git a/yarn.lock b/yarn.lock index 09e1f4b11..c335da9f3 100644 --- a/yarn.lock +++ b/yarn.lock @@ -156,6 +156,18 @@ dependencies: defer-to-connect "^1.0.1" +"@types/chai-as-promised@^7.1.2": + version "7.1.2" + resolved "https://registry.yarnpkg.com/@types/chai-as-promised/-/chai-as-promised-7.1.2.tgz#2f564420e81eaf8650169e5a3a6b93e096e5068b" + integrity sha512-PO2gcfR3Oxa+u0QvECLe1xKXOqYTzCmWf0FhLhjREoW3fPAVamjihL7v1MOVLJLsnAMdLcjkfrs01yvDMwVK4Q== + dependencies: + "@types/chai" "*" + +"@types/chai@*": + version "4.2.11" + resolved "https://registry.yarnpkg.com/@types/chai/-/chai-4.2.11.tgz#d3614d6c5f500142358e6ed24e1bf16657536c50" + integrity sha512-t7uW6eFafjO+qJ3BIV2gGUyZs27egcNRkUdalkud+Qa3+kg//f129iuOFivHDXQ+vnU3fDXuwgv0cqMCbcE8sw== + "@types/chai@4.1.2": version "4.1.2" resolved "https://registry.yarnpkg.com/@types/chai/-/chai-4.1.2.tgz#f1af664769cfb50af805431c407425ed619daa21" @@ -4836,6 +4848,13 @@ in-publish@^2.0.0: resolved "https://registry.yarnpkg.com/in-publish/-/in-publish-2.0.1.tgz#948b1a535c8030561cea522f73f78f4be357e00c" integrity sha512-oDM0kUSNFC31ShNxHKUyfZKy8ZeXZBWMjMdZHKLOk13uvT27VTL/QzRGfRUcevJhpkZAvlhPYuXkF7eNWrtyxQ== +indent-string@^2.1.0: + version "2.1.0" + resolved "https://registry.yarnpkg.com/indent-string/-/indent-string-2.1.0.tgz#8e2d48348742121b4a8218b7a137e9a52049dc80" + integrity sha1-ji1INIdCEhtKghi3oTfppSBJ3IA= + dependencies: + repeating "^2.0.0" + indexes-of@^1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/indexes-of/-/indexes-of-1.0.1.tgz#f30f716c8e2bd346c7b67d3df3915566a7c05607" @@ -9193,6 +9212,11 @@ slice-ansi@1.0.0: dependencies: is-fullwidth-code-point "^2.0.0" +slide@^1.1.5: + version "1.1.6" + resolved "https://registry.yarnpkg.com/slide/-/slide-1.1.6.tgz#56eb027d65b4d2dce6cb2e2d32c4d4afc9e1d707" + integrity sha1-VusCfWW00tzmyy4tMsTUr8nh1wc= + snapdragon-node@^2.0.1: version "2.1.1" resolved "https://registry.yarnpkg.com/snapdragon-node/-/snapdragon-node-2.1.1.tgz#6c175f86ff14bdb0724563e8f3c1b021a286853b" @@ -9650,6 +9674,13 @@ strip-eof@^1.0.0: resolved "https://registry.yarnpkg.com/strip-eof/-/strip-eof-1.0.0.tgz#bb43ff5598a6eb05d89b59fcd129c983313606bf" integrity sha1-u0P/VZim6wXYm1n80SnJgzE2Br8= +strip-indent@^1.0.1: + version "1.0.1" + resolved "https://registry.yarnpkg.com/strip-indent/-/strip-indent-1.0.1.tgz#0c7962a6adefa7bbd4ac366460a638552ae1a0a2" + integrity sha1-DHlipq3vp7vUrDZkYKY4VSrhoKI= + dependencies: + get-stdin "^4.0.1" + strip-json-comments@~2.0.1: version "2.0.1" resolved "https://registry.yarnpkg.com/strip-json-comments/-/strip-json-comments-2.0.1.tgz#3c531942e908c2697c0ec344858c286c7ca0a60a"