diff --git a/src/util/task-queue.js b/src/util/task-queue.js index 8013961a3..a872c53df 100644 --- a/src/util/task-queue.js +++ b/src/util/task-queue.js @@ -30,6 +30,7 @@ class TaskQueue { /** * Get the number of queued tasks which have not yet started. + * * @readonly * @memberof TaskQueue */ @@ -49,12 +50,8 @@ class TaskQueue { const newRecord = { cost }; - const promise = new Promise((resolve, reject) => { + newRecord.promise = new Promise((resolve, reject) => { newRecord.cancel = () => { - const index = this._pendingTaskRecords.indexOf(newRecord); - if (index >= 0) { - this._pendingTaskRecords.splice(index, 1); - } reject(new Error('Task canceled')); }; @@ -74,7 +71,27 @@ class TaskQueue { this._runTasks(); } - return promise; + return newRecord.promise; + } + + /** + * Cancel one pending task, rejecting its promise. + * + * @param {Promise} taskPromise - the promise returned by `do()`. + * @returns {boolean} - true if the task was found, or false otherwise. + * @memberof TaskQueue + */ + cancel (taskPromise) { + const taskIndex = this._pendingTaskRecords.findIndex(r => r.promise === taskPromise); + if (taskIndex !== -1) { + const [taskRecord] = this._pendingTaskRecords.splice(taskIndex, 1); + taskRecord.cancel(); + if (taskIndex === 0 && this._pendingTaskRecords.length > 0) { + this._runTasks(); + } + return true; + } + return false; } /** @@ -140,6 +157,7 @@ class TaskQueue { /** * Loop until the task queue is empty, running each task and spending tokens to do so. * Any time the bucket can't afford the next task, delay asynchronously until it can. + * * @memberof TaskQueue */ _runTasks () { diff --git a/test/fixtures/mock-timer.js b/test/fixtures/mock-timer.js index 4bcf53057..c0e3ae495 100644 --- a/test/fixtures/mock-timer.js +++ b/test/fixtures/mock-timer.js @@ -44,17 +44,32 @@ class MockTimer { /** * Advance this MockTimer's idea of "current time", running timeout handlers if appropriate. - * @param {number} delta - the amount of time to add to the current mock time value, in milliseconds. + * + * @param {number} milliseconds - the amount of time to add to the current mock time value, in milliseconds. * @memberof MockTimer */ - advanceMockTime (delta) { - if (delta < 0) { + advanceMockTime (milliseconds) { + if (milliseconds < 0) { throw new Error('Time may not move backward'); } - this._mockTime += delta; + this._mockTime += milliseconds; this._runTimeouts(); } + /** + * Advance this MockTimer's idea of "current time", running timeout handlers if appropriate. + * + * @param {number} milliseconds - the amount of time to add to the current mock time value, in milliseconds. + * @returns {Promise} - promise which resolves after timeout handlers have had an opportunity to run. + * @memberof MockTimer + */ + advanceMockTimeAsync (milliseconds) { + return new Promise(resolve => { + this.advanceMockTime(milliseconds); + global.setTimeout(resolve, 0); + }); + } + /** * @returns {number} - current mock time elapsed since 1 January 1970 00:00:00 UTC. * @memberof MockTimer diff --git a/test/unit/mock-timer.js b/test/unit/mock-timer.js index e6815117a..db5878942 100644 --- a/test/unit/mock-timer.js +++ b/test/unit/mock-timer.js @@ -16,6 +16,8 @@ test('spec', t => { t.type(timer.clearTimeout, 'function'); // A few members of MockTimer have no Timer equivalent and should only be used in tests. + t.type(timer.advanceMockTime, 'function'); + t.type(timer.advanceMockTimeAsync, 'function'); t.type(timer.hasTimeouts, 'function'); t.end(); diff --git a/test/unit/util_task-queue.js b/test/unit/util_task-queue.js index d9ac12eb7..b514edee1 100644 --- a/test/unit/util_task-queue.js +++ b/test/unit/util_task-queue.js @@ -26,12 +26,86 @@ test('spec', t => { t.type(bukkit.length, 'number'); t.type(bukkit.do, 'function'); + t.type(bukkit.cancel, 'function'); t.type(bukkit.cancelAll, 'function'); t.end(); }); -test('cancelAll', t => { +test('run tasks', async t => { + const bukkit = makeTestQueue(); + + const taskResults = []; + + const promises = [ + bukkit.do(() => { + taskResults.push('a'); + testCompare(t, bukkit._timer.timeElapsed(), '>=', 50, 'Costly task must wait'); + }, 50), + bukkit.do(() => { + taskResults.push('b'); + testCompare(t, bukkit._timer.timeElapsed(), '>=', 60, 'Tasks must run in serial'); + }, 10), + bukkit.do(() => { + taskResults.push('c'); + testCompare(t, bukkit._timer.timeElapsed(), '<=', 70, 'Cheap task should run soon'); + }, 1) + ]; + + // advance 10 simulated milliseconds per JS tick + while (bukkit.length > 0) { + await bukkit._timer.advanceMockTimeAsync(10); + } + + return Promise.all(promises).then(() => { + t.deepEqual(taskResults, ['a', 'b', 'c'], 'All tasks must run in correct order'); + t.end(); + }); +}); + +test('cancel', async t => { + const bukkit = makeTestQueue(); + + const taskResults = []; + const goodCancelMessage = 'Task was canceled correctly'; + const afterCancelMessage = 'Task was run correctly'; + const cancelTaskPromise = bukkit.do( + () => { + taskResults.push('nope'); + }, 999); + const cancelCheckPromise = cancelTaskPromise.then( + () => { + t.fail('Task should have been canceled'); + }, + () => { + taskResults.push(goodCancelMessage); + } + ); + const keepTaskPromise = bukkit.do( + () => { + taskResults.push(afterCancelMessage); + testCompare(t, bukkit._timer.timeElapsed(), '<', 10, 'Canceled task must not delay other tasks'); + }, 5); + + // give the bucket a chance to make a mistake + await bukkit._timer.advanceMockTimeAsync(1); + + t.equal(bukkit.length, 2); + const taskWasCanceled = bukkit.cancel(cancelTaskPromise); + t.ok(taskWasCanceled); + t.equal(bukkit.length, 1); + + while (bukkit.length > 0) { + await bukkit._timer.advanceMockTimeAsync(1); + } + + return Promise.all([cancelCheckPromise, keepTaskPromise]).then(() => { + t.deepEqual(taskResults, [goodCancelMessage, afterCancelMessage]); + t.end(); + }); +}); + +test('cancelAll', async t => { const bukkit = makeTestQueue(); const taskResults = []; @@ -58,12 +132,12 @@ test('cancelAll', t => { ]; // advance time, but not enough that any task should run - bukkit._timer.advanceMockTime(100); + await bukkit._timer.advanceMockTimeAsync(100); bukkit.cancelAll(); // advance enough that both tasks would run if they hadn't been canceled - bukkit._timer.advanceMockTime(10000); + await bukkit._timer.advanceMockTimeAsync(10000); return Promise.all(promises).then(() => { t.deepEqual(taskResults, [goodCancelMessage1, goodCancelMessage2], 'Tasks should cancel in order'); @@ -71,37 +145,3 @@ test('cancelAll', t => { }); }); -test('run tasks', t => { - const bukkit = makeTestQueue(); - - const taskResults = []; - - const promises = [ - bukkit.do(() => { - taskResults.push('a'); - testCompare(t, bukkit._timer.timeElapsed(), '>=', 50, 'Costly task must wait'); - }, 50), - bukkit.do(() => { - taskResults.push('b'); - testCompare(t, bukkit._timer.timeElapsed(), '>=', 60, 'Tasks must run in serial'); - }, 10), - bukkit.do(() => { - taskResults.push('c'); - testCompare(t, bukkit._timer.timeElapsed(), '<=', 70, 'Cheap task should run soon'); - }, 1) - ]; - - // advance 10 simulated milliseconds per JS tick - const step = () => { - bukkit._timer.advanceMockTime(10); - if (bukkit.length > 0) { - global.setTimeout(step, 0); - } - }; - step(); - - return Promise.all(promises).then(() => { - t.deepEqual(taskResults, ['a', 'b', 'c'], 'All tasks must run in correct order'); - t.end(); - }); -});