diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-04 15:10:25 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-06-04 15:10:25 +0000 |
commit | d4194db620cc5b736bb5737ed5e4eab979ccd7ab (patch) | |
tree | 2aab61db9bde950c45f93f43f4033231fb956528 /spec | |
parent | c80a1141e306596202f694b101bfb1aab1864de9 (diff) | |
download | gitlab-ce-d4194db620cc5b736bb5737ed5e4eab979ccd7ab.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
25 files changed, 857 insertions, 368 deletions
diff --git a/spec/controllers/import/bulk_imports_controller_spec.rb b/spec/controllers/import/bulk_imports_controller_spec.rb index b450318f6f7..12aa1d89ecc 100644 --- a/spec/controllers/import/bulk_imports_controller_spec.rb +++ b/spec/controllers/import/bulk_imports_controller_spec.rb @@ -73,7 +73,7 @@ RSpec.describe Import::BulkImportsController do let(:client_params) do { top_level_only: true, - min_access_level: Gitlab::Access::MAINTAINER + min_access_level: Gitlab::Access::OWNER } end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 989f941caea..4fcb63ac616 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -69,6 +69,20 @@ RSpec.describe Projects::MergeRequests::DiffsController do end end + shared_examples 'show the right diff files with previous diff_id' do + context 'with previous diff_id' do + let!(:merge_request_diff_1) { merge_request.merge_request_diffs.create!(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') } + let!(:merge_request_diff_2) { merge_request.merge_request_diffs.create!(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e', diff_type: :merge_head) } + + subject { go(diff_id: merge_request_diff_1.id, diff_head: true) } + + it 'shows the right diff files' do + subject + expect(json_response["diff_files"].size).to eq(merge_request_diff_1.files_count) + end + end + end + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } @@ -142,6 +156,8 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like '404 for unexistent diffable' + it_behaves_like 'show the right diff files with previous diff_id' + context 'when not authorized' do let(:another_user) { create(:user) } @@ -480,6 +496,8 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like '404 for unexistent diffable' + it_behaves_like 'show the right diff files with previous diff_id' + context 'when not authorized' do let(:other_user) { create(:user) } @@ -499,7 +517,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'serializes diffs with expected arguments' do let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch } - let(:expected_options) { collection_arguments(current_page: 1, total_pages: 1) } + let(:expected_options) { collection_arguments(current_page: 1, total_pages: 1).merge(merge_ref_head_diff: false) } end it_behaves_like 'successful request' @@ -522,7 +540,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'serializes diffs with expected arguments' do let(:collection) { Gitlab::Diff::FileCollection::Compare } - let(:expected_options) { collection_arguments } + let(:expected_options) { collection_arguments.merge(merge_ref_head_diff: false) } end it_behaves_like 'successful request' diff --git a/spec/features/groups/import_export/connect_instance_spec.rb b/spec/features/groups/import_export/connect_instance_spec.rb index a281180c54f..563c8f429f8 100644 --- a/spec/features/groups/import_export/connect_instance_spec.rb +++ b/spec/features/groups/import_export/connect_instance_spec.rb @@ -24,7 +24,7 @@ RSpec.describe 'Import/Export - Connect to another instance', :js do pat = 'demo-pat' stub_path = 'stub-group' total = 37 - stub_request(:get, "%{url}/api/v4/groups?page=1&per_page=20&top_level_only=true&min_access_level=40&search=" % { url: source_url }).to_return( + stub_request(:get, "%{url}/api/v4/groups?page=1&per_page=20&top_level_only=true&min_access_level=50&search=" % { url: source_url }).to_return( body: [{ id: 2595438, web_url: 'https://gitlab.com/groups/auto-breakfast', diff --git a/spec/features/markdown/mermaid_spec.rb b/spec/features/markdown/mermaid_spec.rb index 207678e07c3..75c1d03b638 100644 --- a/spec/features/markdown/mermaid_spec.rb +++ b/spec/features/markdown/mermaid_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe 'Mermaid rendering', :js do + let_it_be(:project) { create(:project, :public) } + it 'renders Mermaid diagrams correctly' do description = <<~MERMAID ```mermaid @@ -14,7 +16,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -36,7 +37,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -44,10 +44,33 @@ RSpec.describe 'Mermaid rendering', :js do wait_for_requests wait_for_mermaid - expected = '<text style=""><tspan xml:space="preserve" dy="1em" x="1">Line 1</tspan><tspan xml:space="preserve" dy="1em" x="1">Line 2</tspan></text>' + # From https://github.com/mermaid-js/mermaid/blob/d3f8f03a7d03a052e1fe0251d5a6d8d1f48d67ee/src/dagre-wrapper/createLabel.js#L79-L82 + expected = %(<div xmlns="http://www.w3.org/1999/xhtml" style="display: inline-block; white-space: nowrap;">Line 1<br>Line 2</div>) expect(page.html.scan(expected).count).to be(4) end + it 'does not allow XSS in HTML labels' do + description = <<~MERMAID + ```mermaid + graph LR; + A-->CLICK_HERE_AND_GET_BONUS; + click A alert "aaa" + click CLICK_HERE_AND_GET_BONUS "javascript:alert%28%64%6f%63%75%6d%65%6e%74%2e%64%6f%6d%61%69%6e%29" "Here is the XSS" + ``` + MERMAID + + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + wait_for_requests + wait_for_mermaid + + # From https://github.com/mermaid-js/mermaid/blob/d3f8f03a7d03a052e1fe0251d5a6d8d1f48d67ee/src/dagre-wrapper/createLabel.js#L79-L82 + expected = %(<div xmlns="http://www.w3.org/1999/xhtml" style="display: inline-block; white-space: nowrap;">CLICK_HERE_AND_GET_BONUS</div>) + expect(page.html).to include(expected) + end + it 'renders only 2 Mermaid blocks and', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/234081' do description = <<~MERMAID ```mermaid @@ -64,7 +87,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -94,7 +116,6 @@ RSpec.describe 'Mermaid rendering', :js do </details> MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -108,7 +129,37 @@ RSpec.describe 'Mermaid rendering', :js do expect(svg[:style]).to match(/max-width/) expect(svg[:width].to_i).to eq(100) - expect(svg[:height].to_i).to be_within(5).of(220) + expect(svg[:height].to_i).to be_within(5).of(236) + end + end + + it 'renders V2 state diagrams' do + description = <<~MERMAID + ```mermaid + stateDiagram-v2 + [*] --> Idle + Idle --> Active : CONTINUE + state Active { + [*] --> Run + Run--> Stop: CONTINUE + Stop--> Run: CONTINUE + + Run: Run + Run: entry/start + Run: check + } + ``` + MERMAID + + issue = create(:issue, project: project, description: description) + + visit project_issue_path(project, issue) + + wait_for_requests + wait_for_mermaid + + page.within('.description') do + expect(page).to have_selector('svg') end end @@ -123,7 +174,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -144,7 +194,6 @@ RSpec.describe 'Mermaid rendering', :js do ``` MERMAID - project = create(:project, :public) issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) @@ -183,8 +232,6 @@ RSpec.describe 'Mermaid rendering', :js do description *= 51 - project = create(:project, :public) - issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) diff --git a/spec/features/security/project/snippet/private_access_spec.rb b/spec/features/security/project/snippet/private_access_spec.rb index 0f7ae06a6c5..0c06841399d 100644 --- a/spec/features/security/project/snippet/private_access_spec.rb +++ b/spec/features/security/project/snippet/private_access_spec.rb @@ -5,23 +5,25 @@ require 'spec_helper' RSpec.describe "Private Project Snippets Access" do include AccessMatchers - let(:project) { create(:project, :private) } - - let(:private_snippet) { create(:project_snippet, :private, project: project, author: project.owner) } + let_it_be(:project) { create(:project, :private) } + let_it_be(:private_snippet) { create(:project_snippet, :private, project: project, author: project.owner) } describe "GET /:project_path/snippets" do subject { project_snippets_path(project) } it('is allowed for admin when admin mode is enabled', :enable_admin_mode) { is_expected.to be_allowed_for(:admin) } it('is denied for admin when admin mode is disabled') { is_expected.to be_denied_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:maintainer).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_allowed_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } + + specify :aggregate_failures do + is_expected.to be_allowed_for(:owner).of(project) + is_expected.to be_allowed_for(:maintainer).of(project) + is_expected.to be_allowed_for(:developer).of(project) + is_expected.to be_allowed_for(:reporter).of(project) + is_expected.to be_allowed_for(:guest).of(project) + is_expected.to be_denied_for(:user) + is_expected.to be_denied_for(:external) + is_expected.to be_denied_for(:visitor) + end end describe "GET /:project_path/snippets/new" do @@ -29,14 +31,17 @@ RSpec.describe "Private Project Snippets Access" do it('is allowed for admin when admin mode is enabled', :enable_admin_mode) { is_expected.to be_allowed_for(:admin) } it('is denied for admin when admin mode is disabled') { is_expected.to be_denied_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:maintainer).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_denied_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } + + specify :aggregate_failures do + is_expected.to be_allowed_for(:maintainer).of(project) + is_expected.to be_allowed_for(:owner).of(project) + is_expected.to be_allowed_for(:developer).of(project) + is_expected.to be_allowed_for(:reporter).of(project) + is_expected.to be_denied_for(:guest).of(project) + is_expected.to be_denied_for(:user) + is_expected.to be_denied_for(:external) + is_expected.to be_denied_for(:visitor) + end end describe "GET /:project_path/snippets/:id for a private snippet" do @@ -44,14 +49,17 @@ RSpec.describe "Private Project Snippets Access" do it('is allowed for admin when admin mode is enabled', :enable_admin_mode) { is_expected.to be_allowed_for(:admin) } it('is denied for admin when admin mode is disabled') { is_expected.to be_denied_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:maintainer).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_allowed_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } + + specify :aggregate_failures do + is_expected.to be_allowed_for(:owner).of(project) + is_expected.to be_allowed_for(:maintainer).of(project) + is_expected.to be_allowed_for(:developer).of(project) + is_expected.to be_allowed_for(:reporter).of(project) + is_expected.to be_allowed_for(:guest).of(project) + is_expected.to be_denied_for(:user) + is_expected.to be_denied_for(:external) + is_expected.to be_denied_for(:visitor) + end end describe "GET /:project_path/snippets/:id/raw for a private snippet" do @@ -59,13 +67,16 @@ RSpec.describe "Private Project Snippets Access" do it('is allowed for admin when admin mode is enabled', :enable_admin_mode) { is_expected.to be_allowed_for(:admin) } it('is denied for admin when admin mode is disabled') { is_expected.to be_denied_for(:admin) } - it { is_expected.to be_allowed_for(:owner).of(project) } - it { is_expected.to be_allowed_for(:maintainer).of(project) } - it { is_expected.to be_allowed_for(:developer).of(project) } - it { is_expected.to be_allowed_for(:reporter).of(project) } - it { is_expected.to be_allowed_for(:guest).of(project) } - it { is_expected.to be_denied_for(:user) } - it { is_expected.to be_denied_for(:external) } - it { is_expected.to be_denied_for(:visitor) } + + specify :aggregate_failures do + is_expected.to be_allowed_for(:owner).of(project) + is_expected.to be_allowed_for(:maintainer).of(project) + is_expected.to be_allowed_for(:developer).of(project) + is_expected.to be_allowed_for(:reporter).of(project) + is_expected.to be_allowed_for(:guest).of(project) + is_expected.to be_denied_for(:user) + is_expected.to be_denied_for(:external) + is_expected.to be_denied_for(:visitor) + end end end diff --git a/spec/frontend/pages/shared/nav/sidebar_tracking_spec.js b/spec/frontend/pages/shared/nav/sidebar_tracking_spec.js new file mode 100644 index 00000000000..2c8eb8e459f --- /dev/null +++ b/spec/frontend/pages/shared/nav/sidebar_tracking_spec.js @@ -0,0 +1,160 @@ +import { setHTMLFixture } from 'helpers/fixtures'; +import { initSidebarTracking } from '~/pages/shared/nav/sidebar_tracking'; + +describe('~/pages/shared/nav/sidebar_tracking.js', () => { + beforeEach(() => { + setHTMLFixture(` + <aside class="nav-sidebar"> + <div class="nav-sidebar-inner-scroll"> + <ul class="sidebar-top-level-items"> + <li data-track-label="project_information_menu" class="home"> + <a aria-label="Project information" class="shortcuts-project-information has-sub-items" href=""> + <span class="nav-icon-container"> + <svg class="s16" data-testid="project-icon"> + <use xlink:href="/assets/icons-1b2dadc4c3d49797908ba67b8f10da5d63dd15d859bde28d66fb60bbb97a4dd5.svg#project"></use> + </svg> + </span> + <span class="nav-item-name">Project information</span> + </a> + <ul class="sidebar-sub-level-items"> + <li class="fly-out-top-item"> + <a aria-label="Project information" href="#"> + <strong class="fly-out-top-item-name">Project information</strong> + </a> + </li> + <li class="divider fly-out-top-item"></li> + <li data-track-label="activity" class=""> + <a aria-label="Activity" class="shortcuts-project-activity" href=#"> + <span>Activity</span> + </a> + </li> + <li data-track-label="labels" class=""> + <a aria-label="Labels" href="#"> + <span>Labels</span> + </a> + </li> + <li data-track-label="members" class=""> + <a aria-label="Members" href="#"> + <span>Members</span> + </a> + </li> + </ul> + </li> + </ul> + </div> + </aside> + `); + + initSidebarTracking(); + }); + + describe('sidebar is not collapsed', () => { + describe('menu is not expanded', () => { + it('sets the proper data tracking attributes when clicking on menu', () => { + const menu = document.querySelector('li[data-track-label="project_information_menu"]'); + const menuLink = menu.querySelector('a'); + + menu.classList.add('is-over', 'is-showing-fly-out'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu', + trackExtra: JSON.stringify({ + sidebar_display: 'Expanded', + menu_display: 'Fly out', + }), + }); + }); + + it('sets the proper data tracking attributes when clicking on submenu', () => { + const menu = document.querySelector('li[data-track-label="activity"]'); + const menuLink = menu.querySelector('a'); + const submenuList = document.querySelector('ul.sidebar-sub-level-items'); + + submenuList.classList.add('fly-out-list'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu_item', + trackExtra: JSON.stringify({ + sidebar_display: 'Expanded', + menu_display: 'Fly out', + }), + }); + }); + }); + + describe('menu is expanded', () => { + it('sets the proper data tracking attributes when clicking on menu', () => { + const menu = document.querySelector('li[data-track-label="project_information_menu"]'); + const menuLink = menu.querySelector('a'); + + menu.classList.add('active'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu', + trackExtra: JSON.stringify({ + sidebar_display: 'Expanded', + menu_display: 'Expanded', + }), + }); + }); + + it('sets the proper data tracking attributes when clicking on submenu', () => { + const menu = document.querySelector('li[data-track-label="activity"]'); + const menuLink = menu.querySelector('a'); + + menu.classList.add('active'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu_item', + trackExtra: JSON.stringify({ + sidebar_display: 'Expanded', + menu_display: 'Expanded', + }), + }); + }); + }); + }); + + describe('sidebar is collapsed', () => { + beforeEach(() => { + document.querySelector('aside.nav-sidebar').classList.add('js-sidebar-collapsed'); + }); + + it('sets the proper data tracking attributes when clicking on menu', () => { + const menu = document.querySelector('li[data-track-label="project_information_menu"]'); + const menuLink = menu.querySelector('a'); + + menu.classList.add('is-over', 'is-showing-fly-out'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu', + trackExtra: JSON.stringify({ + sidebar_display: 'Collapsed', + menu_display: 'Fly out', + }), + }); + }); + + it('sets the proper data tracking attributes when clicking on submenu', () => { + const menu = document.querySelector('li[data-track-label="activity"]'); + const menuLink = menu.querySelector('a'); + const submenuList = document.querySelector('ul.sidebar-sub-level-items'); + + submenuList.classList.add('fly-out-list'); + menuLink.click(); + + expect(menu.dataset).toMatchObject({ + trackAction: 'click_menu_item', + trackExtra: JSON.stringify({ + sidebar_display: 'Collapsed', + menu_display: 'Fly out', + }), + }); + }); + }); +}); diff --git a/spec/frontend/pipelines/graph_shared/__snapshots__/links_inner_spec.js.snap b/spec/frontend/pipelines/graph_shared/__snapshots__/links_inner_spec.js.snap index c67b91ae190..16c28791514 100644 --- a/spec/frontend/pipelines/graph_shared/__snapshots__/links_inner_spec.js.snap +++ b/spec/frontend/pipelines/graph_shared/__snapshots__/links_inner_spec.js.snap @@ -21,3 +21,10 @@ exports[`Links Inner component with one need matches snapshot and has expected p <path d=\\"M202,118L42,118C72,118,72,138,102,138\\" stroke-width=\\"2\\" class=\\"gl-fill-transparent gl-transition-duration-slow gl-transition-timing-function-ease gl-stroke-gray-200\\"></path> </svg> </div>" `; + +exports[`Links Inner component with same stage needs matches snapshot and has expected path 1`] = ` +"<div class=\\"gl-display-flex gl-relative\\" totalgroups=\\"10\\"><svg id=\\"link-svg\\" viewBox=\\"0,0,1019,445\\" width=\\"1019px\\" height=\\"445px\\" class=\\"gl-absolute gl-pointer-events-none\\"> + <path d=\\"M192,108L22,108C52,108,52,118,82,118\\" stroke-width=\\"2\\" class=\\"gl-fill-transparent gl-transition-duration-slow gl-transition-timing-function-ease gl-stroke-gray-200\\"></path> + <path d=\\"M202,118L32,118C62,118,62,128,92,128\\" stroke-width=\\"2\\" class=\\"gl-fill-transparent gl-transition-duration-slow gl-transition-timing-function-ease gl-stroke-gray-200\\"></path> + </svg> </div>" +`; diff --git a/spec/frontend/pipelines/graph_shared/links_inner_spec.js b/spec/frontend/pipelines/graph_shared/links_inner_spec.js index bb1f0965469..8f39c8c2405 100644 --- a/spec/frontend/pipelines/graph_shared/links_inner_spec.js +++ b/spec/frontend/pipelines/graph_shared/links_inner_spec.js @@ -10,6 +10,7 @@ import { pipelineData, pipelineDataWithNoNeeds, rootRect, + sameStageNeeds, } from '../pipeline_graph/mock_data'; describe('Links Inner component', () => { @@ -40,7 +41,7 @@ describe('Links Inner component', () => { // We create fixture so that each job has an empty div that represent // the JobPill in the DOM. Each `JobPill` would have different coordinates, - // so we increment their coordinates on each iteration to simulat different positions. + // so we increment their coordinates on each iteration to simulate different positions. const setFixtures = ({ stages }) => { const jobs = createJobsHash(stages); const arrayOfJobs = Object.keys(jobs); @@ -81,7 +82,6 @@ describe('Links Inner component', () => { afterEach(() => { jest.restoreAllMocks(); wrapper.destroy(); - wrapper = null; }); describe('basic SVG creation', () => { @@ -160,6 +160,25 @@ describe('Links Inner component', () => { }); }); + describe('with same stage needs', () => { + beforeEach(() => { + setFixtures(sameStageNeeds); + createComponent({ pipelineData: sameStageNeeds.stages }); + }); + + it('renders the correct number of links', () => { + expect(findAllLinksPath()).toHaveLength(2); + }); + + it('path does not contain NaN values', () => { + expect(wrapper.html()).not.toContain('NaN'); + }); + + it('matches snapshot and has expected path', () => { + expect(wrapper.html()).toMatchSnapshot(); + }); + }); + describe('with a large number of needs', () => { beforeEach(() => { setFixtures(largePipelineData); diff --git a/spec/frontend/pipelines/notification/pipeline_notification_spec.js b/spec/frontend/pipelines/notification/pipeline_notification_spec.js deleted file mode 100644 index 79aa337ba9d..00000000000 --- a/spec/frontend/pipelines/notification/pipeline_notification_spec.js +++ /dev/null @@ -1,79 +0,0 @@ -import { GlBanner } from '@gitlab/ui'; -import { createLocalVue, shallowMount } from '@vue/test-utils'; -import { nextTick } from 'vue'; -import VueApollo from 'vue-apollo'; -import createMockApollo from 'helpers/mock_apollo_helper'; -import PipelineNotification from '~/pipelines/components/notification/pipeline_notification.vue'; -import getUserCallouts from '~/pipelines/graphql/queries/get_user_callouts.query.graphql'; - -describe('Pipeline notification', () => { - const localVue = createLocalVue(); - - let wrapper; - const dagDocPath = 'my/dag/path'; - - const createWrapper = (apolloProvider) => { - return shallowMount(PipelineNotification, { - localVue, - provide: { - dagDocPath, - }, - apolloProvider, - }); - }; - - const createWrapperWithApollo = async ({ callouts = [], isLoading = false } = {}) => { - localVue.use(VueApollo); - - const mappedCallouts = callouts.map((callout) => { - return { featureName: callout, __typename: 'UserCallout' }; - }); - - const mockCalloutsResponse = { - data: { - currentUser: { - id: 45, - __typename: 'User', - callouts: { - id: 5, - __typename: 'UserCalloutConnection', - nodes: mappedCallouts, - }, - }, - }, - }; - const getUserCalloutsHandler = jest.fn().mockResolvedValue(mockCalloutsResponse); - const requestHandlers = [[getUserCallouts, getUserCalloutsHandler]]; - - const apolloWrapper = createWrapper(createMockApollo(requestHandlers)); - if (!isLoading) { - await nextTick(); - } - - return apolloWrapper; - }; - - const findBanner = () => wrapper.findComponent(GlBanner); - - afterEach(() => { - wrapper.destroy(); - }); - - it('shows the banner if the user has never seen it', async () => { - wrapper = await createWrapperWithApollo({ callouts: ['random'] }); - - expect(findBanner().exists()).toBe(true); - }); - - it('does not show the banner while the user callout query is loading', async () => { - wrapper = await createWrapperWithApollo({ callouts: ['random'], isLoading: true }); - - expect(findBanner().exists()).toBe(false); - }); - - it('does not show the banner if the user has previously dismissed it', async () => { - wrapper = await createWrapperWithApollo({ callouts: ['pipeline_needs_banner'.toUpperCase()] }); - - expect(findBanner().exists()).toBe(false); - }); -}); diff --git a/spec/frontend/pipelines/pipeline_graph/mock_data.js b/spec/frontend/pipelines/pipeline_graph/mock_data.js index a79917bfd48..db77e0a0573 100644 --- a/spec/frontend/pipelines/pipeline_graph/mock_data.js +++ b/spec/frontend/pipelines/pipeline_graph/mock_data.js @@ -162,6 +162,38 @@ export const parallelNeedData = { ], }; +export const sameStageNeeds = { + stages: [ + { + name: 'build', + groups: [ + { + name: 'build_1', + jobs: [{ script: 'echo hello', stage: 'build', name: 'build_1' }], + }, + ], + }, + { + name: 'build', + groups: [ + { + name: 'build_2', + jobs: [{ script: 'yarn test', stage: 'build', needs: ['build_1'] }], + }, + ], + }, + { + name: 'build', + groups: [ + { + name: 'build_3', + jobs: [{ script: 'yarn test', stage: 'build', needs: ['build_2'] }], + }, + ], + }, + ], +}; + export const largePipelineData = { stages: [ { diff --git a/spec/lib/gitlab/error_tracking/context_payload_generator_spec.rb b/spec/lib/gitlab/error_tracking/context_payload_generator_spec.rb index dffa7e6522e..0e72dd7ec5e 100644 --- a/spec/lib/gitlab/error_tracking/context_payload_generator_spec.rb +++ b/spec/lib/gitlab/error_tracking/context_payload_generator_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'spec_helper' +require 'fast_spec_helper' require 'rspec-parameterized' RSpec.describe Gitlab::ErrorTracking::ContextPayloadGenerator do diff --git a/spec/lib/gitlab/error_tracking/log_formatter_spec.rb b/spec/lib/gitlab/error_tracking/log_formatter_spec.rb index c97aebf5cd3..188ccd000a1 100644 --- a/spec/lib/gitlab/error_tracking/log_formatter_spec.rb +++ b/spec/lib/gitlab/error_tracking/log_formatter_spec.rb @@ -27,9 +27,9 @@ RSpec.describe Gitlab::ErrorTracking::LogFormatter do end before do - Sentry.set_user(user_flag: 'flag') - Sentry.set_tags(shard: 'catchall') - Sentry.set_extras(some_info: 'info') + Raven.context.user[:user_flag] = 'flag' + Raven.context.tags[:shard] = 'catchall' + Raven.context.extra[:some_info] = 'info' allow(exception).to receive(:backtrace).and_return( [ @@ -40,7 +40,7 @@ RSpec.describe Gitlab::ErrorTracking::LogFormatter do end after do - ::Sentry.get_current_scope.clear + ::Raven::Context.clear! end it 'appends error-related log fields and filters sensitive Sidekiq arguments' do diff --git a/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb index 8b83d073e45..210829056c8 100644 --- a/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/context_payload_processor_spec.rb @@ -4,14 +4,18 @@ require 'spec_helper' RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do describe '.call' do - let(:exception) { StandardError.new('Test exception') } - let(:event) { Sentry.get_current_client.event_from_exception(exception) } + let(:required_options) do + { + configuration: Raven.configuration, + context: Raven.context, + breadcrumbs: Raven.breadcrumbs + } + end + + let(:event) { Raven::Event.new(required_options.merge(payload)) } let(:result_hash) { described_class.call(event).to_hash } before do - Sentry.get_current_scope.update_from_options(**payload) - Sentry.get_current_scope.apply_to_event(event) - allow_next_instance_of(Gitlab::ErrorTracking::ContextPayloadGenerator) do |generator| allow(generator).to receive(:generate).and_return( user: { username: 'root' }, @@ -21,10 +25,6 @@ RSpec.describe Gitlab::ErrorTracking::Processor::ContextPayloadProcessor do end end - after do - Sentry.get_current_scope.clear - end - let(:payload) do { user: { ip_address: '127.0.0.1' }, diff --git a/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb index 20fa5e2dacf..6076e525f06 100644 --- a/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/grpc_error_processor_spec.rb @@ -4,17 +4,16 @@ require 'spec_helper' RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do describe '.call' do - let(:event) { Sentry.get_current_client.event_from_exception(exception) } - let(:result_hash) { described_class.call(event).to_hash } - - before do - Sentry.get_current_scope.update_from_options(**data) - Sentry.get_current_scope.apply_to_event(event) + let(:required_options) do + { + configuration: Raven.configuration, + context: Raven.context, + breadcrumbs: Raven.breadcrumbs + } end - after do - Sentry.get_current_scope.clear - end + let(:event) { Raven::Event.from_exception(exception, required_options.merge(data)) } + let(:result_hash) { described_class.call(event).to_hash } context 'when there is no GRPC exception' do let(:exception) { RuntimeError.new } @@ -57,7 +56,7 @@ RSpec.describe Gitlab::ErrorTracking::Processor::GrpcErrorProcessor do end it 'removes the debug error string and stores it as an extra field' do - expect(result_hash[:fingerprint]).to be_empty + expect(result_hash).not_to include(:fingerprint) expect(result_hash[:exception][:values].first) .to include(type: 'GRPC::DeadlineExceeded', value: '4:Deadline Exceeded.') diff --git a/spec/lib/gitlab/error_tracking/processor/sanitizer_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/sanitizer_processor_spec.rb deleted file mode 100644 index 7e7d836f1d2..00000000000 --- a/spec/lib/gitlab/error_tracking/processor/sanitizer_processor_spec.rb +++ /dev/null @@ -1,96 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::ErrorTracking::Processor::SanitizerProcessor do - describe '.call' do - let(:event) { Sentry.get_current_client.event_from_exception(exception) } - let(:result_hash) { described_class.call(event).to_hash } - - before do - data.each do |key, value| - event.send("#{key}=", value) - end - end - - after do - Sentry.get_current_scope.clear - end - - context 'when event attributes contains sensitive information' do - let(:exception) { RuntimeError.new } - let(:data) do - { - contexts: { - jwt: 'abcdef', - controller: 'GraphController#execute' - }, - tags: { - variables: %w[some sensitive information'], - deep_hash: { - sharedSecret: 'secret123' - } - }, - user: { - email: 'a@a.com', - password: 'nobodyknows' - }, - extra: { - issue_url: 'http://gitlab.com/gitlab-org/gitlab-foss/-/issues/1', - my_token: '[FILTERED]', - another_token: '[FILTERED]' - } - } - end - - it 'filters sensitive attributes' do - expect_next_instance_of(ActiveSupport::ParameterFilter) do |instance| - expect(instance).to receive(:filter).exactly(4).times.and_call_original - end - - expect(result_hash).to include( - contexts: { - jwt: '[FILTERED]', - controller: 'GraphController#execute' - }, - tags: { - variables: '[FILTERED]', - deep_hash: { - sharedSecret: '[FILTERED]' - } - }, - user: { - email: 'a@a.com', - password: '[FILTERED]' - }, - extra: { - issue_url: 'http://gitlab.com/gitlab-org/gitlab-foss/-/issues/1', - my_token: '[FILTERED]', - another_token: '[FILTERED]' - } - ) - end - end - - context 'when request headers contains sensitive information' do - let(:exception) { RuntimeError.new } - let(:data) { {} } - - before do - event.rack_env = { - 'HTTP_AUTHORIZATION' => 'Bearer 123456', - 'HTTP_PRIVATE_TOKEN' => 'abcdef', - 'HTTP_GITLAB_WORKHORSE_PROXY_START' => 123456 - } - end - - it 'filters sensitive headers' do - expect(result_hash[:request][:headers]).to include( - 'Authorization' => '[FILTERED]', - 'Private-Token' => '[FILTERED]', - 'Gitlab-Workhorse-Proxy-Start' => '123456' - ) - end - end - end -end diff --git a/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb b/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb index bf8d31822a6..af5f11c9362 100644 --- a/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb +++ b/spec/lib/gitlab/error_tracking/processor/sidekiq_processor_spec.rb @@ -95,18 +95,16 @@ RSpec.describe Gitlab::ErrorTracking::Processor::SidekiqProcessor do end describe '.call' do - let(:exception) { StandardError.new('Test exception') } - let(:event) { Sentry.get_current_client.event_from_exception(exception) } - let(:result_hash) { described_class.call(event).to_hash } - - before do - Sentry.get_current_scope.update_from_options(**wrapped_value) - Sentry.get_current_scope.apply_to_event(event) + let(:required_options) do + { + configuration: Raven.configuration, + context: Raven.context, + breadcrumbs: Raven.breadcrumbs + } end - after do - Sentry.get_current_scope.clear - end + let(:event) { Raven::Event.new(required_options.merge(wrapped_value)) } + let(:result_hash) { described_class.call(event).to_hash } context 'when there is Sidekiq data' do let(:wrapped_value) { { extra: { sidekiq: value } } } diff --git a/spec/lib/gitlab/error_tracking_spec.rb b/spec/lib/gitlab/error_tracking_spec.rb index 50c85f76ce8..7ad1f52780a 100644 --- a/spec/lib/gitlab/error_tracking_spec.rb +++ b/spec/lib/gitlab/error_tracking_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -require 'sentry/transport/dummy_transport' +require 'raven/transports/dummy' RSpec.describe Gitlab::ErrorTracking do let(:exception) { RuntimeError.new('boom') } @@ -43,7 +43,7 @@ RSpec.describe Gitlab::ErrorTracking do } end - let(:sentry_event) { Sentry.get_current_client.transport.events.last } + let(:sentry_event) { Gitlab::Json.parse(Raven.client.transport.events.last[1]) } before do stub_sentry_settings @@ -53,7 +53,7 @@ RSpec.describe Gitlab::ErrorTracking do allow(I18n).to receive(:locale).and_return('en') described_class.configure do |config| - config.transport.transport_class = Sentry::DummyTransport + config.encoding = 'json' end end @@ -63,10 +63,6 @@ RSpec.describe Gitlab::ErrorTracking do end end - after do - ::Sentry.get_current_scope.clear - end - describe '.track_and_raise_for_dev_exception' do context 'when exceptions for dev should be raised' do before do @@ -74,7 +70,7 @@ RSpec.describe Gitlab::ErrorTracking do end it 'raises the exception' do - expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) + expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect do described_class.track_and_raise_for_dev_exception( @@ -92,7 +88,7 @@ RSpec.describe Gitlab::ErrorTracking do end it 'logs the exception with all attributes passed' do - expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) + expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) described_class.track_and_raise_for_dev_exception( exception, @@ -115,7 +111,7 @@ RSpec.describe Gitlab::ErrorTracking do describe '.track_and_raise_exception' do it 'always raises the exception' do - expect(Sentry).to receive(:capture_exception).with(exception, sentry_payload) + expect(Raven).to receive(:capture_exception).with(exception, sentry_payload) expect do described_class.track_and_raise_for_dev_exception( @@ -143,14 +139,14 @@ RSpec.describe Gitlab::ErrorTracking do subject(:track_exception) { described_class.track_exception(exception, extra) } before do - allow(Sentry).to receive(:capture_exception).and_call_original + allow(Raven).to receive(:capture_exception).and_call_original allow(Gitlab::ErrorTracking::Logger).to receive(:error) end - it 'calls Sentry.capture_exception' do + it 'calls Raven.capture_exception' do track_exception - expect(Sentry).to have_received(:capture_exception).with( + expect(Raven).to have_received(:capture_exception).with( exception, sentry_payload ) @@ -176,31 +172,25 @@ RSpec.describe Gitlab::ErrorTracking do context 'the exception implements :sentry_extra_data' do let(:extra_info) { { event: 'explosion', size: :massive } } - - before do - allow(exception).to receive(:sentry_extra_data).and_return(extra_info) - end + let(:exception) { double(message: 'bang!', sentry_extra_data: extra_info, backtrace: caller, cause: nil) } it 'includes the extra data from the exception in the tracking information' do track_exception - expect(Sentry).to have_received(:capture_exception).with( + expect(Raven).to have_received(:capture_exception).with( exception, a_hash_including(extra: a_hash_including(extra_info)) ) end end context 'the exception implements :sentry_extra_data, which returns nil' do + let(:exception) { double(message: 'bang!', sentry_extra_data: nil, backtrace: caller, cause: nil) } let(:extra) { { issue_url: issue_url } } - before do - allow(exception).to receive(:sentry_extra_data).and_return(nil) - end - it 'just includes the other extra info' do track_exception - expect(Sentry).to have_received(:capture_exception).with( + expect(Raven).to have_received(:capture_exception).with( exception, a_hash_including(extra: a_hash_including(extra)) ) end @@ -212,7 +202,7 @@ RSpec.describe Gitlab::ErrorTracking do it 'injects the normalized sql query into extra' do track_exception - expect(sentry_event.extra[:sql]).to eq('SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1') + expect(sentry_event.dig('extra', 'sql')).to eq('SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1') end end @@ -222,7 +212,7 @@ RSpec.describe Gitlab::ErrorTracking do track_exception - expect(sentry_event.extra[:sql]).to eq('SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1') + expect(sentry_event.dig('extra', 'sql')).to eq('SELECT "users".* FROM "users" WHERE "users"."id" = $2 AND "users"."foo" = $1') end end end @@ -231,27 +221,27 @@ RSpec.describe Gitlab::ErrorTracking do subject(:track_exception) { described_class.track_exception(exception, extra) } before do - allow(Sentry).to receive(:capture_exception).and_call_original + allow(Raven).to receive(:capture_exception).and_call_original allow(Gitlab::ErrorTracking::Logger).to receive(:error) end - context 'custom GitLab context when using Sentry.capture_exception directly' do - subject(:track_exception) { Sentry.capture_exception(exception) } + context 'custom GitLab context when using Raven.capture_exception directly' do + subject(:raven_capture_exception) { Raven.capture_exception(exception) } it 'merges a default set of tags into the existing tags' do - Sentry.set_tags(foo: 'bar') + allow(Raven.context).to receive(:tags).and_return(foo: 'bar') - track_exception + raven_capture_exception - expect(sentry_event.tags).to include(:correlation_id, :feature_category, :foo, :locale, :program) + expect(sentry_event['tags']).to include('correlation_id', 'feature_category', 'foo', 'locale', 'program') end it 'merges the current user information into the existing user information' do - Sentry.set_user(id: -1) + Raven.user_context(id: -1) - track_exception + raven_capture_exception - expect(sentry_event.user).to eq(id: -1, username: user.username) + expect(sentry_event['user']).to eq('id' => -1, 'username' => user.username) end end @@ -275,7 +265,7 @@ RSpec.describe Gitlab::ErrorTracking do it 'does not filter parameters when sending to Sentry' do track_exception - expect(sentry_event.extra[:sidekiq]['args']).to eq([1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value']) + expect(sentry_event.dig('extra', 'sidekiq', 'args')).to eq([1, { 'id' => 2, 'name' => 'hello' }, 'some-value', 'another-value']) end end @@ -285,7 +275,7 @@ RSpec.describe Gitlab::ErrorTracking do it 'filters sensitive arguments before sending and logging' do track_exception - expect(sentry_event.extra[:sidekiq]['args']).to eq(['[FILTERED]', 1, 2]) + expect(sentry_event.dig('extra', 'sidekiq', 'args')).to eq(['[FILTERED]', 1, 2]) expect(Gitlab::ErrorTracking::Logger).to have_received(:error).with( hash_including( 'extra.sidekiq' => { @@ -305,8 +295,8 @@ RSpec.describe Gitlab::ErrorTracking do it 'sets the GRPC debug error string in the Sentry event and adds a custom fingerprint' do track_exception - expect(sentry_event.extra[:grpc_debug_error_string]).to eq('{"hello":1}') - expect(sentry_event.fingerprint).to eq(['GRPC::DeadlineExceeded', '4:unknown cause.']) + expect(sentry_event.dig('extra', 'grpc_debug_error_string')).to eq('{"hello":1}') + expect(sentry_event['fingerprint']).to eq(['GRPC::DeadlineExceeded', '4:unknown cause.']) end end @@ -316,8 +306,8 @@ RSpec.describe Gitlab::ErrorTracking do it 'does not do any processing on the event' do track_exception - expect(sentry_event.extra).not_to include(:grpc_debug_error_string) - expect(sentry_event.fingerprint).to eq(['GRPC::DeadlineExceeded', '4:unknown cause']) + expect(sentry_event['extra']).not_to include('grpc_debug_error_string') + expect(sentry_event['fingerprint']).to eq(['GRPC::DeadlineExceeded', '4:unknown cause']) end end end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb index c2479d20949..a10a8883591 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb @@ -18,14 +18,43 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi end describe '#schedule' do - it 'calls schedule on the strategy' do - expect do |block| - expect_next_instance_of(Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuting) do |strategy| - expect(strategy).to receive(:schedule).with(job, &block) + shared_examples 'scheduling with deduplication class' do |strategy_class| + it 'calls schedule on the strategy' do + expect do |block| + expect_next_instance_of("Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::#{strategy_class}".constantize) do |strategy| + expect(strategy).to receive(:schedule).with(job, &block) + end + + duplicate_job.schedule(&block) + end.to yield_control + end + end + + it_behaves_like 'scheduling with deduplication class', 'UntilExecuting' + + context 'when the deduplication depends on a FF' do + before do + skip_feature_flags_yaml_validation + skip_default_enabled_yaml_check + + allow(AuthorizedProjectsWorker).to receive(:get_deduplication_options).and_return(feature_flag: :my_feature_flag) + end + + context 'when the feature flag is enabled' do + before do + stub_feature_flags(my_feature_flag: true) end - duplicate_job.schedule(&block) - end.to yield_control + it_behaves_like 'scheduling with deduplication class', 'UntilExecuting' + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(my_feature_flag: false) + end + + it_behaves_like 'scheduling with deduplication class', 'None' + end end end diff --git a/spec/lib/peek/views/active_record_spec.rb b/spec/lib/peek/views/active_record_spec.rb index 9eeeca4de61..e5aae2822ed 100644 --- a/spec/lib/peek/views/active_record_spec.rb +++ b/spec/lib/peek/views/active_record_spec.rb @@ -5,16 +5,17 @@ require 'spec_helper' RSpec.describe Peek::Views::ActiveRecord, :request_store do subject { Peek.views.find { |v| v.instance_of?(Peek::Views::ActiveRecord) } } - let(:connection_1) { double(:connection) } - let(:connection_2) { double(:connection) } - let(:connection_3) { double(:connection) } + let(:connection_replica) { double(:connection_replica) } + let(:connection_primary_1) { double(:connection_primary) } + let(:connection_primary_2) { double(:connection_primary) } + let(:connection_unknown) { double(:connection_unknown) } let(:event_1) do { name: 'SQL', sql: 'SELECT * FROM users WHERE id = 10', cached: false, - connection: connection_1 + connection: connection_primary_1 } end @@ -23,7 +24,7 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do name: 'SQL', sql: 'SELECT * FROM users WHERE id = 10', cached: true, - connection: connection_2 + connection: connection_replica } end @@ -32,55 +33,141 @@ RSpec.describe Peek::Views::ActiveRecord, :request_store do name: 'SQL', sql: 'UPDATE users SET admin = true WHERE id = 10', cached: false, - connection: connection_3 + connection: connection_primary_2 + } + end + + let(:event_4) do + { + name: 'SCHEMA', + sql: 'SELECT VERSION()', + cached: false, + connection: connection_unknown } end before do allow(Gitlab::PerformanceBar).to receive(:enabled_for_request?).and_return(true) - allow(connection_1).to receive(:transaction_open?).and_return(false) - allow(connection_2).to receive(:transaction_open?).and_return(false) - allow(connection_3).to receive(:transaction_open?).and_return(true) + allow(connection_replica).to receive(:transaction_open?).and_return(false) + allow(connection_primary_1).to receive(:transaction_open?).and_return(false) + allow(connection_primary_2).to receive(:transaction_open?).and_return(true) + allow(connection_unknown).to receive(:transaction_open?).and_return(false) end - it 'subscribes and store data into peek views' do - Timecop.freeze(2021, 2, 23, 10, 0) do - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) - ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3) + context 'when database load balancing is not enabled' do + it 'subscribes and store data into peek views' do + Timecop.freeze(2021, 2, 23, 10, 0) do + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 4.seconds, '4', event_4) + end + + expect(subject.results).to match( + calls: 4, + summary: { + "Cached" => 1, + "In a transaction" => 1 + }, + duration: '10000.00ms', + warnings: ["active-record duration: 10000.0 over 3000"], + details: contain_exactly( + a_hash_including( + start: be_a(Time), + cached: '', + transaction: '', + duration: 1000.0, + sql: 'SELECT * FROM users WHERE id = 10' + ), + a_hash_including( + start: be_a(Time), + cached: 'Cached', + transaction: '', + duration: 2000.0, + sql: 'SELECT * FROM users WHERE id = 10' + ), + a_hash_including( + start: be_a(Time), + cached: '', + transaction: 'In a transaction', + duration: 3000.0, + sql: 'UPDATE users SET admin = true WHERE id = 10' + ), + a_hash_including( + start: be_a(Time), + cached: '', + transaction: '', + duration: 4000.0, + sql: 'SELECT VERSION()' + ) + ) + ) + end + end + + context 'when database load balancing is enabled' do + before do + allow(Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_replica).and_return(:replica) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_1).and_return(:primary) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_primary_2).and_return(:primary) + allow(Gitlab::Database::LoadBalancing).to receive(:db_role_for_connection).with(connection_unknown).and_return(nil) end - expect(subject.results).to match( - calls: 3, - summary: { - "Cached" => 1, - "In a transaction" => 1 - }, - duration: '6000.00ms', - warnings: ["active-record duration: 6000.0 over 3000"], - details: contain_exactly( - a_hash_including( - start: be_a(Time), - cached: '', - transaction: '', - duration: 1000.0, - sql: 'SELECT * FROM users WHERE id = 10' - ), - a_hash_including( - start: be_a(Time), - cached: 'Cached', - transaction: '', - duration: 2000.0, - sql: 'SELECT * FROM users WHERE id = 10' - ), - a_hash_including( - start: be_a(Time), - cached: '', - transaction: 'In a transaction', - duration: 3000.0, - sql: 'UPDATE users SET admin = true WHERE id = 10' + it 'includes db role data' do + Timecop.freeze(2021, 2, 23, 10, 0) do + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 1.second, '1', event_1) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 2.seconds, '2', event_2) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 3.seconds, '3', event_3) + ActiveSupport::Notifications.publish('sql.active_record', Time.current, Time.current + 4.seconds, '4', event_4) + end + + expect(subject.results).to match( + calls: 4, + summary: { + "Cached" => 1, + "In a transaction" => 1, + "Primary" => 2, + "Replica" => 1, + "Unknown" => 1 + }, + duration: '10000.00ms', + warnings: ["active-record duration: 10000.0 over 3000"], + details: contain_exactly( + a_hash_including( + start: be_a(Time), + cached: '', + transaction: '', + duration: 1000.0, + sql: 'SELECT * FROM users WHERE id = 10', + db_role: 'Primary' + ), + a_hash_including( + start: be_a(Time), + cached: 'Cached', + transaction: '', + duration: 2000.0, + sql: 'SELECT * FROM users WHERE id = 10', + db_role: 'Replica' + ), + a_hash_including( + start: be_a(Time), + cached: '', + transaction: 'In a transaction', + duration: 3000.0, + sql: 'UPDATE users SET admin = true WHERE id = 10', + db_role: 'Primary' + ), + a_hash_including( + start: be_a(Time), + cached: '', + transaction: '', + duration: 4000.0, + sql: 'SELECT VERSION()', + db_role: 'Unknown' + ) ) ) - ) + end end end diff --git a/spec/lib/sidebars/projects/menus/learn_gitlab_menu_spec.rb b/spec/lib/sidebars/projects/menus/learn_gitlab_menu_spec.rb index ef5ae550551..231e5a850c2 100644 --- a/spec/lib/sidebars/projects/menus/learn_gitlab_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/learn_gitlab_menu_spec.rb @@ -27,7 +27,6 @@ RSpec.describe Sidebars::Projects::Menus::LearnGitlabMenu do { class: 'home', data: { - track_action: 'click_menu', track_property: tracking_category, track_label: 'learn_gitlab' } diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index feb2f3630c1..69b4d752f4c 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -79,6 +79,32 @@ RSpec.describe CommitStatus do end end + describe '.updated_before' do + let!(:lookback) { 5.days.ago } + let!(:timeout) { 1.day.ago } + let!(:before_lookback) { lookback - 1.hour } + let!(:after_lookback) { lookback + 1.hour } + let!(:before_timeout) { timeout - 1.hour } + let!(:after_timeout) { timeout + 1.hour } + + subject { described_class.updated_before(lookback: lookback, timeout: timeout) } + + def create_build_with_set_timestamps(created_at:, updated_at:) + travel_to(created_at) { create(:ci_build, created_at: Time.current) }.tap do |build| + travel_to(updated_at) { build.update!(status: :failed) } + end + end + + it 'finds builds updated and created in the window between lookback and timeout' do + build_in_lookback_timeout_window = create_build_with_set_timestamps(created_at: after_lookback, updated_at: before_timeout) + build_outside_lookback_window = create_build_with_set_timestamps(created_at: before_lookback, updated_at: before_timeout) + build_outside_timeout_window = create_build_with_set_timestamps(created_at: after_lookback, updated_at: after_timeout) + + expect(subject).to contain_exactly(build_in_lookback_timeout_window) + expect(subject).not_to include(build_outside_lookback_window, build_outside_timeout_window) + end + end + describe '#processed' do subject { commit_status.processed } diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 3c690a767a6..ce0018d6d0d 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -require 'sentry/transport/dummy_transport' +require 'raven/transports/dummy' require_relative '../../../config/initializers/sentry' RSpec.describe API::Helpers do diff --git a/spec/support/shared_examples/lib/gitlab/ci/ci_trace_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/ci/ci_trace_shared_examples.rb index 9c95d1ff9d9..3760325675a 100644 --- a/spec/support/shared_examples/lib/gitlab/ci/ci_trace_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/ci/ci_trace_shared_examples.rb @@ -29,6 +29,34 @@ RSpec.shared_examples 'common trace features' do end end + describe '#read' do + context 'gitlab_ci_archived_trace_consistent_reads feature flag enabled' do + before do + stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: trace.job.project) + end + + it 'calls ::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking' do + expect(::Gitlab::Database::LoadBalancing::Sticking).to receive(:unstick_or_continue_sticking) + .with(described_class::LOAD_BALANCING_STICKING_NAMESPACE, trace.job.id) + .and_call_original + + trace.read { |stream| stream } + end + end + + context 'gitlab_ci_archived_trace_consistent_reads feature flag disabled' do + before do + stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: false) + end + + it 'does not call ::Gitlab::Database::LoadBalancing::Sticking.unstick_or_continue_sticking' do + expect(::Gitlab::Database::LoadBalancing::Sticking).not_to receive(:unstick_or_continue_sticking) + + trace.read { |stream| stream } + end + end + end + describe '#extract_coverage' do let(:regex) { '\(\d+.\d+\%\) covered' } @@ -253,6 +281,52 @@ RSpec.shared_examples 'common trace features' do describe '#archive!' do subject { trace.archive! } + context 'when live trace chunks exists' do + before do + # Build a trace_chunk manually + # It is possible to do so with trace.set but only if ci_enable_live_trace FF is enabled + # + # We need the job to have a trace_chunk because we only use #stick in + # the case where trace_chunks exist. + stream = Gitlab::Ci::Trace::Stream.new do + Gitlab::Ci::Trace::ChunkedIO.new(trace.job) + end + + stream.set(+"12\n34") + end + + # We check the before setup actually sets up job trace_chunks + it 'has job trace_chunks' do + expect(trace.job.trace_chunks).to be_present + end + + context 'gitlab_ci_archived_trace_consistent_reads feature flag enabled' do + before do + stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: trace.job.project) + end + + it 'calls ::Gitlab::Database::LoadBalancing::Sticking.stick' do + expect(::Gitlab::Database::LoadBalancing::Sticking).to receive(:stick) + .with(described_class::LOAD_BALANCING_STICKING_NAMESPACE, trace.job.id) + .and_call_original + + subject + end + end + + context 'gitlab_ci_archived_trace_consistent_reads feature flag disabled' do + before do + stub_feature_flags(gitlab_ci_archived_trace_consistent_reads: false) + end + + it 'does not call ::Gitlab::Database::LoadBalancing::Sticking.stick' do + expect(::Gitlab::Database::LoadBalancing::Sticking).not_to receive(:stick) + + subject + end + end + end + context 'when build status is success' do let!(:build) { create(:ci_build, :success, :trace_live) } diff --git a/spec/workers/concerns/worker_attributes_spec.rb b/spec/workers/concerns/worker_attributes_spec.rb index a654ecbd3e2..d4b17c65f46 100644 --- a/spec/workers/concerns/worker_attributes_spec.rb +++ b/spec/workers/concerns/worker_attributes_spec.rb @@ -62,6 +62,12 @@ RSpec.describe WorkerAttributes do end describe '.idempotent!' do + it 'sets `idempotent` attribute of the worker class to true' do + worker.idempotent! + + expect(worker.send(:class_attributes)[:idempotent]).to eq(true) + end + context 'when data consistency is not :always' do it 'raise exception' do worker.data_consistency(:sticky) @@ -71,4 +77,66 @@ RSpec.describe WorkerAttributes do end end end + + describe '.idempotent?' do + subject(:idempotent?) { worker.idempotent? } + + context 'when the worker is idempotent' do + before do + worker.idempotent! + end + + it { is_expected.to be_truthy } + end + + context 'when the worker is not idempotent' do + it { is_expected.to be_falsey } + end + end + + describe '.deduplicate' do + it 'sets deduplication_strategy and deduplication_options' do + worker.deduplicate(:until_executing, including_scheduled: true) + + expect(worker.send(:class_attributes)[:deduplication_strategy]).to eq(:until_executing) + expect(worker.send(:class_attributes)[:deduplication_options]).to eq(including_scheduled: true) + end + end + + describe '#deduplication_enabled?' do + subject(:deduplication_enabled?) { worker.deduplication_enabled? } + + context 'when no feature flag is set' do + before do + worker.deduplicate(:until_executing) + end + + it { is_expected.to eq(true) } + end + + context 'when feature flag is set' do + before do + skip_feature_flags_yaml_validation + skip_default_enabled_yaml_check + + worker.deduplicate(:until_executing, feature_flag: :my_feature_flag) + end + + context 'when the FF is enabled' do + before do + stub_feature_flags(my_feature_flag: true) + end + + it { is_expected.to eq(true) } + end + + context 'when the FF is disabled' do + before do + stub_feature_flags(my_feature_flag: false) + end + + it { is_expected.to eq(false) } + end + end + end end diff --git a/spec/workers/stuck_ci_jobs_worker_spec.rb b/spec/workers/stuck_ci_jobs_worker_spec.rb index 24d3b6fadf5..84b2d87494e 100644 --- a/spec/workers/stuck_ci_jobs_worker_spec.rb +++ b/spec/workers/stuck_ci_jobs_worker_spec.rb @@ -9,12 +9,17 @@ RSpec.describe StuckCiJobsWorker do let!(:job) { create :ci_build, runner: runner } let(:worker_lease_key) { StuckCiJobsWorker::EXCLUSIVE_LEASE_KEY } let(:worker_lease_uuid) { SecureRandom.uuid } + let(:created_at) { } + let(:updated_at) { } subject(:worker) { described_class.new } before do stub_exclusive_lease(worker_lease_key, worker_lease_uuid) - job.update!(status: status, updated_at: updated_at) + job_attributes = { status: status } + job_attributes[:created_at] = created_at if created_at + job_attributes[:updated_at] = updated_at if updated_at + job.update!(job_attributes) end shared_examples 'job is dropped' do @@ -63,22 +68,70 @@ RSpec.describe StuckCiJobsWorker do allow_any_instance_of(Ci::Build).to receive(:stuck?).and_return(false) end - context 'when job was not updated for more than 1 day ago' do - let(:updated_at) { 2.days.ago } + context 'when job was updated_at more than 1 day ago' do + let(:updated_at) { 1.5.days.ago } - it_behaves_like 'job is dropped' + context 'when created_at is the same as updated_at' do + let(:created_at) { 1.5.days.ago } + + it_behaves_like 'job is dropped' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 3.days.ago } + + it_behaves_like 'job is dropped' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end - context 'when job was updated in less than 1 day ago' do + context 'when job was updated less than 1 day ago' do let(:updated_at) { 6.hours.ago } - it_behaves_like 'job is unchanged' + context 'when created_at is the same as updated_at' do + let(:created_at) { 1.5.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 3.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end - context 'when job was not updated for more than 1 hour ago' do + context 'when job was updated more than 1 hour ago' do let(:updated_at) { 2.hours.ago } - it_behaves_like 'job is unchanged' + context 'when created_at is the same as updated_at' do + let(:created_at) { 2.hours.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 3.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end end @@ -87,17 +140,48 @@ RSpec.describe StuckCiJobsWorker do allow_any_instance_of(Ci::Build).to receive(:stuck?).and_return(true) end - context 'when job was not updated for more than 1 hour ago' do - let(:updated_at) { 2.hours.ago } + context 'when job was updated_at more than 1 hour ago' do + let(:updated_at) { 1.5.hours.ago } + + context 'when created_at is the same as updated_at' do + let(:created_at) { 1.5.hours.ago } + + it_behaves_like 'job is dropped' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 3.days.ago } - it_behaves_like 'job is dropped' + it_behaves_like 'job is dropped' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end - context 'when job was updated in less than 1 - hour ago' do + context 'when job was updated in less than 1 hour ago' do let(:updated_at) { 30.minutes.ago } - it_behaves_like 'job is unchanged' + context 'when created_at is the same as updated_at' do + let(:created_at) { 30.minutes.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 2.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end end end @@ -105,7 +189,7 @@ RSpec.describe StuckCiJobsWorker do context 'when job is running' do let(:status) { 'running' } - context 'when job was not updated for more than 1 hour ago' do + context 'when job was updated_at more than an hour ago' do let(:updated_at) { 2.hours.ago } it_behaves_like 'job is dropped' @@ -123,7 +207,23 @@ RSpec.describe StuckCiJobsWorker do let(:status) { status } let(:updated_at) { 2.days.ago } - it_behaves_like 'job is unchanged' + context 'when created_at is the same as updated_at' do + let(:created_at) { 2.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is before updated_at' do + let(:created_at) { 3.days.ago } + + it_behaves_like 'job is unchanged' + end + + context 'when created_at is outside lookback window' do + let(:created_at) { described_class::BUILD_LOOKBACK - 1.day } + + it_behaves_like 'job is unchanged' + end end end |