fix: support parsing empty responses

fix: support parsing empty responses

- parsers should treat `undefined` in either `stdout` or `stderr` the same as an empty string

fix: detect fatal exceptions in `git pull`

- In the case that a `git pull` fails with a recognised fatal error (eg: a `--ff-only` pull cannot be fast-forwarded), the exception thrown will be a `GitResponseError<PullFailedResult>` with summary details of the exception rather than a standard process terminated exception

Closes #713

Author: steveukx

simple-git/src/lib/parsers/parse-pull.ts

@@ -1,5 +1,5 @@
-import { PullDetail, PullResult, RemoteMessages } from '../../../typings';
-import { PullSummary } from '../responses/PullSummary';
+import { PullDetail, PullFailedResult, PullResult, RemoteMessages } from '../../../typings';
+import { PullFailedSummary, PullSummary } from '../responses/PullSummary';
 import { TaskParser } from '../types';
 import { append, LineParser, parseStringResponse } from '../utils';
 import { parseRemoteMessages } from './parse-remote-messages';
@@ -35,6 +35,17 @@ const parsers: LineParser<PullResult>[] = [
    }),
 ];
 
+const errorParsers: LineParser<PullFailedResult>[] = [
+   new LineParser(/^from\s(.+)$/i, (result, [remote]) => void (result.remote = remote)),
+   new LineParser(/^fatal:\s(.+)$/, (result, [message]) => void (result.message = message)),
+   new LineParser(/([a-z0-9]+)\.\.([a-z0-9]+)\s+(\S+)\s+->\s+(\S+)$/, (result, [hashLocal, hashRemote, branchLocal, branchRemote]) => {
+      result.branch.local = branchLocal;
+      result.hash.local = hashLocal;
+      result.branch.remote = branchRemote;
+      result.hash.remote = hashRemote;
+   }),
+];
+
 export const parsePullDetail: TaskParser<string, PullDetail> = (stdOut, stdErr) => {
    return parseStringResponse(new PullSummary(), parsers, stdOut, stdErr);
 }
@@ -46,3 +57,9 @@ export const parsePullResult: TaskParser<string, PullResult> = (stdOut, stdErr)
       parseRemoteMessages<RemoteMessages>(stdOut, stdErr),
    );
 }
+
+export function parsePullErrorResult(stdOut: string, stdErr: string) {
+   const pullError = parseStringResponse(new PullFailedSummary(), errorParsers, stdOut, stdErr);
+
+   return pullError.message && pullError;
+}

simple-git/src/lib/responses/PullSummary.ts

