Merge branch 'pipeline-hooks-without-slack' into wall-clock-time-for-showing-pipeline

* pipeline-hooks-without-slack: (156 commits)
  Fix test failures
  Make pipeline to be in created state for hooks tests
  Make `execute_methods` public
  Added specs for started_at and finished_at
  Use explicit events to transition between states
  Fix tests. We cannot reload unless it's already saved:
  Have trait all_events_enabled so that's easier to reuse, feedback:
  Simplify the name for data builder, feedback:
  Prefer extend self over module_function, feedback:
  Make it more grammatically correct, feedback:
  if -> when; when -> `when`; %w() -> %w[]; and fix some typos:
  Prefer described_class, feedback:
  Make the comment more clear, feedback:
  Update CHANGELOG
  render only commit title
  Fix test failures, that did occur because of missing previously used `reload_status!` call
  Use state machine for pipeline event processing
  Upgrade Rails to 4.2.7.1 for security fixes.
  Update gitlab-shell to v3.3.3
  Verify the pipeline status after executing events on builds
  ...

14 files changed, 77 insertions, 118 deletions

diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 3e15a137e33..73f5470cf35 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -54,7 +54,7 @@ describe ApplicationHelper do
   describe 'project_icon' do
     let(:avatar_file_path) { File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') }
 
-    it 'should return an url for the avatar' do
+    it 'returns an url for the avatar' do
       project = create(:project, avatar: File.open(avatar_file_path))
 
       avatar_url = "http://localhost/uploads/project/avatar/#{project.id}/banana_sample.gif"
@@ -62,7 +62,7 @@ describe ApplicationHelper do
         to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />"
     end
 
-    it 'should give uploaded icon when present' do
+    it 'gives uploaded icon when present' do
       project = create(:project)
 
       allow_any_instance_of(Project).to receive(:avatar_in_git).and_return(true)
@@ -76,14 +76,14 @@ describe ApplicationHelper do
   describe 'avatar_icon' do
     let(:avatar_file_path) { File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') }
 
-    it 'should return an url for the avatar' do
+    it 'returns an url for the avatar' do
       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
+    it 'returns an url for the avatar with relative url' do
       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))
@@ -94,14 +94,14 @@ describe ApplicationHelper do
         to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif")
     end
 
-    it 'should call gravatar_icon when no User exists with the given email' do
+    it 'calls gravatar_icon when no User exists with the given email' do
       expect(helper).to receive(:gravatar_icon).with('foo@example.com', 20, 2)
 
       helper.avatar_icon('foo@example.com', 20, 2)
     end
 
     describe 'using a User' do
-      it 'should return an URL for the avatar' do
+      it 'returns an URL for the avatar' do
         user = create(:user, avatar: File.open(avatar_file_path))
 
         expect(helper.avatar_icon(user).to_s).
@@ -146,7 +146,7 @@ describe ApplicationHelper do
           to match('https://secure.gravatar.com')
       end
 
-      it 'should return custom gravatar path when gravatar_url is set' do
+      it 'returns custom gravatar path when gravatar_url is set' do
         stub_gravatar_setting(plain_url: 'http://example.local/?s=%{size}&hash=%{hash}')
 
         expect(gravatar_icon(user_email, 20)).
@@ -266,19 +266,19 @@ describe ApplicationHelper do
       allow(helper).to receive(:current_user).and_return(user)
     end
 
-    it 'should preserve encoding' do
+    it 'preserves encoding' do
       expect(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
+    it "delegates to #markdown when file name corresponds to Markdown" do
       expect(helper).to receive(:gitlab_markdown?).with('foo.md').and_return(true)
       expect(helper).to receive(:markdown).and_return('NOEL')
 
       expect(helper.render_markup('foo.md', content)).to eq('NOEL')
     end
 
-    it "should delegate to #asciidoc when file name corresponds to AsciiDoc" do
+    it "delegates to #asciidoc when file name corresponds to AsciiDoc" do
       expect(helper).to receive(:asciidoc?).with('foo.adoc').and_return(true)
       expect(helper).to receive(:asciidoc).and_return('NOEL')
 
diff --git a/spec/helpers/blob_helper_spec.rb b/spec/helpers/blob_helper_spec.rb
index b2d6d59b1ee..94972eed945 100644
--- a/spec/helpers/blob_helper_spec.rb
+++ b/spec/helpers/blob_helper_spec.rb
@@ -17,19 +17,19 @@ describe BlobHelper do
   end
 
   describe '#highlight' do
-    it 'should return plaintext for unknown lexer context' do
+    it 'returns plaintext for unknown lexer context' do
       result = helper.highlight(blob_name, no_context_content)
       expect(result).to eq(%[<pre class="code highlight"><code><span id="LC1" class="line">:type "assem"))</span></code></pre>])
     end
 
