Skip to content

Commit

Permalink
Merge branch 'main' into issues-433-and-452
Browse files Browse the repository at this point in the history
  • Loading branch information
gustavohenke authored Feb 25, 2024
2 parents 6e1407b + fd21485 commit 3552057
Show file tree
Hide file tree
Showing 27 changed files with 472 additions and 49 deletions.
6 changes: 5 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,13 @@ jobs:
- name: Install dependencies
run: pnpm install && pnpm add --global concurrently

- name: Build & Test
- name: Build & Unit Test
run: concurrently --prefix none --group "pnpm:build" "pnpm:test"

- name: Smoke Test
# Don't collect coverage here as it overrides the unit test's coverage
run: pnpm test:smoke --coverage=false

- name: Submit coverage
uses: coverallsapp/github-action@master
continue-on-error: true
Expand Down
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Outputs
/dist
dist

# Logs
*.log
Expand Down
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
"[javascript, typescript]": {
"editor.formatOnSave": false
},
"editor.rulers": [100]
"editor.rulers": [100],
"typescript.tsdk": "node_modules/typescript/lib"
}
9 changes: 5 additions & 4 deletions bin/concurrently.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import yargs from 'yargs';
import { hideBin } from 'yargs/helpers';

import * as defaults from '../src/defaults';
import concurrently from '../src/index';
import { concurrently } from '../src/index';
import { epilogue } from './epilogue';

// Clean-up arguments (yargs expects only the arguments after the program name)
Expand Down Expand Up @@ -165,9 +165,9 @@ const args = yargs(argsBeforeSep)
type: 'number',
},
'restart-after': {
describe: 'Delay time to respawn the process, in milliseconds.',
describe: 'Delay before restarting the process, in milliseconds, or "exponential".',
default: defaults.restartDelay,
type: 'number',
type: 'string',
},

