Add latest changes from gitlab-org/gitlab@master

8 files changed, 108 insertions, 65 deletions

diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb
index 6594a3dfa8b..b039c572677 100644
--- a/spec/lib/gitlab/ci/config_spec.rb
+++ b/spec/lib/gitlab/ci/config_spec.rb
@@ -157,7 +157,7 @@ describe Gitlab::Ci::Config do
 
       describe '.new' do
         it 'raises error' do
-          expect(Gitlab::Sentry).to receive(:track_exception)
+          expect(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception)
 
           expect { config }.to raise_error(
             described_class::ConfigError,
@@ -367,7 +367,7 @@ describe Gitlab::Ci::Config do
       end
 
       it 'raises error TimeoutError' do
-        expect(Gitlab::Sentry).to receive(:track_exception)
+        expect(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception)
 
         expect { config }.to raise_error(
           described_class::ConfigError,
diff --git a/spec/lib/gitlab/diff/highlight_spec.rb b/spec/lib/gitlab/diff/highlight_spec.rb
index f5d3d14ccc5..1a14f6d4f67 100644
--- a/spec/lib/gitlab/diff/highlight_spec.rb
+++ b/spec/lib/gitlab/diff/highlight_spec.rb
@@ -105,7 +105,7 @@ describe Gitlab::Diff::Highlight do
         end
 
         it 'keeps the original rich line' do
-          allow(Gitlab::Sentry).to receive(:track_exception)
+          allow(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception)
 
           code = %q{+      raise RuntimeError, "System commands must be given as an array of strings"}
 
@@ -114,7 +114,7 @@ describe Gitlab::Diff::Highlight do
         end
 
         it 'reports to Sentry if configured' do
-          expect(Gitlab::Sentry).to receive(:track_exception).and_call_original
+          expect(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception).and_call_original
 
           expect { subject }. to raise_exception(RangeError)
         end
diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb
index 80c1493d01b..b7ad6cebf81 100644
--- a/spec/lib/gitlab/gitaly_client_spec.rb
+++ b/spec/lib/gitlab/gitaly_client_spec.rb
@@ -86,12 +86,11 @@ describe Gitlab::GitalyClient do
 
   describe '.stub_certs' do
     it 'skips certificates if OpenSSLError is raised and report it' do
-      expect(Rails.logger).to receive(:error).at_least(:once)
       expect(Gitlab::Sentry)
-        .to receive(:track_exception)
+        .to receive(:track_and_raise_for_dev_exception)
         .with(
           a_kind_of(OpenSSL::X509::CertificateError),
-          extra: { cert_file: a_kind_of(String) }).at_least(:once)
+          cert_file: a_kind_of(String)).at_least(:once)
 
       expect(OpenSSL::X509::Certificate)
         .to receive(:new)
diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
index 30d23fcf9d4..7065b3f9bcb 100644
--- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
+++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb
@@ -301,7 +301,7 @@ describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitlab_redi
 
       it 'ignores Git command errors when creating a branch' do
         expect(project.repository).to receive(:add_branch).and_raise(Gitlab::Git::CommandError)
-        expect(Gitlab::Sentry).to receive(:track_acceptable_exception).and_call_original
+        expect(Gitlab::Sentry).to receive(:track_exception).and_call_original
 
         mr = insert_git_data
 
diff --git a/spec/lib/gitlab/gpg_spec.rb b/spec/lib/gitlab/gpg_spec.rb
index 5d43023502c..c073d86b4a5 100644
--- a/spec/lib/gitlab/gpg_spec.rb
+++ b/spec/lib/gitlab/gpg_spec.rb
@@ -182,17 +182,17 @@ describe Gitlab::Gpg do
       expected_tmp_dir = nil
 
       expect(described_class).to receive(:cleanup_tmp_dir).and_raise(expected_exception)
-      allow(Gitlab::Sentry).to receive(:track_exception)
+      allow(Gitlab::Sentry).to receive(:track_and_raise_for_dev_exception)
 
       described_class.using_tmp_keychain do
         expected_tmp_dir = described_class.current_home_dir
         FileUtils.touch(File.join(expected_tmp_dir, 'dummy.file'))
       end
 
-      expect(Gitlab::Sentry).to have_received(:track_exception).with(
+      expect(Gitlab::Sentry).to have_received(:track_and_raise_for_dev_exception).with(
         expected_exception,
         issue_url: 'https://gitlab.com/gitlab-org/gitlab/issues/20918',
-        extra: { tmp_dir: expected_tmp_dir, contents: ['dummy.file'] }
+        tmp_dir: expected_tmp_dir, contents: ['dummy.file']
       )
     end
 
diff --git a/spec/lib/gitlab/import_export/shared_spec.rb b/spec/lib/gitlab/import_export/shared_spec.rb
index 8d522252e2d..9a2339e576a 100644
--- a/spec/lib/gitlab/import_export/shared_spec.rb
+++ b/spec/lib/gitlab/import_export/shared_spec.rb
@@ -49,24 +49,9 @@ describe Gitlab::ImportExport::Shared do
     it 'updates the import JID' do
       import_state = create(:import_state, project: project, jid: 'jid-test')
 
-      expect_next_instance_of(Gitlab::Import::Logger) do |logger|
-        expect(logger).to receive(:error).with(hash_including(import_jid: import_state.jid))
-      end
-
-      subject.error(error)
-    end
-
-    it 'calls the error logger without a backtrace' do
-      expect(subject).to receive(:log_error).with(message: error.message)
-
-      subject.error(error)
-    end
-
-    it 'calls the error logger with the full message' do
-      backtrace = caller
-      allow(error).to receive(:backtrace).and_return(caller)
-
-      expect(subject).to receive(:log_error).with(message: error.message, error_backtrace: Gitlab::Profiler.clean_backtrace(backtrace))
+      expect(Gitlab::Sentry)
+        .to receive(:track_exception)
+        .with(error, hash_including(import_jid: import_state.jid))
 
       subject.error(error)
     end
diff --git a/spec/lib/gitlab/import_export/version_checker_spec.rb b/spec/lib/gitlab/import_export/version_checker_spec.rb
index 6a626d864a9..befbd1b4c19 100644
--- a/spec/lib/gitlab/import_export/version_checker_spec.rb
+++ b/spec/lib/gitlab/import_export/version_checker_spec.rb
@@ -8,10 +8,20 @@ describe Gitlab::ImportExport::VersionChecker do
 
   describe 'bundle a project Git repo' do
     let(:version) { Gitlab::ImportExport.version }
+    let(:version_file) { Tempfile.new('VERSION') }
 
     before do
       allow_any_instance_of(Gitlab::ImportExport::Shared).to receive(:relative_archive_path).and_return('')
-      allow(File).to receive(:open).and_return(version)
+
+      version_file.write(version)
+      version_file.rewind
+
+      allow_any_instance_of(described_class).to receive(:version_file).and_return(version_file.path)
+    end
+
+    after do
+      version_file.close
+      version_file.unlink
     end
 
     it 'returns true if Import/Export have the same version' do
diff --git a/spec/lib/gitlab/sentry_spec.rb b/spec/lib/gitlab/sentry_spec.rb
index 024ac733a07..6d05241e72d 100644
--- a/spec/lib/gitlab/sentry_spec.rb
+++ b/spec/lib/gitlab/sentry_spec.rb
@@ -3,12 +3,31 @@
 require 'spec_helper'
 
 describe Gitlab::Sentry do
-  describe '.context' do
-    it 'adds the expected tags' do
-      expect(described_class).to receive(:enabled?).and_return(true)
-      allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('cid')
+  let(:exception) { RuntimeError.new('boom') }
+  let(:issue_url) { 'http://gitlab.com/gitlab-org/gitlab-foss/issues/1' }
+
+  let(:expected_payload_includes) do
+    [
+      { 'exception.class' => 'RuntimeError' },
+      { 'exception.message' => 'boom' },
+      { 'tags.correlation_id' => 'cid' },
+      { 'extra.some_other_info' => 'info' },
+      { 'extra.issue_url' => 'http://gitlab.com/gitlab-org/gitlab-foss/issues/1' }
+    ]
+  end
+
+  before do
+    stub_sentry_settings
 
-      described_class.context(nil)
+    allow(described_class).to receive(:sentry_dsn).and_return(Gitlab.config.sentry.dsn)
+    allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('cid')
+
+    described_class.configure
+  end
+
+  describe '.with_context' do
+    it 'adds the expected tags' do
+      described_class.with_context {}
 
       expect(Raven.tags_context[:locale].to_s).to eq(I18n.locale.to_s)
       expect(Raven.tags_context[Labkit::Correlation::CorrelationId::LOG_KEY.to_sym].to_s)
@@ -16,23 +35,23 @@ describe Gitlab::Sentry do
     end
   end
 
-  describe '.track_exception' do
-    let(:exception) { RuntimeError.new('boom') }
+  describe '.track_and_raise_for_dev_exception' do
+    context 'when exceptions for dev should be raised' do
+      before do
+        expect(described_class).to receive(:should_raise_for_dev?).and_return(true)
+      end
 
-    before do
-      allow(described_class).to receive(:enabled?).and_return(true)
-    end
+      it 'raises the exception' do
+        expect(Raven).to receive(:capture_exception)
 
-    it 'raises the exception if it should' do
-      expect(described_class).to receive(:should_raise_for_dev?).and_return(true)
-      expect { described_class.track_exception(exception) }
-        .to raise_error(RuntimeError)
+        expect { described_class.track_and_raise_for_dev_exception(exception) }
+          .to raise_error(RuntimeError)
+      end
     end
 
-    context 'when exceptions should not be raised' do
+    context 'when exceptions for dev should not be raised' do
       before do
-        allow(described_class).to receive(:should_raise_for_dev?).and_return(false)
-        allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('cid')
+        expect(described_class).to receive(:should_raise_for_dev?).and_return(false)
       end
 
       it 'logs the exception with all attributes passed' do
@@ -50,30 +69,49 @@ describe Gitlab::Sentry do
                             tags: a_hash_including(expected_tags),
                             extra: a_hash_including(expected_extras))
 
-        described_class.track_exception(
+        described_class.track_and_raise_for_dev_exception(
           exception,
-          issue_url: 'http://gitlab.com/gitlab-org/gitlab-foss/issues/1',
-          extra: { some_other_info: 'info' }
+          issue_url: issue_url,
+          some_other_info: 'info'
         )
       end
 
-      it 'sets the context' do
-        expect(described_class).to receive(:context)
+      it 'calls Gitlab::Sentry::Logger.error with formatted payload' do
+        expect(Gitlab::Sentry::Logger).to receive(:error)
+          .with(a_hash_including(*expected_payload_includes))
 
-        described_class.track_exception(exception)
+        described_class.track_and_raise_for_dev_exception(
+          exception,
+          issue_url: issue_url,
+          some_other_info: 'info'
+        )
       end
     end
   end
 
-  context '.track_acceptable_exception' do
-    let(:exception) { RuntimeError.new('boom') }
-    let(:issue_url) { 'http://gitlab.com/gitlab-org/gitlab-foss/issues/1' }
+  describe '.track_and_raise_exception' do
+    it 'always raises the exception' do
+      expect(Raven).to receive(:capture_exception)
+
+      expect { described_class.track_and_raise_exception(exception) }
+        .to raise_error(RuntimeError)
+    end
 
-    before do
-      allow(described_class).to receive(:enabled?).and_return(true)
-      allow(Labkit::Correlation::CorrelationId).to receive(:current_id).and_return('cid')
+    it 'calls Gitlab::Sentry::Logger.error with formatted payload' do
+      expect(Gitlab::Sentry::Logger).to receive(:error)
+        .with(a_hash_including(*expected_payload_includes))
+
+      expect do
+        described_class.track_and_raise_exception(
+          exception,
+          issue_url: issue_url,
+          some_other_info: 'info'
+        )
+      end.to raise_error(RuntimeError)
     end
+  end
 
+  describe '.track_exception' do
     it 'calls Raven.capture_exception' do
       expected_extras = {
         some_other_info: 'info',
@@ -89,34 +127,45 @@ describe Gitlab::Sentry do
                           tags: a_hash_including(expected_tags),
                           extra: a_hash_including(expected_extras))
 
-      described_class.track_acceptable_exception(
+      described_class.track_exception(
+        exception,
+        issue_url: issue_url,
+        some_other_info: 'info'
+      )
+    end
+
+    it 'calls Gitlab::Sentry::Logger.error with formatted payload' do
+      expect(Gitlab::Sentry::Logger).to receive(:error)
+        .with(a_hash_including(*expected_payload_includes))
+
+      described_class.track_exception(
         exception,
         issue_url: issue_url,
-        extra: { some_other_info: 'info' }
+        some_other_info: 'info'
       )
     end
 
     context 'the exception implements :sentry_extra_data' do
       let(:extra_info) { { event: 'explosion', size: :massive } }
-      let(:exception) { double(message: 'bang!', sentry_extra_data: extra_info) }
+      let(:exception) { double(message: 'bang!', sentry_extra_data: extra_info, backtrace: caller) }
 
       it 'includes the extra data from the exception in the tracking information' do
         expect(Raven).to receive(:capture_exception)
           .with(exception, a_hash_including(extra: a_hash_including(extra_info)))
 
-        described_class.track_acceptable_exception(exception)
+        described_class.track_exception(exception)
       end
     end
 
     context 'the exception implements :sentry_extra_data, which returns nil' do
-      let(:exception) { double(message: 'bang!', sentry_extra_data: nil) }
+      let(:exception) { double(message: 'bang!', sentry_extra_data: nil, backtrace: caller) }
 
       it 'just includes the other extra info' do
         extra_info = { issue_url: issue_url }
         expect(Raven).to receive(:capture_exception)
           .with(exception, a_hash_including(extra: a_hash_including(extra_info)))
 
-        described_class.track_acceptable_exception(exception, extra_info)
+        described_class.track_exception(exception, extra_info)
       end
     end
   end