-    it 'should highlight single block' do
+    it 'highlights single block' do
       expected = %Q[<pre class="code highlight"><code><span id="LC1" class="line"><span class="p">(</span><span class="nb">make-pathname</span> <span class="ss">:defaults</span> <span class="nv">name</span></span>
 <span id="LC2" class="line"><span class="ss">:type</span> <span class="s">"assem"</span><span class="p">))</span></span></code></pre>]
 
       expect(helper.highlight(blob_name, blob_content)).to eq(expected)
     end
 
-    it 'should highlight multi-line comments' do
+    it 'highlights multi-line comments' do
       result = helper.highlight(blob_name, multiline_content)
       html = Nokogiri::HTML(result)
       lines = html.search('.s')
@@ -49,7 +49,7 @@ describe BlobHelper do
 <span id="LC4" class="line"> ddd</span></code></pre>)
       end
 
-      it 'should highlight each line properly' do
+      it 'highlights each line properly' do
         result = helper.highlight(blob_name, blob_content)
         expect(result).to eq(expected)
       end
@@ -62,7 +62,7 @@ describe BlobHelper do
     let(:expected_svg_path) { File.join(Rails.root, 'spec', 'fixtures', 'sanitized.svg') }
     let(:expected) { open(expected_svg_path).read }
 
-    it 'should retain essential elements' do
+    it 'retains essential elements' do
       blob = OpenStruct.new(data: data)
       expect(sanitize_svg(blob).data).to eq(expected)
     end
diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb
index b6554de1c64..9c7c79f57c6 100644
--- a/spec/helpers/diff_helper_spec.rb
+++ b/spec/helpers/diff_helper_spec.rb
@@ -32,27 +32,27 @@ describe DiffHelper do
   end
 
   describe 'diff_options' do
-    it 'should return no collapse false' do
+    it 'returns no collapse false' do
       expect(diff_options).to include(no_collapse: false)
     end
 
-    it 'should return no collapse true if expand_all_diffs' do
+    it 'returns no collapse true if expand_all_diffs' do
       allow(controller).to receive(:params) { { expand_all_diffs: true } }
       expect(diff_options).to include(no_collapse: true)
     end
 
-    it 'should return no collapse true if action name diff_for_path' do
+    it 'returns no collapse true if action name diff_for_path' do
       allow(controller).to receive(:action_name) { 'diff_for_path' }
       expect(diff_options).to include(no_collapse: true)
     end
 
-    it 'should return paths if action name diff_for_path and param old path' do
+    it 'returns paths if action name diff_for_path and param old path' do
       allow(controller).to receive(:params) { { old_path: 'lib/wadus.rb' } }
       allow(controller).to receive(:action_name) { 'diff_for_path' }
       expect(diff_options[:paths]).to include('lib/wadus.rb')
     end
 
-    it 'should return paths if action name diff_for_path and param new path' do
+    it 'returns paths if action name diff_for_path and param new path' do
       allow(controller).to receive(:params) { { new_path: 'lib/wadus.rb' } }
       allow(controller).to receive(:action_name) { 'diff_for_path' }
       expect(diff_options[:paths]).to include('lib/wadus.rb')
@@ -60,11 +60,11 @@ describe DiffHelper do
   end
 
   describe '#diff_line_content' do
-    it 'should return non breaking space when line is empty' do
+    it 'returns non breaking space when line is empty' do
       expect(diff_line_content(nil)).to eq(' &nbsp;')
     end
 
-    it 'should return the line itself' do
+    it 'returns the line itself' do
       expect(diff_line_content(diff_file.diff_lines.first.text)).
         to eq('@@ -6,12 +6,18 @@ module Popen')
       expect(diff_line_content(diff_file.diff_lines.first.type)).to eq('match')
