From bae6fe58022f84430998c30070d951cd6429c47d Mon Sep 17 00:00:00 2001 From: Oded Goldglas <49120070+odeadglaz@users.noreply.github.com> Date: Sun, 24 Mar 2024 06:17:30 +0200 Subject: [PATCH] Ensuring completion-listener.ts listen to a single close event emitter (#464) --- src/command.ts | 1 + src/completion-listener.spec.ts | 297 +++++++++++++++++++------------- src/completion-listener.ts | 19 +- 3 files changed, 194 insertions(+), 123 deletions(-) diff --git a/src/command.ts b/src/command.ts index 815b507e..ef9a34f9 100644 --- a/src/command.ts +++ b/src/command.ts @@ -56,6 +56,7 @@ export interface CloseEvent { * The exit code or signal for the command. */ exitCode: string | number; + timings: { startDate: Date; endDate: Date; diff --git a/src/completion-listener.spec.ts b/src/completion-listener.spec.ts index 905b0bcc..054d5926 100644 --- a/src/completion-listener.spec.ts +++ b/src/completion-listener.spec.ts @@ -24,186 +24,249 @@ 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); +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 () => { + const finallyCallback = jest.fn(); + const result = createController().listen(commands).finally(finallyCallback); + + // Emitting multiple close events to mimic calling command `kill/start` APIs. + 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[0].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); + emitFakeCloseEvent(commands[1]); + emitFakeCloseEvent(commands[2]); scheduler.flush(); - return expect(result).resolves.toEqual(expect.anything()); + await expect(result).resolves.toEqual(expect.anything()); + expect(finallyCallback).toHaveBeenCalled(); }); - it('fails if one of the processes exited with non-0 code', () => { + it('takes last event emitted from each command', async () => { const result = createController().listen(commands); - commands[0].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[1].close.next(createFakeCloseEvent({ exitCode: 1 })); - 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(); - return expect(result).rejects.toEqual(expect.anything()); + await expect(result).rejects.toEqual(expect.anything()); }); -}); -describe('with success condition set to first', () => { - it('succeeds if first process to exit has code 0', () => { - const result = createController('first').listen(commands); + it('waits for manually restarted events to close', async () => { + const finallyCallback = jest.fn(); + const result = createController().listen(commands).finally(finallyCallback); - commands[1].close.next(createFakeCloseEvent({ exitCode: 0 })); - commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); - commands[2].close.next(createFakeCloseEvent({ exitCode: 1 })); + emitFakeCloseEvent(commands[0]); + commands[0].state = 'started'; + emitFakeCloseEvent(commands[1]); + emitFakeCloseEvent(commands[2]); + + scheduler.flush(); + // A broken implementantion will have called finallyCallback only after flushing promises + await flushPromises(); + expect(finallyCallback).not.toHaveBeenCalled(); + commands[0].state = 'exited'; + emitFakeCloseEvent(commands[0]); scheduler.flush(); - return expect(result).resolves.toEqual(expect.anything()); + await expect(result).resolves.toEqual(expect.anything()); + expect(finallyCallback).toHaveBeenCalled(); }); +}); - it('fails if first process to exit has non-0 code', () => { - 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: 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: 0 })); + commands[2].close.next(createFakeCloseEvent({ exitCode: 0 })); - scheduler.flush(); + scheduler.flush(); - return expect(result).rejects.toEqual(expect.anything()); - }); -}); + return expect(result).resolves.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); + 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).resolves.toEqual(expect.anything()); + return expect(result).rejects.toEqual(expect.anything()); + }); }); - it('fails if last process to exit has non-0 code', () => { - 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: 0 })); - commands[0].close.next(createFakeCloseEvent({ exitCode: 1 })); - commands[2].close.next(createFakeCloseEvent({ exitCode: 1 })); + 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).rejects.toEqual(expect.anything()); - }); -}); + return expect(result).resolves.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); + it('fails if first process to exit has non-0 code', () => { + const result = createController('first').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).rejects.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); + 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: 0 }); + 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(`fails if command ${nameOrIndex} exits with non-0 code`, () => { - 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: 0 }); - emitFakeCloseEvent(commands[1], { exitCode: 1 }); - 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).rejects.toEqual(expect.anything()); + return expect(result).rejects.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); + 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: 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: 1 }); - 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(`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: 0 }); - scheduler.flush(); + emitFakeCloseEvent(commands[0], { exitCode: 1 }); + emitFakeCloseEvent(commands[1], { exitCode: 0 }); + 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).resolves.toEqual(expect.anything()); + }); - emitFakeCloseEvent(commands[0], { exitCode: 0 }); - emitFakeCloseEvent(commands[1], { exitCode: 1 }); - emitFakeCloseEvent(commands[2], { exitCode: 0 }); + it(`fails if command ${nameOrIndex} exits with non-0 code`, () => { + const result = createController(condition).listen(commands); - scheduler.flush(); + emitFakeCloseEvent(commands[0], { exitCode: 0 }); + emitFakeCloseEvent(commands[1], { exitCode: 1 }); + emitFakeCloseEvent(commands[2], { exitCode: 0 }); - 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).rejects.toEqual(expect.anything()); + }); - emitFakeCloseEvent(commands[0], { exitCode: 1 }); - emitFakeCloseEvent(commands[1], { exitCode: 1 }); - emitFakeCloseEvent(commands[2], { exitCode: 0 }); + it(`fails if some commands ${nameOrIndex} exit with non-0 code`, () => { + const result = createController(condition).listen(commands); - scheduler.flush(); + emitFakeCloseEvent(commands[0], { exitCode: 1 }); + emitFakeCloseEvent(commands[1], { exitCode: 0 }); + emitFakeCloseEvent(commands[2], { exitCode: 1 }); + + scheduler.flush(); + + return expect(result).resolves.toEqual(expect.anything()); + }); + + it(`fails if command ${nameOrIndex} doesn't exist`, () => { + const result = createController(condition).listen([commands[0]]); - return expect(result).rejects.toEqual(expect.anything()); + emitFakeCloseEvent(commands[0], { 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]]); + 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..eaf72c21 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'; @@ -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); } @@ -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) @@ -87,9 +87,16 @@ export class CompletionListener { */ listen(commands: Command[]): Promise { const closeStreams = commands.map((command) => command.close); + return Rx.lastValueFrom( - Rx.merge(...closeStreams).pipe( - bufferCount(closeStreams.length), + Rx.combineLatest(closeStreams).pipe( + filter(() => commands.every((command) => command.state !== 'started')), + map((exitInfos) => + exitInfos.sort( + (first, second) => + first.timings.endDate.getTime() - second.timings.endDate.getTime(), + ), + ), switchMap((exitInfos) => this.isSuccess(exitInfos) ? this.emitWithScheduler(Rx.of(exitInfos))