Merge branch 'complexity/rubocop-metrics' into 'master'

Enable rubocop metrics

This enables rubocop metrics like CyclomaticComplexity and ABCSize.
Initial threshold values are high, should be probably decreased.

See merge request !1802

10 files changed, 368 insertions, 156 deletions

diff --git a/.rubocop.yml b/.rubocop.yml
index d59edbc8b17..b4ca11c8343 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -735,23 +735,39 @@ Metrics/AbcSize:
   Description: >-
                  A calculated magnitude based on number of assignments,
                  branches, and conditions.
-  Enabled: false
+  Enabled: true
+  Max: 70
+
+Metrics/CyclomaticComplexity:
+  Description: >-
+                 A complexity metric that is strongly correlated to the number
+                 of test cases needed to validate a method.
+  Enabled: true
+  Max: 17
+
+Metrics/PerceivedComplexity:
+  Description: >-
+                 A complexity metric geared towards measuring complexity for a
+                 human reader.
+  Enabled: true
+  Max: 17
+
+Metrics/ParameterLists:
+  Description: 'Avoid parameter lists longer than three or four parameters.'
+  StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#too-many-params'
+  Enabled: true
+  Max: 8
 
 Metrics/BlockNesting:
   Description: 'Avoid excessive block nesting'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#three-is-the-number-thou-shalt-count'
-  Enabled: false
+  Enabled: true
+  Max: 4
 
 Metrics/ClassLength:
   Description: 'Avoid classes longer than 100 lines of code.'
   Enabled: false
 
-Metrics/CyclomaticComplexity:
-  Description: >-
-                 A complexity metric that is strongly correlated to the number
-                 of test cases needed to validate a method.
-  Enabled: false
-
 Metrics/LineLength:
   Description: 'Limit lines to 80 characters.'
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#80-character-limits'
@@ -762,17 +778,6 @@ Metrics/MethodLength:
   StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#short-methods'
   Enabled: false
 
-Metrics/ParameterLists:
-  Description: 'Avoid parameter lists longer than three or four parameters.'
-  StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#too-many-params'
-  Enabled: false
-
-Metrics/PerceivedComplexity:
-  Description: >-
-                 A complexity metric geared towards measuring complexity for a
-                 human reader.
-  Enabled: false
-
 #################### Lint ################################
 ### Warnings
 
diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb
index caba63006da..b96418679bd 100644
--- a/app/mailers/emails/projects.rb
+++ b/app/mailers/emails/projects.rb
@@ -59,85 +59,17 @@ module Emails
            subject: subject("Project was moved"))
     end
 
-    def repository_push_email(project_id, recipient,  author_id: nil, 
-                                                      ref: nil, 
-                                                      action: nil, 
-                                                      compare: nil, 
-                                                      reverse_compare: false, 
-                                                      send_from_committer_email: false, 
-                                                      disable_diffs: false)
-      unless author_id && ref && action
-        raise ArgumentError, "missing keywords: author_id, ref, action"
-      end
+    def repository_push_email(project_id, recipient, opts = {})
+      @message =
+        Gitlab::Email::Message::RepositoryPush.new(self, project_id, recipient, opts)
 
