Add latest changes from gitlab-org/gitlab@master

.gitlab/ci/review.gitlab-ci.yml

@@ -23,8 +23,10 @@ build-qa-image:
   stage: prepare
   script:
     - '[[ ! -d "ee/" ]] || export GITLAB_EDITION="ee"'
+    - export QA_MASTER_IMAGE="${CI_REGISTRY}/${CI_PROJECT_PATH}/gitlab/gitlab-${GITLAB_EDITION}-qa:master"
     - export QA_IMAGE="${CI_REGISTRY}/${CI_PROJECT_PATH}/gitlab/gitlab-${GITLAB_EDITION}-qa:${CI_COMMIT_REF_SLUG}"
-    - time docker build --cache-from gitlab/gitlab-${GITLAB_EDITION}-qa:nightly --tag ${QA_IMAGE} --file ./qa/Dockerfile ./
+    - time docker pull "${QA_MASTER_IMAGE}"
+    - time docker build --cache-from "${QA_MASTER_IMAGE}" --tag ${QA_IMAGE} --file ./qa/Dockerfile ./
     - echo "${CI_JOB_TOKEN}" | docker login --username gitlab-ci-token --password-stdin ${CI_REGISTRY}
     - time docker push ${QA_IMAGE}
 

Dangerfile

@@ -5,6 +5,7 @@ require_relative 'lib/gitlab/danger/request_helper'
 
 danger.import_plugin('danger/plugins/helper.rb')
 danger.import_plugin('danger/plugins/roulette.rb')
+danger.import_plugin('danger/plugins/changelog.rb')
 
 unless helper.release_automation?
   GitlabDanger.new(helper.gitlab_helper).rule_names.each do |file|

app/models/error_tracking/project_error_tracking_setting.rb

@@ -86,7 +86,7 @@ module ErrorTracking
     end
 
     def list_sentry_projects
-      { projects: sentry_client.list_projects }
+      { projects: sentry_client.projects }
     end
 
     def issue_details(opts = {})

changelogs/unreleased/37772-add-ui-event-tracking-for-package-details-installation-instructions.yml

@@ -0,0 +1,5 @@
+---
+title: Added event tracking to the package details installation components
+merge_request: 20967
+author:
+type: changed

danger/changelog/Dangerfile

@@ -3,7 +3,6 @@
 
 require 'yaml'
 
-NO_CHANGELOG_LABELS = %w[backstage ci-build meta].freeze
 SEE_DOC = "See [the documentation](https://docs.gitlab.com/ce/development/changelog.html)."
 CREATE_CHANGELOG_MESSAGE = <<~MSG
 You can create one with:
@@ -21,18 +20,6 @@ bin/changelog --ee -m %<mr_iid>s "%<mr_title>s"
 Note: Merge requests with %<labels>s do not trigger this check.
 MSG
 
-def ee_changelog?(changelog_path)
-  changelog_path =~ /unreleased-ee/
-end
-
-def ce_port_changelog?(changelog_path)
-  helper.ee? && !ee_changelog?(changelog_path)
-end
-
-def categories_need_changelog?
-  (helper.changes_by_category.keys - %i[docs none]).any?
-end
-
 def check_changelog(path)
   yaml = YAML.safe_load(File.read(path))
 
@@ -41,7 +28,7 @@ def check_changelog(path)
 
   if yaml["merge_request"].nil?
     message "Consider setting `merge_request` to #{gitlab.mr_json["iid"]} in #{gitlab.html_link(path)}. #{SEE_DOC}"
-  elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !ce_port_changelog?(path)
+  elsif yaml["merge_request"] != gitlab.mr_json["iid"] && !changelog.ce_port_changelog?(path)
     fail "Merge request ID was not set to #{gitlab.mr_json["iid"]}! #{SEE_DOC}"
   end
 rescue Psych::SyntaxError, Psych::DisallowedClass, Psych::BadAlias
@@ -51,27 +38,18 @@ rescue StandardError => e
   warn "There was a problem trying to check the Changelog. Exception: #{e.name} - #{e.message}"
 end
 
