Do not reject detached HEAD when publishing a canary release (#711)

* assertStubbedCalls: cache objectMethodName and use it for helpful diagnostics

src/GitUtilities.js

@@ -3,6 +3,12 @@ import logger from "./logger";
 import escapeArgs from "command-join";
 
 export default class GitUtilities {
+  @logger.logifySync()
+  static isDetachedHead() {
+    const branchName = GitUtilities.getCurrentBranch();
+    return branchName === "HEAD";
+  }
+
   @logger.logifySync()
   static isInitialized() {
     try {
@@ -77,6 +83,11 @@ export default class GitUtilities {
     return ChildProcessUtilities.execSync("git diff --name-only " + since + " -- " + escapeArgs(location));
   }
 
+  @logger.logifySync()
+  static getCurrentBranch() {
+    return ChildProcessUtilities.execSync("git rev-parse --abbrev-ref HEAD");
+  }
+
   @logger.logifySync()
   static getCurrentSHA() {
     return ChildProcessUtilities.execSync("git rev-parse HEAD");
@@ -92,16 +103,6 @@ export default class GitUtilities {
     ChildProcessUtilities.execSync("git checkout -- " + changes);
   }
 
-  @logger.logifySync()
-  static getCurrentBranch() {
-    return ChildProcessUtilities.execSync("git symbolic-ref --short HEAD");
-  }
-
-  @logger.logifySync()
-  static getCurrentBranchDescription() {
-    return ChildProcessUtilities.execSync("git symbolic-ref --short -q HEAD");
-  }
-
   @logger.logifySync()
   static init() {
     return ChildProcessUtilities.execSync("git init");

src/commands/PublishCommand.js

@@ -14,16 +14,12 @@ import { EOL } from "os";
 
 export default class PublishCommand extends Command {
   initialize(callback) {
+    this.gitEnabled = !(this.flags.canary || this.flags.skipGit);
 
     if (this.flags.canary) {
       this.logger.info("Publishing canary build");
     }
 
-    const currentBranch = GitUtilities.getCurrentBranchDescription();
-    if (currentBranch === "") {
-      callback("You are working on detached branch, create new branch to publish changes.");
-    }
-
     if (!this.repository.isIndependent()) {
       this.globalVersion = this.repository.version;
       this.logger.info("Current version: " + this.globalVersion);
@@ -94,12 +90,17 @@ export default class PublishCommand extends Command {
 
   execute(callback) {
     try {
+      if (this.gitEnabled && GitUtilities.isDetachedHead()) {
+        throw new Error("Detached git HEAD, please checkout a branch to publish changes.");
+      }
+
       if (!this.repository.isIndependent() && !this.flags.canary) {
         this.updateVersionInLernaJson();
       }
 
       this.updateUpdatedPackages();
-      if (!this.flags.skipGit) {
+
+      if (this.gitEnabled) {
         this.commitAndTagUpdates();
       }
     } catch (err) {
@@ -137,7 +138,7 @@ export default class PublishCommand extends Command {
           return;
         }
 
-        if (!(this.flags.canary || this.flags.skipGit)) {
+        if (this.gitEnabled) {
           this.logger.info("Pushing tags to git...");
           this.logger.newLine();
           GitUtilities.pushWithTags(this.getOptions().gitRemote || "origin", this.tags);
@@ -220,12 +221,12 @@ export default class PublishCommand extends Command {
         });
       });
       callback(null, { versions });
-      // Independent Non-Canary Mode
+
+    // Independent Non-Canary Mode
     } else {
       async.mapLimit(this.updates, 1, (update, cb) => {
         this.promptVersion(update.package.name, update.package.version, cb);
       }, (err, versions) => {
-
         if (err) {
           return callback(err);
         }
@@ -370,7 +371,7 @@ export default class PublishCommand extends Command {
       changedFiles.push(packageJsonLocation);
     });
 
-    if (!(this.flags.canary || this.flags.skipGit)) {
+    if (this.gitEnabled) {
       changedFiles.forEach(GitUtilities.addFile);
     }
   }
@@ -392,12 +393,10 @@ export default class PublishCommand extends Command {
   }
 
   commitAndTagUpdates() {
-    if (!this.flags.canary) {
-      if (this.repository.isIndependent()) {
-        this.tags = this.gitCommitAndTagVersionForUpdates();
-      } else {
-        this.tags = [this.gitCommitAndTagVersion(this.masterVersion)];
-      }
+    if (this.repository.isIndependent()) {
+      this.tags = this.gitCommitAndTagVersionForUpdates();
+    } else {
+      this.tags = [this.gitCommitAndTagVersion(this.masterVersion)];
     }
   }
 
@@ -407,14 +406,17 @@ export default class PublishCommand extends Command {
 
     GitUtilities.commit(message);
     tags.forEach(GitUtilities.addTag);
+
     return tags;
   }
 
   gitCommitAndTagVersion(version) {
     const tag = "v" + version;
     const message = this.flags.message || tag;
+
     GitUtilities.commit(message);
     GitUtilities.addTag(tag);
+
     return tag;
   }
 

test/GitUtilities.js

@@ -1,5 +1,6 @@
 import assert from "assert";
 
+import ChildProcessUtilities from "../src/ChildProcessUtilities";
 import GitUtilities from "../src/GitUtilities";
 
 /**
@@ -8,6 +9,33 @@ import GitUtilities from "../src/GitUtilities";
  */
 
 describe("GitUtilities", () => {
+  const cpuExecSync = ChildProcessUtilities.execSync;
+
+  beforeEach(() => {
+    ChildProcessUtilities.execSync = jest.fn();
+  });
+
+  afterEach(() => {
+    ChildProcessUtilities.execSync = cpuExecSync;
+  });
+
+  describe(".isDetachedHead()", () => {
+    it("calls getCurrentBranch()", () => {
+      expect(() => GitUtilities.isDetachedHead()).not.toThrow();
+      expect(ChildProcessUtilities.execSync).lastCalledWith("git rev-parse --abbrev-ref HEAD");
+    });
+
+    it("returns true when branchName is HEAD", () => {
+      ChildProcessUtilities.execSync.mockImplementation(() => "HEAD");
+      expect(GitUtilities.isDetachedHead()).toBe(true);
+    });
+
+    it("returns false when branchName is not HEAD", () => {
+      ChildProcessUtilities.execSync.mockImplementation(() => "master");
+      expect(GitUtilities.isDetachedHead()).toBe(false);
+    });
+  });
+
   describe(".isInitialized()", () => {
     it("should exist", () => {
       assert.ok(GitUtilities.isInitialized);
@@ -86,15 +114,16 @@ describe("GitUtilities", () => {
     });
   });
 
-  describe(".getCurrentSHA()", () => {
-    it("should exist", () => {
-      assert.ok(GitUtilities.getCurrentSHA);
+  describe(".getCurrentBranch()", () => {
+    it("calls `git rev-parse --abbrev-ref HEAD`", () => {
+      expect(() => GitUtilities.getCurrentBranch()).not.toThrow();
+      expect(ChildProcessUtilities.execSync).lastCalledWith("git rev-parse --abbrev-ref HEAD");
     });
   });
 
-  describe(".getCurrentBranchDescription()", () => {
+  describe(".getCurrentSHA()", () => {
     it("should exist", () => {
-      assert.ok(GitUtilities.getCurrentBranchDescription);
+      assert.ok(GitUtilities.getCurrentSHA);
     });
   });