-      @project = Project.find(project_id)
-      @current_user = @author  = User.find(author_id)
-      @reverse_compare = reverse_compare
-      @compare = compare
-      @ref_name  = Gitlab::Git.ref_name(ref)
-      @ref_type  = Gitlab::Git.tag_ref?(ref) ? "tag" : "branch"
-      @action  = action
-      @disable_diffs = disable_diffs
-
-      if @compare
-        @commits = Commit.decorate(compare.commits, @project)
-        @diffs   = compare.diffs
-      end
-
-      @action_name = 
-        case action
-        when :create
-          "pushed new"
-        when :delete
-          "deleted"
-        else
-          "pushed to"
-        end
-
-      @subject = "[Git]"
-      @subject << "[#{@project.path_with_namespace}]"
-      @subject << "[#{@ref_name}]" if action == :push
-      @subject << " "
-
-      if action == :push
-        if @commits.length > 1
-          @target_url = namespace_project_compare_url(@project.namespace,
-                                                      @project,
-                                                      from: Commit.new(@compare.base, @project),
-                                                      to:   Commit.new(@compare.head, @project))
-          @subject << "Deleted " if @reverse_compare
-          @subject << "#{@commits.length} commits: #{@commits.first.title}"
-        else
-          @target_url = namespace_project_commit_url(@project.namespace,
-                                                     @project, @commits.first)
-
-          @subject << "Deleted 1 commit: " if @reverse_compare
-          @subject << @commits.first.title
-        end
-      else
-        unless action == :delete
-          @target_url = namespace_project_tree_url(@project.namespace,
-                                                   @project, @ref_name)
-        end
-
-        subject_action = @action_name.dup
-        subject_action[0] = subject_action[0].capitalize
-        @subject << "#{subject_action} #{@ref_type} #{@ref_name}"
-      end
-
-      @disable_footer = true
-
-      reply_to = 
-        if send_from_committer_email && can_send_from_user_email?(@author)
-          @author.email
-        else
-          Gitlab.config.gitlab.email_reply_to
-        end
-
-      mail(from:      sender(author_id, send_from_committer_email),
-           reply_to:  reply_to,
-           to:        recipient,
-           subject:   @subject)
+      # used in notify layout
+      @target_url = @message.target_url
+
+      mail(from:      sender(@message.author_id, @message.send_from_committer_email?),
+           reply_to:  @message.reply_to,
+           to:        @message.recipient,
+           subject:   @message.subject)
     end
   end
 end
diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb
index 50a409c3754..0534eb025cd 100644
--- a/app/mailers/notify.rb
+++ b/app/mailers/notify.rb
@@ -33,13 +33,13 @@ class Notify < BaseMailer
     allowed_domains
   end
 
-  private
-
   def can_send_from_user_email?(sender)
     sender_domain = sender.email.split("@").last
     self.class.allowed_email_domains.include?(sender_domain)
   end
 
+  private
+
   # Return an email address that displays the name of the sender.
   # Only the displayed name changes; the actual email address is always the same.
   def sender(sender_id, send_from_user_email = false)
diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml
index 12f83aae04b..4361f67a74d 100644
--- a/app/views/notify/repository_push_email.html.haml
+++ b/app/views/notify/repository_push_email.html.haml
@@ -1,30 +1,32 @@
-%h3 #{@author.name} #{@action_name} #{@ref_type} #{@ref_name} at #{link_to @project.name_with_namespace, namespace_project_url(@project.namespace, @project)}
+%h3
+  #{@message.author_name} #{@message.action_name} #{@message.ref_type} #{@message.ref_name}
+  at #{link_to(@message.project_name_with_namespace, namespace_project_url(@message.project_namespace, @message.project))}
 
-- if @compare
-  - if @reverse_compare
+- if @message.compare
+  - if @message.reverse_compare?
     %p
       %strong WARNING:
       The push did not contain any new commits, but force pushed to delete the commits and changes below.
 
   %h4
-    = @reverse_compare ? "Deleted commits:" : "Commits:"
+    = @message.reverse_compare? ? "Deleted commits:" : "Commits:"
 
   %ul
-    - @commits.each do |commit|
+    - @message.commits.each do |commit|
       %li
-        %strong #{link_to commit.short_id, namespace_project_commit_url(@project.namespace, @project, commit)}
+        %strong #{link_to(commit.short_id, namespace_project_commit_url(@message.project_namespace, @message.project, commit))}
         %div
           %span by #{commit.author_name}
           %i at #{commit.committed_date.strftime("%Y-%m-%dT%H:%M:%SZ")}
         %pre.commit-message 
           = commit.safe_message
 
-  %h4 #{pluralize @diffs.count, "changed file"}:
+  %h4 #{pluralize @message.diffs_count, "changed file"}:
 
   %ul
-    - @diffs.each_with_index do |diff, i|
+    - @message.diffs.each_with_index do |diff, i|
       %li.file-stats
-        %a{href: "#{@target_url if @disable_diffs}#diff-#{i}" }
+        %a{href: "#{@message.target_url if @message.disable_diffs?}#diff-#{i}" }
           - if diff.deleted_file
             %span.deleted-file
               &minus;
@@ -40,11 +42,11 @@
           - else
             = diff.new_path
 