-def presented_no_changelog_labels
-  NO_CHANGELOG_LABELS.map { |label| "~#{label}" }.join(', ')
-end
-
-def sanitized_mr_title
-  gitlab.mr_json["title"].gsub(/^WIP: */, '').gsub(/`/, '\\\`')
-end
-
-changelog_needed = categories_need_changelog? && (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty?
-changelog_found = git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} }
-
 if git.modified_files.include?("CHANGELOG.md")
   fail "**CHANGELOG.md was edited.** Please remove the additions and create a CHANGELOG entry.\n\n" +
-    format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: presented_no_changelog_labels)
+    format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: changelog.sanitized_mr_title, labels: changelog.presented_no_changelog_labels)
 end
 
-if changelog_needed
+changelog_found = changelog.found
+
+if changelog.needed?
   if changelog_found
     check_changelog(changelog_found)
   else
     message "**[CHANGELOG missing](https://docs.gitlab.com/ce/development/changelog.html)**: If this merge request [doesn't need a CHANGELOG entry](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry), feel free to ignore this message.\n\n" +
-      format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: sanitized_mr_title, labels: presented_no_changelog_labels)
+      format(CREATE_CHANGELOG_MESSAGE, mr_iid: gitlab.mr_json["iid"], mr_title: changelog.sanitized_mr_title, labels: changelog.presented_no_changelog_labels)
   end
 end

danger/plugins/changelog.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+require_relative '../../lib/gitlab/danger/changelog'
+
+module Danger
+  class Changelog < Plugin
+    # Put the helper code somewhere it can be tested
+    include Gitlab::Danger::Changelog
+  end
+end

doc/development/architecture.md

@@ -116,6 +116,7 @@ graph TB
 - ⚙ - Requires additional configuration, or GitLab Managed Apps
 - ⤓ - Manual installation required
 - ❌ - Not supported or no instructions available
+- N/A - Not applicable
 
 Component statuses are linked to configuration documentation for each component.
 
@@ -144,7 +145,7 @@ Component statuses are linked to configuration documentation for each component.
 | [Postgres Exporter](#postgres-exporter) | Prometheus endpoint with PostgreSQL metrics | [✅][postgres-exporter-omnibus] | [✅][postgres-exporter-charts] | [✅][postgres-exporter-charts] | [✅](https://about.gitlab.com/handbook/engineering/monitoring/) | ❌ | ❌ | CE & EE |
 | [PgBouncer Exporter](#pgbouncer-exporter) | Prometheus endpoint with PgBouncer metrics | [⚙][pgbouncer-exporter-omnibus] | [❌][pgbouncer-exporter-charts] | [❌][pgbouncer-exporter-charts] | [✅](https://about.gitlab.com/handbook/engineering/monitoring/) | ❌ | ❌ | CE & EE |
 | [GitLab Exporter](#gitlab-exporter) | Generates a variety of GitLab metrics | [✅][gitlab-exporter-omnibus] | [✅][gitlab-exporter-charts] | [✅][gitlab-exporter-charts] | [✅](https://about.gitlab.com/handbook/engineering/monitoring/) | ❌ | ❌ | CE & EE |
-| [Node Exporter](#node-exporter) | Prometheus endpoint with system metrics | [✅][node-exporter-omnibus] | [❌][node-exporter-charts] | [❌][node-exporter-charts] | [✅](https://about.gitlab.com/handbook/engineering/monitoring/) | ❌ | ❌ | CE & EE |
+| [Node Exporter](#node-exporter) | Prometheus endpoint with system metrics | [✅][node-exporter-omnibus] | [N/A][node-exporter-charts] | [N/A][node-exporter-charts] | [✅](https://about.gitlab.com/handbook/engineering/monitoring/) | ❌ | ❌ | CE & EE |
 | [Mattermost](#mattermost) | Open-source Slack alternative | [⚙][mattermost-omnibus] | [⤓][mattermost-charts] | [⤓][mattermost-charts] | [⤓](../user/project/integrations/mattermost.md) | ❌ | ❌ | CE & EE |
 | [MinIO](#minio) | Object storage service | [⤓][minio-omnibus] | [✅][minio-charts] | [✅][minio-charts] | [✅](https://about.gitlab.com/handbook/engineering/infrastructure/production-architecture/#storage-architecture) | ❌ | [⚙][minio-gdk] | CE & EE |
 | [Runner](#gitlab-runner) | Executes GitLab CI jobs | [⤓][runner-omnibus] | [✅][runner-charts] | [⚙][runner-charts] | [✅](../user/gitlab_com/index.md#shared-runners) | [⚙][runner-source] | [⚙][runner-gdk] | CE & EE |

lib/gitlab/danger/changelog.rb

@@ -0,0 +1,40 @@
+# frozen_string_literal: true
+
+module Gitlab
+  module Danger
+    module Changelog
+      NO_CHANGELOG_LABELS = %w[backstage ci-build meta].freeze
+      NO_CHANGELOG_CATEGORIES = %i[docs none].freeze
+
+      def needed?
+        categories_need_changelog? && (gitlab.mr_labels & NO_CHANGELOG_LABELS).empty?
+      end
+
+      def found
+        git.added_files.find { |path| path =~ %r{\A(ee/)?(changelogs/unreleased)(-ee)?/} }
+      end
+
+      def presented_no_changelog_labels
+        NO_CHANGELOG_LABELS.map { |label| "~#{label}" }.join(', ')
+      end
+
+      def sanitized_mr_title
+        gitlab.mr_json["title"].gsub(/^WIP: */, '').gsub(/`/, '\\\`')
+      end
+
+      def ee_changelog?(changelog_path)
+        changelog_path =~ /unreleased-ee/
+      end
+
+      def ce_port_changelog?(changelog_path)
+        helper.ee? && !ee_changelog?(changelog_path)
+      end
+
+      private
+
+      def categories_need_changelog?
+        (helper.changes_by_category.keys - NO_CHANGELOG_CATEGORIES).any?
+      end
+    end
+  end
+end

