Merge branch 'master' into rubocop-for-tests

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

Conflicts:
	spec/features/issues_spec.rb
	spec/models/forked_project_link_spec.rb
	spec/models/hooks/service_hook_spec.rb
	spec/models/hooks/web_hook_spec.rb
	spec/models/project_services/hipchat_service_spec.rb
	spec/requests/api/project_members_spec.rb
	spec/requests/api/projects_spec.rb
	spec/requests/api/system_hooks_spec.rb
	spec/services/archive_repository_service_spec.rb
	spec/support/matchers.rb
	spec/tasks/gitlab/backup_rake_spec.rb

6 files changed, 70 insertions, 109 deletions

diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb
index 3e41aec425a..fed1ab6ee33 100644
--- a/spec/support/capybara.rb
+++ b/spec/support/capybara.rb
@@ -19,36 +19,3 @@ unless ENV['CI'] || ENV['CI_SERVER']
   # Keep only the screenshots generated from the last failing test suite
   Capybara::Screenshot.prune_strategy = :keep_last_run
 end
-
-module CapybaraHelpers
-  # Execute a block a certain number of times before considering it a failure
-  #
-  # The given block is called, and if it raises a `Capybara::ExpectationNotMet`
-  # error, we wait `interval` seconds and then try again, until `retries` is
-  # met.
-  #
-  # This allows for better handling of timing-sensitive expectations in a
-  # sketchy CI environment, for example.
-  #
-  # interval - Delay between retries in seconds (default: 0.5)
-  # retries  - Number of times to execute before failing (default: 5)
-  def allowing_for_delay(interval: 0.5, retries: 5)
-    tries = 0
-
-    begin
-      yield
-    rescue Capybara::ExpectationNotMet => ex
-      if tries <= retries
-        tries += 1
-        sleep interval
-        retry
-      else
-        raise ex
-      end
-    end
-  end
-end
-
-RSpec.configure do |config|
-  config.include CapybaraHelpers, type: :feature
-end
diff --git a/spec/support/capybara_helpers.rb b/spec/support/capybara_helpers.rb
new file mode 100644
index 00000000000..9b5c3065eed
--- /dev/null
+++ b/spec/support/capybara_helpers.rb
@@ -0,0 +1,34 @@
+module CapybaraHelpers
+  # Execute a block a certain number of times before considering it a failure
+  #
+  # The given block is called, and if it raises a `Capybara::ExpectationNotMet`
+  # error, we wait `interval` seconds and then try again, until `retries` is
+  # met.
+  #
+  # This allows for better handling of timing-sensitive expectations in a
+  # sketchy CI environment, for example.
+  #
+  # interval - Delay between retries in seconds (default: 0.5)
+  # retries  - Number of times to execute before failing (default: 5)
+  def allowing_for_delay(interval: 0.5, retries: 5)
+    tries = 0
+
+    begin
+      sleep interval
+
+      yield
+    rescue Capybara::ExpectationNotMet => ex
+      if tries <= retries
+        tries += 1
+        sleep interval
+        retry
+      else
+        raise ex
+      end
+    end
+  end
+end
+
+RSpec.configure do |config|
+  config.include CapybaraHelpers, type: :feature
+end
diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb
index 6250737b9cd..e0dbc9aa84c 100644
--- a/spec/support/db_cleaner.rb
+++ b/spec/support/db_cleaner.rb
@@ -1,21 +1,3 @@
-# RSpec.configure do |config|
-
-#   config.around(:each) do |example|
-#     DatabaseCleaner.strategy = :transaction
-#     DatabaseCleaner.clean_with(:truncation)
-#     DatabaseCleaner.cleaning do
-#       example.run
-#     end
-#   end
-
-#   config.around(:each, js: true) do |example|
-#     DatabaseCleaner.strategy = :truncation
-#     DatabaseCleaner.clean_with(:truncation)
-#     DatabaseCleaner.cleaning do
-#       example.run
-#     end
-#   end
-# end
 RSpec.configure do |config|
   config.before(:suite) do
     DatabaseCleaner.clean_with(:truncation)
@@ -36,15 +18,4 @@ RSpec.configure do |config|
   config.after(:each) do
     DatabaseCleaner.clean
   end
-
-  # rspec-rails 3 will no longer automatically infer an example group's spec type
-  # from the file location. You can explicitly opt-in to the feature using this
-  # config option.
-  # To explicitly tag specs without using automatic inference, set the `:type`
-  # metadata manually:
-  #
-  #     describe ThingsController, :type => :controller do
-  #       # Equivalent to being in spec/controllers
-  #     end
-  config.infer_spec_type_from_file_location!
 end
diff --git a/spec/support/matchers.rb b/spec/support/matchers.rb
index 5f0c270f967..e5ebc6e7ec8 100644
--- a/spec/support/matchers.rb
+++ b/spec/support/matchers.rb
@@ -1,30 +1,43 @@
 RSpec::Matchers.define :be_valid_commit do
   match do |actual|
