From 0ab9e466137c801f97252f626136e205a34b9258 Mon Sep 17 00:00:00 2001 From: Pascal Jufer Date: Mon, 27 Jun 2022 12:16:29 +0200 Subject: [PATCH 1/3] Enhance test for logger option As suggested by @gustavohenke in https://github.com/open-cli-tools/concurrently/pull/334#discussion_r906512429 --- src/concurrently.spec.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/concurrently.spec.ts b/src/concurrently.spec.ts index 6eba78a4..29977807 100644 --- a/src/concurrently.spec.ts +++ b/src/concurrently.spec.ts @@ -3,9 +3,8 @@ import { concurrently, ConcurrentlyCommandInput, ConcurrentlyOptions } from './c import { createFakeProcess, FakeCommand } from './fixtures/fake-command'; import { FlowController } from './flow-control/flow-controller'; import { Logger } from './logger'; -import { OutputWriter } from './output-writer'; - -jest.mock('./output-writer'); +import { Writable } from 'stream'; +import { createMockInstance } from 'jest-create-mock-instance'; let spawn: SpawnCommand; let kill: KillProcess; @@ -48,10 +47,15 @@ it('spawns all commands', () => { expect(spawn).toHaveBeenCalledWith('kill', expect.objectContaining({})); }); -it('output writer is created if logger is passed in options', () => { +it('log output is passed to output stream if logger is specified in options', () => { const logger = new Logger({ hide: [] }); - create(['foo'], { logger }); - expect(OutputWriter).toHaveBeenCalledTimes(1); + const outputStream = createMockInstance(Writable); + create(['foo'], { logger, outputStream }); + logger.log('foo', 'bar'); + + expect(outputStream.write).toHaveBeenCalledTimes(2); + expect(outputStream.write).toHaveBeenCalledWith('foo'); + expect(outputStream.write).toHaveBeenCalledWith('bar'); }); it('spawns commands up to configured limit at once', () => { From 150d5db0adba53a7bf0bab991935c32c78a02fcf Mon Sep 17 00:00:00 2001 From: Pascal Jufer Date: Mon, 27 Jun 2022 12:17:45 +0200 Subject: [PATCH 2/3] Ensure OutputWriter is created only if outputStream is defined --- src/concurrently.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/concurrently.ts b/src/concurrently.ts index 8cff159e..e190bde8 100644 --- a/src/concurrently.ts +++ b/src/concurrently.ts @@ -176,7 +176,7 @@ export function concurrently( ); commands = handleResult.commands; - if (options.logger) { + if (options.logger && options.outputStream) { const outputWriter = new OutputWriter({ outputStream: options.outputStream, group: options.group, From d7fcb11806a992061fc95a358765b75013c63293 Mon Sep 17 00:00:00 2001 From: Pascal Jufer Date: Mon, 27 Jun 2022 12:19:50 +0200 Subject: [PATCH 3/3] Make printExitInfoTimingTable method private and test indirectly With this change we can be sure that the 'logger' is defined. Increases coverage on Node.js 12. --- src/flow-control/log-timings.spec.ts | 10 ++-------- src/flow-control/log-timings.ts | 10 +++++----- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/flow-control/log-timings.spec.ts b/src/flow-control/log-timings.spec.ts index 6a80a915..677f614c 100644 --- a/src/flow-control/log-timings.spec.ts +++ b/src/flow-control/log-timings.spec.ts @@ -109,20 +109,14 @@ it('does not log timings summary if there was an error', () => { }); it('logs the sorted timings summary when all processes close successfully', () => { - jest.spyOn(controller, 'printExitInfoTimingTable'); controller.handle(commands); commands[0].close.next(command0ExitInfo); commands[1].close.next(command1ExitInfo); + expect(logger.logGlobalEvent).toHaveBeenCalledTimes(1); + expect(logger.logGlobalEvent).toHaveBeenCalledWith('Timings:'); expect(logger.logTable).toHaveBeenCalledTimes(1); - - // un-sorted ie by finish order - expect(controller.printExitInfoTimingTable).toHaveBeenCalledWith([ - command0ExitInfo, - command1ExitInfo, - ]); - // sorted by duration expect(logger.logTable).toHaveBeenCalledWith([ LogTimings.mapCloseEventToTimingInfo(command1ExitInfo), diff --git a/src/flow-control/log-timings.ts b/src/flow-control/log-timings.ts index 59884eb7..5e32eebb 100644 --- a/src/flow-control/log-timings.ts +++ b/src/flow-control/log-timings.ts @@ -51,15 +51,15 @@ export class LogTimings implements FlowController { this.timestampFormat = timestampFormat; } - printExitInfoTimingTable(exitInfos: CloseEvent[]) { + private printExitInfoTimingTable(exitInfos: CloseEvent[]) { const exitInfoTable = _(exitInfos) .sortBy(({ timings }) => timings.durationSeconds) .reverse() .map(LogTimings.mapCloseEventToTimingInfo) .value(); - this.logger?.logGlobalEvent('Timings:'); - this.logger?.logTable(exitInfoTable); + this.logger.logGlobalEvent('Timings:'); + this.logger.logTable(exitInfoTable); return exitInfos; } @@ -73,14 +73,14 @@ export class LogTimings implements FlowController { command.timer.subscribe(({ startDate, endDate }) => { if (!endDate) { const formattedStartDate = formatDate(startDate, this.timestampFormat); - this.logger?.logCommandEvent( + this.logger.logCommandEvent( `${command.command} started at ${formattedStartDate}`, command ); } else { const durationMs = endDate.getTime() - startDate.getTime(); const formattedEndDate = formatDate(endDate, this.timestampFormat); - this.logger?.logCommandEvent( + this.logger.logCommandEvent( `${ command.command } stopped at ${formattedEndDate} after ${durationMs.toLocaleString()}ms`,