Properly account for tags in PushResults

Despite the name, tags are also pushed via the `push_remote_branches`
method in `Gitlab::Git::RepositoryMirroring`.

5 files changed, 37 insertions, 18 deletions

diff --git a/changelogs/unreleased/rs-push-results-with-tags.yml b/changelogs/unreleased/rs-push-results-with-tags.yml
new file mode 100644
index 000000000..fce22d392
--- /dev/null
+++ b/changelogs/unreleased/rs-push-results-with-tags.yml
@@ -0,0 +1,5 @@
+---
+title: Properly account for tags in PushResults
+merge_request: 1874
+author:
+type: fixed
diff --git a/ruby/lib/gitlab/git/push_results.rb b/ruby/lib/gitlab/git/push_results.rb
index 87ab3c145..dd4ee2795 100644
--- a/ruby/lib/gitlab/git/push_results.rb
+++ b/ruby/lib/gitlab/git/push_results.rb
@@ -30,14 +30,14 @@ module Gitlab
         end.compact
       end
 
-      # Returns an Array of branch names that were not rejected nor up-to-date
-      def accepted_branches
-        all.select(&:accepted?).collect(&:branch_name)
+      # Returns an Array of ref names that were not rejected nor up-to-date
+      def accepted_refs
+        all.select(&:accepted?).collect(&:ref_name)
       end
 
-      # Returns an Array of branch names that were rejected
-      def rejected_branches
-        all.select(&:rejected?).collect(&:branch_name)
+      # Returns an Array of ref names that were rejected
+      def rejected_refs
+        all.select(&:rejected?).collect(&:ref_name)
       end
 
       Result = Struct.new(:flag_char, :from, :to, :summary) do
@@ -67,8 +67,8 @@ module Gitlab
           !rejected? && flag != :up_to_date
         end
 
-        def branch_name
-          to.delete_prefix('refs/heads/')
+        def ref_name
+          to.sub(%r{\Arefs/(heads|tags)/}, '')
         end
       end
     end
diff --git a/ruby/lib/gitlab/git/repository_mirroring.rb b/ruby/lib/gitlab/git/repository_mirroring.rb
index 7e0cb99d2..4b226fef9 100644
--- a/ruby/lib/gitlab/git/repository_mirroring.rb
+++ b/ruby/lib/gitlab/git/repository_mirroring.rb
@@ -38,13 +38,13 @@ module Gitlab
 
         results = PushResults.new(@gitlab_projects.output)
 
-        accepted_branches = results.accepted_branches.join(', ')
-        rejected_branches = results.rejected_branches.join(', ')
+        accepted_refs = results.accepted_refs.join(', ')
+        rejected_refs = results.rejected_refs.join(', ')
 
         @gitlab_projects.logger.info(
           "Failed to push to remote #{remote_name}. " \
-          "Accepted: #{accepted_branches} / " \
-          "Rejected: #{rejected_branches}"
+          "Accepted: #{accepted_refs} / " \
+          "Rejected: #{rejected_refs}"
         )
 
         gitlab_projects_error
diff --git a/ruby/spec/lib/gitlab/git/push_results_spec.rb b/ruby/spec/lib/gitlab/git/push_results_spec.rb
index 4a14123b4..4a2cd2e02 100644
--- a/ruby/spec/lib/gitlab/git/push_results_spec.rb
+++ b/ruby/spec/lib/gitlab/git/push_results_spec.rb
@@ -13,6 +13,7 @@ describe Gitlab::Git::PushResults do
       -\trefs/heads/rs-deleted:refs/heads/rs-deleted\t[deleted]
       +\trefs/heads/rs-forced:refs/heads/rs-forced\t[forced]
       !\trefs/heads/12-7-stable:refs/heads/12-7-stable\t[rejected] (fetch first)
+      *\trefs/tags/v1.2.3:refs/tags/v1.2.3\t[new tag]
       Done
       error: failed to push some refs to 'git@gitlab.com:gitlab-org/security/gitlab-foss.git'
       hint: Updates were rejected because the remote contains work that you do
@@ -24,14 +25,15 @@ describe Gitlab::Git::PushResults do
 
     results = described_class.new(output)
 
-    expect(results.all.size).to eq(7)
-    expect(results.accepted_branches).to contain_exactly(
+    expect(results.all.size).to eq(8)
+    expect(results.accepted_refs).to contain_exactly(
       'rs-some-new-branch',
       'rs-fast-forward',
       'rs-forced',
-      'rs-deleted'
+      'rs-deleted',
+      'v1.2.3'
     )
-    expect(results.rejected_branches).to contain_exactly('12-7-stable')
+    expect(results.rejected_refs).to contain_exactly('12-7-stable')
   end
 
   it 'ignores non-porcelain output' do
@@ -67,4 +69,16 @@ describe Gitlab::Git::PushResults do
 
     expect(described_class.new(output).all).to eq([])
   end
+
+  describe Gitlab::Git::PushResults::Result do
+    describe '#ref_name' do
+      it 'deletes only one prefix' do
+        # It's  valid (albeit insane) for a branch to be named `refs/tags/foo`
+        ref_name = 'refs/heads/refs/tags/branch'
+        result = described_class.new('!', ref_name, ref_name, '')
+
+        expect(result.ref_name).to eq('refs/tags/branch')
+      end
+    end
+  end
 end
diff --git a/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb b/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb
index 57edf5af4..927cb863e 100644
--- a/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb
+++ b/ruby/spec/lib/gitlab/git/repository_mirroring_spec.rb
@@ -43,8 +43,8 @@ describe Gitlab::Git::RepositoryMirroring do
         allow(projects_stub).to receive(:logger).and_return(logger)
 
         push_results = double(
-          accepted_branches: %w[develop],
-          rejected_branches: %w[master]
+          accepted_refs: %w[develop],
+          rejected_refs: %w[master]
         )
         expect(Gitlab::Git::PushResults).to receive(:new)
           .with(output)