lib/sentry/client.rb

@@ -2,6 +2,8 @@
 
 module Sentry
   class Client
+    include Sentry::Client::Projects
+
     Error = Class.new(StandardError)
     MissingKeysError = Class.new(StandardError)
     ResponseInvalidSizeError = Class.new(StandardError)
@@ -49,14 +51,6 @@ module Sentry
       end
     end
 
-    def list_projects
-      projects = get_projects
-
-      handle_mapping_exceptions do
-        map_to_projects(projects)
-      end
-    end
-
     private
 
     def validate_size(issues)
@@ -121,10 +115,6 @@ module Sentry
       http_get(issue_latest_event_api_url(issue_id))[:body]
     end
 
-    def get_projects
-      http_get(projects_api_url)[:body]
-    end
-
     def handle_request_exceptions
       yield
     rescue Gitlab::HTTP::Error => e
@@ -155,13 +145,6 @@ module Sentry
       raise Client::Error, message
     end
 
-    def projects_api_url
-      projects_url = URI(@url)
-      projects_url.path = '/api/0/projects/'
-
-      projects_url
-    end
-
     def issue_api_url(issue_id)
       issue_url = URI(@url)
       issue_url.path = "/api/0/issues/#{issue_id}/"
@@ -187,10 +170,6 @@ module Sentry
       issues.map(&method(:map_to_error))
     end
 
-    def map_to_projects(projects)
-      projects.map(&method(:map_to_project))
-    end
-
     def issue_url(id)
       issues_url = @url + "/issues/#{id}"
 
@@ -289,19 +268,5 @@ module Sentry
         project_slug: issue.dig('project', 'slug')
       )
     end
-
-    def map_to_project(project)
-      organization = project.fetch('organization')
-
-      Gitlab::ErrorTracking::Project.new(
-        id: project.fetch('id', nil),
-        name: project.fetch('name'),
-        slug: project.fetch('slug'),
-        status: project.dig('status'),
-        organization_name: organization.fetch('name'),
-        organization_id: organization.fetch('id', nil),
-        organization_slug: organization.fetch('slug')
-      )
-    end
   end
 end

lib/sentry/client/projects.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+module Sentry
+  class Client
+    module Projects
+      def projects
+        projects = get_projects
+
+        handle_mapping_exceptions do
+          map_to_projects(projects)
+        end
+      end
+
+      private
+
+      def get_projects
+        http_get(projects_api_url)[:body]
+      end
+
+      def projects_api_url
+        projects_url = URI(url)
+        projects_url.path = '/api/0/projects/'
+
+        projects_url
+      end
+
+      def map_to_projects(projects)
+        projects.map(&method(:map_to_project))
+      end
+
+      def map_to_project(project)
+        organization = project.fetch('organization')
+
+        Gitlab::ErrorTracking::Project.new(
+          id: project.fetch('id', nil),
+          name: project.fetch('name'),
+          slug: project.fetch('slug'),
+          status: project.dig('status'),
+          organization_name: organization.fetch('name'),
+          organization_id: organization.fetch('id', nil),
+          organization_slug: organization.fetch('slug')
+        )
+      end
+    end
+  end
+end

