From 37156a98b950613cf09fd4865c4185909f738a98 Mon Sep 17 00:00:00 2001 From: Oded Goldglas Date: Thu, 1 Feb 2024 10:49:45 +0200 Subject: [PATCH 01/10] Ensuring completion-listener.ts listen to a single close event emitted from each command. --- src/completion-listener.spec.ts | 266 ++++++++++++++++++-------------- src/completion-listener.ts | 13 +- 2 files changed, 154 insertions(+), 125 deletions(-) diff --git a/src/completion-listener.spec.ts b/src/completion-listener.spec.ts index 905b0bcc..6680f122 100644 --- a/src/completion-listener.spec.ts +++ b/src/completion-listener.spec.ts @@ -24,186 +24,214 @@ const createController = (successCondition?: SuccessCondition) => const emitFakeCloseEvent = (command: FakeCommand, event?: Partial) => command.close.next(createFakeCloseEvent({ ...event, command, index: command.index })); -describe('with default success condition set', () => { - it('succeeds if all processes exited with code 0', () => { - const result = createController().listen(commands); +describe('listen', () => { + it('check for success once all commands have emitted at least a single close event', async () => { + const finallyCallback = jest.fn(); + const result = createController().listen(commands).finally(finallyCallback); + // Emitting multiple close events to mimic calling command `kill/start` APIs. + commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); scheduler.flush(); - return expect(result).resolves.toEqual(expect.anything()); - }); - - it('fails if one of the processes exited with non-0 code', () => { - const result = createController().listen(commands); + await Promise.resolve(); + expect(finallyCallback).not.toHaveBeenCalled(); - commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[1].close.next(createFakeCloseEvent({ exitCode: 1 })); + commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); scheduler.flush(); - return expect(result).rejects.toEqual(expect.anything()); + await expect(result).resolves.toEqual(expect.anything()); + + expect(finallyCallback).toHaveBeenCalled(); }); }); -describe('with success condition set to first', () => { - it('succeeds if first process to exit has code 0', () => { - const result = createController('first').listen(commands); +describe('Detect commands exit conditions', () => { + describe('with default success condition set', () => { + it('succeeds if all processes exited with code 0', () => { + const result = createController().listen(commands); - commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); - commands[2].close.next(createFakeCloseEvent({ exitCode: 1 })); + commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); - scheduler.flush(); + scheduler.flush(); - return expect(result).resolves.toEqual(expect.anything()); - }); + return expect(result).resolves.toEqual(expect.anything()); + }); - it('fails if first process to exit has non-0 code', () => { - const result = createController('first').listen(commands); + it('fails if one of the processes exited with non-0 code', () => { + const result = createController().listen(commands); - commands[1].close.next(createFakeCloseEvent({ exitCode: 1 })); - commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[1].close.next(createFakeCloseEvent({ exitCode: 1 })); + commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); - scheduler.flush(); + scheduler.flush(); - return expect(result).rejects.toEqual(expect.anything()); + return expect(result).rejects.toEqual(expect.anything()); + }); }); -}); -describe('with success condition set to last', () => { - it('succeeds if last process to exit has code 0', () => { - const result = createController('last').listen(commands); + describe('with success condition set to first', () => { + it('succeeds if first process to exit has code 0', () => { + const result = createController('first').listen(commands); - commands[1].close.next(createFakeCloseEvent({ exitCode: 1 })); - commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); + commands[2].close.next(createFakeCloseEvent({ exitCode: 1 })); - scheduler.flush(); + scheduler.flush(); - return expect(result).resolves.toEqual(expect.anything()); - }); + return expect(result).resolves.toEqual(expect.anything()); + }); - it('fails if last process to exit has non-0 code', () => { - const result = createController('last').listen(commands); + it('fails if first process to exit has non-0 code', () => { + const result = createController('first').listen(commands); - commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); - commands[2].close.next(createFakeCloseEvent({ exitCode: 1 })); + commands[1].close.next(createFakeCloseEvent({ exitCode: 1 })); + commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); - scheduler.flush(); + scheduler.flush(); - return expect(result).rejects.toEqual(expect.anything()); + return expect(result).rejects.toEqual(expect.anything()); + }); }); -}); -describe.each([ - // Use the middle command for both cases to make it more difficult to make a mess up - // in the implementation cause false passes. - ['command-bar' as const, 'bar'], - ['command-1' as const, 1], -])('with success condition set to %s', (condition, nameOrIndex) => { - it(`succeeds if command ${nameOrIndex} exits with code 0`, () => { - const result = createController(condition).listen(commands); + describe('with success condition set to last', () => { + it('succeeds if last process to exit has code 0', () => { + const result = createController('last').listen(commands); - emitFakeCloseEvent(commands[0], { exitCode: 1 }); - emitFakeCloseEvent(commands[1], { exitCode: 0 }); - emitFakeCloseEvent(commands[2], { exitCode: 1 }); + commands[1].close.next(createFakeCloseEvent({ exitCode: 1 })); + commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); - scheduler.flush(); + scheduler.flush(); - return expect(result).resolves.toEqual(expect.anything()); - }); + return expect(result).resolves.toEqual(expect.anything()); + }); - it(`succeeds if all commands ${nameOrIndex} exit with code 0`, () => { - commands = [commands[0], commands[1], commands[1]]; - const result = createController(condition).listen(commands); + it('fails if last process to exit has non-0 code', () => { + const result = createController('last').listen(commands); - emitFakeCloseEvent(commands[0], { exitCode: 1 }); - emitFakeCloseEvent(commands[1], { exitCode: 0 }); - emitFakeCloseEvent(commands[2], { exitCode: 0 }); + commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); + commands[2].close.next(createFakeCloseEvent({ exitCode: 1 })); - scheduler.flush(); + scheduler.flush(); - return expect(result).resolves.toEqual(expect.anything()); + return expect(result).rejects.toEqual(expect.anything()); + }); }); - it(`fails if command ${nameOrIndex} exits with non-0 code`, () => { - const result = createController(condition).listen(commands); + describe.each([ + // Use the middle command for both cases to make it more difficult to make a mess up + // in the implementation cause false passes. + ['command-bar' as const, 'bar'], + ['command-1' as const, 1], + ])('with success condition set to %s', (condition, nameOrIndex) => { + it(`succeeds if command ${nameOrIndex} exits with code 0`, () => { + const result = createController(condition).listen(commands); - emitFakeCloseEvent(commands[0], { exitCode: 0 }); - emitFakeCloseEvent(commands[1], { exitCode: 1 }); - emitFakeCloseEvent(commands[2], { exitCode: 0 }); + emitFakeCloseEvent(commands[0], { exitCode: 1 }); + emitFakeCloseEvent(commands[1], { exitCode: 0 }); + emitFakeCloseEvent(commands[2], { exitCode: 1 }); - scheduler.flush(); + scheduler.flush(); - return expect(result).rejects.toEqual(expect.anything()); - }); + return expect(result).resolves.toEqual(expect.anything()); + }); - it(`fails if some commands ${nameOrIndex} exit with non-0 code`, () => { - commands = [commands[0], commands[1], commands[1]]; - const result = createController(condition).listen(commands); + it(`succeeds if all commands ${nameOrIndex} exit with code 0`, () => { + commands = [commands[0], commands[1], commands[1]]; + const result = createController(condition).listen(commands); - emitFakeCloseEvent(commands[0], { exitCode: 1 }); - emitFakeCloseEvent(commands[1], { exitCode: 0 }); - emitFakeCloseEvent(commands[2], { exitCode: 1 }); + emitFakeCloseEvent(commands[0], { exitCode: 1 }); + emitFakeCloseEvent(commands[1], { exitCode: 0 }); + emitFakeCloseEvent(commands[2], { exitCode: 0 }); - scheduler.flush(); + scheduler.flush(); - return expect(result).resolves.toEqual(expect.anything()); - }); + return expect(result).resolves.toEqual(expect.anything()); + }); - it(`fails if command ${nameOrIndex} doesn't exist`, () => { - const result = createController(condition).listen([commands[0]]); + it(`fails if command ${nameOrIndex} exits with non-0 code`, () => { + const result = createController(condition).listen(commands); - emitFakeCloseEvent(commands[0], { exitCode: 0 }); - scheduler.flush(); + emitFakeCloseEvent(commands[0], { exitCode: 0 }); + emitFakeCloseEvent(commands[1], { exitCode: 1 }); + emitFakeCloseEvent(commands[2], { exitCode: 0 }); - return expect(result).rejects.toEqual(expect.anything()); - }); -}); + scheduler.flush(); -describe.each([ - // Use the middle command for both cases to make it more difficult to make a mess up - // in the implementation cause false passes. - ['!command-bar' as const, 'bar'], - ['!command-1' as const, 1], -])('with success condition set to %s', (condition, nameOrIndex) => { - it(`succeeds if all commands but ${nameOrIndex} exit with code 0`, () => { - const result = createController(condition).listen(commands); + return expect(result).rejects.toEqual(expect.anything()); + }); - emitFakeCloseEvent(commands[0], { exitCode: 0 }); - emitFakeCloseEvent(commands[1], { exitCode: 1 }); - emitFakeCloseEvent(commands[2], { exitCode: 0 }); + it(`fails if some commands ${nameOrIndex} exit with non-0 code`, () => { + commands = [commands[0], commands[1], commands[1]]; + const result = createController(condition).listen(commands); - scheduler.flush(); + emitFakeCloseEvent(commands[0], { exitCode: 1 }); + emitFakeCloseEvent(commands[1], { exitCode: 0 }); + emitFakeCloseEvent(commands[2], { exitCode: 1 }); - return expect(result).resolves.toEqual(expect.anything()); - }); + scheduler.flush(); - it(`fails if any commands but ${nameOrIndex} exit with non-0 code`, () => { - const result = createController(condition).listen(commands); + return expect(result).resolves.toEqual(expect.anything()); + }); - emitFakeCloseEvent(commands[0], { exitCode: 1 }); - emitFakeCloseEvent(commands[1], { exitCode: 1 }); - emitFakeCloseEvent(commands[2], { exitCode: 0 }); + it(`fails if command ${nameOrIndex} doesn't exist`, () => { + const result = createController(condition).listen([commands[0]]); - scheduler.flush(); + emitFakeCloseEvent(commands[0], { exitCode: 0 }); + scheduler.flush(); - return expect(result).rejects.toEqual(expect.anything()); + return expect(result).rejects.toEqual(expect.anything()); + }); }); - it(`succeeds if command ${nameOrIndex} doesn't exist`, () => { - const result = createController(condition).listen([commands[0]]); + describe.each([ + // Use the middle command for both cases to make it more difficult to make a mess up + // in the implementation cause false passes. + ['!command-bar' as const, 'bar'], + ['!command-1' as const, 1], + ])('with success condition set to %s', (condition, nameOrIndex) => { + it(`succeeds if all commands but ${nameOrIndex} exit with code 0`, () => { + const result = createController(condition).listen(commands); - emitFakeCloseEvent(commands[0], { exitCode: 0 }); - scheduler.flush(); + emitFakeCloseEvent(commands[0], { exitCode: 0 }); + emitFakeCloseEvent(commands[1], { exitCode: 1 }); + emitFakeCloseEvent(commands[2], { exitCode: 0 }); + + scheduler.flush(); + + return expect(result).resolves.toEqual(expect.anything()); + }); + + it(`fails if any commands but ${nameOrIndex} exit with non-0 code`, () => { + const result = createController(condition).listen(commands); + + emitFakeCloseEvent(commands[0], { exitCode: 1 }); + emitFakeCloseEvent(commands[1], { exitCode: 1 }); + emitFakeCloseEvent(commands[2], { exitCode: 0 }); + + scheduler.flush(); + + return expect(result).rejects.toEqual(expect.anything()); + }); + + it(`succeeds if command ${nameOrIndex} doesn't exist`, () => { + const result = createController(condition).listen([commands[0]]); + + emitFakeCloseEvent(commands[0], { exitCode: 0 }); + scheduler.flush(); - return expect(result).resolves.toEqual(expect.anything()); + return expect(result).resolves.toEqual(expect.anything()); + }); }); }); diff --git a/src/completion-listener.ts b/src/completion-listener.ts index 6889a676..92abd467 100644 --- a/src/completion-listener.ts +++ b/src/completion-listener.ts @@ -27,9 +27,9 @@ export class CompletionListener { private readonly scheduler?: Rx.SchedulerLike; constructor({ - successCondition = 'all', - scheduler, - }: { + successCondition = 'all', + scheduler, + }: { /** * How this instance will define that a list of commands ran successfully. * Defaults to `all`. @@ -68,12 +68,12 @@ export class CompletionListener { ({ command, index }) => command.name === nameOrIndex || index === Number(nameOrIndex), ); if (this.successCondition.startsWith('!')) { - // All commands except the specified ones must exit succesfully + // All commands except the specified ones must exit successfully return events.every( (event) => targetCommandsEvents.includes(event) || event.exitCode === 0, ); } - // Only the specified commands must exit succesfully + // Only the specified commands must exit successfully return ( targetCommandsEvents.length > 0 && targetCommandsEvents.every((event) => event.exitCode === 0) @@ -86,7 +86,8 @@ export class CompletionListener { * @returns A Promise that resolves if the success condition is met, or rejects otherwise. */ listen(commands: Command[]): Promise { - const closeStreams = commands.map((command) => command.close); + const closeStreams = commands.map((command) => command.close.pipe(take(1))); + return Rx.lastValueFrom( Rx.merge(...closeStreams).pipe( bufferCount(closeStreams.length), From 2eee2c26baf7359275e93b5fc12de50dfc4e33de Mon Sep 17 00:00:00 2001 From: Oded Goldglas Date: Thu, 1 Feb 2024 10:58:37 +0200 Subject: [PATCH 02/10] Format. --- src/completion-listener.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/completion-listener.ts b/src/completion-listener.ts index 92abd467..4701828a 100644 --- a/src/completion-listener.ts +++ b/src/completion-listener.ts @@ -27,9 +27,9 @@ export class CompletionListener { private readonly scheduler?: Rx.SchedulerLike; constructor({ - successCondition = 'all', - scheduler, - }: { + successCondition = 'all', + scheduler, + }: { /** * How this instance will define that a list of commands ran successfully. * Defaults to `all`. From 5c9b16facbbe5a73e0e5f4be0e6bb3bc74b0c625 Mon Sep 17 00:00:00 2001 From: Oded Goldglas Date: Sun, 25 Feb 2024 12:08:54 +0200 Subject: [PATCH 03/10] -Apply Latest. -Adding `ipc` option by default. -Adjusting specs. --- src/completion-listener.spec.ts | 18 ++++++++++++++++-- src/completion-listener.ts | 15 +++++++++++---- src/concurrently.spec.ts | 2 +- src/get-spawn-opts.ts | 1 + 4 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/completion-listener.spec.ts b/src/completion-listener.spec.ts index 6680f122..7ac95e88 100644 --- a/src/completion-listener.spec.ts +++ b/src/completion-listener.spec.ts @@ -24,6 +24,8 @@ const createController = (successCondition?: SuccessCondition) => const emitFakeCloseEvent = (command: FakeCommand, event?: Partial) => command.close.next(createFakeCloseEvent({ ...event, command, index: command.index })); +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + describe('listen', () => { it('check for success once all commands have emitted at least a single close event', async () => { const finallyCallback = jest.fn(); @@ -36,7 +38,7 @@ describe('listen', () => { scheduler.flush(); - await Promise.resolve(); + await sleep(100); expect(finallyCallback).not.toHaveBeenCalled(); commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); @@ -48,6 +50,19 @@ describe('listen', () => { expect(finallyCallback).toHaveBeenCalled(); }); + + it('Takes last event emitted from each command', async () => { + const result = createController().listen(commands); + + commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); + commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); + commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); + + scheduler.flush(); + + await expect(result).rejects.toEqual(expect.anything()); + }); }); describe('Detect commands exit conditions', () => { @@ -173,7 +188,6 @@ describe('Detect commands exit conditions', () => { }); it(`fails if some commands ${nameOrIndex} exit with non-0 code`, () => { - commands = [commands[0], commands[1], commands[1]]; const result = createController(condition).listen(commands); emitFakeCloseEvent(commands[0], { exitCode: 1 }); diff --git a/src/completion-listener.ts b/src/completion-listener.ts index 4701828a..6e1cfb60 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 { filter, map, switchMap, take } from 'rxjs/operators'; import { CloseEvent, Command } from './command'; @@ -86,11 +86,18 @@ export class CompletionListener { * @returns A Promise that resolves if the success condition is met, or rejects otherwise. */ listen(commands: Command[]): Promise { - const closeStreams = commands.map((command) => command.close.pipe(take(1))); + const closeStreams = commands.map((command) => command.close.pipe()); return Rx.lastValueFrom( - Rx.merge(...closeStreams).pipe( - bufferCount(closeStreams.length), + Rx.combineLatest(closeStreams).pipe( + filter((exitInfos) => exitInfos.every((exitInfo) => exitInfo.exitCode !== null)), + map((exitInfos) => + exitInfos.sort((first, second) => { + return ( + first.timings.startDate.getTime() - second.timings.startDate.getTime() + ); + }), + ), switchMap((exitInfos) => this.isSuccess(exitInfos) ? this.emitWithScheduler(Rx.of(exitInfos)) diff --git a/src/concurrently.spec.ts b/src/concurrently.spec.ts index dcd65b0b..0f289b22 100644 --- a/src/concurrently.spec.ts +++ b/src/concurrently.spec.ts @@ -281,7 +281,7 @@ it('uses overridden raw option for each command if specified', () => { expect(spawn).toHaveBeenCalledTimes(2); expect(spawn).toHaveBeenCalledWith( 'echo', - expect.not.objectContaining({ + expect.objectContaining({ stdio: expect.anything(), }), ); diff --git a/src/get-spawn-opts.ts b/src/get-spawn-opts.ts index a65d6faa..fbe1ab61 100644 --- a/src/get-spawn-opts.ts +++ b/src/get-spawn-opts.ts @@ -39,6 +39,7 @@ export const getSpawnOpts = ({ env?: Record; }): SpawnOptions => ({ cwd: cwd || process.cwd(), + stdio: ['pipe', 'pipe', 'pipe', 'ipc'], ...(raw && { stdio: 'inherit' as const }), ...(/^win/.test(process.platform) && { detached: false }), env: { From f5dd736a3a4677a3c5cd2eb30be6c741e60eec5b Mon Sep 17 00:00:00 2001 From: Oded Goldglas Date: Sun, 25 Feb 2024 12:17:40 +0200 Subject: [PATCH 04/10] Remove IPC support --- src/concurrently.spec.ts | 2 +- src/get-spawn-opts.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/concurrently.spec.ts b/src/concurrently.spec.ts index 0f289b22..dcd65b0b 100644 --- a/src/concurrently.spec.ts +++ b/src/concurrently.spec.ts @@ -281,7 +281,7 @@ it('uses overridden raw option for each command if specified', () => { expect(spawn).toHaveBeenCalledTimes(2); expect(spawn).toHaveBeenCalledWith( 'echo', - expect.objectContaining({ + expect.not.objectContaining({ stdio: expect.anything(), }), ); diff --git a/src/get-spawn-opts.ts b/src/get-spawn-opts.ts index fbe1ab61..a65d6faa 100644 --- a/src/get-spawn-opts.ts +++ b/src/get-spawn-opts.ts @@ -39,7 +39,6 @@ export const getSpawnOpts = ({ env?: Record; }): SpawnOptions => ({ cwd: cwd || process.cwd(), - stdio: ['pipe', 'pipe', 'pipe', 'ipc'], ...(raw && { stdio: 'inherit' as const }), ...(/^win/.test(process.platform) && { detached: false }), env: { From 1c03da7a67717f4bac1629f9cdce873da8c513b1 Mon Sep 17 00:00:00 2001 From: Oded Goldglas Date: Mon, 26 Feb 2024 08:59:14 +0200 Subject: [PATCH 05/10] Apply PR changes. --- src/command.ts | 7 +++++++ src/completion-listener.spec.ts | 2 +- src/completion-listener.ts | 13 ++++++------- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/command.ts b/src/command.ts index 815b507e..ddcfe419 100644 --- a/src/command.ts +++ b/src/command.ts @@ -56,6 +56,12 @@ export interface CloseEvent { * The exit code or signal for the command. */ exitCode: string | number; + + /** + * The state of the command. + */ + state: CommandState; + timings: { startDate: Date; endDate: Date; @@ -180,6 +186,7 @@ export class Command implements CommandInfo { const [durationSeconds, durationNanoSeconds] = process.hrtime(highResStartTime); this.close.next({ command: this, + state: this.state, index: this.index, exitCode: exitCode ?? String(signal), killed: this.killed, diff --git a/src/completion-listener.spec.ts b/src/completion-listener.spec.ts index 7ac95e88..302d2c49 100644 --- a/src/completion-listener.spec.ts +++ b/src/completion-listener.spec.ts @@ -51,7 +51,7 @@ describe('listen', () => { expect(finallyCallback).toHaveBeenCalled(); }); - it('Takes last event emitted from each command', async () => { + it('takes last event emitted from each command', async () => { const result = createController().listen(commands); commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); diff --git a/src/completion-listener.ts b/src/completion-listener.ts index 6e1cfb60..de682ddb 100644 --- a/src/completion-listener.ts +++ b/src/completion-listener.ts @@ -86,17 +86,16 @@ export class CompletionListener { * @returns A Promise that resolves if the success condition is met, or rejects otherwise. */ listen(commands: Command[]): Promise { - const closeStreams = commands.map((command) => command.close.pipe()); + const closeStreams = commands.map((command) => command.close); return Rx.lastValueFrom( Rx.combineLatest(closeStreams).pipe( - filter((exitInfos) => exitInfos.every((exitInfo) => exitInfo.exitCode !== null)), + filter((exitInfos) => exitInfos.every(({ state }) => state !== 'started')), map((exitInfos) => - exitInfos.sort((first, second) => { - return ( - first.timings.startDate.getTime() - second.timings.startDate.getTime() - ); - }), + exitInfos.sort( + (first, second) => + first.timings.endDate.getTime() - second.timings.endDate.getTime(), + ), ), switchMap((exitInfos) => this.isSuccess(exitInfos) From 9c8edfb7391f9fc5c88bd4946771c89343590e65 Mon Sep 17 00:00:00 2001 From: Oded Goldglas Date: Mon, 26 Feb 2024 09:04:52 +0200 Subject: [PATCH 06/10] Faked. --- src/fixtures/fake-command.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fixtures/fake-command.ts b/src/fixtures/fake-command.ts index 6d0107fe..293301bc 100644 --- a/src/fixtures/fake-command.ts +++ b/src/fixtures/fake-command.ts @@ -40,6 +40,7 @@ export const createFakeCloseEvent = (overrides?: Partial): CloseEven index: 0, killed: false, exitCode: 0, + state: 'started', timings: { startDate: new Date(), endDate: new Date(), From f81fa007f6b9db9749c16fed657db08b7a0c5fbd Mon Sep 17 00:00:00 2001 From: Oded Goldglas Date: Mon, 26 Feb 2024 09:10:50 +0200 Subject: [PATCH 07/10] Exited --- src/fixtures/fake-command.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fixtures/fake-command.ts b/src/fixtures/fake-command.ts index 293301bc..452a7815 100644 --- a/src/fixtures/fake-command.ts +++ b/src/fixtures/fake-command.ts @@ -40,7 +40,7 @@ export const createFakeCloseEvent = (overrides?: Partial): CloseEven index: 0, killed: false, exitCode: 0, - state: 'started', + state: 'exited', timings: { startDate: new Date(), endDate: new Date(), From bfd20946992622a66c699684592ac979435805aa Mon Sep 17 00:00:00 2001 From: Oded Goldglas Date: Mon, 26 Feb 2024 12:48:58 +0200 Subject: [PATCH 08/10] . --- src/completion-listener.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/completion-listener.ts b/src/completion-listener.ts index de682ddb..c32e6dcd 100644 --- a/src/completion-listener.ts +++ b/src/completion-listener.ts @@ -56,7 +56,7 @@ export class CompletionListener { 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. + // If not a `command-` syntax, then it's an 'all' condition, or it's treated as such. return events.every(({ exitCode }) => exitCode === 0); } From c86d247a1ecbb52721f03a0bb844bc2069de9148 Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Sun, 24 Mar 2024 14:37:20 +1100 Subject: [PATCH 09/10] Check command state, instead of close event state --- src/command.ts | 6 ------ src/completion-listener.spec.ts | 28 +++++++++++++++++++++++++--- src/completion-listener.ts | 2 +- src/fixtures/fake-command.ts | 1 - 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/command.ts b/src/command.ts index ddcfe419..ef9a34f9 100644 --- a/src/command.ts +++ b/src/command.ts @@ -57,11 +57,6 @@ export interface CloseEvent { */ exitCode: string | number; - /** - * The state of the command. - */ - state: CommandState; - timings: { startDate: Date; endDate: Date; @@ -186,7 +181,6 @@ export class Command implements CommandInfo { const [durationSeconds, durationNanoSeconds] = process.hrtime(highResStartTime); this.close.next({ command: this, - state: this.state, index: this.index, exitCode: exitCode ?? String(signal), killed: this.killed, diff --git a/src/completion-listener.spec.ts b/src/completion-listener.spec.ts index 302d2c49..e1c383f1 100644 --- a/src/completion-listener.spec.ts +++ b/src/completion-listener.spec.ts @@ -24,7 +24,7 @@ const createController = (successCondition?: SuccessCondition) => const emitFakeCloseEvent = (command: FakeCommand, event?: Partial) => command.close.next(createFakeCloseEvent({ ...event, command, index: command.index })); -const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); +const flushPromises = () => new Promise((resolve) => setTimeout(resolve, 0)); describe('listen', () => { it('check for success once all commands have emitted at least a single close event', async () => { @@ -37,8 +37,8 @@ describe('listen', () => { commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); scheduler.flush(); - - await sleep(100); + // A broken implementantion will have called finallyCallback only after flushing promises + await flushPromises(); expect(finallyCallback).not.toHaveBeenCalled(); commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); @@ -63,6 +63,28 @@ describe('listen', () => { await expect(result).rejects.toEqual(expect.anything()); }); + + it('waits for manually restarted events to close', async () => { + const finallyCallback = jest.fn(); + const result = createController().listen(commands).finally(finallyCallback); + + commands[0].close.next(createFakeCloseEvent()); + commands[0].state = 'started'; + commands[1].close.next(createFakeCloseEvent()); + commands[2].close.next(createFakeCloseEvent()); + + scheduler.flush(); + // A broken implementantion will have called finallyCallback only after flushing promises + await flushPromises(); + expect(finallyCallback).not.toHaveBeenCalled(); + + commands[0].state = 'exited'; + commands[0].close.next(createFakeCloseEvent()); + scheduler.flush(); + + await expect(result).resolves.toEqual(expect.anything()); + expect(finallyCallback).toHaveBeenCalled(); + }); }); describe('Detect commands exit conditions', () => { diff --git a/src/completion-listener.ts b/src/completion-listener.ts index c32e6dcd..eaf72c21 100644 --- a/src/completion-listener.ts +++ b/src/completion-listener.ts @@ -90,7 +90,7 @@ export class CompletionListener { return Rx.lastValueFrom( Rx.combineLatest(closeStreams).pipe( - filter((exitInfos) => exitInfos.every(({ state }) => state !== 'started')), + filter(() => commands.every((command) => command.state !== 'started')), map((exitInfos) => exitInfos.sort( (first, second) => diff --git a/src/fixtures/fake-command.ts b/src/fixtures/fake-command.ts index 452a7815..6d0107fe 100644 --- a/src/fixtures/fake-command.ts +++ b/src/fixtures/fake-command.ts @@ -40,7 +40,6 @@ export const createFakeCloseEvent = (overrides?: Partial): CloseEven index: 0, killed: false, exitCode: 0, - state: 'exited', timings: { startDate: new Date(), endDate: new Date(), From 54818fb809766c5cf63f81c7e1e99c59e3bf9bb3 Mon Sep 17 00:00:00 2001 From: Gustavo Henke Date: Sun, 24 Mar 2024 14:39:56 +1100 Subject: [PATCH 10/10] Use emitFakeCloseEvent util more --- src/completion-listener.spec.ts | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/completion-listener.spec.ts b/src/completion-listener.spec.ts index e1c383f1..054d5926 100644 --- a/src/completion-listener.spec.ts +++ b/src/completion-listener.spec.ts @@ -32,32 +32,31 @@ describe('listen', () => { const result = createController().listen(commands).finally(finallyCallback); // Emitting multiple close events to mimic calling command `kill/start` APIs. - commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); + emitFakeCloseEvent(commands[0]); + emitFakeCloseEvent(commands[0]); + emitFakeCloseEvent(commands[0]); scheduler.flush(); // A broken implementantion will have called finallyCallback only after flushing promises await flushPromises(); expect(finallyCallback).not.toHaveBeenCalled(); - commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); + emitFakeCloseEvent(commands[1]); + emitFakeCloseEvent(commands[2]); scheduler.flush(); await expect(result).resolves.toEqual(expect.anything()); - expect(finallyCallback).toHaveBeenCalled(); }); it('takes last event emitted from each command', async () => { const result = createController().listen(commands); - commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); - commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); + emitFakeCloseEvent(commands[0], { exitCode: 0 }); + emitFakeCloseEvent(commands[0], { exitCode: 1 }); + emitFakeCloseEvent(commands[1], { exitCode: 0 }); + emitFakeCloseEvent(commands[2], { exitCode: 0 }); scheduler.flush(); @@ -68,10 +67,10 @@ describe('listen', () => { const finallyCallback = jest.fn(); const result = createController().listen(commands).finally(finallyCallback); - commands[0].close.next(createFakeCloseEvent()); + emitFakeCloseEvent(commands[0]); commands[0].state = 'started'; - commands[1].close.next(createFakeCloseEvent()); - commands[2].close.next(createFakeCloseEvent()); + emitFakeCloseEvent(commands[1]); + emitFakeCloseEvent(commands[2]); scheduler.flush(); // A broken implementantion will have called finallyCallback only after flushing promises @@ -79,7 +78,7 @@ describe('listen', () => { expect(finallyCallback).not.toHaveBeenCalled(); commands[0].state = 'exited'; - commands[0].close.next(createFakeCloseEvent()); + emitFakeCloseEvent(commands[0]); scheduler.flush(); await expect(result).resolves.toEqual(expect.anything());