Merge branch 'rs-rubocop-rspec' into 'master'

Enable Style/RedundantException, Lint/EmptyWhen, and RSpec/ExpectActual cops

See merge request !9151

6 files changed, 67 insertions, 74 deletions

diff --git a/.rubocop.yml b/.rubocop.yml
index d30846e6e0b..b093d4d25d4 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -339,6 +339,10 @@ Style/OpMethod:
 Style/ParenthesesAroundCondition:
   Enabled: true
 
+# Checks for an obsolete RuntimeException argument in raise/fail.
+Style/RedundantException:
+  Enabled: true
+
 # Checks for parentheses that seem not to serve any purpose.
 Style/RedundantParentheses:
   Enabled: true
@@ -568,6 +572,10 @@ Lint/ElseLayout:
 Lint/EmptyEnsure:
   Enabled: true
 
+# Checks for the presence of `when` branches without a body.
+Lint/EmptyWhen:
+  Enabled: true
+
 # Align ends correctly.
 Lint/EndAlignment:
   Enabled: true
@@ -769,6 +777,10 @@ Rails/ScopeArgs:
 RSpec/AnyInstance:
   Enabled: false
 
+# Check for expectations where `be(...)` can replace `eql(...)`.
+RSpec/BeEql:
+  Enabled: false
+
 # Check that the first argument to the top level describe is the tested class or
 # module.
 RSpec/DescribeClass:
@@ -797,6 +809,10 @@ RSpec/ExampleWording:
     not: does not
   IgnoredWords: []
 
+# Checks for `expect(...)` calls containing literal values.
+RSpec/ExpectActual:
+  Enabled: true
+
 # Checks the file and folder naming of the spec file.
 RSpec/FilePath:
   Enabled: false
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index d581610162f..648b3fc49d2 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -21,10 +21,6 @@ Lint/AmbiguousRegexpLiteral:
 Lint/AssignmentInCondition:
   Enabled: false
 
-# Offense count: 1
-Lint/EmptyWhen:
-  Enabled: false
-
 # Offense count: 20
 Lint/HandleExceptions:
   Enabled: false
@@ -80,19 +76,11 @@ Performance/RedundantMatch:
 Performance/RedundantMerge:
   Enabled: false
 
-# Offense count: 7
-RSpec/BeEql:
-  Enabled: false
-
 # Offense count: 15
 # Configuration parameters: CustomIncludeMethods.
 RSpec/EmptyExampleGroup:
   Enabled: false
 
-# Offense count: 24
-RSpec/ExpectActual:
-  Enabled: false
-
 # Offense count: 58
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: implicit, each, example
@@ -424,11 +412,6 @@ Style/RaiseArgs:
 Style/RedundantBegin:
   Enabled: false
 
-# Offense count: 1
-# Cop supports --auto-correct.
-Style/RedundantException:
-  Enabled: false
-
 # Offense count: 29
 # Cop supports --auto-correct.
 Style/RedundantFreeze:
diff --git a/app/helpers/preferences_helper.rb b/app/helpers/preferences_helper.rb
index 6e68aad4cb7..dd0a4ea03f0 100644
--- a/app/helpers/preferences_helper.rb
+++ b/app/helpers/preferences_helper.rb
@@ -23,7 +23,7 @@ module PreferencesHelper
 
     if defined.size != DASHBOARD_CHOICES.size
       # Ensure that anyone adding new options updates this method too
-      raise RuntimeError, "`User` defines #{defined.size} dashboard choices," +
+      raise "`User` defines #{defined.size} dashboard choices," \
         " but `DASHBOARD_CHOICES` defined #{DASHBOARD_CHOICES.size}."
     else
       defined.map do |key, _|
diff --git a/lib/gitlab/diff/parser.rb b/lib/gitlab/diff/parser.rb
index 59a2367b65d..89320f5d9dc 100644
--- a/lib/gitlab/diff/parser.rb
+++ b/lib/gitlab/diff/parser.rb
@@ -45,7 +45,7 @@ module Gitlab
               line_new += 1
             when "-"
               line_old += 1
-            when "\\"
+            when "\\" # rubocop:disable Lint/EmptyWhen
               # No increment
             else
               line_new += 1
diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb
index 6f7046c8461..64f448a83b7 100644
--- a/spec/features/issues/filtered_search/filter_issues_spec.rb
+++ b/spec/features/issues/filtered_search/filter_issues_spec.rb
@@ -113,13 +113,11 @@ describe 'Filter issues', js: true, feature: true do
       end
 
       it 'filters issues by invalid author' do
-        pending('to be tested, issue #26546')
-        expect(true).to be(false)
+        skip('to be tested, issue #26546')
       end
 
       it 'filters issues by multiple authors' do
-        pending('to be tested, issue #26546')
-        expect(true).to be(false)
+        skip('to be tested, issue #26546')
       end
     end
 
@@ -158,8 +156,7 @@ describe 'Filter issues', js: true, feature: true do
     end
 
     it 'sorting' do
-      pending('to be tested, issue #26546')
-      expect(true).to be(false)
+      skip('to be tested, issue #26546')
     end
   end
 
@@ -182,13 +179,11 @@ describe 'Filter issues', js: true, feature: true do
       end
 
       it 'filters issues by invalid assignee' do
-        pending('to be tested, issue #26546')
-        expect(true).to be(false)
+        skip('to be tested, issue #26546')
       end
 
       it 'filters issues by multiple assignees' do
-        pending('to be tested, issue #26546')
-        expect(true).to be(false)
+        skip('to be tested, issue #26546')
       end
     end
 
@@ -228,8 +223,7 @@ describe 'Filter issues', js: true, feature: true do
 
     context 'sorting' do
       it 'sorts' do
