refactor: revert changes to make path normalization optional

Revert the changes introduced in #64 that allowed the user of this lib to make
path normalization optional. The purpose of #64 was to make tests easier to
write. The downside was that the library code itself became more complicated.

Instead this change removes that and adds mocking for path normalization
when it is appropriate to shut it off for tests.

Author: jcouball

lib/ruby_git/repository.rb

@@ -25,32 +25,19 @@ class Repository
     #   RubyGit::Repository.new('/path/to/repository') #=> #<RubyGit::Repository ...>
     #
     # @param [String] repository_path the path to the repository
-    # @param normalize_path [Boolean] if true, path is converted to an absolute path to the root of the working tree
     #
     #   The purpose of this flag is to allow tests to not have to mock the
     #   normalization of the path. This allows testing that the right git command
     #   is contructed based on the options passed any particular method.
     #
     # @raise [ArgumentError] if the path is not a directory
     #
-    def initialize(repository_path, normalize_path: true)
-      @normalize_path = normalize_path
-
-      @path =
-        if normalize_path?
-          normalize_path(repository_path)
-        else
-          repository_path
-        end
+    def initialize(repository_path)
+      @path = normalize_path(repository_path)
     end
 
     private
 
-    # true if the path should be expanded and converted to a absolute, real path
-    # @return [Boolean]
-    # @api private
-    def normalize_path? = @normalize_path
-
     # Expand and convert the given path to an absolute, real path
     #
     # @example Expand the path

lib/ruby_git/worktree.rb

@@ -34,14 +34,14 @@ class Worktree
     #
     # @return [RubyGit::Worktree] the working tree whose root is at `path`
     #
-    def self.init(worktree_path, normalize_path: true)
+    def self.init(worktree_path)
       raise RubyGit::Error, "Path '#{worktree_path}' not valid." unless File.directory?(worktree_path)
 
       command = ['init']
       options = { chdir: worktree_path, out: StringIO.new, err: StringIO.new }
       RubyGit::CommandLine.run(*command, **options)
 
-      new(worktree_path, normalize_path:)
+      new(worktree_path)
     end
 
     # Open an existing Git working tree that contains worktree_path
@@ -58,8 +58,8 @@ def self.init(worktree_path, normalize_path: true)
     #
     # @return [RubyGit::Worktree] the Git working tree that contains `worktree_path`
     #
-    def self.open(worktree_path, normalize_path: true)
-      new(worktree_path, normalize_path:)
+    def self.open(worktree_path)
+      new(worktree_path)
     end
 
     # Copy the remote repository and checkout the default branch
@@ -97,12 +97,12 @@ def self.open(worktree_path, normalize_path: true)
     #
     # @return [RubyGit::Worktree] the Git working tree checked out from the cloned repository
     #
-    def self.clone(repository_url, to_path: nil, normalize_path: true)
+    def self.clone(repository_url, to_path: nil)
       command = ['clone', '--', repository_url]
       command << to_path if to_path
       options = { out: StringIO.new, err: StringIO.new }
       clone_output = RubyGit::CommandLine.run(*command, **options).stderr
-      new(cloned_to(clone_output), normalize_path:)
+      new(cloned_to(clone_output))
     end
 
     # Show the working tree and index status
@@ -195,21 +195,27 @@ def add(*pathspecs, all: false, force: false, refresh: false, update: false) # r
     # @return [RubyGit::Repository] the repository associated with the worktree
     #
     def repository
-      @repository ||= begin
-        command = %w[rev-parse --git-dir]
-        options = { chdir: path, chomp: true, out: StringIO.new, err: StringIO.new }
-        # rev-parse path might be relative to the worktree, thus the need to expand it
-        git_dir = File.realpath(RubyGit::CommandLine.run(*command, **options).stdout, path)
-        Repository.new(git_dir, normalize_path: normalize_path?)
-      end
+      @repository ||= Repository.new(repository_path)
     end
 
     private
 
+    # The path to the repository associated with this worktree
+    #
+    # @return [String]
+    #
+    # @api private
+    #
+    def repository_path
+      command = %w[rev-parse --git-dir]
+      options = { chdir: path, chomp: true, out: StringIO.new, err: StringIO.new }
+      # rev-parse path might be relative to the worktree, thus the need to expand it
+      File.realpath(RubyGit::CommandLine.run(*command, **options).stdout, path)
+    end
+
     # Create a Worktree object
     #
     # @param worktree_path [String] a path anywhere in the worktree
