diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-04-26 21:19:16 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-04-26 21:19:16 +0300 |
commit | fa69a57b46f4893c488445f79d6d290463820f7d (patch) | |
tree | 4a73ccd1f8ffb4dbb47a42c6edb309238828a44e /spec | |
parent | 34283a71d9ac31eb4da0b59d0b25fc2be014bc9c (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
25 files changed, 643 insertions, 1402 deletions
diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 4928cc46d7f..fdc0ddda9f4 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -385,6 +385,57 @@ RSpec.describe Import::GithubController, feature_category: :importers do end end + describe "GET failures" do + let_it_be_with_reload(:project) { create(:project, import_type: 'github', import_status: :started, import_source: 'example/repo', import_url: 'https://fake.url') } + let!(:import_failure) do + create(:import_failure, + project: project, + source: 'Gitlab::GithubImport::Importer::PullRequestImporter', + external_identifiers: { iid: 2, object_type: 'pull_request', title: 'My Pull Request' } + ) + end + + context 'when import is not finished' do + it 'return bad_request' do + get :failures, params: { project_id: project.id } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('The import is not complete.') + end + end + + context 'when import is finished' do + before do + project.import_state.finish + end + + it 'includes failure details in response' do + get :failures, params: { project_id: project.id } + + expect(json_response[0]['type']).to eq('pull_request') + expect(json_response[0]['title']).to eq('My Pull Request') + expect(json_response[0]['provider_url']).to eq("https://fake.url/example/repo/pull/2") + expect(json_response[0]['details']['source']).to eq(import_failure.source) + end + + it 'paginates records' do + issue_title = 'My Issue' + + create( + :import_failure, + project: project, + source: 'Gitlab::GithubImport::Importer::IssueAndLabelLinksImporter', + external_identifiers: { iid: 3, object_type: 'issue', title: issue_title } + ) + + get :failures, params: { project_id: project.id, page: 2, per_page: 1 } + + expect(json_response.size).to eq(1) + expect(json_response.first['title']).to eq(issue_title) + end + end + end + describe "POST cancel" do let_it_be(:project) do create( diff --git a/spec/controllers/projects/commits_controller_spec.rb b/spec/controllers/projects/commits_controller_spec.rb index 55ad92c517c..c87b48bf442 100644 --- a/spec/controllers/projects/commits_controller_spec.rb +++ b/spec/controllers/projects/commits_controller_spec.rb @@ -108,22 +108,6 @@ RSpec.describe Projects::CommitsController, feature_category: :source_code_manag get :show, params: { namespace_id: project.namespace, project_id: project, id: 'master/README.md' } end - context 'when the show_tags_on_commits_view flag is disabled' do - let(:id) { "master/README.md" } - - before do - stub_feature_flags(show_tags_on_commits_view: false) - end - - it 'does not load tags' do - expect_next_instance_of(CommitCollection) do |collection| - expect(collection).not_to receive(:load_tags) - end - - get :show, params: { namespace_id: project.namespace, project_id: project, id: id } - end - end - context "when the ref name ends in .atom" do context "when the ref does not exist with the suffix" do before do diff --git a/spec/factories/import_failures.rb b/spec/factories/import_failures.rb index df0793664f4..b4a7c6c46b1 100644 --- a/spec/factories/import_failures.rb +++ b/spec/factories/import_failures.rb @@ -21,5 +21,9 @@ FactoryBot.define do trait :soft_failure do retry_count { 1 } end + + trait :github_import_failure do + external_identifiers { { iid: 2, object_type: 'pull_request', title: 'Implement cool feature' } } + end end end diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index d82f9acdd07..582535790bd 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -32,15 +32,30 @@ RSpec.describe "Admin Runners", feature_category: :runner_fleet do end describe "runners registration" do - before do - stub_feature_flags(create_runner_workflow_for_admin: false) + context 'when create_runner_workflow_for_namespace is enabled' do + before do + stub_feature_flags(create_runner_workflow_for_admin: true) - visit admin_runners_path + visit admin_runners_path + end + + it_behaves_like "shows and resets runner registration token" do + let(:dropdown_text) { s_('Runners|Register an instance runner') } + let(:registration_token) { Gitlab::CurrentSettings.runners_registration_token } + end end - it_behaves_like "shows and resets runner registration token" do - let(:dropdown_text) { s_('Runners|Register an instance runner') } - let(:registration_token) { Gitlab::CurrentSettings.runners_registration_token } + context 'when create_runner_workflow_for_namespace is disabled' do + before do + stub_feature_flags(create_runner_workflow_for_admin: false) + + visit admin_runners_path + end + + it_behaves_like "shows and resets runner registration token" do + let(:dropdown_text) { s_('Runners|Register an instance runner') } + let(:registration_token) { Gitlab::CurrentSettings.runners_registration_token } + end end end diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index 54482d141c7..2de95c21003 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -21,19 +21,28 @@ RSpec.describe 'Runners', feature_category: :runner_fleet do project.add_maintainer(user) end - context 'when create_runner_workflow_for_namespace is enabled' do + context 'when create_runner_workflow_for_namespace is enabled', :js do before do stub_feature_flags(create_runner_workflow_for_namespace: [project.namespace]) - end - it 'user can see a link with instructions on how to install GitLab Runner' do visit project_runners_path(project) + end + it 'user can see a link with instructions on how to install GitLab Runner' do expect(page).to have_link(s_('Runners|New project runner'), href: new_project_runner_path(project)) end - describe 'runner registration', :js do + it_behaves_like "shows and resets runner registration token" do + let(:dropdown_text) { s_('Runners|Register a project runner') } + let(:registration_token) { project.runners_token } + end + end + + context 'when user views new runner page' do + context 'when create_runner_workflow_for_namespace is enabled', :js do before do + stub_feature_flags(create_runner_workflow_for_namespace: [project.namespace]) + visit new_project_runner_path(project) end diff --git a/spec/frontend/ci/runner/components/registration/registration_dropdown_spec.js b/spec/frontend/ci/runner/components/registration/registration_dropdown_spec.js index 9df7a974af3..e564cf49ca0 100644 --- a/spec/frontend/ci/runner/components/registration/registration_dropdown_spec.js +++ b/spec/frontend/ci/runner/components/registration/registration_dropdown_spec.js @@ -12,7 +12,14 @@ import RegistrationDropdown from '~/ci/runner/components/registration/registrati import RegistrationToken from '~/ci/runner/components/registration/registration_token.vue'; import RegistrationTokenResetDropdownItem from '~/ci/runner/components/registration/registration_token_reset_dropdown_item.vue'; -import { INSTANCE_TYPE, GROUP_TYPE, PROJECT_TYPE } from '~/ci/runner/constants'; +import { + INSTANCE_TYPE, + GROUP_TYPE, + PROJECT_TYPE, + I18N_REGISTER_INSTANCE_TYPE, + I18N_REGISTER_GROUP_TYPE, + I18N_REGISTER_PROJECT_TYPE, +} from '~/ci/runner/constants'; import getRunnerPlatformsQuery from '~/vue_shared/components/runner_instructions/graphql/get_runner_platforms.query.graphql'; import getRunnerSetupInstructionsQuery from '~/vue_shared/components/runner_instructions/graphql/get_runner_setup.query.graphql'; @@ -81,13 +88,13 @@ describe('RegistrationDropdown', () => { it.each` type | text - ${INSTANCE_TYPE} | ${s__('Runners|Register an instance runner')} - ${GROUP_TYPE} | ${s__('Runners|Register a group runner')} - ${PROJECT_TYPE} | ${s__('Runners|Register a project runner')} - `('Dropdown text for type $type is "$text"', () => { - createComponent({ props: { type: INSTANCE_TYPE } }, mountExtended); + ${INSTANCE_TYPE} | ${I18N_REGISTER_INSTANCE_TYPE} + ${GROUP_TYPE} | ${I18N_REGISTER_GROUP_TYPE} + ${PROJECT_TYPE} | ${I18N_REGISTER_PROJECT_TYPE} + `('Dropdown text for type $type is "$text"', ({ type, text }) => { + createComponent({ props: { type } }, mountExtended); - expect(wrapper.text()).toContain('Register an instance runner'); + expect(wrapper.text()).toContain(text); }); it('Passes attributes to dropdown', () => { @@ -214,7 +221,7 @@ describe('RegistrationDropdown', () => { { createRunnerWorkflowForAdmin: true }, { createRunnerWorkflowForNamespace: true }, ])('When showing a "deprecated" warning', (glFeatures) => { - it('Passes deprecated variant props and attributes to dropdown', () => { + it('passes deprecated variant props and attributes to dropdown', () => { createComponent({ provide: { glFeatures }, }); @@ -230,6 +237,17 @@ describe('RegistrationDropdown', () => { }); }); + it.each` + type | text + ${INSTANCE_TYPE} | ${I18N_REGISTER_INSTANCE_TYPE} + ${GROUP_TYPE} | ${I18N_REGISTER_GROUP_TYPE} + ${PROJECT_TYPE} | ${I18N_REGISTER_PROJECT_TYPE} + `('dropdown text for type $type is "$text"', ({ type, text }) => { + createComponent({ props: { type } }, mountExtended); + + expect(wrapper.text()).toContain(text); + }); + it('shows warning text', () => { createComponent( { @@ -243,7 +261,7 @@ describe('RegistrationDropdown', () => { expect(text.exists()).toBe(true); }); - it('button shows only ellipsis icon', () => { + it('button shows ellipsis icon', () => { createComponent( { provide: { glFeatures }, @@ -251,7 +269,6 @@ describe('RegistrationDropdown', () => { mountExtended, ); - expect(findDropdownBtn().text()).toBe(''); expect(findDropdownBtn().findComponent(GlIcon).props('name')).toBe('ellipsis_v'); expect(findDropdownBtn().findAllComponents(GlIcon)).toHaveLength(1); }); diff --git a/spec/frontend/work_items/components/work_item_created_updated_spec.js b/spec/frontend/work_items/components/work_item_created_updated_spec.js index fe31c01df36..2a5b2853b5e 100644 --- a/spec/frontend/work_items/components/work_item_created_updated_spec.js +++ b/spec/frontend/work_items/components/work_item_created_updated_spec.js @@ -5,14 +5,12 @@ import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import WorkItemCreatedUpdated from '~/work_items/components/work_item_created_updated.vue'; -import workItemQuery from '~/work_items/graphql/work_item.query.graphql'; import workItemByIidQuery from '~/work_items/graphql/work_item_by_iid.query.graphql'; -import { workItemResponseFactory, mockAssignees } from '../mock_data'; +import { workItemByIidResponseFactory, mockAssignees } from '../mock_data'; describe('WorkItemCreatedUpdated component', () => { let wrapper; let successHandler; - let successByIidHandler; Vue.use(VueApollo); @@ -21,39 +19,17 @@ describe('WorkItemCreatedUpdated component', () => { const findCreatedAtText = () => findCreatedAt().text().replace(/\s+/g, ' '); - const createComponent = async ({ - workItemId = 'gid://gitlab/WorkItem/1', - workItemIid = '1', - fetchByIid = false, - author = null, - updatedAt, - } = {}) => { - const workItemQueryResponse = workItemResponseFactory({ + const createComponent = async ({ workItemIid = '1', author = null, updatedAt } = {}) => { + const workItemQueryResponse = workItemByIidResponseFactory({ author, updatedAt, }); - const byIidResponse = { - data: { - workspace: { - id: 'gid://gitlab/Project/1', - workItems: { - nodes: [workItemQueryResponse.data.workItem], - }, - }, - }, - }; successHandler = jest.fn().mockResolvedValue(workItemQueryResponse); - successByIidHandler = jest.fn().mockResolvedValue(byIidResponse); - - const handlers = [ - [workItemQuery, successHandler], - [workItemByIidQuery, successByIidHandler], - ]; wrapper = shallowMount(WorkItemCreatedUpdated, { - apolloProvider: createMockApollo(handlers), - propsData: { workItemId, workItemIid, fetchByIid, fullPath: '/some/project' }, + apolloProvider: createMockApollo([[workItemByIidQuery, successHandler]]), + propsData: { workItemIid, fullPath: '/some/project' }, stubs: { GlAvatarLink, GlSprintf, @@ -63,42 +39,34 @@ describe('WorkItemCreatedUpdated component', () => { await waitForPromises(); }; - describe.each([true, false])('fetchByIid is %s', (fetchByIid) => { - describe('work item id and iid undefined', () => { - beforeEach(async () => { - await createComponent({ workItemId: null, workItemIid: null, fetchByIid }); - }); - - it('skips the work item query', () => { - expect(successHandler).not.toHaveBeenCalled(); - expect(successByIidHandler).not.toHaveBeenCalled(); - }); - }); + it('skips the work item query when workItemIid is not defined', async () => { + await createComponent({ workItemIid: null }); - it('shows author name and link', async () => { - const author = mockAssignees[0]; + expect(successHandler).not.toHaveBeenCalled(); + }); - await createComponent({ fetchByIid, author }); + it('shows author name and link', async () => { + const author = mockAssignees[0]; + await createComponent({ author }); - expect(findCreatedAtText()).toEqual(`Created by ${author.name}`); - }); + expect(findCreatedAtText()).toBe(`Created by ${author.name}`); + }); - it('shows created time when author is null', async () => { - await createComponent({ fetchByIid, author: null }); + it('shows created time when author is null', async () => { + await createComponent({ author: null }); - expect(findCreatedAtText()).toEqual('Created'); - }); + expect(findCreatedAtText()).toBe('Created'); + }); - it('shows updated time', async () => { - await createComponent({ fetchByIid }); + it('shows updated time', async () => { + await createComponent(); - expect(findUpdatedAt().exists()).toBe(true); - }); + expect(findUpdatedAt().exists()).toBe(true); + }); - it('does not show updated time for new work items', async () => { - await createComponent({ fetchByIid, updatedAt: null }); + it('does not show updated time for new work items', async () => { + await createComponent({ updatedAt: null }); - expect(findUpdatedAt().exists()).toBe(false); - }); + expect(findUpdatedAt().exists()).toBe(false); }); }); diff --git a/spec/frontend/work_items/components/work_item_labels_spec.js b/spec/frontend/work_items/components/work_item_labels_spec.js index 6d51448194b..e6f7793b43f 100644 --- a/spec/frontend/work_items/components/work_item_labels_spec.js +++ b/spec/frontend/work_items/components/work_item_labels_spec.js @@ -6,7 +6,6 @@ import waitForPromises from 'helpers/wait_for_promises'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '~/lib/utils/constants'; import labelSearchQuery from '~/sidebar/components/labels/labels_select_widget/graphql/project_labels.query.graphql'; -import workItemQuery from '~/work_items/graphql/work_item.query.graphql'; import workItemLabelsSubscription from 'ee_else_ce/work_items/graphql/work_item_labels.subscription.graphql'; import updateWorkItemMutation from '~/work_items/graphql/update_work_item.mutation.graphql'; import workItemByIidQuery from '~/work_items/graphql/work_item_by_iid.query.graphql'; @@ -15,11 +14,9 @@ import { i18n, I18N_WORK_ITEM_ERROR_FETCHING_LABELS } from '~/work_items/constan import { projectLabelsResponse, mockLabels, - workItemQueryResponse, - workItemResponseFactory, + workItemByIidResponseFactory, updateWorkItemMutationResponse, workItemLabelsSubscriptionResponse, - projectWorkItemResponse, } from '../mock_data'; Vue.use(VueApollo); @@ -34,8 +31,9 @@ describe('WorkItemLabels component', () => { const findEmptyState = () => wrapper.findByTestId('empty-state'); const findLabelsTitle = () => wrapper.findByTestId('labels-title'); - const workItemQuerySuccess = jest.fn().mockResolvedValue(workItemQueryResponse); - const workItemByIidResponseHandler = jest.fn().mockResolvedValue(projectWorkItemResponse); + const workItemQuerySuccess = jest + .fn() + .mockResolvedValue(workItemByIidResponseFactory({ labels: null })); const successSearchQueryHandler = jest.fn().mockResolvedValue(projectLabelsResponse); const successUpdateWorkItemMutationHandler = jest .fn() @@ -48,27 +46,22 @@ describe('WorkItemLabels component', () => { workItemQueryHandler = workItemQuerySuccess, searchQueryHandler = successSearchQueryHandler, updateWorkItemMutationHandler = successUpdateWorkItemMutationHandler, - fetchByIid = false, - queryVariables = { id: workItemId }, + queryVariables = { iid: '1' }, } = {}) => { - const apolloProvider = createMockApollo([ - [workItemQuery, workItemQueryHandler], - [labelSearchQuery, searchQueryHandler], - [updateWorkItemMutation, updateWorkItemMutationHandler], - [workItemLabelsSubscription, subscriptionHandler], - [workItemByIidQuery, workItemByIidResponseHandler], - ]); - wrapper = mountExtended(WorkItemLabels, { + apolloProvider: createMockApollo([ + [workItemByIidQuery, workItemQueryHandler], + [labelSearchQuery, searchQueryHandler], + [updateWorkItemMutation, updateWorkItemMutationHandler], + [workItemLabelsSubscription, subscriptionHandler], + ]), propsData: { workItemId, canUpdate, fullPath: 'test-project-path', queryVariables, - fetchByIid, }, attachTo: document.body, - apolloProvider, }); }; @@ -186,7 +179,7 @@ describe('WorkItemLabels component', () => { }); it('adds new labels to the end', async () => { - const response = workItemResponseFactory({ labels: [mockLabels[1]] }); + const response = workItemByIidResponseFactory({ labels: [mockLabels[1]] }); const workItemQueryHandler = jest.fn().mockResolvedValue(response); createComponent({ workItemQueryHandler, @@ -263,24 +256,15 @@ describe('WorkItemLabels component', () => { }); }); - it('calls the global ID work item query when `fetchByIid` prop is false', async () => { - createComponent({ fetchByIid: false }); + it('calls the work item query', async () => { + createComponent(); await waitForPromises(); expect(workItemQuerySuccess).toHaveBeenCalled(); - expect(workItemByIidResponseHandler).not.toHaveBeenCalled(); - }); - - it('calls the IID work item query when when `fetchByIid` prop is true', async () => { - createComponent({ fetchByIid: true }); - await waitForPromises(); - - expect(workItemQuerySuccess).not.toHaveBeenCalled(); - expect(workItemByIidResponseHandler).toHaveBeenCalled(); }); - it('skips calling the handlers when missing the needed queryVariables', async () => { - createComponent({ queryVariables: {}, fetchByIid: false }); + it('skips calling the work item query when missing queryVariables', async () => { + createComponent({ queryVariables: {} }); await waitForPromises(); expect(workItemQuerySuccess).not.toHaveBeenCalled(); diff --git a/spec/frontend/work_items/components/work_item_milestone_spec.js b/spec/frontend/work_items/components/work_item_milestone_spec.js index 5997de01274..95b0aee1315 100644 --- a/spec/frontend/work_items/components/work_item_milestone_spec.js +++ b/spec/frontend/work_items/components/work_item_milestone_spec.js @@ -9,27 +9,20 @@ import { import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import WorkItemMilestone from '~/work_items/components/work_item_milestone.vue'; -import { resolvers, config } from '~/graphql_shared/issuable_client'; import createMockApollo from 'helpers/mock_apollo_helper'; import { mockTracking } from 'helpers/tracking_helper'; import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { TRACKING_CATEGORY_SHOW } from '~/work_items/constants'; +import updateWorkItemMutation from '~/work_items/graphql/update_work_item.mutation.graphql'; import projectMilestonesQuery from '~/sidebar/queries/project_milestones.query.graphql'; import { projectMilestonesResponse, projectMilestonesResponseWithNoMilestones, mockMilestoneWidgetResponse, - workItemResponseFactory, updateWorkItemMutationErrorResponse, - workItemMilestoneSubscriptionResponse, - projectWorkItemResponse, updateWorkItemMutationResponse, -} from 'jest/work_items/mock_data'; -import workItemQuery from '~/work_items/graphql/work_item.query.graphql'; -import workItemByIidQuery from '~/work_items/graphql/work_item_by_iid.query.graphql'; -import workItemMilestoneSubscription from '~/work_items/graphql/work_item_milestone.subscription.graphql'; -import updateWorkItemMutation from '~/work_items/graphql/update_work_item.mutation.graphql'; +} from '../mock_data'; describe('WorkItemMilestone component', () => { Vue.use(VueApollo); @@ -52,72 +45,34 @@ describe('WorkItemMilestone component', () => { const findDropdownTextAtIndex = (index) => findDropdownTexts().at(index); const findInputGroup = () => wrapper.findComponent(GlFormGroup); - const workItemQueryResponse = workItemResponseFactory({ canUpdate: true, canDelete: true }); - const workItemQueryHandler = jest.fn().mockResolvedValue(workItemQueryResponse); - const workItemByIidResponseHandler = jest.fn().mockResolvedValue(projectWorkItemResponse); - - const networkResolvedValue = new Error(); - const successSearchQueryHandler = jest.fn().mockResolvedValue(projectMilestonesResponse); const successSearchWithNoMatchingMilestones = jest .fn() .mockResolvedValue(projectMilestonesResponseWithNoMilestones); - const milestoneSubscriptionHandler = jest - .fn() - .mockResolvedValue(workItemMilestoneSubscriptionResponse); const successUpdateWorkItemMutationHandler = jest .fn() .mockResolvedValue(updateWorkItemMutationResponse); - const showDropdown = () => { - findDropdown().vm.$emit('shown'); - }; - - const hideDropdown = () => { - findDropdown().vm.$emit('hide'); - }; + const showDropdown = () => findDropdown().vm.$emit('shown'); + const hideDropdown = () => findDropdown().vm.$emit('hide'); const createComponent = ({ canUpdate = true, milestone = mockMilestoneWidgetResponse, searchQueryHandler = successSearchQueryHandler, - fetchByIid = false, mutationHandler = successUpdateWorkItemMutationHandler, } = {}) => { - const apolloProvider = createMockApollo( - [ - [workItemQuery, workItemQueryHandler], - [workItemMilestoneSubscription, milestoneSubscriptionHandler], + wrapper = shallowMountExtended(WorkItemMilestone, { + apolloProvider: createMockApollo([ [projectMilestonesQuery, searchQueryHandler], [updateWorkItemMutation, mutationHandler], - [workItemByIidQuery, workItemByIidResponseHandler], - ], - resolvers, - { - typePolicies: config.cacheConfig.typePolicies, - }, - ); - - apolloProvider.clients.defaultClient.writeQuery({ - query: workItemQuery, - variables: { - id: workItemId, - }, - data: workItemQueryResponse.data, - }); - - wrapper = shallowMountExtended(WorkItemMilestone, { - apolloProvider, + ]), propsData: { canUpdate, workItemMilestone: milestone, workItemId, workItemType, fullPath, - queryVariables: { - id: workItemId, - }, - fetchByIid, }, stubs: { GlDropdown, @@ -244,7 +199,7 @@ describe('WorkItemMilestone component', () => { it.each` errorType | expectedErrorMessage | mockValue | resolveFunction ${'graphql error'} | ${'Something went wrong while updating the task. Please try again.'} | ${updateWorkItemMutationErrorResponse} | ${'mockResolvedValue'} - ${'network error'} | ${'Something went wrong while updating the task. Please try again.'} | ${networkResolvedValue} | ${'mockRejectedValue'} + ${'network error'} | ${'Something went wrong while updating the task. Please try again.'} | ${new Error()} | ${'mockRejectedValue'} `( 'emits an error when there is a $errorType', async ({ mockValue, expectedErrorMessage, resolveFunction }) => { diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb index ce2c6d0477b..7f3eefbe0c8 100644 --- a/spec/helpers/application_helper_spec.rb +++ b/spec/helpers/application_helper_spec.rb @@ -702,16 +702,16 @@ RSpec.describe ApplicationHelper do expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" media="print" href="/stylesheets/test.css" />') end - it 'uses regular stylesheet when feature flag disabled' do + it 'uses regular stylesheet when feature flag enabled' do stub_feature_flags(remove_startup_css: true) - expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" media="screen" href="/stylesheets/test.css" />') + expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" media="all" href="/stylesheets/test.css" />') end it 'uses regular stylesheet when no_startup_css param present' do allow(helper.controller).to receive(:params).and_return({ no_startup_css: '' }) - expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" media="screen" href="/stylesheets/test.css" />') + expect(helper.stylesheet_link_tag_defer('test')).to eq( '<link rel="stylesheet" media="all" href="/stylesheets/test.css" />') end end diff --git a/spec/helpers/commits_helper_spec.rb b/spec/helpers/commits_helper_spec.rb index c947d11e9de..2d06f42dee4 100644 --- a/spec/helpers/commits_helper_spec.rb +++ b/spec/helpers/commits_helper_spec.rb @@ -341,27 +341,6 @@ RSpec.describe CommitsHelper do ]) end - context 'when the show_tags_on_commits_view flag is disabled' do - before do - stub_feature_flags(show_tags_on_commits_view: false) - end - - specify do - expect(subject).to eq([ - commit, - commit.author, - ref, - { - merge_request: merge_request.cache_key, - pipeline_status: pipeline.cache_key, - xhr: true, - controller: "commits", - path: current_path - } - ]) - end - end - describe "final cache key output" do subject { ActiveSupport::Cache.expand_cache_key(cache_key) } diff --git a/spec/lib/gitlab/background_migration/recalculate_vulnerabilities_occurrences_uuid_spec.rb b/spec/lib/gitlab/background_migration/recalculate_vulnerabilities_occurrences_uuid_spec.rb deleted file mode 100644 index 543dd204f89..00000000000 --- a/spec/lib/gitlab/background_migration/recalculate_vulnerabilities_occurrences_uuid_spec.rb +++ /dev/null @@ -1,530 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -def create_background_migration_job(ids, status) - proper_status = case status - when :pending - Gitlab::Database::BackgroundMigrationJob.statuses['pending'] - when :succeeded - Gitlab::Database::BackgroundMigrationJob.statuses['succeeded'] - else - raise ArgumentError - end - - background_migration_jobs.create!( - class_name: 'RecalculateVulnerabilitiesOccurrencesUuid', - arguments: Array(ids), - status: proper_status, - created_at: Time.now.utc - ) -end - -RSpec.describe Gitlab::BackgroundMigration::RecalculateVulnerabilitiesOccurrencesUuid, :suppress_gitlab_schemas_validate_connection, schema: 20211202041233 do - let(:background_migration_jobs) { table(:background_migration_jobs) } - let(:pending_jobs) { background_migration_jobs.where(status: Gitlab::Database::BackgroundMigrationJob.statuses['pending']) } - let(:succeeded_jobs) { background_migration_jobs.where(status: Gitlab::Database::BackgroundMigrationJob.statuses['succeeded']) } - let(:namespace) { table(:namespaces).create!(name: 'user', path: 'user') } - let(:users) { table(:users) } - let(:user) { create_user! } - let(:project) { table(:projects).create!(id: 123, namespace_id: namespace.id) } - let(:scanners) { table(:vulnerability_scanners) } - let(:scanner) { scanners.create!(project_id: project.id, external_id: 'test 1', name: 'test scanner 1') } - let(:scanner2) { scanners.create!(project_id: project.id, external_id: 'test 2', name: 'test scanner 2') } - let(:vulnerabilities) { table(:vulnerabilities) } - let(:vulnerability_findings) { table(:vulnerability_occurrences) } - let(:vulnerability_finding_pipelines) { table(:vulnerability_occurrence_pipelines) } - let(:vulnerability_finding_signatures) { table(:vulnerability_finding_signatures) } - let(:vulnerability_identifiers) { table(:vulnerability_identifiers) } - - let(:identifier_1) { 'identifier-1' } - let!(:vulnerability_identifier) do - vulnerability_identifiers.create!( - project_id: project.id, - external_type: identifier_1, - external_id: identifier_1, - fingerprint: Gitlab::Database::ShaAttribute.serialize('ff9ef548a6e30a0462795d916f3f00d1e2b082ca'), - name: 'Identifier 1') - end - - let(:identifier_2) { 'identifier-2' } - let!(:vulnerability_identfier2) do - vulnerability_identifiers.create!( - project_id: project.id, - external_type: identifier_2, - external_id: identifier_2, - fingerprint: Gitlab::Database::ShaAttribute.serialize('4299e8ddd819f9bde9cfacf45716724c17b5ddf7'), - name: 'Identifier 2') - end - - let(:identifier_3) { 'identifier-3' } - let!(:vulnerability_identifier3) do - vulnerability_identifiers.create!( - project_id: project.id, - external_type: identifier_3, - external_id: identifier_3, - fingerprint: Gitlab::Database::ShaAttribute.serialize('8e91632f9c6671e951834a723ee221c44cc0d844'), - name: 'Identifier 3') - end - - let(:known_uuid_v4) { "b3cc2518-5446-4dea-871c-89d5e999c1ac" } - let(:known_uuid_v5) { "05377088-dc26-5161-920e-52a7159fdaa1" } - let(:desired_uuid_v5) { "f3e9a23f-9181-54bf-a5ab-c5bc7a9b881a" } - - subject { described_class.new.perform(start_id, end_id) } - - context "when finding has a UUIDv4" do - before do - @uuid_v4 = create_finding!( - vulnerability_id: nil, - project_id: project.id, - scanner_id: scanner2.id, - primary_identifier_id: vulnerability_identfier2.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize("fa18f432f1d56675f4098d318739c3cd5b14eb3e"), - uuid: known_uuid_v4 - ) - end - - let(:start_id) { @uuid_v4.id } - let(:end_id) { @uuid_v4.id } - - it "replaces it with UUIDv5" do - expect(vulnerability_findings.pluck(:uuid)).to match_array([known_uuid_v4]) - - subject - - expect(vulnerability_findings.pluck(:uuid)).to match_array([desired_uuid_v5]) - end - - it 'logs recalculation' do - expect_next_instance_of(Gitlab::BackgroundMigration::Logger) do |instance| - expect(instance).to receive(:info).twice - end - - subject - end - end - - context "when finding has a UUIDv5" do - before do - @uuid_v5 = create_finding!( - vulnerability_id: nil, - project_id: project.id, - scanner_id: scanner.id, - primary_identifier_id: vulnerability_identifier.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize("838574be0210968bf6b9f569df9c2576242cbf0a"), - uuid: known_uuid_v5 - ) - end - - let(:start_id) { @uuid_v5.id } - let(:end_id) { @uuid_v5.id } - - it "stays the same" do - expect(vulnerability_findings.pluck(:uuid)).to match_array([known_uuid_v5]) - - subject - - expect(vulnerability_findings.pluck(:uuid)).to match_array([known_uuid_v5]) - end - end - - context 'if a duplicate UUID would be generated' do # rubocop: disable RSpec/MultipleMemoizedHelpers - let(:v1) do - create_vulnerability!( - project_id: project.id, - author_id: user.id - ) - end - - let!(:finding_with_incorrect_uuid) do - create_finding!( - vulnerability_id: v1.id, - project_id: project.id, - scanner_id: scanner.id, - primary_identifier_id: vulnerability_identifier.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize('ca41a2544e941a007a73a666cb0592b255316ab8'), # sha1('youshouldntusethis') - uuid: 'bd95c085-71aa-51d7-9bb6-08ae669c262e' - ) - end - - let(:v2) do - create_vulnerability!( - project_id: project.id, - author_id: user.id - ) - end - - let!(:finding_with_correct_uuid) do - create_finding!( - vulnerability_id: v2.id, - project_id: project.id, - primary_identifier_id: vulnerability_identifier.id, - scanner_id: scanner2.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize('ca41a2544e941a007a73a666cb0592b255316ab8'), # sha1('youshouldntusethis') - uuid: '91984483-5efe-5215-b471-d524ac5792b1' - ) - end - - let(:v3) do - create_vulnerability!( - project_id: project.id, - author_id: user.id - ) - end - - let!(:finding_with_incorrect_uuid2) do - create_finding!( - vulnerability_id: v3.id, - project_id: project.id, - scanner_id: scanner.id, - primary_identifier_id: vulnerability_identfier2.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize('ca41a2544e941a007a73a666cb0592b255316ab8'), # sha1('youshouldntusethis') - uuid: '00000000-1111-2222-3333-444444444444' - ) - end - - let(:v4) do - create_vulnerability!( - project_id: project.id, - author_id: user.id - ) - end - - let!(:finding_with_correct_uuid2) do - create_finding!( - vulnerability_id: v4.id, - project_id: project.id, - scanner_id: scanner2.id, - primary_identifier_id: vulnerability_identfier2.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize('ca41a2544e941a007a73a666cb0592b255316ab8'), # sha1('youshouldntusethis') - uuid: '1edd751e-ef9a-5391-94db-a832c8635bfc' - ) - end - - let!(:finding_with_incorrect_uuid3) do - create_finding!( - vulnerability_id: nil, - project_id: project.id, - scanner_id: scanner.id, - primary_identifier_id: vulnerability_identifier3.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize('ca41a2544e941a007a73a666cb0592b255316ab8'), # sha1('youshouldntusethis') - uuid: '22222222-3333-4444-5555-666666666666' - ) - end - - let!(:duplicate_not_in_the_same_batch) do - create_finding!( - id: 99999, - vulnerability_id: nil, - project_id: project.id, - scanner_id: scanner2.id, - primary_identifier_id: vulnerability_identifier3.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize('ca41a2544e941a007a73a666cb0592b255316ab8'), # sha1('youshouldntusethis') - uuid: '4564f9d5-3c6b-5cc3-af8c-7c25285362a7' - ) - end - - let(:start_id) { finding_with_incorrect_uuid.id } - let(:end_id) { finding_with_incorrect_uuid3.id } - - before do - 4.times do - create_finding_pipeline!(project_id: project.id, finding_id: finding_with_incorrect_uuid.id) - create_finding_pipeline!(project_id: project.id, finding_id: finding_with_correct_uuid.id) - create_finding_pipeline!(project_id: project.id, finding_id: finding_with_incorrect_uuid2.id) - create_finding_pipeline!(project_id: project.id, finding_id: finding_with_correct_uuid2.id) - end - end - - it 'drops duplicates and related records', :aggregate_failures do - expect(vulnerability_findings.pluck(:id)).to match_array( - [ - finding_with_correct_uuid.id, - finding_with_incorrect_uuid.id, - finding_with_correct_uuid2.id, - finding_with_incorrect_uuid2.id, - finding_with_incorrect_uuid3.id, - duplicate_not_in_the_same_batch.id - ]) - - expect { subject }.to change(vulnerability_finding_pipelines, :count).from(16).to(8) - .and change(vulnerability_findings, :count).from(6).to(3) - .and change(vulnerabilities, :count).from(4).to(2) - - expect(vulnerability_findings.pluck(:id)).to match_array([finding_with_incorrect_uuid.id, finding_with_incorrect_uuid2.id, finding_with_incorrect_uuid3.id]) - end - - context 'if there are conflicting UUID values within the batch' do # rubocop: disable RSpec/MultipleMemoizedHelpers - let(:end_id) { finding_with_broken_data_integrity.id } - let(:vulnerability_5) { create_vulnerability!(project_id: project.id, author_id: user.id) } - let(:different_project) { table(:projects).create!(namespace_id: namespace.id) } - let!(:identifier_with_broken_data_integrity) do - vulnerability_identifiers.create!( - project_id: different_project.id, - external_type: identifier_2, - external_id: identifier_2, - fingerprint: Gitlab::Database::ShaAttribute.serialize('4299e8ddd819f9bde9cfacf45716724c17b5ddf7'), - name: 'Identifier 2') - end - - let(:finding_with_broken_data_integrity) do - create_finding!( - vulnerability_id: vulnerability_5, - project_id: project.id, - scanner_id: scanner.id, - primary_identifier_id: identifier_with_broken_data_integrity.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize('ca41a2544e941a007a73a666cb0592b255316ab8'), # sha1('youshouldntusethis') - uuid: SecureRandom.uuid - ) - end - - it 'deletes the conflicting record' do - expect { subject }.to change { vulnerability_findings.find_by_id(finding_with_broken_data_integrity.id) }.to(nil) - end - end - - context 'if a conflicting UUID is found during the migration' do # rubocop:disable RSpec/MultipleMemoizedHelpers - let(:finding_class) { Gitlab::BackgroundMigration::RecalculateVulnerabilitiesOccurrencesUuid::VulnerabilitiesFinding } - let(:uuid) { '4564f9d5-3c6b-5cc3-af8c-7c25285362a7' } - - before do - exception = ActiveRecord::RecordNotUnique.new("(uuid)=(#{uuid})") - - call_count = 0 - allow(::Gitlab::Database::BulkUpdate).to receive(:execute) do - call_count += 1 - call_count.eql?(1) ? raise(exception) : {} - end - - allow(finding_class).to receive(:find_by).with(uuid: uuid).and_return(duplicate_not_in_the_same_batch) - end - - it 'retries the recalculation' do - subject - - expect(Gitlab::BackgroundMigration::RecalculateVulnerabilitiesOccurrencesUuid::VulnerabilitiesFinding) - .to have_received(:find_by).with(uuid: uuid).once - end - - it 'logs the conflict' do - expect_next_instance_of(Gitlab::BackgroundMigration::Logger) do |instance| - expect(instance).to receive(:info).exactly(6).times - end - - subject - end - - it 'marks the job as done' do - create_background_migration_job([start_id, end_id], :pending) - - subject - - expect(pending_jobs.count).to eq(0) - expect(succeeded_jobs.count).to eq(1) - end - end - - it 'logs an exception if a different uniquness problem was found' do - exception = ActiveRecord::RecordNotUnique.new("Totally not an UUID uniqueness problem") - allow(::Gitlab::Database::BulkUpdate).to receive(:execute).and_raise(exception) - allow(Gitlab::ErrorTracking).to receive(:track_and_raise_exception) - - subject - - expect(Gitlab::ErrorTracking).to have_received(:track_and_raise_exception).with(exception).once - end - - it 'logs a duplicate found message' do - expect_next_instance_of(Gitlab::BackgroundMigration::Logger) do |instance| - expect(instance).to receive(:info).exactly(3).times - end - - subject - end - end - - context 'when finding has a signature' do - before do - @f1 = create_finding!( - vulnerability_id: nil, - project_id: project.id, - scanner_id: scanner.id, - primary_identifier_id: vulnerability_identifier.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize('ca41a2544e941a007a73a666cb0592b255316ab8'), # sha1('youshouldntusethis') - uuid: 'd15d774d-e4b1-5a1b-929b-19f2a53e35ec' - ) - - vulnerability_finding_signatures.create!( - finding_id: @f1.id, - algorithm_type: 2, # location - signature_sha: Gitlab::Database::ShaAttribute.serialize('57d4e05205f6462a73f039a5b2751aa1ab344e6e') # sha1('youshouldusethis') - ) - - vulnerability_finding_signatures.create!( - finding_id: @f1.id, - algorithm_type: 1, # hash - signature_sha: Gitlab::Database::ShaAttribute.serialize('c554d8d8df1a7a14319eafdaae24af421bf5b587') # sha1('andnotthis') - ) - - @f2 = create_finding!( - vulnerability_id: nil, - project_id: project.id, - scanner_id: scanner.id, - primary_identifier_id: vulnerability_identfier2.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize('ca41a2544e941a007a73a666cb0592b255316ab8'), # sha1('youshouldntusethis') - uuid: '4be029b5-75e5-5ac0-81a2-50ab41726135' - ) - - vulnerability_finding_signatures.create!( - finding_id: @f2.id, - algorithm_type: 2, # location - signature_sha: Gitlab::Database::ShaAttribute.serialize('57d4e05205f6462a73f039a5b2751aa1ab344e6e') # sha1('youshouldusethis') - ) - - vulnerability_finding_signatures.create!( - finding_id: @f2.id, - algorithm_type: 1, # hash - signature_sha: Gitlab::Database::ShaAttribute.serialize('c554d8d8df1a7a14319eafdaae24af421bf5b587') # sha1('andnotthis') - ) - end - - let(:start_id) { @f1.id } - let(:end_id) { @f2.id } - - let(:uuids_before) { [@f1.uuid, @f2.uuid] } - let(:uuids_after) { %w[d3b60ddd-d312-5606-b4d3-ad058eebeacb 349d9bec-c677-5530-a8ac-5e58889c3b1a] } - - it 'is recalculated using signature' do - expect(vulnerability_findings.pluck(:uuid)).to match_array(uuids_before) - - subject - - expect(vulnerability_findings.pluck(:uuid)).to match_array(uuids_after) - end - end - - context 'if all records are removed before the job ran' do - let(:start_id) { 1 } - let(:end_id) { 9 } - - before do - create_background_migration_job([start_id, end_id], :pending) - end - - it 'does not error out' do - expect { subject }.not_to raise_error - end - - it 'marks the job as done' do - subject - - expect(pending_jobs.count).to eq(0) - expect(succeeded_jobs.count).to eq(1) - end - end - - context 'when recalculation fails' do - before do - @uuid_v4 = create_finding!( - vulnerability_id: nil, - project_id: project.id, - scanner_id: scanner2.id, - primary_identifier_id: vulnerability_identfier2.id, - report_type: 0, # "sast" - location_fingerprint: Gitlab::Database::ShaAttribute.serialize("fa18f432f1d56675f4098d318739c3cd5b14eb3e"), - uuid: known_uuid_v4 - ) - - allow(Gitlab::ErrorTracking).to receive(:track_and_raise_exception) - allow(::Gitlab::Database::BulkUpdate).to receive(:execute).and_raise(expected_error) - end - - let(:start_id) { @uuid_v4.id } - let(:end_id) { @uuid_v4.id } - let(:expected_error) { RuntimeError.new } - - it 'captures the errors and does not crash entirely' do - expect { subject }.not_to raise_error - - allow(Gitlab::ErrorTracking).to receive(:track_and_raise_exception) - expect(Gitlab::ErrorTracking).to have_received(:track_and_raise_exception).with(expected_error).once - end - - it_behaves_like 'marks background migration job records' do - let(:arguments) { [1, 4] } - subject { described_class.new } - end - end - - it_behaves_like 'marks background migration job records' do - let(:arguments) { [1, 4] } - subject { described_class.new } - end - - private - - def create_vulnerability!(project_id:, author_id:, title: 'test', severity: 7, confidence: 7, report_type: 0) - vulnerabilities.create!( - project_id: project_id, - author_id: author_id, - title: title, - severity: severity, - confidence: confidence, - report_type: report_type - ) - end - - # rubocop:disable Metrics/ParameterLists - def create_finding!( - vulnerability_id:, project_id:, scanner_id:, primary_identifier_id:, id: nil, - name: "test", severity: 7, confidence: 7, report_type: 0, - project_fingerprint: '123qweasdzxc', location_fingerprint: 'test', - metadata_version: 'test', raw_metadata: 'test', uuid: SecureRandom.uuid) - vulnerability_findings.create!({ - id: id, - vulnerability_id: vulnerability_id, - project_id: project_id, - name: name, - severity: severity, - confidence: confidence, - report_type: report_type, - project_fingerprint: project_fingerprint, - scanner_id: scanner_id, - primary_identifier_id: primary_identifier_id, - location_fingerprint: location_fingerprint, - metadata_version: metadata_version, - raw_metadata: raw_metadata, - uuid: uuid - }.compact - ) - end - # rubocop:enable Metrics/ParameterLists - - def create_user!(name: "Example User", email: "user@example.com", user_type: nil, created_at: Time.zone.now, confirmed_at: Time.zone.now) - users.create!( - name: name, - email: email, - username: name, - projects_limit: 0, - user_type: user_type, - confirmed_at: confirmed_at - ) - end - - def create_finding_pipeline!(project_id:, finding_id:) - pipeline = table(:ci_pipelines).create!(project_id: project_id) - vulnerability_finding_pipelines.create!(pipeline_id: pipeline.id, occurrence_id: finding_id) - end -end diff --git a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb index 56fabe854f9..3e76b4ae698 100644 --- a/spec/lib/gitlab/github_import/representation/diff_note_spec.rb +++ b/spec/lib/gitlab/github_import/representation/diff_note_spec.rb @@ -131,7 +131,7 @@ RSpec.describe Gitlab::GithubImport::Representation::DiffNote, :clean_gitlab_red describe '#github_identifiers' do it 'returns a hash with needed identifiers' do expect(note.github_identifiers).to eq( - noteable_id: 42, + noteable_iid: 42, noteable_type: 'MergeRequest', note_id: 1 ) diff --git a/spec/lib/gitlab/github_import/representation/issue_event_spec.rb b/spec/lib/gitlab/github_import/representation/issue_event_spec.rb index 33f0c6d3c64..6620dee0fd0 100644 --- a/spec/lib/gitlab/github_import/representation/issue_event_spec.rb +++ b/spec/lib/gitlab/github_import/representation/issue_event_spec.rb @@ -158,7 +158,7 @@ RSpec.describe Gitlab::GithubImport::Representation::IssueEvent do it 'returns a hash with needed identifiers' do expect(issue_event.github_identifiers).to eq( id: 6501124486, - iid: 2, + issuable_iid: 2, event: 'closed' ) end diff --git a/spec/lib/gitlab/github_import/representation/note_spec.rb b/spec/lib/gitlab/github_import/representation/note_spec.rb index 49126dbe9c5..5c2cea3653f 100644 --- a/spec/lib/gitlab/github_import/representation/note_spec.rb +++ b/spec/lib/gitlab/github_import/representation/note_spec.rb @@ -43,6 +43,16 @@ RSpec.describe Gitlab::GithubImport::Representation::Note do it 'includes the note ID' do expect(note.note_id).to eq(1) end + + describe '#github_identifiers' do + it 'returns a hash with needed identifiers' do + expect(note.github_identifiers).to eq( + noteable_iid: 42, + noteable_type: 'Issue', + note_id: 1 + ) + end + end end end @@ -103,18 +113,4 @@ RSpec.describe Gitlab::GithubImport::Representation::Note do expect(note.author).to be_nil end end - - describe '#github_identifiers' do - it 'returns a hash with needed identifiers' do - github_identifiers = { - noteable_id: 42, - noteable_type: 'Issue', - note_id: 1 - } - other_attributes = { something_else: '_something_else_' } - note = described_class.new(github_identifiers.merge(other_attributes)) - - expect(note.github_identifiers).to eq(github_identifiers) - end - end end diff --git a/spec/lib/gitlab/sidekiq_daemon/memory_killer_spec.rb b/spec/lib/gitlab/sidekiq_daemon/memory_killer_spec.rb deleted file mode 100644 index 6f46a5aea3b..00000000000 --- a/spec/lib/gitlab/sidekiq_daemon/memory_killer_spec.rb +++ /dev/null @@ -1,562 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::SidekiqDaemon::MemoryKiller do - let(:memory_killer) { described_class.new } - let(:sidekiq_daemon_monitor) { instance_double(Gitlab::SidekiqDaemon::Monitor) } - let(:running_jobs) { {} } - let(:pid) { 12345 } - let(:worker) do - Class.new do - def self.name - 'DummyWorker' - end - end - end - - before do - stub_const('DummyWorker', worker) - allow(Sidekiq.logger).to receive(:info) - allow(Sidekiq.logger).to receive(:warn) - allow(Gitlab::SidekiqDaemon::Monitor).to receive(:instance).and_return(sidekiq_daemon_monitor) - allow(sidekiq_daemon_monitor).to receive(:jobs).and_return(running_jobs) - allow(memory_killer).to receive(:pid).and_return(pid) - - # make sleep no-op - allow(memory_killer).to receive(:sleep) {} - end - - describe '#run_thread' do - subject { memory_killer.send(:run_thread) } - - before do - # let enabled? return 3 times: true, true, false - allow(memory_killer).to receive(:enabled?).and_return(true, true, false) - end - - context 'when structured logging is used' do - it 'logs start message once' do - expect(Sidekiq.logger).to receive(:info).once - .with( - class: described_class.to_s, - action: 'start', - pid: pid, - message: 'Starting Gitlab::SidekiqDaemon::MemoryKiller Daemon') - - subject - end - - it 'logs StandardError message twice' do - expect(Sidekiq.logger).to receive(:warn).twice - .with( - class: described_class.to_s, - pid: pid, - message: "Exception from run_thread: My Exception") - - expect(memory_killer).to receive(:rss_within_range?) - .twice - .and_raise(StandardError, 'My Exception') - - expect { subject }.not_to raise_exception - end - - it 'logs exception message once and raise exception and log stop message' do - expect(Sidekiq.logger).to receive(:warn).once - .with( - class: described_class.to_s, - pid: pid, - message: "Exception from run_thread: My Exception") - - expect(memory_killer).to receive(:rss_within_range?) - .once - .and_raise(Exception, 'My Exception') - - expect(memory_killer).to receive(:sleep).with(Gitlab::SidekiqDaemon::MemoryKiller::CHECK_INTERVAL_SECONDS) - expect(Sidekiq.logger).to receive(:warn).once - .with( - class: described_class.to_s, - action: 'stop', - pid: pid, - message: 'Stopping Gitlab::SidekiqDaemon::MemoryKiller Daemon') - - expect { subject }.to raise_exception(Exception, 'My Exception') - end - - it 'logs stop message once' do - expect(Sidekiq.logger).to receive(:warn).once - .with( - class: described_class.to_s, - action: 'stop', - pid: pid, - message: 'Stopping Gitlab::SidekiqDaemon::MemoryKiller Daemon') - - subject - end - end - - it 'not invoke restart_sidekiq when rss in range' do - expect(memory_killer).to receive(:rss_within_range?) - .twice - .and_return(true) - - expect(memory_killer).not_to receive(:restart_sidekiq) - - subject - end - - it 'invoke restart_sidekiq when rss not in range' do - expect(memory_killer).to receive(:rss_within_range?) - .at_least(:once) - .and_return(false) - - expect(memory_killer).to receive(:restart_sidekiq) - .at_least(:once) - - subject - end - end - - describe '#stop_working' do - subject { memory_killer.send(:stop_working) } - - it 'changes enable? to false' do - expect { subject }.to change { memory_killer.send(:enabled?) } - .from(true).to(false) - end - end - - describe '#rss_within_range?' do - let(:shutdown_timeout_seconds) { 7 } - let(:check_interval_seconds) { 2 } - let(:grace_balloon_seconds) { 5 } - - subject { memory_killer.send(:rss_within_range?) } - - before do - stub_const("#{described_class}::SHUTDOWN_TIMEOUT_SECONDS", shutdown_timeout_seconds) - stub_const("#{described_class}::CHECK_INTERVAL_SECONDS", check_interval_seconds) - stub_const("#{described_class}::GRACE_BALLOON_SECONDS", grace_balloon_seconds) - allow(Process).to receive(:getpgrp).and_return(pid) - allow(Sidekiq).to receive(:[]).with(:timeout).and_return(9) - end - - it 'return true when everything is within limit', :aggregate_failures do - expect(memory_killer).to receive(:get_rss_kb).and_return(100) - expect(memory_killer).to receive(:get_soft_limit_rss_kb).and_return(200) - expect(memory_killer).to receive(:get_hard_limit_rss_kb).and_return(300) - expect(memory_killer).to receive(:get_memory_total_kb).and_return(3072) - - expect(memory_killer).to receive(:refresh_state) - .with(:running) - .and_call_original - - expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_call_original - expect(memory_killer).not_to receive(:log_rss_out_of_range) - - expect(subject).to be true - end - - it 'return false when rss exceeds hard_limit_rss', :aggregate_failures do - expect(memory_killer).to receive(:get_rss_kb).at_least(:once).and_return(400) - expect(memory_killer).to receive(:get_soft_limit_rss_kb).at_least(:once).and_return(200) - expect(memory_killer).to receive(:get_hard_limit_rss_kb).at_least(:once).and_return(300) - expect(memory_killer).to receive(:get_memory_total_kb).at_least(:once).and_return(3072) - - expect(memory_killer).to receive(:refresh_state) - .with(:running) - .and_call_original - - expect(memory_killer).to receive(:refresh_state) - .with(:above_soft_limit) - .and_call_original - - expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_call_original - - expect(memory_killer).to receive(:out_of_range_description).with(400, 300, 200, true) - - expect(subject).to be false - end - - it 'return false when rss exceed hard_limit_rss after a while', :aggregate_failures do - expect(memory_killer).to receive(:get_rss_kb).and_return(250, 400, 400) - expect(memory_killer).to receive(:get_soft_limit_rss_kb).at_least(:once).and_return(200) - expect(memory_killer).to receive(:get_hard_limit_rss_kb).at_least(:once).and_return(300) - expect(memory_killer).to receive(:get_memory_total_kb).at_least(:once).and_return(3072) - - expect(memory_killer).to receive(:refresh_state) - .with(:running) - .and_call_original - - expect(memory_killer).to receive(:refresh_state) - .at_least(:once) - .with(:above_soft_limit) - .and_call_original - - expect(Gitlab::Metrics::System).to receive(:monotonic_time).twice.and_call_original - expect(memory_killer).to receive(:sleep).with(check_interval_seconds) - expect(memory_killer).to receive(:out_of_range_description).with(400, 300, 200, false) - expect(memory_killer).to receive(:out_of_range_description).with(400, 300, 200, true) - - expect(subject).to be false - end - - it 'return true when rss below soft_limit_rss after a while within GRACE_BALLOON_SECONDS', :aggregate_failures do - expect(memory_killer).to receive(:get_rss_kb).and_return(250, 100) - expect(memory_killer).to receive(:get_soft_limit_rss_kb).and_return(200, 200) - expect(memory_killer).to receive(:get_hard_limit_rss_kb).and_return(300, 300) - expect(memory_killer).to receive(:get_memory_total_kb).and_return(3072, 3072) - - expect(memory_killer).to receive(:refresh_state) - .with(:running) - .and_call_original - - expect(memory_killer).to receive(:refresh_state) - .with(:above_soft_limit) - .and_call_original - - expect(Gitlab::Metrics::System).to receive(:monotonic_time).twice.and_call_original - expect(memory_killer).to receive(:sleep).with(check_interval_seconds) - - expect(memory_killer).to receive(:out_of_range_description).with(100, 300, 200, false) - - expect(subject).to be true - end - - context 'when exceeds GRACE_BALLOON_SECONDS' do - let(:grace_balloon_seconds) { 0 } - - it 'return false when rss exceed soft_limit_rss', :aggregate_failures do - allow(memory_killer).to receive(:get_rss_kb).and_return(250) - allow(memory_killer).to receive(:get_soft_limit_rss_kb).and_return(200) - allow(memory_killer).to receive(:get_hard_limit_rss_kb).and_return(300) - allow(memory_killer).to receive(:get_memory_total_kb).and_return(3072) - - expect(memory_killer).to receive(:refresh_state) - .with(:running) - .and_call_original - - expect(memory_killer).to receive(:refresh_state) - .with(:above_soft_limit) - .and_call_original - - expect(memory_killer).to receive(:out_of_range_description).with(250, 300, 200, true) - - expect(subject).to be false - end - end - end - - describe '#restart_sidekiq' do - let(:shutdown_timeout_seconds) { 7 } - - subject { memory_killer.send(:restart_sidekiq) } - - context 'when sidekiq_memory_killer_read_only_mode is enabled' do - before do - stub_feature_flags(sidekiq_memory_killer_read_only_mode: true) - end - - it 'does not send signal' do - expect(memory_killer).not_to receive(:refresh_state) - expect(memory_killer).not_to receive(:signal_and_wait) - - subject - end - end - - context 'when sidekiq_memory_killer_read_only_mode is disabled' do - before do - stub_const("#{described_class}::SHUTDOWN_TIMEOUT_SECONDS", shutdown_timeout_seconds) - stub_feature_flags(sidekiq_memory_killer_read_only_mode: false) - allow(Sidekiq).to receive(:[]).with(:timeout).and_return(9) - allow(memory_killer).to receive(:get_rss_kb).and_return(100) - allow(memory_killer).to receive(:get_soft_limit_rss_kb).and_return(200) - allow(memory_killer).to receive(:get_hard_limit_rss_kb).and_return(300) - allow(memory_killer).to receive(:get_memory_total_kb).and_return(3072) - end - - it 'send signal' do - expect(memory_killer).to receive(:refresh_state) - .with(:stop_fetching_new_jobs) - .ordered - .and_call_original - expect(memory_killer).to receive(:signal_and_wait) - .with(shutdown_timeout_seconds, 'SIGTSTP', 'stop fetching new jobs') - .ordered - - expect(memory_killer).to receive(:refresh_state) - .with(:shutting_down) - .ordered - .and_call_original - expect(memory_killer).to receive(:signal_and_wait) - .with(11, 'SIGTERM', 'gracefully shut down') - .ordered - - expect(memory_killer).to receive(:refresh_state) - .with(:killing_sidekiq) - .ordered - .and_call_original - expect(memory_killer).to receive(:signal_pgroup) - .with('SIGKILL', 'die') - .ordered - - subject - end - end - end - - describe '#signal_and_wait' do - let(:time) { 0.1 } - let(:signal) { 'my-signal' } - let(:explanation) { 'my-explanation' } - let(:check_interval_seconds) { 0.1 } - - subject { memory_killer.send(:signal_and_wait, time, signal, explanation) } - - before do - stub_const("#{described_class}::CHECK_INTERVAL_SECONDS", check_interval_seconds) - end - - it 'send signal and wait till deadline' do - expect(Process).to receive(:kill) - .with(signal, pid) - .ordered - - expect(Gitlab::Metrics::System).to receive(:monotonic_time) - .and_call_original - .at_least(3) - - expect(memory_killer).to receive(:enabled?).and_return(true).at_least(:twice) - expect(memory_killer).to receive(:sleep).at_least(:once).and_call_original - - subject - end - end - - describe '#signal_pgroup' do - let(:signal) { 'my-signal' } - let(:explanation) { 'my-explanation' } - - subject { memory_killer.send(:signal_pgroup, signal, explanation) } - - it 'send signal to this process if it is not group leader' do - expect(Process).to receive(:getpgrp).and_return(pid + 1) - - expect(Sidekiq.logger).to receive(:warn).once - .with( - class: described_class.to_s, - signal: signal, - pid: pid, - message: "sending Sidekiq worker PID-#{pid} #{signal} (#{explanation})") - expect(Process).to receive(:kill).with(signal, pid).ordered - - subject - end - - it 'send signal to whole process group as group leader' do - expect(Process).to receive(:getpgrp).and_return(pid) - - expect(Sidekiq.logger).to receive(:warn).once - .with( - class: described_class.to_s, - signal: signal, - pid: pid, - message: "sending Sidekiq worker PGRP-#{pid} #{signal} (#{explanation})") - expect(Process).to receive(:kill).with(signal, 0).ordered - - subject - end - end - - describe '#log_rss_out_of_range' do - let(:current_rss) { 100 } - let(:soft_limit_rss) { 200 } - let(:hard_limit_rss) { 300 } - let(:memory_total) { 3072 } - let(:jid) { 1 } - let(:reason) { 'rss out of range reason description' } - let(:queue) { 'default' } - - let(:metrics) { memory_killer.instance_variable_get(:@metrics) } - let(:running_jobs) { { jid => { worker_class: DummyWorker } } } - - before do - allow(memory_killer).to receive(:get_rss_kb).and_return(*current_rss) - allow(memory_killer).to receive(:get_soft_limit_rss_kb).and_return(soft_limit_rss) - allow(memory_killer).to receive(:get_hard_limit_rss_kb).and_return(hard_limit_rss) - allow(memory_killer).to receive(:get_memory_total_kb).and_return(memory_total) - - memory_killer.send(:refresh_state, :running) - end - - subject { memory_killer.send(:log_rss_out_of_range) } - - it 'invoke sidekiq logger warn' do - expect(memory_killer).to receive(:out_of_range_description).with(current_rss, hard_limit_rss, soft_limit_rss, true).and_return(reason) - expect(Sidekiq.logger).to receive(:warn) - .with( - class: described_class.to_s, - pid: pid, - message: 'Sidekiq worker RSS out of range', - current_rss: current_rss, - hard_limit_rss: hard_limit_rss, - soft_limit_rss: soft_limit_rss, - reason: reason, - running_jobs: [jid: jid, worker_class: 'DummyWorker'], - memory_total_kb: memory_total) - - expect(metrics[:sidekiq_memory_killer_running_jobs]).to receive(:increment) - .with({ worker_class: "DummyWorker", deadline_exceeded: true }) - - subject - end - end - - describe '#out_of_range_description' do - let(:hard_limit) { 300 } - let(:soft_limit) { 200 } - let(:grace_balloon_seconds) { 12 } - let(:deadline_exceeded) { true } - - subject { memory_killer.send(:out_of_range_description, rss, hard_limit, soft_limit, deadline_exceeded) } - - context 'when rss > hard_limit' do - let(:rss) { 400 } - - it 'tells reason' do - expect(subject).to eq("current_rss(#{rss}) > hard_limit_rss(#{hard_limit})") - end - end - - context 'when rss <= hard_limit' do - let(:rss) { 300 } - - context 'deadline exceeded' do - let(:deadline_exceeded) { true } - - it 'tells reason' do - stub_const("#{described_class}::GRACE_BALLOON_SECONDS", grace_balloon_seconds) - expect(subject).to eq("current_rss(#{rss}) > soft_limit_rss(#{soft_limit}) longer than GRACE_BALLOON_SECONDS(#{grace_balloon_seconds})") - end - end - - context 'deadline not exceeded' do - let(:deadline_exceeded) { false } - - it 'tells reason' do - expect(subject).to eq("current_rss(#{rss}) > soft_limit_rss(#{soft_limit})") - end - end - end - end - - describe '#rss_increase_by_jobs' do - let(:running_jobs) { { 'job1' => { worker_class: "Job1" }, 'job2' => { worker_class: "Job2" } } } - - subject { memory_killer.send(:rss_increase_by_jobs) } - - before do - allow(memory_killer).to receive(:rss_increase_by_job).and_return(11, 22) - end - - it 'adds up individual rss_increase_by_job' do - expect(subject).to eq(33) - end - - context 'when there is no running job' do - let(:running_jobs) { {} } - - it 'return 0 if no job' do - expect(subject).to eq(0) - end - end - end - - describe '#rss_increase_by_job' do - let(:worker_class) { Chaos::SleepWorker } - let(:job) { { worker_class: worker_class, started_at: 321 } } - let(:max_memory_kb) { 100000 } - - subject { memory_killer.send(:rss_increase_by_job, job) } - - before do - stub_const("#{described_class}::DEFAULT_MAX_MEMORY_GROWTH_KB", max_memory_kb) - end - - it 'return 0 if memory_growth_kb return 0' do - expect(memory_killer).to receive(:get_job_options).with(job, 'memory_killer_memory_growth_kb', 0).and_return(0) - expect(memory_killer).to receive(:get_job_options).with(job, 'memory_killer_max_memory_growth_kb', max_memory_kb).and_return(0) - - expect(Time).not_to receive(:now) - expect(subject).to eq(0) - end - - it 'return time factored growth value when it does not exceed max growth limit for whilited job' do - expect(memory_killer).to receive(:get_job_options).with(job, 'memory_killer_memory_growth_kb', 0).and_return(10) - expect(memory_killer).to receive(:get_job_options).with(job, 'memory_killer_max_memory_growth_kb', max_memory_kb).and_return(100) - - expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(323) - expect(subject).to eq(20) - end - - it 'return max growth limit when time factored growth value exceed max growth limit for whilited job' do - expect(memory_killer).to receive(:get_job_options).with(job, 'memory_killer_memory_growth_kb', 0).and_return(10) - expect(memory_killer).to receive(:get_job_options).with(job, 'memory_killer_max_memory_growth_kb', max_memory_kb).and_return(100) - - expect(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(332) - expect(subject).to eq(100) - end - end - - describe '#get_job_options' do - let(:worker_class) { Chaos::SleepWorker } - let(:job) { { worker_class: worker_class, started_at: 321 } } - let(:key) { 'my-key' } - let(:default) { 'my-default' } - - subject { memory_killer.send(:get_job_options, job, key, default) } - - it 'return default if key is not defined' do - expect(worker_class).to receive(:sidekiq_options).and_return({ "retry" => 5 }) - - expect(subject).to eq(default) - end - - it 'return default if get StandardError when retrieve sidekiq_options' do - expect(worker_class).to receive(:sidekiq_options).and_raise(StandardError) - - expect(subject).to eq(default) - end - - it 'return right value if sidekiq_options has the key' do - expect(worker_class).to receive(:sidekiq_options).and_return({ key => 10 }) - - expect(subject).to eq(10) - end - end - - describe '#refresh_state' do - let(:metrics) { memory_killer.instance_variable_get(:@metrics) } - - subject { memory_killer.send(:refresh_state, :shutting_down) } - - it 'calls gitlab metrics gauge set methods' do - expect(memory_killer).to receive(:get_rss_kb) { 1010 } - expect(memory_killer).to receive(:get_soft_limit_rss_kb) { 1020 } - expect(memory_killer).to receive(:get_hard_limit_rss_kb) { 1040 } - expect(memory_killer).to receive(:get_memory_total_kb) { 3072 } - - expect(metrics[:sidekiq_memory_killer_phase]).to receive(:set) - .with({}, described_class::PHASE[:shutting_down]) - expect(metrics[:sidekiq_current_rss]).to receive(:set) - .with({}, 1010) - expect(metrics[:sidekiq_memory_killer_soft_limit_rss]).to receive(:set) - .with({}, 1020) - expect(metrics[:sidekiq_memory_killer_hard_limit_rss]).to receive(:set) - .with({}, 1040) - - subject - end - end -end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index f7fafd93f4b..a34657adf60 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -483,9 +483,11 @@ RSpec.describe Ci::JobArtifact, feature_category: :build_artifacts do context "when #{file_type} type with other formats" do described_class.file_formats.except(file_format).values.each do |other_format| - let(:artifact) { build(:ci_job_artifact, file_type: file_type, file_format: other_format) } + context "with #{other_format}" do + let(:artifact) { build(:ci_job_artifact, file_type: file_type, file_format: other_format) } - it { is_expected.not_to be_valid } + it { is_expected.not_to be_valid } + end end end end diff --git a/spec/models/concerns/ci/has_status_spec.rb b/spec/models/concerns/ci/has_status_spec.rb index 4ef690ca4c1..5e0a430aa13 100644 --- a/spec/models/concerns/ci/has_status_spec.rb +++ b/spec/models/concerns/ci/has_status_spec.rb @@ -393,24 +393,26 @@ RSpec.describe Ci::HasStatus, feature_category: :continuous_integration do subject { object.blocked? } %w[ci_pipeline ci_stage ci_build generic_commit_status].each do |type| - let(:object) { build(type, status: status) } + context "when #{type}" do + let(:object) { build(type, status: status) } - context 'when status is scheduled' do - let(:status) { :scheduled } + context 'when status is scheduled' do + let(:status) { :scheduled } - it { is_expected.to be_truthy } - end + it { is_expected.to be_truthy } + end - context 'when status is manual' do - let(:status) { :manual } + context 'when status is manual' do + let(:status) { :manual } - it { is_expected.to be_truthy } - end + it { is_expected.to be_truthy } + end - context 'when status is created' do - let(:status) { :created } + context 'when status is created' do + let(:status) { :created } - it { is_expected.to be_falsy } + it { is_expected.to be_falsy } + end end end end diff --git a/spec/models/import_failure_spec.rb b/spec/models/import_failure_spec.rb index 4c22ed2e10f..101da1212cf 100644 --- a/spec/models/import_failure_spec.rb +++ b/spec/models/import_failure_spec.rb @@ -8,8 +8,13 @@ RSpec.describe ImportFailure do let_it_be(:correlation_id) { 'ABC' } let_it_be(:hard_failure) { create(:import_failure, :hard_failure, project: project, correlation_id_value: correlation_id) } let_it_be(:soft_failure) { create(:import_failure, :soft_failure, project: project, correlation_id_value: correlation_id) } + let_it_be(:github_import_failure) { create(:import_failure, :github_import_failure, project: project) } let_it_be(:unrelated_failure) { create(:import_failure, project: project) } + it 'returns failures with external_identifiers' do + expect(ImportFailure.with_external_identifiers).to match_array([github_import_failure]) + end + it 'returns failures for the given correlation ID' do expect(ImportFailure.failures_by_correlation_id(correlation_id)).to match_array([hard_failure, soft_failure]) end diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 65afeb2cb56..4bec45de455 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do let_it_be(:user) { create(:user, :admin) } let_it_be(:another_admin) { create(:user, :admin) } - let_it_be(:group) { create(:group) } + let_it_be_with_reload(:group) { create(:group) } let_it_be(:active_instance_runner) do create(:ci_runner, :instance, :with_runner_manager, @@ -379,6 +379,7 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do end describe 'ephemeralRegisterUrl' do + let(:runner_args) { { registration_type: :authenticated_user, creator: creator } } let(:query) do %( query { @@ -403,54 +404,46 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do end end - context 'with an instance runner' do - context 'with registration available' do - let_it_be(:runner) { create(:ci_runner, registration_type: :authenticated_user) } + context 'with an instance runner', :freeze_time do + let(:creator) { user } + let(:runner) { create(:ci_runner, **runner_args) } + context 'with valid ephemeral registration' do it_behaves_like 'has register url' do let(:expected_url) { "http://localhost/admin/runners/#{runner.id}/register" } end end - context 'with no registration available' do - let_it_be(:runner) { create(:ci_runner) } + context 'when runner ephemeral registration has expired' do + let(:runner) do + create(:ci_runner, created_at: (Ci::Runner::REGISTRATION_AVAILABILITY_TIME + 1.second).ago, **runner_args) + end + + it_behaves_like 'has no register url' + end + + context 'when runner has already been registered' do + let(:runner) { create(:ci_runner, :with_runner_manager, **runner_args) } it_behaves_like 'has no register url' end end context 'with a group runner' do - context 'with registration available' do - let_it_be(:runner) { create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) } + let(:creator) { user } + let(:runner) { create(:ci_runner, :group, groups: [group], **runner_args) } + context 'with valid ephemeral registration' do it_behaves_like 'has register url' do let(:expected_url) { "http://localhost/groups/#{group.path}/-/runners/#{runner.id}/register" } end end - context 'with no group' do - let(:destroyed_group) { create(:group) } - let(:runner) { create(:ci_runner, :group, groups: [destroyed_group], registration_type: :authenticated_user) } - - before do - destroyed_group.destroy! - end - - it_behaves_like 'has no register url' - end - - context 'with no registration available' do - let_it_be(:runner) { create(:ci_runner, :group, groups: [group]) } - - it_behaves_like 'has no register url' - end - - context 'with no access' do - let_it_be(:user) { create(:user) } - let_it_be(:runner) { create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) } + context 'when request not from creator' do + let(:creator) { another_admin } before do - group.add_maintainer(user) + group.add_owner(another_admin) end it_behaves_like 'has no register url' @@ -458,37 +451,20 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do end context 'with a project runner' do - context 'with registration available' do - let_it_be(:runner) { create(:ci_runner, :project, projects: [project1], registration_type: :authenticated_user) } + let(:creator) { user } + let(:runner) { create(:ci_runner, :project, projects: [project1], **runner_args) } + context 'with valid ephemeral registration' do it_behaves_like 'has register url' do let(:expected_url) { "http://localhost/#{project1.full_path}/-/runners/#{runner.id}/register" } end end - context 'with no project' do - let(:destroyed_project) { create(:project) } - let(:runner) { create(:ci_runner, :project, projects: [destroyed_project], registration_type: :authenticated_user) } - - before do - destroyed_project.destroy! - end - - it_behaves_like 'has no register url' - end - - context 'with no registration available' do - let_it_be(:runner) { create(:ci_runner, :project, projects: [project1]) } - - it_behaves_like 'has no register url' - end - - context 'with no access' do - let_it_be(:user) { create(:user) } - let_it_be(:runner) { create(:ci_runner, :project, projects: [project1], registration_type: :authenticated_user) } + context 'when request not from creator' do + let(:creator) { another_admin } before do - group.add_maintainer(user) + project1.add_owner(another_admin) end it_behaves_like 'has no register url' @@ -1016,11 +992,11 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do describe 'sorting and pagination' do let(:query) do <<~GQL - query($id: CiRunnerID!, $projectSearchTerm: String, $n: Int, $cursor: String) { - runner(id: $id) { - #{fields} + query($id: CiRunnerID!, $projectSearchTerm: String, $n: Int, $cursor: String) { + runner(id: $id) { + #{fields} + } } - } GQL end @@ -1039,18 +1015,18 @@ RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do let(:fields) do <<~QUERY - projects(search: $projectSearchTerm, first: $n, after: $cursor) { - count - nodes { - id - } - pageInfo { - hasPreviousPage - startCursor - endCursor - hasNextPage + projects(search: $projectSearchTerm, first: $n, after: $cursor) { + count + nodes { + id + } + pageInfo { + hasPreviousPage + startCursor + endCursor + hasNextPage + } } - } QUERY end diff --git a/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb b/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb index f592a2a3fe3..1658c277ed0 100644 --- a/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/runner/create_spec.rb @@ -107,11 +107,7 @@ RSpec.describe 'RunnerCreate', feature_category: :runner_fleet do end end - shared_context 'when runner is created successfully' do - before do - stub_feature_flags(create_runner_workflow_for_namespace: [group]) - end - + shared_examples 'when runner is created successfully' do it do expected_args = { user: current_user, params: anything } expect_next_instance_of(::Ci::Runners::CreateRunnerService, expected_args) do |service| @@ -168,6 +164,10 @@ RSpec.describe 'RunnerCreate', feature_category: :runner_fleet do } end + before do + stub_feature_flags(create_runner_workflow_for_namespace: [group]) + end + it_behaves_like 'when user does not have permissions' context 'when user has permissions' do @@ -218,10 +218,7 @@ RSpec.describe 'RunnerCreate', feature_category: :runner_fleet do it 'returns an error' do post_graphql_mutation(mutation, current_user: current_user) - expect_graphql_errors_to_include( - 'The resource that you are attempting to access does not exist ' \ - "or you don't have permission to perform this action" - ) + expect(flattened_errors).not_to be_empty end end end diff --git a/spec/serializers/import/github_failure_entity_spec.rb b/spec/serializers/import/github_failure_entity_spec.rb new file mode 100644 index 00000000000..251e26a5e9d --- /dev/null +++ b/spec/serializers/import/github_failure_entity_spec.rb @@ -0,0 +1,319 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Import::GithubFailureEntity, feature_category: :importers do + let(:project) { instance_double(Project, id: 123456, import_url: 'https://github.com/example/repo.git', import_source: 'example/repo') } + let(:source) { 'Gitlab::GithubImport::Importer::PullRequestImporter' } + let(:github_identifiers) { { 'iid' => 2, 'object_type' => 'pull_request', 'title' => 'Implement cool feature' } } + let(:import_failure) do + instance_double( + ImportFailure, + project: project, + exception_class: 'Some class', + exception_message: 'Something went wrong', + source: source, + correlation_id_value: '2ea9c4b8587b6df49f35a3fb703688aa', + external_identifiers: github_identifiers, + created_at: Time.current + ) + end + + let(:failure_details) do + { + exception_class: import_failure.exception_class, + exception_message: import_failure.exception_message, + correlation_id_value: import_failure.correlation_id_value, + source: import_failure.source, + github_identifiers: github_identifiers, + created_at: import_failure.created_at + } + end + + subject(:entity) { described_class.new(import_failure).as_json.with_indifferent_access } + + shared_examples 'import failure entity' do + it 'exposes required fields for import entity' do + expect(entity).to eq( + { + type: import_failure.external_identifiers['object_type'], + title: title, + provider_url: provider_url, + details: failure_details + }.with_indifferent_access + ) + end + end + + it 'exposes correct attributes' do + expect(entity.keys).to match_array(%w[type title provider_url details]) + end + + context 'with `pull_request` failure' do + it_behaves_like 'import failure entity' do + let(:title) { 'Implement cool feature' } + let(:provider_url) { 'https://github.com/example/repo/pull/2' } + end + end + + context 'with `pull_request_merged_by` failure' do + before do + import_failure.external_identifiers.merge!({ 'object_type' => 'pull_request_merged_by' }) + end + + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::PullRequestMergedByImporter' } + let(:title) { 'Pull request 2 merger' } + let(:provider_url) { 'https://github.com/example/repo/pull/2' } + end + end + + context 'with `pull_request_review_request` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::PullRequests::ReviewRequestImporter' } + let(:title) { 'Pull request 2 review request' } + let(:provider_url) { 'https://github.com/example/repo/pull/2' } + let(:github_identifiers) do + { + 'merge_request_iid' => 2, + 'requested_reviewers' => %w[alice bob], + 'object_type' => 'pull_request_review_request' + } + end + end + end + + context 'with `pull_request_review` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::PullRequestReviewImporter' } + let(:title) { 'Pull request review 123456' } + let(:provider_url) { 'https://github.com/example/repo/pull/2#pullrequestreview-123456' } + let(:github_identifiers) do + { + 'merge_request_iid' => 2, + 'review_id' => 123456, + 'object_type' => 'pull_request_review' + } + end + end + end + + context 'with `issue` failure' do + before do + import_failure.external_identifiers.merge!({ 'object_type' => 'issue' }) + end + + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::IssueAndLabelLinksImporter' } + let(:title) { 'Implement cool feature' } + let(:provider_url) { 'https://github.com/example/repo/issues/2' } + end + end + + context 'with `collaborator` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::CollaboratorImporter' } + let(:title) { 'alice' } + let(:provider_url) { 'https://github.com/alice' } + let(:github_identifiers) do + { + 'id' => 123456, + 'login' => 'alice', + 'object_type' => 'collaborator' + } + end + end + end + + context 'with `protected_branch` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::ProtectedBranchImporter' } + let(:title) { 'main' } + let(:provider_url) { 'https://github.com/example/repo/tree/main' } + let(:github_identifiers) do + { + 'id' => 'main', + 'object_type' => 'protected_branch' + } + end + end + end + + context 'with `issue_event` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::IssueEventImporter' } + let(:title) { 'closed' } + let(:provider_url) { 'https://github.com/example/repo/issues/2#event-123456' } + let(:github_identifiers) do + { + 'id' => 123456, + 'issuable_iid' => 2, + 'event' => 'closed', + 'object_type' => 'issue_event' + } + end + end + end + + context 'with `label` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::LabelsImporter' } + let(:title) { 'bug' } + let(:provider_url) { 'https://github.com/example/repo/labels/bug' } + let(:github_identifiers) { { 'title' => 'bug', 'object_type' => 'label' } } + end + end + + context 'with `milestone` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::MilestonesImporter' } + let(:title) { '1 release' } + let(:provider_url) { 'https://github.com/example/repo/milestone/1' } + let(:github_identifiers) { { 'iid' => 1, 'title' => '1 release', 'object_type' => 'milestone' } } + end + end + + context 'with `release` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::ReleasesImporter' } + let(:title) { 'v1.0' } + let(:provider_url) { 'https://github.com/example/repo/releases/tag/v1.0' } + let(:github_identifiers) do + { + 'tag' => 'v1.0', + 'object_type' => 'release' + } + end + end + end + + context 'with `note` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::NoteImporter' } + let(:title) { 'MergeRequest comment 123456' } + let(:provider_url) { 'https://github.com/example/repo/issues/2#issuecomment-123456' } + let(:github_identifiers) do + { + 'note_id' => 123456, + 'noteable_iid' => 2, + 'noteable_type' => 'MergeRequest', + 'object_type' => 'note' + } + end + end + end + + context 'with `diff_note` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::DiffNoteImporter' } + let(:title) { 'Pull request review comment 123456' } + let(:provider_url) { 'https://github.com/example/repo/pull/2#discussion_r123456' } + let(:github_identifiers) do + { + 'note_id' => 123456, + 'noteable_iid' => 2, + 'noteable_type' => 'MergeRequest', + 'object_type' => 'diff_note' + } + end + end + end + + context 'with `issue_attachment` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::NoteAttachmentsImporter' } + let(:title) { 'Issue 2 attachment' } + let(:provider_url) { 'https://github.com/example/repo/issues/2' } + let(:github_identifiers) do + { + 'db_id' => 123456, + 'noteable_iid' => 2, + 'object_type' => 'issue_attachment' + } + end + end + end + + context 'with `merge_request_attachment` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::NoteAttachmentsImporter' } + let(:title) { 'Merge request 2 attachment' } + let(:provider_url) { 'https://github.com/example/repo/pull/2' } + let(:github_identifiers) do + { + 'db_id' => 123456, + 'noteable_iid' => 2, + 'object_type' => 'merge_request_attachment' + } + end + end + end + + context 'with `release_attachment` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::NoteAttachmentsImporter' } + let(:title) { 'Release v1.0 attachment' } + let(:provider_url) { 'https://github.com/example/repo/releases/tag/v1.0' } + let(:github_identifiers) do + { + 'db_id' => 123456, + 'tag' => 'v1.0', + 'object_type' => 'release_attachment' + } + end + end + end + + context 'with `note_attachment` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::NoteAttachmentsImporter' } + let(:title) { 'Note attachment' } + let(:provider_url) { '' } + let(:github_identifiers) do + { + 'db_id' => 123456, + 'noteable_type' => 'Issue', + 'object_type' => 'note_attachment' + } + end + end + end + + context 'with `lfs_object` failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::LfsObjectImporter' } + let(:title) { '42' } + let(:provider_url) { '' } + let(:github_identifiers) do + { + 'oid' => 42, + 'size' => 123456, + 'object_type' => 'lfs_object' + } + end + end + end + + context 'with unknown failure' do + it_behaves_like 'import failure entity' do + let(:source) { 'Gitlab::GithubImport::Importer::NewObjectTypeImporter' } + let(:title) { '' } + let(:provider_url) { '' } + let(:github_identifiers) do + { + 'id' => 123456, + 'object_type' => 'new_object_type' + } + end + end + end + + context 'with an invalid import_url' do + let(:project) { instance_double(Project, id: 123456, import_url: 'Invalid url', import_source: 'example/repo') } + + it_behaves_like 'import failure entity' do + let(:title) { 'Implement cool feature' } + let(:provider_url) { '' } + end + end +end diff --git a/spec/serializers/import/github_failure_serializer_spec.rb b/spec/serializers/import/github_failure_serializer_spec.rb new file mode 100644 index 00000000000..170b2739cfc --- /dev/null +++ b/spec/serializers/import/github_failure_serializer_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Import::GithubFailureSerializer, feature_category: :importers do + subject(:serializer) { described_class.new } + + it 'represents GithubFailureEntity entities' do + expect(described_class.entity_class).to eq(Import::GithubFailureEntity) + end + + describe '#represent' do + let(:timestamp) { Time.new(2023, 1, 1).utc } + let(:github_identifiers) { { 'iid' => 2, 'object_type' => 'pull_request', 'title' => 'Implement cool feature' } } + let(:project) do + instance_double( + Project, + id: 123456, + import_status: 'finished', + import_url: 'https://github.com/example/repo.git', + import_source: 'example/repo' + ) + end + + let(:import_failure) do + instance_double( + ImportFailure, + project: project, + exception_class: 'Some class', + exception_message: 'Something went wrong', + source: 'Gitlab::GithubImport::Importer::PullRequestImporter', + correlation_id_value: '2ea9c4b8587b6df49f35a3fb703688aa', + external_identifiers: github_identifiers, + created_at: timestamp + ) + end + + let(:expected_data) do + { + type: 'pull_request', + title: 'Implement cool feature', + provider_url: 'https://github.com/example/repo/pull/2', + details: { + exception_class: import_failure.exception_class, + exception_message: import_failure.exception_message, + correlation_id_value: import_failure.correlation_id_value, + source: import_failure.source, + github_identifiers: github_identifiers, + created_at: timestamp.iso8601(3) + } + }.deep_stringify_keys + end + + context 'when a single object is being serialized' do + let(:resource) { import_failure } + + it 'serializes import failure' do + expect(serializer.represent(resource).as_json).to eq expected_data + end + end + + context 'when multiple objects are being serialized' do + let(:count) { 3 } + let(:resource) { Array.new(count, import_failure) } + + it 'serializes array of import failures' do + expect(serializer.represent(resource).as_json).to all(eq(expected_data)) + end + end + end +end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index b5e4543fabe..bb1383eee1d 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -7006,7 +7006,6 @@ - './spec/lib/gitlab/sidekiq_config/worker_matcher_spec.rb' - './spec/lib/gitlab/sidekiq_config/worker_router_spec.rb' - './spec/lib/gitlab/sidekiq_config/worker_spec.rb' -- './spec/lib/gitlab/sidekiq_daemon/memory_killer_spec.rb' - './spec/lib/gitlab/sidekiq_daemon/monitor_spec.rb' - './spec/lib/gitlab/sidekiq_death_handler_spec.rb' - './spec/lib/gitlab/sidekiq_logging/deduplication_logger_spec.rb' diff --git a/spec/views/layouts/_head.html.haml_spec.rb b/spec/views/layouts/_head.html.haml_spec.rb index 0ceef3fbd0e..a44c69748e5 100644 --- a/spec/views/layouts/_head.html.haml_spec.rb +++ b/spec/views/layouts/_head.html.haml_spec.rb @@ -59,7 +59,7 @@ RSpec.describe 'layouts/_head' do render - expect(rendered).to match('<link rel="stylesheet" media="screen" href="/stylesheets/highlight/themes/solarised-light.css" />') + expect(rendered).to match('<link rel="stylesheet" media="all" href="/stylesheets/highlight/themes/solarised-light.css" />') end context 'when an asset_host is set and snowplow url is set', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/346542' do |