-        pending('to be tested, issue #26546')
-        expect(true).to be(false)
+        skip('to be tested, issue #26546')
       end
     end
   end
@@ -253,8 +247,7 @@ describe 'Filter issues', js: true, feature: true do
       end
 
       it 'filters issues by invalid label' do
-        pending('to be tested, issue #26546')
-        expect(true).to be(false)
+        skip('to be tested, issue #26546')
       end
 
       it 'filters issues by multiple labels' do
@@ -429,8 +422,7 @@ describe 'Filter issues', js: true, feature: true do
 
     context 'sorting' do
       it 'sorts' do
-        pending('to be tested, issue #26546')
-        expect(true).to be(false)
+        skip('to be tested, issue #26546')
       end
     end
   end
@@ -456,13 +448,11 @@ describe 'Filter issues', js: true, feature: true do
       end
 
       it 'filters issues by invalid milestones' do
-        pending('to be tested, issue #26546')
-        expect(true).to be(false)
+        skip('to be tested, issue #26546')
       end
 
       it 'filters issues by multiple milestones' do
-        pending('to be tested, issue #26546')
-        expect(true).to be(false)
+        skip('to be tested, issue #26546')
       end
 
       it 'filters issues by milestone containing special characters' do
@@ -523,8 +513,7 @@ describe 'Filter issues', js: true, feature: true do
 
     context 'sorting' do
       it 'sorts' do
-        pending('to be tested, issue #26546')
-        expect(true).to be(false)
+        skip('to be tested, issue #26546')
       end
     end
   end
diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb
index c78cd30157e..1dbc2f6eb13 100644
--- a/spec/lib/gitlab/regex_spec.rb
+++ b/spec/lib/gitlab/regex_spec.rb
@@ -2,47 +2,52 @@
 require 'spec_helper'
 
 describe Gitlab::Regex, lib: true do
-  describe 'project path regex' do
-    it { expect('gitlab-ce').to match(Gitlab::Regex.project_path_regex) }
-    it { expect('gitlab_git').to match(Gitlab::Regex.project_path_regex) }
-    it { expect('_underscore.js').to match(Gitlab::Regex.project_path_regex) }
-    it { expect('100px.com').to match(Gitlab::Regex.project_path_regex) }
-    it { expect('?gitlab').not_to match(Gitlab::Regex.project_path_regex) }
-    it { expect('git lab').not_to match(Gitlab::Regex.project_path_regex) }
-    it { expect('gitlab.git').not_to match(Gitlab::Regex.project_path_regex) }
-  end
+  describe '.project_path_regex' do
+    subject { described_class.project_path_regex }
 
-  describe 'project name regex' do
-    it { expect('gitlab-ce').to match(Gitlab::Regex.project_name_regex) }
-    it { expect('GitLab CE').to match(Gitlab::Regex.project_name_regex) }
-    it { expect('100 lines').to match(Gitlab::Regex.project_name_regex) }
-    it { expect('gitlab.git').to match(Gitlab::Regex.project_name_regex) }
-    it { expect('Český název').to match(Gitlab::Regex.project_name_regex) }
-    it { expect('Dash – is this').to match(Gitlab::Regex.project_name_regex) }
-    it { expect('?gitlab').not_to match(Gitlab::Regex.project_name_regex) }
+    it { is_expected.to match('gitlab-ce') }
+    it { is_expected.to match('gitlab_git') }
+    it { is_expected.to match('_underscore.js') }
+    it { is_expected.to match('100px.com') }
+    it { is_expected.not_to match('?gitlab') }
+    it { is_expected.not_to match('git lab') }
+    it { is_expected.not_to match('gitlab.git') }
   end
 
-  describe 'file name regex' do
-    it { expect('foo@bar').to match(Gitlab::Regex.file_name_regex) }
+  describe '.project_name_regex' do
+    subject { described_class.project_name_regex }
+
+    it { is_expected.to match('gitlab-ce') }
+    it { is_expected.to match('GitLab CE') }
+    it { is_expected.to match('100 lines') }
+    it { is_expected.to match('gitlab.git') }
+    it { is_expected.to match('Český název') }
+    it { is_expected.to match('Dash – is this') }
+    it { is_expected.not_to match('?gitlab') }
   end
 
-  describe 'file path regex' do
-    it { expect('foo@/bar').to match(Gitlab::Regex.file_path_regex) }
+  describe '.file_name_regex' do
+    subject { described_class.file_name_regex }
+
+    it { is_expected.to match('foo@bar') }
   end
 
-  describe 'environment slug regex' do
-    def be_matched
-      match(Gitlab::Regex.environment_slug_regex)
-    end
+  describe '.file_path_regex' do
+    subject { described_class.file_path_regex }
+
+    it { is_expected.to match('foo@/bar') }
+  end
 
-    it { expect('foo').to be_matched }
-    it { expect('foo-1').to be_matched }
+  describe '.environment_slug_regex' do
+    subject { described_class.environment_slug_regex }
 
-    it { expect('FOO').not_to be_matched }
-    it { expect('foo/1').not_to be_matched }
-    it { expect('foo.1').not_to be_matched }
-    it { expect('foo*1').not_to be_matched }
-    it { expect('9foo').not_to be_matched }
-    it { expect('foo-').not_to be_matched }
+    it { is_expected.to match('foo') }
+    it { is_expected.to match('foo-1') }
+    it { is_expected.not_to match('FOO') }
+    it { is_expected.not_to match('foo/1') }
+    it { is_expected.not_to match('foo.1') }
+    it { is_expected.not_to match('foo*1') }
+    it { is_expected.not_to match('9foo') }
+    it { is_expected.not_to match('foo-') }
   end
 end