locale/gitlab.pot

@@ -53,10 +53,10 @@ msgstr ""
 msgid " or <!merge request id>"
 msgstr ""
 
-msgid " or <#epic id>"
+msgid " or <#issue id>"
 msgstr ""
 
-msgid " or <#issue id>"
+msgid " or <&epic id>"
 msgstr ""
 
 msgid " or references (e.g. path/to/project!merge_request_id)"
@@ -18254,6 +18254,9 @@ msgstr ""
 msgid "Those emails automatically become issues (with the comments becoming the email conversation) listed here."
 msgstr ""
 
+msgid "Threat Monitoring"
+msgstr ""
+
 msgid "Thursday"
 msgstr ""
 

spec/lib/gitlab/danger/changelog_spec.rb

@@ -0,0 +1,163 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rspec-parameterized'
+require_relative 'danger_spec_helper'
+
+require 'gitlab/danger/changelog'
+
+describe Gitlab::Danger::Changelog do
+  using RSpec::Parameterized::TableSyntax
+  include DangerSpecHelper
+
+  let(:added_files) { nil }
+  let(:fake_git) { double('fake-git', added_files: added_files) }
+
+  let(:mr_labels) { nil }
+  let(:mr_json) { nil }
+  let(:fake_gitlab) { double('fake-gitlab', mr_labels: mr_labels, mr_json: mr_json) }
+
+  let(:changes_by_category) { nil }
+  let(:ee?) { false }
+  let(:fake_helper) { double('fake-helper', changes_by_category: changes_by_category, ee?: ee?) }
+
+  let(:fake_danger) { new_fake_danger.include(described_class) }
+
+  subject(:changelog) { fake_danger.new(git: fake_git, gitlab: fake_gitlab, helper: fake_helper) }
+
+  describe '#needed?' do
+    subject { changelog.needed? }
+
+    [
+      { docs: nil },
+      { none: nil },
+      { docs: nil, none: nil }
+    ].each do |categories|
+      let(:changes_by_category) { categories }
+      it "is falsy when categories don't require a changelog" do
+        is_expected.to be_falsy
+      end
+    end
+
+    where(:categories, :labels) do
+      { backend: nil }                             | %w[backend backstage]
+      { frontend: nil, docs: nil }                 | ['ci-build']
+      { engineering_productivity: nil, none: nil } | ['meta']
+    end
+
+    with_them do
+      let(:changes_by_category) { categories }
+      let(:mr_labels) { labels }
+
+      it "is falsy when labels require no changelog" do
+        is_expected.to be_falsy
+      end
+    end
+
+    where(:categories, :labels) do
+      { frontend: nil, docs: nil }                 | ['database::review pending', 'feature']
+      { backend: nil }                             | ['backend', 'technical debt']
+      { engineering_productivity: nil, none: nil } | ['frontend']
+    end
+
+    with_them do
+      let(:changes_by_category) { categories }
+      let(:mr_labels) { labels }
+
+      it "is truthy when categories and labels require a changelog" do
+        is_expected.to be_truthy
+      end
+    end
+  end
+
+  describe '#found' do
+    subject { changelog.found }
+
+    context 'added files contain a changelog' do
+      [
+        'changelogs/unreleased/entry.md',
+        'ee/changelogs/unreleased/entry.md',
+        'changelogs/unreleased-ee/entry.md',
+        'ee/changelogs/unreleased-ee/entry.md'
+      ].each do |file_path|
+        let(:added_files) { [file_path] }
+
+        it { is_expected.to be_truthy }
+      end
+    end
+
+    context 'added files do not contain a changelog' do
+      [
+        'app/models/model.rb',
+        'app/assets/javascripts/file.js'
+      ].each do |file_path|
+        let(:added_files) { [file_path] }
+        it { is_expected.to eq(nil) }
+      end
+    end
+  end
+
+  describe '#presented_no_changelog_labels' do
+    subject { changelog.presented_no_changelog_labels }
+
+    it 'returns the labels formatted' do
+      is_expected.to eq('~backstage, ~ci-build, ~meta')
+    end
+  end
+
+  describe '#sanitized_mr_title' do
+    subject { changelog.sanitized_mr_title }
+
+    [
+      'WIP: My MR title',
+      'My MR title'
+    ].each do |mr_title|
+      let(:mr_json) { { "title" => mr_title } }
+      it { is_expected.to eq("My MR title") }
+    end
+  end
+
+  describe '#ee_changelog?' do
+    context 'is ee changelog' do
+      [
+        'changelogs/unreleased-ee/entry.md',
+        'ee/changelogs/unreleased-ee/entry.md'
+      ].each do |file_path|
+        subject { changelog.ee_changelog?(file_path) }
+
+        it { is_expected.to be_truthy }
+      end
+    end
+
+    context 'is not ee changelog' do
+      [
+        'changelogs/unreleased/entry.md',
+        'ee/changelogs/unreleased/entry.md'
+      ].each do |file_path|
+        subject { changelog.ee_changelog?(file_path) }
+
+        it { is_expected.to be_falsy }
+      end
+    end
+  end
+
+  describe '#ce_port_changelog?' do
+    where(:helper_ee?, :file_path, :expected) do
+      true  | 'changelogs/unreleased-ee/entry.md'    | false
+      true  | 'ee/changelogs/unreleased-ee/entry.md' | false
+      false | 'changelogs/unreleased-ee/entry.md'    | false
+      false | 'ee/changelogs/unreleased-ee/entry.md' | false
+      true  | 'changelogs/unreleased/entry.md'       | true
+      true  | 'ee/changelogs/unreleased/entry.md'    | true
+      false | 'changelogs/unreleased/entry.md'       | false
+      false | 'ee/changelogs/unreleased/entry.md'    | false
+    end
+
+    with_them do
+      let(:ee?) { helper_ee? }
+      subject { changelog.ce_port_changelog?(file_path) }
+
+      it { is_expected.to eq(expected) }
+    end
+  end
+end