diff --git a/spec/helpers/emails_helper_spec.rb b/spec/helpers/emails_helper_spec.rb
index 7a3e38d7e63..3223556e1d3 100644
--- a/spec/helpers/emails_helper_spec.rb
+++ b/spec/helpers/emails_helper_spec.rb
@@ -8,37 +8,37 @@ describe EmailsHelper do
     end
 
     context 'when time limit is less than 2 hours' do
-      it 'should display the time in hours using a singular unit' do
+      it 'displays the time in hours using a singular unit' do
         validate_time_string(1.hour, '1 hour')
       end
     end
 
     context 'when time limit is 2 or more hours' do
-      it 'should display the time in hours using a plural unit' do
+      it 'displays the time in hours using a plural unit' do
         validate_time_string(2.hours, '2 hours')
       end
     end
 
     context 'when time limit contains fractions of an hour' do
-      it 'should round down to the nearest hour' do
+      it 'rounds down to the nearest hour' do
         validate_time_string(96.minutes, '1 hour')
       end
     end
 
     context 'when time limit is 24 or more hours' do
-      it 'should display the time in days using a singular unit' do
+      it 'displays the time in days using a singular unit' do
         validate_time_string(24.hours, '1 day')
       end
     end
 
     context 'when time limit is 2 or more days' do
-      it 'should display the time in days using a plural unit' do
+      it 'displays the time in days using a plural unit' do
         validate_time_string(2.days, '2 days')
       end
     end
 
     context 'when time limit contains fractions of a day' do
-      it 'should round down to the nearest day' do
+      it 'rounds down to the nearest day' do
         validate_time_string(57.hours, '2 days')
       end
     end
diff --git a/spec/helpers/events_helper_spec.rb b/spec/helpers/events_helper_spec.rb
index 6b5e3d93d48..022aba0c0d0 100644
--- a/spec/helpers/events_helper_spec.rb
+++ b/spec/helpers/events_helper_spec.rb
@@ -6,34 +6,34 @@ describe EventsHelper do
       allow(helper).to receive(:current_user).and_return(double)
     end
 
-    it 'should display one line of plain text without alteration' do
+    it 'displays one line of plain text without alteration' do
       input = 'A short, plain note'
       expect(helper.event_note(input)).to match(input)
       expect(helper.event_note(input)).not_to match(/\.\.\.\z/)
     end
 
-    it 'should display inline code' do
+    it 'displays inline code' do
       input = 'A note with `inline code`'
       expected = 'A note with <code>inline code</code>'
 
       expect(helper.event_note(input)).to match(expected)
     end
 
-    it 'should truncate a note with multiple paragraphs' do
+    it 'truncates a note with multiple paragraphs' do
       input = "Paragraph 1\n\nParagraph 2"
       expected = 'Paragraph 1...'
 
       expect(helper.event_note(input)).to match(expected)
     end
 
-    it 'should display the first line of a code block' do
+    it 'displays the first line of a code block' do
       input = "```\nCode block\nwith two lines\n```"
       expected = %r{<pre.+><code>Code block\.\.\.</code></pre>}
 
       expect(helper.event_note(input)).to match(expected)
     end
 
-    it 'should truncate a single long line of text' do
+    it 'truncates a single long line of text' do
       text = 'The quick brown fox jumped over the lazy dog twice' # 50 chars
       input = text * 4
       expected = (text * 2).sub(/.{3}/, '...')
@@ -41,7 +41,7 @@ describe EventsHelper do
       expect(helper.event_note(input)).to match(expected)
     end
 
-    it 'should preserve a link href when link text is truncated' do
+    it 'preserves a link href when link text is truncated' do
       text = 'The quick brown fox jumped over the lazy dog' # 44 chars
       input = "#{text}#{text}#{text} " # 133 chars
       link_url = 'http://example.com/foo/bar/baz' # 30 chars
@@ -52,7 +52,7 @@ describe EventsHelper do
       expect(helper.event_note(input)).to match(expected_link_text)
     end
 
-    it 'should preserve code color scheme' do
+    it 'preserves code color scheme' do
       input = "```ruby\ndef test\n  'hello world'\nend\n```"
       expected = '<pre class="code highlight js-syntax-highlight ruby">' \
         "<code><span class=\"k\">def</span> <span class=\"nf\">test</span>\n" \
diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb
index ade5c3b02d9..5368e5fab06 100644
--- a/spec/helpers/gitlab_markdown_helper_spec.rb
+++ b/spec/helpers/gitlab_markdown_helper_spec.rb
@@ -26,17 +26,17 @@ describe GitlabMarkdownHelper do
     describe "referencing multiple objects" do
       let(:actual) { "#{merge_request.to_reference} -> #{commit.to_reference} -> #{issue.to_reference}" }
 
-      it "should link to the merge request" do
+      it "links to the merge request" do
         expected = namespace_project_merge_request_path(project.namespace, project, merge_request)
         expect(helper.markdown(actual)).to match(expected)
       end
 
-      it "should link to the commit" do
+      it "links to the commit" do
         expected = namespace_project_commit_path(project.namespace, project, commit)
         expect(helper.markdown(actual)).to match(expected)
       end
 
-      it "should link to the issue" do
+      it "links to the issue" do
         expected = namespace_project_issue_path(project.namespace, project, issue)
         expect(helper.markdown(actual)).to match(expected)
       end
@@ -47,7 +47,7 @@ describe GitlabMarkdownHelper do
       let(:second_project) { create(:project, :public) }
       let(:second_issue) { create(:issue, project: second_project) }
 
-      it 'should link to the issue' do
+      it 'links to the issue' do
         expected = namespace_project_issue_path(second_project.namespace, second_project, second_issue)
         expect(markdown(actual, project: second_project)).to match(expected)
       end
@@ -58,7 +58,7 @@ describe GitlabMarkdownHelper do
     let(:commit_path) { namespace_project_commit_path(project.namespace, project, commit) }
     let(:issues)      { create_list(:issue, 2, project: project) }
 
-    it 'should handle references nested in links with all the text' do
+    it 'handles references nested in links with all the text' do
       actual = helper.link_to_gfm("This should finally fix #{issues[0].to_reference} and #{issues[1].to_reference} for real", commit_path)
       doc = Nokogiri::HTML.parse(actual)
 
@@ -88,7 +88,7 @@ describe GitlabMarkdownHelper do
       expect(doc.css('a')[4].text).to eq ' for real'
     end
 
-    it 'should forward HTML options' do
+    it 'forwards HTML options' do
       actual = helper.link_to_gfm("Fixed in #{commit.id}", commit_path, class: 'foo')
       doc = Nokogiri::HTML.parse(actual)
 
