Skip to content

Commit

Permalink
Update metadata commands to be more resilient
Browse files Browse the repository at this point in the history
1. Shorter timeouts, but errors will just lead to undefined
2. Don't pipe to `head` or `wc` as they may not exist
  • Loading branch information
tmeasday committed Nov 13, 2024
1 parent 6c98eeb commit 3221fcb
Show file tree
Hide file tree
Showing 6 changed files with 177 additions and 128 deletions.
95 changes: 95 additions & 0 deletions node-src/git/execGit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import { createInterface } from 'node:readline';

import { execaCommand } from 'execa';

import gitNoCommits from '../ui/messages/errors/gitNoCommits';
import gitNotInitialized from '../ui/messages/errors/gitNotInitialized';
import gitNotInstalled from '../ui/messages/errors/gitNotInstalled';

/**
* Execute a Git command in the local terminal.
*
* @param command The command to execute.
* @param options Execa options
*
* @returns The result of the command from the terminal.
*/
export async function execGitCommand(
command: string,
options?: Parameters<typeof execaCommand>[1]
) {
try {
const { all } = await execaCommand(command, {
env: { LANG: 'C', LC_ALL: 'C' }, // make sure we're speaking English
timeout: 20_000, // 20 seconds
all: true, // interleave stdout and stderr
shell: true, // we'll deal with escaping ourselves (for now)
...options,
});

if (all === undefined) {
throw new Error(`Unexpected missing git command output for command: '${command}`);
}

return all.toString();
} catch (error) {
const { message } = error;

if (message.includes('not a git repository')) {
throw new Error(gitNotInitialized({ command }));
}

if (message.includes('git not found')) {
throw new Error(gitNotInstalled({ command }));
}

if (message.includes('does not have any commits yet')) {
throw new Error(gitNoCommits({ command }));
}

throw error;
}
}

/**
* Execute a Git command in the local terminal and just get the first line.
*
* @param command The command to execute.
* @param options Execa options
*
* @returns The first line of the command from the terminal.
*/
export async function execGitCommandOneLine(
command: string,
options?: Parameters<typeof execaCommand>[1]
) {
const process = execaCommand(command, {
env: { LANG: 'C', LC_ALL: 'C' }, // make sure we're speaking English
timeout: 20_000, // 20 seconds
all: true, // interleave stdout and stderr
shell: true, // we'll deal with escaping ourselves (for now)
...options,
});

return Promise.race([
// This promise will resolve only if there is an error or it times out
(async () => {
await process;
throw new Error(`Unexpected missing git command output for command: '${command}`);
})(),
// We expect this promise to resolve first
new Promise<string>((resolve, reject) => {
if (!process.stdout) {
return reject(new Error('Unexpected missing stdout'));
}

const rl = createInterface(process.stdout);
rl.once('line', (line) => {
rl.close();
process.kill();

resolve(line);
});
}),
]);
}
3 changes: 2 additions & 1 deletion node-src/git/getParentCommits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import gql from 'fake-tag';

import { localBuildsSpecifier } from '../lib/localBuildsSpecifier';
import { Context } from '../types';
import { commitExists, execGitCommand } from './git';
import { execGitCommand } from './execGit';
import { commitExists } from './git';

export const FETCH_N_INITIAL_BUILD_COMMITS = 20;

Expand Down
99 changes: 35 additions & 64 deletions node-src/git/git.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { execaCommand } from 'execa';
import { afterEach, describe, expect, it, vi } from 'vitest';

