From f7990dbaf1a5717f427a9fc0ce27537454708752 Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Tue, 12 Dec 2023 16:09:26 +1100 Subject: [PATCH 1/9] kill-on-signal: use describe.each() for signal testing --- src/flow-control/kill-on-signal.spec.ts | 34 +++++++------------------ 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/src/flow-control/kill-on-signal.spec.ts b/src/flow-control/kill-on-signal.spec.ts index 0780fcc5..e0887233 100644 --- a/src/flow-control/kill-on-signal.spec.ts +++ b/src/flow-control/kill-on-signal.spec.ts @@ -51,29 +51,13 @@ it('returns commands that keep non-SIGINT exit codes', () => { expect(callback).toHaveBeenCalledWith(expect.objectContaining({ exitCode: 1 })); }); -it('kills all commands on SIGINT', () => { - controller.handle(commands); - process.emit('SIGINT'); - - expect(process.listenerCount('SIGINT')).toBe(1); - expect(commands[0].kill).toHaveBeenCalledWith('SIGINT'); - expect(commands[1].kill).toHaveBeenCalledWith('SIGINT'); -}); - -it('kills all commands on SIGTERM', () => { - controller.handle(commands); - process.emit('SIGTERM'); - - expect(process.listenerCount('SIGTERM')).toBe(1); - expect(commands[0].kill).toHaveBeenCalledWith('SIGTERM'); - expect(commands[1].kill).toHaveBeenCalledWith('SIGTERM'); -}); - -it('kills all commands on SIGHUP', () => { - controller.handle(commands); - process.emit('SIGHUP'); - - expect(process.listenerCount('SIGHUP')).toBe(1); - expect(commands[0].kill).toHaveBeenCalledWith('SIGHUP'); - expect(commands[1].kill).toHaveBeenCalledWith('SIGHUP'); +describe.each(['SIGINT', 'SIGTERM', 'SIGHUP'])('on %s', (signal) => { + it('kills all commands', () => { + controller.handle(commands); + process.emit(signal); + + expect(process.listenerCount(signal)).toBe(1); + expect(commands[0].kill).toHaveBeenCalledWith(signal); + expect(commands[1].kill).toHaveBeenCalledWith(signal); + }); }); From 5820f80519dd8aa488a58fa61cd53631475a99c0 Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Tue, 12 Dec 2023 16:47:56 +1100 Subject: [PATCH 2/9] kill-on-signal: call abort controller on signal --- src/flow-control/kill-on-signal.spec.ts | 11 ++++++++++- src/flow-control/kill-on-signal.ts | 11 ++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/flow-control/kill-on-signal.spec.ts b/src/flow-control/kill-on-signal.spec.ts index e0887233..020e3ca8 100644 --- a/src/flow-control/kill-on-signal.spec.ts +++ b/src/flow-control/kill-on-signal.spec.ts @@ -7,10 +7,12 @@ import { KillOnSignal } from './kill-on-signal'; let commands: Command[]; let controller: KillOnSignal; let process: EventEmitter; +let abortController: AbortController; beforeEach(() => { process = new EventEmitter(); commands = [new FakeCommand(), new FakeCommand()]; - controller = new KillOnSignal({ process }); + abortController = new AbortController(); + controller = new KillOnSignal({ process, abortController }); }); it('returns commands that keep non-close streams from original commands', () => { @@ -60,4 +62,11 @@ describe.each(['SIGINT', 'SIGTERM', 'SIGHUP'])('on %s', (signal) => { expect(commands[0].kill).toHaveBeenCalledWith(signal); expect(commands[1].kill).toHaveBeenCalledWith(signal); }); + + it('sends abort signal', () => { + controller.handle(commands); + process.emit(signal); + + expect(abortController.signal.aborted).toBe(true); + }); }); diff --git a/src/flow-control/kill-on-signal.ts b/src/flow-control/kill-on-signal.ts index 599409aa..eff7eca9 100644 --- a/src/flow-control/kill-on-signal.ts +++ b/src/flow-control/kill-on-signal.ts @@ -10,9 +10,17 @@ import { FlowController } from './flow-controller'; */ export class KillOnSignal implements FlowController { private readonly process: EventEmitter; + private readonly abortController?: AbortController; - constructor({ process }: { process: EventEmitter }) { + constructor({ + process, + abortController, + }: { + process: EventEmitter; + abortController?: AbortController; + }) { this.process = process; + this.abortController = abortController; } handle(commands: Command[]) { @@ -20,6 +28,7 @@ export class KillOnSignal implements FlowController { (['SIGINT', 'SIGTERM', 'SIGHUP'] as NodeJS.Signals[]).forEach((signal) => { this.process.on(signal, () => { caughtSignal = signal; + this.abortController?.abort(); commands.forEach((command) => command.kill(signal)); }); }); From d8deb095bd3a43e21e6737ee1c15a57bee855da8 Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Tue, 12 Dec 2023 17:06:27 +1100 Subject: [PATCH 3/9] kill-others: use describe.each() for condition testing --- src/flow-control/kill-others.spec.ts | 62 ++++++++++------------------ 1 file changed, 22 insertions(+), 40 deletions(-) diff --git a/src/flow-control/kill-others.spec.ts b/src/flow-control/kill-others.spec.ts index a96e9179..7dbded77 100644 --- a/src/flow-control/kill-others.spec.ts +++ b/src/flow-control/kill-others.spec.ts @@ -41,26 +41,30 @@ it('does not kill others if condition does not match', () => { expect(commands[1].kill).not.toHaveBeenCalled(); }); -it('kills other killable processes on success', () => { - createWithConditions(['success']).handle(commands); - commands[1].isKillable = true; - commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); - - expect(logger.logGlobalEvent).toHaveBeenCalledTimes(1); - expect(logger.logGlobalEvent).toHaveBeenCalledWith('Sending SIGTERM to other processes..'); - expect(commands[0].kill).not.toHaveBeenCalled(); - expect(commands[1].kill).toHaveBeenCalledWith(undefined); -}); +describe.each(['success', 'failure'] as const)('on %s', (condition) => { + const exitCode = condition === 'success' ? 0 : 1; + + it('kills other killable processes', () => { + createWithConditions([condition]).handle(commands); + commands[1].isKillable = true; + commands[0].close.next(createFakeCloseEvent({ exitCode })); + + expect(logger.logGlobalEvent).toHaveBeenCalledTimes(1); + expect(logger.logGlobalEvent).toHaveBeenCalledWith('Sending SIGTERM to other processes..'); + expect(commands[0].kill).not.toHaveBeenCalled(); + expect(commands[1].kill).toHaveBeenCalledWith(undefined); + }); -it('kills other killable processes on success, with specified signal', () => { - createWithConditions(['success'], 'SIGKILL').handle(commands); - commands[1].isKillable = true; - commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); + it('kills other killable processes on success, with specified signal', () => { + createWithConditions([condition], 'SIGKILL').handle(commands); + commands[1].isKillable = true; + commands[0].close.next(createFakeCloseEvent({ exitCode })); - expect(logger.logGlobalEvent).toHaveBeenCalledTimes(1); - expect(logger.logGlobalEvent).toHaveBeenCalledWith('Sending SIGKILL to other processes..'); - expect(commands[0].kill).not.toHaveBeenCalled(); - expect(commands[1].kill).toHaveBeenCalledWith('SIGKILL'); + expect(logger.logGlobalEvent).toHaveBeenCalledTimes(1); + expect(logger.logGlobalEvent).toHaveBeenCalledWith('Sending SIGKILL to other processes..'); + expect(commands[0].kill).not.toHaveBeenCalled(); + expect(commands[1].kill).toHaveBeenCalledWith('SIGKILL'); + }); }); it('does nothing if called without conditions', () => { @@ -73,28 +77,6 @@ it('does nothing if called without conditions', () => { expect(commands[1].kill).not.toHaveBeenCalled(); }); -it('kills other killable processes on failure', () => { - createWithConditions(['failure']).handle(commands); - commands[1].isKillable = true; - commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); - - expect(logger.logGlobalEvent).toHaveBeenCalledTimes(1); - expect(logger.logGlobalEvent).toHaveBeenCalledWith('Sending SIGTERM to other processes..'); - expect(commands[0].kill).not.toHaveBeenCalled(); - expect(commands[1].kill).toHaveBeenCalledWith(undefined); -}); - -it('kills other killable processes on failure, with specified signal', () => { - createWithConditions(['failure'], 'SIGKILL').handle(commands); - commands[1].isKillable = true; - commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); - - expect(logger.logGlobalEvent).toHaveBeenCalledTimes(1); - expect(logger.logGlobalEvent).toHaveBeenCalledWith('Sending SIGKILL to other processes..'); - expect(commands[0].kill).not.toHaveBeenCalled(); - expect(commands[1].kill).toHaveBeenCalledWith('SIGKILL'); -}); - it('does not try to kill processes already dead', () => { createWithConditions(['failure']).handle(commands); commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); From 4cb67972536e74722ec102734a08c1edbb6c16ce Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Tue, 12 Dec 2023 17:12:36 +1100 Subject: [PATCH 4/9] kill-others: call abort controller on condition match --- src/flow-control/kill-others.spec.ts | 18 ++++++++++++++++++ src/flow-control/kill-others.ts | 6 ++++++ 2 files changed, 24 insertions(+) diff --git a/src/flow-control/kill-others.spec.ts b/src/flow-control/kill-others.spec.ts index 7dbded77..3da3639c 100644 --- a/src/flow-control/kill-others.spec.ts +++ b/src/flow-control/kill-others.spec.ts @@ -14,14 +14,17 @@ beforeAll(() => { let commands: FakeCommand[]; let logger: Logger; +let abortController: AbortController; beforeEach(() => { commands = [new FakeCommand(), new FakeCommand()]; logger = createMockInstance(Logger); + abortController = new AbortController(); }); const createWithConditions = (conditions: ProcessCloseCondition[], killSignal?: string) => new KillOthers({ logger, + abortController, conditions, killSignal, }); @@ -43,6 +46,7 @@ it('does not kill others if condition does not match', () => { describe.each(['success', 'failure'] as const)('on %s', (condition) => { const exitCode = condition === 'success' ? 0 : 1; + const inversedCode = exitCode === 1 ? 0 : 1; it('kills other killable processes', () => { createWithConditions([condition]).handle(commands); @@ -65,6 +69,20 @@ describe.each(['success', 'failure'] as const)('on %s', (condition) => { expect(commands[0].kill).not.toHaveBeenCalled(); expect(commands[1].kill).toHaveBeenCalledWith('SIGKILL'); }); + + it('sends abort signal on condition match', () => { + createWithConditions([condition]).handle(commands); + commands[0].close.next(createFakeCloseEvent({ exitCode })); + + expect(abortController.signal.aborted).toBe(true); + }); + + it('does not send abort signal on condition mismatch', () => { + createWithConditions([condition]).handle(commands); + commands[0].close.next(createFakeCloseEvent({ exitCode: inversedCode })); + + expect(abortController.signal.aborted).toBe(false); + }); }); it('does nothing if called without conditions', () => { diff --git a/src/flow-control/kill-others.ts b/src/flow-control/kill-others.ts index abc20e06..f693c879 100644 --- a/src/flow-control/kill-others.ts +++ b/src/flow-control/kill-others.ts @@ -12,19 +12,23 @@ export type ProcessCloseCondition = 'failure' | 'success'; */ export class KillOthers implements FlowController { private readonly logger: Logger; + private readonly abortController?: AbortController; private readonly conditions: ProcessCloseCondition[]; private readonly killSignal: string | undefined; constructor({ logger, + abortController, conditions, killSignal, }: { logger: Logger; + abortController?: AbortController; conditions: ProcessCloseCondition | ProcessCloseCondition[]; killSignal: string | undefined; }) { this.logger = logger; + this.abortController = abortController; this.conditions = _.castArray(conditions); this.killSignal = killSignal; } @@ -49,6 +53,8 @@ export class KillOthers implements FlowController { closeStates.forEach((closeState) => closeState.subscribe(() => { + this.abortController?.abort(); + const killableCommands = commands.filter((command) => Command.canKill(command)); if (killableCommands.length) { this.logger.logGlobalEvent( From 74485c68a90d29d6bf394c8f9727cfa017ee5e7a Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Tue, 12 Dec 2023 17:37:35 +1100 Subject: [PATCH 5/9] Don't spawn further commands on abort signal --- src/concurrently.spec.ts | 10 ++++++++++ src/concurrently.ts | 13 +++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/concurrently.spec.ts b/src/concurrently.spec.ts index dcd65b0b..a07a6399 100644 --- a/src/concurrently.spec.ts +++ b/src/concurrently.spec.ts @@ -111,6 +111,16 @@ it('spawns commands up to percent based limit at once', () => { expect(spawn).toHaveBeenCalledWith('qux', expect.objectContaining({})); }); +it('does not spawn further commands on abort signal aborted', () => { + const abortController = new AbortController(); + create(['foo', 'bar'], { maxProcesses: 1, abortSignal: abortController.signal }); + expect(spawn).toHaveBeenCalledTimes(1); + + abortController.abort(); + processes[0].emit('close', 0, null); + expect(spawn).toHaveBeenCalledTimes(1); +}); + it('runs controllers with the commands', () => { create(['echo', '"echo wrapped"']); diff --git a/src/concurrently.ts b/src/concurrently.ts index 2c5b9ec8..991f2c0d 100644 --- a/src/concurrently.ts +++ b/src/concurrently.ts @@ -105,6 +105,11 @@ export type ConcurrentlyOptions = { */ successCondition?: SuccessCondition; + /** + * A signal to stop spawning further processes. + */ + abortSignal?: AbortSignal; + /** * Which flow controllers should be applied on commands spawned by concurrently. * Defaults to an empty array. @@ -217,7 +222,7 @@ export function concurrently( : Number(options.maxProcesses)) || commandsLeft.length, ); for (let i = 0; i < maxProcesses; i++) { - maybeRunMore(commandsLeft); + maybeRunMore(commandsLeft, options.abortSignal); } const result = new CompletionListener({ successCondition: options.successCondition }) @@ -263,14 +268,14 @@ function parseCommand(command: CommandInfo, parsers: CommandParser[]) { ); } -function maybeRunMore(commandsLeft: Command[]) { +function maybeRunMore(commandsLeft: Command[], abortSignal?: AbortSignal) { const command = commandsLeft.shift(); - if (!command) { + if (!command || abortSignal?.aborted) { return; } command.start(); command.close.subscribe(() => { - maybeRunMore(commandsLeft); + maybeRunMore(commandsLeft, abortSignal); }); } From 07553f82791ddaee216911edafa842df7224c04c Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Wed, 10 Jan 2024 13:13:55 -0300 Subject: [PATCH 6/9] Interpret aborted commands as success --- src/completion-listener.spec.ts | 25 ++++++++++++++ src/completion-listener.ts | 58 +++++++++++++++++++++++---------- src/concurrently.ts | 3 +- 3 files changed, 67 insertions(+), 19 deletions(-) diff --git a/src/completion-listener.spec.ts b/src/completion-listener.spec.ts index 905b0bcc..188e14f3 100644 --- a/src/completion-listener.spec.ts +++ b/src/completion-listener.spec.ts @@ -24,6 +24,31 @@ const createController = (successCondition?: SuccessCondition) => const emitFakeCloseEvent = (command: FakeCommand, event?: Partial) => command.close.next(createFakeCloseEvent({ ...event, command, index: command.index })); +it('completes only when commands emit a close event, returns close event', async () => { + const abortCtrl = new AbortController(); + const result = createController('all').listen(commands, abortCtrl.signal); + + commands[0].state = 'started'; + abortCtrl.abort(); + + const event = createFakeCloseEvent({ exitCode: 0 }); + commands[0].close.next(event); + scheduler.flush(); + + await expect(result).resolves.toHaveLength(1); + await expect(result).resolves.toEqual([event]); +}); + +it('completes when abort signal is received and command is stopped, returns nothing', async () => { + const abortCtrl = new AbortController(); + const result = createController('all').listen([new FakeCommand()], abortCtrl.signal); + + abortCtrl.abort(); + scheduler.flush(); + + await expect(result).resolves.toHaveLength(0); +}); + describe('with default success condition set', () => { it('succeeds if all processes exited with code 0', () => { const result = createController().listen(commands); diff --git a/src/completion-listener.ts b/src/completion-listener.ts index 6889a676..bd7da000 100644 --- a/src/completion-listener.ts +++ b/src/completion-listener.ts @@ -1,5 +1,5 @@ import * as Rx from 'rxjs'; -import { bufferCount, switchMap, take } from 'rxjs/operators'; +import { bufferCount, delay, filter, map, switchMap, take } from 'rxjs/operators'; import { CloseEvent, Command } from './command'; @@ -47,17 +47,17 @@ export class CompletionListener { this.scheduler = scheduler; } - private isSuccess(events: CloseEvent[]) { + private isSuccess(events: (CloseEvent | undefined)[]) { if (this.successCondition === 'first') { - return events[0].exitCode === 0; + return isSuccess(events[0]); } else if (this.successCondition === 'last') { - return events[events.length - 1].exitCode === 0; + return isSuccess(events[events.length - 1]); } const commandSyntaxMatch = this.successCondition.match(/^!?command-(.+)$/); if (commandSyntaxMatch == null) { // If not a `command-` syntax, then it's an 'all' condition or it's treated as such. - return events.every(({ exitCode }) => exitCode === 0); + return events.every(isSuccess); } // Check `command-` syntax condition. @@ -65,36 +65,54 @@ export class CompletionListener { // in which case all of them must meet the success condition. const nameOrIndex = commandSyntaxMatch[1]; const targetCommandsEvents = events.filter( - ({ command, index }) => command.name === nameOrIndex || index === Number(nameOrIndex), + (event) => event?.command.name === nameOrIndex || event?.index === Number(nameOrIndex), ); if (this.successCondition.startsWith('!')) { // All commands except the specified ones must exit succesfully return events.every( - (event) => targetCommandsEvents.includes(event) || event.exitCode === 0, + (event) => targetCommandsEvents.includes(event) || isSuccess(event), ); } // Only the specified commands must exit succesfully - return ( - targetCommandsEvents.length > 0 && - targetCommandsEvents.every((event) => event.exitCode === 0) - ); + return targetCommandsEvents.length > 0 && targetCommandsEvents.every(isSuccess); } /** * Given a list of commands, wait for all of them to exit and then evaluate their exit codes. * * @returns A Promise that resolves if the success condition is met, or rejects otherwise. + * In either case, the value is a list of close events for commands that spawned. + * Commands that didn't spawn are filtered out. */ - listen(commands: Command[]): Promise { - const closeStreams = commands.map((command) => command.close); + listen(commands: Command[], abortSignal?: AbortSignal): Promise { + const abort = + abortSignal && + Rx.fromEvent(abortSignal, 'abort', { once: true }).pipe( + // The abort signal must happen before commands are killed, otherwise new commands + // might spawn. Because of this, it's not be possible to capture the close events + // without an immediate delay + delay(0, this.scheduler), + map(() => undefined), + ); + + const closeStreams = commands.map((command) => + abort + ? // Commands that have been started must close. + Rx.race(command.close, abort.pipe(filter(() => command.state === 'stopped'))) + : command.close, + ); return Rx.lastValueFrom( Rx.merge(...closeStreams).pipe( bufferCount(closeStreams.length), - switchMap((exitInfos) => - this.isSuccess(exitInfos) - ? this.emitWithScheduler(Rx.of(exitInfos)) - : this.emitWithScheduler(Rx.throwError(() => exitInfos)), - ), + switchMap((events) => { + const success = this.isSuccess(events); + const filteredEvents = events.filter( + (event): event is CloseEvent => event != null, + ); + return success + ? this.emitWithScheduler(Rx.of(filteredEvents)) + : this.emitWithScheduler(Rx.throwError(() => filteredEvents)); + }), take(1), ), ); @@ -104,3 +122,7 @@ export class CompletionListener { return this.scheduler ? input.pipe(Rx.observeOn(this.scheduler)) : input; } } + +function isSuccess(event: CloseEvent | undefined) { + return event == null || event.exitCode === 0; +} diff --git a/src/concurrently.ts b/src/concurrently.ts index 991f2c0d..a8ec5232 100644 --- a/src/concurrently.ts +++ b/src/concurrently.ts @@ -43,7 +43,8 @@ export type ConcurrentlyResult = { * A promise that resolves when concurrently ran successfully according to the specified * success condition, or reject otherwise. * - * Both the resolved and rejected value is the list of all command's close events. + * Both the resolved and rejected value is a list of all the close events for commands that + * spawned; commands that didn't spawn are filtered out. */ result: Promise; }; From 6e1407b68510b6b6b80012e6ca79a824d5cc3666 Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Wed, 10 Jan 2024 13:14:48 -0300 Subject: [PATCH 7/9] Pass abort controller/signal from main concurrently function --- src/concurrently.ts | 2 +- src/index.ts | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/concurrently.ts b/src/concurrently.ts index a8ec5232..e995939d 100644 --- a/src/concurrently.ts +++ b/src/concurrently.ts @@ -227,7 +227,7 @@ export function concurrently( } const result = new CompletionListener({ successCondition: options.successCondition }) - .listen(commands) + .listen(commands, options.abortSignal) .finally(() => { handleResult.onFinishCallbacks.forEach((onFinish) => onFinish()); }); diff --git a/src/index.ts b/src/index.ts index 3dce77b6..8737738c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -18,7 +18,7 @@ import { LogTimings } from './flow-control/log-timings'; import { RestartProcess } from './flow-control/restart-process'; import { Logger } from './logger'; -export type ConcurrentlyOptions = BaseConcurrentlyOptions & { +export type ConcurrentlyOptions = Omit & { // Logger options /** * Which command(s) should have their output hidden. @@ -103,6 +103,8 @@ export default ( timestampFormat: options.timestampFormat, }); + const abortController = new AbortController(); + return concurrently(commands, { maxProcesses: options.maxProcesses, raw: options.raw, @@ -111,6 +113,7 @@ export default ( logger, outputStream: options.outputStream || process.stdout, group: options.group, + abortSignal: abortController.signal, controllers: [ new LogError({ logger }), new LogOutput({ logger }), @@ -122,7 +125,7 @@ export default ( options.inputStream || (options.handleInput ? process.stdin : undefined), pauseInputStreamOnFinish: options.pauseInputStreamOnFinish, }), - new KillOnSignal({ process }), + new KillOnSignal({ process, abortController }), new RestartProcess({ logger, delay: options.restartDelay, @@ -132,6 +135,7 @@ export default ( logger, conditions: options.killOthers || [], killSignal: options.killSignal, + abortController, }), new LogTimings({ logger: options.timings ? logger : undefined, From 67ad002077bd1b1ca8cd343b9036b702563bab65 Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Mon, 25 Mar 2024 08:18:28 +1100 Subject: [PATCH 8/9] Improve code coverage while fixing an edge case --- src/completion-listener.spec.ts | 3 +- src/completion-listener.ts | 52 +++++++++++++++++---------------- 2 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/completion-listener.spec.ts b/src/completion-listener.spec.ts index 4ecad1bd..c4931798 100644 --- a/src/completion-listener.spec.ts +++ b/src/completion-listener.spec.ts @@ -47,7 +47,8 @@ describe('listen', () => { it('completes when abort signal is received and command is stopped, returns nothing', async () => { const abortCtrl = new AbortController(); - const result = createController('all').listen([new FakeCommand()], abortCtrl.signal); + // Use success condition = first to test index access when there are no close events + const result = createController('first').listen([new FakeCommand()], abortCtrl.signal); abortCtrl.abort(); scheduler.flush(); diff --git a/src/completion-listener.ts b/src/completion-listener.ts index 56ae37a2..6251ac92 100644 --- a/src/completion-listener.ts +++ b/src/completion-listener.ts @@ -47,17 +47,22 @@ export class CompletionListener { this.scheduler = scheduler; } - private isSuccess(events: (CloseEvent | undefined)[]) { + private isSuccess(events: CloseEvent[]) { + if (!events.length) { + // When every command was aborted, consider a success. + return true; + } + if (this.successCondition === 'first') { - return isSuccess(events[0]); + return events[0].exitCode === 0; } else if (this.successCondition === 'last') { - return isSuccess(events[events.length - 1]); + return events[events.length - 1].exitCode === 0; } const commandSyntaxMatch = this.successCondition.match(/^!?command-(.+)$/); if (commandSyntaxMatch == null) { // If not a `command-` syntax, then it's an 'all' condition or it's treated as such. - return events.every(isSuccess); + return events.every(({ exitCode }) => exitCode === 0); } // Check `command-` syntax condition. @@ -65,16 +70,19 @@ export class CompletionListener { // in which case all of them must meet the success condition. const nameOrIndex = commandSyntaxMatch[1]; const targetCommandsEvents = events.filter( - (event) => event?.command.name === nameOrIndex || event?.index === Number(nameOrIndex), + ({ command, index }) => command.name === nameOrIndex || index === Number(nameOrIndex), ); if (this.successCondition.startsWith('!')) { // All commands except the specified ones must exit successfully return events.every( - (event) => targetCommandsEvents.includes(event) || isSuccess(event), + (event) => targetCommandsEvents.includes(event) || event.exitCode === 0, ); } // Only the specified commands must exit succesfully - return targetCommandsEvents.length > 0 && targetCommandsEvents.every(isSuccess); + return ( + targetCommandsEvents.length > 0 && + targetCommandsEvents.every((event) => event.exitCode === 0) + ); } /** @@ -108,22 +116,20 @@ export class CompletionListener { (command, i) => command.state !== 'started' || events[i] === undefined, ), ), - map((exitInfos) => - exitInfos.sort((first, second) => { - if (!first || !second) { - return 0; - } - return first.timings.endDate.getTime() - second.timings.endDate.getTime(); - }), + map((events) => + events + // Filter out aborts, since they cannot be sorted and are considered success condition anyways + .filter((event): event is CloseEvent => event != null) + // Sort according to exit time + .sort( + (first, second) => + first.timings.endDate.getTime() - second.timings.endDate.getTime(), + ), ), switchMap((events) => { - const success = this.isSuccess(events); - const filteredEvents = events.filter( - (event): event is CloseEvent => event != null, - ); - return success - ? this.emitWithScheduler(Rx.of(filteredEvents)) - : this.emitWithScheduler(Rx.throwError(() => filteredEvents)); + return this.isSuccess(events) + ? this.emitWithScheduler(Rx.of(events)) + : this.emitWithScheduler(Rx.throwError(() => events)); }), take(1), ), @@ -134,7 +140,3 @@ export class CompletionListener { return this.scheduler ? input.pipe(Rx.observeOn(this.scheduler)) : input; } } - -function isSuccess(event: CloseEvent | undefined) { - return event == null || event.exitCode === 0; -} From 91d6b3ff820cfbca6d6ed5f34116c5f58fcb7266 Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Mon, 25 Mar 2024 08:20:22 +1100 Subject: [PATCH 9/9] Undo some changes that don't affect the result --- src/completion-listener.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/completion-listener.ts b/src/completion-listener.ts index 6251ac92..e03414e1 100644 --- a/src/completion-listener.ts +++ b/src/completion-listener.ts @@ -126,11 +126,11 @@ export class CompletionListener { first.timings.endDate.getTime() - second.timings.endDate.getTime(), ), ), - switchMap((events) => { - return this.isSuccess(events) + switchMap((events) => + this.isSuccess(events) ? this.emitWithScheduler(Rx.of(events)) - : this.emitWithScheduler(Rx.throwError(() => events)); - }), + : this.emitWithScheduler(Rx.throwError(() => events)), + ), take(1), ), );