From 27859ed5eaeae234162b7cce7fd8bd351b5f9369 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 12 Dec 2019 15:08:41 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .gitlab/ci/review.gitlab-ci.yml | 4 +- Dangerfile | 1 + .../project_error_tracking_setting.rb | 2 +- ...r-package-details-installation-instructions.yml | 5 + danger/changelog/Dangerfile | 34 +--- danger/plugins/changelog.rb | 10 ++ doc/development/architecture.md | 3 +- lib/gitlab/danger/changelog.rb | 40 +++++ lib/sentry/client.rb | 39 +---- lib/sentry/client/projects.rb | 46 +++++ locale/gitlab.pot | 7 +- spec/lib/gitlab/danger/changelog_spec.rb | 163 ++++++++++++++++++ spec/lib/gitlab/danger/danger_spec_helper.rb | 17 ++ spec/lib/gitlab/danger/helper_spec.rb | 17 +- spec/lib/sentry/client/projects_spec.rb | 119 +++++++++++++ spec/lib/sentry/client_spec.rb | 185 +-------------------- .../project_error_tracking_setting_spec.rb | 2 +- spec/support/helpers/sentry_client_helpers.rb | 14 ++ .../lib/sentry/client_shared_examples.rb | 56 +++++++ vendor/gitignore/C++.gitignore | 0 vendor/gitignore/Java.gitignore | 0 21 files changed, 500 insertions(+), 264 deletions(-) create mode 100644 changelogs/unreleased/37772-add-ui-event-tracking-for-package-details-installation-instructions.yml create mode 100644 danger/plugins/changelog.rb create mode 100644 lib/gitlab/danger/changelog.rb create mode 100644 lib/sentry/client/projects.rb create mode 100644 spec/lib/gitlab/danger/changelog_spec.rb create mode 100644 spec/lib/gitlab/danger/danger_spec_helper.rb create mode 100644 spec/lib/sentry/client/projects_spec.rb create mode 100644 spec/support/helpers/sentry_client_helpers.rb create mode 100644 spec/support/shared_examples/lib/sentry/client_shared_examples.rb mode change 100755 => 100644 vendor/gitignore/C++.gitignore mode change 100755 => 100644 vendor/gitignore/Java.gitignore diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index af69fdc239c..49447bc629b 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.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} diff --git a/Dangerfile b/Dangerfile index b65a9074078..7879c14b31e 100644 --- a/Dangerfile +++ b/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| diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 3af14d7eef2..6a9986e806b 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/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 = {}) diff --git a/changelogs/unreleased/37772-add-ui-event-tracking-for-package-details-installation-instructions.yml b/changelogs/unreleased/37772-add-ui-event-tracking-for-package-details-installation-instructions.yml new file mode 100644 index 00000000000..c23243dbf4c --- /dev/null +++ b/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 diff --git a/danger/changelog/Dangerfile b/danger/changelog/Dangerfile index f8e31323f41..62b41d14bee 100644 --- a/danger/changelog/Dangerfile +++ b/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 %s "%s" Note: Merge requests with %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 diff --git a/danger/plugins/changelog.rb b/danger/plugins/changelog.rb new file mode 100644 index 00000000000..84f399e9e97 --- /dev/null +++ b/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 diff --git a/doc/development/architecture.md b/doc/development/architecture.md index b579f812d99..eff83da523b 100644 --- a/doc/development/architecture.md +++ b/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 | diff --git a/lib/gitlab/danger/changelog.rb b/lib/gitlab/danger/changelog.rb new file mode 100644 index 00000000000..b53516081be --- /dev/null +++ b/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 diff --git a/lib/sentry/client.rb b/lib/sentry/client.rb index 6cbee830b17..47b497accc1 100644 --- a/lib/sentry/client.rb +++ b/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 diff --git a/lib/sentry/client/projects.rb b/lib/sentry/client/projects.rb new file mode 100644 index 00000000000..68f8fe0f9c9 --- /dev/null +++ b/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 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5315f4e0d1a..9c7dcdcae63 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -53,10 +53,10 @@ msgstr "" msgid " or " 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 "" diff --git a/spec/lib/gitlab/danger/changelog_spec.rb b/spec/lib/gitlab/danger/changelog_spec.rb new file mode 100644 index 00000000000..888094eaf6e --- /dev/null +++ b/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 diff --git a/spec/lib/gitlab/danger/danger_spec_helper.rb b/spec/lib/gitlab/danger/danger_spec_helper.rb new file mode 100644 index 00000000000..b1e84b3c13d --- /dev/null +++ b/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 diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index 8056418e697..d7e67444fca 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/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 diff --git a/spec/lib/sentry/client/projects_spec.rb b/spec/lib/sentry/client/projects_spec.rb new file mode 100644 index 00000000000..462f74eaac9 --- /dev/null +++ b/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 diff --git a/spec/lib/sentry/client_spec.rb b/spec/lib/sentry/client_spec.rb index 388a6418929..cff06bf4a5f 100644 --- a/spec/lib/sentry/client_spec.rb +++ b/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 diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 6bb09205a7b..ef426661066 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/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 diff --git a/spec/support/helpers/sentry_client_helpers.rb b/spec/support/helpers/sentry_client_helpers.rb new file mode 100644 index 00000000000..7476b5fb249 --- /dev/null +++ b/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 diff --git a/spec/support/shared_examples/lib/sentry/client_shared_examples.rb b/spec/support/shared_examples/lib/sentry/client_shared_examples.rb new file mode 100644 index 00000000000..76b71ebd3c5 --- /dev/null +++ b/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 diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore old mode 100755 new mode 100644 diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore old mode 100755 new mode 100644 -- cgit v1.2.1