spec/lib/gitlab/danger/danger_spec_helper.rb

@@ -0,0 +1,17 @@
+# frozen_string_literal: true
+
+module DangerSpecHelper
+  def new_fake_danger
+    Class.new do
+      attr_reader :git, :gitlab, :helper
+
+      # rubocop:disable Gitlab/ModuleWithInstanceVariables
+      def initialize(git: nil, gitlab: nil, helper: nil)
+        @git = git
+        @gitlab = gitlab
+        @helper = helper
+      end
+      # rubocop:enable Gitlab/ModuleWithInstanceVariables
+    end
+  end
+end

spec/lib/gitlab/danger/helper_spec.rb

@@ -2,29 +2,22 @@
 
 require 'fast_spec_helper'
 require 'rspec-parameterized'
+require_relative 'danger_spec_helper'
 
 require 'gitlab/danger/helper'
 
 describe Gitlab::Danger::Helper do
   using RSpec::Parameterized::TableSyntax
-
-  class FakeDanger
-    include Gitlab::Danger::Helper
-
-    attr_reader :git, :gitlab
-
-    def initialize(git:, gitlab:)
-      @git = git
-      @gitlab = gitlab
-    end
-  end
+  include DangerSpecHelper
 
   let(:fake_git) { double('fake-git') }
 
   let(:mr_author) { nil }
   let(:fake_gitlab) { double('fake-gitlab', mr_author: mr_author) }
 
-  subject(:helper) { FakeDanger.new(git: fake_git, gitlab: fake_gitlab) }
+  let(:fake_danger) { new_fake_danger.include(described_class) }
+
+  subject(:helper) { fake_danger.new(git: fake_git, gitlab: fake_gitlab) }
 
   describe '#gitlab_helper' do
     context 'when gitlab helper is not available' do

spec/lib/sentry/client/projects_spec.rb

