diff options
Diffstat (limited to 'spec')
56 files changed, 860 insertions, 377 deletions
diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index e01382cf31f..03680f681ba 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -1002,7 +1002,7 @@ RSpec.describe 'File blob', :js do end it 'renders sandboxed iframe' do - expected = %(<iframe src="/-/sandbox/swagger" sandbox="allow-scripts allow-popups" frameborder="0" width="100%" height="1000">) + expected = %(<iframe src="/-/sandbox/swagger" sandbox="allow-scripts allow-popups allow-forms" frameborder="0" width="100%" height="1000">) expect(page.html).to include(expected) end end diff --git a/spec/features/security/admin_access_spec.rb b/spec/features/security/admin_access_spec.rb index 8070ae066e7..de81444ed71 100644 --- a/spec/features/security/admin_access_spec.rb +++ b/spec/features/security/admin_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe "Admin::Projects" do +RSpec.describe "Admin::Projects", feature_category: :permissions do include AccessMatchers describe "GET /admin/projects" do diff --git a/spec/features/security/dashboard_access_spec.rb b/spec/features/security/dashboard_access_spec.rb index 5430329d47d..948a4567624 100644 --- a/spec/features/security/dashboard_access_spec.rb +++ b/spec/features/security/dashboard_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe "Dashboard access" do +RSpec.describe "Dashboard access", feature_category: :permissions do include AccessMatchers describe "GET /dashboard" do diff --git a/spec/features/security/group/internal_access_spec.rb b/spec/features/security/group/internal_access_spec.rb index 755f170a93e..904431b4a0f 100644 --- a/spec/features/security/group/internal_access_spec.rb +++ b/spec/features/security/group/internal_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Internal Group access' do +RSpec.describe 'Internal Group access', feature_category: :permissions do include AccessMatchers let(:group) { create(:group, :internal) } diff --git a/spec/features/security/group/private_access_spec.rb b/spec/features/security/group/private_access_spec.rb index f733145b5e3..3d56468a1c9 100644 --- a/spec/features/security/group/private_access_spec.rb +++ b/spec/features/security/group/private_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Private Group access' do +RSpec.describe 'Private Group access', feature_category: :permissions do include AccessMatchers let(:group) { create(:group, :private) } diff --git a/spec/features/security/group/public_access_spec.rb b/spec/features/security/group/public_access_spec.rb index 90de2b58044..ac6b8a8ddd1 100644 --- a/spec/features/security/group/public_access_spec.rb +++ b/spec/features/security/group/public_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Public Group access' do +RSpec.describe 'Public Group access', feature_category: :permissions do include AccessMatchers let(:group) { create(:group, :public) } diff --git a/spec/features/security/profile_access_spec.rb b/spec/features/security/profile_access_spec.rb index 301efd2d99b..991ff115d3d 100644 --- a/spec/features/security/profile_access_spec.rb +++ b/spec/features/security/profile_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe "Profile access" do +RSpec.describe "Profile access", feature_category: :user_management do include AccessMatchers describe "GET /-/profile/keys" do diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb index 48cee4b1f19..e35e7ed742b 100644 --- a/spec/features/security/project/internal_access_spec.rb +++ b/spec/features/security/project/internal_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe "Internal Project Access" do +RSpec.describe "Internal Project Access", feature_category: :permissions do include AccessMatchers let_it_be(:project, reload: true) { create(:project, :internal, :repository, :with_namespace_settings) } diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb index c06b1e5da54..59ddb18ae8a 100644 --- a/spec/features/security/project/private_access_spec.rb +++ b/spec/features/security/project/private_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe "Private Project Access" do +RSpec.describe "Private Project Access", feature_category: :permissions do include AccessMatchers let_it_be(:project, reload: true) do diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb index d2112430638..425691001f2 100644 --- a/spec/features/security/project/public_access_spec.rb +++ b/spec/features/security/project/public_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe "Public Project Access" do +RSpec.describe "Public Project Access", feature_category: :permissions do include AccessMatchers let_it_be(:project, reload: true) do diff --git a/spec/features/security/project/snippet/internal_access_spec.rb b/spec/features/security/project/snippet/internal_access_spec.rb index ab080f0a460..b7dcc5f31d3 100644 --- a/spec/features/security/project/snippet/internal_access_spec.rb +++ b/spec/features/security/project/snippet/internal_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe "Internal Project Snippets Access" do +RSpec.describe "Internal Project Snippets Access", feature_category: :permissions do include AccessMatchers let_it_be(:project) { create(:project, :internal) } diff --git a/spec/features/security/project/snippet/private_access_spec.rb b/spec/features/security/project/snippet/private_access_spec.rb index 1e0afc09b74..0ae45abb7ec 100644 --- a/spec/features/security/project/snippet/private_access_spec.rb +++ b/spec/features/security/project/snippet/private_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe "Private Project Snippets Access" do +RSpec.describe "Private Project Snippets Access", feature_category: :permissions do include AccessMatchers let_it_be(:project) { create(:project, :private) } diff --git a/spec/features/security/project/snippet/public_access_spec.rb b/spec/features/security/project/snippet/public_access_spec.rb index f734f7ba9e2..b98f665c0dc 100644 --- a/spec/features/security/project/snippet/public_access_spec.rb +++ b/spec/features/security/project/snippet/public_access_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe "Public Project Snippets Access" do +RSpec.describe "Public Project Snippets Access", feature_category: :permissions do include AccessMatchers let_it_be(:project) { create(:project, :public) } diff --git a/spec/features/sentry_js_spec.rb b/spec/features/sentry_js_spec.rb index 1d277ba7b3c..2e46dc459ec 100644 --- a/spec/features/sentry_js_spec.rb +++ b/spec/features/sentry_js_spec.rb @@ -3,26 +3,61 @@ require 'spec_helper' RSpec.describe 'Sentry' do - let(:sentry_regex_path) { '\/sentry.*\.chunk\.js' } + context 'when enable_new_sentry_clientside_integration is disabled' do + before do + stub_feature_flags(enable_new_sentry_clientside_integration: false) + end + + it 'does not load sentry if sentry is disabled' do + allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) + + visit new_user_session_path + + expect(has_requested_legacy_sentry).to eq(false) + end - it 'does not load sentry if sentry is disabled' do - allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) - visit new_user_session_path + it 'loads legacy sentry if sentry config is enabled', :js do + allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) - expect(has_requested_sentry).to eq(false) + visit new_user_session_path + + expect(has_requested_legacy_sentry).to eq(true) + expect(evaluate_script('window._Sentry.SDK_VERSION')).to match(%r{^5\.}) + end end - it 'loads sentry if sentry is enabled' do - stub_sentry_settings + context 'when enable_new_sentry_clientside_integration is enabled' do + before do + stub_feature_flags(enable_new_sentry_clientside_integration: true) + end + + it 'does not load sentry if sentry settings are disabled' do + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(false) - visit new_user_session_path + visit new_user_session_path - expect(has_requested_sentry).to eq(true) + expect(has_requested_sentry).to eq(false) + end + + it 'loads sentry if sentry settings are enabled', :js do + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) + + visit new_user_session_path + + expect(has_requested_sentry).to eq(true) + expect(evaluate_script('window._Sentry.SDK_VERSION')).to match(%r{^7\.}) + end + end + + def has_requested_legacy_sentry + page.all('script', visible: false).one? do |elm| + elm[:src] =~ %r{/legacy_sentry.*\.chunk\.js\z} + end end def has_requested_sentry page.all('script', visible: false).one? do |elm| - elm[:src] =~ /#{sentry_regex_path}$/ + elm[:src] =~ %r{/sentry.*\.chunk\.js\z} end end end diff --git a/spec/features/snippets/embedded_snippet_spec.rb b/spec/features/snippets/embedded_snippet_spec.rb index 90d877d29b7..7bd61824494 100644 --- a/spec/features/snippets/embedded_snippet_spec.rb +++ b/spec/features/snippets/embedded_snippet_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Embedded Snippets' do +RSpec.describe 'Embedded Snippets', feature_category: :snippets do let_it_be(:snippet) { create(:personal_snippet, :public, :repository) } let(:blobs) { snippet.blobs.first(3) } diff --git a/spec/features/snippets/explore_spec.rb b/spec/features/snippets/explore_spec.rb index b62c35bf96e..d0f4d888d94 100644 --- a/spec/features/snippets/explore_spec.rb +++ b/spec/features/snippets/explore_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Explore Snippets' do +RSpec.describe 'Explore Snippets', feature_category: :snippets do let!(:public_snippet) { create(:personal_snippet, :public) } let!(:internal_snippet) { create(:personal_snippet, :internal) } let!(:private_snippet) { create(:personal_snippet, :private) } diff --git a/spec/features/snippets/internal_snippet_spec.rb b/spec/features/snippets/internal_snippet_spec.rb index 2fcd11c2a47..5169cc2a0f8 100644 --- a/spec/features/snippets/internal_snippet_spec.rb +++ b/spec/features/snippets/internal_snippet_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Internal Snippets', :js do +RSpec.describe 'Internal Snippets', :js, feature_category: :snippets do let(:internal_snippet) { create(:personal_snippet, :internal, :repository) } let(:content) { internal_snippet.blobs.first.data.strip! } diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb index 8d55a7a64f4..b9d5e56d432 100644 --- a/spec/features/snippets/notes_on_personal_snippets_spec.rb +++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Comments on personal snippets', :js do +RSpec.describe 'Comments on personal snippets', :js, feature_category: :snippets do include NoteInteractionHelpers include Spec::Support::Helpers::ModalHelpers diff --git a/spec/features/snippets/private_snippets_spec.rb b/spec/features/snippets/private_snippets_spec.rb index 7ff27419cf7..198ea11972d 100644 --- a/spec/features/snippets/private_snippets_spec.rb +++ b/spec/features/snippets/private_snippets_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Private Snippets', :js do +RSpec.describe 'Private Snippets', :js, feature_category: :snippets do let(:user) { create(:user) } let(:private_snippet) { create(:personal_snippet, :repository, :private, author: user) } let(:content) { private_snippet.blobs.first.data.strip! } diff --git a/spec/features/snippets/public_snippets_spec.rb b/spec/features/snippets/public_snippets_spec.rb index 0f27d96d8e9..627a12aa765 100644 --- a/spec/features/snippets/public_snippets_spec.rb +++ b/spec/features/snippets/public_snippets_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Public Snippets', :js do +RSpec.describe 'Public Snippets', :js, feature_category: :snippets do let(:public_snippet) { create(:personal_snippet, :public, :repository) } let(:content) { public_snippet.blobs.first.data.strip! } diff --git a/spec/features/snippets/search_snippets_spec.rb b/spec/features/snippets/search_snippets_spec.rb index d18729d080a..002fe05fcb0 100644 --- a/spec/features/snippets/search_snippets_spec.rb +++ b/spec/features/snippets/search_snippets_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Search Snippets', :js do +RSpec.describe 'Search Snippets', :js, feature_category: :snippets do it 'user searches for snippets by title' do public_snippet = create(:personal_snippet, :public, title: 'Beginning and Middle') private_snippet = create(:personal_snippet, :private, title: 'Middle and End') diff --git a/spec/features/snippets/show_spec.rb b/spec/features/snippets/show_spec.rb index 2103d362f94..6cce60fbef3 100644 --- a/spec/features/snippets/show_spec.rb +++ b/spec/features/snippets/show_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Snippet', :js do +RSpec.describe 'Snippet', :js, feature_category: :snippets do let_it_be(:user) { create(:user) } let_it_be(:snippet) { create(:personal_snippet, :public, :repository, author: user) } diff --git a/spec/features/snippets/spam_snippets_spec.rb b/spec/features/snippets/spam_snippets_spec.rb index 3748a916780..9e74df3ac05 100644 --- a/spec/features/snippets/spam_snippets_spec.rb +++ b/spec/features/snippets/spam_snippets_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe 'snippet editor with spam', skip: "Will be handled in https://gitlab.com/gitlab-org/gitlab/-/issues/217722" do +RSpec.describe 'snippet editor with spam', skip: "Will be handled in https://gitlab.com/gitlab-org/gitlab/-/issues/217722", + feature_category: :snippets do include_context 'includes Spam constants' let_it_be(:user) { create(:user) } diff --git a/spec/features/snippets/user_creates_snippet_spec.rb b/spec/features/snippets/user_creates_snippet_spec.rb index fd95516090a..4aa099e5737 100644 --- a/spec/features/snippets/user_creates_snippet_spec.rb +++ b/spec/features/snippets/user_creates_snippet_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'User creates snippet', :js do +RSpec.describe 'User creates snippet', :js, feature_category: :snippets do include DropzoneHelper include Spec::Support::Helpers::Features::SnippetSpecHelpers diff --git a/spec/features/snippets/user_deletes_snippet_spec.rb b/spec/features/snippets/user_deletes_snippet_spec.rb index e896f7eb25b..23436a8933c 100644 --- a/spec/features/snippets/user_deletes_snippet_spec.rb +++ b/spec/features/snippets/user_deletes_snippet_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'User deletes snippet', :js do +RSpec.describe 'User deletes snippet', :js, feature_category: :snippets do let(:user) { create(:user) } let(:content) { 'puts "test"' } let(:snippet) { create(:personal_snippet, :repository, :public, content: content, author: user) } diff --git a/spec/features/snippets/user_edits_snippet_spec.rb b/spec/features/snippets/user_edits_snippet_spec.rb index a04c59b53d2..561f1cce26b 100644 --- a/spec/features/snippets/user_edits_snippet_spec.rb +++ b/spec/features/snippets/user_edits_snippet_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'User edits snippet', :js do +RSpec.describe 'User edits snippet', :js, feature_category: :snippets do include DropzoneHelper include Spec::Support::Helpers::Features::SnippetSpecHelpers diff --git a/spec/features/snippets/user_snippets_spec.rb b/spec/features/snippets/user_snippets_spec.rb index bb733431b22..9839f8a472a 100644 --- a/spec/features/snippets/user_snippets_spec.rb +++ b/spec/features/snippets/user_snippets_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'User Snippets' do +RSpec.describe 'User Snippets', feature_category: :snippets do let(:author) { create(:user) } let!(:public_snippet) { create(:personal_snippet, :public, author: author, title: "This is a public snippet") } let!(:internal_snippet) { create(:personal_snippet, :internal, author: author, title: "This is an internal snippet") } diff --git a/spec/features/tags/developer_creates_tag_spec.rb b/spec/features/tags/developer_creates_tag_spec.rb index 5657115fb3c..39d34a5ae64 100644 --- a/spec/features/tags/developer_creates_tag_spec.rb +++ b/spec/features/tags/developer_creates_tag_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Developer creates tag' do +RSpec.describe 'Developer creates tag', feature_category: :source_code_management do let(:user) { create(:user) } let(:group) { create(:group) } let(:project) { create(:project, :repository, namespace: group) } diff --git a/spec/features/tags/developer_deletes_tag_spec.rb b/spec/features/tags/developer_deletes_tag_spec.rb index efd4b42c136..76cf3aa691d 100644 --- a/spec/features/tags/developer_deletes_tag_spec.rb +++ b/spec/features/tags/developer_deletes_tag_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Developer deletes tag', :js do +RSpec.describe 'Developer deletes tag', :js, feature_category: :source_code_management do let(:user) { create(:user) } let(:group) { create(:group) } let(:project) { create(:project, :repository, namespace: group) } diff --git a/spec/features/tags/developer_views_tags_spec.rb b/spec/features/tags/developer_views_tags_spec.rb index 57e1f7da04e..0057ff1a5f3 100644 --- a/spec/features/tags/developer_views_tags_spec.rb +++ b/spec/features/tags/developer_views_tags_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Developer views tags' do +RSpec.describe 'Developer views tags', feature_category: :source_code_management do include RepoHelpers let(:user) { create(:user) } diff --git a/spec/features/tags/maintainer_deletes_protected_tag_spec.rb b/spec/features/tags/maintainer_deletes_protected_tag_spec.rb index 0bf9645c2fb..ce518b962cd 100644 --- a/spec/features/tags/maintainer_deletes_protected_tag_spec.rb +++ b/spec/features/tags/maintainer_deletes_protected_tag_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Maintainer deletes protected tag', :js do +RSpec.describe 'Maintainer deletes protected tag', :js, feature_category: :source_code_management do let(:user) { create(:user) } let(:group) { create(:group) } let(:project) { create(:project, :repository, namespace: group) } diff --git a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb index 8daa869a6e3..78cede77fea 100644 --- a/spec/features/uploads/user_uploads_avatar_to_group_spec.rb +++ b/spec/features/uploads/user_uploads_avatar_to_group_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'User uploads avatar to group' do +RSpec.describe 'User uploads avatar to group', feature_category: :users do it 'they see the new avatar' do user = create(:user) group = create(:group) diff --git a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb index 02f9d57fcfe..fb62b5eadc5 100644 --- a/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb +++ b/spec/features/uploads/user_uploads_avatar_to_profile_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'User uploads avatar to profile' do +RSpec.describe 'User uploads avatar to profile', feature_category: :users do let!(:user) { create(:user) } let(:avatar_file_path) { Rails.root.join('spec', 'fixtures', 'dk.png') } diff --git a/spec/features/uploads/user_uploads_file_to_note_spec.rb b/spec/features/uploads/user_uploads_file_to_note_spec.rb index 2547e2d274c..e5ad62592ae 100644 --- a/spec/features/uploads/user_uploads_file_to_note_spec.rb +++ b/spec/features/uploads/user_uploads_file_to_note_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'User uploads file to note' do +RSpec.describe 'User uploads file to note', feature_category: :team_planning do include DropzoneHelper let(:user) { create(:user) } diff --git a/spec/frontend/blob/openapi/index_spec.js b/spec/frontend/blob/openapi/index_spec.js index 17e718df495..d9d65258516 100644 --- a/spec/frontend/blob/openapi/index_spec.js +++ b/spec/frontend/blob/openapi/index_spec.js @@ -21,7 +21,7 @@ describe('OpenAPI blob viewer', () => { it('initializes SwaggerUI with the correct configuration', () => { expect(document.body.innerHTML).toContain( - '<iframe src="/-/sandbox/swagger" sandbox="allow-scripts allow-popups" frameborder="0" width="100%" height="1000"></iframe>', + '<iframe src="/-/sandbox/swagger" sandbox="allow-scripts allow-popups allow-forms" frameborder="0" width="100%" height="1000"></iframe>', ); }); }); diff --git a/spec/frontend/clusters_list/components/clusters_spec.js b/spec/frontend/clusters_list/components/clusters_spec.js index a3f42c1f161..e8e705a6384 100644 --- a/spec/frontend/clusters_list/components/clusters_spec.js +++ b/spec/frontend/clusters_list/components/clusters_spec.js @@ -61,6 +61,10 @@ describe('Clusters', () => { let captureException; beforeEach(() => { + jest.spyOn(Sentry, 'withScope').mockImplementation((fn) => { + const mockScope = { setTag: () => {} }; + fn(mockScope); + }); captureException = jest.spyOn(Sentry, 'captureException'); mock = new MockAdapter(axios); diff --git a/spec/frontend/clusters_list/store/actions_spec.js b/spec/frontend/clusters_list/store/actions_spec.js index 09b1f80ff9b..1deebf8b75a 100644 --- a/spec/frontend/clusters_list/store/actions_spec.js +++ b/spec/frontend/clusters_list/store/actions_spec.js @@ -17,6 +17,10 @@ describe('Clusters store actions', () => { describe('reportSentryError', () => { beforeEach(() => { + jest.spyOn(Sentry, 'withScope').mockImplementation((fn) => { + const mockScope = { setTag: () => {} }; + fn(mockScope); + }); captureException = jest.spyOn(Sentry, 'captureException'); }); diff --git a/spec/frontend/sentry/index_spec.js b/spec/frontend/sentry/index_spec.js index d1f098112e8..2dd528a8a1c 100644 --- a/spec/frontend/sentry/index_spec.js +++ b/spec/frontend/sentry/index_spec.js @@ -1,17 +1,20 @@ import index from '~/sentry/index'; + +import LegacySentryConfig from '~/sentry/legacy_sentry_config'; import SentryConfig from '~/sentry/sentry_config'; -describe('SentryConfig options', () => { +describe('Sentry init', () => { + let originalGon; + const dsn = 'https://123@sentry.gitlab.test/123'; - const currentUserId = 'currentUserId'; - const gitlabUrl = 'gitlabUrl'; const environment = 'test'; + const currentUserId = '1'; + const gitlabUrl = 'gitlabUrl'; const revision = 'revision'; const featureCategory = 'my_feature_category'; - let indexReturnValue; - beforeEach(() => { + originalGon = window.gon; window.gon = { sentry_dsn: dsn, sentry_environment: environment, @@ -21,28 +24,41 @@ describe('SentryConfig options', () => { feature_category: featureCategory, }; - process.env.HEAD_COMMIT_SHA = revision; - + jest.spyOn(LegacySentryConfig, 'init').mockImplementation(); jest.spyOn(SentryConfig, 'init').mockImplementation(); + }); - indexReturnValue = index(); + afterEach(() => { + window.gon = originalGon; }); - it('should init with .sentryDsn, .currentUserId, .whitelistUrls and environment', () => { - expect(SentryConfig.init).toHaveBeenCalledWith({ - dsn, - currentUserId, - whitelistUrls: [gitlabUrl, 'webpack-internal://'], - environment, - release: revision, - tags: { - revision, - feature_category: featureCategory, - }, - }); + it('exports new version of Sentry in the global object', () => { + // eslint-disable-next-line no-underscore-dangle + expect(window._Sentry.SDK_VERSION).not.toMatch(/^5\./); }); - it('should return SentryConfig', () => { - expect(indexReturnValue).toBe(SentryConfig); + describe('when called', () => { + beforeEach(() => { + index(); + }); + + it('configures sentry', () => { + expect(SentryConfig.init).toHaveBeenCalledTimes(1); + expect(SentryConfig.init).toHaveBeenCalledWith({ + dsn, + currentUserId, + allowUrls: [gitlabUrl, 'webpack-internal://'], + environment, + release: revision, + tags: { + revision, + feature_category: featureCategory, + }, + }); + }); + + it('does not configure legacy sentry', () => { + expect(LegacySentryConfig.init).not.toHaveBeenCalled(); + }); }); }); diff --git a/spec/frontend/sentry/legacy_index_spec.js b/spec/frontend/sentry/legacy_index_spec.js new file mode 100644 index 00000000000..5c336f8392e --- /dev/null +++ b/spec/frontend/sentry/legacy_index_spec.js @@ -0,0 +1,64 @@ +import index from '~/sentry/legacy_index'; + +import LegacySentryConfig from '~/sentry/legacy_sentry_config'; +import SentryConfig from '~/sentry/sentry_config'; + +describe('Sentry init', () => { + let originalGon; + + const dsn = 'https://123@sentry.gitlab.test/123'; + const environment = 'test'; + const currentUserId = '1'; + const gitlabUrl = 'gitlabUrl'; + const revision = 'revision'; + const featureCategory = 'my_feature_category'; + + beforeEach(() => { + originalGon = window.gon; + window.gon = { + sentry_dsn: dsn, + sentry_environment: environment, + current_user_id: currentUserId, + gitlab_url: gitlabUrl, + revision, + feature_category: featureCategory, + }; + + jest.spyOn(LegacySentryConfig, 'init').mockImplementation(); + jest.spyOn(SentryConfig, 'init').mockImplementation(); + }); + + afterEach(() => { + window.gon = originalGon; + }); + + it('exports legacy version of Sentry in the global object', () => { + // eslint-disable-next-line no-underscore-dangle + expect(window._Sentry.SDK_VERSION).toMatch(/^5\./); + }); + + describe('when called', () => { + beforeEach(() => { + index(); + }); + + it('configures legacy sentry', () => { + expect(LegacySentryConfig.init).toHaveBeenCalledTimes(1); + expect(LegacySentryConfig.init).toHaveBeenCalledWith({ + dsn, + currentUserId, + whitelistUrls: [gitlabUrl, 'webpack-internal://'], + environment, + release: revision, + tags: { + revision, + feature_category: featureCategory, + }, + }); + }); + + it('does not configure new sentry', () => { + expect(SentryConfig.init).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/spec/frontend/sentry/legacy_sentry_config_spec.js b/spec/frontend/sentry/legacy_sentry_config_spec.js new file mode 100644 index 00000000000..fe90cb49074 --- /dev/null +++ b/spec/frontend/sentry/legacy_sentry_config_spec.js @@ -0,0 +1,215 @@ +import * as Sentry5 from 'sentrybrowser5'; +import LegacySentryConfig from '~/sentry/legacy_sentry_config'; + +describe('LegacySentryConfig', () => { + describe('IGNORE_ERRORS', () => { + it('should be an array of strings', () => { + const areStrings = LegacySentryConfig.IGNORE_ERRORS.every( + (error) => typeof error === 'string', + ); + + expect(areStrings).toBe(true); + }); + }); + + describe('BLACKLIST_URLS', () => { + it('should be an array of regexps', () => { + const areRegExps = LegacySentryConfig.BLACKLIST_URLS.every((url) => url instanceof RegExp); + + expect(areRegExps).toBe(true); + }); + }); + + describe('SAMPLE_RATE', () => { + it('should be a finite number', () => { + expect(typeof LegacySentryConfig.SAMPLE_RATE).toEqual('number'); + }); + }); + + describe('init', () => { + const options = { + currentUserId: 1, + }; + + beforeEach(() => { + jest.spyOn(LegacySentryConfig, 'configure'); + jest.spyOn(LegacySentryConfig, 'bindSentryErrors'); + jest.spyOn(LegacySentryConfig, 'setUser'); + + LegacySentryConfig.init(options); + }); + + it('should set the options property', () => { + expect(LegacySentryConfig.options).toEqual(options); + }); + + it('should call the configure method', () => { + expect(LegacySentryConfig.configure).toHaveBeenCalled(); + }); + + it('should call the error bindings method', () => { + expect(LegacySentryConfig.bindSentryErrors).toHaveBeenCalled(); + }); + + it('should call setUser', () => { + expect(LegacySentryConfig.setUser).toHaveBeenCalled(); + }); + + it('should not call setUser if there is no current user ID', () => { + LegacySentryConfig.setUser.mockClear(); + options.currentUserId = undefined; + + LegacySentryConfig.init(options); + + expect(LegacySentryConfig.setUser).not.toHaveBeenCalled(); + }); + }); + + describe('configure', () => { + const sentryConfig = {}; + const options = { + dsn: 'https://123@sentry.gitlab.test/123', + whitelistUrls: ['//gitlabUrl', 'webpack-internal://'], + environment: 'test', + release: 'revision', + tags: { + revision: 'revision', + feature_category: 'my_feature_category', + }, + }; + + beforeEach(() => { + jest.spyOn(Sentry5, 'init').mockImplementation(); + jest.spyOn(Sentry5, 'setTags').mockImplementation(); + + sentryConfig.options = options; + sentryConfig.IGNORE_ERRORS = 'ignore_errors'; + sentryConfig.BLACKLIST_URLS = 'blacklist_urls'; + + LegacySentryConfig.configure.call(sentryConfig); + }); + + it('should call Sentry5.init', () => { + expect(Sentry5.init).toHaveBeenCalledWith({ + dsn: options.dsn, + release: options.release, + sampleRate: 0.95, + whitelistUrls: options.whitelistUrls, + environment: 'test', + ignoreErrors: sentryConfig.IGNORE_ERRORS, + blacklistUrls: sentryConfig.BLACKLIST_URLS, + }); + }); + + it('should call Sentry5.setTags', () => { + expect(Sentry5.setTags).toHaveBeenCalledWith(options.tags); + }); + + it('should set environment from options', () => { + sentryConfig.options.environment = 'development'; + + LegacySentryConfig.configure.call(sentryConfig); + + expect(Sentry5.init).toHaveBeenCalledWith({ + dsn: options.dsn, + release: options.release, + sampleRate: 0.95, + whitelistUrls: options.whitelistUrls, + environment: 'development', + ignoreErrors: sentryConfig.IGNORE_ERRORS, + blacklistUrls: sentryConfig.BLACKLIST_URLS, + }); + }); + }); + + describe('setUser', () => { + let sentryConfig; + + beforeEach(() => { + sentryConfig = { options: { currentUserId: 1 } }; + jest.spyOn(Sentry5, 'setUser'); + + LegacySentryConfig.setUser.call(sentryConfig); + }); + + it('should call .setUser', () => { + expect(Sentry5.setUser).toHaveBeenCalledWith({ + id: sentryConfig.options.currentUserId, + }); + }); + }); + + describe('handleSentryErrors', () => { + let event; + let req; + let config; + let err; + + beforeEach(() => { + event = {}; + req = { status: 'status', responseText: 'Unknown response text', statusText: 'statusText' }; + config = { type: 'type', url: 'url', data: 'data' }; + err = {}; + + jest.spyOn(Sentry5, 'captureMessage'); + + LegacySentryConfig.handleSentryErrors(event, req, config, err); + }); + + it('should call Sentry5.captureMessage', () => { + expect(Sentry5.captureMessage).toHaveBeenCalledWith(err, { + extra: { + type: config.type, + url: config.url, + data: config.data, + status: req.status, + response: req.responseText, + error: err, + event, + }, + }); + }); + + describe('if no err is provided', () => { + beforeEach(() => { + LegacySentryConfig.handleSentryErrors(event, req, config); + }); + + it('should use req.statusText as the error value', () => { + expect(Sentry5.captureMessage).toHaveBeenCalledWith(req.statusText, { + extra: { + type: config.type, + url: config.url, + data: config.data, + status: req.status, + response: req.responseText, + error: req.statusText, + event, + }, + }); + }); + }); + + describe('if no req.responseText is provided', () => { + beforeEach(() => { + req.responseText = undefined; + + LegacySentryConfig.handleSentryErrors(event, req, config, err); + }); + + it('should use `Unknown response text` as the response', () => { + expect(Sentry5.captureMessage).toHaveBeenCalledWith(err, { + extra: { + type: config.type, + url: config.url, + data: config.data, + status: req.status, + response: 'Unknown response text', + error: err, + event, + }, + }); + }); + }); + }); +}); diff --git a/spec/frontend/sentry/sentry_browser_wrapper_spec.js b/spec/frontend/sentry/sentry_browser_wrapper_spec.js new file mode 100644 index 00000000000..f4d646bab78 --- /dev/null +++ b/spec/frontend/sentry/sentry_browser_wrapper_spec.js @@ -0,0 +1,59 @@ +import * as Sentry from '~/sentry/sentry_browser_wrapper'; + +const mockError = new Error('error!'); +const mockMsg = 'msg!'; +const mockFn = () => {}; + +describe('SentryBrowserWrapper', () => { + afterEach(() => { + // eslint-disable-next-line no-underscore-dangle + delete window._Sentry; + }); + + describe('when _Sentry is not defined', () => { + it('methods fail silently', () => { + expect(() => { + Sentry.captureException(mockError); + Sentry.captureMessage(mockMsg); + Sentry.withScope(mockFn); + }).not.toThrow(); + }); + }); + + describe('when _Sentry is defined', () => { + let mockCaptureException; + let mockCaptureMessage; + let mockWithScope; + + beforeEach(async () => { + mockCaptureException = jest.fn(); + mockCaptureMessage = jest.fn(); + mockWithScope = jest.fn(); + + // eslint-disable-next-line no-underscore-dangle + window._Sentry = { + captureException: mockCaptureException, + captureMessage: mockCaptureMessage, + withScope: mockWithScope, + }; + }); + + it('captureException is called', () => { + Sentry.captureException(mockError); + + expect(mockCaptureException).toHaveBeenCalledWith(mockError); + }); + + it('captureMessage is called', () => { + Sentry.captureMessage(mockMsg); + + expect(mockCaptureMessage).toHaveBeenCalledWith(mockMsg); + }); + + it('withScope is called', () => { + Sentry.withScope(mockFn); + + expect(mockWithScope).toHaveBeenCalledWith(mockFn); + }); + }); +}); diff --git a/spec/frontend/sentry/sentry_config_spec.js b/spec/frontend/sentry/sentry_config_spec.js index 9f67b681b8d..44acbee9b38 100644 --- a/spec/frontend/sentry/sentry_config_spec.js +++ b/spec/frontend/sentry/sentry_config_spec.js @@ -1,29 +1,9 @@ -import * as Sentry from '@sentry/browser'; +import * as Sentry from 'sentrybrowser7'; +import { IGNORE_ERRORS, DENY_URLS, SAMPLE_RATE } from '~/sentry/constants'; + import SentryConfig from '~/sentry/sentry_config'; describe('SentryConfig', () => { - describe('IGNORE_ERRORS', () => { - it('should be an array of strings', () => { - const areStrings = SentryConfig.IGNORE_ERRORS.every((error) => typeof error === 'string'); - - expect(areStrings).toBe(true); - }); - }); - - describe('BLACKLIST_URLS', () => { - it('should be an array of regexps', () => { - const areRegExps = SentryConfig.BLACKLIST_URLS.every((url) => url instanceof RegExp); - - expect(areRegExps).toBe(true); - }); - }); - - describe('SAMPLE_RATE', () => { - it('should be a finite number', () => { - expect(typeof SentryConfig.SAMPLE_RATE).toEqual('number'); - }); - }); - describe('init', () => { const options = { currentUserId: 1, @@ -31,7 +11,6 @@ describe('SentryConfig', () => { beforeEach(() => { jest.spyOn(SentryConfig, 'configure'); - jest.spyOn(SentryConfig, 'bindSentryErrors'); jest.spyOn(SentryConfig, 'setUser'); SentryConfig.init(options); @@ -45,19 +24,13 @@ describe('SentryConfig', () => { expect(SentryConfig.configure).toHaveBeenCalled(); }); - it('should call the error bindings method', () => { - expect(SentryConfig.bindSentryErrors).toHaveBeenCalled(); - }); - it('should call setUser', () => { expect(SentryConfig.setUser).toHaveBeenCalled(); }); it('should not call setUser if there is no current user ID', () => { SentryConfig.setUser.mockClear(); - options.currentUserId = undefined; - - SentryConfig.init(options); + SentryConfig.init({ currentUserId: undefined }); expect(SentryConfig.setUser).not.toHaveBeenCalled(); }); @@ -67,7 +40,7 @@ describe('SentryConfig', () => { const sentryConfig = {}; const options = { dsn: 'https://123@sentry.gitlab.test/123', - whitelistUrls: ['//gitlabUrl', 'webpack-internal://'], + allowUrls: ['//gitlabUrl', 'webpack-internal://'], environment: 'test', release: 'revision', tags: { @@ -81,8 +54,6 @@ describe('SentryConfig', () => { jest.spyOn(Sentry, 'setTags').mockImplementation(); sentryConfig.options = options; - sentryConfig.IGNORE_ERRORS = 'ignore_errors'; - sentryConfig.BLACKLIST_URLS = 'blacklist_urls'; SentryConfig.configure.call(sentryConfig); }); @@ -91,11 +62,11 @@ describe('SentryConfig', () => { expect(Sentry.init).toHaveBeenCalledWith({ dsn: options.dsn, release: options.release, - sampleRate: 0.95, - whitelistUrls: options.whitelistUrls, - environment: 'test', - ignoreErrors: sentryConfig.IGNORE_ERRORS, - blacklistUrls: sentryConfig.BLACKLIST_URLS, + sampleRate: SAMPLE_RATE, + allowUrls: options.allowUrls, + environment: options.environment, + ignoreErrors: IGNORE_ERRORS, + denyUrls: DENY_URLS, }); }); @@ -111,11 +82,11 @@ describe('SentryConfig', () => { expect(Sentry.init).toHaveBeenCalledWith({ dsn: options.dsn, release: options.release, - sampleRate: 0.95, - whitelistUrls: options.whitelistUrls, + sampleRate: SAMPLE_RATE, + allowUrls: options.allowUrls, environment: 'development', - ignoreErrors: sentryConfig.IGNORE_ERRORS, - blacklistUrls: sentryConfig.BLACKLIST_URLS, + ignoreErrors: IGNORE_ERRORS, + denyUrls: DENY_URLS, }); }); }); @@ -136,78 +107,4 @@ describe('SentryConfig', () => { }); }); }); - - describe('handleSentryErrors', () => { - let event; - let req; - let config; - let err; - - beforeEach(() => { - event = {}; - req = { status: 'status', responseText: 'Unknown response text', statusText: 'statusText' }; - config = { type: 'type', url: 'url', data: 'data' }; - err = {}; - - jest.spyOn(Sentry, 'captureMessage'); - - SentryConfig.handleSentryErrors(event, req, config, err); - }); - - it('should call Sentry.captureMessage', () => { - expect(Sentry.captureMessage).toHaveBeenCalledWith(err, { - extra: { - type: config.type, - url: config.url, - data: config.data, - status: req.status, - response: req.responseText, - error: err, - event, - }, - }); - }); - - describe('if no err is provided', () => { - beforeEach(() => { - SentryConfig.handleSentryErrors(event, req, config); - }); - - it('should use req.statusText as the error value', () => { - expect(Sentry.captureMessage).toHaveBeenCalledWith(req.statusText, { - extra: { - type: config.type, - url: config.url, - data: config.data, - status: req.status, - response: req.responseText, - error: req.statusText, - event, - }, - }); - }); - }); - - describe('if no req.responseText is provided', () => { - beforeEach(() => { - req.responseText = undefined; - - SentryConfig.handleSentryErrors(event, req, config, err); - }); - - it('should use `Unknown response text` as the response', () => { - expect(Sentry.captureMessage).toHaveBeenCalledWith(err, { - extra: { - type: config.type, - url: config.url, - data: config.data, - status: req.status, - response: 'Unknown response text', - error: err, - event, - }, - }); - }); - }); - }); }); diff --git a/spec/initializers/diagnostic_reports_spec.rb b/spec/initializers/diagnostic_reports_spec.rb index 2022076072b..20d0b2714f0 100644 --- a/spec/initializers/diagnostic_reports_spec.rb +++ b/spec/initializers/diagnostic_reports_spec.rb @@ -28,10 +28,10 @@ RSpec.describe 'diagnostic reports' do end let(:report_daemon) { instance_double(Gitlab::Memory::ReportsDaemon) } + let(:reporter) { instance_double(Gitlab::Memory::Reporter) } it 'modifies worker startup hooks, starts Gitlab::Memory::ReportsDaemon' do expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_start).and_call_original - expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_stop) expect_next_instance_of(Gitlab::Memory::ReportsDaemon) do |daemon| expect(daemon).to receive(:start) end @@ -39,14 +39,30 @@ RSpec.describe 'diagnostic reports' do load_initializer end - it 'writes scheduled heap dumps in on_worker_stop' do - expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_start) - expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_stop).and_call_original - expect(Gitlab::Memory::Reports::HeapDump).to receive(:write_conditionally) + context 'when GITLAB_MEMWD_DUMP_HEAP is set' do + before do + stub_env('GITLAB_MEMWD_DUMP_HEAP', '1') + end - load_initializer - # This is necessary because this hook normally fires during worker shutdown. - Gitlab::Cluster::LifecycleEvents.do_worker_stop + it 'writes scheduled heap dumps in on_worker_stop' do + expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_start) + expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_stop).and_call_original + expect(Gitlab::Memory::Reporter).to receive(:new).and_return(reporter) + expect(reporter).to receive(:run_report).with(an_instance_of(Gitlab::Memory::Reports::HeapDump)) + + load_initializer + # This is necessary because this hook normally fires during worker shutdown. + Gitlab::Cluster::LifecycleEvents.do_worker_stop + end + end + + context 'when GITLAB_MEMWD_DUMP_HEAP is not set' do + it 'does not write heap dumps' do + expect(Gitlab::Cluster::LifecycleEvents).to receive(:on_worker_start) + expect(Gitlab::Cluster::LifecycleEvents).not_to receive(:on_worker_stop) + + load_initializer + end end end diff --git a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb index aadfb41a46e..88bffd41947 100644 --- a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb +++ b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb @@ -102,13 +102,65 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do end context 'when sentry is configured' do + let(:legacy_dsn) { 'dummy://abc@legacy-sentry.example.com/1' } + let(:dsn) { 'dummy://def@sentry.example.com/2' } + before do - stub_sentry_settings stub_config_setting(host: 'gitlab.example.com') end - it 'adds sentry path to CSP without user' do - expect(directives['connect_src']).to eq("'self' ws://gitlab.example.com dummy://example.com") + context 'when legacy sentry is configured' do + before do + allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) + allow(Gitlab.config.sentry).to receive(:clientside_dsn).and_return(legacy_dsn) + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(false) + end + + it 'adds legacy sentry path to CSP' do + expect(directives['connect_src']).to eq("'self' ws://gitlab.example.com dummy://legacy-sentry.example.com") + end + end + + context 'when sentry is configured' do + before do + allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) + allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_return(dsn) + end + + it 'adds new sentry path to CSP' do + expect(directives['connect_src']).to eq("'self' ws://gitlab.example.com dummy://sentry.example.com") + end + end + + context 'when sentry settings are from older schemas and sentry setting are missing' do + before do + allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) + + allow(Gitlab::CurrentSettings).to receive(:respond_to?).with(:sentry_enabled).and_return(false) + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_raise(NoMethodError) + + allow(Gitlab::CurrentSettings).to receive(:respond_to?).with(:sentry_clientside_dsn).and_return(false) + allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_raise(NoMethodError) + end + + it 'config is backwards compatible, does not add sentry path to CSP' do + expect(directives['connect_src']).to eq("'self' ws://gitlab.example.com") + end + end + + context 'when legacy sentry and sentry are both configured' do + before do + allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) + allow(Gitlab.config.sentry).to receive(:clientside_dsn).and_return(legacy_dsn) + + allow(Gitlab::CurrentSettings).to receive(:sentry_enabled).and_return(true) + allow(Gitlab::CurrentSettings).to receive(:sentry_clientside_dsn).and_return(dsn) + end + + it 'adds both sentry paths to CSP' do + expect(directives['connect_src']).to eq("'self' ws://gitlab.example.com dummy://legacy-sentry.example.com dummy://sentry.example.com") + end end end diff --git a/spec/lib/gitlab/gon_helper_spec.rb b/spec/lib/gitlab/gon_helper_spec.rb index 5a1fcc5e2dc..f8fb998b84d 100644 --- a/spec/lib/gitlab/gon_helper_spec.rb +++ b/spec/lib/gitlab/gon_helper_spec.rb @@ -44,7 +44,7 @@ RSpec.describe Gitlab::GonHelper do let(:clientside_dsn) { 'https://xxx@sentry.example.com/1' } let(:environment) { 'staging' } - describe 'sentry integration' do + describe 'legacy sentry integration' do before do stub_config(sentry: { enabled: true, clientside_dsn: clientside_dsn, environment: environment }) end @@ -57,7 +57,7 @@ RSpec.describe Gitlab::GonHelper do end end - describe 'new sentry integration' do + describe 'sentry integration' do before do stub_application_setting(sentry_enabled: true) stub_application_setting(sentry_clientside_dsn: clientside_dsn) diff --git a/spec/lib/gitlab/memory/jemalloc_spec.rb b/spec/lib/gitlab/memory/jemalloc_spec.rb index 9986af4bc13..8cce2278f8e 100644 --- a/spec/lib/gitlab/memory/jemalloc_spec.rb +++ b/spec/lib/gitlab/memory/jemalloc_spec.rb @@ -1,15 +1,14 @@ # frozen_string_literal: true require 'fast_spec_helper' -require 'tmpdir' +require 'tempfile' RSpec.describe Gitlab::Memory::Jemalloc do - let(:outdir) { Dir.mktmpdir } - let(:tmp_outdir) { Dir.mktmpdir } + let(:outfile) { Tempfile.new } after do - FileUtils.rm_f(outdir) - FileUtils.rm_f(tmp_outdir) + outfile.close + outfile.unlink end context 'when jemalloc is loaded' do @@ -31,12 +30,11 @@ RSpec.describe Gitlab::Memory::Jemalloc do describe '.dump_stats' do it 'writes stats JSON file' do - file_path = described_class.dump_stats(path: outdir, tmp_dir: tmp_outdir, format: format) + described_class.dump_stats(outfile, format: format) - file = Dir.entries(outdir).find { |e| e.match(/jemalloc_stats\..*\.json$/) } - expect(file).not_to be_nil - expect(file_path).to eq(File.join(outdir, file)) - expect(File.read(file_path)).to eq(output) + outfile.rewind + + expect(outfile.read).to eq(output) end end end @@ -56,25 +54,12 @@ RSpec.describe Gitlab::Memory::Jemalloc do end describe '.dump_stats' do - shared_examples 'writes stats text file' do |filename_label, filename_pattern| - it do - described_class.dump_stats( - path: outdir, tmp_dir: tmp_outdir, format: format, filename_label: filename_label) - - file = Dir.entries(outdir).find { |e| e.match(filename_pattern) } - expect(file).not_to be_nil - expect(File.read(File.join(outdir, file))).to eq(output) - end - end + it 'writes stats text file' do + described_class.dump_stats(outfile, format: format) - context 'when custom filename label is passed' do - include_examples 'writes stats text file', - 'puma_0.some-uuid', - /jemalloc_stats\..*\.puma_0\.some-uuid\.txt$/ - end + outfile.rewind - context 'when custom filename label is not passed' do - include_examples 'writes stats text file', nil, /jemalloc_stats\..*\.txt$/ + expect(outfile.read).to eq(output) end end end @@ -93,7 +78,7 @@ RSpec.describe Gitlab::Memory::Jemalloc do describe '.dump_stats' do it 'raises an error' do expect do - described_class.dump_stats(path: outdir, tmp_dir: tmp_outdir, format: format) + described_class.dump_stats(outfile, format: format) end.to raise_error(/format must be one of/) end end @@ -106,18 +91,18 @@ RSpec.describe Gitlab::Memory::Jemalloc do end describe '.stats' do - it 'returns nil' do - expect(described_class.stats).to be_nil + it 'returns empty string' do + expect(described_class.stats).to be_empty end end describe '.dump_stats' do it 'does nothing' do - stub_env('LD_PRELOAD', nil) + described_class.dump_stats(outfile) - described_class.dump_stats(path: outdir, tmp_dir: tmp_outdir) + outfile.rewind - expect(Dir.empty?(outdir)).to be(true) + expect(outfile.read).to be_empty end end end diff --git a/spec/lib/gitlab/memory/reporter_spec.rb b/spec/lib/gitlab/memory/reporter_spec.rb index 7924b37cb4b..ad6e556b3dd 100644 --- a/spec/lib/gitlab/memory/reporter_spec.rb +++ b/spec/lib/gitlab/memory/reporter_spec.rb @@ -3,75 +3,144 @@ require 'spec_helper' RSpec.describe Gitlab::Memory::Reporter, :aggregate_failures do - subject(:reporter) { described_class.new } - let(:fake_report) do Class.new do - attr_reader :did_run - def name 'fake_report' end - def run(report_id) - @did_run = true - '/path/to/report' + def run(writer) + writer << 'I ran' end end end + let(:logger) { instance_double(::Logger) } let(:report) { fake_report.new } - describe '#run_report' do + after do + FileUtils.rm_rf(reports_path) + end + + describe '#run_report', time_travel_to: '2020-02-02 10:30:45 0000' do let(:report_duration_counter) { instance_double(::Prometheus::Client::Counter) } let(:file_size) { 1_000_000 } + let(:report_file) { "#{reports_path}/fake_report.2020-02-02.10:30:45:000.worker_1.abc123" } before do + allow(SecureRandom).to receive(:uuid).and_return('abc123') + allow(Gitlab::Metrics).to receive(:counter).and_return(report_duration_counter) allow(report_duration_counter).to receive(:increment) allow(::Prometheus::PidProvider).to receive(:worker_id).and_return('worker_1') - allow(File).to receive(:size).with('/path/to/report').and_return(file_size) + allow(File).to receive(:size).with(report_file).and_return(file_size) - allow(SecureRandom).to receive(:uuid).and_return('abc123') + allow(logger).to receive(:info) end - it 'runs the given report' do - expect { reporter.run_report(report) }.to change { report.did_run }.from(nil).to(true) - end + shared_examples 'runs and stores reports' do + it 'runs the given report and returns true' do + expect(reporter.run_report(report)).to be(true) - it 'logs duration and other metrics' do - expect(Gitlab::AppLogger).to receive(:info).with( - hash_including( - :duration_s, - :cpu_s, - perf_report_size_bytes: file_size, - message: 'finished', - pid: Process.pid, - worker_id: 'worker_1', - perf_report_worker_uuid: 'abc123', - perf_report: 'fake_report' - )) - - reporter.run_report(report) + expect(File.read(report_file)).to eq('I ran') + end + + it 'logs start and finish event' do + expect(logger).to receive(:info).ordered.with( + hash_including( + message: 'started', + pid: Process.pid, + worker_id: 'worker_1', + perf_report_worker_uuid: 'abc123', + perf_report: 'fake_report' + )) + expect(logger).to receive(:info).ordered.with( + hash_including( + :duration_s, + :cpu_s, + perf_report_file: report_file, + perf_report_size_bytes: file_size, + message: 'finished', + pid: Process.pid, + worker_id: 'worker_1', + perf_report_worker_uuid: 'abc123', + perf_report: 'fake_report' + )) + + reporter.run_report(report) + end + + it 'increments Prometheus duration counter' do + expect(report_duration_counter).to receive(:increment).with({ report: 'fake_report' }, an_instance_of(Float)) + + reporter.run_report(report) + end + + context 'when the report returns invalid file path' do + before do + allow(File).to receive(:size).with(report_file).and_raise(Errno::ENOENT) + end + + it 'logs `0` as `perf_report_size_bytes`' do + expect(logger).to receive(:info).ordered.with( + hash_including(message: 'started') + ) + expect(logger).to receive(:info).ordered.with( + hash_including(message: 'finished', perf_report_size_bytes: 0) + ) + + reporter.run_report(report) + end + end + + context 'when an error occurs' do + before do + allow(report).to receive(:run).and_raise(RuntimeError.new('report failed')) + end + + it 'logs the error and returns false' do + expect(logger).to receive(:info).ordered.with(hash_including(message: 'started')) + expect(logger).to receive(:error).ordered.with( + hash_including( + message: 'failed', error: '#<RuntimeError: report failed>' + )) + + expect(reporter.run_report(report)).to be(false) + end + end end - it 'increments Prometheus duration counter' do - expect(report_duration_counter).to receive(:increment).with({ report: 'fake_report' }, an_instance_of(Float)) + context 'when reports path is specified directly' do + let(:reports_path) { Dir.mktmpdir } + + subject(:reporter) { described_class.new(reports_path: reports_path, logger: logger) } - reporter.run_report(report) + it_behaves_like 'runs and stores reports' end - context 'when the report returns invalid file path' do + context 'when reports path is specified via environment' do + let(:reports_path) { Dir.mktmpdir } + + subject(:reporter) { described_class.new(logger: logger) } + before do - allow(File).to receive(:size).with('/path/to/report').and_raise(Errno::ENOENT) + stub_env('GITLAB_DIAGNOSTIC_REPORTS_PATH', reports_path) end - it 'logs `0` as `perf_report_size_bytes`' do - expect(Gitlab::AppLogger).to receive(:info).with(hash_including(perf_report_size_bytes: 0)) + it_behaves_like 'runs and stores reports' + end - reporter.run_report(report) + context 'when reports path is not specified' do + let(:reports_path) { reporter.reports_path } + + subject(:reporter) { described_class.new(logger: logger) } + + it 'defaults to a temporary location' do + expect(reports_path).not_to be_empty end + + it_behaves_like 'runs and stores reports' end end end diff --git a/spec/lib/gitlab/memory/reports/heap_dump_spec.rb b/spec/lib/gitlab/memory/reports/heap_dump_spec.rb index 2532b8c5295..9cf455b3202 100644 --- a/spec/lib/gitlab/memory/reports/heap_dump_spec.rb +++ b/spec/lib/gitlab/memory/reports/heap_dump_spec.rb @@ -3,20 +3,34 @@ require 'spec_helper' RSpec.describe Gitlab::Memory::Reports::HeapDump do - describe '.write_conditionally' do - subject(:call) { described_class.write_conditionally } + # Copy this class so we do not mess with its state. + let(:klass) { described_class.dup } + + subject(:report) { klass.new } + + describe '#name' do + # This is a bit silly, but it caused code coverage failures. + it 'is set' do + expect(report.name).to eq('heap_dump') + end + end + + describe '#run' do + subject(:run) { report.run(writer) } + + let(:writer) { StringIO.new } context 'when no heap dump is enqueued' do it 'does nothing and returns false' do - expect(call).to be(false) + expect(run).to be(false) end end context 'when a heap dump is enqueued' do it 'does nothing and returns true' do - described_class.enqueue! + klass.enqueue! - expect(call).to be(true) + expect(run).to be(true) end end end diff --git a/spec/lib/gitlab/memory/reports/jemalloc_stats_spec.rb b/spec/lib/gitlab/memory/reports/jemalloc_stats_spec.rb index ee4c6004c52..ce06c270a05 100644 --- a/spec/lib/gitlab/memory/reports/jemalloc_stats_spec.rb +++ b/spec/lib/gitlab/memory/reports/jemalloc_stats_spec.rb @@ -3,96 +3,16 @@ require 'spec_helper' RSpec.describe Gitlab::Memory::Reports::JemallocStats do - let_it_be(:outdir) { Dir.mktmpdir } + subject(:jemalloc_stats) { described_class.new } - let(:jemalloc_stats) { described_class.new(reports_path: outdir) } - - after do - FileUtils.rm_f(outdir) - end + let(:writer) { StringIO.new } describe '.run' do context 'when :report_jemalloc_stats ops FF is enabled' do - let(:worker_id) { 'puma_1' } - let(:report_name) { 'report.json' } - let(:report_path) { File.join(outdir, report_name) } - - before do - allow(Prometheus::PidProvider).to receive(:worker_id).and_return(worker_id) - end - - it 'invokes Jemalloc.dump_stats and returns file path' do - expect(Gitlab::Memory::Jemalloc) - .to receive(:dump_stats) - .with(path: outdir, - tmp_dir: File.join(outdir, '/tmp'), - filename_label: 'test_report') - .and_return(report_path) - - expect(jemalloc_stats.run('test_report')).to eq(report_path) - end - - describe 'reports cleanup' do - let(:jemalloc_stats) { described_class.new(reports_path: outdir) } - - before do - stub_env('GITLAB_DIAGNOSTIC_REPORTS_JEMALLOC_MAX_REPORTS_STORED', 3) - allow(Gitlab::Memory::Jemalloc).to receive(:dump_stats) - end - - context 'when number of reports exceeds `max_reports_stored`' do - let_it_be(:reports) do - now = Time.current - - (1..5).map do |i| - Tempfile.new("jemalloc_stats.#{i}.worker_#{i}.#{Time.current.to_i}.json", outdir).tap do |f| - FileUtils.touch(f, mtime: (now + i.second).to_i) - end - end - end - - after do - reports.each do |f| - f.close - f.unlink - rescue Errno::ENOENT - # Some of the files are already unlinked by the code we test; Ignore - end - end - - it 'keeps only `max_reports_stored` total newest files' do - expect { jemalloc_stats.run('test_report') } - .to change { Dir.entries(outdir).count { |e| e.match(/jemalloc_stats.*/) } } - .from(5).to(3) - - # Keeps only the newest reports - expect(reports.last(3).all? { |r| File.exist?(r) }).to be true - end - end - - context 'when number of reports does not exceed `max_reports_stored`' do - let_it_be(:reports) do - now = Time.current - - (1..3).map do |i| - Tempfile.new("jemalloc_stats.#{i}.worker_#{i}.#{Time.current.to_i}.json", outdir).tap do |f| - FileUtils.touch(f, mtime: (now + i.second).to_i) - end - end - end - - after do - reports.each do |f| - f.close - f.unlink - end - end + it 'dumps jemalloc stats to the given writer' do + expect(Gitlab::Memory::Jemalloc).to receive(:dump_stats).with(writer) - it 'does not remove any reports' do - expect { jemalloc_stats.run('test_report') } - .not_to change { Dir.entries(outdir).count { |e| e.match(/jemalloc_stats.*/) } } - end - end + jemalloc_stats.run(writer) end end @@ -101,10 +21,10 @@ RSpec.describe Gitlab::Memory::Reports::JemallocStats do stub_feature_flags(report_jemalloc_stats: false) end - it 'does not run the report and returns nil' do + it 'does not run the report' do expect(Gitlab::Memory::Jemalloc).not_to receive(:dump_stats) - expect(jemalloc_stats.run('test_report')).to be_nil + jemalloc_stats.run(writer) end end end diff --git a/spec/lib/gitlab/memory/reports_daemon_spec.rb b/spec/lib/gitlab/memory/reports_daemon_spec.rb index b6be4eac919..91c36c87253 100644 --- a/spec/lib/gitlab/memory/reports_daemon_spec.rb +++ b/spec/lib/gitlab/memory/reports_daemon_spec.rb @@ -6,14 +6,8 @@ RSpec.describe Gitlab::Memory::ReportsDaemon, :aggregate_failures do let(:reporter) { instance_double(Gitlab::Memory::Reporter) } let(:reports) { nil } - let_it_be(:tmp_dir) { Dir.mktmpdir } - subject(:daemon) { described_class.new(reporter: reporter, reports: reports) } - after(:all) do - FileUtils.remove_entry(tmp_dir) - end - describe '#run_thread' do before do # make sleep no-op @@ -58,8 +52,9 @@ RSpec.describe Gitlab::Memory::ReportsDaemon, :aggregate_failures do context 'sleep timers logic' do it 'wakes up every (fixed interval + defined delta), sleeps between reports each cycle' do stub_env('GITLAB_DIAGNOSTIC_REPORTS_SLEEP_MAX_DELTA_S', 1) # rand(1) == 0, so we will have fixed sleep interval - daemon = described_class.new + daemon = described_class.new(reporter: reporter, reports: reports) allow(daemon).to receive(:alive).and_return(true, true, false) + allow(reporter).to receive(:run_report) expect(daemon).to receive(:sleep).with(described_class::DEFAULT_SLEEP_S).ordered expect(daemon).to receive(:sleep).with(described_class::DEFAULT_SLEEP_BETWEEN_REPORTS_S).ordered @@ -85,7 +80,6 @@ RSpec.describe Gitlab::Memory::ReportsDaemon, :aggregate_failures do expect(daemon.sleep_s).to eq(described_class::DEFAULT_SLEEP_S) expect(daemon.sleep_max_delta_s).to eq(described_class::DEFAULT_SLEEP_MAX_DELTA_S) expect(daemon.sleep_between_reports_s).to eq(described_class::DEFAULT_SLEEP_BETWEEN_REPORTS_S) - expect(daemon.reports_path).to eq(described_class::DEFAULT_REPORTS_PATH) end end @@ -94,7 +88,6 @@ RSpec.describe Gitlab::Memory::ReportsDaemon, :aggregate_failures do stub_env('GITLAB_DIAGNOSTIC_REPORTS_SLEEP_S', 100) stub_env('GITLAB_DIAGNOSTIC_REPORTS_SLEEP_MAX_DELTA_S', 50) stub_env('GITLAB_DIAGNOSTIC_REPORTS_SLEEP_BETWEEN_REPORTS_S', 2) - stub_env('GITLAB_DIAGNOSTIC_REPORTS_PATH', tmp_dir) end it 'uses provided values' do @@ -103,7 +96,6 @@ RSpec.describe Gitlab::Memory::ReportsDaemon, :aggregate_failures do expect(daemon.sleep_s).to eq(100) expect(daemon.sleep_max_delta_s).to eq(50) expect(daemon.sleep_between_reports_s).to eq(2) - expect(daemon.reports_path).to eq(tmp_dir) end end end diff --git a/spec/lib/gitlab/metrics/rails_slis_spec.rb b/spec/lib/gitlab/metrics/rails_slis_spec.rb index b30eb57101f..9da102fb8b8 100644 --- a/spec/lib/gitlab/metrics/rails_slis_spec.rb +++ b/spec/lib/gitlab/metrics/rails_slis_spec.rb @@ -14,8 +14,8 @@ RSpec.describe Gitlab::Metrics::RailsSlis do end describe '.initialize_request_slis!' do - it "initializes the SLI for all possible endpoints if they weren't", :aggregate_failures do - possible_labels = [ + let(:possible_labels) do + [ { endpoint_id: "GET /api/:version/version", feature_category: :not_owned, @@ -27,17 +27,32 @@ RSpec.describe Gitlab::Metrics::RailsSlis do request_urgency: :default } ] + end - possible_graphql_labels = ['graphql:foo', 'graphql:bar', 'graphql:unknown'].map do |endpoint_id| + let(:possible_graphql_labels) do + ['graphql:foo', 'graphql:bar', 'graphql:unknown'].map do |endpoint_id| { endpoint_id: endpoint_id, feature_category: nil, query_urgency: ::Gitlab::EndpointAttributes::DEFAULT_URGENCY.name } end + end + + it "initializes the SLI for all possible endpoints if they weren't", :aggregate_failures do + expect(Gitlab::Metrics::Sli::Apdex).to receive(:initialize_sli).with(:rails_request, array_including(*possible_labels)).and_call_original + expect(Gitlab::Metrics::Sli::Apdex).to receive(:initialize_sli).with(:graphql_query, array_including(*possible_graphql_labels)).and_call_original + expect(Gitlab::Metrics::Sli::ErrorRate).to receive(:initialize_sli).with(:rails_request, array_including(*possible_labels)).and_call_original + + described_class.initialize_request_slis! + end + + it "initializes the SLI for all possible endpoints if they weren't given error rate feature flag is disabled", :aggregate_failures do + stub_feature_flags(gitlab_metrics_error_rate_sli: false) expect(Gitlab::Metrics::Sli::Apdex).to receive(:initialize_sli).with(:rails_request, array_including(*possible_labels)).and_call_original expect(Gitlab::Metrics::Sli::Apdex).to receive(:initialize_sli).with(:graphql_query, array_including(*possible_graphql_labels)).and_call_original + expect(Gitlab::Metrics::Sli::ErrorRate).not_to receive(:initialize_sli) described_class.initialize_request_slis! end @@ -51,6 +66,14 @@ RSpec.describe Gitlab::Metrics::RailsSlis do end end + describe '.request_error' do + it 'returns the initialized request error rate SLI object' do + described_class.initialize_request_slis! + + expect(described_class.request_error_rate).to be_initialized + end + end + describe '.graphql_query_apdex' do it 'returns the initialized request apdex SLI object' do described_class.initialize_request_slis! diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb index ed78548ef62..56ba880c906 100644 --- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb +++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb @@ -38,6 +38,20 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(described_class).to receive_message_chain(:http_request_duration_seconds, :observe).with({ method: 'get' }, a_positive_execution_time) expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment) .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown', request_urgency: :default }, success: true) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment) + .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown' }, error: false) + + subject.call(env) + end + + it 'does not track error rate when feature flag is disabled' do + stub_feature_flags(gitlab_metrics_error_rate_sli: false) + + expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '200', feature_category: 'unknown') + expect(described_class).to receive_message_chain(:http_request_duration_seconds, :observe).with({ method: 'get' }, a_positive_execution_time) + expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment) + .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown', request_urgency: :default }, success: true) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).not_to receive(:increment) subject.call(env) end @@ -84,10 +98,23 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do context '@app.call returns an error code' do let(:status) { '500' } - it 'tracks count but not duration or apdex' do + it 'tracks count and error rate but not duration and apdex' do expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '500', feature_category: 'unknown') expect(described_class).not_to receive(:http_request_duration_seconds) expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment) + .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown' }, error: true) + + subject.call(env) + end + + it 'does not track error rate when feature flag is disabled' do + stub_feature_flags(gitlab_metrics_error_rate_sli: false) + + expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '500', feature_category: 'unknown') + expect(described_class).not_to receive(:http_request_duration_seconds) + expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).not_to receive(:increment) subject.call(env) end @@ -108,6 +135,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: 'undefined', feature_category: 'unknown') expect(described_class.http_request_duration_seconds).not_to receive(:observe) expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).not_to receive(:increment) expect { subject.call(env) }.to raise_error(StandardError) end @@ -124,6 +152,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(described_class).not_to receive(:http_health_requests_total) expect(Gitlab::Metrics::RailsSlis.request_apdex) .to receive(:increment).with(labels: { feature_category: 'team_planning', endpoint_id: 'IssuesController#show', request_urgency: :default }, success: true) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment) + .with(labels: { feature_category: 'team_planning', endpoint_id: 'IssuesController#show' }, error: false) subject.call(env) end @@ -134,6 +164,7 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(described_class).to receive_message_chain(:http_health_requests_total, :increment).with(method: 'get', status: '200') expect(described_class).not_to receive(:http_requests_total) expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) + expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_error_rate) subject.call(env) end @@ -147,8 +178,9 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do it 'adds the feature category to the labels for http_requests_total' do expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: 'undefined', feature_category: 'team_planning') - expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) + expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_apdex) + expect(Gitlab::Metrics::RailsSlis).not_to receive(:request_error_rate) expect { subject.call(env) }.to raise_error(StandardError) end end @@ -159,6 +191,8 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do expect(described_class).not_to receive(:http_health_requests_total) expect(Gitlab::Metrics::RailsSlis.request_apdex).to receive(:increment) .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown', request_urgency: :default }, success: true) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment) + .with(labels: { feature_category: 'unknown', endpoint_id: 'unknown' }, error: false) subject.call(env) end @@ -214,6 +248,14 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do }, success: success ) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( + labels: { + feature_category: 'hello_world', + endpoint_id: 'GET /projects/:id/archive' + }, + error: false + ) + subject.call(env) end end @@ -247,6 +289,14 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do }, success: success ) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( + labels: { + feature_category: 'hello_world', + endpoint_id: 'AnonymousController#index' + }, + error: false + ) + subject.call(env) end end @@ -273,6 +323,13 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do }, success: true ) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( + labels: { + feature_category: 'unknown', + endpoint_id: 'unknown' + }, + error: false + ) subject.call(env) allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 101) @@ -284,6 +341,13 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do }, success: false ) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( + labels: { + feature_category: 'unknown', + endpoint_id: 'unknown' + }, + error: false + ) subject.call(env) end end @@ -307,6 +371,13 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do }, success: true ) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( + labels: { + feature_category: 'unknown', + endpoint_id: 'unknown' + }, + error: false + ) subject.call(env) allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 101) @@ -318,6 +389,13 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do }, success: false ) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( + labels: { + feature_category: 'unknown', + endpoint_id: 'unknown' + }, + error: false + ) subject.call(env) end end @@ -337,6 +415,13 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do }, success: true ) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( + labels: { + feature_category: 'unknown', + endpoint_id: 'unknown' + }, + error: false + ) subject.call(env) allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(100, 101) @@ -348,6 +433,13 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do }, success: false ) + expect(Gitlab::Metrics::RailsSlis.request_error_rate).to receive(:increment).with( + labels: { + feature_category: 'unknown', + endpoint_id: 'unknown' + }, + error: false + ) subject.call(env) end end diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb index 366843a4c03..dbd6c07ef75 100644 --- a/spec/lib/gitlab/metrics_spec.rb +++ b/spec/lib/gitlab/metrics_spec.rb @@ -101,14 +101,32 @@ RSpec.describe Gitlab::Metrics do 401 | true nil | false 500 | false - 503 | false - '100' | false - '201' | true + 503 | false 'nothing' | false end with_them do specify { expect(described_class.record_duration_for_status?(status)).to be(should_record) } + specify { expect(described_class.record_duration_for_status?(status.to_s)).to be(should_record) } + end + end + + describe '.server_error?' do + using RSpec::Parameterized::TableSyntax + + where(:status, :should_record) do + 100 | false + 200 | false + 401 | false + 500 | true + 503 | true + nil | false + 'nothing' | false + end + + with_them do + specify { expect(described_class.server_error?(status)).to be(should_record) } + specify { expect(described_class.server_error?(status.to_s)).to be(should_record) } end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c03bb61f1f8..52d35f6d79d 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1753,8 +1753,8 @@ RSpec.describe Ci::Build do end end - describe '#starts_environment?' do - subject { build.starts_environment? } + describe '#deployment_job?' do + subject { build.deployment_job? } context 'when environment is defined' do before do diff --git a/spec/requests/api/container_repositories_spec.rb b/spec/requests/api/container_repositories_spec.rb index 90f0243dbfc..0f0c88bef74 100644 --- a/spec/requests/api/container_repositories_spec.rb +++ b/spec/requests/api/container_repositories_spec.rb @@ -75,6 +75,13 @@ RSpec.describe API::ContainerRepositories do expect(json_response['id']).to eq(repository.id) expect(response.body).to include('tags') + expect(json_response['tags']).to eq(repository.tags.map do |tag| + { + "location" => tag.location, + "name" => tag.name, + "path" => tag.path + } + end) end context 'with a network error' do diff --git a/spec/requests/jira_connect/subscriptions_controller_spec.rb b/spec/requests/jira_connect/subscriptions_controller_spec.rb index 73a825d471a..1a47c9d8139 100644 --- a/spec/requests/jira_connect/subscriptions_controller_spec.rb +++ b/spec/requests/jira_connect/subscriptions_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JiraConnect::SubscriptionsController do +RSpec.describe JiraConnect::SubscriptionsController, feature_category: :integrations do describe 'GET /-/jira_connect/subscriptions' do let_it_be(:installation) { create(:jira_connect_installation, instance_url: 'http://self-managed-gitlab.com') } let(:qsh) do |