diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index d8ad54d16a..f431a44961 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -61,9 +61,9 @@ const DISABLE_COLOR_FLAGS = [ * Ex: on Mac ~kuychaco/ is expanded to the specified user’s home directory (/Users/kuychaco) * Regex translation: * ^~ line starts with tilde - * ([^/]*)/ captures non-forwardslash characters before first slash + * ([^\\\\/]*)[\\\\/] captures non-slash characters before first slash */ -const EXPAND_TILDE_REGEX = new RegExp('^~([^/]*)/'); +const EXPAND_TILDE_REGEX = new RegExp('^~([^\\\\/]*)[\\\\/]'); export default class GitShellOutStrategy { static defaultExecArgs = { @@ -119,6 +119,7 @@ export default class GitShellOutStrategy { // Attempt to collect the --exec-path from a native git installation. execPathPromise = new Promise((resolve, reject) => { childProcess.exec('git --exec-path', (error, stdout, stderr) => { + /* istanbul ignore if */ if (error) { // Oh well resolve(null); @@ -202,6 +203,7 @@ export default class GitShellOutStrategy { env.ATOM_GITHUB_GPG_PROMPT = 'true'; } + /* istanbul ignore if */ if (diagnosticsEnabled) { env.GIT_TRACE = 'true'; env.GIT_TRACE_CURL = 'true'; @@ -214,9 +216,11 @@ export default class GitShellOutStrategy { opts.stdinEncoding = 'utf8'; } + /* istanbul ignore if */ if (process.env.PRINT_GIT_TIMES) { console.time(`git:${formattedArgs}`); } + return new Promise(async (resolve, reject) => { if (options.beforeRun) { const newArgsOpts = await options.beforeRun({args, opts}); @@ -236,6 +240,7 @@ export default class GitShellOutStrategy { // chance to fall back to GIT_ASKPASS from the credential handler. await new Promise((resolveKill, rejectKill) => { require('tree-kill')(handlerPid, 'SIGTERM', err => { + /* istanbul ignore if */ if (err) { rejectKill(err); } else { resolveKill(); } }); }); @@ -258,14 +263,18 @@ export default class GitShellOutStrategy { timingMarker.mark('ipc', now - ipcTime); } timingMarker.finalize(); + + /* istanbul ignore if */ if (process.env.PRINT_GIT_TIMES) { console.timeEnd(`git:${formattedArgs}`); } + if (gitPromptServer) { gitPromptServer.terminate(); } subscriptions.dispose(); + /* istanbul ignore if */ if (diagnosticsEnabled) { const exposeControlCharacters = raw => { if (!raw) { return ''; } @@ -374,6 +383,7 @@ export default class GitShellOutStrategy { options.processCallback = child => { childPid = child.pid; + /* istanbul ignore next */ child.stdin.on('error', err => { throw new Error( `Error writing to stdin: git ${args.join(' ')} in ${this.workingDir}\n${options.stdin}\n${err}`); @@ -385,12 +395,14 @@ export default class GitShellOutStrategy { return { promise, cancel: () => { + /* istanbul ignore if */ if (!childPid) { return Promise.resolve(); } return new Promise((resolve, reject) => { require('tree-kill')(childPid, 'SIGTERM', err => { + /* istanbul ignore if */ if (err) { reject(err); } else { resolve(); } }); }); @@ -411,11 +423,7 @@ export default class GitShellOutStrategy { await fs.stat(this.workingDir); // fails if folder doesn't exist const output = await this.exec(['rev-parse', '--resolve-git-dir', path.join(this.workingDir, '.git')]); const dotGitDir = output.trim(); - if (path.isAbsolute(dotGitDir)) { - return toNativePathSep(dotGitDir); - } else { - return toNativePathSep(path.resolve(path.join(this.workingDir, dotGitDir))); - } + return toNativePathSep(dotGitDir); } catch (e) { return null; } @@ -446,6 +454,7 @@ export default class GitShellOutStrategy { // if no user is specified, fall back to using the home directory. return `${user ? path.join(path.dirname(homeDir), user) : homeDir}/`; }); + templatePath = toNativePathSep(templatePath); if (!path.isAbsolute(templatePath)) { templatePath = path.join(this.workingDir, templatePath); @@ -807,11 +816,8 @@ export default class GitShellOutStrategy { } } - mergeTrailers(commitMessage, trailers, unfold) { + mergeTrailers(commitMessage, trailers) { const args = ['interpret-trailers']; - if (unfold) { - args.push('--unfold'); - } for (const trailer of trailers) { args.push('--trailer', `${trailer.token}=${trailer.value}`); } diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 6dbd031fdf..7ed32ece11 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -1,6 +1,7 @@ import fs from 'fs-extra'; import path from 'path'; import http from 'http'; +import os from 'os'; import mkdirp from 'mkdirp'; import dedent from 'dedent-js'; @@ -8,7 +9,7 @@ import hock from 'hock'; import {GitProcess} from 'dugite'; import CompositeGitStrategy from '../lib/composite-git-strategy'; -import GitShellOutStrategy from '../lib/git-shell-out-strategy'; +import GitShellOutStrategy, {LargeRepoError} from '../lib/git-shell-out-strategy'; import WorkerManager from '../lib/worker-manager'; import {cloneRepository, initRepository, assertDeepPropertyVals, setUpLocalAndRemoteRepositories} from './helpers'; @@ -29,6 +30,53 @@ import * as reporterProxy from '../lib/reporter-proxy'; }; describe(`Git commands for CompositeGitStrategy made of [${strategies.map(s => s.name).join(', ')}]`, function() { + describe('exec', function() { + let git, incrementCounterStub; + + beforeEach(async function() { + const workingDir = await cloneRepository(); + git = createTestStrategy(workingDir); + incrementCounterStub = sinon.stub(reporterProxy, 'incrementCounter'); + }); + + describe('when the WorkerManager is not ready or disabled', function() { + beforeEach(function() { + sinon.stub(WorkerManager.getInstance(), 'isReady').returns(false); + }); + + it('kills the git process when cancel is triggered by the prompt server', async function() { + const promptStub = sinon.stub().rejects(); + git.setPromptCallback(promptStub); + + const stdin = dedent` + host=noway.com + username=me + + `; + await git.exec(['credential', 'fill'], {useGitPromptServer: true, stdin}); + + assert.isTrue(promptStub.called); + }); + }); + + it('rejects if the process fails to spawn for an unexpected reason', async function() { + sinon.stub(git, 'executeGitCommand').returns({promise: Promise.reject(new Error('wat'))}); + await assert.isRejected(git.exec(['version']), /wat/); + }); + + it('does not call incrementCounter when git command is on the ignore list', async function() { + await git.exec(['status']); + assert.equal(incrementCounterStub.callCount, 0); + }); + + it('does call incrementCounter when git command is NOT on the ignore list', async function() { + await git.exec(['commit', '--allow-empty', '-m', 'make an empty commit']); + + assert.equal(incrementCounterStub.callCount, 1); + assert.deepEqual(incrementCounterStub.lastCall.args, ['commit']); + }); + }); + // https://github.com/atom/github/issues/1051 // https://github.com/atom/github/issues/898 it('passes all environment variables to spawned git process', async function() { @@ -86,11 +134,15 @@ import * as reporterProxy from '../lib/reporter-proxy'; }); describe('fetchCommitMessageTemplate', function() { - it('gets commit message from template', async function() { - const workingDirPath = await cloneRepository('three-files'); - const git = createTestStrategy(workingDirPath); - const templateText = 'some commit message'; + let git, workingDirPath, templateText; + + beforeEach(async function() { + workingDirPath = await cloneRepository('three-files'); + git = createTestStrategy(workingDirPath); + templateText = 'some commit message'; + }); + it('gets commit message from template', async function() { const commitMsgTemplatePath = path.join(workingDirPath, '.gitmessage'); await fs.writeFile(commitMsgTemplatePath, templateText, {encoding: 'utf8'}); @@ -99,17 +151,11 @@ import * as reporterProxy from '../lib/reporter-proxy'; }); it('if config is not set return null', async function() { - const workingDirPath = await cloneRepository('three-files'); - const git = createTestStrategy(workingDirPath); - assert.isNotOk(await git.getConfig('commit.template')); // falsy value of null or '' assert.isNull(await git.fetchCommitMessageTemplate()); }); it('if config is set but file does not exist throw an error', async function() { - const workingDirPath = await cloneRepository('three-files'); - const git = createTestStrategy(workingDirPath); - const nonExistentCommitTemplatePath = path.join(workingDirPath, 'file-that-doesnt-exist'); await git.setConfig('commit.template', nonExistentCommitTemplatePath); await assert.isRejected( @@ -117,10 +163,38 @@ import * as reporterProxy from '../lib/reporter-proxy'; `Invalid commit template path set in Git config: ${nonExistentCommitTemplatePath}`, ); }); + + it('replaces ~ with your home directory', async function() { + // Fun fact: even on Windows, git does not accept "~\does-not-exist.txt" + await git.setConfig('commit.template', '~/does-not-exist.txt'); + await assert.isRejected( + git.fetchCommitMessageTemplate(), + `Invalid commit template path set in Git config: ${path.join(os.homedir(), 'does-not-exist.txt')}`, + ); + }); + + it("replaces ~user with user's home directory", async function() { + const expectedFullPath = path.join(path.dirname(os.homedir()), 'nope/does-not-exist.txt'); + await git.setConfig('commit.template', '~nope/does-not-exist.txt'); + await assert.isRejected( + git.fetchCommitMessageTemplate(), + `Invalid commit template path set in Git config: ${expectedFullPath}`, + ); + }); + + it('interprets relative paths local to the working directory', async function() { + const subDir = path.join(workingDirPath, 'abc/def/ghi'); + const subPath = path.join(subDir, 'template.txt'); + await fs.mkdirs(subDir); + await fs.writeFile(subPath, templateText, {encoding: 'utf8'}); + await git.setConfig('commit.template', path.join('abc/def/ghi/template.txt')); + assert.strictEqual(await git.fetchCommitMessageTemplate(), templateText); + }); }); - if (process.platform === 'win32') { - describe('getStatusBundle()', function() { + + describe('getStatusBundle()', function() { + if (process.platform === 'win32') { it('normalizes the path separator on Windows', async function() { const workingDir = await cloneRepository('three-files'); const git = createTestStrategy(workingDir); @@ -135,8 +209,17 @@ import * as reporterProxy from '../lib/reporter-proxy'; const changedPaths = changedEntries.map(entry => entry.filePath); assert.deepEqual(changedPaths, [relPathA, relPathB]); }); + } + + it('throws a LargeRepoError when the status output is too large', async function() { + const workingDir = await cloneRepository('three-files'); + const git = createTestStrategy(workingDir); + + sinon.stub(git, 'exec').resolves({length: 1024 * 1024 * 10 + 1}); + + await assert.isRejected(git.getStatusBundle(), LargeRepoError); }); - } + }); describe('getHeadCommit()', function() { it('gets the SHA and message of the most recent commit', async function() { @@ -426,6 +509,14 @@ import * as reporterProxy from '../lib/reporter-proxy'; const authors = await git.getAuthors({max: 1}); assert.deepEqual(authors, []); }); + + it('propagates other git errors', async function() { + const workingDirPath = await cloneRepository('multiple-commits'); + const git = createTestStrategy(workingDirPath); + sinon.stub(git, 'exec').rejects(new Error('oh no')); + + await assert.isRejected(git.getAuthors(), /oh no/); + }); }); describe('diffFileStatus', function() { @@ -524,6 +615,42 @@ import * as reporterProxy from '../lib/reporter-proxy'; assert.isDefined(diffOutput); }); + it('rejects if an unexpected number of diffs is returned', async function() { + const workingDirPath = await cloneRepository(); + const git = createTestStrategy(workingDirPath); + sinon.stub(git, 'exec').resolves(dedent` + diff --git aaa.txt aaa.txt + index df565d30..244a7225 100644 + --- aaa.txt + +++ aaa.txt + @@ -100,3 +100,3 @@ + 000 + -001 + +002 + 003 + diff --git aaa.txt aaa.txt + index df565d30..244a7225 100644 + --- aaa.txt + +++ aaa.txt + @@ -100,3 +100,3 @@ + 000 + -001 + +002 + 003 + diff --git aaa.txt aaa.txt + index df565d30..244a7225 100644 + --- aaa.txt + +++ aaa.txt + @@ -100,3 +100,3 @@ + 000 + -001 + +002 + 003 + `); + + await assert.isRejected(git.getDiffsForFilePath('aaa.txt'), /Expected between 0 and 2 diffs/); + }); + describe('when the file is unstaged', function() { it('returns a diff comparing the working directory copy of the file and the version on the index', async function() { const workingDirPath = await cloneRepository('three-files'); @@ -850,6 +977,12 @@ import * as reporterProxy from '../lib/reporter-proxy'; assert.deepEqual(stagedChange.hunks[0].lines, [' foo', '+bar']); }); }); + + it('fails when an invalid type is passed', async function() { + const workingDirPath = await cloneRepository('three-files'); + const git = createTestStrategy(workingDirPath); + assert.throws(() => git.reset('scrambled'), /Invalid type scrambled/); + }); }); describe('deleteRef()', function() { @@ -982,6 +1115,14 @@ import * as reporterProxy from '../lib/reporter-proxy'; await git.setConfig('awesome.devs', 'BinaryMuse,kuychaco,smashwilson'); assert.equal('BinaryMuse,kuychaco,smashwilson', await git.getConfig('awesome.devs')); }); + + it('propagates unexpected git errors', async function() { + const workingDirPath = await cloneRepository('three-files'); + const git = createTestStrategy(workingDirPath); + sinon.stub(git, 'exec').rejects(new Error('AHHHH')); + + await assert.isRejected(git.getConfig('some.key'), /AHHHH/); + }); }); describe('commit(message, options)', function() { @@ -1063,6 +1204,13 @@ import * as reporterProxy from '../lib/reporter-proxy'; assert.strictEqual(amendedCommit.messageSubject, 'first'); assert.strictEqual(amendedCommit.messageBody, 'second\n\nthird'); }); + + it('attempts to amend an unborn commit', async function() { + const workingDirPath = await initRepository(); + const git = createTestStrategy(workingDirPath); + + await assert.isRejected(git.commit('', {amend: true, allowEmpty: true}), /You have nothing to amend/); + }); }); }); @@ -1160,6 +1308,17 @@ import * as reporterProxy from '../lib/reporter-proxy'; }); }); + describe('checkoutSide', function() { + it('is a no-op when no paths are provided', async function() { + const workdir = await cloneRepository(); + const git = await createTestStrategy(workdir); + sinon.spy(git, 'exec'); + + await git.checkoutSide('ours', []); + assert.isFalse(git.exec.called); + }); + }); + // Only needs to be tested on strategies that actually implement gpgExec describe('GPG signing', function() { let git; @@ -1370,6 +1529,20 @@ import * as reporterProxy from '../lib/reporter-proxy'; const contents = await git.exec(['cat-file', '-p', sha]); assert.equal(contents, 'foo\n'); }); + + it('propagates unexpected git errors from hash-object', async function() { + const workingDirPath = await cloneRepository(); + const git = createTestStrategy(workingDirPath); + sinon.stub(git, 'exec').rejects(new Error('shiiiit')); + + await assert.isRejected(git.createBlob({filePath: 'a.txt'}), /shiiiit/); + }); + + it('rejects if neither file path or stdin are provided', async function() { + const workingDirPath = await cloneRepository(); + const git = createTestStrategy(workingDirPath); + await assert.isRejected(git.createBlob(), /Must supply file path or stdin/); + }); }); describe('expandBlobToFile(absFilePath, sha)', function() { @@ -1465,6 +1638,14 @@ import * as reporterProxy from '../lib/reporter-proxy'; assert.isTrue(contents.includes('<<<<<<<')); assert.isTrue(contents.includes('>>>>>>>')); }); + + it('propagates unexpected git errors', async function() { + const workingDirPath = await cloneRepository('three-files'); + const git = createTestStrategy(workingDirPath); + sinon.stub(git, 'exec').rejects(new Error('ouch')); + + await assert.isRejected(git.mergeFile('a.txt', 'b.txt', 'c.txt', 'result.txt'), /ouch/); + }); }); describe('updateIndex(filePath, commonBaseSha, oursSha, theirsSha)', function() { @@ -1524,24 +1705,7 @@ import * as reporterProxy from '../lib/reporter-proxy'; }); }); }); - describe('exec', function() { - let workingDirPath, git, incrementCounterStub; - beforeEach(async function() { - workingDirPath = await cloneRepository('three-files'); - git = createTestStrategy(workingDirPath); - incrementCounterStub = sinon.stub(reporterProxy, 'incrementCounter'); - }); - it('does not call incrementCounter when git command is on the ignore list', async function() { - await git.exec(['status']); - assert.equal(incrementCounterStub.callCount, 0); - }); - it('does call incrementCounter when git command is NOT on the ignore list', async function() { - await git.exec(['commit', '--allow-empty', '-m', 'make an empty commit']); - assert.equal(incrementCounterStub.callCount, 1); - assert.deepEqual(incrementCounterStub.lastCall.args, ['commit']); - }); - }); describe('executeGitCommand', function() { it('shells out in process until WorkerManager instance is ready', async function() { if (process.env.ATOM_GITHUB_INLINE_GIT_EXEC) {