-    # @param normalize_path [Boolean] if true, path is converted to an absolute path to the root of the working tree
     #
     #   The purpose of this flag is to allow tests to not have to mock the
     #   normalization of the path. This allows testing that the right git command
@@ -221,16 +227,8 @@ def repository
     # @return [RubyGit::Worktree] the worktree whose root is at `path`
     # @api private
     #
-    def initialize(worktree_path, normalize_path: true)
-      @normalize_path = normalize_path
-
-      @path =
-        if normalize_path?
-          normalize_worktree_path(worktree_path)
-        else
-          worktree_path
-        end
-
+    def initialize(worktree_path)
+      @path = normalize_path(worktree_path)
       RubyGit.logger.debug("Created #{inspect}")
     end
 
@@ -245,17 +243,6 @@ def initialize(worktree_path, normalize_path: true)
       clone_output.match(/Cloning into ['"](.+)['"]\.\.\./)[1]
     end
 
-    # True if the path should be normalized
-    #
-    # This means that the path should be expanded and converted to a absolute, real
-    # path to the working tree root dir.
-    #
-    # @return [Boolean]
-    #
-    # @api private
-    #
-    def normalize_path? = @normalize_path
-
     # Return the absolute path to the root of the working tree containing path
     #
     # @example Expand the path
@@ -274,7 +261,7 @@ def normalize_path? = @normalize_path
     #
     # @api private
     #
-    def normalize_worktree_path(path)
+    def normalize_path(path)
       raise ArgumentError, "Directory '#{path}' does not exist." unless File.directory?(path)
 
       begin

spec/lib/ruby_git/repository_spec.rb

@@ -1,31 +1,22 @@
 # frozen_string_literal: true
 
 RSpec.describe RubyGit::Repository do
-  let(:repository) { RubyGit::Repository.new(repository_path, normalize_path:) }
+  let(:repository) { RubyGit::Repository.new(repository_path) }
   let(:repository_path) { '.' }
 
   describe '#initialize' do
     subject { repository }
 
     around do |example|
-      in_temp_dir do |_repository_path|
+      in_temp_dir do
+        run %w[git init --initial-branch=main]
         example.run
       end
     end
 
-    context 'when normalize_path is true' do
-      let(:normalize_path) { true }
-      it 'path should be the absolute path to the repository' do
-        expected_path = File.realpath(File.expand_path(repository_path))
-        expect(subject).to have_attributes(path: expected_path)
-      end
-    end
-
-    context 'when normalize_path is false' do
-      let(:normalize_path) { false }
-      it 'path should be as given' do
-        expect(subject).to have_attributes(path: repository_path)
-      end
+    it 'path should be the absolute path to the repository' do
+      expected_path = File.realpath(File.expand_path(repository_path))
+      expect(subject).to have_attributes(path: expected_path)
     end
   end
 end

spec/lib/ruby_git/worktree_add_spec.rb

@@ -33,7 +33,7 @@ def untracked_entries
     end
 
     describe 'calling the git add command line' do
-      let(:worktree) { described_class.new(worktree_path, normalize_path: false) }
+      let(:worktree) { described_class.new(worktree_path) }
       let(:worktree_path) { '/some/worktree_path' } # Dummy path for testing
 
       let(:subject_object) { worktree } # for the it_behaves_like 'it runs the git command'
@@ -79,11 +79,7 @@ def untracked_entries
           let(:pathspecs) { [] }
           let(:options) { { all: 'invalid' } }
 
-          it 'should raise an error' do
-            expect { subject }.to(
-              raise_error(ArgumentError, %(The 'all:' option must be a Boolean value but was "invalid"))
-            )
-          end
+          it_behaves_like 'it raises an ArgumentError', %(The 'all:' option must be a Boolean value but was "invalid")
         end
       end
 
@@ -106,11 +102,10 @@ def untracked_entries
           let(:pathspecs) { [] }
           let(:options) { { force: 'invalid' } }
 
-          it 'should raise an error' do
-            expect { subject }.to(
-              raise_error(ArgumentError, %(The 'force:' option must be a Boolean value but was "invalid"))
-            )
-          end
+          it_behaves_like(
+            'it raises an ArgumentError',
+            %(The 'force:' option must be a Boolean value but was "invalid")
+          )
         end
       end
 
@@ -133,11 +128,10 @@ def untracked_entries
           let(:pathspecs) { [] }
           let(:options) { { update: 'invalid' } }
 
-          it 'should raise an error' do
-            expect { subject }.to(
-              raise_error(ArgumentError, %(The 'update:' option must be a Boolean value but was "invalid"))
-            )
-          end
+          it_behaves_like(
+            'it raises an ArgumentError',
+            %(The 'update:' option must be a Boolean value but was "invalid")
+          )
         end
       end
 
@@ -160,11 +154,10 @@ def untracked_entries
           let(:pathspecs) { [] }
           let(:options) { { refresh: 'invalid' } }
 
-          it 'should raise an error' do
-            expect { subject }.to(
-              raise_error(ArgumentError, %(The 'refresh:' option must be a Boolean value but was "invalid"))
-            )
-          end
+          it_behaves_like(
+            'it raises an ArgumentError',
+            %(The 'refresh:' option must be a Boolean value but was "invalid")
+          )
         end
       end
     end

spec/lib/ruby_git/worktree_spec.rb

@@ -1,9 +1,8 @@
 # frozen_string_literal: true
 
 RSpec.describe RubyGit::Worktree do
-  let(:worktree) { RubyGit::Worktree.open(worktree_path, normalize_path:) }
+  let(:worktree) { RubyGit::Worktree.open(worktree_path) }
   let(:worktree_path) { '.' }
-  let(:normalize_path) { true }
 
   describe '#initialize' do
     subject { worktree }
@@ -16,30 +15,19 @@
       end
     end
 
-    context 'when normalize_path is true' do
-      let(:normalize_path) { true }
-      context 'when worktree_path is the working tree root directory' do
-        let(:worktree_path) { '.' }
-        it 'should have a path to the working tree root directory' do
-          expected_path = File.realpath(File.expand_path(worktree_path))
-          expect(subject).to have_attributes(path: expected_path)
-        end
-      end
-
-      context 'when worktree_path is a subdirectory within the working tree' do
-        let(:worktree_path) { 'subdir' }
-        it 'should have a path to the working tree root directory' do
-          expected_path = File.realpath(File.expand_path('.'))
-          expect(subject).to have_attributes(path: expected_path)
-        end
+    context 'when worktree_path is the working tree root directory' do
+      let(:worktree_path) { '.' }
+      it 'should have a path to the working tree root directory' do
+        expected_path = File.realpath(File.expand_path('.'))
+        expect(subject).to have_attributes(path: expected_path)
       end
     end
 
-    context 'when normalize_path is false' do
-      let(:normalize_path) { false }
-      let(:worktree_path) { 'blah/blah/blah' }
-      it 'should have the given path' do
-        expect(subject).to have_attributes(path: worktree_path)
+    context 'when worktree_path is a subdirectory within the working tree' do
+      let(:worktree_path) { 'subdir' }
+      it 'should have a path to the working tree root directory' do
+        expected_path = File.realpath(File.expand_path('.'))
+        expect(subject).to have_attributes(path: expected_path)
       end
     end
   end

spec/spec_helper.rb

@@ -173,6 +173,10 @@ def eol = windows? ? "\r\n" : "\n"
 #
 RSpec.shared_examples 'it runs the git command' do |command, options = Hash|
   it 'should build the correct command' do
+    allow_any_instance_of(described_class).to(
+      receive(:normalize_path) { |_, path| path }
+    )
+
     if options.is_a?(Hash)
       # If specific options are given, double splat them into the argument list
       # binding.irb
@@ -191,6 +195,16 @@ def eol = windows? ? "\r\n" : "\n"
   end
 end
 
+RSpec.shared_examples 'it raises an ArgumentError' do |message|
+  it 'should raise an Argument' do
+    allow_any_instance_of(described_class).to(
+      receive(:normalize_path) { |_, path| path }
+    )
+
+    expect { subject }.to(raise_error(ArgumentError, message))
+  end
+end
+
 SimpleCov::RSpec.start(list_uncovered_lines: ci_build?)
 
 require 'ruby_git'