@@ -0,0 +1,119 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Sentry::Client::Projects do
+  include SentryClientHelpers
+
+  let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
+  let(:token) { 'test-token' }
+  let(:client) { Sentry::Client.new(sentry_url, token) }
+  let(:projects_sample_response) do
+    Gitlab::Utils.deep_indifferent_access(
+      JSON.parse(fixture_file('sentry/list_projects_sample_response.json'))
+    )
+  end
+
+  shared_examples 'has correct return type' do |klass|
+    it "returns objects of type #{klass}" do
+      expect(subject).to all( be_a(klass) )
+    end
+  end
+
+  shared_examples 'has correct length' do |length|
+    it { expect(subject.length).to eq(length) }
+  end
+
+  describe '#projects' do
+    let(:sentry_list_projects_url) { 'https://sentrytest.gitlab.com/api/0/projects/' }
+    let(:sentry_api_response) { projects_sample_response }
+    let!(:sentry_api_request) { stub_sentry_request(sentry_list_projects_url, body: sentry_api_response) }
+
+    subject { client.projects }
+
+    it_behaves_like 'calls sentry api'
+
+    it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project
+    it_behaves_like 'has correct length', 2
+
+    context 'essential keys missing in API response' do
+      let(:sentry_api_response) do
+        projects_sample_response[0...1].map do |project|
+          project.except(:slug)
+        end
+      end
+
+      it 'raises exception' do
+        expect { subject }.to raise_error(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "slug"')
+      end
+    end
+
+    context 'optional keys missing in sentry response' do
+      let(:sentry_api_response) do
+        projects_sample_response[0...1].map do |project|
+          project[:organization].delete(:id)
+          project.delete(:id)
+          project.except(:status)
+        end
+      end
+
+      it_behaves_like 'calls sentry api'
+
+      it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project
+      it_behaves_like 'has correct length', 1
+    end
+
+    context 'error object created from sentry response' do
+      using RSpec::Parameterized::TableSyntax
+
+      where(:sentry_project_object, :sentry_response) do
+        :id                | :id
+        :name              | :name
+        :status            | :status
+        :slug              | :slug
+        :organization_name | [:organization, :name]
+        :organization_id   | [:organization, :id]
+        :organization_slug | [:organization, :slug]
+      end
+
+      with_them do
+        it do
+          expect(subject[0].public_send(sentry_project_object)).to(
+            eq(sentry_api_response[0].dig(*sentry_response))
+          )
+        end
+      end
+    end
+
+    context 'redirects' do
+      let(:sentry_api_url) { sentry_list_projects_url }
+
+      it_behaves_like 'no Sentry redirects'
+    end
+
+    # Sentry API returns 404 if there are extra slashes in the URL!
+    context 'extra slashes in URL' do
+      let(:sentry_url) { 'https://sentrytest.gitlab.com/api//0/projects//' }
+      let!(:valid_req_stub) do
+        stub_sentry_request(sentry_list_projects_url)
+      end
+
+      it 'removes extra slashes in api url' do
+        expect(Gitlab::HTTP).to receive(:get).with(
+          URI(sentry_list_projects_url),
+          anything
+        ).and_call_original
+
+        subject
+
+        expect(valid_req_stub).to have_been_requested
+      end
+    end
+
+    context 'when exception is raised' do
+      let(:sentry_request_url) { sentry_list_projects_url }
+
+      it_behaves_like 'maps Sentry exceptions'
+    end
+  end
+end

spec/lib/sentry/client_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Sentry::Client do
+  include SentryClientHelpers
+
   let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' }
   let(:token) { 'test-token' }
   let(:default_httparty_options) do
@@ -18,89 +20,18 @@ describe Sentry::Client do
     )
   end
 
-  let(:projects_sample_response) do
-    Gitlab::Utils.deep_indifferent_access(
-      JSON.parse(fixture_file('sentry/list_projects_sample_response.json'))
-    )
-  end
-
   subject(:client) { described_class.new(sentry_url, token) }
 
-  # Requires sentry_api_url and subject to be defined
-  shared_examples 'no redirects' do
-    let(:redirect_to) { 'https://redirected.example.com' }
-    let(:other_url) { 'https://other.example.org' }
-
-    let!(:redirected_req_stub) { stub_sentry_request(other_url) }
-
-    let!(:redirect_req_stub) do
-      stub_sentry_request(
-        sentry_api_url,
-        status: 302,
-        headers: { location: redirect_to }
-      )
-    end
-
-    it 'does not follow redirects' do
-      expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response status code: 302')
-      expect(redirect_req_stub).to have_been_requested
-      expect(redirected_req_stub).not_to have_been_requested
-    end
-  end
-
-  shared_examples 'has correct return type' do |klass|
-    it "returns objects of type #{klass}" do
-      expect(subject).to all( be_a(klass) )
-    end
-  end
-
   shared_examples 'issues has correct return type' do |klass|
     it "returns objects of type #{klass}" do
       expect(subject[:issues]).to all( be_a(klass) )
     end
   end
 