@@ -110,7 +110,7 @@ describe GitlabMarkdownHelper do
       expect(act).to eq %Q(<a href="/foo">#{issues[0].to_reference}</a>)
     end
 
-    it 'should replace commit message with emoji to link' do
+    it 'replaces commit message with emoji to link' do
       actual = link_to_gfm(':book:Book', '/foo')
       expect(actual).
         to eq %Q(<img class="emoji" title=":book:" alt=":book:" src="http://localhost/assets/1F4D6.png" height="20" width="20" align="absmiddle"><a href="/foo">Book</a>)
@@ -125,7 +125,7 @@ describe GitlabMarkdownHelper do
       helper.instance_variable_set(:@project_wiki, @wiki)
     end
 
-    it "should use Wiki pipeline for markdown files" do
+    it "uses Wiki pipeline for markdown files" do
       allow(@wiki).to receive(:format).and_return(:markdown)
 
       expect(helper).to receive(:markdown).with('wiki content', pipeline: :wiki, project_wiki: @wiki, page_slug: "nested/page")
@@ -133,7 +133,7 @@ describe GitlabMarkdownHelper do
       helper.render_wiki_content(@wiki)
     end
 
-    it "should use Asciidoctor for asciidoc files" do
+    it "uses Asciidoctor for asciidoc files" do
       allow(@wiki).to receive(:format).and_return(:asciidoc)
 
       expect(helper).to receive(:asciidoc).with('wiki content')
@@ -141,7 +141,7 @@ describe GitlabMarkdownHelper do
       helper.render_wiki_content(@wiki)
     end
 
-    it "should use the Gollum renderer for all other file types" do
+    it "uses the Gollum renderer for all other file types" do
       allow(@wiki).to receive(:format).and_return(:rdoc)
       formatted_content_stub = double('formatted_content')
       expect(formatted_content_stub).to receive(:html_safe)
diff --git a/spec/helpers/graph_helper_spec.rb b/spec/helpers/graph_helper_spec.rb
index 4acf38771b7..51c49f0e587 100644
--- a/spec/helpers/graph_helper_spec.rb
+++ b/spec/helpers/graph_helper_spec.rb
@@ -6,7 +6,7 @@ describe GraphHelper do
     let(:commit)  { project.commit("master") }
     let(:graph) { Network::Graph.new(project, 'master', commit, '') }
 
-    it 'filter our refs used by GitLab' do
+    it 'filters our refs used by GitLab' do
       allow(commit).to receive(:ref_names).and_return(['refs/merge-requests/abc', 'master', 'refs/tmp/xyz'])
       self.instance_variable_set(:@graph, graph)
       refs = get_refs(project.repository, commit)
diff --git a/spec/helpers/groups_helper_spec.rb b/spec/helpers/groups_helper_spec.rb
index 4ea90a80a92..0807534720a 100644
--- a/spec/helpers/groups_helper_spec.rb
+++ b/spec/helpers/groups_helper_spec.rb
@@ -4,7 +4,7 @@ describe GroupsHelper do
   describe 'group_icon' do
     avatar_file_path = File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif')
 
-    it 'should return an url for the avatar' do
+    it 'returns an url for the avatar' do
       group = create(:group)
       group.avatar = File.open(avatar_file_path)
       group.save!
@@ -12,7 +12,7 @@ describe GroupsHelper do
         to match("/uploads/group/avatar/#{group.id}/banana_sample.gif")
     end
 
-    it 'should give default avatar_icon when no avatar is present' do
+    it 'gives default avatar_icon when no avatar is present' do
       group = create(:group)
       group.save!
       expect(group_icon(group.path)).to match('group_avatar.png')
diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb
index 9ee46dd2508..5e4655dfc95 100644
--- a/spec/helpers/issues_helper_spec.rb
+++ b/spec/helpers/issues_helper_spec.rb
@@ -10,18 +10,19 @@ describe IssuesHelper do
     let(:ext_expected) { issues_url.gsub(':id', issue.iid.to_s).gsub(':project_id', ext_project.id.to_s) }
     let(:int_expected) { polymorphic_path([@project.namespace, project, issue]) }
 
-    it "should return internal path if used internal tracker" do
+    it "returns internal path if used internal tracker" do
       @project = project
+
       expect(url_for_issue(issue.iid)).to match(int_expected)
     end
 
-    it "should return path to external tracker" do
+    it "returns path to external tracker" do
       @project = ext_project
 
       expect(url_for_issue(issue.iid)).to match(ext_expected)
     end
 
-    it "should return empty string if project nil" do
+    it "returns empty string if project nil" do
       @project = nil
 
       expect(url_for_issue(issue.iid)).to eq ""
@@ -45,7 +46,7 @@ describe IssuesHelper do
         allow(Gitlab.config).to receive(:issues_tracker).and_return(nil)
       end
 
-      it "should return external path" do
+      it "returns external path" do
         expect(url_for_issue(issue.iid)).to match(ext_expected)
       end
     end
diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb
index f75fdb739f6..7998209b7b0 100644
--- a/spec/helpers/members_helper_spec.rb
+++ b/spec/helpers/members_helper_spec.rb
@@ -9,54 +9,6 @@ describe MembersHelper do
     it { expect(action_member_permission(:admin, group_member)).to eq :admin_group_member }
   end
 
-  describe '#default_show_roles' do
-    let(:user) { double }
-    let(:member) { build(:project_member) }
-
-    before do
-      allow(helper).to receive(:current_user).and_return(user)
-      allow(helper).to receive(:can?).with(user, :update_project_member, member).and_return(false)
-      allow(helper).to receive(:can?).with(user, :destroy_project_member, member).and_return(false)
-      allow(helper).to receive(:can?).with(user, :admin_project_member, member.source).and_return(false)
-    end
-
-    context 'when the current cannot update, destroy or admin the passed member' do
-      it 'returns false' do
-        expect(helper.default_show_roles(member)).to be_falsy
-      end
-    end
-
-    context 'when the current can update the passed member' do
-      before do
-        allow(helper).to receive(:can?).with(user, :update_project_member, member).and_return(true)
-      end
-
-      it 'returns true' do
-        expect(helper.default_show_roles(member)).to be_truthy
-      end
-    end
-
-    context 'when the current can destroy the passed member' do
-      before do
-        allow(helper).to receive(:can?).with(user, :destroy_project_member, member).and_return(true)
-      end
-
-      it 'returns true' do
-        expect(helper.default_show_roles(member)).to be_truthy
-      end
-    end
-
-    context 'when the current can admin the passed member source' do
-      before do
-        allow(helper).to receive(:can?).with(user, :admin_project_member, member.source).and_return(true)
-      end
-
-      it 'returns true' do
-        expect(helper.default_show_roles(member)).to be_truthy
-      end
-    end
-  end
-
   describe '#remove_member_message' do
     let(:requester) { build(:user) }
     let(:project) { create(:project) }
diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb
index af371248ae9..853b8b2f7f7 100644
--- a/spec/helpers/notes_helper_spec.rb
+++ b/spec/helpers/notes_helper_spec.rb
@@ -21,7 +21,7 @@ describe NotesHelper do
   end
 
   describe "#notes_max_access_for_users" do
-    it 'return human access levels' do
+    it 'returns human access levels' do
       expect(helper.note_max_access_for_user(owner_note)).to eq('Owner')
       expect(helper.note_max_access_for_user(master_note)).to eq('Master')
       expect(helper.note_max_access_for_user(reporter_note)).to eq('Reporter')
@@ -38,6 +38,12 @@ describe NotesHelper do
   end
 
   describe '#preload_max_access_for_authors' do
+    before do
+      # #preload_max_access_for_authors would read cache from RequestStore,
+      # so we should make sure it's clean.
+      RequestStore.clear!
+    end
+
     it 'loads multiple users' do
       expected_access = {
         owner.id => Gitlab::Access::OWNER,
diff --git a/spec/helpers/search_helper_spec.rb b/spec/helpers/search_helper_spec.rb
index 601b6915e27..b0bb991539b 100644
--- a/spec/helpers/search_helper_spec.rb
+++ b/spec/helpers/search_helper_spec.rb
@@ -42,7 +42,7 @@ describe SearchHelper do
         expect(search_autocomplete_opts(project.name).size).to eq(1)
       end
 
-      it "should not include the public group" do
+      it "does not include the public group" do
         group = create(:group)
         expect(search_autocomplete_opts(group.name).size).to eq(0)
       end
diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb
index 10121759132..37ac6a2699d 100644
--- a/spec/helpers/submodule_helper_spec.rb
+++ b/spec/helpers/submodule_helper_spec.rb
@@ -17,35 +17,35 @@ describe SubmoduleHelper do
         allow(Gitlab.config.gitlab).to receive(:protocol).and_return('http') # set this just to be sure
       end
 
-      it 'should detect ssh on standard port' do
+      it 'detects ssh on standard port' do
         allow(Gitlab.config.gitlab_shell).to receive(:ssh_port).and_return(22) # set this just to be sure
         allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix))
         stub_url([ config.user, '@', config.host, ':gitlab-org/gitlab-ce.git' ].join(''))
         expect(submodule_links(submodule_item)).to eq([ namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash') ])
       end
 
-      it 'should detect ssh on non-standard port' do
+      it 'detects ssh on non-standard port' do
         allow(Gitlab.config.gitlab_shell).to receive(:ssh_port).and_return(2222)
         allow(Gitlab.config.gitlab_shell).to receive(:ssh_path_prefix).and_return(Settings.send(:build_gitlab_shell_ssh_path_prefix))
         stub_url([ 'ssh://', config.user, '@', config.host, ':2222/gitlab-org/gitlab-ce.git' ].join(''))
         expect(submodule_links(submodule_item)).to eq([ namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash') ])
       end
 
-      it 'should detect http on standard port' do
+      it 'detects http on standard port' do
         allow(Gitlab.config.gitlab).to receive(:port).and_return(80)
         allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
         stub_url([ 'http://', config.host, '/gitlab-org/gitlab-ce.git' ].join(''))
         expect(submodule_links(submodule_item)).to eq([ namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash') ])
       end
 
-      it 'should detect http on non-standard port' do
+      it 'detects http on non-standard port' do
         allow(Gitlab.config.gitlab).to receive(:port).and_return(3000)
         allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
         stub_url([ 'http://', config.host, ':3000/gitlab-org/gitlab-ce.git' ].join(''))
         expect(submodule_links(submodule_item)).to eq([ namespace_project_path('gitlab-org', 'gitlab-ce'), namespace_project_tree_path('gitlab-org', 'gitlab-ce', 'hash') ])
       end
 
-      it 'should work with relative_url_root' do
+      it 'works with relative_url_root' do
         allow(Gitlab.config.gitlab).to receive(:port).and_return(80) # set this just to be sure
         allow(Gitlab.config.gitlab).to receive(:relative_url_root).and_return('/gitlab/root')
         allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
@@ -55,22 +55,22 @@ describe SubmoduleHelper do
     end
 
     context 'submodule on github.com' do
-      it 'should detect ssh' do
+      it 'detects ssh' do
         stub_url('git@github.com:gitlab-org/gitlab-ce.git')
         expect(submodule_links(submodule_item)).to eq([ 'https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash' ])
       end
 
-      it 'should detect http' do
+      it 'detects http' do
         stub_url('http://github.com/gitlab-org/gitlab-ce.git')
         expect(submodule_links(submodule_item)).to eq([ 'https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash' ])
       end
 
-      it 'should detect https' do
+      it 'detects https' do
         stub_url('https://github.com/gitlab-org/gitlab-ce.git')
         expect(submodule_links(submodule_item)).to eq([ 'https://github.com/gitlab-org/gitlab-ce', 'https://github.com/gitlab-org/gitlab-ce/tree/hash' ])
       end
 
-      it 'should return original with non-standard url' do
+      it 'returns original with non-standard url' do
         stub_url('http://github.com/gitlab-org/gitlab-ce')
         expect(submodule_links(submodule_item)).to eq([ repo.submodule_url_for, nil ])
 
@@ -80,22 +80,22 @@ describe SubmoduleHelper do
     end
 
     context 'submodule on gitlab.com' do
-      it 'should detect ssh' do
+      it 'detects ssh' do
         stub_url('git@gitlab.com:gitlab-org/gitlab-ce.git')
         expect(submodule_links(submodule_item)).to eq([ 'https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash' ])
       end
 
-      it 'should detect http' do
+      it 'detects http' do
         stub_url('http://gitlab.com/gitlab-org/gitlab-ce.git')
         expect(submodule_links(submodule_item)).to eq([ 'https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash' ])
       end
 
-      it 'should detect https' do
+      it 'detects https' do
         stub_url('https://gitlab.com/gitlab-org/gitlab-ce.git')
         expect(submodule_links(submodule_item)).to eq([ 'https://gitlab.com/gitlab-org/gitlab-ce', 'https://gitlab.com/gitlab-org/gitlab-ce/tree/hash' ])
       end
 
-      it 'should return original with non-standard url' do
+      it 'returns original with non-standard url' do
         stub_url('http://gitlab.com/gitlab-org/gitlab-ce')
         expect(submodule_links(submodule_item)).to eq([ repo.submodule_url_for, nil ])
 
@@ -105,7 +105,7 @@ describe SubmoduleHelper do
     end
 
     context 'submodule on unsupported' do
-      it 'should return original' do
+      it 'returns original' do
         stub_url('http://mygitserver.com/gitlab-org/gitlab-ce')
         expect(submodule_links(submodule_item)).to eq([ repo.submodule_url_for, nil ])
 
diff --git a/spec/helpers/tree_helper_spec.rb b/spec/helpers/tree_helper_spec.rb
index c70dd8076e0..8d6537ba4b5 100644
--- a/spec/helpers/tree_helper_spec.rb
+++ b/spec/helpers/tree_helper_spec.rb
@@ -12,7 +12,7 @@ describe TreeHelper do
     context "on a directory containing more than one file/directory" do
       let(:tree_item) { double(name: "files", path: "files") }
 
-      it "should return the directory name" do
+      it "returns the directory name" do
         expect(flatten_tree(tree_item)).to match('files')
       end
     end
@@ -20,7 +20,7 @@ describe TreeHelper do
     context "on a directory containing only one directory" do
       let(:tree_item) { double(name: "foo", path: "foo") }
 
-      it "should return the flattened path" do
+      it "returns the flattened path" do
         expect(flatten_tree(tree_item)).to match('foo/bar')
       end
     end