fix(collect-uncommitted): Call git with correct arguments, test pro…

…perly

Fixes #2091

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`

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 () => {

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);
   });
 });

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);
 }

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"
   }
 }