-  shared_examples 'has correct length' do |length|
-    it { expect(subject.length).to eq(length) }
-  end
-
   shared_examples 'issues has correct length' do |length|
     it { expect(subject[:issues].length).to eq(length) }
   end
 
-  # Requires sentry_api_request and subject to be defined
-  shared_examples 'calls sentry api' do
-    it 'calls sentry api' do
-      subject
-
-      expect(sentry_api_request).to have_been_requested
-    end
-  end
-
-  shared_examples 'maps exceptions' do
-    exceptions = {
-      Gitlab::HTTP::Error => 'Error when connecting to Sentry',
-      Net::OpenTimeout => 'Connection to Sentry timed out',
-      SocketError => 'Received SocketError when trying to connect to Sentry',
-      OpenSSL::SSL::SSLError => 'Sentry returned invalid SSL data',
-      Errno::ECONNREFUSED => 'Connection refused',
-      StandardError => 'Sentry request failed due to StandardError'
-    }
-
-    exceptions.each do |exception, message|
-      context "#{exception}" do
-        before do
-          stub_request(:get, sentry_request_url).to_raise(exception)
-        end
-
-        it do
-          expect { subject }
-            .to raise_exception(Sentry::Client::Error, message)
-        end
-      end
-    end
-  end
-
   describe '#list_issues' do
     let(:issue_status) { 'unresolved' }
     let(:limit) { 20 }
@@ -174,7 +105,7 @@ describe Sentry::Client do
     context 'redirects' do
       let(:sentry_api_url) { sentry_url + '/issues/?limit=20&query=is:unresolved' }
 
-      it_behaves_like 'no redirects'
+      it_behaves_like 'no Sentry redirects'
     end
 
     # Sentry API returns 404 if there are extra slashes in the URL!
@@ -265,7 +196,7 @@ describe Sentry::Client do
       end
     end
 
-    it_behaves_like 'maps exceptions'
+    it_behaves_like 'maps Sentry exceptions'
 
     context 'when search term is present' do
       let(:search_term) { 'NoMethodError' }
@@ -287,112 +218,4 @@ describe Sentry::Client do
       it_behaves_like 'issues has correct length', 1
     end
   end
-
-  describe '#list_projects' do
-    let(:sentry_list_projects_url) { 'https://sentrytest.gitlab.com/api/0/projects/' }
-
-    let(:sentry_api_response) { projects_sample_response }
-
-    let!(:sentry_api_request) { stub_sentry_request(sentry_list_projects_url, body: sentry_api_response) }
-
-    subject { client.list_projects }
-
-    it_behaves_like 'calls sentry api'
-
-    it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project
-    it_behaves_like 'has correct length', 2
-
-    context 'essential keys missing in API response' do
-      let(:sentry_api_response) do
-        projects_sample_response[0...1].map do |project|
-          project.except(:slug)
-        end
-      end
-
-      it 'raises exception' do
-        expect { subject }.to raise_error(Sentry::Client::MissingKeysError, 'Sentry API response is missing keys. key not found: "slug"')
-      end
-    end
-
-    context 'optional keys missing in sentry response' do
-      let(:sentry_api_response) do
-        projects_sample_response[0...1].map do |project|
-          project[:organization].delete(:id)
-          project.delete(:id)
-          project.except(:status)
-        end
-      end
-
-      it_behaves_like 'calls sentry api'
-
-      it_behaves_like 'has correct return type', Gitlab::ErrorTracking::Project
-      it_behaves_like 'has correct length', 1
-    end
-
-    context 'error object created from sentry response' do
-      using RSpec::Parameterized::TableSyntax
-
-      where(:sentry_project_object, :sentry_response) do
-        :id                | :id
-        :name              | :name
-        :status            | :status
-        :slug              | :slug
-        :organization_name | [:organization, :name]
-        :organization_id   | [:organization, :id]
-        :organization_slug | [:organization, :slug]
-      end
-
-      with_them do
-        it do
-          expect(subject[0].public_send(sentry_project_object)).to(
-            eq(sentry_api_response[0].dig(*sentry_response))
-          )
-        end
-      end
-    end
-
-    context 'redirects' do
-      let(:sentry_api_url) { sentry_list_projects_url }
-
-      it_behaves_like 'no redirects'
-    end
-
-    # Sentry API returns 404 if there are extra slashes in the URL!
-    context 'extra slashes in URL' do
-      let(:sentry_url) { 'https://sentrytest.gitlab.com/api//0/projects//' }
-      let(:client) { described_class.new(sentry_url, token) }
-
-      let!(:valid_req_stub) do
-        stub_sentry_request(sentry_list_projects_url)
-      end
-
-      it 'removes extra slashes in api url' do
-        expect(Gitlab::HTTP).to receive(:get).with(
-          URI(sentry_list_projects_url),
-          anything
-        ).and_call_original
-
-        subject
-
-        expect(valid_req_stub).to have_been_requested
-      end
-    end
-
-    context 'when exception is raised' do
-      let(:sentry_request_url) { sentry_list_projects_url }
-
-      it_behaves_like 'maps exceptions'
-    end
-  end
-
-  private
-
-  def stub_sentry_request(url, body: {}, status: 200, headers: {})
-    stub_request(:get, url)
-      .to_return(
-        status: status,
-        headers: { 'Content-Type' => 'application/json' }.merge(headers),
-        body: body.to_json
-      )
-  end
 end

