From a6149058a12f9feec97bbd13a825f0acf35c7721 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:27:44 -0500 Subject: [PATCH 01/18] Ignore conditionals that are too much of a pain to test --- lib/git-shell-out-strategy.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index d8ad54d16a..2df45a5051 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -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(); } }); }); From 3b5e0b2fb046b383dbfb52d634d6ceca422806d2 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:29:39 -0500 Subject: [PATCH 02/18] git rev-parse --resolve-git-dir will always return an absolute path --- lib/git-shell-out-strategy.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 2df45a5051..323a11fecf 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -423,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; } From 3313d22bde5fc1d457c78b0b34359dfd06f7e5be Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:30:09 -0500 Subject: [PATCH 03/18] "unfold" parameter is never used --- lib/git-shell-out-strategy.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 323a11fecf..68ac46c68e 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -815,11 +815,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}`); } From df55ac0ddec0ec9122409ad1d73f7296a2c82b95 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:32:05 -0500 Subject: [PATCH 04/18] Cover throwing LargeRepoError from getStatusBundle() --- test/git-strategies.test.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 6dbd031fdf..8aa15ed933 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -8,7 +8,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'; @@ -119,8 +119,9 @@ import * as reporterProxy from '../lib/reporter-proxy'; }); }); - 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 +136,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() { From 3c2b06f3bc5edd62766bf98beb59212fc52fd571 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:32:51 -0500 Subject: [PATCH 05/18] Cover codepaths in exec() --- test/git-strategies.test.js | 59 ++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 8aa15ed933..eb123cc51e 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -29,6 +29,48 @@ 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('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() { @@ -1534,24 +1576,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) { From 240ccdf0afac8353cf59f27021ed0289cd031266 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:33:24 -0500 Subject: [PATCH 06/18] Cover fetchCommitMessageTemplate() --- test/git-strategies.test.js | 45 ++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index eb123cc51e..f713da487b 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'; @@ -128,11 +129,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'}); @@ -141,17 +146,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( @@ -159,6 +158,32 @@ import * as reporterProxy from '../lib/reporter-proxy'; `Invalid commit template path set in Git config: ${nonExistentCommitTemplatePath}`, ); }); + + it('replaces ~ with your home directory', async function() { + 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); + }); }); From 0b454873767d356403d75920216eeb2506079797 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:34:47 -0500 Subject: [PATCH 07/18] Cover handling of unexpected git errors --- test/git-strategies.test.js | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index f713da487b..bb8d9456c3 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -503,6 +503,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() { @@ -1059,6 +1067,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() { @@ -1447,6 +1463,14 @@ 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/); + }); }); describe('expandBlobToFile(absFilePath, sha)', function() { @@ -1542,6 +1566,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() { From 24da545a9180ebbb271fc1f1260baefb6820e311 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:35:34 -0500 Subject: [PATCH 08/18] Cover unexpected git diff output --- test/git-strategies.test.js | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index bb8d9456c3..852064efd1 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -609,6 +609,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'); From 97200a16e93c7c858ca4c4f8a253045df7962e44 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:35:51 -0500 Subject: [PATCH 09/18] Input argument validation tests --- test/git-strategies.test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 852064efd1..378f24abf0 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -971,6 +971,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() { @@ -1507,6 +1513,12 @@ import * as reporterProxy from '../lib/reporter-proxy'; 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() { From 8bf9385ac242da5f4849da714933d4983d2ff68c Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:36:10 -0500 Subject: [PATCH 10/18] Fail correctly when amending an unborn commit --- test/git-strategies.test.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 378f24abf0..9cb81fb169 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -1198,6 +1198,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/); + }); }); }); From 4860e97ddfc46d5b3a128ef3daa76ec1036dfb28 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 11:36:27 -0500 Subject: [PATCH 11/18] Test degenerate case of checkoutSide() --- test/git-strategies.test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 9cb81fb169..aa9ade3c8a 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -1302,6 +1302,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; From 6539e2b2ad7767ef16ed6a37686f2dcd75cb8e90 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 13:19:09 -0500 Subject: [PATCH 12/18] Cover git process spawn rejection --- test/git-strategies.test.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index aa9ade3c8a..8620ea1010 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -59,6 +59,11 @@ import * as reporterProxy from '../lib/reporter-proxy'; }); }); + 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); From f99e1e22e09e81a22c3fe0cb6520db0a565fa71e Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 13:38:50 -0500 Subject: [PATCH 13/18] Use correct path separators in fake template paths --- test/git-strategies.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 8620ea1010..d782777f19 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -165,7 +165,7 @@ import * as reporterProxy from '../lib/reporter-proxy'; }); it('replaces ~ with your home directory', async function() { - await git.setConfig('commit.template', '~/does-not-exist.txt'); + await git.setConfig('commit.template', path.join('~/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')}`, @@ -174,7 +174,7 @@ import * as reporterProxy from '../lib/reporter-proxy'; 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 git.setConfig('commit.template', path.join('~nope/does-not-exist.txt')); await assert.isRejected( git.fetchCommitMessageTemplate(), `Invalid commit template path set in Git config: ${expectedFullPath}`, From 73a6354530563d574f3bc9394e78605c1b3c2a14 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 14:10:18 -0500 Subject: [PATCH 14/18] Handle either path separator in EXPAND_TILDE_REGEX --- lib/git-shell-out-strategy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 68ac46c68e..a8cde05c5c 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-forwardslash characters before first slash */ -const EXPAND_TILDE_REGEX = new RegExp('^~([^/]*)/'); +const EXPAND_TILDE_REGEX = new RegExp('^~([^/]*)[\\/]'); export default class GitShellOutStrategy { static defaultExecArgs = { From 3d34f7e008245a5a2152fc0a858b17acf9ce6013 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 14:50:42 -0500 Subject: [PATCH 15/18] path.join() looks like it's resolving the path on Windows? --- test/git-strategies.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index d782777f19..56aaf34248 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -165,7 +165,7 @@ import * as reporterProxy from '../lib/reporter-proxy'; }); it('replaces ~ with your home directory', async function() { - await git.setConfig('commit.template', path.join('~/does-not-exist.txt')); + await git.setConfig('commit.template', `~${path.sep}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')}`, @@ -174,7 +174,7 @@ import * as reporterProxy from '../lib/reporter-proxy'; 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', path.join('~nope/does-not-exist.txt')); + await git.setConfig('commit.template', `~nope${path.sep}does-not-exist.txt`); await assert.isRejected( git.fetchCommitMessageTemplate(), `Invalid commit template path set in Git config: ${expectedFullPath}`, From efb6ddcecda891f2d3018a1894df792bb0504d30 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 15:36:15 -0500 Subject: [PATCH 16/18] More Regexp tries --- lib/git-shell-out-strategy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index a8cde05c5c..36dd15fbaf 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 = { From 89be380251dba8a6951d22df806e828fd8651f98 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 10 Jan 2019 16:12:50 -0500 Subject: [PATCH 17/18] Grrr --- lib/git-shell-out-strategy.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index 36dd15fbaf..dd4280aeba 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-slash 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 = { From b346a451b0e637f9969203e02a10dbffc36e0e86 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Fri, 11 Jan 2019 09:02:25 -0500 Subject: [PATCH 18/18] Normalize commit template paths on Windows --- lib/git-shell-out-strategy.js | 1 + test/git-strategies.test.js | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/git-shell-out-strategy.js b/lib/git-shell-out-strategy.js index dd4280aeba..f431a44961 100644 --- a/lib/git-shell-out-strategy.js +++ b/lib/git-shell-out-strategy.js @@ -454,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); diff --git a/test/git-strategies.test.js b/test/git-strategies.test.js index 56aaf34248..7ed32ece11 100644 --- a/test/git-strategies.test.js +++ b/test/git-strategies.test.js @@ -165,7 +165,8 @@ import * as reporterProxy from '../lib/reporter-proxy'; }); it('replaces ~ with your home directory', async function() { - await git.setConfig('commit.template', `~${path.sep}does-not-exist.txt`); + // 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')}`, @@ -174,7 +175,7 @@ import * as reporterProxy from '../lib/reporter-proxy'; 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${path.sep}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}`,