Skip to content

Commit

Permalink
fix: support parsing empty responses
Browse files Browse the repository at this point in the history
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
  • Loading branch information
steveukx committed Jan 23, 2022
1 parent 08ac626 commit 91eb7fb
Show file tree
Hide file tree
Showing 9 changed files with 162 additions and 7 deletions.
21 changes: 19 additions & 2 deletions 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';
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
}
18 changes: 17 additions & 1 deletion 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 = {
Expand All @@ -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;
}
}

12 changes: 11 additions & 1 deletion 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];
Expand All @@ -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);
}
}
}
2 changes: 1 addition & 1 deletion simple-git/src/lib/utils/util.ts
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion simple-git/test/__fixtures__/setup-init.ts
Expand Up @@ -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);
}
Expand Down
87 changes: 87 additions & 0 deletions 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),
}))
});

});
7 changes: 7 additions & 0 deletions simple-git/test/unit/utils.spec.ts
Expand Up @@ -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 ']);
Expand Down
17 changes: 17 additions & 0 deletions simple-git/typings/response.d.ts
Expand Up @@ -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
*/
Expand Down
3 changes: 2 additions & 1 deletion simple-git/typings/simple-git.d.ts
Expand Up @@ -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>;

Expand Down

0 comments on commit 91eb7fb

Please sign in to comment.