From 551e6e4d609e61176d6befbeb0b88c2d2f8ea1c3 Mon Sep 17 00:00:00 2001 From: Daniel Stockman Date: Wed, 15 May 2019 10:52:48 -0700 Subject: [PATCH] fix(collect-uncommitted): Call `git` with correct arguments, test properly Fixes #2091 --- package-lock.json | 4 +- utils/collect-uncommitted/CHANGELOG.md | 11 +- utils/collect-uncommitted/README.md | 1 + .../__tests__/collect-uncommitted.test.js | 160 +++++++++++++----- .../lib/collect-uncommitted.js | 25 ++- utils/collect-uncommitted/package.json | 4 +- 6 files changed, 140 insertions(+), 65 deletions(-) diff --git a/package-lock.json b/package-lock.json index 32cca919c8..996a2d6627 100644 --- a/package-lock.json +++ b/package-lock.json @@ -776,7 +776,9 @@ "version": "file:utils/collect-uncommitted", "requires": { "@lerna/child-process": "file:core/child-process", - "chalk": "^2.3.1" + "chalk": "^2.3.1", + "figgy-pudding": "^3.5.1", + "npmlog": "^4.1.2" } }, "@lerna/collect-updates": { diff --git a/utils/collect-uncommitted/CHANGELOG.md b/utils/collect-uncommitted/CHANGELOG.md index 148c7bacb0..302d60a494 100644 --- a/utils/collect-uncommitted/CHANGELOG.md +++ b/utils/collect-uncommitted/CHANGELOG.md @@ -9,13 +9,4 @@ See [Conventional Commits](https://conventionalcommits.org) for commit guideline ### Features * **publish:** Display uncommitted changes when validation fails ([#2066](https://github.com/lerna/lerna/issues/2066)) ([ea41fe9](https://github.com/lerna/lerna/commit/ea41fe9)) - - - - - -## Unpublished - -### Features - -* Create `[@lerna](https://github.com/lerna)/collect-uncommitted` +* Create `@lerna/collect-uncommitted` diff --git a/utils/collect-uncommitted/README.md b/utils/collect-uncommitted/README.md index 1054204e5b..2d86ace30a 100644 --- a/utils/collect-uncommitted/README.md +++ b/utils/collect-uncommitted/README.md @@ -10,6 +10,7 @@ const collectUncommitted = require("@lerna/collect-uncommitted"); // values listed here are their defaults const options = { cwd: process.cwd(), + log: require("npmlog"), }; (async () => { diff --git a/utils/collect-uncommitted/__tests__/collect-uncommitted.test.js b/utils/collect-uncommitted/__tests__/collect-uncommitted.test.js index a07ef8516e..b57a84b80e 100644 --- a/utils/collect-uncommitted/__tests__/collect-uncommitted.test.js +++ b/utils/collect-uncommitted/__tests__/collect-uncommitted.test.js @@ -1,78 +1,144 @@ "use strict"; -jest.mock("@lerna/child-process"); - +const fs = require("fs-extra"); +const path = require("path"); const chalk = require("chalk"); -const childProcess = require("@lerna/child-process"); -const collectUncommitted = require("../lib/collect-uncommitted"); -const stats = `AD file1 - D file2 - M path/to/file3 -AM path/file4 -MM path/file5 -M file6 -D file7 -UU file8 -?? file9`; +// helpers +const { getPackages } = require("@lerna/project"); +const gitAdd = require("@lerna-test/git-add"); +const initFixture = require("@lerna-test/init-fixture")(__dirname); + +// file under test +const collectUncommitted = require("../lib/collect-uncommitted"); +// primary assertion setup const GREEN_A = chalk.green("A"); const GREEN_M = chalk.green("M"); const GREEN_D = chalk.green("D"); const RED_D = chalk.red("D"); const RED_M = chalk.red("M"); -const RED_UU = chalk.red("UU"); const RED_QQ = chalk.red("??"); const colorizedAry = [ - `${GREEN_A}${RED_D} file1`, - ` ${RED_D} file2`, - ` ${RED_M} path/to/file3`, - `${GREEN_A}${RED_M} path/file4`, - `${GREEN_M}${RED_M} path/file5`, - `${GREEN_M} file6`, - `${GREEN_D} file7`, - `${RED_UU} file8`, - `${RED_QQ} file9`, + `${GREEN_D} package.json`, + `${GREEN_A}${RED_D} packages/package-1/file-1.js`, + ` ${RED_D} packages/package-1/package.json`, + `${GREEN_A}${RED_M} packages/package-2/file-2.js`, + ` ${RED_M} packages/package-2/package.json`, + `${GREEN_M}${RED_M} packages/package-3/package.json`, + `${GREEN_M} packages/package-4/package.json`, + // no UU assertion, only for merge conflicts + `${RED_QQ} poopy.txt`, ]; -childProcess.exec.mockResolvedValue(stats); -childProcess.execSync.mockReturnValue(stats); +// D package.json +// AD packages/package-1/file-1.js +// D packages/package-1/package.json +// AM packages/package-2/file-2.js +// M packages/package-2/package.json +// MM packages/package-3/package.json +// M packages/package-4/package.json +// ?? poopy.txt + +const setupChanges = async cwd => { + const [pkg1, pkg2, pkg3, pkg4] = await getPackages(cwd); + + // "AD": (added to index, deleted in working tree) + const file1 = path.join(pkg1.location, "file-1.js"); + await fs.outputFile(file1, "yay"); + await gitAdd(cwd, file1); + await fs.remove(file1); + + // " D": (deleted in working tree) + await fs.remove(pkg1.manifestLocation); + + // " M": (modified in working tree) + pkg2.set("modified", true); + await pkg2.serialize(); + + // "AM": (added to index, modified in working tree) + const file2 = path.join(pkg2.location, "file-2.js"); + await fs.outputFile(file2, "woo"); + await gitAdd(cwd, file2); + await fs.outputFile(file2, "hoo"); + + // "MM": (updated in index, modified in working tree) + pkg3.set("updated", true); + await pkg3.serialize(); + await gitAdd(cwd, pkg3.manifestLocation); + pkg3.set("modified", true); + await pkg3.serialize(); + + // "M ": (updated in index) + pkg4.set("updated", true); + await pkg4.serialize(); + await gitAdd(cwd, pkg4.manifestLocation); + + // "D ": (deleted in index) + const rootManifest = path.join(cwd, "package.json"); + await fs.remove(rootManifest); + await gitAdd(cwd, rootManifest); + + // "??": (untracked) + const poopy = path.join(cwd, "poopy.txt"); + await fs.outputFile(poopy, "pants"); +}; describe("collectUncommitted()", () => { + it("resolves empty array on clean repo", async () => { + const cwd = await initFixture("normal"); + const result = await collectUncommitted({ cwd }); + + expect(result).toEqual([]); + }); + it("resolves an array of uncommitted changes", async () => { - const result = await collectUncommitted(); - expect(childProcess.exec).toHaveBeenLastCalledWith("git", "status -s", {}); + const cwd = await initFixture("normal"); + + await setupChanges(cwd); + + const result = await collectUncommitted({ cwd }); + expect(result).toEqual(colorizedAry); }); - it("empty array on clean repo", async () => { - childProcess.exec.mockResolvedValueOnce(""); - const result = await collectUncommitted(); - expect(childProcess.exec).toHaveBeenLastCalledWith("git", "status -s", {}); - expect(result).toEqual([]); + it("accepts options.log", async () => { + // re-uses previous cwd + const log = { silly: jest.fn() }; + + const result = await collectUncommitted({ log }); + + expect(log.silly).toHaveBeenCalled(); + expect(result).toEqual(colorizedAry); }); +}); - it("accepts options.cwd", async () => { - const options = { cwd: "foo" }; - await collectUncommitted(options); +describe("collectUncommitted.sync()", () => { + it("resolves empty array on clean repo", async () => { + const cwd = await initFixture("normal"); + const result = collectUncommitted.sync({ cwd }); - expect(childProcess.exec).toHaveBeenLastCalledWith("git", "status -s", options); + expect(result).toEqual([]); }); - describe("collectUncommitted.sync()", () => { - it("returns an array of uncommitted changes", async () => { - const result = collectUncommitted.sync(); + it("returns an array of uncommitted changes", async () => { + const cwd = await initFixture("normal"); + + await setupChanges(cwd); + + const result = collectUncommitted.sync({ cwd }); - expect(childProcess.execSync).toHaveBeenLastCalledWith("git", "status -s", {}); - expect(result).toEqual(colorizedAry); - }); + expect(result).toEqual(colorizedAry); + }); - it("accepts options.cwd", async () => { - const options = { cwd: "foo" }; - collectUncommitted.sync(options); + it("accepts options.log", async () => { + // re-uses previous cwd + const log = { silly: jest.fn() }; - expect(childProcess.execSync).toHaveBeenLastCalledWith("git", "status -s", options); - }); + const result = collectUncommitted.sync({ log }); + + expect(log.silly).toHaveBeenCalled(); + expect(result).toEqual(colorizedAry); }); }); diff --git a/utils/collect-uncommitted/lib/collect-uncommitted.js b/utils/collect-uncommitted/lib/collect-uncommitted.js index 17dbf60c80..ae768f9ed3 100644 --- a/utils/collect-uncommitted/lib/collect-uncommitted.js +++ b/utils/collect-uncommitted/lib/collect-uncommitted.js @@ -1,11 +1,18 @@ "use strict"; const chalk = require("chalk"); +const figgyPudding = require("figgy-pudding"); +const npmlog = require("npmlog"); const { exec, execSync } = require("@lerna/child-process"); module.exports = collectUncommitted; module.exports.sync = sync; +const UncommittedConfig = figgyPudding({ + cwd: {}, + log: { default: npmlog }, +}); + const maybeColorize = colorize => s => (s !== " " ? colorize(s) : s); const cRed = maybeColorize(chalk.red); const cGreen = maybeColorize(chalk.green); @@ -15,19 +22,25 @@ const replaceStatus = (_, maybeGreen, maybeRed) => `${cGreen(maybeGreen)}${cRed( const colorizeStats = stats => stats.replace(/^([^U]| )([A-Z]| )/gm, replaceStatus).replace(/^\?{2}|U{2}/gm, cRed("$&")); -const splitOnNewLine = (string = "") => string.split("\n"); +const splitOnNewLine = str => str.split("\n"); -const filterEmpty = (strings = []) => strings.filter(s => s.length !== 0); +const filterEmpty = lines => lines.filter(line => line.length); const o = (l, r) => x => l(r(x)); const transformOutput = o(filterEmpty, o(splitOnNewLine, colorizeStats)); -function collectUncommitted(options = {}) { - return exec("git", "status -s", options).then(transformOutput); +function collectUncommitted(options) { + const { cwd, log } = UncommittedConfig(options); + log.silly("collect-uncommitted", "git status --porcelain (async)"); + + return exec("git", ["status", "--porcelain"], { cwd }).then(({ stdout }) => transformOutput(stdout)); } -function sync(options = {}) { - const stdout = execSync("git", "status -s", options); +function sync(options) { + const { cwd, log } = UncommittedConfig(options); + log.silly("collect-uncommitted", "git status --porcelain (sync)"); + + const stdout = execSync("git", ["status", "--porcelain"], { cwd }); return transformOutput(stdout); } diff --git a/utils/collect-uncommitted/package.json b/utils/collect-uncommitted/package.json index 2503528aad..934cdbd3c1 100644 --- a/utils/collect-uncommitted/package.json +++ b/utils/collect-uncommitted/package.json @@ -34,6 +34,8 @@ }, "dependencies": { "@lerna/child-process": "file:../../core/child-process", - "chalk": "^2.3.1" + "chalk": "^2.3.1", + "figgy-pudding": "^3.5.1", + "npmlog": "^4.1.2" } }