import * as execGit from './execGit';
import {
findFilesFromRepositoryRoot,
getCommit,
Expand All @@ -14,21 +14,19 @@ import {
NULL_BYTE,
} from './git';

vi.mock('execa');
vi.mock('./execGit');

const command = vi.mocked(execaCommand);
const execGitCommand = vi.mocked(execGit.execGitCommand);
const execGitCommandOneLine = vi.mocked(execGit.execGitCommandOneLine);

afterEach(() => {
vi.clearAllMocks();
});

describe('getCommit', () => {
it('parses log output', async () => {
command.mockImplementation(
() =>
Promise.resolve({
all: `19b6c9c5b3d34d9fc55627fcaf8a85bd5d5e5b2a ## 1696588814 ## info@ghengeveld.nl ## Gert Hengeveld`,
}) as any
execGitCommand.mockResolvedValue(
`19b6c9c5b3d34d9fc55627fcaf8a85bd5d5e5b2a ## 1696588814 ## info@ghengeveld.nl ## Gert Hengeveld`
);
expect(await getCommit()).toEqual({
commit: '19b6c9c5b3d34d9fc55627fcaf8a85bd5d5e5b2a',
Expand All @@ -39,16 +37,11 @@ describe('getCommit', () => {
});

it('ignores gpg signature information', async () => {
command.mockImplementation(
() =>
Promise.resolve({
all: `
gpg: Signature made Fri Oct 6 12:40:14 2023 CEST
execGitCommand.mockResolvedValue(
`gpg: Signature made Fri Oct 6 12:40:14 2023 CEST
gpg: using RSA key 4AEE18F83AFDEB23
gpg: Can't check signature: No public key
19b6c9c5b3d34d9fc55627fcaf8a85bd5d5e5b2a ## 1696588814 ## info@ghengeveld.nl ## Gert Hengeveld
`.trim(),
}) as any
19b6c9c5b3d34d9fc55627fcaf8a85bd5d5e5b2a ## 1696588814 ## info@ghengeveld.nl ## Gert Hengeveld`.trim()
);
expect(await getCommit()).toEqual({
commit: '19b6c9c5b3d34d9fc55627fcaf8a85bd5d5e5b2a',
Expand All @@ -61,47 +54,35 @@ gpg: Can't check signature: No public key

describe('getSlug', () => {
it('returns the slug portion of the git url', async () => {
command.mockImplementation(
() => Promise.resolve({ all: 'git@github.com:chromaui/chromatic-cli.git' }) as any
);
execGitCommand.mockResolvedValue('git@github.com:chromaui/chromatic-cli.git');
expect(await getSlug()).toBe('chromaui/chromatic-cli');

command.mockImplementation(
() => Promise.resolve({ all: 'https://github.com/chromaui/chromatic-cli' }) as any
);
execGitCommand.mockResolvedValue('https://github.com/chromaui/chromatic-cli');
expect(await getSlug()).toBe('chromaui/chromatic-cli');

command.mockImplementation(
() => Promise.resolve({ all: 'https://gitlab.com/foo/bar.baz.git' }) as any
);
execGitCommand.mockResolvedValue('https://gitlab.com/foo/bar.baz.git');
expect(await getSlug()).toBe('foo/bar.baz');
});
});

describe('hasPreviousCommit', () => {
it('returns true if a commit is found', async () => {
command.mockImplementation(
() => Promise.resolve({ all: `19b6c9c5b3d34d9fc55627fcaf8a85bd5d5e5b2a` }) as any
);
execGitCommand.mockResolvedValue(`19b6c9c5b3d34d9fc55627fcaf8a85bd5d5e5b2a`);
expect(await hasPreviousCommit()).toEqual(true);
});

it('returns false if no commit is found', async () => {
command.mockImplementation(() => Promise.resolve({ all: `` }) as any);
execGitCommand.mockResolvedValue(``);
expect(await hasPreviousCommit()).toEqual(false);
});

it('ignores gpg signature information', async () => {
command.mockImplementation(
() =>
Promise.resolve({
all: `
execGitCommand.mockResolvedValue(
`
gpg: Signature made Fri Oct 6 12:40:14 2023 CEST
gpg: using RSA key 4AEE18F83AFDEB23
gpg: Can't check signature: No public key
19b6c9c5b3d34d9fc55627fcaf8a85bd5d5e5b2a
`.trim(),
}) as any
19b6c9c5b3d34d9fc55627fcaf8a85bd5d5e5b2a`.trim()
);
expect(await hasPreviousCommit()).toEqual(true);
});
Expand All @@ -124,55 +105,46 @@ describe('findFilesFromRepositoryRoot', () => {
const filesFound = ['package.json', 'another/package/package.json'];

// first call from getRepositoryRoot()
command.mockImplementationOnce(
() =>
Promise.resolve({
all: '/root',
}) as any
);

command.mockImplementationOnce(
() =>
Promise.resolve({
all: filesFound.join(NULL_BYTE),
}) as any
);
execGitCommand.mockResolvedValueOnce('/root');
execGitCommand.mockResolvedValueOnce(filesFound.join(NULL_BYTE));

const results = await findFilesFromRepositoryRoot('package.json', '**/package.json');

expect(command).toBeCalledTimes(2);
expect(command).toHaveBeenNthCalledWith(
expect(execGitCommand).toBeCalledTimes(2);
expect(execGitCommand).toHaveBeenNthCalledWith(
2,
'git ls-files --full-name -z "/root/package.json" "/root/**/package.json"',
expect.any(Object)
'git ls-files --full-name -z "/root/package.json" "/root/**/package.json"'
);
expect(results).toEqual(filesFound);
});
});

describe('getRepositoryCreationDate', () => {
it('parses the date successfully', async () => {
command.mockImplementation(() => Promise.resolve({ all: `2017-05-17 10:00:35 -0700` }) as any);
execGitCommandOneLine.mockResolvedValue(`2017-05-17 10:00:35 -0700`);
expect(await getRepositoryCreationDate()).toEqual(new Date('2017-05-17T17:00:35.000Z'));
});
});

describe('getStorybookCreationDate', () => {
it('passes the config dir to the git command', async () => {
await getStorybookCreationDate({ options: { storybookConfigDir: 'special-config-dir' } });
expect(command).toHaveBeenCalledWith(
expect(execGitCommandOneLine).toHaveBeenCalledWith(
expect.stringMatching(/special-config-dir/),
expect.anything()
);
});

it('defaults the config dir to the git command', async () => {
await getStorybookCreationDate({ options: {} });
expect(command).toHaveBeenCalledWith(expect.stringMatching(/.storybook/), expect.anything());
expect(execGitCommandOneLine).toHaveBeenCalledWith(
expect.stringMatching(/.storybook/),
expect.anything()
);
});

it('parses the date successfully', async () => {
command.mockImplementation(() => Promise.resolve({ all: `2017-05-17 10:00:35 -0700` }) as any);
execGitCommandOneLine.mockResolvedValue(`2017-05-17 10:00:35 -0700`);
expect(
await getStorybookCreationDate({ options: { storybookConfigDir: '.storybook' } })
).toEqual(new Date('2017-05-17T17:00:35.000Z'));
Expand All @@ -181,21 +153,20 @@ describe('getStorybookCreationDate', () => {

describe('getNumberOfComitters', () => {
it('parses the count successfully', async () => {
command.mockImplementation(() => Promise.resolve({ all: ` 17` }) as any);
expect(await getNumberOfComitters()).toEqual(17);
execGitCommand.mockResolvedValue(`tom\nzol\ndom`);
expect(await getNumberOfComitters()).toEqual(3);
});
});

describe('getCommittedFileCount', () => {
it('constructs the correct command', async () => {
await getCommittedFileCount(['page', 'screen'], ['js', 'ts']);
expect(command).toHaveBeenCalledWith(
'git ls-files -- "*page*.js" "*page*.ts" "*Page*.js" "*Page*.ts" "*screen*.js" "*screen*.ts" "*Screen*.js" "*Screen*.ts" | wc -l',
expect.anything()
expect(execGitCommand).toHaveBeenCalledWith(
'git ls-files -- "*page*.js" "*page*.ts" "*Page*.js" "*Page*.ts" "*screen*.js" "*screen*.ts" "*Screen*.js" "*Screen*.ts"'
);
});
it('parses the count successfully', async () => {
command.mockImplementation(() => Promise.resolve({ all: ` 17` }) as any);
expect(await getCommittedFileCount(['page', 'screen'], ['js', 'ts'])).toEqual(17);
execGitCommand.mockResolvedValue(`pages/Main.ts\npages/Pricing.ts\npagesLogin.ts`);
expect(await getCommittedFileCount(['page', 'screen'], ['js', 'ts'])).toEqual(3);
});
});
Loading

0 comments on commit 3221fcb

Please sign in to comment.