// Input
Expand Down Expand Up @@ -223,7 +223,8 @@ concurrently(
prefix: args.prefix,
prefixColors: args.prefixColors.split(','),
prefixLength: args.prefixLength,
restartDelay: args.restartAfter,
restartDelay:
args.restartAfter === 'exponential' ? 'exponential' : Number(args.restartAfter),
restartTries: args.restartTries,
successCondition: args.success,
timestampFormat: args.timestampFormat,
Expand Down
7 changes: 7 additions & 0 deletions index.d.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/*
* While in local development, make sure you've run `pnpm run build` first.
*/
import { concurrently } from './dist/src/index.js';

export * from './dist/src/index.js';
export default concurrently;
11 changes: 11 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* While in local development, make sure you've run `pnpm run build` first.
*/
import { concurrently } from './dist/src/index.js';

export * from './dist/src/index.js';
// @ts-expect-error ignore the usage of `export =` along with `export default`.
// This is seemingly fine, but the file needs to be included in the TS project, which can't be done
// due to importing from `dist`. See https://stackoverflow.com/q/42609768/2083599
export = concurrently;
export default concurrently;
7 changes: 6 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,10 @@
// eslint-disable-next-line @typescript-eslint/no-var-requires
const concurrently = require('./dist/src/index.js');

module.exports = exports = concurrently.default;
// For require()
module.exports = exports = concurrently.concurrently;

// For TS + import syntax; mimics `export default`
exports.default = exports;

Object.assign(exports, concurrently);
4 changes: 2 additions & 2 deletions index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
* While in local development, make sure you've run `pnpm run build` first.
*/

import concurrently from './dist/src/index.js';
import { concurrently } from './dist/src/index.js';

// NOTE: the star reexport doesn't work in Node <12.20, <14.13 and <15.
export * from './dist/src/index.js';

export default concurrently.default;
export default concurrently;
29 changes: 24 additions & 5 deletions jest.config.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,30 @@
/** @type {import('@jest/types').Config.InitialOptions} */
const config = {
// Need to extend from a base config because projects don't inherit configurations as documented
// https://github.com/jestjs/jest/issues/11411
/** @type {import('@jest/types').Config.InitialProjectOptions} */
const baseConfig = {
transform: {
'^.+\\.(t|j)sx?$': ['@swc/jest'],
'.*': ['@swc/jest'],
},
collectCoverage: true,
collectCoverageFrom: ['src/**/*.ts', '!src/index.ts'],
testPathIgnorePatterns: ['/node_modules/', '/dist'],
};

/** @type {import('@jest/types').Config.InitialOptions} */
const config = {
collectCoverage: true,
projects: [
{
...baseConfig,
displayName: 'unit',
collectCoverageFrom: ['src/**/*.ts', '!src/index.ts'],
// (src|bin) doesn't seem to work on Windows
testMatch: ['src', 'bin'].map((dir) => `<rootDir>/${dir}/**/*.spec.ts`),
},
{
...baseConfig,
displayName: 'smoke',
testMatch: ['<rootDir>/tests/**/*.spec.ts'],
},
],
};

module.exports = config;
19 changes: 13 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"version": "8.2.2",
"description": "Run commands concurrently",
"main": "index.js",
"types": "dist/src/index.d.ts",
"types": "index.d.ts",
"type": "commonjs",
"bin": {
"concurrently": "./dist/bin/concurrently.js",
Expand All @@ -14,10 +14,14 @@
},
"exports": {
".": {
"types": "./dist/src/index.d.ts",
"import": "./index.mjs",
"require": "./index.js",
"default": "./index.js"
"import": {
"types": "./index.d.mts",
"default": "./index.mjs"
},
"require": {
"types": "./index.d.ts",
"default": "./index.js"
}
},
"./package.json": "./package.json"
},
Expand All @@ -31,7 +35,8 @@
"lint:fix": "pnpm run lint --fix",
"prepublishOnly": "safe-publish-latest && pnpm run build",
"report-coverage": "cat coverage/lcov.info | coveralls",
"test": "jest",
"test": "jest --selectProjects unit",
"test:smoke": "jest --selectProjects smoke",
"prepare": "husky install"
},
"repository": {
Expand Down Expand Up @@ -94,7 +99,9 @@
"files": [
"dist",
"index.js",
"index.d.ts",
"index.mjs",
"index.d.mts",
"!**/fixtures",
"!**/*.spec.js",
"!**/*.spec.d.ts"
Expand Down
8 changes: 8 additions & 0 deletions src/command.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,14 @@ describe('#start()', () => {
expect(command.state).toBe('exited');
});

it('does not change state if there was an error', () => {
const { command } = createCommand();
command.start();
process.emit('error', 'foo');
process.emit('close', 0, null);
expect(command.state).toBe('errored');
});

it('shares start and close timing events to the timing stream', async () => {
const { command, values } = createCommand();
const startDate = new Date();
Expand Down
6 changes: 5 additions & 1 deletion src/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,11 @@ export class Command implements CommandInfo {
.pipe(Rx.map((event) => event as [number | null, NodeJS.Signals | null]))
.subscribe(([exitCode, signal]) => {
this.process = undefined;
this.state = 'exited';

// Don't override error event
if (this.state !== 'errored') {
this.state = 'exited';
}

const endDate = new Date(Date.now());
this.timer.next({ startDate, endDate });
Expand Down
46 changes: 41 additions & 5 deletions src/flow-control/restart-process.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createMockInstance } from 'jest-create-mock-instance';
import { TestScheduler } from 'rxjs/testing';
import { VirtualTimeScheduler } from 'rxjs';

import { createFakeCloseEvent, FakeCommand } from '../fixtures/fake-command';
import { Logger } from '../logger';
Expand All @@ -8,12 +8,14 @@ import { RestartProcess } from './restart-process';
let commands: FakeCommand[];
let controller: RestartProcess;
let logger: Logger;
let scheduler: TestScheduler;
let scheduler: VirtualTimeScheduler;
beforeEach(() => {
commands = [new FakeCommand(), new FakeCommand()];

logger = createMockInstance(Logger);
scheduler = new TestScheduler(() => true);

// Don't use TestScheduler as it's hardcoded to a max number of "frames" (time),
// which don't work for some tests in this suite
scheduler = new VirtualTimeScheduler();
controller = new RestartProcess({
logger,
scheduler,
Expand All @@ -34,14 +36,31 @@ it('does not restart processes that complete with success', () => {
expect(commands[1].start).toHaveBeenCalledTimes(0);
});

it('restarts processes that fail after delay has passed', () => {
it('restarts processes that fail immediately, if no delay was passed', () => {
controller = new RestartProcess({ logger, scheduler, tries: 1 });
controller.handle(commands);

commands[0].close.next(createFakeCloseEvent({ exitCode: 1 }));
scheduler.flush();

expect(scheduler.now()).toBe(0);
expect(logger.logCommandEvent).toHaveBeenCalledTimes(1);
expect(logger.logCommandEvent).toHaveBeenCalledWith(
`${commands[0].command} restarted`,
commands[0],
);
expect(commands[0].start).toHaveBeenCalledTimes(1);
});

it('restarts processes that fail after delay ms has passed', () => {
controller.handle(commands);

commands[0].close.next(createFakeCloseEvent({ exitCode: 1 }));
commands[1].close.next(createFakeCloseEvent({ exitCode: 0 }));

scheduler.flush();

expect(scheduler.now()).toBe(100);
expect(logger.logCommandEvent).toHaveBeenCalledTimes(1);
expect(logger.logCommandEvent).toHaveBeenCalledWith(
`${commands[0].command} restarted`,
Expand All @@ -51,6 +70,23 @@ it('restarts processes that fail after delay has passed', () => {
expect(commands[1].start).not.toHaveBeenCalled();
});

it('restarts processes that fail with an exponential back-off', () => {
const tries = 4;
controller = new RestartProcess({ logger, scheduler, tries, delay: 'exponential' });
controller.handle(commands);

let time = 0;
for (let i = 0; i < tries; i++) {
commands[0].close.next(createFakeCloseEvent({ exitCode: 1 }));
scheduler.flush();

time += Math.pow(2, i) * 1000;
expect(scheduler.now()).toBe(time);
expect(logger.logCommandEvent).toHaveBeenCalledTimes(i + 1);
expect(commands[0].start).toHaveBeenCalledTimes(i + 1);
}
});

it('restarts processes up to tries', () => {
controller.handle(commands);

Expand Down
18 changes: 13 additions & 5 deletions src/flow-control/restart-process.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import * as Rx from 'rxjs';
import { defaultIfEmpty, delay, filter, map, skip, take, takeWhile } from 'rxjs/operators';
import { defaultIfEmpty, delayWhen, filter, map, skip, take, takeWhile } from 'rxjs/operators';

import { Command } from '../command';
import * as defaults from '../defaults';
import { Logger } from '../logger';
import { FlowController } from './flow-controller';

export type RestartDelay = number | 'exponential';

/**
* Restarts commands that fail up to a defined number of times.
*/
export class RestartProcess implements FlowController {
private readonly logger: Logger;
private readonly scheduler?: Rx.SchedulerLike;
readonly delay: number;
private readonly delay: RestartDelay;
readonly tries: number;

constructor({
Expand All @@ -21,13 +23,13 @@ export class RestartProcess implements FlowController {
logger,
scheduler,
}: {
delay?: number;
delay?: RestartDelay;
tries?: number;
logger: Logger;
scheduler?: Rx.SchedulerLike;
}) {
this.logger = logger;
this.delay = delay != null ? +delay : defaults.restartDelay;
this.delay = delay ?? 0;
this.tries = tries != null ? +tries : defaults.restartTries;
this.tries = this.tries < 0 ? Infinity : this.tries;
this.scheduler = scheduler;
Expand All @@ -38,6 +40,12 @@ export class RestartProcess implements FlowController {
return { commands };
}

const delayOperator = delayWhen((_, index) => {
const { delay } = this;
const value = delay === 'exponential' ? Math.pow(2, index) * 1000 : delay;
return Rx.timer(value, this.scheduler);
});

commands
.map((command) =>
command.close.pipe(
Expand All @@ -50,7 +58,7 @@ export class RestartProcess implements FlowController {
// Delay the emission (so that the restarts happen on time),
// explicitly telling the subscriber that a restart is needed
failure.pipe(
delay(this.delay, this.scheduler),
delayOperator,
map(() => true),
),
// Skip the first N emissions (as these would be duplicates of the above),
Expand Down
Loading

0 comments on commit 3552057

Please sign in to comment.