From d562af90fe79f96817af2cdcb2d42c79b528338c Mon Sep 17 00:00:00 2001 From: Mat Dodgson Date: Fri, 15 Aug 2025 14:36:35 +1000 Subject: [PATCH] Remove on exit --- .../plugins/completion-detection.plugin.ts | 14 +------- simple-git/src/lib/plugins/timout-plugin.ts | 2 -- .../test/integration/plugin.progress.spec.ts | 2 +- .../test/unit/__fixtures__/child-processes.ts | 13 ++++--- .../test/unit/__mocks__/mock-child-process.ts | 3 +- simple-git/test/unit/git-executor.spec.ts | 36 ++----------------- .../plugin.completion-detection.spec.ts | 27 +++----------- 7 files changed, 15 insertions(+), 82 deletions(-) diff --git a/simple-git/src/lib/plugins/completion-detection.plugin.ts b/simple-git/src/lib/plugins/completion-detection.plugin.ts index b8a7cc5c..07362f1f 100644 --- a/simple-git/src/lib/plugins/completion-detection.plugin.ts +++ b/simple-git/src/lib/plugins/completion-detection.plugin.ts @@ -7,34 +7,23 @@ const never = deferred().promise; export function completionDetectionPlugin({ onClose = true, - onExit = 50, }: SimpleGitPluginConfig['completion'] = {}): SimpleGitPlugin<'spawn.after'> { function createEvents() { let exitCode = -1; const events = { close: deferred(), closeTimeout: deferred(), - exit: deferred(), - exitTimeout: deferred(), }; - const result = Promise.race([ - onClose === false ? never : events.closeTimeout.promise, - onExit === false ? never : events.exitTimeout.promise, - ]); + const result = onClose === false ? never : events.closeTimeout.promise; configureTimeout(onClose, events.close, events.closeTimeout); - configureTimeout(onExit, events.exit, events.exitTimeout); return { close(code: number) { exitCode = code; events.close.done(); }, - exit(code: number) { - exitCode = code; - events.exit.done(); - }, get exitCode() { return exitCode; }, @@ -67,7 +56,6 @@ export function completionDetectionPlugin({ spawned.on('error', quickClose); spawned.on('close', (code: number) => events.close(code)); - spawned.on('exit', (code: number) => events.exit(code)); try { await events.result; diff --git a/simple-git/src/lib/plugins/timout-plugin.ts b/simple-git/src/lib/plugins/timout-plugin.ts index 684fbec0..106b7e56 100644 --- a/simple-git/src/lib/plugins/timout-plugin.ts +++ b/simple-git/src/lib/plugins/timout-plugin.ts @@ -22,7 +22,6 @@ export function timeoutPlugin({ function stop() { context.spawned.stdout?.off('data', wait); context.spawned.stderr?.off('data', wait); - context.spawned.off('exit', stop); context.spawned.off('close', stop); timeout && clearTimeout(timeout); } @@ -34,7 +33,6 @@ export function timeoutPlugin({ stdOut && context.spawned.stdout?.on('data', wait); stdErr && context.spawned.stderr?.on('data', wait); - context.spawned.on('exit', stop); context.spawned.on('close', stop); wait(); diff --git a/simple-git/test/integration/plugin.progress.spec.ts b/simple-git/test/integration/plugin.progress.spec.ts index 7430ad44..b1712dcc 100644 --- a/simple-git/test/integration/plugin.progress.spec.ts +++ b/simple-git/test/integration/plugin.progress.spec.ts @@ -33,7 +33,7 @@ describe('progress-monitor', () => { expect(update.progress).toBeGreaterThanOrEqual(previous); return update.progress; }, 0); - }); + }, 10000); }); function progressEventsAtStage(mock: jest.Mock, stage: string) { diff --git a/simple-git/test/unit/__fixtures__/child-processes.ts b/simple-git/test/unit/__fixtures__/child-processes.ts index 70e7c7fd..1fc8e2db 100644 --- a/simple-git/test/unit/__fixtures__/child-processes.ts +++ b/simple-git/test/unit/__fixtures__/child-processes.ts @@ -12,7 +12,7 @@ export async function writeToStdErr(data = '') { throw new Error(`writeToStdErr unable to find matching child process`); } - if (proc.$emitted('exit')) { + if (proc.$emitted('close')) { throw new Error('writeToStdErr: attempting to write to an already closed process'); } @@ -27,7 +27,7 @@ export async function writeToStdOut(data = '') { throw new Error(`writeToStdOut unable to find matching child process`); } - if (proc.$emitted('exit')) { + if (proc.$emitted('close')) { throw new Error('writeToStdOut: attempting to write to an already closed process'); } @@ -45,7 +45,7 @@ export async function closeWithError(stack = 'CLOSING WITH ERROR', code = EXIT_C export async function closeWithSuccess(message = '') { await wait(); - const match = mockChildProcessModule.$matchingChildProcess((p) => !p.$emitted('exit')); + const match = mockChildProcessModule.$matchingChildProcess((p) => !p.$emitted('close')); if (!match) { throw new Error(`closeWithSuccess unable to find matching child process`); } @@ -82,15 +82,14 @@ export function theChildProcessMatching(what: string[] | ((mock: MockChildProces } async function exitChildProcess(proc: MockChildProcess, data: string | null, exitSignal: number) { - if (proc.$emitted('exit')) { - throw new Error('exitChildProcess: attempting to exit an already closed process'); + if (proc.$emitted('close')) { + throw new Error('exitChildProcess: attempting to close an already closed process'); } if (typeof data === 'string') { proc.stdout.$emit('data', Buffer.from(data)); } - // exit/close events are bound to the process itself - proc.$emit('exit', exitSignal); + // close event is bound to the process itself proc.$emit('close', exitSignal); } diff --git a/simple-git/test/unit/__mocks__/mock-child-process.ts b/simple-git/test/unit/__mocks__/mock-child-process.ts index 26459ab9..da4d2a44 100644 --- a/simple-git/test/unit/__mocks__/mock-child-process.ts +++ b/simple-git/test/unit/__mocks__/mock-child-process.ts @@ -33,11 +33,10 @@ class MockEventTargetImpl implements MockEventTarget { }; public kill = jest.fn((_signal = 'SIGINT') => { - if (this.$emitted('exit')) { + if (this.$emitted('close')) { throw new Error('MockEventTarget:kill called on process after exit'); } - this.$emit('exit', 1); this.$emit('close', 1); }); diff --git a/simple-git/test/unit/git-executor.spec.ts b/simple-git/test/unit/git-executor.spec.ts index 96e8e458..4e4a11e0 100644 --- a/simple-git/test/unit/git-executor.spec.ts +++ b/simple-git/test/unit/git-executor.spec.ts @@ -13,7 +13,7 @@ async function withStdErr() { } async function childProcessEmits( - event: 'close' | 'exit', + event: 'close', code: number, before?: () => Promise ) { @@ -57,16 +57,6 @@ describe('git-executor', () => { await thenTheTaskHasCompleted(); }); - it('with no stdErr and just an exit event, terminates after a delay', async () => { - givenTheTaskIsAdded(); - - await childProcessEmits('exit', 0); - await thenTheTaskHasNotCompleted(); - - await aWhile(); - await thenTheTaskHasCompleted(); - }); - it('with stdErr and just a close event, terminates immediately', async () => { givenTheTaskIsAdded(); @@ -74,13 +64,6 @@ describe('git-executor', () => { await thenTheTaskHasCompleted(); }); - it('with stdErr and just an exit event, terminates immediately', async () => { - givenTheTaskIsAdded(); - - await childProcessEmits('exit', 0, withStdErr); - await thenTheTaskHasCompleted(); - }); - it('with stdOut and just a close event, terminates immediately', async () => { givenTheTaskIsAdded(); @@ -88,25 +71,10 @@ describe('git-executor', () => { await thenTheTaskHasCompleted(); }); - it('with stdOut and just an exit event, terminates immediately', async () => { - givenTheTaskIsAdded(); - - await childProcessEmits('exit', 0, withStdOut); - await thenTheTaskHasCompleted(); - }); - - it('with both cancel and exit events, only terminates once', async () => { + it('with 2 close events, only terminates once', async () => { givenTheTaskIsAdded(); await childProcessEmits('close', 0); - await childProcessEmits('exit', 0); - await thenTheTaskHasCompleted(); - }); - - it('with both exit and cancel events, only terminates once', async () => { - givenTheTaskIsAdded(); - - await childProcessEmits('exit', 0); await childProcessEmits('close', 0); await thenTheTaskHasCompleted(); }); diff --git a/simple-git/test/unit/plugins/plugin.completion-detection.spec.ts b/simple-git/test/unit/plugins/plugin.completion-detection.spec.ts index 5c5e23d9..04347c9e 100644 --- a/simple-git/test/unit/plugins/plugin.completion-detection.spec.ts +++ b/simple-git/test/unit/plugins/plugin.completion-detection.spec.ts @@ -2,13 +2,12 @@ import { newSimpleGit, theChildProcessMatching, wait } from '../__fixtures__'; import { MockChildProcess } from '../__mocks__/mock-child-process'; describe('completionDetectionPlugin', () => { - function process(proc: MockChildProcess, data: string, close = false, exit = false) { + function process(proc: MockChildProcess, data: string, close = false) { proc.stdout.$emit('data', Buffer.from(data)); close && proc.$emit('close', 1); - exit && proc.$emit('exit', 1); } - it('can respond to just close events', async () => { + it('can respond to close events', async () => { const git = newSimpleGit({ completion: { onClose: true, @@ -20,27 +19,9 @@ describe('completionDetectionPlugin', () => { await wait(); - process(theChildProcessMatching(['foo']), 'foo', false, true); - process(theChildProcessMatching(['bar']), 'bar', true, false); + process(theChildProcessMatching(['foo']), 'foo', false); + process(theChildProcessMatching(['bar']), 'bar', true); expect(await output).toBe('bar'); }); - - it('can respond to just exit events', async () => { - const git = newSimpleGit({ - completion: { - onClose: false, - onExit: true, - }, - }); - - const output = Promise.race([git.raw('foo'), git.raw('bar')]); - - await wait(); - - process(theChildProcessMatching(['foo']), 'foo', false, true); - process(theChildProcessMatching(['bar']), 'bar', true, false); - - expect(await output).toBe('foo'); - }); });