From bda851bda8a2ca2adcbb3cce070b26188bd1648a Mon Sep 17 00:00:00 2001 From: Matt Hinchliffe Date: Fri, 23 Nov 2018 14:46:26 +0000 Subject: [PATCH 1/8] Add a check in the task runner to wait until tasks running in any depended upon packages are complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🐿 v2.10.0 --- src/cli-task.js | 2 +- src/run-parallel.js | 42 +++++++++++++++++++++++++++++++++++++----- src/wait-until.js | 15 +++++++++++++++ 3 files changed, 53 insertions(+), 6 deletions(-) create mode 100644 src/wait-until.js diff --git a/src/cli-task.js b/src/cli-task.js index a217174..8084a25 100644 --- a/src/cli-task.js +++ b/src/cli-task.js @@ -39,7 +39,7 @@ module.exports = (task) => { logger.info(`Running ${tasks.length} tasks`); // 6. execute all tasks - await runParallel(tasks, globals.concurrency); + await runParallel(sortedPackages, tasks, globals.concurrency); timer.stop(); diff --git a/src/run-parallel.js b/src/run-parallel.js index a304f62..da6478b 100644 --- a/src/run-parallel.js +++ b/src/run-parallel.js @@ -1,14 +1,46 @@ +const Semaphore = require('async-sema'); const logger = require('./logger'); -const Sema = require('async-sema'); +const waitUntil = require('./wait-until'); -module.exports = (tasks = [], concurrency = 1) => { - const sema = new Sema(concurrency); +const noRunningDependencies = (running, dependencies) => { + return !dependencies.some((dependency) => running.has(dependency)); +}; + +module.exports = (packages = [], tasks = [], concurrency = 1) => { + const semaphore = new Semaphore(concurrency); logger.info(`Executing up to ${concurrency} tasks at a time`); + const packagesRunning = new Set(); + return Promise.all( - tasks.map((task) => { - return sema.acquire().then(task).then(() => sema.release()); + tasks.map((task, i) => { + // TODO: number of tasks ~= number of packages + const packageName = packages[i].name; + const packageManifest = packages[i].manifest; + + const allDependencies = Object.keys({ + ...packageManifest.dependencies, + ...packageManifest.devDependencies, + ...packageManifest.peerDependencies, + ...packageManifest.optionalDependencies + }); + + return semaphore + .acquire() + .then(() => { + return waitUntil(() => { + return noRunningDependencies(packagesRunning, allDependencies); + }); + }) + .then(() => { + packagesRunning.add(packageName); + return task(); + }) + .then(() => { + packagesRunning.delete(packageName); + return semaphore.release(); + }); }) ); }; diff --git a/src/wait-until.js b/src/wait-until.js new file mode 100644 index 0000000..edd0faa --- /dev/null +++ b/src/wait-until.js @@ -0,0 +1,15 @@ +const wait = (ms = 100) => new Promise((resolve) => setTimeout(resolve, ms)); + +module.exports = (check) => { + return new Promise((resolve) => { + const test = () => { + if (check()) { + resolve(); + } else { + wait().then(test); + } + }; + + test(); + }); +}; From a9be892c382d6c6316874aaa8c96eac8b8e5b188 Mon Sep 17 00:00:00 2001 From: Matt Hinchliffe Date: Fri, 7 Dec 2018 08:29:19 +0000 Subject: [PATCH 2/8] =?UTF-8?q?Refactor=20tasks=20to=20return=20package=20?= =?UTF-8?q?instance=20with=20callback=20function=20=20=F0=9F=90=BF=20v2.10?= =?UTF-8?q?.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/cli-task.js | 2 +- src/run-parallel.js | 22 +++++++++------------- src/sort-packages.js | 1 + src/tasks/exec.js | 3 ++- src/tasks/publish.js | 3 ++- src/tasks/run.js | 3 ++- src/tasks/script.js | 3 ++- src/tasks/version.js | 4 +++- test/src/tasks/exec.spec.js | 13 ++++++------- test/src/tasks/publish.spec.js | 13 ++++++------- test/src/tasks/run.spec.js | 13 ++++++------- test/src/tasks/script.spec.js | 13 ++++++------- test/src/tasks/version.spec.js | 5 +++-- 13 files changed, 49 insertions(+), 49 deletions(-) diff --git a/src/cli-task.js b/src/cli-task.js index 8084a25..a217174 100644 --- a/src/cli-task.js +++ b/src/cli-task.js @@ -39,7 +39,7 @@ module.exports = (task) => { logger.info(`Running ${tasks.length} tasks`); // 6. execute all tasks - await runParallel(sortedPackages, tasks, globals.concurrency); + await runParallel(tasks, globals.concurrency); timer.stop(); diff --git a/src/run-parallel.js b/src/run-parallel.js index da6478b..3f7e7a3 100644 --- a/src/run-parallel.js +++ b/src/run-parallel.js @@ -6,7 +6,7 @@ const noRunningDependencies = (running, dependencies) => { return !dependencies.some((dependency) => running.has(dependency)); }; -module.exports = (packages = [], tasks = [], concurrency = 1) => { +module.exports = (tasks = [], concurrency = 1) => { const semaphore = new Semaphore(concurrency); logger.info(`Executing up to ${concurrency} tasks at a time`); @@ -14,16 +14,12 @@ module.exports = (packages = [], tasks = [], concurrency = 1) => { const packagesRunning = new Set(); return Promise.all( - tasks.map((task, i) => { - // TODO: number of tasks ~= number of packages - const packageName = packages[i].name; - const packageManifest = packages[i].manifest; - + tasks.map(({ pkg, apply }) => { const allDependencies = Object.keys({ - ...packageManifest.dependencies, - ...packageManifest.devDependencies, - ...packageManifest.peerDependencies, - ...packageManifest.optionalDependencies + ...pkg.manifest.dependencies, + ...pkg.manifest.devDependencies, + ...pkg.manifest.peerDependencies, + ...pkg.manifest.optionalDependencies }); return semaphore @@ -34,11 +30,11 @@ module.exports = (packages = [], tasks = [], concurrency = 1) => { }); }) .then(() => { - packagesRunning.add(packageName); - return task(); + packagesRunning.add(pkg.name); + return apply(); }) .then(() => { - packagesRunning.delete(packageName); + packagesRunning.delete(pkg.name); return semaphore.release(); }); }) diff --git a/src/sort-packages.js b/src/sort-packages.js index aa02306..659a3bc 100644 --- a/src/sort-packages.js +++ b/src/sort-packages.js @@ -1,6 +1,7 @@ const toposort = require('toposort'); const collateDependencies = (manifest) => { + // TODO: refactor into package class return Object.keys({ ...manifest.dependencies, ...manifest.devDependencies, diff --git a/src/tasks/exec.js b/src/tasks/exec.js index a987cf4..c4f9bd0 100644 --- a/src/tasks/exec.js +++ b/src/tasks/exec.js @@ -3,7 +3,8 @@ const runPackage = require('../run-package'); function exec (packages = [], command, args = []) { return packages.map((pkg) => { - return () => runPackage(command, args, pkg.location); + const apply = () => runPackage(command, args, pkg.location); + return { pkg, apply }; }); }; diff --git a/src/tasks/publish.js b/src/tasks/publish.js index a611428..798ab9e 100644 --- a/src/tasks/publish.js +++ b/src/tasks/publish.js @@ -10,7 +10,8 @@ function publish (packages = [], args = []) { // create a queue of tasks to run return filteredPackages.map((pkg) => { - return () => runPackage('npm', ['publish', ...args], pkg.location); + const apply = () => runPackage('npm', ['publish', ...args], pkg.location); + return { pkg, apply }; }); }; diff --git a/src/tasks/run.js b/src/tasks/run.js index 998fb3c..9e8b208 100644 --- a/src/tasks/run.js +++ b/src/tasks/run.js @@ -12,7 +12,8 @@ function run (packages = [], script) { // create a queue of tasks to run return filteredPackages.map((pkg) => { - return () => runPackage('npm', ['run', script], pkg.location); + const apply = () => runPackage('npm', ['run', script], pkg.location); + return { pkg, apply }; }); }; diff --git a/src/tasks/script.js b/src/tasks/script.js index 6e611ce..77c57a4 100644 --- a/src/tasks/script.js +++ b/src/tasks/script.js @@ -6,7 +6,8 @@ function script (packages = [], scriptPath) { const resolvedScript = path.resolve(process.cwd(), scriptPath); return packages.map((pkg) => { - return () => runPackage('node', [resolvedScript], pkg.location); + const apply = () => runPackage('node', [resolvedScript], pkg.location); + return { pkg, apply }; }); } diff --git a/src/tasks/version.js b/src/tasks/version.js index 7faaa84..d102b7f 100644 --- a/src/tasks/version.js +++ b/src/tasks/version.js @@ -16,10 +16,12 @@ function version (packages = [], tag) { const packageNames = new Set(packages.map((pkg) => pkg.name)); return packages.map((pkg) => { - return () => { + const apply = () => { const newManifest = updateVersions(pkg.manifest, number, packageNames); return pkg.writeManifest(newManifest); }; + + return { pkg, apply }; }); }; diff --git a/test/src/tasks/exec.spec.js b/test/src/tasks/exec.spec.js index 797af1f..f8fb776 100644 --- a/test/src/tasks/exec.spec.js +++ b/test/src/tasks/exec.spec.js @@ -31,11 +31,12 @@ describe('src/tasks/exec', () => { mockRun.mockReset(); }); - it('it returns an array of functions', () => { + it('it returns an array of tasks', () => { expect(result).toBeInstanceOf(Array); result.forEach((item) => { - expect(item).toBeInstanceOf(Function); + expect(item.pkg).toBeDefined(); + expect(item.apply).toEqual(expect.any(Function)); }); }); @@ -44,12 +45,10 @@ describe('src/tasks/exec', () => { }); it('provides the correct arguments to run helper', () => { - result.forEach((item, i) => { - const pkg = packages[i]; - - item(); + result.forEach((item) => { + item.apply(); - expect(mockRun).toHaveBeenCalledWith(command, args, pkg.location); + expect(mockRun).toHaveBeenCalledWith(command, args, item.pkg.location); }); }); }); diff --git a/test/src/tasks/publish.spec.js b/test/src/tasks/publish.spec.js index accb190..88a2a5b 100644 --- a/test/src/tasks/publish.spec.js +++ b/test/src/tasks/publish.spec.js @@ -30,11 +30,12 @@ describe('src/tasks/publish', () => { mockRun.mockReset(); }); - it('it returns an array of functions', () => { + it('it returns an array of tasks', () => { expect(result).toBeInstanceOf(Array); result.forEach((item) => { - expect(item).toBeInstanceOf(Function); + expect(item.pkg).toBeDefined(); + expect(item.apply).toEqual(expect.any(Function)); }); }); @@ -43,12 +44,10 @@ describe('src/tasks/publish', () => { }); it('provides the correct arguments to run helper', () => { - result.forEach((item, i) => { - const pkg = packages[i]; - - item(); + result.forEach((item) => { + item.apply(); - expect(mockRun).toHaveBeenCalledWith('npm', ['publish'].concat(args), pkg.location); + expect(mockRun).toHaveBeenCalledWith('npm', ['publish'].concat(args), item.pkg.location); }); }); }); diff --git a/test/src/tasks/run.spec.js b/test/src/tasks/run.spec.js index bd7d33e..5413f7b 100644 --- a/test/src/tasks/run.spec.js +++ b/test/src/tasks/run.spec.js @@ -30,11 +30,12 @@ describe('src/tasks/run', () => { mockRun.mockReset(); }); - it('it returns an array of functions', () => { + it('it returns an array of tasks', () => { expect(result).toBeInstanceOf(Array); result.forEach((item) => { - expect(item).toBeInstanceOf(Function); + expect(item.pkg).toBeDefined(); + expect(item.apply).toEqual(expect.any(Function)); }); }); @@ -43,12 +44,10 @@ describe('src/tasks/run', () => { }); it('provides the correct arguments to run helper', () => { - result.forEach((item, i) => { - const pkg = packages[i]; - - item(); + result.forEach((item) => { + item.apply(); - expect(mockRun).toHaveBeenCalledWith('npm', ['run', command], pkg.location); + expect(mockRun).toHaveBeenCalledWith('npm', ['run', command], item.pkg.location); }); }); }); diff --git a/test/src/tasks/script.spec.js b/test/src/tasks/script.spec.js index 15cb41a..3d3da3f 100644 --- a/test/src/tasks/script.spec.js +++ b/test/src/tasks/script.spec.js @@ -29,11 +29,12 @@ describe('src/tasks/script', () => { mockRun.mockReset(); }); - it('it returns an array of functions', () => { + it('it returns an array of tasks', () => { expect(result).toBeInstanceOf(Array); result.forEach((item) => { - expect(item).toBeInstanceOf(Function); + expect(item.pkg).toBeDefined(); + expect(item.apply).toEqual(expect.any(Function)); }); }); @@ -44,12 +45,10 @@ describe('src/tasks/script', () => { it('provides the correct arguments to run helper', () => { const resolvedPath = process.cwd() + '/' + scriptPath; - result.forEach((item, i) => { - const pkg = packages[i]; - - item(); + result.forEach((item) => { + item.apply(); - expect(mockRun).toHaveBeenCalledWith('node', [ resolvedPath ], pkg.location); + expect(mockRun).toHaveBeenCalledWith('node', [ resolvedPath ], item.pkg.location); }); }); }); diff --git a/test/src/tasks/version.spec.js b/test/src/tasks/version.spec.js index 3cc3b19..39bcd57 100644 --- a/test/src/tasks/version.spec.js +++ b/test/src/tasks/version.spec.js @@ -29,11 +29,12 @@ describe('src/tasks/version', () => { mockRun.mockReset(); }); - it('it returns an array of functions', () => { + it('it returns an array of tasks', () => { expect(result).toBeInstanceOf(Array); result.forEach((item) => { - expect(item).toBeInstanceOf(Function); + expect(item.pkg).toBeDefined(); + expect(item.apply).toEqual(expect.any(Function)); }); }); From 8941bb16a47d320b5794c7d5bc860ad829830479 Mon Sep 17 00:00:00 2001 From: Matt Hinchliffe Date: Fri, 7 Dec 2018 08:40:41 +0000 Subject: [PATCH 3/8] =?UTF-8?q?Refactor=20logic=20to=20fetch=20all=20depen?= =?UTF-8?q?dencies=20into=20package=20class=20=20=F0=9F=90=BF=20v2.10.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/package.js | 9 +++++++++ src/run-parallel.js | 9 +-------- src/sort-packages.js | 4 +--- test/src/package.spec.js | 18 +++++++++++++++++- 4 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/package.js b/src/package.js index a220d76..7c6c258 100644 --- a/src/package.js +++ b/src/package.js @@ -31,6 +31,15 @@ class Package { return path.relative(process.cwd(), this.location); } + get allDependencies () { + return Object.keys({ + ...this.manifest.dependencies, + ...this.manifest.devDependencies, + ...this.manifest.peerDependencies, + ...this.manifest.optionalDependencies + }); + }; + async writeManifest (manifest) { const json = JSON.stringify(manifest, null, 2); await writeFile(this.manifestLocation, json); diff --git a/src/run-parallel.js b/src/run-parallel.js index 3f7e7a3..a15ef18 100644 --- a/src/run-parallel.js +++ b/src/run-parallel.js @@ -15,18 +15,11 @@ module.exports = (tasks = [], concurrency = 1) => { return Promise.all( tasks.map(({ pkg, apply }) => { - const allDependencies = Object.keys({ - ...pkg.manifest.dependencies, - ...pkg.manifest.devDependencies, - ...pkg.manifest.peerDependencies, - ...pkg.manifest.optionalDependencies - }); - return semaphore .acquire() .then(() => { return waitUntil(() => { - return noRunningDependencies(packagesRunning, allDependencies); + return noRunningDependencies(packagesRunning, pkg.allDependencies); }); }) .then(() => { diff --git a/src/sort-packages.js b/src/sort-packages.js index 659a3bc..c3fabd1 100644 --- a/src/sort-packages.js +++ b/src/sort-packages.js @@ -14,9 +14,7 @@ module.exports = (reverse = false, packages = []) => { const packageNames = new Set(packages.map((pkg) => pkg.name)); const edges = packages.reduce((edges, pkg) => { - const dependencyNames = collateDependencies(pkg.manifest); - - const localDependencies = dependencyNames.filter((dependency) => { + const localDependencies = pkg.allDependencies.filter((dependency) => { return packageNames.has(dependency); }); diff --git a/test/src/package.spec.js b/test/src/package.spec.js index 3932d38..fe6cee3 100644 --- a/test/src/package.spec.js +++ b/test/src/package.spec.js @@ -6,6 +6,14 @@ const Subject = require('../../src/package'); const fixture = Object.freeze({ name: 'my-package', version: '0.0.0', + dependencies: { + lodash: '^3.0.0', + hyperons: '^0.5.0' + }, + devDependencies: { + jest: '^16.0.0', + prettier: '^12.0.0' + } }); describe('src/package', () => { @@ -53,6 +61,14 @@ describe('src/package', () => { }); }); + describe('get #allDependencies', () => { + it('returns a list of all dependencies', () => { + const instance = factory(fixture); + expect(instance.allDependencies).toBeInstanceOf(Array); + expect(instance.allDependencies.length).toEqual(4); + }); + }) + describe('#writeManifest', () => { beforeEach(() => { // The final arg is a callback that needs calling! @@ -65,7 +81,7 @@ describe('src/package', () => { expect(fs.writeFile).toHaveBeenCalledWith( '/root/path/to/package/package.json', - JSON.stringify({ name : 'my-package', version: '1.0.0' }, null, 2), + JSON.stringify({ ...fixture, version: '1.0.0' }, null, 2), expect.any(Function) ); }); From 64c637b4bef63a34bd0e08676a50458775142f3c Mon Sep 17 00:00:00 2001 From: Matt Hinchliffe Date: Fri, 7 Dec 2018 08:55:02 +0000 Subject: [PATCH 4/8] Refactor specs to use shared package helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🐿 v2.10.0 --- src/sort-packages.js | 10 ------- test/helpers/create-package.js | 10 +++++++ test/src/package.spec.js | 2 +- test/src/sort-packages.spec.js | 50 +++++++++++++--------------------- test/src/tasks/exec.spec.js | 8 +----- test/src/tasks/publish.spec.js | 9 +----- test/src/tasks/run.spec.js | 15 +++------- test/src/tasks/script.spec.js | 8 +----- test/src/tasks/version.spec.js | 8 +----- 9 files changed, 38 insertions(+), 82 deletions(-) create mode 100644 test/helpers/create-package.js diff --git a/src/sort-packages.js b/src/sort-packages.js index c3fabd1..b1ef7c7 100644 --- a/src/sort-packages.js +++ b/src/sort-packages.js @@ -1,15 +1,5 @@ const toposort = require('toposort'); -const collateDependencies = (manifest) => { - // TODO: refactor into package class - return Object.keys({ - ...manifest.dependencies, - ...manifest.devDependencies, - ...manifest.peerDependencies, - ...manifest.optionalDependencies - }); -}; - module.exports = (reverse = false, packages = []) => { const packageNames = new Set(packages.map((pkg) => pkg.name)); diff --git a/test/helpers/create-package.js b/test/helpers/create-package.js new file mode 100644 index 0000000..6be7e86 --- /dev/null +++ b/test/helpers/create-package.js @@ -0,0 +1,10 @@ +const Package = require('../../src/package'); + +module.exports = (name, options = {}) => { + const manifest = { name, ...options }; + const instance = new Package(manifest, `/Path/to/${name}`); + + instance.writeManifest = jest.fn(); + + return instance; +}; diff --git a/test/src/package.spec.js b/test/src/package.spec.js index fe6cee3..9ced93c 100644 --- a/test/src/package.spec.js +++ b/test/src/package.spec.js @@ -67,7 +67,7 @@ describe('src/package', () => { expect(instance.allDependencies).toBeInstanceOf(Array); expect(instance.allDependencies.length).toEqual(4); }); - }) + }); describe('#writeManifest', () => { beforeEach(() => { diff --git a/test/src/sort-packages.spec.js b/test/src/sort-packages.spec.js index 7dcbc16..d58b0ca 100644 --- a/test/src/sort-packages.spec.js +++ b/test/src/sort-packages.spec.js @@ -1,39 +1,27 @@ const subject = require('../../src/sort-packages'); +const createPackage = require('../helpers/create-package'); -const fixture = Object.freeze([ - { - name: 'foo', - manifest: { - dependencies: { - qux: '0.0.0' - } +const fixture = [ + createPackage('foo', { + dependencies: { + qux: '0.0.0' } - }, - { - name: 'bar', - manifest: { - dependencies: { - baz: '0.0.0' - } + }), + createPackage('bar', { + dependencies: { + baz: '0.0.0' } - }, - { - name: 'baz', - manifest: { - dependencies: { - foo: '0.0.0', - qux: '0.0.0' - } - }, - }, - { - name: 'qux', - manifest: { - dependencies: { - } + }), + createPackage('baz', { + dependencies: { + foo: '0.0.0', + qux: '0.0.0' } - } -]); + }), + createPackage('qux', { + dependencies: {} + }) +]; describe('src/sort-packages', () => { it('returns a new array', () => { diff --git a/test/src/tasks/exec.spec.js b/test/src/tasks/exec.spec.js index f8fb776..205aed8 100644 --- a/test/src/tasks/exec.spec.js +++ b/test/src/tasks/exec.spec.js @@ -2,13 +2,7 @@ const mockRun = jest.fn(); jest.mock('../../../src/run-package', () => mockRun); const { task: subject } = require('../../../src/tasks/exec'); - -const createPackage = (name) => ( - { - name, - location: `/Path/to/${name}` - } -); +const createPackage = require('../../helpers/create-package'); describe('src/tasks/exec', () => { const packages = [ diff --git a/test/src/tasks/publish.spec.js b/test/src/tasks/publish.spec.js index 88a2a5b..31edec0 100644 --- a/test/src/tasks/publish.spec.js +++ b/test/src/tasks/publish.spec.js @@ -2,14 +2,7 @@ const mockRun = jest.fn(); jest.mock('../../../src/run-package', () => mockRun); const { task: subject } = require('../../../src/tasks/publish'); - -const createPackage = (name, options = {}) => ( - { - name, - location: `/Path/to/${name}`, - ...options - } -); +const createPackage = require('../../helpers/create-package'); describe('src/tasks/publish', () => { const packages = [ diff --git a/test/src/tasks/run.spec.js b/test/src/tasks/run.spec.js index 5413f7b..392d643 100644 --- a/test/src/tasks/run.spec.js +++ b/test/src/tasks/run.spec.js @@ -2,20 +2,13 @@ const mockRun = jest.fn(); jest.mock('../../../src/run-package', () => mockRun); const { task: subject } = require('../../../src/tasks/run'); - -const createPackage = (name, options = {}) => ( - { - name, - location: `/Path/to/${name}`, - ...options - } -); +const createPackage = require('../../helpers/create-package'); describe('src/tasks/run', () => { const packages = [ - createPackage('foo', { manifest: { scripts: { test: '' } } }), - createPackage('bar', { manifest: { scripts: { test: '' } } }), - createPackage('baz', { manifest: {} }), + createPackage('foo', { scripts: { test: '' } }), + createPackage('bar', { scripts: { test: '' } }), + createPackage('baz', { scripts: {} }), ]; const command = 'test'; diff --git a/test/src/tasks/script.spec.js b/test/src/tasks/script.spec.js index 3d3da3f..0aa2638 100644 --- a/test/src/tasks/script.spec.js +++ b/test/src/tasks/script.spec.js @@ -2,13 +2,7 @@ const mockRun = jest.fn(); jest.mock('../../../src/run-package', () => mockRun); const { task: subject } = require('../../../src/tasks/script'); - -const createPackage = (name) => ( - { - name, - location: `/Path/to/${name}` - } -); +const createPackage = require('../../helpers/create-package'); describe('src/tasks/script', () => { const packages = [ diff --git a/test/src/tasks/version.spec.js b/test/src/tasks/version.spec.js index 39bcd57..4e0bd0b 100644 --- a/test/src/tasks/version.spec.js +++ b/test/src/tasks/version.spec.js @@ -2,13 +2,7 @@ const mockRun = jest.fn(); jest.mock('../../../src/run-package', () => mockRun); const { task: subject } = require('../../../src/tasks/version'); - -const createPackage = (name) => ( - { - name, - location: `/Path/to/${name}` - } -); +const createPackage = require('../../helpers/create-package'); describe('src/tasks/version', () => { const packages = [ From cda9d8d64f5cffc46193827b5ecf954fb0051156 Mon Sep 17 00:00:00 2001 From: Matt Hinchliffe Date: Fri, 7 Dec 2018 11:26:02 +0000 Subject: [PATCH 5/8] =?UTF-8?q?Refactor=20queue=20logic=20from=20a=20polle?= =?UTF-8?q?r=20(wait=20until...)=20to=20use=20an=20event=20emitter=20=20?= =?UTF-8?q?=F0=9F=90=BF=20v2.10.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/event-queue.js | 37 +++++++++++++++++++++++++++++++++++++ src/run-parallel.js | 19 +++++++------------ src/wait-until.js | 15 --------------- 3 files changed, 44 insertions(+), 27 deletions(-) create mode 100644 src/event-queue.js delete mode 100644 src/wait-until.js diff --git a/src/event-queue.js b/src/event-queue.js new file mode 100644 index 0000000..fe40e62 --- /dev/null +++ b/src/event-queue.js @@ -0,0 +1,37 @@ +const EventEmitter = require('events'); + +class EventQueue extends EventEmitter { + constructor() { + super(); + this.queue = new Set(); + } + + add(item) { + this.queue.add(item); + this.emit('queue:add', item); + } + + delete(item) { + this.queue.delete(item); + this.emit('queue:delete', item); + } + + // Resolves when the queue contains none of the given items + done(items = []) { + return new Promise((resolve) => { + const callback = () => { + const noItemsRunning = items.every((item) => !this.queue.has(item)); + + if (noItemsRunning) { + this.off('queue:delete', callback); + resolve(); + } + }; + + this.on('queue:delete', callback); + callback(); + }); + } +} + +module.exports = EventQueue; diff --git a/src/run-parallel.js b/src/run-parallel.js index a15ef18..bd95d15 100644 --- a/src/run-parallel.js +++ b/src/run-parallel.js @@ -1,33 +1,28 @@ const Semaphore = require('async-sema'); const logger = require('./logger'); -const waitUntil = require('./wait-until'); - -const noRunningDependencies = (running, dependencies) => { - return !dependencies.some((dependency) => running.has(dependency)); -}; +const EventQueue = require('./event-queue'); module.exports = (tasks = [], concurrency = 1) => { const semaphore = new Semaphore(concurrency); + const queue = new EventQueue(); logger.info(`Executing up to ${concurrency} tasks at a time`); - const packagesRunning = new Set(); - return Promise.all( tasks.map(({ pkg, apply }) => { return semaphore .acquire() .then(() => { - return waitUntil(() => { - return noRunningDependencies(packagesRunning, pkg.allDependencies); - }); + // Queue the package now to maintain running order... + queue.add(pkg.name); + // ...but wait for any dependencies in the queue to finish + return queue.done(pkg.allDependencies); }) .then(() => { - packagesRunning.add(pkg.name); return apply(); }) .then(() => { - packagesRunning.delete(pkg.name); + queue.delete(pkg.name); return semaphore.release(); }); }) diff --git a/src/wait-until.js b/src/wait-until.js deleted file mode 100644 index edd0faa..0000000 --- a/src/wait-until.js +++ /dev/null @@ -1,15 +0,0 @@ -const wait = (ms = 100) => new Promise((resolve) => setTimeout(resolve, ms)); - -module.exports = (check) => { - return new Promise((resolve) => { - const test = () => { - if (check()) { - resolve(); - } else { - wait().then(test); - } - }; - - test(); - }); -}; From f74bb551c7ac5596ad75c0bb7388ff0420af791d Mon Sep 17 00:00:00 2001 From: Matt Hinchliffe Date: Fri, 7 Dec 2018 14:31:22 +0000 Subject: [PATCH 6/8] Add a spec for evented queue module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🐿 v2.10.0 --- src/event-queue.js | 20 ++++++------ src/run-parallel.js | 2 +- test/src/event-queue.spec.js | 61 ++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 11 deletions(-) create mode 100644 test/src/event-queue.spec.js diff --git a/src/event-queue.js b/src/event-queue.js index fe40e62..5f775dc 100644 --- a/src/event-queue.js +++ b/src/event-queue.js @@ -8,28 +8,28 @@ class EventQueue extends EventEmitter { add(item) { this.queue.add(item); - this.emit('queue:add', item); + this.emit('add', item); } delete(item) { this.queue.delete(item); - this.emit('queue:delete', item); + this.emit('delete', item); } - // Resolves when the queue contains none of the given items - done(items = []) { + waitFor(items = []) { return new Promise((resolve) => { - const callback = () => { - const noItemsRunning = items.every((item) => !this.queue.has(item)); + const callback = (item) => { + const itemsRunning = items.some((item) => this.queue.has(item)); - if (noItemsRunning) { - this.off('queue:delete', callback); + if (!itemsRunning) { + this.off('delete', callback); resolve(); } }; - this.on('queue:delete', callback); - callback(); + this.on('delete', callback); + + callback(null); }); } } diff --git a/src/run-parallel.js b/src/run-parallel.js index bd95d15..eb27a38 100644 --- a/src/run-parallel.js +++ b/src/run-parallel.js @@ -16,7 +16,7 @@ module.exports = (tasks = [], concurrency = 1) => { // Queue the package now to maintain running order... queue.add(pkg.name); // ...but wait for any dependencies in the queue to finish - return queue.done(pkg.allDependencies); + return queue.waitFor(pkg.allDependencies); }) .then(() => { return apply(); diff --git a/test/src/event-queue.spec.js b/test/src/event-queue.spec.js new file mode 100644 index 0000000..d15a37c --- /dev/null +++ b/test/src/event-queue.spec.js @@ -0,0 +1,61 @@ +const Subject = require('../../src/event-queue'); + +describe('src/event-queue', () => { + let instance; + + beforeEach(() => { + instance = new Subject(); + }); + + describe('#add', () => { + it('adds the given item to the queue', () => { + instance.add('foo'); + expect(instance.queue.size).toEqual(1); + }); + + it('emits an event when items are added to the queue', (done) => { + instance.on('add', () => { + done(); + }); + + instance.add('foo'); + }); + }); + + describe('#delete', () => { + it('removes the given item to the queue', () => { + instance.add('foo'); + expect(instance.queue.size).toEqual(1); + + instance.delete('foo'); + expect(instance.queue.size).toEqual(0); + }); + + it('emits an event when items are removed from the queue', (done) => { + instance.add('foo'); + + instance.on('delete', (item) => { + done(); + }); + + instance.delete('foo'); + }); + }); + + describe('#waitFor', () => { + it('resolves when the queue no longer contains any of the given items', (done) => { + instance.add('foo'); + instance.add('bar'); + instance.add('baz'); + + instance.waitFor(['foo', 'bar', 'baz']).then(() => { + expect(instance.queue.size).toEqual(0); + done(); + }); + + instance.delete('foo'); + instance.delete('bar'); + instance.delete('baz'); + }); + }); +}); From c096ff54ad7df2b46e77827da7d6f4dd2e035194 Mon Sep 17 00:00:00 2001 From: Matt Hinchliffe Date: Wed, 12 Dec 2018 08:33:42 +0000 Subject: [PATCH 7/8] =?UTF-8?q?Ensure=20Node=208=20support=20by=20switchin?= =?UTF-8?q?g=20from=20eventemitter.off()=20to=20eventemitter.removeEvent()?= =?UTF-8?q?=20=20=F0=9F=90=BF=20v2.10.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/{event-queue.js => evented-queue.js} | 10 ++++++---- src/run-parallel.js | 4 ++-- ...nt-queue.spec.js => evented-queue.spec.js} | 20 ++++++++++--------- 3 files changed, 19 insertions(+), 15 deletions(-) rename src/{event-queue.js => evented-queue.js} (73%) rename test/src/{event-queue.spec.js => evented-queue.spec.js} (78%) diff --git a/src/event-queue.js b/src/evented-queue.js similarity index 73% rename from src/event-queue.js rename to src/evented-queue.js index 5f775dc..a84d6af 100644 --- a/src/event-queue.js +++ b/src/evented-queue.js @@ -1,6 +1,6 @@ const EventEmitter = require('events'); -class EventQueue extends EventEmitter { +class EventedQueue extends EventEmitter { constructor() { super(); this.queue = new Set(); @@ -9,20 +9,22 @@ class EventQueue extends EventEmitter { add(item) { this.queue.add(item); this.emit('add', item); + return this; } delete(item) { this.queue.delete(item); this.emit('delete', item); + return this; } waitFor(items = []) { return new Promise((resolve) => { - const callback = (item) => { + const callback = () => { const itemsRunning = items.some((item) => this.queue.has(item)); if (!itemsRunning) { - this.off('delete', callback); + this.removeListener('delete', callback); resolve(); } }; @@ -34,4 +36,4 @@ class EventQueue extends EventEmitter { } } -module.exports = EventQueue; +module.exports = EventedQueue; diff --git a/src/run-parallel.js b/src/run-parallel.js index eb27a38..c49a3b6 100644 --- a/src/run-parallel.js +++ b/src/run-parallel.js @@ -1,10 +1,10 @@ const Semaphore = require('async-sema'); const logger = require('./logger'); -const EventQueue = require('./event-queue'); +const EventedQueue = require('./evented-queue'); module.exports = (tasks = [], concurrency = 1) => { const semaphore = new Semaphore(concurrency); - const queue = new EventQueue(); + const queue = new EventedQueue(); logger.info(`Executing up to ${concurrency} tasks at a time`); diff --git a/test/src/event-queue.spec.js b/test/src/evented-queue.spec.js similarity index 78% rename from test/src/event-queue.spec.js rename to test/src/evented-queue.spec.js index d15a37c..0a4feb0 100644 --- a/test/src/event-queue.spec.js +++ b/test/src/evented-queue.spec.js @@ -1,6 +1,6 @@ -const Subject = require('../../src/event-queue'); +const Subject = require('../../src/evented-queue'); -describe('src/event-queue', () => { +describe('src/evented-queue', () => { let instance; beforeEach(() => { @@ -34,7 +34,7 @@ describe('src/event-queue', () => { it('emits an event when items are removed from the queue', (done) => { instance.add('foo'); - instance.on('delete', (item) => { + instance.on('delete', () => { done(); }); @@ -44,18 +44,20 @@ describe('src/event-queue', () => { describe('#waitFor', () => { it('resolves when the queue no longer contains any of the given items', (done) => { - instance.add('foo'); - instance.add('bar'); - instance.add('baz'); + instance + .add('foo') + .add('bar') + .add('baz'); instance.waitFor(['foo', 'bar', 'baz']).then(() => { expect(instance.queue.size).toEqual(0); done(); }); - instance.delete('foo'); - instance.delete('bar'); - instance.delete('baz'); + instance + .delete('foo') + .delete('bar') + .delete('baz'); }); }); }); From f94c7e9e2ed1e4f5814a36a9be9d7008f00a6b70 Mon Sep 17 00:00:00 2001 From: Matt Hinchliffe Date: Thu, 13 Dec 2018 12:06:13 +0000 Subject: [PATCH 8/8] =?UTF-8?q?Add=20--preserve-order=20flag=20and=20refac?= =?UTF-8?q?tor=20queue=20to=20only=20enable=20waiting=20when=20the=20flag?= =?UTF-8?q?=20is=20on=20=20=F0=9F=90=BF=20v2.10.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- readme.md | 27 +++++++++++++++++---------- src/bin/cli | 6 +++++- src/cli-task.js | 2 +- src/run-parallel.js | 10 +++++----- 4 files changed, 28 insertions(+), 17 deletions(-) diff --git a/readme.md b/readme.md index 99c8d39..92ded58 100644 --- a/readme.md +++ b/readme.md @@ -34,21 +34,21 @@ _Please note:_ Before executing a command Athloi will sort the packages [topolog ### exec -Runs an arbitrary command in the scope of each package. +Runs an arbitrary command within the scope of each package. ```sh athloi exec npm install ``` -A double-dash (`--`) is necessary to pass any dashed arguments to the script being executed. +A double-dash (`--`) is necessary to pass any dashed arguments to the command being executed. ```sh -athloi exec -- npm i -D +athloi exec -- npm i -D lodash ``` ### run -Runs an [npm script] in each package that contains that script. +Runs an [npm script] in each package that defines that script. ```sh athloi run build @@ -66,7 +66,7 @@ athloi script path/to/task.js ### version -Updates the release number for all packages and writes the new data back to `package.json`. The given tag must parseable as a valid semver number. +Updates the release number for all public packages and writes the new data back to `package.json`. The given tag must parseable as a valid semver number. ```sh athloi version v1.0.0 @@ -93,18 +93,25 @@ athloi publish -- --access=public ### concurrency -A global concurrency option which can be used to execute multiple tasks in parallel. By default only one task will run at a time. +A global option which will execute up to the given number of tasks concurrently. By default one task will be run at a time. ```sh -# run a build script 3 packages at a time -athloi run build --concurrency 3 +# run a lint script in up to 3 packages at a time +athloi run lint --concurrency 3 ``` -_Please note:_ using a concurrency value higher than 1 no longer ensures that tasks will finish for packages which are dependencies of other packages. +### preserve-order + +A global flag which will ensure tasks maintain topological sort order. When used with a concurrency value higher than 1 this option will force queued tasks to wait for any still running tasks in cross-dependent packages to finish first. + +```sh +# run a concurrent build script but ensure dependencies are built first +athloi run build --concurrency 5 --preserve-order +``` ### filter -A global filter option which can be used for all tasks. It can filter packages based on the value of a field within their package manifest. +A global option which can be used for all tasks. It filters packages based on the value of a field within their package manifest or the package name. ```sh # Run a build script in only the packages marked as private diff --git a/src/bin/cli b/src/bin/cli index 9700498..050d132 100755 --- a/src/bin/cli +++ b/src/bin/cli @@ -12,7 +12,11 @@ program .option( '-C, --concurrency ', 'Number of tasks to be run concurrently', - (arg) => /\d/.test(arg) ? parseInt(arg, 10) : undefined + (arg) => /^\d$/.test(arg) ? parseInt(arg, 10) : null + ) + .option( + '-P, --preserve-order', + 'Preserve topological sort order when running tasks concurrently' ) .option( '-R, --reverse', diff --git a/src/cli-task.js b/src/cli-task.js index a217174..f4d8ce1 100644 --- a/src/cli-task.js +++ b/src/cli-task.js @@ -39,7 +39,7 @@ module.exports = (task) => { logger.info(`Running ${tasks.length} tasks`); // 6. execute all tasks - await runParallel(tasks, globals.concurrency); + await runParallel(tasks, globals.concurrency, globals.preserveOrder); timer.stop(); diff --git a/src/run-parallel.js b/src/run-parallel.js index c49a3b6..5e698de 100644 --- a/src/run-parallel.js +++ b/src/run-parallel.js @@ -2,7 +2,7 @@ const Semaphore = require('async-sema'); const logger = require('./logger'); const EventedQueue = require('./evented-queue'); -module.exports = (tasks = [], concurrency = 1) => { +module.exports = (tasks = [], concurrency = 1, preserveOrder = false) => { const semaphore = new Semaphore(concurrency); const queue = new EventedQueue(); @@ -10,13 +10,13 @@ module.exports = (tasks = [], concurrency = 1) => { return Promise.all( tasks.map(({ pkg, apply }) => { + queue.add(pkg.name); + return semaphore .acquire() .then(() => { - // Queue the package now to maintain running order... - queue.add(pkg.name); - // ...but wait for any dependencies in the queue to finish - return queue.waitFor(pkg.allDependencies); + // wait for any dependencies still in the queue to finish + return preserveOrder ? queue.waitFor(pkg.allDependencies) : null; }) .then(() => { return apply();