Merge branch 'rs-fix-application-helper-specs' into 'master'

Fix ApplicationHelper specs

On current `master` if you run these specs by themselves, they fail. I have no idea why they work when run with other specs, but now they work all the time.

- Use the `helper` object, as per RSpec 3 standards
- Use `assign` to assign instance variables that helpers expect
- Add `StubConfiguration` support module

See merge request !906

spec/helpers/application_helper_spec.rb

@@ -2,157 +2,177 @@ require 'spec_helper'
 
 describe ApplicationHelper do
   describe 'current_controller?' do
-    before do
-      allow(controller).to receive(:controller_name).and_return('foo')
-    end
-
     it 'returns true when controller matches argument' do
-      expect(current_controller?(:foo)).to be_truthy
+      stub_controller_name('foo')
+
+      expect(helper.current_controller?(:foo)).to eq true
     end
 
     it 'returns false when controller does not match argument' do
-      expect(current_controller?(:bar)).not_to be_truthy
+      stub_controller_name('foo')
+
+      expect(helper.current_controller?(:bar)).to eq false
     end
 
-    it 'should take any number of arguments' do
-      expect(current_controller?(:baz, :bar)).not_to be_truthy
-      expect(current_controller?(:baz, :bar, :foo)).to be_truthy
+    it 'takes any number of arguments' do
+      stub_controller_name('foo')
+
+      expect(helper.current_controller?(:baz, :bar)).to eq false
+      expect(helper.current_controller?(:baz, :bar, :foo)).to eq true
+    end
+
+    def stub_controller_name(value)
+      allow(helper.controller).to receive(:controller_name).and_return(value)
     end
   end
 
   describe 'current_action?' do
-    before do
-      allow(self).to receive(:action_name).and_return('foo')
+    it 'returns true when action matches' do
+      stub_action_name('foo')
+
+      expect(helper.current_action?(:foo)).to eq true
     end
 
-    it 'returns true when action matches argument' do
-      expect(current_action?(:foo)).to be_truthy
+    it 'returns false when action does not match' do
+      stub_action_name('foo')
+
+      expect(helper.current_action?(:bar)).to eq false
     end
 
-    it 'returns false when action does not match argument' do
-      expect(current_action?(:bar)).not_to be_truthy
+    it 'takes any number of arguments' do
+      stub_action_name('foo')
+
+      expect(helper.current_action?(:baz, :bar)).to eq false
+      expect(helper.current_action?(:baz, :bar, :foo)).to eq true
     end
 
-    it 'should take any number of arguments' do
-      expect(current_action?(:baz, :bar)).not_to be_truthy
-      expect(current_action?(:baz, :bar, :foo)).to be_truthy
+    def stub_action_name(value)
+      allow(helper).to receive(:action_name).and_return(value)
     end
   end
 
   describe 'project_icon' do
