#push without args should do same as git push with no args (#636)

jcouball · web-flow · commit 0c908da90980 · 2023-03-02T08:09:35.000-08:00
Signed-off-by: James Couball &lt;jcouball@yahoo.com&gt;
diff --git a/lib/git/base.rb b/lib/git/base.rb
@@ -374,11 +374,8 @@ def fetch(remote = 'origin', opts = {})
     #
     #  @git.config('remote.remote-name.push', 'refs/heads/master:refs/heads/master')
     #
-    def push(remote = 'origin', branch = 'master', opts = {})
-      # Small hack to keep backwards compatibility with the 'push(remote, branch, tags)' method signature.
-      opts = {:tags => opts} if [true, false].include?(opts)
-
-      self.lib.push(remote, branch, opts)
+    def push(*args, **options)
+      self.lib.push(*args, **options)
     end
 
     # merges one or more branches into the current working branch
diff --git a/lib/git/lib.rb b/lib/git/lib.rb
@@ -908,20 +908,36 @@ def fetch(remote, opts)
       command('fetch', *arr_opts)
     end
 
-    def push(remote, branch = 'master', opts = {})
+    def push(remote = nil, branch = nil, opts = nil)
+      if opts.nil? && branch.instance_of?(Hash)
+        opts = branch
+        branch = nil
+      end
+
+      if opts.nil? && remote.instance_of?(Hash)
+        opts = remote
+        remote = nil
+      end
+
+      opts ||= {}
+
       # Small hack to keep backwards compatibility with the 'push(remote, branch, tags)' method signature.
       opts = {:tags => opts} if [true, false].include?(opts)
 
+      raise ArgumentError, "You must specify a remote if a branch is specified" if remote.nil? && !branch.nil?
+
       arr_opts = []
       arr_opts << '--mirror'  if opts[:mirror]
       arr_opts << '--delete'  if opts[:delete]
       arr_opts << '--force'  if opts[:force] || opts[:f]
-      arr_opts << remote
+      arr_opts << remote if remote
+      arr_opts_with_branch = arr_opts.dup
+      arr_opts_with_branch << branch if branch
 
       if opts[:mirror]
-          command('push', *arr_opts)
+          command('push', *arr_opts_with_branch)
       else
-          command('push', *arr_opts, branch)
+          command('push', *arr_opts_with_branch)
           command('push', '--tags', *arr_opts) if opts[:tags]
       end
     end
diff --git a/tests/units/test_push.rb b/tests/units/test_push.rb
@@ -0,0 +1,110 @@
+require 'test_helper'
+
+class TestPush < Test::Unit::TestCase
+  test 'push with no args' do
+    expected_command_line = ['push']
+    git_cmd = :push
+    git_cmd_args = []
+    assert_command_line(expected_command_line, git_cmd, git_cmd_args)
+  end
+
+  test 'push with no args and options' do
+    expected_command_line = ['push', '--force']
+    git_cmd = :push
+    git_cmd_args = [force: true]
+    assert_command_line(expected_command_line, git_cmd, git_cmd_args)
+  end
+
+  test 'push with only a remote name' do
+    expected_command_line = ['push', 'origin']
+    git_cmd = :push
+    git_cmd_args = ['origin']
+    assert_command_line(expected_command_line, git_cmd, git_cmd_args)
+  end
+
+  test 'push with only a remote name and options' do
+    expected_command_line = ['push', '--force', 'origin']
+    git_cmd = :push
+    git_cmd_args = ['origin', force: true]
+    assert_command_line(expected_command_line, git_cmd, git_cmd_args)
+  end
+
+  test 'push with only a branch name' do
+    expected_command_line = ['push', 'master']
+    git_cmd = :push
+    git_cmd_args = [nil, 'origin']
+
+    in_temp_dir do
+      git = Git.init('.', initial_branch: 'master')
+      assert_raises(ArgumentError) { git.push(nil, 'master') }
+    end
+  end
+
+  test 'push with both remote and branch name' do
+    expected_command_line = ['push', 'origin', 'master']
+    git_cmd = :push
+    git_cmd_args = ['origin', 'master']
+    assert_command_line(expected_command_line, git_cmd, git_cmd_args)
+  end
+
+  test 'push with force: true' do
+    expected_command_line = ['push', '--force', 'origin', 'master']
+    git_cmd = :push
+    git_cmd_args = ['origin', 'master', force: true]
+    assert_command_line(expected_command_line, git_cmd, git_cmd_args)
+  end
+
+  test 'push with f: true' do
+    expected_command_line = ['push', '--force', 'origin', 'master']
+    git_cmd = :push
+    git_cmd_args = ['origin', 'master', f: true]
+    assert_command_line(expected_command_line, git_cmd, git_cmd_args)
+  end
+
+  test 'push with mirror: true' do
+    expected_command_line = ['push', '--force', 'origin', 'master']
+    git_cmd = :push
+    git_cmd_args = ['origin', 'master', f: true]
+    assert_command_line(expected_command_line, git_cmd, git_cmd_args)
+  end
+
+  test 'push with delete: true' do
+    expected_command_line = ['push', '--delete', 'origin', 'master']
+    git_cmd = :push
+    git_cmd_args = ['origin', 'master', delete: true]
+    assert_command_line(expected_command_line, git_cmd, git_cmd_args)
+  end
+
+  test 'push with tags: true' do
+    expected_command_line = ['push', '--tags', 'origin']
+    git_cmd = :push
+    git_cmd_args = ['origin', nil, tags: true]
+    assert_command_line(expected_command_line, git_cmd, git_cmd_args)
+  end
+
+  test 'when push succeeds an error should not be raised' do
+    in_temp_dir do
+      Git.init('remote.git', initial_branch: 'master', bare: true)
+
+      git = Git.clone('remote.git', 'local')
+      Dir.chdir 'local' do
+        File.write('File2.txt', 'hello world')
+        git.add('File2.txt')
+        git.commit('Second commit')
+        assert_nothing_raised { git.push }
+      end
+    end
+  end
+
+  test 'when push fails a Git::FailedError should be raised' do
+    in_temp_dir do
+      Git.init('remote.git', initial_branch: 'master', bare: true)
+
+      git = Git.clone('remote.git', 'local')
+      Dir.chdir 'local' do
+        # Pushing when there is nothing to push fails
+        assert_raises(Git::FailedError) { git.push }
+      end
+    end
+  end
+end

Original file line number	Diff line number	Diff line change
`@@ -374,11 +374,8 @@ def fetch(remote = 'origin', opts = {})`
`374`	`374`	`#`
`375`	`375`	`# @git.config('remote.remote-name.push', 'refs/heads/master:refs/heads/master')`
`376`	`376`	`#`
`377`		`- def push(remote = 'origin', branch = 'master', opts = {})`
`378`		`- # Small hack to keep backwards compatibility with the 'push(remote, branch, tags)' method signature.`
`379`		`- opts = {:tags => opts} if [true, false].include?(opts)`
`380`		`-`
`381`		`- self.lib.push(remote, branch, opts)`
	`377`	`+ def push(args, *options)`
	`378`	`+ self.lib.push(args, *options)`
`382`	`379`	`end`
`383`	`380`
`384`	`381`	`# merges one or more branches into the current working branch`