@@ -1,4 +1,4 @@
-import { PullDetailFileChanges, PullDetailSummary, PullResult } from '../../../typings';
+import { PullDetailFileChanges, PullDetailSummary, PullFailedResult, PullResult } from '../../../typings';
 
 export class PullSummary implements PullResult {
    public remoteMessages = {
@@ -16,4 +16,20 @@ export class PullSummary implements PullResult {
    };
 }
 
+export class PullFailedSummary implements PullFailedResult {
+   remote = '';
+   hash = {
+      local: '',
+      remote: '',
+   };
+   branch = {
+      local: '',
+      remote: '',
+   };
+   message = '';
+
+   toString() {
+      return this.message;
+   }
+}
 

simple-git/src/lib/tasks/pull.ts

@@ -1,6 +1,8 @@
 import { PullResult } from '../../../typings';
-import { parsePullResult } from '../parsers/parse-pull';
+import { GitResponseError } from '../errors/git-response-error';
+import { parsePullErrorResult, parsePullResult } from '../parsers/parse-pull';
 import { Maybe, StringTask } from '../types';
+import { bufferToString } from '../utils';
 
 export function pullTask(remote: Maybe<string>, branch: Maybe<string>, customArgs: string[]): StringTask<PullResult> {
    const commands: string[] = ['pull', ...customArgs];
@@ -13,6 +15,14 @@ export function pullTask(remote: Maybe<string>, branch: Maybe<string>, customArg
       format: 'utf-8',
       parser(stdOut, stdErr): PullResult {
          return parsePullResult(stdOut, stdErr);
+      },
+      onError(result, _error, _done, fail) {
+         const pullError = parsePullErrorResult(bufferToString(result.stdOut), bufferToString(result.stdErr));
+         if (pullError) {
+            return fail(new GitResponseError(pullError));
+         }
+
+         fail(_error);
       }
    }
 }

simple-git/src/lib/utils/util.ts

@@ -55,7 +55,7 @@ function isArrayLike(input: any): input is ArrayLike {
    return !!(input && typeof input.length === 'number');
 }
 
-export function toLinesWithContent(input: string, trimmed = true, separator = '\n'): string[] {
+export function toLinesWithContent(input = '', trimmed = true, separator = '\n'): string[] {
    return input.split(separator)
       .reduce((output, line) => {
          const lineContent = trimmed ? line.trim() : line;

simple-git/test/__fixtures__/setup-init.ts

@@ -4,7 +4,7 @@ import { SimpleGitTestContext } from './create-test-context';
 export const GIT_USER_NAME = 'Simple Git Tests';
 export const GIT_USER_EMAIL = 'tests@simple-git.dev';
 
-export async function setUpInit ({git}: SimpleGitTestContext) {
+export async function setUpInit({git}: Pick<SimpleGitTestContext, 'git'>) {
    await git.raw('-c', 'init.defaultbranch=master', 'init');
    await configureGitCommitter(git);
 }

simple-git/test/integration/pull-fails-ff.spec.ts

@@ -0,0 +1,87 @@
+import { promiseError } from '@kwsites/promise-result';
+import { GitResponseError, PullFailedResult } from '../../typings';
+import { createTestContext, like, newSimpleGit, setUpInit, SimpleGitTestContext } from '../__fixtures__';
+
+describe('pull --ff-only', () => {
+   let context: SimpleGitTestContext;
+
+   beforeEach(async () => context = await createTestContext());
+   beforeEach(async () => {
+      const upstream = await context.dir('upstream');
+      const local = context.path('local');
+      await context.file(['upstream', 'file']);
+
+      await givenRemote(upstream);
+      await givenLocal(upstream, local);
+   });
+
+   async function givenLocal(upstream: string, local: string) {
+      await newSimpleGit(context.root).clone(upstream, local);
+      await setUpInit({git: newSimpleGit(local)});
+   }
+
+   async function givenRemote(upstream: string) {
+      const git = newSimpleGit(upstream);
+      await setUpInit({git});
+      await git.add('.');
+      await git.commit('first');
+   }
+
+   async function givenRemoteFileChanged() {
+      await context.file(['upstream', 'file'], 'new remote file content');
+      await newSimpleGit(context.path('upstream')).add('.').commit('remote updated');
+   }
+
+   async function givenLocalFileChanged() {
+      await context.file(['local', 'file'], 'new local file content');
+      await newSimpleGit(context.path('local')).add('.').commit('local updated');
+   }
+
+   it('allows fast-forward when there are no changes local or remote', async () => {
+      const git = newSimpleGit(context.path('local'));
+      const result = await git.pull(['--ff-only']);
+
+      expect(result.files).toEqual([]);
+   });
+
+   it('allows fast-forward when there are some remote but no local changes', async () => {
+      await givenRemoteFileChanged();
+
+      const git = newSimpleGit(context.path('local'));
+      const result = await git.pull(['--ff-only']);
+
+      expect(result.files).toEqual(['file']);
+   });
+
+   it('allows fast-forward when there are no remote but some local changes', async () => {
+      await givenLocalFileChanged();
+
+      const git = newSimpleGit(context.path('local'));
+      const result = await git.pull(['--ff-only']);
+
+      expect(result.files).toEqual([]);
+   });
+
+   it('fails fast-forward when there are both remote and local changes', async () => {
+      await givenLocalFileChanged();
+      await givenRemoteFileChanged();
+
+      const git = newSimpleGit(context.path('local'));
+      const err = await promiseError<GitResponseError<PullFailedResult>>(git.pull(['--ff-only']));
+
+      expect(err?.git.message).toMatch('Not possible to fast-forward, aborting');
+      expect(err?.git).toEqual(like({
+         remote: context.path('upstream'),
+         hash: {
+            local: expect.any(String),
+            remote: expect.any(String),
+         },
+         branch: {
+            local: expect.any(String),
+            remote: expect.any(String),
+         },
+         message: String(err?.git),
+      }))
+   });
+
+});

simple-git/test/unit/utils.spec.ts

@@ -60,6 +60,13 @@ describe('utils', () => {
 
    describe('content', () => {
 
+      it('caters for empty values', () => {
+         expect(toLinesWithContent()).toEqual([]);
+         expect(toLinesWithContent(undefined, false)).toEqual([]);
+         expect(toLinesWithContent('')).toEqual([]);
+         expect(toLinesWithContent('', false)).toEqual([]);
+      });
+
       it('filters lines with content', () => {
          expect(toLinesWithContent(' \n content \n\n')).toEqual(['content']);
          expect(toLinesWithContent(' \n content \n\n', false)).toEqual([' ', ' content ']);

simple-git/typings/response.d.ts

@@ -256,6 +256,23 @@ export interface PullDetail {
 export interface PullResult extends PullDetail, RemoteMessageResult {
 }
 
+/**
+ * Wrapped with the `GitResponseError` as the exception thrown from a `git.pull` task
+ * to provide additional detail as to what failed.
+ */
+export interface PullFailedResult {
+   remote: string,
+   hash: {
+      local: string;
+      remote: string;
+   },
+   branch: {
+      local: string;
+      remote: string;
+   },
+   message: string;
+}
+
 /**
  * Represents file name changes in a StatusResult
  */

simple-git/typings/simple-git.d.ts

@@ -442,7 +442,8 @@ export interface SimpleGit extends SimpleGitBase {
    mv(from: string | string[], to: string, callback?: types.SimpleGitTaskCallback<resp.MoveSummary>): Response<resp.MoveSummary>;
 
    /**
-    * Fetch from and integrate with another repository or a local branch.
+    * Fetch from and integrate with another repository or a local branch. In the case that the `git pull` fails with a
+    * recognised fatal error, the exception thrown by this function will be a `GitResponseError<PullFailedResult>`.
     */
    pull(remote?: string, branch?: string, options?: types.TaskOptions, callback?: types.SimpleGitTaskCallback<resp.PullResult>): Response<resp.PullResult>;