-    avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')
+    let(:avatar_file_path) { File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') }
 
     it 'should return an url for the avatar' do
-      project = create(:project)
-      project.avatar = File.open(avatar_file_path)
-      project.save!
-      avatar_url = "http://localhost/uploads/project/avatar/#{ project.id }/banana_sample.gif"
-      expect(project_icon("#{project.namespace.to_param}/#{project.to_param}").to_s).to eq(
-        "<img alt=\"Banana sample\" src=\"#{avatar_url}\" />"
-      )
+      project = create(:project, avatar: File.open(avatar_file_path))
+
+      avatar_url = "http://localhost/uploads/project/avatar/#{project.id}/banana_sample.gif"
+      expect(helper.project_icon("#{project.namespace.to_param}/#{project.to_param}").to_s).
+        to eq "<img alt=\"Banana sample\" src=\"#{avatar_url}\" />"
     end
 
     it 'should give uploaded icon when present' do
       project = create(:project)
-      project.save!
 
       allow_any_instance_of(Project).to receive(:avatar_in_git).and_return(true)
 
       avatar_url = 'http://localhost' + namespace_project_avatar_path(project.namespace, project)
-      expect(project_icon("#{project.namespace.to_param}/#{project.to_param}").to_s).to match(
+      expect(helper.project_icon("#{project.namespace.to_param}/#{project.to_param}").to_s).to match(
         image_tag(avatar_url))
     end
   end
 
   describe 'avatar_icon' do
-    avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')
+    let(:avatar_file_path) { File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') }
 
     it 'should return an url for the avatar' do
-      user = create(:user)
-      user.avatar = File.open(avatar_file_path)
-      user.save!
-      expect(avatar_icon(user.email).to_s).
-        to match("/uploads/user/avatar/#{ user.id }/banana_sample.gif")
+      user = create(:user, avatar: File.open(avatar_file_path))
+
+      expect(helper.avatar_icon(user.email).to_s).
+        to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
     end
 
     it 'should return an url for the avatar with relative url' do
-      allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab')
-      allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
+      stub_config_setting(relative_url_root: '/gitlab')
+      # Must be stubbed after the stub above, and separately
+      stub_config_setting(url: Settings.send(:build_gitlab_url))
 
-      user = create(:user)
-      user.avatar = File.open(avatar_file_path)
-      user.save!
-      expect(avatar_icon(user.email).to_s).
-        to match("/gitlab/uploads/user/avatar/#{ user.id }/banana_sample.gif")
+      user = create(:user, avatar: File.open(avatar_file_path))
+
+      expect(helper.avatar_icon(user.email).to_s).
+        to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif")
     end
 
-    it 'should call gravatar_icon when no avatar is present' do
-      user = create(:user, email: 'test@example.com')
-      user.save!
-      expect(avatar_icon(user.email).to_s).to eq('http://www.gravatar.com/avatar/55502f40dc8b7c769880b10874abc9d0?s=40&d=identicon')
+    it 'should call gravatar_icon when no User exists with the given email' do
+      expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20)
+
+      helper.avatar_icon('foo@example.com', 20)
     end
   end
 
   describe 'gravatar_icon' do
     let(:user_email) { 'user@email.com' }
 
-    it 'should return a generic avatar path when Gravatar is disabled' do
-      allow_any_instance_of(ApplicationSetting).to receive(:gravatar_enabled?).and_return(false)
-      expect(gravatar_icon(user_email)).to match('no_avatar.png')
-    end
+    context 'with Gravatar disabled' do
+      before do
+        stub_application_setting(gravatar_enabled?: false)
+      end
 
-    it 'should return a generic avatar path when email is blank' do
-      expect(gravatar_icon('')).to match('no_avatar.png')
+      it 'returns a generic avatar' do
+        expect(helper.gravatar_icon(user_email)).to match('no_avatar.png')
+      end
     end
 
-    it 'should return default gravatar url' do
-      allow(Gitlab.config.gitlab).to receive(:https).and_return(false)
-      url = 'http://www.gravatar.com/avatar/b58c6f14d292556214bd64909bcdb118'
-      expect(gravatar_icon(user_email)).to match(url)
-    end
+    context 'with Gravatar enabled' do
+      before do
+        stub_application_setting(gravatar_enabled?: true)
+      end
 
-    it 'should use SSL when appropriate' do
-      allow(Gitlab.config.gitlab).to receive(:https).and_return(true)
-      expect(gravatar_icon(user_email)).to match('https://secure.gravatar.com')
-    end
+      it 'returns a generic avatar when email is blank' do
+        expect(helper.gravatar_icon('')).to match('no_avatar.png')
+      end
 
-    it 'should return custom gravatar path when gravatar_url is set' do
-      allow(self).to receive(:request).and_return(double(:ssl? => false))
-      allow(Gitlab.config.gravatar).
-        to receive(:plain_url).
-        and_return('http://example.local/?s=%{size}&hash=%{hash}')
-      url = 'http://example.local/?s=20&hash=b58c6f14d292556214bd64909bcdb118'
-      expect(gravatar_icon(user_email, 20)).to eq(url)
-    end
+      it 'returns a valid Gravatar URL' do
+        stub_config_setting(https: false)
 
-    it 'should accept a custom size' do
-      allow(self).to receive(:request).and_return(double(:ssl? => false))
-      expect(gravatar_icon(user_email, 64)).to match(/\?s=64/)
-    end
+        expect(helper.gravatar_icon(user_email)).
+          to match('http://www.gravatar.com/avatar/b58c6f14d292556214bd64909bcdb118')
+      end
 
-    it 'should use default size when size is wrong' do
-      allow(self).to receive(:request).and_return(double(:ssl? => false))
-      expect(gravatar_icon(user_email, nil)).to match(/\?s=40/)
-    end
+      it 'uses HTTPs when configured' do
+        stub_config_setting(https: true)
+
+        expect(helper.gravatar_icon(user_email)).
+          to match('https://secure.gravatar.com')
+      end
+
+      it 'should return custom gravatar path when gravatar_url is set' do
+        stub_gravatar_setting(plain_url: 'http://example.local/?s=%{size}&hash=%{hash}')
 
-    it 'should be case insensitive' do
-      allow(self).to receive(:request).and_return(double(:ssl? => false))
-      expect(gravatar_icon(user_email)).
-        to eq(gravatar_icon(user_email.upcase + ' '))
+        expect(gravatar_icon(user_email, 20)).
+          to eq('http://example.local/?s=20&hash=b58c6f14d292556214bd64909bcdb118')
+      end
+
+      it 'accepts a custom size argument' do
+        expect(helper.gravatar_icon(user_email, 64)).to include '?s=64'
+      end
+
+      it 'defaults size to 40 when given an invalid size' do
+        expect(helper.gravatar_icon(user_email, nil)).to include '?s=40'
+      end
+
+      it 'ignores case and surrounding whitespace' do
+        normal = helper.gravatar_icon('foo@example.com')
+        upcase = helper.gravatar_icon(' FOO@EXAMPLE.COM ')
+
+        expect(normal).to eq upcase
+      end
     end
   end
 
   describe 'grouped_options_refs' do
-    # Override Rails' grouped_options_for_select helper since HTML is harder to work with
-    def grouped_options_for_select(options, *args)
-      options
-    end
-
-    let(:options) { grouped_options_refs }
+    let(:options) { helper.grouped_options_refs }
+    let(:project) { create(:project) }
 
     before do
-      # Must be an instance variable
-      @project = create(:project)
+      assign(:project, project)
+
+      # Override Rails' grouped_options_for_select helper to just return the
+      # first argument (`options`), since it's easier to work with than the
+      # generated HTML.
+      allow(helper).to receive(:grouped_options_for_select).
+        and_wrap_original { |_, *args| args.first }
     end
 
     it 'includes a list of branch names' do
@@ -167,15 +187,16 @@ describe ApplicationHelper do
 
     it 'includes a specific commit ref if defined' do
       # Must be an instance variable
-      @ref = '2ed06dc41dbb5936af845b87d79e05bbf24c73b8'
+      ref = '2ed06dc41dbb5936af845b87d79e05bbf24c73b8'
+      assign(:ref, ref)
 
       expect(options[2][0]).to eq('Commit')
-      expect(options[2][1]).to eq([@ref])
+      expect(options[2][1]).to eq([ref])
     end
 
     it 'sorts tags in a natural order' do
       # Stub repository.tag_names to make sure we get some valid testing data
-      expect(@project.repository).to receive(:tag_names).
+      expect(project.repository).to receive(:tag_names).
         and_return(['v1.0.9', 'v1.0.10', 'v2.0', 'v3.1.4.2', 'v2.0rc1¿',
                     'v1.0.9a', 'v2.0-rc1', 'v2.0rc2'])
 
@@ -189,17 +210,17 @@ describe ApplicationHelper do
     let(:a_tag) { '<a href="#">Foo</a>' }
 
     it 'allows the a tag' do
-      expect(simple_sanitize(a_tag)).to eq(a_tag)
+      expect(helper.simple_sanitize(a_tag)).to eq(a_tag)
     end
 
     it 'allows the span tag' do
       input = '<span class="foo">Bar</span>'
-      expect(simple_sanitize(input)).to eq(input)
+      expect(helper.simple_sanitize(input)).to eq(input)
     end
 
     it 'disallows other tags' do
       input = "<strike><b>#{a_tag}</b></strike>"
-      expect(simple_sanitize(input)).to eq(a_tag)
+      expect(helper.simple_sanitize(input)).to eq(a_tag)
     end
   end
 
@@ -207,7 +228,7 @@ describe ApplicationHelper do
     def element(*arguments)
       Time.zone = 'UTC'
       time = Time.zone.parse('2015-07-02 08:00')
-      element = time_ago_with_tooltip(time, *arguments)
+      element = helper.time_ago_with_tooltip(time, *arguments)
 
       Nokogiri::HTML::DocumentFragment.parse(element).first_element_child
     end
@@ -257,21 +278,21 @@ describe ApplicationHelper do
 
     it 'should preserve encoding' do
       expect(content.encoding.name).to eq('UTF-8')
-      expect(render_markup('foo.rst', content).encoding.name).to eq('UTF-8')
+      expect(helper.render_markup('foo.rst', content).encoding.name).to eq('UTF-8')
     end
 
     it "should delegate to #markdown when file name corresponds to Markdown" do
-      expect(self).to receive(:gitlab_markdown?).with('foo.md').and_return(true)
-      expect(self).to receive(:markdown).and_return('NOEL')
+      expect(helper).to receive(:gitlab_markdown?).with('foo.md').and_return(true)
+      expect(helper).to receive(:markdown).and_return('NOEL')
 
-      expect(render_markup('foo.md', content)).to eq('NOEL')
+      expect(helper.render_markup('foo.md', content)).to eq('NOEL')
     end
 
     it "should delegate to #asciidoc when file name corresponds to AsciiDoc" do
-      expect(self).to receive(:asciidoc?).with('foo.adoc').and_return(true)
-      expect(self).to receive(:asciidoc).and_return('NOEL')
+      expect(helper).to receive(:asciidoc?).with('foo.adoc').and_return(true)
+      expect(helper).to receive(:asciidoc).and_return('NOEL')
 
-      expect(render_markup('foo.adoc', content)).to eq('NOEL')
+      expect(helper.render_markup('foo.adoc', content)).to eq('NOEL')
     end
   end
 end

spec/spec_helper.rb

@@ -13,12 +13,12 @@ RSpec.configure do |config|
   config.use_instantiated_fixtures  = false
   config.mock_with :rspec
 
-  config.include LoginHelpers, type: :feature
-  config.include LoginHelpers, type: :request
-  config.include FactoryGirl::Syntax::Methods
   config.include Devise::TestHelpers, type: :controller
-
+  config.include LoginHelpers,        type: :feature
+  config.include LoginHelpers,        type: :request
+  config.include StubConfiguration
   config.include TestEnv
+
   config.infer_spec_type_from_file_location!
   config.raise_errors_for_deprecations!
 

spec/support/factory_girl.rb

+RSpec.configure do |config|
+  config.include FactoryGirl::Syntax::Methods
+end

spec/support/stub_configuration.rb

+module StubConfiguration
+  def stub_application_setting(messages)
+    allow(Gitlab::CurrentSettings.current_application_settings).
+      to receive_messages(messages)
+  end
+
+  def stub_config_setting(messages)
+    allow(Gitlab.config.gitlab).to receive_messages(messages)
+  end
+
+  def stub_gravatar_setting(messages)
+    allow(Gitlab.config.gravatar).to receive_messages(messages)
+  end
+end