-  - unless @disable_diffs
+  - unless @message.disable_diffs?
     %h4 Changes:
-    - @diffs.each_with_index do |diff, i|
+    - @message.diffs.each_with_index do |diff, i|
       %li{id: "diff-#{i}"}
-        %a{href: @target_url + "#diff-#{i}"}
+        %a{href: @message.target_url + "#diff-#{i}"}
           - if diff.deleted_file
             %strong
               = diff.old_path
@@ -62,5 +64,5 @@
         = color_email_diff(diff.diff)
         %br
 
-  - if @compare.timeout
+  - if @message.compare_timeout
     %h5 Huge diff. To prevent performance issues changes are hidden
diff --git a/app/views/notify/repository_push_email.text.haml b/app/views/notify/repository_push_email.text.haml
index 97a176ed2a3..aa0e263b6df 100644
--- a/app/views/notify/repository_push_email.text.haml
+++ b/app/views/notify/repository_push_email.text.haml
@@ -1,21 +1,21 @@
-#{@author.name} #{@action_name} #{@ref_type} #{@ref_name} at #{@project.name_with_namespace}
-- if @compare
+#{@message.author_name} #{@message.action_name} #{@message.ref_type} #{@message.ref_name} at #{@message.project_name_with_namespace}
+- if @message.compare
   \
   \
-  - if @reverse_compare
+  - if @message.reverse_compare?
     WARNING: The push did not contain any new commits, but force pushed to delete the commits and changes below.
     \
     \
-  = @reverse_compare ? "Deleted commits:" : "Commits:"
-  - @commits.each do |commit|
+  = @message.reverse_compare? ? "Deleted commits:" : "Commits:"
+  - @message.commits.each do |commit|
     #{commit.short_id} by #{commit.author_name} at #{commit.committed_date.strftime("%Y-%m-%dT%H:%M:%SZ")}
     #{commit.safe_message}
     \- - - - -
   \
   \
-  #{pluralize @diffs.count, "changed file"}:
+  #{pluralize @message.diffs_count, "changed file"}:
   \
-  - @diffs.each do |diff|
+  - @message.diffs.each do |diff|
     - if diff.deleted_file
       \- − #{diff.old_path}
     - elsif diff.renamed_file
@@ -24,11 +24,11 @@
       \- + #{diff.new_path}
     - else
       \- #{diff.new_path}
-  - unless @disable_diffs
+  - unless @message.disable_diffs?
     \
     \
     Changes:
-    - @diffs.each do |diff|
+    - @message.diffs.each do |diff|
       \
       \=====================================
       - if diff.deleted_file
@@ -39,11 +39,11 @@
         = diff.new_path
       \=====================================
       != diff.diff
-  - if @compare.timeout
+  - if @message.compare_timeout
     \
     \
     Huge diff. To prevent performance issues it was hidden
-  - if @target_url
+  - if @message.target_url
     \
     \
-    View it on GitLab: #{@target_url}
+    View it on GitLab: #{@message.target_url}
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index e7fb1b79b7c..bcdfd38d292 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -132,26 +132,36 @@ module Ci
     end
 
     def validate_job!(name, job)
+      validate_job_name!(name)
+      validate_job_keys!(name, job)
+      validate_job_types!(name, job)
+
+      validate_job_stage!(name, job) if job[:stage]
+      validate_job_cache!(name, job) if job[:cache]
+      validate_job_artifacts!(name, job) if job[:artifacts]
+    end
+
+    private
+
+    def validate_job_name!(name)
       if name.blank? || !validate_string(name)
         raise ValidationError, "job name should be non-empty string"
       end
+    end
 
+    def validate_job_keys!(name, job)
       job.keys.each do |key|
         unless ALLOWED_JOB_KEYS.include? key
           raise ValidationError, "#{name} job: unknown parameter #{key}"
         end
       end
+    end
 
+    def validate_job_types!(name, job)
       if !validate_string(job[:script]) && !validate_array_of_strings(job[:script])
         raise ValidationError, "#{name} job: script should be a string or an array of a strings"
       end
 