-    !actual.nil?
-    actual.id == ValidCommit::ID
-    actual.message == ValidCommit::MESSAGE
-    actual.author_name == ValidCommit::AUTHOR_FULL_NAME
+    actual &&
+      actual.id == ValidCommit::ID &&
+      actual.message == ValidCommit::MESSAGE &&
+      actual.author_name == ValidCommit::AUTHOR_FULL_NAME
   end
 end
 
+def emulate_user(user)
+  user = case user
+        when :user then create(:user)
+        when :visitor then nil
+        when :admin then create(:admin)
+        else user
+        end
+  login_with(user) if user
+end
+
 RSpec::Matchers.define :be_allowed_for do |user|
   match do |url|
-    include UrlAccess
-    url_allowed?(user, url)
+    emulate_user(user)
+    visit url
+    status_code != 404 && current_path != new_user_session_path
   end
 end
 
 RSpec::Matchers.define :be_denied_for do |user|
   match do |url|
-    include UrlAccess
-    url_denied?(user, url)
+    emulate_user(user)
+    visit url
+    status_code == 404 || current_path == new_user_session_path
   end
 end
 
-RSpec::Matchers.define :be_404_for do |user|
+RSpec::Matchers.define :be_not_found_for do |user|
   match do |url|
-    include UrlAccess
-    url_404?(user, url)
+    emulate_user(user)
+    visit url
+    status_code == 404
   end
 end
 
@@ -33,38 +46,12 @@ RSpec::Matchers.define :include_module do |expected|
     described_class.included_modules.include?(expected)
   end
 
-  failure_message_for_should do
-    "expected #{described_class} to include the #{expected} module"
+  description do
+    "includes the #{expected} module"
   end
-end
 
-module UrlAccess
-  def url_allowed?(user, url)
-    emulate_user(user)
-    visit url
-    (status_code != 404 && current_path != new_user_session_path)
-  end
-
-  def url_denied?(user, url)
-    emulate_user(user)
-    visit url
-    (status_code == 404 || current_path == new_user_session_path)
-  end
-
-  def url_404?(user, url)
-    emulate_user(user)
-    visit url
-    status_code == 404
-  end
-
-  def emulate_user(user)
-    user = case user
-           when :user then create(:user)
-           when :visitor then nil
-           when :admin then create(:admin)
-           else user
-           end
-    login_with(user) if user
+  failure_message do
+    "expected #{described_class} to include the #{expected} module"
   end
 end
 
diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb
index d29c8a55c82..a2a0b6905f9 100644
--- a/spec/support/mentionable_shared_examples.rb
+++ b/spec/support/mentionable_shared_examples.rb
@@ -80,7 +80,7 @@ shared_examples 'a mentionable' do
                          ext_issue, ext_mr, ext_commit]
 
     mentioned_objects.each do |referenced|
-      expect(Note).to receive(:create_cross_reference_note).
+      expect(SystemNoteService).to receive(:cross_reference).
         with(referenced, subject.local_reference, author)
     end
 
@@ -88,7 +88,7 @@ shared_examples 'a mentionable' do
   end
 
   it 'detects existing cross-references' do
-    Note.create_cross_reference_note(mentioned_issue, subject.local_reference, author)
+    SystemNoteService.cross_reference(mentioned_issue, subject.local_reference, author)
 
     expect(subject).to have_mentioned(mentioned_issue)
     expect(subject).not_to have_mentioned(mentioned_mr)
@@ -132,13 +132,13 @@ shared_examples 'an editable mentionable' do
     # These three objects were already referenced, and should not receive new
     # notes
     [mentioned_issue, mentioned_commit, ext_issue].each do |oldref|
-      expect(Note).not_to receive(:create_cross_reference_note).
+      expect(SystemNoteService).not_to receive(:cross_reference).
         with(oldref, any_args)
     end
 
     # These two issues are new and should receive reference notes
     new_issues.each do |newref|
-      expect(Note).to receive(:create_cross_reference_note).
+      expect(SystemNoteService).to receive(:cross_reference).
         with(newref, subject.local_reference, author)
     end
 
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 6d4a8067910..8bdd6b43cdd 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -41,11 +41,13 @@ module TestEnv
   end
 
   def disable_mailer
-    NotificationService.any_instance.stub(mailer: double.as_null_object)
+    allow_any_instance_of(NotificationService).to receive(:mailer).
+      and_return(double.as_null_object)
   end
 
   def enable_mailer
-    allow_any_instance_of(NotificationService).to receive(:mailer).and_call_original
+    allow_any_instance_of(NotificationService).to receive(:mailer).
+      and_call_original
   end
 
   # Clean /tmp/tests