Rework commit message mechanics

We can rely on Pathname to do some of the heavy lifting, while also
hiding some implementation details (KEYWORD_REGEX), and optimizing some
of our regex usage by using #match? when we don't need the MatchData
object that would be created by a `#~`.

We also remove our dependence on the FakeFile object, trading off that
explicit isolation for a more pliable implementation. That is, our test
setup doesn't rely on stubbing out Stdlib file access. Instead we set up
some fixures via temp dirs and then read/write actual files.

Author: stevenharman

lib/git_tracker/commit_message.rb

@@ -1,37 +1,36 @@
 module GitTracker
   class CommitMessage
     def initialize(file)
-      @file = file
-      @message = File.read(@file)
-    end
-
-    def mentions_story?(number)
-      @message =~ /^(?!#).*\[(\w+\s)?(#\d+\s)*##{number}(\s#\d+)*(\s\w+)?\]/i
-    end
-
-    def keyword
-      @message =~ /\[(fix|fixes|fixed|complete|completes|completed|finish|finishes|finished|deliver|delivers|delivered)\]/io
-      $1
+      @file = Pathname(file)
     end
 
     def append(text)
-      body, postscript = parse(@message)
+      body, postscript = parse(message)
       new_message = format_message(body, text, postscript)
-      File.open(@file, "w") do |f|
+
+      file.open("w") do |f|
         f.write(new_message)
       end
+
       new_message
     end
 
-    private
+    def keyword
+      matches = message.match(KEYWORD_REGEX) || {}
+      matches[:keyword]
+    end
 
-    def parse(message)
-      lines = message.split($/)
-      body = lines.take_while { |line| !line.start_with?("#") }
-      postscript = lines.slice(body.length..-1)
-      [body.join("\n"), postscript.join("\n")]
+    def mentions_story?(number)
+      /^(?!#).*\[(\w+\s)?(#\d+\s)*##{number}(\s#\d+)*(\s\w+)?\]/i.match?(message)
     end
 
+    private
+
+    KEYWORD_REGEX = /\[(?<keyword>fix|fixes|fixed|complete|completes|completed|finish|finishes|finished|deliver|delivers|delivered)\]/io.freeze
+    private_constant :KEYWORD_REGEX
+
+    attr_reader :file
+
     def format_message(preamble, text, postscript)
       <<~MESSAGE
         #{preamble.strip}
@@ -40,5 +39,16 @@ def format_message(preamble, text, postscript)
         #{postscript}
       MESSAGE
     end
+
+    def message
+      @message ||= file.read.freeze
+    end
+
+    def parse(raw_message)
+      lines = raw_message.split($/)
+      body = lines.take_while { |line| !line.start_with?("#") }
+      postscript = lines.slice(body.length..-1)
+      [body.join("\n"), postscript.join("\n")]
+    end
   end
 end

spec/git_tracker/commit_message_spec.rb

@@ -3,101 +3,98 @@
 RSpec.describe GitTracker::CommitMessage do
   include CommitMessageHelper
 
-  subject(:commit_message) { described_class.new(file) }
-  let(:file) { "COMMIT_EDITMSG" }
+  subject(:commit_message) { described_class.new(commit_editmsg_file.to_path) }
+  let(:commit_editmsg_file) { Pathname(@git_dir).join("COMMIT_EDITMSG") }
+
+  around do |example|
+    Dir.mktmpdir do |dir|
+      @git_dir = dir
+      example.call
+      remove_instance_variable(:@git_dir)
+    end
+  end
 
   it "requires path to the temporary commit message file" do
     expect { GitTracker::CommitMessage.new }.to raise_error ArgumentError
   end
 
-  def stub_commit_message(story_text)
-    allow(File).to receive(:read).with(file) { example_commit_message(story_text) }
-  end
-
   describe "#keyword" do
     %w[fix Fixed FIXES Complete completed completes FINISH finished Finishes Deliver delivered DELIVERS].each do |keyword|
       it "detects the #{keyword} keyword" do
-        stub_commit_message("Did the darn thing. [#{keyword}]")
+        setup_commit_editmsg_file("Did the darn thing. [#{keyword}]")
         expect(commit_message.keyword).to eq(keyword)
       end
     end
 
     it "does not find the keyword when it does not exist" do
-      stub_commit_message("Did the darn thing. [Something]")
+      setup_commit_editmsg_file("Did the darn thing. [Something]")
       expect(commit_message.keyword).to_not be
     end
   end
 
   describe "#mentions_story?" do
     context "commit message contains the special Pivotal Tracker story syntax" do
       it "allows just the number" do
-        stub_commit_message("[#8675309]")
+        setup_commit_editmsg_file("[#8675309]")
         expect(commit_message).to be_mentions_story("8675309")
       end
 
       it "allows multiple numbers" do
-        stub_commit_message("[#99 #777 #8675309 #111222]")
-        expect(commit_message).to be_mentions_story("99")
-        expect(commit_message).to be_mentions_story("777")
-        expect(commit_message).to be_mentions_story("8675309")
-        expect(commit_message).to be_mentions_story("111222")
+        setup_commit_editmsg_file("[#99 #777 #8675308 #111222]")
+
+        aggregate_failures do
+          expect(commit_message).to be_mentions_story("99")
+          expect(commit_message).to be_mentions_story("777")
+          expect(commit_message).to be_mentions_story("8675308")
+          expect(commit_message).to be_mentions_story("111222")
+        end
       end
 
       it "allows state change before number" do
-        stub_commit_message("[Fixes #8675309]")
-        expect(commit_message).to be_mentions_story("8675309")
+        setup_commit_editmsg_file("[Fixes #8675307]")
+        expect(commit_message).to be_mentions_story("8675307")
       end
 
       it "allows state change after the number" do
-        stub_commit_message("[#8675309 Delivered]")
-        expect(commit_message).to be_mentions_story("8675309")
+        setup_commit_editmsg_file("[#8675306 Delivered]")
+        expect(commit_message).to be_mentions_story("8675306")
       end
 
       it "allows surrounding text" do
-        stub_commit_message("derp de #herp [Fixes #8675309] de herp-ity derp")
-        expect(commit_message).to be_mentions_story("8675309")
+        setup_commit_editmsg_file("derp de #herp [Fixes #8675305] de herp-ity derp")
+        expect(commit_message).to be_mentions_story("8675305")
       end
     end
 
     context "commit message doesn not contain the special Pivotal Tracker story syntax" do
       it "requires brackets" do
-        stub_commit_message("#8675309")
+        setup_commit_editmsg_file("#8675309")
         expect(commit_message).to_not be_mentions_story("8675309")
       end
 
       it "requires a pound sign" do
-        stub_commit_message("[8675309]")
+        setup_commit_editmsg_file("[8675309]")
         expect(commit_message).to_not be_mentions_story("8675309")
       end
 
       it "does not allow the bare number" do
-        stub_commit_message("8675309")
+        setup_commit_editmsg_file("8675309")
         expect(commit_message).to_not be_mentions_story("8675309")
       end
 
       it "does not allow multiple state changes" do
-        stub_commit_message("[Fixes Deploys #8675309]")
+        setup_commit_editmsg_file("[Fixes Deploys #8675309]")
         expect(commit_message).to_not be_mentions_story("8675309")
       end
 
       it "does not allow comments" do
-        stub_commit_message("#[#8675309]")
+        setup_commit_editmsg_file("#[#8675309]")
         expect(commit_message).to_not be_mentions_story("8675309")
       end
     end
   end
 
   describe "#append" do
-    let(:fake_file) { GitTracker::FakeFile.new }
-
-    before do
-      allow(File).to receive(:open).and_yield(fake_file)
-    end
-
-    def stub_original_commit_message(message)
-      allow(File).to receive(:read) { message }
-    end
-
     it "handles no existing message" do
       commit_message_text = <<~COMMIT_MESSAGE
 
@@ -106,10 +103,10 @@ def stub_original_commit_message(message)
         # some other comments
       COMMIT_MESSAGE
 
-      stub_original_commit_message("\n\n# some other comments\n")
+      write_commit_editmsg_file("\n\n# some other comments\n")
       commit_message.append("[#8675309]")
 
-      expect(fake_file.content).to eq(commit_message_text)
+      expect(commit_editmsg_file.read).to eq(commit_message_text)
     end
 
     it "preserves existing messages" do
@@ -122,10 +119,10 @@ def stub_original_commit_message(message)
         # other comments
       COMMIT_MESSAGE
 
-      stub_original_commit_message("A first line\n\nWith more here\n# other comments\n")
+      write_commit_editmsg_file("A first line\n\nWith more here\n# other comments\n")
       commit_message.append("[#8675309]")
 
-      expect(fake_file.content).to eq(commit_message_text)
+      expect(commit_editmsg_file.read).to eq(commit_message_text)
     end
 
     it "preserves line breaks in comments" do
@@ -138,10 +135,10 @@ def stub_original_commit_message(message)
         # comment III
       COMMIT_MESSAGE
 
-      stub_original_commit_message("# comment #1\n# comment B\n# comment III")
+      write_commit_editmsg_file("# comment #1\n# comment B\n# comment III")
       commit_message.append("[#8675309]")
 
-      expect(fake_file.content).to eq(commit_message_text)
+      expect(commit_editmsg_file.read).to eq(commit_message_text)
     end
   end
 end

spec/git_tracker/repository_spec.rb

@@ -3,6 +3,7 @@
 RSpec.describe GitTracker::Repository do
   subject(:repository) { described_class }
   let(:git_command) { "git rev-parse --show-toplevel" }
+
   before do
     allow_message_expectations_on_nil
     allow(repository).to receive(:`).with(git_command) { "/path/to/git/repo/root\n" }

spec/spec_helper.rb

@@ -5,7 +5,6 @@
 
 require "pry-byebug"
 require_relative "support/commit_message_helper"
-require_relative "support/fake_file"
 require_relative "support/output_helper"
 require_relative "support/matchers/exit_code_matchers"
 

spec/support/commit_message_helper.rb

@@ -1,12 +1,12 @@
 module CommitMessageHelper
-  def example_commit_message(pattern_to_match)
+  def example_commit_message(pattern:)
     <<~EXAMPLE
       Got Jenny's number, gonna' make her mine!
 
-      #{pattern_to_match}
+      #{pattern}
       # Please enter the commit message for your changes. Lines starting
       # with '#' will be ignored, and an empty message aborts the commit.
-      # On branch get_jennys_number_#8675309
+      # On branch just_a_branchy_##{pattern}
       # Changes to be committed:
       #   (use "git reset HEAD <file>..." to unstage)
       #
@@ -15,4 +15,17 @@ def example_commit_message(pattern_to_match)
 
     EXAMPLE
   end
+
+  # NOTE: The default value of `file` is ✨ magically ✨ assumed to just exist.
+  # So either pass it in explicitly, or use a `let` to define it.
+  def setup_commit_editmsg_file(story_text, file: commit_editmsg_file)
+    body = example_commit_message(pattern: story_text)
+    write_commit_editmsg_file(body, file: file)
+  end
+
+  def write_commit_editmsg_file(body, file: commit_editmsg_file)
+    Pathname(file).open("w") do |f|
+      f.write(body)
+    end
+  end
 end