spec/models/error_tracking/project_error_tracking_setting_spec.rb

@@ -193,7 +193,7 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
 
     it 'calls sentry client' do
       expect(subject).to receive(:sentry_client).and_return(sentry_client)
-      expect(sentry_client).to receive(:list_projects).and_return(projects)
+      expect(sentry_client).to receive(:projects).and_return(projects)
 
       result = subject.list_sentry_projects
 

spec/support/helpers/sentry_client_helpers.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module SentryClientHelpers
+  private
+
+  def stub_sentry_request(url, body: {}, status: 200, headers: {})
+    stub_request(:get, url)
+      .to_return(
+        status: status,
+        headers: { 'Content-Type' => 'application/json' }.merge(headers),
+        body: body.to_json
+      )
+  end
+end

spec/support/shared_examples/lib/sentry/client_shared_examples.rb

@@ -0,0 +1,56 @@
+# frozen_string_literal: true
+
+# Requires sentry_api_request and subject to be defined
+RSpec.shared_examples 'calls sentry api' do
+  it 'calls sentry api' do
+    subject
+
+    expect(sentry_api_request).to have_been_requested
+  end
+end
+
+# Requires sentry_api_url and subject to be defined
+RSpec.shared_examples 'no Sentry redirects' do
+  let(:redirect_to) { 'https://redirected.example.com' }
+  let(:other_url) { 'https://other.example.org' }
+
+  let!(:redirected_req_stub) { stub_sentry_request(other_url) }
+
+  let!(:redirect_req_stub) do
+    stub_sentry_request(
+      sentry_api_url,
+      status: 302,
+      headers: { location: redirect_to }
+    )
+  end
+
+  it 'does not follow redirects' do
+    expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response status code: 302')
+    expect(redirect_req_stub).to have_been_requested
+    expect(redirected_req_stub).not_to have_been_requested
+  end
+end
+
+RSpec.shared_examples 'maps Sentry exceptions' do
+  exceptions = {
+    Gitlab::HTTP::Error => 'Error when connecting to Sentry',
+    Net::OpenTimeout => 'Connection to Sentry timed out',
+    SocketError => 'Received SocketError when trying to connect to Sentry',
+    OpenSSL::SSL::SSLError => 'Sentry returned invalid SSL data',
+    Errno::ECONNREFUSED => 'Connection refused',
+    StandardError => 'Sentry request failed due to StandardError'
+  }
+
+  exceptions.each do |exception, message|
+    context "#{exception}" do
+      before do
+        stub_request(:get, sentry_request_url).to_raise(exception)
+      end
+
+      it do
+        expect { subject }
+          .to raise_exception(Sentry::Client::Error, message)
+      end
+    end
+  end
+end