-      if job[:stage]
-        unless job[:stage].is_a?(String) && job[:stage].in?(stages)
-          raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}"
-        end
-      end
-
       if job[:image] && !validate_string(job[:image])
         raise ValidationError, "#{name} job: image should be a string"
       end
@@ -172,36 +182,40 @@ module Ci
         raise ValidationError, "#{name} job: except parameter should be an array of strings"
       end
 
-      if job[:cache]
-        if job[:cache][:untracked] && !validate_boolean(job[:cache][:untracked])
-          raise ValidationError, "#{name} job: cache:untracked parameter should be an boolean"
-        end
-
-        if job[:cache][:paths] && !validate_array_of_strings(job[:cache][:paths])
-          raise ValidationError, "#{name} job: cache:paths parameter should be an array of strings"
-        end
+      if job[:allow_failure] && !validate_boolean(job[:allow_failure])
+        raise ValidationError, "#{name} job: allow_failure parameter should be an boolean"
       end
 
-      if job[:artifacts]
-        if job[:artifacts][:untracked] && !validate_boolean(job[:artifacts][:untracked])
-          raise ValidationError, "#{name} job: artifacts:untracked parameter should be an boolean"
-        end
+      if job[:when] && !job[:when].in?(%w(on_success on_failure always))
+        raise ValidationError, "#{name} job: when parameter should be on_success, on_failure or always"
+      end
+    end
 
-        if job[:artifacts][:paths] && !validate_array_of_strings(job[:artifacts][:paths])
-          raise ValidationError, "#{name} job: artifacts:paths parameter should be an array of strings"
-        end
+    def validate_job_stage!(name, job)
+      unless job[:stage].is_a?(String) && job[:stage].in?(stages)
+        raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}"
       end
+    end
 
-      if job[:allow_failure] && !validate_boolean(job[:allow_failure])
-        raise ValidationError, "#{name} job: allow_failure parameter should be an boolean"
+    def validate_job_cache!(name, job)
+      if job[:cache][:untracked] && !validate_boolean(job[:cache][:untracked])
+        raise ValidationError, "#{name} job: cache:untracked parameter should be an boolean"
       end
 
-      if job[:when] && !job[:when].in?(%w(on_success on_failure always))
-        raise ValidationError, "#{name} job: when parameter should be on_success, on_failure or always"
+      if job[:cache][:paths] && !validate_array_of_strings(job[:cache][:paths])
+        raise ValidationError, "#{name} job: cache:paths parameter should be an array of strings"
       end
     end
 
-    private
+    def validate_job_artifacts!(name, job)
+      if job[:artifacts][:untracked] && !validate_boolean(job[:artifacts][:untracked])
+        raise ValidationError, "#{name} job: artifacts:untracked parameter should be an boolean"
+      end
+
+      if job[:artifacts][:paths] && !validate_array_of_strings(job[:artifacts][:paths])
+        raise ValidationError, "#{name} job: artifacts:paths parameter should be an array of strings"
+      end
+    end
 
     def validate_array_of_strings(values)
       values.is_a?(Array) && values.all? { |value| validate_string(value) }
diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb
new file mode 100644
index 00000000000..a2eb7a70bd2
--- /dev/null
+++ b/lib/gitlab/email/message/repository_push.rb
@@ -0,0 +1,137 @@
+module Gitlab
+  module Email
+    module Message
+      class RepositoryPush
+        attr_accessor :recipient
+        attr_reader :author_id, :ref, :action
+
+        include Gitlab::Application.routes.url_helpers
+
+        delegate :namespace, :name_with_namespace, to: :project, prefix: :project
+        delegate :name, to: :author, prefix: :author
+
+        def initialize(notify, project_id, recipient, opts = {})
+          raise ArgumentError, 'Missing options: author_id, ref, action' unless
+            opts[:author_id] && opts[:ref] && opts[:action]
+
+          @notify = notify
+          @project_id = project_id
+          @recipient = recipient
+          @opts = opts.dup
+
+          @author_id = @opts.delete(:author_id)
+          @ref = @opts.delete(:ref)
+          @action = @opts.delete(:action)
+        end
+
+        def project
+          @project ||= Project.find(@project_id)
+        end
+
+        def author
+          @author ||= User.find(@author_id)
+        end
+
+        def commits
+          @commits ||= (Commit.decorate(compare.commits, project) if compare)
+        end
+
+        def diffs
+          @diffs ||= (compare.diffs if compare)
+        end
+
+        def diffs_count
+          diffs.count if diffs
+        end
+
+        def compare
+          @opts[:compare]
+        end
+
+        def compare_timeout
+          compare.timeout if compare
+        end
+
+        def reverse_compare?
+          @opts[:reverse_compare] || false
+        end
+
+        def disable_diffs?
+          @opts[:disable_diffs] || false
+        end
+
+        def send_from_committer_email?
+          @opts[:send_from_committer_email] || false
+        end
+
+        def action_name
+          @action_name ||=
+            case @action
+            when :create
+              'pushed new'
+            when :delete
+              'deleted'
+            else
+              'pushed to'
+            end
+        end
+
+        def ref_name
+          @ref_name ||= Gitlab::Git.ref_name(@ref)
+        end
+
+        def ref_type
+          @ref_type ||= Gitlab::Git.tag_ref?(@ref) ? 'tag' : 'branch'
+        end
+
+        def target_url
+          if @action == :push && commits
+            if commits.length > 1
+              namespace_project_compare_url(project_namespace,
+                                            project,
+                                            from: Commit.new(compare.base, project),
+                                            to:   Commit.new(compare.head, project))
+            else
+              namespace_project_commit_url(project_namespace,
+                                           project, commits.first)
+            end
+          else
+            unless @action == :delete
+              namespace_project_tree_url(project_namespace,
+                                         project, ref_name)
+            end
+          end
+        end
+
+        def reply_to
+          if send_from_committer_email? && @notify.can_send_from_user_email?(author)
+            author.email
+          else
+            Gitlab.config.gitlab.email_reply_to
+          end
+        end
+
+        def subject
+          subject_text = '[Git]'
+          subject_text << "[#{project.path_with_namespace}]"
+          subject_text << "[#{ref_name}]" if @action == :push
+          subject_text << ' '
+
+          if @action == :push && commits
+            if commits.length > 1
+              subject_text << "Deleted " if reverse_compare?
+              subject_text << "#{commits.length} commits: #{commits.first.title}"
+            else
+              subject_text << "Deleted 1 commit: " if reverse_compare?
+              subject_text << commits.first.title
+            end
+          else
+            subject_action = action_name.dup
+            subject_action[0] = subject_action[0].capitalize
+            subject_text << "#{subject_action} #{ref_type} #{ref_name}"
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index ce4a5244bd0..c90133fbf03 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -532,21 +532,21 @@ module Ci
       end
 
       it "returns errors if job stage is not a string" do
-        config = YAML.dump({ rspec: { script: "test", type: 1, allow_failure: "string" } })
+        config = YAML.dump({ rspec: { script: "test", type: 1 } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy")
       end
 
       it "returns errors if job stage is not a pre-defined stage" do
-        config = YAML.dump({ rspec: { script: "test", type: "acceptance", allow_failure: "string" } })
+        config = YAML.dump({ rspec: { script: "test", type: "acceptance" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy")
       end
 
       it "returns errors if job stage is not a defined stage" do
-        config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", type: "acceptance", allow_failure: "string" } })
+        config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", type: "acceptance" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test")
diff --git a/spec/lib/gitlab/email/message/repository_push_spec.rb b/spec/lib/gitlab/email/message/repository_push_spec.rb
new file mode 100644
index 00000000000..56ae2a8d121
--- /dev/null
+++ b/spec/lib/gitlab/email/message/repository_push_spec.rb
@@ -0,0 +1,122 @@
+require 'spec_helper'
+
+describe Gitlab::Email::Message::RepositoryPush do
+  include RepoHelpers
+
+  let!(:group) { create(:group, name: 'my_group') }
+  let!(:project) { create(:project, name: 'my_project', namespace: group) }
+  let!(:author) { create(:author, name: 'Author') }
+
+  let(:message) do
+    described_class.new(Notify, project.id, 'recipient@example.com', opts)
+  end
+
+  context 'new commits have been pushed to repository' do
+    let(:opts) do
+      { author_id: author.id, ref: 'master', action: :push, compare: compare,
+        send_from_committer_email: true }
+    end
+    let(:compare) do
+      Gitlab::Git::Compare.new(project.repository.raw_repository,
+                               sample_image_commit.id, sample_commit.id)
+    end
+
+    describe '#project' do
+      subject { message.project }
+      it { is_expected.to eq project }
+      it { is_expected.to be_an_instance_of Project }
+    end
+
+    describe '#project_namespace' do
+      subject { message.project_namespace }
+      it { is_expected.to eq group }
+      it { is_expected.to be_kind_of Namespace }
+    end
+
+    describe '#project_name_with_namespace' do
+      subject { message.project_name_with_namespace }
+      it { is_expected.to eq 'my_group / my_project' }
+    end
+
+    describe '#author' do
+      subject { message.author }
+      it { is_expected.to eq author }
+      it { is_expected.to be_an_instance_of User }
+    end
+
+    describe '#author_name' do
+      subject { message.author_name }
+      it { is_expected.to eq 'Author' }
+    end
+
+    describe '#commits' do
+      subject { message.commits }
+      it { is_expected.to be_kind_of Array }
+      it { is_expected.to all(be_instance_of Commit) }
+    end
+
+    describe '#diffs' do
+      subject { message.diffs }
+      it { is_expected.to all(be_an_instance_of Gitlab::Git::Diff) }
+    end
+
+    describe '#diffs_count' do
+      subject { message.diffs_count }
+      it { is_expected.to eq compare.diffs.count }
+    end
+
+    describe '#compare' do
+      subject { message.compare }
+      it { is_expected.to be_an_instance_of Gitlab::Git::Compare }
+    end
+
+    describe '#compare_timeout' do
+      subject { message.compare_timeout }
+      it { is_expected.to eq compare.timeout }
+    end
+
+    describe '#reverse_compare?' do
+      subject { message.reverse_compare? }
+      it { is_expected.to eq false }
+    end
+
+    describe '#disable_diffs?' do
+      subject { message.disable_diffs? }
+      it { is_expected.to eq false }
+    end
+
+    describe '#send_from_committer_email?' do
+      subject { message.send_from_committer_email? }
+      it { is_expected.to eq true }
+    end
+
+    describe '#action_name' do
+      subject { message.action_name }
+      it { is_expected.to eq 'pushed to' }
+    end
+
+    describe '#ref_name' do
+      subject { message.ref_name }
+      it { is_expected.to eq 'master' }
+    end
+
+    describe '#ref_type' do
+      subject { message.ref_type }
+      it { is_expected.to eq 'branch' }
+    end
+
+    describe '#target_url' do
+      subject { message.target_url }
+      it { is_expected.to include 'compare' }
+      it { is_expected.to include compare.commits.first.parents.first.id }
+      it { is_expected.to include compare.commits.last.id }
+    end
+
+    describe '#subject' do
+      subject { message.subject }
+      it { is_expected.to include "[Git][#{project.path_with_namespace}]" }
+      it { is_expected.to include "#{compare.commits.length} commits" }
+      it { is_expected.to include compare.commits.first.message.split("\n").first }
+    end
+  end
+end
diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb
index 33d2b14583c..fce91015fd4 100644
--- a/spec/support/mentionable_shared_examples.rb
+++ b/spec/support/mentionable_shared_examples.rb
@@ -4,7 +4,7 @@
 # - let(:backref_text) { "the way that +subject+ should refer to itself in backreferences " }
 # - let(:set_mentionable_text) { lambda { |txt| "block that assigns txt to the subject's mentionable_text" } }
 
-def common_mentionable_setup
+shared_context 'mentionable context' do
   let(:project) { subject.project }
   let(:author)  { subject.author }
 
@@ -56,7 +56,7 @@ def common_mentionable_setup
 end
 
 shared_examples 'a mentionable' do
-  common_mentionable_setup
+  include_context 'mentionable context'
 
   it 'generates a descriptive back-reference' do
     expect(subject.gfm_reference).to eq(backref_text)
@@ -88,7 +88,7 @@ shared_examples 'a mentionable' do
 end
 
 shared_examples 'an editable mentionable' do
-  common_mentionable_setup
+  include_context 'mentionable context'
 
   it_behaves_like 'a mentionable'