diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-08 18:08:30 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2022-12-08 18:08:30 +0000 |
commit | 54943f1e68bd4e1951bf3d6c438acab8d783fc31 (patch) | |
tree | 7528e59c2fd305422f64c44c124ed2cda35b6cd2 /spec | |
parent | 8a7e48133fa53ea53107eafb4e71fbe72545588e (diff) | |
download | gitlab-ce-54943f1e68bd4e1951bf3d6c438acab8d783fc31.tar.gz |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
119 files changed, 1297 insertions, 505 deletions
diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index aff010d137a..50f0baef5e0 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -71,6 +71,11 @@ FactoryBot.define do association :work_item_type, :default, :task end + trait :objective do + issue_type { :objective } + association :work_item_type, :default, :objective + end + factory :incident do issue_type { :incident } association :work_item_type, :default, :incident diff --git a/spec/finders/ci/pipelines_finder_spec.rb b/spec/finders/ci/pipelines_finder_spec.rb index 908210e0296..09d60f2e9f9 100644 --- a/spec/finders/ci/pipelines_finder_spec.rb +++ b/spec/finders/ci/pipelines_finder_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Ci::PipelinesFinder do - let(:project) { create(:project, :public, :repository) } + let_it_be(:project) { create(:project, :public, :repository) } let(:current_user) { nil } let(:params) { {} } @@ -242,6 +242,35 @@ RSpec.describe Ci::PipelinesFinder do end end + context 'when name is specified' do + let_it_be(:pipeline) { create(:ci_pipeline, project: project, name: 'Build pipeline') } + let_it_be(:pipeline_other) { create(:ci_pipeline, project: project, name: 'Some other pipeline') } + + let(:params) { { name: 'build Pipeline' } } + + it 'performs case insensitive compare' do + is_expected.to contain_exactly(pipeline) + end + + context 'when name does not exist' do + let(:params) { { name: 'absent-name' } } + + it 'returns empty' do + is_expected.to be_empty + end + end + + context 'when pipeline_name feature flag is off' do + before do + stub_feature_flags(pipeline_name: false) + end + + it 'ignores name parameter' do + is_expected.to contain_exactly(pipeline, pipeline_other) + end + end + end + describe 'ordering' do using RSpec::Parameterized::TableSyntax diff --git a/spec/frontend/boards/components/sidebar/board_sidebar_time_tracker_spec.js b/spec/frontend/boards/components/sidebar/board_sidebar_time_tracker_spec.js index 5c435643425..e2e4baefad0 100644 --- a/spec/frontend/boards/components/sidebar/board_sidebar_time_tracker_spec.js +++ b/spec/frontend/boards/components/sidebar/board_sidebar_time_tracker_spec.js @@ -42,13 +42,20 @@ describe('BoardSidebarTimeTracker', () => { wrapper = null; }); - it.each([[true], [false]])( - 'renders IssuableTimeTracker with correct spent and estimated time (timeTrackingLimitToHours=%s)', - (timeTrackingLimitToHours) => { - createComponent({ provide: { timeTrackingLimitToHours } }); + it.each` + timeTrackingLimitToHours | canUpdate + ${true} | ${false} + ${true} | ${true} + ${false} | ${false} + ${false} | ${true} + `( + 'renders IssuableTimeTracker with correct spent and estimated time (timeTrackingLimitToHours=$timeTrackingLimitToHours, canUpdate=$canUpdate)', + ({ timeTrackingLimitToHours, canUpdate }) => { + createComponent({ provide: { timeTrackingLimitToHours, canUpdate } }); expect(wrapper.findComponent(IssuableTimeTracker).props()).toEqual({ limitToHours: timeTrackingLimitToHours, + canAddTimeEntries: canUpdate, showCollapsed: false, issuableId: '1', issuableIid: '1', diff --git a/spec/frontend/ci/runner/runner_search_utils_spec.js b/spec/frontend/ci/runner/runner_search_utils_spec.js index c5c6a29cd6c..f64b89d47fd 100644 --- a/spec/frontend/ci/runner/runner_search_utils_spec.js +++ b/spec/frontend/ci/runner/runner_search_utils_spec.js @@ -65,12 +65,13 @@ describe('search_params.js', () => { it.each([ 'http://test.host/?status[]=ACTIVE', 'http://test.host/?runner_type[]=INSTANCE_TYPE', + 'http://test.host/?paused[]=true', 'http://test.host/?search=my_text', - ])('When a filter is removed, it is removed from the URL', (initalUrl) => { + ])('When a filter is removed, it is removed from the URL', (initialUrl) => { const search = { filters: [], sort: 'CREATED_DESC' }; const expectedUrl = `http://test.host/`; - expect(fromSearchToUrl(search, initalUrl)).toBe(expectedUrl); + expect(fromSearchToUrl(search, initialUrl)).toBe(expectedUrl); }); it('When unrelated search parameter is present, it does not get removed', () => { diff --git a/spec/frontend/sidebar/components/incidents/sidebar_escalation_status_spec.js b/spec/frontend/sidebar/components/incidents/sidebar_escalation_status_spec.js index 562e29edf43..2dded61c073 100644 --- a/spec/frontend/sidebar/components/incidents/sidebar_escalation_status_spec.js +++ b/spec/frontend/sidebar/components/incidents/sidebar_escalation_status_spec.js @@ -1,5 +1,4 @@ -import { createLocalVue } from '@vue/test-utils'; -import { nextTick } from 'vue'; +import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import { fetchData, @@ -25,16 +24,15 @@ import { logError } from '~/lib/logger'; jest.mock('~/lib/logger'); jest.mock('~/flash'); -const localVue = createLocalVue(); +Vue.use(VueApollo); describe('SidebarEscalationStatus', () => { let wrapper; + let mockApollo; const queryResolverMock = jest.fn(); const mutationResolverMock = jest.fn(); function createMockApolloProvider({ hasFetchError = false, hasMutationError = false } = {}) { - localVue.use(VueApollo); - queryResolverMock.mockResolvedValue({ data: hasFetchError ? fetchError : fetchData }); mutationResolverMock.mockResolvedValue({ data: hasMutationError ? mutationError : mutationData, @@ -48,15 +46,7 @@ describe('SidebarEscalationStatus', () => { return createMockApollo(requestHandlers); } - function createComponent({ mockApollo } = {}) { - let config; - - if (mockApollo) { - config = { apolloProvider: mockApollo }; - } else { - config = { mocks: { $apollo: { queries: { status: { loading: false } } } } }; - } - + function createComponent(apolloProvider) { wrapper = mountExtended(SidebarEscalationStatus, { propsData: { iid: '1', @@ -69,13 +59,15 @@ describe('SidebarEscalationStatus', () => { directives: { GlTooltip: createMockDirective(), }, - localVue, - ...config, + apolloProvider, }); + + // wait for apollo requests + return waitForPromises(); } - afterEach(() => { - wrapper.destroy(); + beforeEach(() => { + mockApollo = createMockApolloProvider(); }); const findSidebarComponent = () => wrapper.findComponent(SidebarEditableItem); @@ -83,36 +75,32 @@ describe('SidebarEscalationStatus', () => { const findEditButton = () => wrapper.findByTestId('edit-button'); const findIcon = () => wrapper.findByTestId('status-icon'); - const clickEditButton = async () => { + const clickEditButton = () => { findEditButton().vm.$emit('click'); - await nextTick(); + return nextTick(); }; - const selectAcknowledgedStatus = async () => { + const selectAcknowledgedStatus = () => { findStatusComponent().vm.$emit('input', STATUS_ACKNOWLEDGED); // wait for apollo requests - await waitForPromises(); + return waitForPromises(); }; describe('sidebar', () => { - it('renders the sidebar component', () => { - createComponent(); + it('renders the sidebar component', async () => { + await createComponent(mockApollo); expect(findSidebarComponent().exists()).toBe(true); }); describe('status icon', () => { - it('is visible', () => { - createComponent(); + it('is visible', async () => { + await createComponent(mockApollo); expect(findIcon().exists()).toBe(true); expect(findIcon().isVisible()).toBe(true); }); it('has correct tooltip', async () => { - const mockApollo = createMockApolloProvider(); - createComponent({ mockApollo }); - - // wait for apollo requests - await waitForPromises(); + await createComponent(mockApollo); const tooltip = getBinding(findIcon().element, 'gl-tooltip'); @@ -123,11 +111,7 @@ describe('SidebarEscalationStatus', () => { describe('status dropdown', () => { beforeEach(async () => { - const mockApollo = createMockApolloProvider(); - createComponent({ mockApollo }); - - // wait for apollo requests - await waitForPromises(); + await createComponent(mockApollo); }); it('is closed by default', () => { @@ -151,11 +135,7 @@ describe('SidebarEscalationStatus', () => { describe('update Status event', () => { beforeEach(async () => { - const mockApollo = createMockApolloProvider(); - createComponent({ mockApollo }); - - // wait for apollo requests - await waitForPromises(); + await createComponent(mockApollo); await clickEditButton(); await selectAcknowledgedStatus(); @@ -187,22 +167,16 @@ describe('SidebarEscalationStatus', () => { describe('mutation errors', () => { it('should error upon fetch', async () => { - const mockApollo = createMockApolloProvider({ hasFetchError: true }); - createComponent({ mockApollo }); - - // wait for apollo requests - await waitForPromises(); + mockApollo = createMockApolloProvider({ hasFetchError: true }); + await createComponent(mockApollo); expect(createAlert).toHaveBeenCalled(); expect(logError).toHaveBeenCalled(); }); it('should error upon mutation', async () => { - const mockApollo = createMockApolloProvider({ hasMutationError: true }); - createComponent({ mockApollo }); - - // wait for apollo requests - await waitForPromises(); + mockApollo = createMockApolloProvider({ hasMutationError: true }); + await createComponent(mockApollo); await clickEditButton(); await selectAcknowledgedStatus(); diff --git a/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js b/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js new file mode 100644 index 00000000000..cb3bb7a4538 --- /dev/null +++ b/spec/frontend/sidebar/components/time_tracking/create_timelog_form_spec.js @@ -0,0 +1,219 @@ +import Vue, { nextTick } from 'vue'; +import VueApollo from 'vue-apollo'; +import { GlAlert, GlModal } from '@gitlab/ui'; +import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import { convertToGraphQLId } from '~/graphql_shared/utils'; +import CreateTimelogForm from '~/sidebar/components/time_tracking/create_timelog_form.vue'; +import createTimelogMutation from '~/sidebar/queries/create_timelog.mutation.graphql'; +import { TYPE_ISSUE, TYPE_MERGE_REQUEST } from '~/graphql_shared/constants'; + +const mockMutationErrorMessage = 'Example error message'; + +const resolvedMutationWithoutErrorsMock = jest.fn().mockResolvedValue({ + data: { + timelogCreate: { + errors: [], + timelog: { + id: 'gid://gitlab/Timelog/1', + issue: {}, + mergeRequest: {}, + }, + }, + }, +}); + +const resolvedMutationWithErrorsMock = jest.fn().mockResolvedValue({ + data: { + timelogCreate: { + errors: [{ message: mockMutationErrorMessage }], + timelog: null, + }, + }, +}); + +const rejectedMutationMock = jest.fn().mockRejectedValue(); +const modalCloseMock = jest.fn(); + +describe('Create Timelog Form', () => { + Vue.use(VueApollo); + + let wrapper; + let fakeApollo; + + const findForm = () => wrapper.find('form'); + const findModal = () => wrapper.findComponent(GlModal); + const findAlert = () => wrapper.findComponent(GlAlert); + const findDocsLink = () => wrapper.findByTestId('timetracking-docs-link'); + const findSaveButton = () => findModal().props('actionPrimary'); + const findSaveButtonLoadingState = () => findSaveButton().attributes[0].loading; + const findSaveButtonDisabledState = () => findSaveButton().attributes[0].disabled; + + const submitForm = () => findForm().trigger('submit'); + + const mountComponent = ( + { props, data, providedProps } = {}, + mutationResolverMock = rejectedMutationMock, + ) => { + fakeApollo = createMockApollo([[createTimelogMutation, mutationResolverMock]]); + + wrapper = shallowMountExtended(CreateTimelogForm, { + data() { + return { + ...data, + }; + }, + provide: { + issuableType: 'issue', + ...providedProps, + }, + propsData: { + issuableId: '1', + ...props, + }, + apolloProvider: fakeApollo, + }); + + wrapper.vm.$refs.modal.close = modalCloseMock; + }; + + afterEach(() => { + fakeApollo = null; + }); + + describe('save button', () => { + it('is disabled and not loading by default', () => { + mountComponent(); + + expect(findSaveButtonLoadingState()).toBe(false); + expect(findSaveButtonDisabledState()).toBe(true); + }); + + it('is enabled and not loading when time spent is not empty', () => { + mountComponent({ data: { timeSpent: '2d' } }); + + expect(findSaveButtonLoadingState()).toBe(false); + expect(findSaveButtonDisabledState()).toBe(false); + }); + + it('is disabled and loading when the the form is submitted', async () => { + mountComponent({ data: { timeSpent: '2d' } }); + + submitForm(); + + await nextTick(); + + expect(findSaveButtonLoadingState()).toBe(true); + expect(findSaveButtonDisabledState()).toBe(true); + }); + + it('is enabled and not loading the when form is submitted but the mutation has errors', async () => { + mountComponent({ data: { timeSpent: '2d' } }); + + submitForm(); + + await waitForPromises(); + + expect(rejectedMutationMock).toHaveBeenCalled(); + expect(findSaveButtonLoadingState()).toBe(false); + expect(findSaveButtonDisabledState()).toBe(false); + }); + + it('is enabled and not loading the when form is submitted but the mutation returns errors', async () => { + mountComponent({ data: { timeSpent: '2d' } }, resolvedMutationWithErrorsMock); + + submitForm(); + + await waitForPromises(); + + expect(resolvedMutationWithErrorsMock).toHaveBeenCalled(); + expect(findSaveButtonLoadingState()).toBe(false); + expect(findSaveButtonDisabledState()).toBe(false); + }); + }); + + describe('form', () => { + it('does not call any mutation when the the form is incomplete', async () => { + mountComponent(); + + submitForm(); + + await waitForPromises(); + + expect(rejectedMutationMock).not.toHaveBeenCalled(); + }); + + it('closes the modal after a successful mutation', async () => { + mountComponent({ data: { timeSpent: '2d' } }, resolvedMutationWithoutErrorsMock); + + submitForm(); + + await waitForPromises(); + await nextTick(); + + expect(modalCloseMock).toHaveBeenCalled(); + }); + + it.each` + issuableType | typeConstant + ${'issue'} | ${TYPE_ISSUE} + ${'merge_request'} | ${TYPE_MERGE_REQUEST} + `( + 'calls the mutation with all the fields when the the form is submitted and issuable type is $issuableType', + async ({ issuableType, typeConstant }) => { + const timeSpent = '2d'; + const spentAt = '2022-11-20T21:53:00+0000'; + const summary = 'Example'; + + mountComponent({ data: { timeSpent, spentAt, summary }, providedProps: { issuableType } }); + + submitForm(); + + await waitForPromises(); + + expect(rejectedMutationMock).toHaveBeenCalledWith({ + input: { timeSpent, spentAt, summary, issuableId: convertToGraphQLId(typeConstant, '1') }, + }); + }, + ); + }); + + describe('alert', () => { + it('is hidden by default', () => { + mountComponent(); + + expect(findAlert().exists()).toBe(false); + }); + + it('shows an error if the submission fails with a handled error', async () => { + mountComponent({ data: { timeSpent: '2d' } }, resolvedMutationWithErrorsMock); + + submitForm(); + + await waitForPromises(); + + expect(findAlert().exists()).toBe(true); + expect(findAlert().text()).toBe(mockMutationErrorMessage); + }); + + it('shows an error if the submission fails with an unhandled error', async () => { + mountComponent({ data: { timeSpent: '2d' } }); + + submitForm(); + + await waitForPromises(); + + expect(findAlert().exists()).toBe(true); + expect(findAlert().text()).toBe('An error occurred while saving the time entry.'); + }); + }); + + describe('docs link message', () => { + it('is present', () => { + mountComponent(); + + expect(findDocsLink().exists()).toBe(true); + }); + }); +}); diff --git a/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js b/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js index 835e700e63c..45d8b5e4647 100644 --- a/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js +++ b/spec/frontend/sidebar/components/time_tracking/time_tracker_spec.js @@ -268,47 +268,32 @@ describe('Issuable Time Tracker', () => { }); }); - describe('Help pane', () => { - const findHelpButton = () => findByTestId('helpButton'); - const findCloseHelpButton = () => findByTestId('closeHelpButton'); - - beforeEach(async () => { - wrapper = mountComponent({ - props: { - initialTimeTracking: { - timeEstimate: 0, - totalTimeSpent: 0, - humanTimeEstimate: '', - humanTotalTimeSpent: '', + describe('Add button', () => { + const findAddButton = () => findByTestId('add-time-entry-button'); + + it.each` + visibility | canAddTimeEntries + ${'not visible'} | ${false} + ${'visible'} | ${true} + `( + 'is $visibility when canAddTimeEntries is $canAddTimeEntries', + async ({ canAddTimeEntries }) => { + wrapper = mountComponent({ + props: { + initialTimeTracking: { + timeEstimate: 0, + totalTimeSpent: 0, + humanTimeEstimate: '', + humanTotalTimeSpent: '', + }, + canAddTimeEntries, }, - }, - }); - await nextTick(); - }); - - it('should not show the "Help" pane by default', () => { - expect(findByTestId('helpPane').exists()).toBe(false); - }); - - it('should show the "Help" pane when help button is clicked', async () => { - findHelpButton().trigger('click'); - - await nextTick(); - - expect(findByTestId('helpPane').exists()).toBe(true); - }); - - it('should not show the "Help" pane when help button is clicked and then closed', async () => { - findHelpButton().trigger('click'); - await nextTick(); - - expect(findByTestId('helpPane').exists()).toBe(true); - - findCloseHelpButton().trigger('click'); - await nextTick(); + }); + await nextTick(); - expect(findByTestId('helpPane').exists()).toBe(false); - }); + expect(findAddButton().exists()).toBe(canAddTimeEntries); + }, + ); }); }); diff --git a/spec/helpers/todos_helper_spec.rb b/spec/helpers/todos_helper_spec.rb index 48aff14180e..5e8e62451a9 100644 --- a/spec/helpers/todos_helper_spec.rb +++ b/spec/helpers/todos_helper_spec.rb @@ -155,6 +155,26 @@ RSpec.describe TodosHelper do expect(path).to eq("/#{issue.project.full_path}/-/issues/#{issue.iid}##{dom_id(note)}") end end + + context 'when a user requests access to group' do + let(:group) { create(:group, :public) } + + let(:group_access_request_todo) do + create(:todo, + target_id: group.id, + target_type: group.class.polymorphic_name, + group: group, + action: Todo::MEMBER_ACCESS_REQUESTED) + end + + it 'responds with access requests tab' do + path = helper.todo_target_path(group_access_request_todo) + + access_request_path = Gitlab::Routing.url_helpers.group_group_members_url(group, tab: 'access_requests') + + expect(path).to eq(access_request_path) + end + end end describe '#todo_target_type_name' do @@ -340,6 +360,7 @@ RSpec.describe TodosHelper do Todo::APPROVAL_REQUIRED | false | format(s_("Todos|set %{who} as an approver for"), who: _('you')) Todo::UNMERGEABLE | true | s_('Todos|Could not merge') Todo::MERGE_TRAIN_REMOVED | true | s_("Todos|Removed from Merge Train:") + Todo::MEMBER_ACCESS_REQUESTED | true | s_("Todos|has requested access to") end with_them do diff --git a/spec/lib/gitlab/auth/ldap/config_spec.rb b/spec/lib/gitlab/auth/ldap/config_spec.rb index 4e7fbab396f..160fd78b2b9 100644 --- a/spec/lib/gitlab/auth/ldap/config_spec.rb +++ b/spec/lib/gitlab/auth/ldap/config_spec.rb @@ -122,7 +122,8 @@ AtlErSqafbECNDSwS5BX8yDpu5yRBJ4xegO/rNlmb8ICRYkuJapD1xXicFOsmfUK host: 'ldap.example.com', port: 386, hosts: nil, - encryption: nil + encryption: nil, + instrumentation_service: ActiveSupport::Notifications ) end diff --git a/spec/lib/gitlab/ci/build/context/build_spec.rb b/spec/lib/gitlab/ci/build/context/build_spec.rb index 7f862a3b80a..74739a67be0 100644 --- a/spec/lib/gitlab/ci/build/context/build_spec.rb +++ b/spec/lib/gitlab/ci/build/context/build_spec.rb @@ -2,11 +2,11 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Build::Context::Build do +RSpec.describe Gitlab::Ci::Build::Context::Build, feature_category: :pipeline_authoring do let(:pipeline) { create(:ci_pipeline) } let(:seed_attributes) { { 'name' => 'some-job' } } - let(:context) { described_class.new(pipeline, seed_attributes) } + subject(:context) { described_class.new(pipeline, seed_attributes) } shared_examples 'variables collection' do it { is_expected.to include('CI_COMMIT_REF_NAME' => 'master') } @@ -22,6 +22,12 @@ RSpec.describe Gitlab::Ci::Build::Context::Build do it { is_expected.to include('CI_BUILD_REF_NAME' => 'master') } it { is_expected.to include('CI_PROJECT_PATH' => pipeline.project.full_path) } end + + context 'when environment:name is provided' do + let(:seed_attributes) { { 'name' => 'some-job', 'environment' => 'test' } } + + it { is_expected.to include('CI_ENVIRONMENT_NAME' => 'test') } + end end describe '#variables' do diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index e552b9e9c0c..5433f03fb09 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do +RSpec.describe Gitlab::Ci::Pipeline::Seed::Build, feature_category: :pipeline_authoring do let_it_be_with_reload(:project) { create(:project, :repository) } let_it_be(:head_sha) { project.repository.head_commit.id } @@ -949,6 +949,40 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do end end + context 'with a rule using CI_ENVIRONMENT_NAME variable' do + let(:rule_set) do + [{ if: '$CI_ENVIRONMENT_NAME == "test"' }] + end + + context 'when environment:name satisfies the rule' do + let(:attributes) { { name: 'rspec', rules: rule_set, environment: 'test', when: 'on_success' } } + + it { is_expected.to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'on_success') + end + end + + context 'when environment:name does not satisfy rule' do + let(:attributes) { { name: 'rspec', rules: rule_set, environment: 'dev', when: 'on_success' } } + + it { is_expected.not_to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'never') + end + end + + context 'when environment:name is not set' do + it { is_expected.not_to be_included } + + it 'correctly populates when:' do + expect(seed_build.attributes).to include(when: 'never') + end + end + end + context 'with no rules' do let(:rule_set) { [] } diff --git a/spec/lib/gitlab/ci/variables/builder_spec.rb b/spec/lib/gitlab/ci/variables/builder_spec.rb index 052d3024254..6b0faf5c5ca 100644 --- a/spec/lib/gitlab/ci/variables/builder_spec.rb +++ b/spec/lib/gitlab/ci/variables/builder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Variables::Builder, :clean_gitlab_redis_cache do +RSpec.describe Gitlab::Ci::Variables::Builder, :clean_gitlab_redis_cache, feature_category: :pipeline_authoring do include Ci::TemplateHelpers let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, namespace: group) } @@ -13,7 +13,8 @@ RSpec.describe Gitlab::Ci::Variables::Builder, :clean_gitlab_redis_cache do name: 'rspec:test 1', pipeline: pipeline, user: user, - yaml_variables: [{ key: 'YAML_VARIABLE', value: 'value' }] + yaml_variables: [{ key: 'YAML_VARIABLE', value: 'value' }], + environment: 'test' ) end @@ -32,6 +33,8 @@ RSpec.describe Gitlab::Ci::Variables::Builder, :clean_gitlab_redis_cache do value: job.stage_name }, { key: 'CI_NODE_TOTAL', value: '1' }, + { key: 'CI_ENVIRONMENT_NAME', + value: 'test' }, { key: 'CI_BUILD_NAME', value: 'rspec:test 1' }, { key: 'CI_BUILD_STAGE', diff --git a/spec/lib/gitlab/github_import/bulk_importing_spec.rb b/spec/lib/gitlab/github_import/bulk_importing_spec.rb index e170496ff7b..af31cb6c873 100644 --- a/spec/lib/gitlab/github_import/bulk_importing_spec.rb +++ b/spec/lib/gitlab/github_import/bulk_importing_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::BulkImporting do +RSpec.describe Gitlab::GithubImport::BulkImporting, feature_category: :importer do let(:project) { instance_double(Project, id: 1) } let(:importer) { MyImporter.new(project, double) } let(:importer_class) do @@ -12,22 +12,33 @@ RSpec.describe Gitlab::GithubImport::BulkImporting do def object_type :object_type end + + def model + Label + end end end + let(:label) { instance_double('Label', invalid?: false) } + before do stub_const 'MyImporter', importer_class end describe '#build_database_rows' do - it 'returns an Array containing the rows to insert' do + it 'returns an Array containing the rows to insert and validation errors if object invalid' do object = double(:object, title: 'Foo') expect(importer) - .to receive(:build) + .to receive(:build_attributes) .with(object) .and_return({ title: 'Foo' }) + expect(Label) + .to receive(:new) + .with({ title: 'Foo' }) + .and_return(label) + expect(importer) .to receive(:already_imported?) .with(object) @@ -53,14 +64,17 @@ RSpec.describe Gitlab::GithubImport::BulkImporting do enum = [[object, 1]].to_enum - expect(importer.build_database_rows(enum)).to eq([{ title: 'Foo' }]) + rows, errors = importer.build_database_rows(enum) + + expect(rows).to match_array([{ title: 'Foo' }]) + expect(errors).to be_empty end it 'does not import objects that have already been imported' do object = double(:object, title: 'Foo') expect(importer) - .not_to receive(:build) + .not_to receive(:build_attributes) expect(importer) .to receive(:already_imported?) @@ -87,14 +101,16 @@ RSpec.describe Gitlab::GithubImport::BulkImporting do enum = [[object, 1]].to_enum - expect(importer.build_database_rows(enum)).to be_empty + rows, errors = importer.build_database_rows(enum) + + expect(rows).to be_empty + expect(errors).to be_empty end end describe '#bulk_insert' do it 'bulk inserts rows into the database' do rows = [{ title: 'Foo' }] * 10 - model = double(:model, table_name: 'kittens') expect(Gitlab::Import::Logger) .to receive(:info) @@ -119,14 +135,43 @@ RSpec.describe Gitlab::GithubImport::BulkImporting do expect(ApplicationRecord) .to receive(:legacy_bulk_insert) .ordered - .with('kittens', rows.first(5)) + .with('labels', rows.first(5)) expect(ApplicationRecord) .to receive(:legacy_bulk_insert) .ordered - .with('kittens', rows.last(5)) + .with('labels', rows.last(5)) + + importer.bulk_insert(rows, batch_size: 5) + end + end + + describe '#bulk_insert_failures', :timecop do + let(:import_failures) { instance_double('ImportFailure::ActiveRecord_Associations_CollectionProxy') } + let(:label) { Label.new(title: 'invalid,title') } + let(:validation_errors) { ActiveModel::Errors.new(label) } + let(:formatted_errors) do + [{ + source: 'MyImporter', + exception_class: 'ActiveRecord::RecordInvalid', + exception_message: 'Title invalid', + correlation_id_value: 'cid', + retry_count: nil, + created_at: Time.zone.now + }] + end - importer.bulk_insert(model, rows, batch_size: 5) + it 'bulk inserts validation errors into import_failures' do + error = ActiveModel::Errors.new(label) + error.add(:base, 'Title invalid') + + freeze_time do + expect(project).to receive(:import_failures).and_return(import_failures) + expect(import_failures).to receive(:insert_all).with(formatted_errors) + expect(Labkit::Correlation::CorrelationId).to receive(:current_or_new_id).and_return('cid') + + importer.bulk_insert_failures([error]) + end end end end diff --git a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb index 8eeb2332131..1bcb7fec327 100644 --- a/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/diff_note_importer_spec.rb @@ -218,6 +218,16 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_fail end end end + + context 'when diff note is invalid' do + it 'fails validation' do + stub_user_finder(user.id, true) + + expect(note_representation).to receive(:line_code).and_return(nil) + + expect { subject.execute }.to raise_error(ActiveRecord::RecordInvalid) + end + end end end diff --git a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb index e68849755b2..e005d8eda84 100644 --- a/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/label_links_importer_spec.rb @@ -36,26 +36,11 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelLinksImporter do expect(importer) .to receive(:find_target_id) - .and_return(1) + .and_return(4) - freeze_time do - expect(ApplicationRecord) - .to receive(:legacy_bulk_insert) - .with( - LabelLink.table_name, - [ - { - label_id: 2, - target_id: 1, - target_type: Issue, - created_at: Time.zone.now, - updated_at: Time.zone.now - } - ] - ) + expect(LabelLink).to receive(:bulk_insert!) - importer.create_labels - end + importer.create_labels end it 'does not insert label links for non-existing labels' do @@ -64,9 +49,9 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelLinksImporter do .with('bug') .and_return(nil) - expect(ApplicationRecord) - .to receive(:legacy_bulk_insert) - .with(LabelLink.table_name, []) + expect(LabelLink) + .to receive(:bulk_insert!) + .with([]) importer.create_labels end diff --git a/spec/lib/gitlab/github_import/importer/labels_importer_spec.rb b/spec/lib/gitlab/github_import/importer/labels_importer_spec.rb index 81d534c566f..ad9ef4afddd 100644 --- a/spec/lib/gitlab/github_import/importer/labels_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/labels_importer_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::Importer::LabelsImporter, :clean_gitlab_redis_cache do +RSpec.describe Gitlab::GithubImport::Importer::LabelsImporter, :clean_gitlab_redis_cache, feature_category: :importer do let(:project) { create(:project, import_source: 'foo/bar') } let(:client) { double(:client) } let(:importer) { described_class.new(project, client) } @@ -11,40 +11,58 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelsImporter, :clean_gitlab_red it 'imports the labels in bulk' do label_hash = { title: 'bug', color: '#fffaaa' } - expect(importer) - .to receive(:build_labels) - .and_return([label_hash]) - - expect(importer) - .to receive(:bulk_insert) - .with(Label, [label_hash]) - - expect(importer) - .to receive(:build_labels_cache) + expect(importer).to receive(:build_labels).and_return([[label_hash], []]) + expect(importer).to receive(:bulk_insert).with([label_hash]) + expect(importer).to receive(:build_labels_cache) importer.execute end end describe '#build_labels' do - it 'returns an Array containnig label rows' do + it 'returns an Array containing label rows' do label = { name: 'bug', color: 'ffffff' } expect(importer).to receive(:each_label).and_return([label]) - rows = importer.build_labels + rows, errors = importer.build_labels expect(rows.length).to eq(1) expect(rows[0][:title]).to eq('bug') + expect(errors).to be_blank end - it 'does not create labels that already exist' do + it 'does not build labels that already exist' do create(:label, project: project, title: 'bug') label = { name: 'bug', color: 'ffffff' } expect(importer).to receive(:each_label).and_return([label]) - expect(importer.build_labels).to be_empty + + rows, errors = importer.build_labels + + expect(rows).to be_empty + expect(errors).to be_empty + end + + it 'does not build labels that are invalid' do + label = { id: 1, name: 'bug,bug', color: 'ffffff' } + + expect(importer).to receive(:each_label).and_return([label]) + expect(Gitlab::Import::Logger).to receive(:error) + .with( + import_type: :github, + project_id: project.id, + importer: described_class.name, + message: ['Title is invalid'], + github_identifier: 1 + ) + + rows, errors = importer.build_labels + + expect(rows).to be_empty + expect(errors.length).to eq(1) + expect(errors[0].full_messages).to match_array(['Title is invalid']) end end @@ -58,9 +76,9 @@ RSpec.describe Gitlab::GithubImport::Importer::LabelsImporter, :clean_gitlab_red end end - describe '#build' do + describe '#build_attributes' do let(:label_hash) do - importer.build({ name: 'bug', color: 'ffffff' }) + importer.build_attributes({ name: 'bug', color: 'ffffff' }) end it 'returns the attributes of the label as a Hash' do diff --git a/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb b/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb index 04d76bd1f06..8667729d79b 100644 --- a/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/milestones_importer_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::Importer::MilestonesImporter, :clean_gitlab_redis_cache do +RSpec.describe Gitlab::GithubImport::Importer::MilestonesImporter, :clean_gitlab_redis_cache, + feature_category: :importer do let(:project) { create(:project, import_source: 'foo/bar') } let(:client) { double(:client) } let(:importer) { described_class.new(project, client) } @@ -38,41 +39,61 @@ RSpec.describe Gitlab::GithubImport::Importer::MilestonesImporter, :clean_gitlab it 'imports the milestones in bulk' do milestone_hash = { number: 1, title: '1.0' } - expect(importer) - .to receive(:build_milestones) - .and_return([milestone_hash]) - - expect(importer) - .to receive(:bulk_insert) - .with(Milestone, [milestone_hash]) - - expect(importer) - .to receive(:build_milestones_cache) + expect(importer).to receive(:build_milestones).and_return([[milestone_hash], []]) + expect(importer).to receive(:bulk_insert).with([milestone_hash]) + expect(importer).to receive(:build_milestones_cache) importer.execute end end describe '#build_milestones' do - it 'returns an Array containnig milestone rows' do + it 'returns an Array containing milestone rows' do expect(importer) .to receive(:each_milestone) .and_return([milestone]) - rows = importer.build_milestones + rows, errors = importer.build_milestones expect(rows.length).to eq(1) expect(rows[0][:title]).to eq('1.0') + expect(errors).to be_empty end - it 'does not create milestones that already exist' do + it 'does not build milestones that already exist' do create(:milestone, project: project, title: '1.0', iid: 1) expect(importer) .to receive(:each_milestone) .and_return([milestone]) - expect(importer.build_milestones).to be_empty + rows, errors = importer.build_milestones + + expect(rows).to be_empty + expect(errors).to be_empty + end + + it 'does not build milestones that are invalid' do + milestone = { id: 1, title: nil } + + expect(importer) + .to receive(:each_milestone) + .and_return([milestone]) + + expect(Gitlab::Import::Logger).to receive(:error) + .with( + import_type: :github, + project_id: project.id, + importer: described_class.name, + message: ["Title can't be blank"], + github_identifier: 1 + ) + + rows, errors = importer.build_milestones + + expect(rows).to be_empty + expect(errors.length).to eq(1) + expect(errors[0].full_messages).to match_array(["Title can't be blank"]) end end @@ -86,9 +107,9 @@ RSpec.describe Gitlab::GithubImport::Importer::MilestonesImporter, :clean_gitlab end end - describe '#build' do - let(:milestone_hash) { importer.build(milestone) } - let(:milestone_hash2) { importer.build(milestone2) } + describe '#build_attributes' do + let(:milestone_hash) { importer.build_attributes(milestone) } + let(:milestone_hash2) { importer.build_attributes(milestone2) } it 'returns the attributes of the milestone as a Hash' do expect(milestone_hash).to be_an_instance_of(Hash) diff --git a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb index c60ecd85e92..5ac50578b6a 100644 --- a/spec/lib/gitlab/github_import/importer/note_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/note_importer_spec.rb @@ -113,6 +113,19 @@ RSpec.describe Gitlab::GithubImport::Importer::NoteImporter do .to eq('There were an invalid char "" <= right here') end end + + context 'when note is invalid' do + it 'fails validation' do + expect(importer.user_finder) + .to receive(:author_id_for) + .with(github_note) + .and_return([user.id, true]) + + expect(github_note).to receive(:discussion_id).and_return('invalid') + + expect { importer.execute }.to raise_error(ActiveRecord::RecordInvalid) + end + end end context 'when the noteable does not exist' do diff --git a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb index 08c4af3035d..dd73b6879e0 100644 --- a/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_request_importer_spec.rb @@ -202,6 +202,20 @@ RSpec.describe Gitlab::GithubImport::Importer::PullRequestImporter, :clean_gitla importer.create_merge_request end end + + context 'when merge request is invalid' do + before do + allow(pull_request).to receive(:formatted_source_branch).and_return(nil) + allow(importer.user_finder) + .to receive(:author_id_for) + .with(pull_request) + .and_return([project.creator_id, false]) + end + + it 'fails validation' do + expect { importer.create_merge_request }.to raise_error(ActiveRecord::RecordInvalid) + end + end end describe '#set_merge_request_assignees' do diff --git a/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb b/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb index 84d639a09ef..ccbe5b5fc50 100644 --- a/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/releases_importer_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do +RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter, feature_category: :importer do let(:project) { create(:project) } let(:client) { double(:client) } let(:importer) { described_class.new(project, client) } @@ -48,8 +48,8 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do released_at: released_at } - expect(importer).to receive(:build_releases).and_return([release_hash]) - expect(importer).to receive(:bulk_insert).with(Release, [release_hash]) + expect(importer).to receive(:build_releases).and_return([[release_hash], []]) + expect(importer).to receive(:bulk_insert).with([release_hash]) importer.execute end @@ -86,24 +86,29 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do it 'returns an Array containing release rows' do expect(importer).to receive(:each_release).and_return([github_release]) - rows = importer.build_releases + rows, errors = importer.build_releases expect(rows.length).to eq(1) expect(rows[0][:tag]).to eq('1.0') + expect(errors).to be_empty end it 'does not create releases that already exist' do create(:release, project: project, tag: '1.0', description: '1.0') expect(importer).to receive(:each_release).and_return([github_release]) - expect(importer.build_releases).to be_empty + + rows, errors = importer.build_releases + + expect(rows).to be_empty + expect(errors).to be_empty end it 'uses a default release description if none is provided' do github_release[:body] = nil expect(importer).to receive(:each_release).and_return([github_release]) - release = importer.build_releases.first + release, _ = importer.build_releases.first expect(release[:description]).to eq('Release for tag 1.0') end @@ -115,20 +120,36 @@ RSpec.describe Gitlab::GithubImport::Importer::ReleasesImporter do } expect(importer).to receive(:each_release).and_return([null_tag_release]) - expect(importer.build_releases).to be_empty + + rows, errors = importer.build_releases + + expect(rows).to be_empty + expect(errors).to be_empty end it 'does not create duplicate release tags' do expect(importer).to receive(:each_release).and_return([github_release, github_release]) - releases = importer.build_releases + releases, _ = importer.build_releases expect(releases.length).to eq(1) expect(releases[0][:description]).to eq('This is my release') end + + it 'does not create invalid release' do + github_release[:body] = SecureRandom.alphanumeric(Gitlab::Database::MAX_TEXT_SIZE_LIMIT + 1) + + expect(importer).to receive(:each_release).and_return([github_release]) + + releases, errors = importer.build_releases + + expect(releases).to be_empty + expect(errors.length).to eq(1) + expect(errors[0].full_messages).to match_array(['Description is too long (maximum is 1000000 characters)']) + end end - describe '#build' do - let(:release_hash) { importer.build(github_release) } + describe '#build_attributes' do + let(:release_hash) { importer.build_attributes(github_release) } context 'the returned Hash' do before do diff --git a/spec/lib/gitlab/instrumentation/redis_base_spec.rb b/spec/lib/gitlab/instrumentation/redis_base_spec.rb index a84a26a8d92..9cf65a11bce 100644 --- a/spec/lib/gitlab/instrumentation/redis_base_spec.rb +++ b/spec/lib/gitlab/instrumentation/redis_base_spec.rb @@ -153,29 +153,39 @@ RSpec.describe Gitlab::Instrumentation::RedisBase, :request_store do end describe '.redis_cluster_validate!' do - context 'Rails environments' do - where(:env, :should_raise) do - 'production' | false - 'staging' | false - 'development' | true - 'test' | true - end + let(:args) { [[:mget, 'foo', 'bar']] } + + before do + instrumentation_class_a.enable_redis_cluster_validation + end - before do - instrumentation_class_a.enable_redis_cluster_validation + context 'Rails environments' do + where(:env, :allowed, :should_raise) do + 'production' | false | false + 'production' | true | false + 'staging' | false | false + 'staging' | true | false + 'development' | true | false + 'development' | false | true + 'test' | true | false + 'test' | false | true end with_them do it do stub_rails_env(env) - args = [[:mget, 'foo', 'bar']] + validation = -> { instrumentation_class_a.redis_cluster_validate!(args) } + under_test = if allowed + -> { Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands(&validation) } + else + validation + end if should_raise - expect { instrumentation_class_a.redis_cluster_validate!(args) } - .to raise_error(::Gitlab::Instrumentation::RedisClusterValidator::CrossSlotError) + expect(&under_test).to raise_error(::Gitlab::Instrumentation::RedisClusterValidator::CrossSlotError) else - expect { instrumentation_class_a.redis_cluster_validate!(args) }.not_to raise_error + expect(&under_test).not_to raise_error end end end diff --git a/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb b/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb index c5462bd8545..892b8e69124 100644 --- a/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb +++ b/spec/lib/gitlab/instrumentation/redis_cluster_validator_spec.rb @@ -7,83 +7,102 @@ require 'rspec-parameterized' RSpec.describe Gitlab::Instrumentation::RedisClusterValidator do include RailsHelpers - describe '.validate!' do + describe '.validate' do using RSpec::Parameterized::TableSyntax - where(:command, :arguments, :should_raise) do - :rename | %w(foo bar) | true - :RENAME | %w(foo bar) | true - 'rename' | %w(foo bar) | true - 'RENAME' | %w(foo bar) | true - :rename | %w(iaa ahy) | false # 'iaa' and 'ahy' hash to the same slot - :rename | %w({foo}:1 {foo}:2) | false - :rename | %w(foo foo bar) | false # This is not a valid command but should not raise here - :mget | %w(foo bar) | true - :mget | %w(foo foo bar) | true - :mget | %w(foo foo) | false - :blpop | %w(foo bar 1) | true - :blpop | %w(foo foo 1) | false - :mset | %w(foo a bar a) | true - :mset | %w(foo a foo a) | false - :del | %w(foo bar) | true - :del | [%w(foo bar)] | true # Arguments can be a nested array - :del | %w(foo foo) | false - :hset | %w(foo bar) | false # Not a multi-key command - :mget | [] | false # This is invalid, but not because it's a cross-slot command + where(:command, :arguments, :keys, :is_valid) do + :rename | %w(foo bar) | 2 | false + :RENAME | %w(foo bar) | 2 | false + 'rename' | %w(foo bar) | 2 | false + 'RENAME' | %w(foo bar) | 2 | false + :rename | %w(iaa ahy) | 2 | true # 'iaa' and 'ahy' hash to the same slot + :rename | %w({foo}:1 {foo}:2) | 2 | true + :rename | %w(foo foo bar) | 2 | true # This is not a valid command but should not raise here + :mget | %w(foo bar) | 2 | false + :mget | %w(foo foo bar) | 3 | false + :mget | %w(foo foo) | 2 | true + :blpop | %w(foo bar 1) | 2 | false + :blpop | %w(foo foo 1) | 2 | true + :mset | %w(foo a bar a) | 2 | false + :mset | %w(foo a foo a) | 2 | true + :del | %w(foo bar) | 2 | false + :del | [%w(foo bar)] | 2 | false # Arguments can be a nested array + :del | %w(foo foo) | 2 | true + :hset | %w(foo bar) | 1 | nil # Single key write + :get | %w(foo) | 1 | nil # Single key read + :mget | [] | 0 | true # This is invalid, but not because it's a cross-slot command end with_them do it do args = [[command] + arguments] - - if should_raise - expect { described_class.validate!(args) } - .to raise_error(described_class::CrossSlotError) + if is_valid.nil? + expect(described_class.validate(args)).to eq(nil) else - expect { described_class.validate!(args) }.not_to raise_error + expect(described_class.validate(args)[:valid]).to eq(is_valid) + expect(described_class.validate(args)[:allowed]).to eq(false) + expect(described_class.validate(args)[:command_name]).to eq(command.to_s.upcase) + expect(described_class.validate(args)[:key_count]).to eq(keys) end end end - where(:arguments, :should_raise) do - [[:get, "foo"], [:get, "bar"]] | true - [[:get, "foo"], [:mget, "foo", "bar"]] | true # mix of single-key and multi-key cmds - [[:get, "{foo}:name"], [:get, "{foo}:profile"]] | false - [[:del, "foo"], [:del, "bar"]] | true - [] | false # pipeline or transaction opened and closed without ops + where(:arguments, :should_raise, :output) do + [ + [ + [[:get, "foo"], [:get, "bar"]], + true, + { valid: false, key_count: 2, command_name: 'PIPELINE/MULTI', allowed: false } + ], + [ + [[:get, "foo"], [:mget, "foo", "bar"]], + true, + { valid: false, key_count: 3, command_name: 'PIPELINE/MULTI', allowed: false } + ], + [ + [[:get, "{foo}:name"], [:get, "{foo}:profile"]], + false, + { valid: true, key_count: 2, command_name: 'PIPELINE/MULTI', allowed: false } + ], + [ + [[:del, "foo"], [:del, "bar"]], + true, + { valid: false, key_count: 2, command_name: 'PIPELINE/MULTI', allowed: false } + ], + [ + [], + false, + nil # pipeline or transaction opened and closed without ops + ] + ] end with_them do it do - if should_raise - expect { described_class.validate!(arguments) } - .to raise_error(described_class::CrossSlotError) - else - expect { described_class.validate!(arguments) }.not_to raise_error - end + expect(described_class.validate(arguments)).to eq(output) end end end describe '.allow_cross_slot_commands' do - it 'does not raise for invalid arguments' do - expect do + it 'skips validation for allowed commands' do + expect( described_class.allow_cross_slot_commands do - described_class.validate!([[:mget, 'foo', 'bar']]) + described_class.validate([[:mget, 'foo', 'bar']]) end - end.not_to raise_error + ).to eq({ valid: true, key_count: 2, command_name: 'MGET', allowed: true }) end it 'allows nested invocation' do - expect do + expect( described_class.allow_cross_slot_commands do described_class.allow_cross_slot_commands do - described_class.validate!([[:mget, 'foo', 'bar']]) + described_class.validate([[:mget, 'foo', 'bar']]) end - described_class.validate!([[:mget, 'foo', 'bar']]) + described_class.validate([[:mget, 'foo', 'bar']]) end - end.not_to raise_error + ).to eq({ valid: true, key_count: 2, command_name: 'MGET', allowed: true }) end end end diff --git a/spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb b/spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb index 0bcc85190c2..120430accf1 100644 --- a/spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb +++ b/spec/lib/gitlab/instrumentation/redis_interceptor_spec.rb @@ -64,12 +64,6 @@ RSpec.describe Gitlab::Instrumentation::RedisInterceptor, :clean_gitlab_redis_sh end end - it 'skips count for non-cross-slot requests' do - expect(instrumentation_class).not_to receive(:increment_cross_slot_request_count).and_call_original - - Gitlab::Redis::SharedState.with { |redis| redis.call(:mget, '{foo}bar', '{foo}baz') } - end - it 'counts exceptions' do expect(instrumentation_class).to receive(:instance_count_exception) .with(instance_of(Redis::CommandError)).and_call_original @@ -82,16 +76,42 @@ RSpec.describe Gitlab::Instrumentation::RedisInterceptor, :clean_gitlab_redis_sh end.to raise_exception(Redis::CommandError) end - context 'in production env' do + context 'in production environment' do before do stub_rails_env('production') # to avoid raising CrossSlotError end - it 'counts cross-slot requests' do + it 'counts disallowed cross-slot requests' do expect(instrumentation_class).to receive(:increment_cross_slot_request_count).and_call_original Gitlab::Redis::SharedState.with { |redis| redis.call(:mget, 'foo', 'bar') } end + + it 'does not count allowed cross-slot requests' do + expect(instrumentation_class).not_to receive(:increment_cross_slot_request_count).and_call_original + + Gitlab::Instrumentation::RedisClusterValidator.allow_cross_slot_commands do + Gitlab::Redis::SharedState.with { |redis| redis.call(:mget, 'foo', 'bar') } + end + end + + it 'skips count for non-cross-slot requests' do + expect(instrumentation_class).not_to receive(:increment_cross_slot_request_count).and_call_original + + Gitlab::Redis::SharedState.with { |redis| redis.call(:mget, '{foo}bar', '{foo}baz') } + end + end + + context 'without active RequestStore' do + before do + ::RequestStore.end! + end + + it 'still runs cross-slot validation' do + expect do + Gitlab::Redis::SharedState.with { |redis| redis.mget('foo', 'bar') } + end.to raise_error(instance_of(Gitlab::Instrumentation::RedisClusterValidator::CrossSlotError)) + end end end diff --git a/spec/lib/gitlab/instrumentation_helper_spec.rb b/spec/lib/gitlab/instrumentation_helper_spec.rb index b2d6e1af1e7..b4738ed663c 100644 --- a/spec/lib/gitlab/instrumentation_helper_spec.rb +++ b/spec/lib/gitlab/instrumentation_helper_spec.rb @@ -71,6 +71,26 @@ RSpec.describe Gitlab::InstrumentationHelper do end end + context 'when LDAP requests are made' do + let(:provider) { 'ldapmain' } + let(:adapter) { Gitlab::Auth::Ldap::Adapter.new(provider) } + let(:conn) { instance_double(Net::LDAP::Connection, search: search) } + let(:search) { double(:search, result_code: 200) } # rubocop: disable RSpec/VerifiedDoubles + + it 'adds LDAP data' do + allow_next_instance_of(Net::LDAP) do |net_ldap| + allow(net_ldap).to receive(:use_connection).and_yield(conn) + end + + adapter.users('uid', 'foo') + subject + + # Query count should be 2, as it will call `open` then `search` + expect(payload[:net_ldap_count]).to eq(2) + expect(payload[:net_ldap_duration_s]).to be >= 0 + end + end + context 'when the request matched a Rack::Attack safelist' do it 'logs the safelist name' do Gitlab::Instrumentation::Throttle.safelist = 'foobar' diff --git a/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb b/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb new file mode 100644 index 00000000000..b81000be62a --- /dev/null +++ b/spec/lib/gitlab/metrics/subscribers/ldap_spec.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe Gitlab::Metrics::Subscribers::Ldap, :request_store, feature_category: :logging do + let(:transaction) { Gitlab::Metrics::WebTransaction.new({}) } + let(:subscriber) { described_class.new } + + let(:attributes) do + [ + :altServer, :namingContexts, :supportedCapabilities, :supportedControl, + :supportedExtension, :supportedFeatures, :supportedLdapVersion, :supportedSASLMechanisms + ] + end + + let(:event_1) do + instance_double( + ActiveSupport::Notifications::Event, + name: "open.net_ldap", + payload: { + ignore_server_caps: true, + base: "", + scope: 0, + attributes: attributes, + result: nil + }, + time: Time.current, + duration: 0.321 + ) + end + + let(:event_2) do + instance_double( + ActiveSupport::Notifications::Event, + name: "search.net_ldap", + payload: { + ignore_server_caps: true, + base: "", + scope: 0, + attributes: attributes, + result: nil + }, + time: Time.current, + duration: 0.12 + ) + end + + let(:event_3) do + instance_double( + ActiveSupport::Notifications::Event, + name: "search.net_ldap", + payload: { + ignore_server_caps: true, + base: "", + scope: 0, + attributes: attributes, + result: nil + }, + time: Time.current, + duration: 5.3 + ) + end + + around do |example| + freeze_time { example.run } + end + + describe ".payload" do + context "when SafeRequestStore is empty" do + it "returns an empty array" do + expect(described_class.payload).to eql(net_ldap_count: 0, net_ldap_duration_s: 0.0) + end + end + + context "when LDAP recorded some values" do + before do + Gitlab::SafeRequestStore[:net_ldap_count] = 7 + Gitlab::SafeRequestStore[:net_ldap_duration_s] = 1.2 + end + + it "returns the populated payload" do + expect(described_class.payload).to eql(net_ldap_count: 7, net_ldap_duration_s: 1.2) + end + end + end + + describe "#observe_event" do + before do + allow(subscriber).to receive(:current_transaction).and_return(transaction) + end + + it "tracks LDAP request count" do + expect(transaction).to receive(:increment) + .with(:gitlab_net_ldap_total, 1, { name: "open" }) + expect(transaction).to receive(:increment) + .with(:gitlab_net_ldap_total, 1, { name: "search" }) + + subscriber.observe_event(event_1) + subscriber.observe_event(event_2) + end + + it "tracks LDAP request duration" do + expect(transaction).to receive(:observe) + .with(:gitlab_net_ldap_duration_seconds, 0.321, { name: "open" }) + expect(transaction).to receive(:observe) + .with(:gitlab_net_ldap_duration_seconds, 0.12, { name: "search" }) + expect(transaction).to receive(:observe) + .with(:gitlab_net_ldap_duration_seconds, 5.3, { name: "search" }) + + subscriber.observe_event(event_1) + subscriber.observe_event(event_2) + subscriber.observe_event(event_3) + end + + it "stores per-request counters" do + subscriber.observe_event(event_1) + subscriber.observe_event(event_2) + subscriber.observe_event(event_3) + + expect(Gitlab::SafeRequestStore[:net_ldap_count]).to eq(3) + expect(Gitlab::SafeRequestStore[:net_ldap_duration_s]).to eq(5.741) # 0.321 + 0.12 + 5.3 + end + end +end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index fd86a784b2d..e231879beae 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -125,6 +125,8 @@ RSpec.describe ApplicationSetting do it { is_expected.to validate_numericality_of(:max_yaml_depth).only_integer.is_greater_than(0) } it { is_expected.to validate_presence_of(:max_pages_size) } it { is_expected.to validate_presence_of(:max_pages_custom_domains_per_project) } + it { is_expected.to validate_presence_of(:max_terraform_state_size_bytes) } + it { is_expected.to validate_numericality_of(:max_terraform_state_size_bytes).only_integer.is_greater_than_or_equal_to(0) } it 'ensures max_pages_size is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do is_expected.to validate_numericality_of(:max_pages_size).only_integer.is_greater_than_or_equal_to(0) diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index f39ff4c0bf3..5f1937d078c 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::Build do +RSpec.describe Ci::Build, feature_category: :continuous_integration do include Ci::TemplateHelpers include AfterNextHelpers @@ -2883,6 +2883,9 @@ RSpec.describe Ci::Build do value: 'var', public: true }] build.environment = 'staging' + + # CI_ENVIRONMENT_NAME is set in predefined_variables when job environment is provided + predefined_variables.insert(20, { key: 'CI_ENVIRONMENT_NAME', value: 'staging', public: true, masked: false }) end it 'matches explicit variables ordering' do diff --git a/spec/models/ci/group_variable_spec.rb b/spec/models/ci/group_variable_spec.rb index 1d904ac8025..fc5a9c879f6 100644 --- a/spec/models/ci/group_variable_spec.rb +++ b/spec/models/ci/group_variable_spec.rb @@ -2,11 +2,10 @@ require 'spec_helper' -RSpec.describe Ci::GroupVariable, feature_category: :pipeline_authoring do +RSpec.describe Ci::GroupVariable do subject { build(:ci_group_variable) } it_behaves_like "CI variable" - it_behaves_like 'includes Limitable concern' it { is_expected.to include_module(Presentable) } it { is_expected.to include_module(Ci::Maskable) } diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index 2c945898e61..cdcfe80f455 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -219,6 +219,29 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '.for_name' do + subject { described_class.for_name(name) } + + let_it_be(:pipeline1) { create(:ci_pipeline, name: 'Build pipeline') } + let_it_be(:pipeline2) { create(:ci_pipeline, name: 'Chatops pipeline') } + + context 'when name exists' do + let(:name) { 'build Pipeline' } + + it 'performs case insensitive compare' do + is_expected.to contain_exactly(pipeline1) + end + end + + context 'when name does not exist' do + let(:name) { 'absent-name' } + + it 'returns empty' do + is_expected.to be_empty + end + end + end + describe '.created_after' do let_it_be(:old_pipeline) { create(:ci_pipeline, created_at: 1.week.ago) } let_it_be(:pipeline) { create(:ci_pipeline) } diff --git a/spec/models/ci/variable_spec.rb b/spec/models/ci/variable_spec.rb index 3884bbae373..5f2b5971508 100644 --- a/spec/models/ci/variable_spec.rb +++ b/spec/models/ci/variable_spec.rb @@ -2,11 +2,10 @@ require 'spec_helper' -RSpec.describe Ci::Variable, feature_category: :pipeline_authoring do +RSpec.describe Ci::Variable do subject { build(:ci_variable) } it_behaves_like "CI variable" - it_behaves_like 'includes Limitable concern' describe 'validations' do it { is_expected.to include_module(Presentable) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 8cdb195f9c7..d3ef342168c 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -842,10 +842,12 @@ RSpec.describe Group do it 'returns matching records based on paths' do expect(described_class.by_ids_or_paths(nil, [group_path])).to match_array([group]) + expect(described_class.by_ids_or_paths(nil, [group_path.upcase])).to match_array([group]) end it 'returns matching records based on ids' do expect(described_class.by_ids_or_paths([group_id], nil)).to match_array([group]) + expect(described_class.by_ids_or_paths([group_id], [])).to match_array([group]) end it 'returns matching records based on both paths and ids' do @@ -853,6 +855,13 @@ RSpec.describe Group do expect(described_class.by_ids_or_paths([new_group.id], [group_path])).to match_array([group, new_group]) end + + it 'returns matching records based on full_paths' do + new_group = create(:group, parent: group) + + expect(described_class.by_ids_or_paths(nil, [new_group.full_path])).to match_array([new_group]) + expect(described_class.by_ids_or_paths(nil, [new_group.full_path.upcase])).to match_array([new_group]) + end end describe 'accessible_to_user' do diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 9b7a63bffdf..0ebccf1cb65 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -799,9 +799,35 @@ RSpec.describe Member do end describe '#request?' do - subject { create(:project_member, requested_at: Time.current.utc) } + context 'when request for project' do + subject { create(:project_member, requested_at: Time.current.utc) } - it { is_expected.to be_request } + it 'calls notification service but not todo service' do + expect_next_instance_of(NotificationService) do |instance| + expect(instance).to receive(:new_access_request) + end + + expect(TodoService).not_to receive(:new) + + is_expected.to be_request + end + end + + context 'when request for group' do + subject { create(:group_member, requested_at: Time.current.utc) } + + it 'calls notification and todo service' do + expect_next_instance_of(NotificationService) do |instance| + expect(instance).to receive(:new_access_request) + end + + expect_next_instance_of(TodoService) do |instance| + expect(instance).to receive(:create_member_access_request) + end + + is_expected.to be_request + end + end end describe '#pending?' do diff --git a/spec/requests/abuse_reports_controller_spec.rb b/spec/requests/abuse_reports_controller_spec.rb index 94c80ccb89a..510855d95e0 100644 --- a/spec/requests/abuse_reports_controller_spec.rb +++ b/spec/requests/abuse_reports_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe AbuseReportsController do +RSpec.describe AbuseReportsController, feature_category: :users do let(:reporter) { create(:user) } let(:user) { create(:user) } let(:attrs) do diff --git a/spec/requests/api/ci/variables_spec.rb b/spec/requests/api/ci/variables_spec.rb index b27b9054432..cafb841995d 100644 --- a/spec/requests/api/ci/variables_spec.rb +++ b/spec/requests/api/ci/variables_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Ci::Variables, feature_category: :pipeline_authoring do +RSpec.describe API::Ci::Variables do let(:user) { create(:user) } let(:user2) { create(:user) } let!(:project) { create(:project, creator_id: user.id) } @@ -114,92 +114,73 @@ RSpec.describe API::Ci::Variables, feature_category: :pipeline_authoring do describe 'POST /projects/:id/variables' do context 'authorized user with proper permissions' do - context 'when the project is below the plan limit for variables' do - it 'creates variable' do - expect do - post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, masked: true, raw: true } - end.to change { project.variables.count }.by(1) - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq('TEST_VARIABLE_2') - expect(json_response['value']).to eq('PROTECTED_VALUE_2') - expect(json_response['protected']).to be_truthy - expect(json_response['masked']).to be_truthy - expect(json_response['raw']).to be_truthy - expect(json_response['variable_type']).to eq('env_var') - end - - it 'masks the new value when logging' do - masked_params = { 'key' => 'VAR_KEY', 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' } - - expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params))) - - post api("/projects/#{project.id}/variables", user), - params: { key: 'VAR_KEY', value: 'SENSITIVE', protected: true, masked: true } - end + it 'creates variable' do + expect do + post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, masked: true, raw: true } + end.to change { project.variables.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('PROTECTED_VALUE_2') + expect(json_response['protected']).to be_truthy + expect(json_response['masked']).to be_truthy + expect(json_response['raw']).to be_truthy + expect(json_response['variable_type']).to eq('env_var') + end - it 'creates variable with optional attributes' do - expect do - post api("/projects/#{project.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' } - end.to change { project.variables.count }.by(1) - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq('TEST_VARIABLE_2') - expect(json_response['value']).to eq('VALUE_2') - expect(json_response['protected']).to be_falsey - expect(json_response['masked']).to be_falsey - expect(json_response['raw']).to be_falsey - expect(json_response['variable_type']).to eq('file') - end + it 'masks the new value when logging' do + masked_params = { 'key' => 'VAR_KEY', 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' } - it 'does not allow to duplicate variable key' do - expect do - post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2' } - end.to change { project.variables.count }.by(0) + expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params))) - expect(response).to have_gitlab_http_status(:bad_request) - end + post api("/projects/#{project.id}/variables", user), + params: { key: 'VAR_KEY', value: 'SENSITIVE', protected: true, masked: true } + end - it 'creates variable with a specific environment scope' do - expect do - post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'VALUE_2', environment_scope: 'review/*' } - end.to change { project.variables.reload.count }.by(1) + it 'creates variable with optional attributes' do + expect do + post api("/projects/#{project.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' } + end.to change { project.variables.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['protected']).to be_falsey + expect(json_response['masked']).to be_falsey + expect(json_response['raw']).to be_falsey + expect(json_response['variable_type']).to eq('file') + end - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq('TEST_VARIABLE_2') - expect(json_response['value']).to eq('VALUE_2') - expect(json_response['environment_scope']).to eq('review/*') - end + it 'does not allow to duplicate variable key' do + expect do + post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2' } + end.to change { project.variables.count }.by(0) - it 'allows duplicated variable key given different environment scopes' do - variable = create(:ci_variable, project: project) + expect(response).to have_gitlab_http_status(:bad_request) + end - expect do - post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2', environment_scope: 'review/*' } - end.to change { project.variables.reload.count }.by(1) + it 'creates variable with a specific environment scope' do + expect do + post api("/projects/#{project.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'VALUE_2', environment_scope: 'review/*' } + end.to change { project.variables.reload.count }.by(1) - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq(variable.key) - expect(json_response['value']).to eq('VALUE_2') - expect(json_response['environment_scope']).to eq('review/*') - end + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['environment_scope']).to eq('review/*') end - context 'when the project is at the plan limit for variables' do - it 'returns a variable limit error' do - create(:plan_limits, :default_plan, project_ci_variables: 1) - - expect(project.variables.count).to be(1) + it 'allows duplicated variable key given different environment scopes' do + variable = create(:ci_variable, project: project) - expect do - post api("/projects/#{project.id}/variables", user), params: { key: 'TOO_MANY_VARS', value: 'too many' } - end.not_to change { project.variables.count } + expect do + post api("/projects/#{project.id}/variables", user), params: { key: variable.key, value: 'VALUE_2', environment_scope: 'review/*' } + end.to change { project.variables.reload.count }.by(1) - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']['base']).to contain_exactly( - 'Maximum number of project ci variables (1) exceeded' - ) - end + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq(variable.key) + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['environment_scope']).to eq('review/*') end end diff --git a/spec/requests/api/graphql/mutations/timelogs/create_spec.rb b/spec/requests/api/graphql/mutations/timelogs/create_spec.rb index eea04b89783..9b89d053901 100644 --- a/spec/requests/api/graphql/mutations/timelogs/create_spec.rb +++ b/spec/requests/api/graphql/mutations/timelogs/create_spec.rb @@ -11,14 +11,6 @@ RSpec.describe 'Create a timelog' do let(:current_user) { nil } let(:users_container) { project } - let(:mutation) do - graphql_mutation(:timelogCreate, { - 'time_spent' => time_spent, - 'spent_at' => '2022-07-08', - 'summary' => 'Test summary', - 'issuable_id' => issuable.to_global_id.to_s - }) - end let(:mutation_response) { graphql_mutation_response(:timelog_create) } diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index b1213dfd66a..a07a8ae4292 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::GroupVariables, feature_category: :pipeline_authoring do +RSpec.describe API::GroupVariables do let_it_be(:group) { create(:group) } let_it_be(:user) { create(:user) } let_it_be(:variable) { create(:ci_group_variable, group: group) } @@ -88,70 +88,51 @@ RSpec.describe API::GroupVariables, feature_category: :pipeline_authoring do context 'authorized user with proper permissions' do let(:access_level) { :owner } - context 'when the group is below the plan limit for variables' do - it 'creates variable' do - expect do - post api("/groups/#{group.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, masked: true, raw: true } - end.to change { group.variables.count }.by(1) - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq('TEST_VARIABLE_2') - expect(json_response['value']).to eq('PROTECTED_VALUE_2') - expect(json_response['protected']).to be_truthy - expect(json_response['masked']).to be_truthy - expect(json_response['variable_type']).to eq('env_var') - expect(json_response['environment_scope']).to eq('*') - expect(json_response['raw']).to be_truthy - end - - it 'masks the new value when logging' do - masked_params = { 'key' => 'VAR_KEY', 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' } - - expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params))) - - post api("/groups/#{group.id}/variables", user), - params: { key: 'VAR_KEY', value: 'SENSITIVE', protected: true, masked: true } - end - - it 'creates variable with optional attributes' do - expect do - post api("/groups/#{group.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' } - end.to change { group.variables.count }.by(1) - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['key']).to eq('TEST_VARIABLE_2') - expect(json_response['value']).to eq('VALUE_2') - expect(json_response['protected']).to be_falsey - expect(json_response['masked']).to be_falsey - expect(json_response['raw']).to be_falsey - expect(json_response['variable_type']).to eq('file') - expect(json_response['environment_scope']).to eq('*') - end - - it 'does not allow to duplicate variable key' do - expect do - post api("/groups/#{group.id}/variables", user), params: { key: variable.key, value: 'VALUE_2' } - end.to change { group.variables.count }.by(0) - - expect(response).to have_gitlab_http_status(:bad_request) - end + it 'creates variable' do + expect do + post api("/groups/#{group.id}/variables", user), params: { key: 'TEST_VARIABLE_2', value: 'PROTECTED_VALUE_2', protected: true, masked: true, raw: true } + end.to change { group.variables.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('PROTECTED_VALUE_2') + expect(json_response['protected']).to be_truthy + expect(json_response['masked']).to be_truthy + expect(json_response['variable_type']).to eq('env_var') + expect(json_response['environment_scope']).to eq('*') + expect(json_response['raw']).to be_truthy end - context 'when the group is at the plan limit for variables' do - it 'returns a variable limit error' do - create(:plan_limits, :default_plan, group_ci_variables: 1) + it 'masks the new value when logging' do + masked_params = { 'key' => 'VAR_KEY', 'value' => '[FILTERED]', 'protected' => 'true', 'masked' => 'true' } + + expect(::API::API::LOGGER).to receive(:info).with(include(params: include(masked_params))) + + post api("/groups/#{group.id}/variables", user), + params: { key: 'VAR_KEY', value: 'SENSITIVE', protected: true, masked: true } + end - expect(group.variables.count).to be(1) + it 'creates variable with optional attributes' do + expect do + post api("/groups/#{group.id}/variables", user), params: { variable_type: 'file', key: 'TEST_VARIABLE_2', value: 'VALUE_2' } + end.to change { group.variables.count }.by(1) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['key']).to eq('TEST_VARIABLE_2') + expect(json_response['value']).to eq('VALUE_2') + expect(json_response['protected']).to be_falsey + expect(json_response['masked']).to be_falsey + expect(json_response['raw']).to be_falsey + expect(json_response['variable_type']).to eq('file') + expect(json_response['environment_scope']).to eq('*') + end - expect do - post api("/groups/#{group.id}/variables", user), params: { key: 'TOO_MANY_VARS', value: 'too many' } - end.not_to change { group.variables.count } + it 'does not allow to duplicate variable key' do + expect do + post api("/groups/#{group.id}/variables", user), params: { key: variable.key, value: 'VALUE_2' } + end.to change { group.variables.count }.by(0) - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']['base']).to contain_exactly( - 'Maximum number of group ci variables (1) exceeded' - ) - end + expect(response).to have_gitlab_http_status(:bad_request) end end diff --git a/spec/requests/content_security_policy_spec.rb b/spec/requests/content_security_policy_spec.rb index 06fc5b0e190..3f0665f1ce5 100644 --- a/spec/requests/content_security_policy_spec.rb +++ b/spec/requests/content_security_policy_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' # The AnonymousController doesn't support setting the CSP # This is why an arbitrary test request was chosen instead # of testing in application_controller_spec. -RSpec.describe 'Content Security Policy' do +RSpec.describe 'Content Security Policy', feature_category: :application_instrumentation do let(:snowplow_host) { 'snowplow.example.com' } shared_examples 'snowplow is not in the CSP' do diff --git a/spec/requests/dashboard_controller_spec.rb b/spec/requests/dashboard_controller_spec.rb index 62655d720c5..9edacb27c93 100644 --- a/spec/requests/dashboard_controller_spec.rb +++ b/spec/requests/dashboard_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe DashboardController do +RSpec.describe DashboardController, feature_category: :authentication_and_authorization do context 'token authentication' do it_behaves_like 'authenticates sessionless user for the request spec', 'issues atom', public_resource: false do let(:url) { issues_dashboard_url(:atom, assignee_username: user.username) } diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 20d298edfe5..66337b94c75 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Git HTTP requests' do +RSpec.describe 'Git HTTP requests', feature_category: :source_code_management do include ProjectForksHelper include TermsHelper include GitHttpHelpers diff --git a/spec/requests/groups_controller_spec.rb b/spec/requests/groups_controller_spec.rb index 422c108f2ad..7fc14910819 100644 --- a/spec/requests/groups_controller_spec.rb +++ b/spec/requests/groups_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe GroupsController do +RSpec.describe GroupsController, feature_category: :subgroups do context 'token authentication' do context 'when public group' do let_it_be(:public_group) { create(:group, :public) } diff --git a/spec/requests/health_controller_spec.rb b/spec/requests/health_controller_spec.rb index ae15b63df19..83ec1565095 100644 --- a/spec/requests/health_controller_spec.rb +++ b/spec/requests/health_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe HealthController do +RSpec.describe HealthController, feature_category: :database do include StubENV let(:token) { Gitlab::CurrentSettings.health_check_access_token } diff --git a/spec/requests/ide_controller_spec.rb b/spec/requests/ide_controller_spec.rb index 8d61399c824..6786d313ce3 100644 --- a/spec/requests/ide_controller_spec.rb +++ b/spec/requests/ide_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe IdeController do +RSpec.describe IdeController, feature_category: :web_ide do using RSpec::Parameterized::TableSyntax let_it_be(:reporter) { create(:user) } diff --git a/spec/requests/jira_authorizations_spec.rb b/spec/requests/jira_authorizations_spec.rb index f67288b286b..8c27b61712c 100644 --- a/spec/requests/jira_authorizations_spec.rb +++ b/spec/requests/jira_authorizations_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Jira authorization requests' do +RSpec.describe 'Jira authorization requests', feature_category: :integrations do let(:user) { create :user } let(:application) { create :oauth_application, scopes: 'api' } let(:redirect_uri) { oauth_jira_dvcs_callback_url(host: "http://www.example.com") } diff --git a/spec/requests/jira_routing_spec.rb b/spec/requests/jira_routing_spec.rb index e0e170044de..f1edb58bb10 100644 --- a/spec/requests/jira_routing_spec.rb +++ b/spec/requests/jira_routing_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Jira referenced paths', type: :request do +RSpec.describe 'Jira referenced paths', type: :request, feature_category: :integrations do using RSpec::Parameterized::TableSyntax let(:user) { create(:user) } diff --git a/spec/requests/jwks_controller_spec.rb b/spec/requests/jwks_controller_spec.rb index c9dcc238c29..ac9765c35d8 100644 --- a/spec/requests/jwks_controller_spec.rb +++ b/spec/requests/jwks_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JwksController do +RSpec.describe JwksController, feature_category: :authentication_and_authorization do describe 'Endpoints from the parent Doorkeeper::OpenidConnect::DiscoveryController' do it 'respond successfully' do [ diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index e6916e02fde..00222cb1977 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe JwtController do +RSpec.describe JwtController, feature_category: :authentication_and_authorization do include_context 'parsed logs' let(:service) { double(execute: {} ) } diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb index a3fa85dee73..e5f03d16dda 100644 --- a/spec/requests/lfs_http_spec.rb +++ b/spec/requests/lfs_http_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe 'Git LFS API and storage' do +RSpec.describe 'Git LFS API and storage', feature_category: :source_code_management do using RSpec::Parameterized::TableSyntax include LfsHttpHelpers diff --git a/spec/requests/lfs_locks_api_spec.rb b/spec/requests/lfs_locks_api_spec.rb index 0eb3cb4ca07..363a16f014b 100644 --- a/spec/requests/lfs_locks_api_spec.rb +++ b/spec/requests/lfs_locks_api_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Git LFS File Locking API' do +RSpec.describe 'Git LFS File Locking API', feature_category: :source_code_management do include LfsHttpHelpers include WorkhorseHelpers diff --git a/spec/requests/oauth_tokens_spec.rb b/spec/requests/oauth_tokens_spec.rb index f2fb380bde0..053bd317fcc 100644 --- a/spec/requests/oauth_tokens_spec.rb +++ b/spec/requests/oauth_tokens_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'OAuth Tokens requests' do +RSpec.describe 'OAuth Tokens requests', feature_category: :authentication_and_authorization do let(:user) { create :user } let(:application) { create :oauth_application, scopes: 'api' } let(:grant_type) { 'authorization_code' } diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 3a40fec58e8..b45f4f1e39f 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'OpenID Connect requests' do +RSpec.describe 'OpenID Connect requests', feature_category: :authentication_and_authorization do let(:user) do create( :user, diff --git a/spec/requests/projects/ci/promeheus_metrics/histograms_controller_spec.rb b/spec/requests/projects/ci/promeheus_metrics/histograms_controller_spec.rb index c5e7369b0a9..b0c7427fa81 100644 --- a/spec/requests/projects/ci/promeheus_metrics/histograms_controller_spec.rb +++ b/spec/requests/projects/ci/promeheus_metrics/histograms_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Projects::Ci::PrometheusMetrics::HistogramsController' do +RSpec.describe 'Projects::Ci::PrometheusMetrics::HistogramsController', feature_category: :pipeline_authoring do let_it_be(:project) { create(:project, :public) } describe 'POST /*namespace_id/:project_id/-/ci/prometheus_metrics/histograms' do diff --git a/spec/requests/projects/cluster_agents_controller_spec.rb b/spec/requests/projects/cluster_agents_controller_spec.rb index 914d5b17ba8..d7c791fa0c1 100644 --- a/spec/requests/projects/cluster_agents_controller_spec.rb +++ b/spec/requests/projects/cluster_agents_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::ClusterAgentsController do +RSpec.describe Projects::ClusterAgentsController, feature_category: :kubernetes_management do let_it_be(:cluster_agent) { create(:cluster_agent) } let(:project) { cluster_agent.project } diff --git a/spec/requests/projects/clusters/integrations_controller_spec.rb b/spec/requests/projects/clusters/integrations_controller_spec.rb index c05e3da675c..505b63e1ff6 100644 --- a/spec/requests/projects/clusters/integrations_controller_spec.rb +++ b/spec/requests/projects/clusters/integrations_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::Clusters::IntegrationsController do +RSpec.describe Projects::Clusters::IntegrationsController, feature_category: :integrations do include AccessMatchersForController shared_examples 'a secure endpoint' do diff --git a/spec/requests/projects/commits_controller_spec.rb b/spec/requests/projects/commits_controller_spec.rb index 158902c0ffd..f9e3ef82fc1 100644 --- a/spec/requests/projects/commits_controller_spec.rb +++ b/spec/requests/projects/commits_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::CommitsController do +RSpec.describe Projects::CommitsController, feature_category: :source_code_management do context 'token authentication' do context 'when public project' do let_it_be(:public_project) { create(:project, :repository, :public) } diff --git a/spec/requests/projects/cycle_analytics_events_spec.rb b/spec/requests/projects/cycle_analytics_events_spec.rb index 370febf82ff..3f9dd74c145 100644 --- a/spec/requests/projects/cycle_analytics_events_spec.rb +++ b/spec/requests/projects/cycle_analytics_events_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'value stream analytics events' do +RSpec.describe 'value stream analytics events', feature_category: :planning_analytics do include CycleAnalyticsHelpers let(:user) { create(:user) } diff --git a/spec/requests/projects/environments_controller_spec.rb b/spec/requests/projects/environments_controller_spec.rb index 66ab265fc0f..41ae2d434fa 100644 --- a/spec/requests/projects/environments_controller_spec.rb +++ b/spec/requests/projects/environments_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::EnvironmentsController do +RSpec.describe Projects::EnvironmentsController, feature_category: :continuous_delivery do let_it_be_with_refind(:project) { create(:project, :repository) } let(:environment) { create(:environment, name: 'production', project: project) } diff --git a/spec/requests/projects/google_cloud/configuration_controller_spec.rb b/spec/requests/projects/google_cloud/configuration_controller_spec.rb index 41593b8d7a7..1aa44d1a49a 100644 --- a/spec/requests/projects/google_cloud/configuration_controller_spec.rb +++ b/spec/requests/projects/google_cloud/configuration_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::GoogleCloud::ConfigurationController do +RSpec.describe Projects::GoogleCloud::ConfigurationController, feature_category: :kubernetes_management do let_it_be(:project) { create(:project, :public) } let_it_be(:url) { project_google_cloud_configuration_path(project) } diff --git a/spec/requests/projects/google_cloud/databases_controller_spec.rb b/spec/requests/projects/google_cloud/databases_controller_spec.rb index 4edef71f326..e91a51ce2ef 100644 --- a/spec/requests/projects/google_cloud/databases_controller_spec.rb +++ b/spec/requests/projects/google_cloud/databases_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::GoogleCloud::DatabasesController, :snowplow do +RSpec.describe Projects::GoogleCloud::DatabasesController, :snowplow, feature_category: :kubernetes_management do shared_examples 'shared examples for database controller endpoints' do include_examples 'requires `admin_project_google_cloud` role' diff --git a/spec/requests/projects/google_cloud/deployments_controller_spec.rb b/spec/requests/projects/google_cloud/deployments_controller_spec.rb index c777e8c1f69..d564a31f835 100644 --- a/spec/requests/projects/google_cloud/deployments_controller_spec.rb +++ b/spec/requests/projects/google_cloud/deployments_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::GoogleCloud::DeploymentsController do +RSpec.describe Projects::GoogleCloud::DeploymentsController, feature_category: :kubernetes_management do let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:repository) { project.repository } diff --git a/spec/requests/projects/google_cloud/gcp_regions_controller_spec.rb b/spec/requests/projects/google_cloud/gcp_regions_controller_spec.rb index e77bcdb40b8..de4b96a2e01 100644 --- a/spec/requests/projects/google_cloud/gcp_regions_controller_spec.rb +++ b/spec/requests/projects/google_cloud/gcp_regions_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::GoogleCloud::GcpRegionsController do +RSpec.describe Projects::GoogleCloud::GcpRegionsController, feature_category: :kubernetes_management do let_it_be(:project) { create(:project, :public, :repository) } let_it_be(:repository) { project.repository } diff --git a/spec/requests/projects/google_cloud/revoke_oauth_controller_spec.rb b/spec/requests/projects/google_cloud/revoke_oauth_controller_spec.rb index 9bd8468767d..5965953cf6f 100644 --- a/spec/requests/projects/google_cloud/revoke_oauth_controller_spec.rb +++ b/spec/requests/projects/google_cloud/revoke_oauth_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::GoogleCloud::RevokeOauthController do +RSpec.describe Projects::GoogleCloud::RevokeOauthController, feature_category: :kubernetes_management do include SessionHelpers describe 'POST #create', :snowplow, :clean_gitlab_redis_sessions, :aggregate_failures do diff --git a/spec/requests/projects/google_cloud/service_accounts_controller_spec.rb b/spec/requests/projects/google_cloud/service_accounts_controller_spec.rb index d91e5a4f068..9b048f814ef 100644 --- a/spec/requests/projects/google_cloud/service_accounts_controller_spec.rb +++ b/spec/requests/projects/google_cloud/service_accounts_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::GoogleCloud::ServiceAccountsController do +RSpec.describe Projects::GoogleCloud::ServiceAccountsController, feature_category: :kubernetes_management do let_it_be(:project) { create(:project, :public) } describe 'GET index', :snowplow do diff --git a/spec/requests/projects/harbor/artifacts_controller_spec.rb b/spec/requests/projects/harbor/artifacts_controller_spec.rb index 310fbcf0a0f..d0ed93acaf7 100644 --- a/spec/requests/projects/harbor/artifacts_controller_spec.rb +++ b/spec/requests/projects/harbor/artifacts_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::Harbor::ArtifactsController do +RSpec.describe Projects::Harbor::ArtifactsController, feature_category: :build_artifacts do it_behaves_like 'a harbor artifacts controller', anonymous_status_code: '302' do let_it_be(:container) { create(:project) } let_it_be(:harbor_integration) { create(:harbor_integration, project: container) } diff --git a/spec/requests/projects/harbor/repositories_controller_spec.rb b/spec/requests/projects/harbor/repositories_controller_spec.rb index 751becaa20a..7430ac5a64f 100644 --- a/spec/requests/projects/harbor/repositories_controller_spec.rb +++ b/spec/requests/projects/harbor/repositories_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::Harbor::RepositoriesController do +RSpec.describe Projects::Harbor::RepositoriesController, feature_category: :source_code_management do it_behaves_like 'a harbor repositories controller', anonymous_status_code: '302' do let_it_be(:container, reload: true) { create(:project) } let_it_be(:harbor_integration) { create(:harbor_integration, project: container) } diff --git a/spec/requests/projects/harbor/tags_controller_spec.rb b/spec/requests/projects/harbor/tags_controller_spec.rb index 119d1c746ac..f1ac2f01c57 100644 --- a/spec/requests/projects/harbor/tags_controller_spec.rb +++ b/spec/requests/projects/harbor/tags_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::Harbor::TagsController do +RSpec.describe Projects::Harbor::TagsController, feature_category: :source_code_management do it_behaves_like 'a harbor tags controller', anonymous_status_code: '302' do let_it_be(:container) { create(:project) } let_it_be(:harbor_integration) { create(:harbor_integration, project: container) } diff --git a/spec/requests/projects/hook_logs_controller_spec.rb b/spec/requests/projects/hook_logs_controller_spec.rb index 8b3ec307e53..c71906b4895 100644 --- a/spec/requests/projects/hook_logs_controller_spec.rb +++ b/spec/requests/projects/hook_logs_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::HookLogsController do +RSpec.describe Projects::HookLogsController, feature_category: :integrations do let_it_be(:user) { create(:user) } let_it_be_with_refind(:web_hook) { create(:project_hook) } let_it_be_with_refind(:web_hook_log) { create(:web_hook_log, web_hook: web_hook) } diff --git a/spec/requests/projects/incident_management/pagerduty_incidents_spec.rb b/spec/requests/projects/incident_management/pagerduty_incidents_spec.rb index 32434435475..a25dcb7f299 100644 --- a/spec/requests/projects/incident_management/pagerduty_incidents_spec.rb +++ b/spec/requests/projects/incident_management/pagerduty_incidents_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'PagerDuty webhook' do +RSpec.describe 'PagerDuty webhook', feature_category: :incident_management do let_it_be(:project) { create(:project) } describe 'POST /incidents/pagerduty' do diff --git a/spec/requests/projects/incident_management/timeline_events_spec.rb b/spec/requests/projects/incident_management/timeline_events_spec.rb index f7dead4834d..22a1f654ee2 100644 --- a/spec/requests/projects/incident_management/timeline_events_spec.rb +++ b/spec/requests/projects/incident_management/timeline_events_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Timeline Events' do +RSpec.describe 'Timeline Events', feature_category: :incident_management do let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user) } let_it_be(:incident) { create(:incident, project: project) } diff --git a/spec/requests/projects/integrations/shimos_controller_spec.rb b/spec/requests/projects/integrations/shimos_controller_spec.rb index 7322143f87e..bd7af0bb4ac 100644 --- a/spec/requests/projects/integrations/shimos_controller_spec.rb +++ b/spec/requests/projects/integrations/shimos_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ::Projects::Integrations::ShimosController do +RSpec.describe ::Projects::Integrations::ShimosController, feature_category: :integrations do let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user, developer_projects: [project]) } let_it_be(:shimo_integration) { create(:shimo_integration, project: project) } diff --git a/spec/requests/projects/issue_links_controller_spec.rb b/spec/requests/projects/issue_links_controller_spec.rb index e5f40625cfa..0535156b4b8 100644 --- a/spec/requests/projects/issue_links_controller_spec.rb +++ b/spec/requests/projects/issue_links_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::IssueLinksController do +RSpec.describe Projects::IssueLinksController, feature_category: :team_planning do let(:user) { create :user } let(:project) { create(:project_empty_repo) } let(:issue) { create :issue, project: project } diff --git a/spec/requests/projects/issues/discussions_spec.rb b/spec/requests/projects/issues/discussions_spec.rb index dcdca2d9c27..0f4a0bd2e5c 100644 --- a/spec/requests/projects/issues/discussions_spec.rb +++ b/spec/requests/projects/issues/discussions_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'issue discussions' do +RSpec.describe 'issue discussions', feature_category: :team_planning do describe 'GET /:namespace/:project/-/issues/:iid/discussions' do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project) } diff --git a/spec/requests/projects/issues_controller_spec.rb b/spec/requests/projects/issues_controller_spec.rb index aa2ba5e114b..8db21e21235 100644 --- a/spec/requests/projects/issues_controller_spec.rb +++ b/spec/requests/projects/issues_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::IssuesController do +RSpec.describe Projects::IssuesController, feature_category: :team_planning do let_it_be(:issue) { create(:issue) } let_it_be(:group) { create(:group) } let_it_be(:project) { issue.project } diff --git a/spec/requests/projects/merge_requests/content_spec.rb b/spec/requests/projects/merge_requests/content_spec.rb index 7e5ec6f64c4..6c58dcb5722 100644 --- a/spec/requests/projects/merge_requests/content_spec.rb +++ b/spec/requests/projects/merge_requests/content_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'merge request content spec' do +RSpec.describe 'merge request content spec', feature_category: :code_review do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } let_it_be(:merge_request) { create(:merge_request, :with_head_pipeline, target_project: project, source_project: project) } diff --git a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb index ec65e8cf11e..10e57970704 100644 --- a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb +++ b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Merge Requests Context Commit Diffs' do +RSpec.describe 'Merge Requests Context Commit Diffs', feature_category: :code_review do let_it_be(:sha1) { "33f3729a45c02fc67d00adb1b8bca394b0e761d9" } let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } diff --git a/spec/requests/projects/merge_requests/creations_spec.rb b/spec/requests/projects/merge_requests/creations_spec.rb index 842ad01656e..fc409d8d676 100644 --- a/spec/requests/projects/merge_requests/creations_spec.rb +++ b/spec/requests/projects/merge_requests/creations_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'merge requests creations' do +RSpec.describe 'merge requests creations', feature_category: :code_review do describe 'GET /:namespace/:project/merge_requests/new' do include ProjectForksHelper diff --git a/spec/requests/projects/merge_requests/diffs_spec.rb b/spec/requests/projects/merge_requests/diffs_spec.rb index 11a01bf1713..858acac7f0d 100644 --- a/spec/requests/projects/merge_requests/diffs_spec.rb +++ b/spec/requests/projects/merge_requests/diffs_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Merge Requests Diffs' do +RSpec.describe 'Merge Requests Diffs', feature_category: :code_review do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } let_it_be(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } diff --git a/spec/requests/projects/merge_requests_controller_spec.rb b/spec/requests/projects/merge_requests_controller_spec.rb index 2ee86bb423b..a131d38186a 100644 --- a/spec/requests/projects/merge_requests_controller_spec.rb +++ b/spec/requests/projects/merge_requests_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::MergeRequestsController do +RSpec.describe Projects::MergeRequestsController, feature_category: :source_code_management do describe 'GET #discussions' do let_it_be(:merge_request) { create(:merge_request) } let_it_be(:project) { merge_request.project } diff --git a/spec/requests/projects/merge_requests_discussions_spec.rb b/spec/requests/projects/merge_requests_discussions_spec.rb index 305ca6147be..d82fa284a42 100644 --- a/spec/requests/projects/merge_requests_discussions_spec.rb +++ b/spec/requests/projects/merge_requests_discussions_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'merge requests discussions' do +RSpec.describe 'merge requests discussions', feature_category: :source_code_management do # Further tests can be found at merge_requests_controller_spec.rb describe 'GET /:namespace/:project/-/merge_requests/:iid/discussions' do let(:project) { create(:project, :repository, :public) } diff --git a/spec/requests/projects/merge_requests_spec.rb b/spec/requests/projects/merge_requests_spec.rb index 91153554e55..9600d1a3656 100644 --- a/spec/requests/projects/merge_requests_spec.rb +++ b/spec/requests/projects/merge_requests_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'merge requests actions' do +RSpec.describe 'merge requests actions', feature_category: :source_code_management do let_it_be(:project) { create(:project, :repository) } let(:merge_request) do diff --git a/spec/requests/projects/metrics/dashboards/builder_spec.rb b/spec/requests/projects/metrics/dashboards/builder_spec.rb index 002acca2135..c929beaed70 100644 --- a/spec/requests/projects/metrics/dashboards/builder_spec.rb +++ b/spec/requests/projects/metrics/dashboards/builder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Projects::Metrics::Dashboards::BuilderController' do +RSpec.describe 'Projects::Metrics::Dashboards::BuilderController', feature_category: :metrics do let_it_be(:project) { create(:project) } let_it_be(:environment) { create(:environment, project: project) } let_it_be(:user) { create(:user) } diff --git a/spec/requests/projects/metrics_dashboard_spec.rb b/spec/requests/projects/metrics_dashboard_spec.rb index 61bfe1c6edf..01925f8345b 100644 --- a/spec/requests/projects/metrics_dashboard_spec.rb +++ b/spec/requests/projects/metrics_dashboard_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Projects::MetricsDashboardController' do +RSpec.describe 'Projects::MetricsDashboardController', feature_category: :metrics do let_it_be(:project) { create(:project) } let_it_be(:environment) { create(:environment, project: project) } let_it_be(:environment2) { create(:environment, project: project) } diff --git a/spec/requests/projects/noteable_notes_spec.rb b/spec/requests/projects/noteable_notes_spec.rb index 44ee50ca002..5699bf17b80 100644 --- a/spec/requests/projects/noteable_notes_spec.rb +++ b/spec/requests/projects/noteable_notes_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Project noteable notes' do +RSpec.describe 'Project noteable notes', feature_category: :team_planning do describe '#index' do let_it_be(:merge_request) { create(:merge_request) } diff --git a/spec/requests/projects/packages/package_files_controller_spec.rb b/spec/requests/projects/packages/package_files_controller_spec.rb index a6daf57f0fa..e5849be9f13 100644 --- a/spec/requests/projects/packages/package_files_controller_spec.rb +++ b/spec/requests/projects/packages/package_files_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::Packages::PackageFilesController do +RSpec.describe Projects::Packages::PackageFilesController, feature_category: :package_registry do let_it_be(:project) { create(:project, :public) } let_it_be(:package) { create(:package, project: project) } let_it_be(:package_file) { create(:package_file, package: package) } diff --git a/spec/requests/projects/pipelines_controller_spec.rb b/spec/requests/projects/pipelines_controller_spec.rb index 1c6b1039aee..7f185ade339 100644 --- a/spec/requests/projects/pipelines_controller_spec.rb +++ b/spec/requests/projects/pipelines_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::PipelinesController do +RSpec.describe Projects::PipelinesController, feature_category: :continuous_integration do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project, :repository) } let_it_be(:pipeline) { create(:ci_pipeline, project: project) } diff --git a/spec/requests/projects/redirect_controller_spec.rb b/spec/requests/projects/redirect_controller_spec.rb index 3bbca3ca32b..e828c546198 100644 --- a/spec/requests/projects/redirect_controller_spec.rb +++ b/spec/requests/projects/redirect_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe "Projects::RedirectController requests" do +RSpec.describe "Projects::RedirectController requests", feature_category: :projects do using RSpec::Parameterized::TableSyntax let_it_be(:private_project) { create(:project, :private) } diff --git a/spec/requests/projects/releases_controller_spec.rb b/spec/requests/projects/releases_controller_spec.rb index 8e492125ace..d331142583d 100644 --- a/spec/requests/projects/releases_controller_spec.rb +++ b/spec/requests/projects/releases_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Projects::ReleasesController' do +RSpec.describe 'Projects::ReleasesController', feature_category: :release_orchestration do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } diff --git a/spec/requests/projects/settings/access_tokens_controller_spec.rb b/spec/requests/projects/settings/access_tokens_controller_spec.rb index 17389cdcce7..defb35fd496 100644 --- a/spec/requests/projects/settings/access_tokens_controller_spec.rb +++ b/spec/requests/projects/settings/access_tokens_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::Settings::AccessTokensController do +RSpec.describe Projects::Settings::AccessTokensController, feature_category: :authentication_and_authorization do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:resource) { create(:project, group: group) } diff --git a/spec/requests/projects/settings/integration_hook_logs_controller_spec.rb b/spec/requests/projects/settings/integration_hook_logs_controller_spec.rb index 77daff901a1..6cd0df19468 100644 --- a/spec/requests/projects/settings/integration_hook_logs_controller_spec.rb +++ b/spec/requests/projects/settings/integration_hook_logs_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::Settings::IntegrationHookLogsController do +RSpec.describe Projects::Settings::IntegrationHookLogsController, feature_category: :integrations do let_it_be(:user) { create(:user) } let_it_be(:integration) { create(:datadog_integration) } let_it_be_with_refind(:web_hook) { integration.service_hook } diff --git a/spec/requests/projects/settings/packages_and_registries_controller_spec.rb b/spec/requests/projects/settings/packages_and_registries_controller_spec.rb index 6d8a152c769..2806beadd4e 100644 --- a/spec/requests/projects/settings/packages_and_registries_controller_spec.rb +++ b/spec/requests/projects/settings/packages_and_registries_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::Settings::PackagesAndRegistriesController do +RSpec.describe Projects::Settings::PackagesAndRegistriesController, feature_category: :package_registry do let_it_be(:user) { create(:user) } let_it_be(:project, reload: true) { create(:project, namespace: user.namespace) } diff --git a/spec/requests/projects/tags_controller_spec.rb b/spec/requests/projects/tags_controller_spec.rb index b9531a2739c..c0b0b1728c2 100644 --- a/spec/requests/projects/tags_controller_spec.rb +++ b/spec/requests/projects/tags_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::TagsController do +RSpec.describe Projects::TagsController, feature_category: :source_code_management do context 'token authentication' do context 'when public project' do let_it_be(:public_project) { create(:project, :repository, :public) } diff --git a/spec/requests/projects/uploads_spec.rb b/spec/requests/projects/uploads_spec.rb index de5ef36be7e..aec2636b69c 100644 --- a/spec/requests/projects/uploads_spec.rb +++ b/spec/requests/projects/uploads_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'File uploads' do +RSpec.describe 'File uploads', feature_category: :not_owned do include WorkhorseHelpers let(:project) { create(:project, :public, :repository) } diff --git a/spec/requests/projects/usage_quotas_spec.rb b/spec/requests/projects/usage_quotas_spec.rb index 6e449a21804..60ab64c30c3 100644 --- a/spec/requests/projects/usage_quotas_spec.rb +++ b/spec/requests/projects/usage_quotas_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Project Usage Quotas' do +RSpec.describe 'Project Usage Quotas', feature_category: :subscription_cost_management do let_it_be(:project) { create(:project) } let_it_be(:role) { :maintainer } let_it_be(:user) { create(:user) } diff --git a/spec/requests/projects/work_items_spec.rb b/spec/requests/projects/work_items_spec.rb index 4d7acc73d4f..056416d380d 100644 --- a/spec/requests/projects/work_items_spec.rb +++ b/spec/requests/projects/work_items_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Work Items' do +RSpec.describe 'Work Items', feature_category: :team_planning do let_it_be(:work_item) { create(:work_item) } let_it_be(:developer) { create(:user) } diff --git a/spec/requests/projects_controller_spec.rb b/spec/requests/projects_controller_spec.rb index d2200d5a4ec..f08f3578dc0 100644 --- a/spec/requests/projects_controller_spec.rb +++ b/spec/requests/projects_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ProjectsController do +RSpec.describe ProjectsController, feature_category: :projects do context 'token authentication' do context 'when public project' do let_it_be(:public_project) { create(:project, :public) } diff --git a/spec/requests/pwa_controller_spec.rb b/spec/requests/pwa_controller_spec.rb index 7a295b17231..68adcb3179f 100644 --- a/spec/requests/pwa_controller_spec.rb +++ b/spec/requests/pwa_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe PwaController do +RSpec.describe PwaController, feature_category: :navigation do describe 'GET #manifest' do it 'responds with json' do get manifest_path(format: :json) diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb index f6b9bc527ac..643a98da441 100644 --- a/spec/requests/rack_attack_global_spec.rb +++ b/spec/requests/rack_attack_global_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_caching do +RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_caching, +feature_category: :authentication_and_authorization do include RackAttackSpecHelpers include SessionHelpers diff --git a/spec/requests/recursive_webhook_detection_spec.rb b/spec/requests/recursive_webhook_detection_spec.rb index fe27c90b6c8..a74d4f9a603 100644 --- a/spec/requests/recursive_webhook_detection_spec.rb +++ b/spec/requests/recursive_webhook_detection_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_redis_shared_state, :request_store do +RSpec.describe 'Recursive webhook detection', :sidekiq_inline, :clean_gitlab_redis_shared_state, :request_store, +feature_category: :integrations do include StubRequests let_it_be(:user) { create(:user) } diff --git a/spec/requests/robots_txt_spec.rb b/spec/requests/robots_txt_spec.rb index 7c0b7d8117a..18a14677e0c 100644 --- a/spec/requests/robots_txt_spec.rb +++ b/spec/requests/robots_txt_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Robots.txt Requests', :aggregate_failures do +RSpec.describe 'Robots.txt Requests', :aggregate_failures, feature_category: :build do before do Gitlab::Testing::RobotsBlockerMiddleware.block_requests! end diff --git a/spec/requests/runner_setup_controller_spec.rb b/spec/requests/runner_setup_controller_spec.rb index 665c896e30d..5a9c296afd1 100644 --- a/spec/requests/runner_setup_controller_spec.rb +++ b/spec/requests/runner_setup_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe RunnerSetupController do +RSpec.describe RunnerSetupController, feature_category: :runner do let(:user) { create(:user) } before do diff --git a/spec/requests/sandbox_controller_spec.rb b/spec/requests/sandbox_controller_spec.rb index 4fc26580123..77913065380 100644 --- a/spec/requests/sandbox_controller_spec.rb +++ b/spec/requests/sandbox_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe SandboxController do +RSpec.describe SandboxController, feature_category: :not_owned do describe 'GET #mermaid' do it 'renders page without template' do get sandbox_mermaid_path diff --git a/spec/requests/search_controller_spec.rb b/spec/requests/search_controller_spec.rb index 41f4418b582..98dda75a2b0 100644 --- a/spec/requests/search_controller_spec.rb +++ b/spec/requests/search_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe SearchController, type: :request do +RSpec.describe SearchController, type: :request, feature_category: :global_search do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :public, :repository, :wiki_repo, name: 'awesome project', group: group) } diff --git a/spec/requests/self_monitoring_project_spec.rb b/spec/requests/self_monitoring_project_spec.rb index 64c5f94657d..ce4dd10a52d 100644 --- a/spec/requests/self_monitoring_project_spec.rb +++ b/spec/requests/self_monitoring_project_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Self-Monitoring project requests' do +RSpec.describe 'Self-Monitoring project requests', feature_category: :projects do let(:admin) { create(:admin) } describe 'POST #create_self_monitoring_project' do diff --git a/spec/requests/sessions_spec.rb b/spec/requests/sessions_spec.rb index 95df181b7b0..7b3fd23980a 100644 --- a/spec/requests/sessions_spec.rb +++ b/spec/requests/sessions_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Sessions' do +RSpec.describe 'Sessions', feature_category: :authentication_and_authorization do context 'authentication', :allow_forgery_protection do let(:user) { create(:user) } diff --git a/spec/requests/terraform/services_controller_spec.rb b/spec/requests/terraform/services_controller_spec.rb index 54f7348513e..928c57613fa 100644 --- a/spec/requests/terraform/services_controller_spec.rb +++ b/spec/requests/terraform/services_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Terraform::ServicesController do +RSpec.describe Terraform::ServicesController, feature_category: :package_registry do describe 'GET /.well-known/terraform.json' do subject { get '/.well-known/terraform.json' } diff --git a/spec/requests/user_activity_spec.rb b/spec/requests/user_activity_spec.rb index 148bb2d6fae..f9682d81640 100644 --- a/spec/requests/user_activity_spec.rb +++ b/spec/requests/user_activity_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Update of user activity' do +RSpec.describe 'Update of user activity', feature_category: :users do paths_to_visit = [ '/group', '/group/project', diff --git a/spec/requests/user_avatar_spec.rb b/spec/requests/user_avatar_spec.rb index 1397741af18..4e3c2744d56 100644 --- a/spec/requests/user_avatar_spec.rb +++ b/spec/requests/user_avatar_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Loading a user avatar' do +RSpec.describe 'Loading a user avatar', feature_category: :users do let(:user) { create(:user, :with_avatar) } context 'when logged in' do diff --git a/spec/requests/user_sends_malformed_strings_spec.rb b/spec/requests/user_sends_malformed_strings_spec.rb index da533606be5..4c131bfe452 100644 --- a/spec/requests/user_sends_malformed_strings_spec.rb +++ b/spec/requests/user_sends_malformed_strings_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'User sends malformed strings' do +RSpec.describe 'User sends malformed strings', feature_category: :user_management do include GitHttpHelpers let(:null_byte) { "\u0000" } diff --git a/spec/requests/user_spoofs_ip_spec.rb b/spec/requests/user_spoofs_ip_spec.rb index 833dae78529..0244d60bc3b 100644 --- a/spec/requests/user_spoofs_ip_spec.rb +++ b/spec/requests/user_spoofs_ip_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'User spoofs their IP' do +RSpec.describe 'User spoofs their IP', feature_category: :user_management do it 'raises a 400 error' do get '/nonexistent', headers: { 'Client-Ip' => '1.2.3.4', 'X-Forwarded-For' => '5.6.7.8' } diff --git a/spec/requests/users/group_callouts_spec.rb b/spec/requests/users/group_callouts_spec.rb index a8680c3add4..d186bf92bc7 100644 --- a/spec/requests/users/group_callouts_spec.rb +++ b/spec/requests/users/group_callouts_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Group callouts' do +RSpec.describe 'Group callouts', feature_category: :navigation do let_it_be(:user) { create(:user) } let_it_be(:group) { create(:group) } diff --git a/spec/requests/users/project_callouts_spec.rb b/spec/requests/users/project_callouts_spec.rb index 98c00fef052..a15dd225e84 100644 --- a/spec/requests/users/project_callouts_spec.rb +++ b/spec/requests/users/project_callouts_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Project callouts' do +RSpec.describe 'Project callouts', feature_category: :navigation do let_it_be(:user) { create(:user) } let_it_be(:project) { create(:project) } diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index e78d4cc326e..608284c05f3 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe UsersController do +RSpec.describe UsersController, feature_category: :user_management do # This user should have the same e-mail address associated with the GPG key prepared for tests let(:user) { create(:user, email: GpgHelpers::User1.emails[0]) } let(:private_user) { create(:user, private_profile: true) } diff --git a/spec/requests/verifies_with_email_spec.rb b/spec/requests/verifies_with_email_spec.rb index 34fda1cce4d..b8a11c04a16 100644 --- a/spec/requests/verifies_with_email_spec.rb +++ b/spec/requests/verifies_with_email_spec.rb @@ -2,7 +2,8 @@ require 'spec_helper' -RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_redis_rate_limiting do +RSpec.describe 'VerifiesWithEmail', :clean_gitlab_redis_sessions, :clean_gitlab_redis_rate_limiting, +feature_category: :user_management do include SessionHelpers include EmailHelpers diff --git a/spec/requests/whats_new_controller_spec.rb b/spec/requests/whats_new_controller_spec.rb index d4976a2bba3..1e8cb514945 100644 --- a/spec/requests/whats_new_controller_spec.rb +++ b/spec/requests/whats_new_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe WhatsNewController, :clean_gitlab_redis_cache do +RSpec.describe WhatsNewController, :clean_gitlab_redis_cache, feature_category: :navigation do after do ReleaseHighlight.instance_variable_set(:@file_paths, nil) end diff --git a/spec/services/timelogs/create_service_spec.rb b/spec/services/timelogs/create_service_spec.rb index b5ed4a005c7..73860619bcc 100644 --- a/spec/services/timelogs/create_service_spec.rb +++ b/spec/services/timelogs/create_service_spec.rb @@ -2,16 +2,12 @@ require 'spec_helper' -RSpec.describe Timelogs::CreateService do +RSpec.describe Timelogs::CreateService, feature_category: :team_planning do let_it_be(:author) { create(:user) } let_it_be(:project) { create(:project, :public) } - let_it_be(:time_spent) { 3600 } - let_it_be(:spent_at) { "2022-07-08" } - let_it_be(:summary) { "Test summary" } let(:issuable) { nil } let(:users_container) { project } - let(:service) { described_class.new(issuable, time_spent, spent_at, summary, user) } describe '#execute' do subject { service.execute } diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 774a6ddcfb3..56440d7b5f5 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -1250,6 +1250,83 @@ RSpec.describe TodoService do end end + describe '#create_member_access_request' do + context 'when the group has more than 10 owners' do + it 'creates todos for 10 recently active group owners' do + group = create(:group, :public) + + users = create_list(:user, 12, :with_sign_ins) + users.each do |user| + group.add_owner(user) + end + ten_most_recently_active_group_owners = users.sort_by(&:last_sign_in_at).last(10) + excluded_group_owners = users - ten_most_recently_active_group_owners + + requester = create(:group_member, group: group, user: assignee) + + service.create_member_access_request(requester) + + ten_most_recently_active_group_owners.each do |owner| + expect(Todo.where(user: owner, target: group, action: Todo::MEMBER_ACCESS_REQUESTED, author: assignee).count).to eq 1 + end + + excluded_group_owners.each do |owner| + expect(Todo.where(user: owner, target: group, action: Todo::MEMBER_ACCESS_REQUESTED, author: assignee).count).to eq 0 + end + end + end + + context 'when total owners are less than 10' do + it 'creates todos for all group owners' do + group = create(:group, :public) + + users = create_list(:user, 4, :with_sign_ins) + users.map do |user| + group.add_owner(user) + end + + requester = create(:group_member, user: assignee, group: group) + requester.requested_at = Time.now.utc + requester.save! + + service.create_member_access_request(requester) + + users.each do |owner| + expect(Todo.where(user: owner, target: group, action: Todo::MEMBER_ACCESS_REQUESTED, author: assignee).count).to eq 1 + end + end + end + + context 'when multiple access requests are raised' do + it 'creates todos for 10 recently active group owners for multiple requests' do + group = create(:group, :public) + + users = create_list(:user, 12, :with_sign_ins) + users.each do |user| + group.add_owner(user) + end + ten_most_recently_active_group_owners = users.sort_by(&:last_sign_in_at).last(10) + excluded_group_owners = users - ten_most_recently_active_group_owners + + requester1 = create(:group_member, group: group, user: assignee) + requester2 = create(:group_member, group: group, user: non_member) + + service.create_member_access_request(requester1) + service.create_member_access_request(requester2) + + ten_most_recently_active_group_owners.each do |owner| + expect(Todo.where(user: owner, target: group, action: Todo::MEMBER_ACCESS_REQUESTED, author: assignee).count).to eq 1 + expect(Todo.where(user: owner, target: group, action: Todo::MEMBER_ACCESS_REQUESTED, author: non_member).count).to eq 1 + end + + excluded_group_owners.each do |owner| + expect(Todo.where(user: owner, target: group, action: Todo::MEMBER_ACCESS_REQUESTED, author: assignee).count).to eq 0 + expect(Todo.where(user: owner, target: group, action: Todo::MEMBER_ACCESS_REQUESTED, author: non_member).count).to eq 0 + end + end + end + end + def should_create_todo(attributes = {}) attributes.reverse_merge!( project: project, diff --git a/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb b/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb index 9e8478d7638..c6402a89f02 100644 --- a/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb +++ b/spec/support/shared_examples/graphql/mutations/timelogs/create_shared_examples.rb @@ -1,6 +1,17 @@ # frozen_string_literal: true RSpec.shared_examples 'issuable supports timelog creation mutation' do + let(:mutation_response) { graphql_mutation_response(:timelog_create) } + let(:mutation) do + variables = { + 'time_spent' => time_spent, + 'spent_at' => '2022-11-16T12:59:35+0100', + 'summary' => 'Test summary', + 'issuable_id' => issuable.to_global_id.to_s + } + graphql_mutation(:timelogCreate, variables) + end + context 'when the user is anonymous' do before do post_graphql_mutation(mutation, current_user: current_user) @@ -38,7 +49,8 @@ RSpec.shared_examples 'issuable supports timelog creation mutation' do expect(mutation_response['errors']).to be_empty expect(mutation_response['timelog']).to include( 'timeSpent' => 3600, - 'spentAt' => '2022-07-08T00:00:00Z', + # This also checks that the ISO time was converted to UTC + 'spentAt' => '2022-11-16T11:59:35Z', 'summary' => 'Test summary' ) end @@ -53,7 +65,8 @@ RSpec.shared_examples 'issuable supports timelog creation mutation' do end.to change { Timelog.count }.by(0) expect(response).to have_gitlab_http_status(:success) - expect(mutation_response['errors']).to match_array(['Time spent can\'t be blank']) + expect(mutation_response['errors']).to match_array( + ['Time spent must be formatted correctly. For example: 1h 30m.']) expect(mutation_response['timelog']).to be_nil end end @@ -61,6 +74,17 @@ RSpec.shared_examples 'issuable supports timelog creation mutation' do end RSpec.shared_examples 'issuable does not support timelog creation mutation' do + let(:mutation_response) { graphql_mutation_response(:timelog_create) } + let(:mutation) do + variables = { + 'time_spent' => time_spent, + 'spent_at' => '2022-11-16T12:59:35+0100', + 'summary' => 'Test summary', + 'issuable_id' => issuable.to_global_id.to_s + } + graphql_mutation(:timelogCreate, variables) + end + context 'when the user is anonymous' do before do post_graphql_mutation(mutation, current_user: current_user) diff --git a/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb b/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb index 18304951e41..56a1cee44c8 100644 --- a/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb +++ b/spec/support/shared_examples/quick_actions/issuable/time_tracking_quick_action_shared_examples.rb @@ -22,6 +22,12 @@ RSpec.shared_examples 'issuable time tracker' do |issuable_type| end end + def open_create_timelog_form + page.within time_tracker_selector do + find('[data-testid="add-time-entry-button"]').click + end + end + it 'renders the sidebar component empty state' do page.within '[data-testid="noTrackingPane"]' do expect(page).to have_content 'No estimate or time spent' @@ -74,11 +80,13 @@ RSpec.shared_examples 'issuable time tracker' do |issuable_type| end end - it 'shows the help state when icon is clicked' do - page.within time_tracker_selector do - find('[data-testid="helpButton"]').click - expect(page).to have_content 'Track time with quick actions' - expect(page).to have_content 'Learn more' + it 'shows the create timelog form when add button is clicked' do + open_create_timelog_form + + page.within '[data-testid="create-timelog-modal"]' do + expect(page).to have_content 'Add time entry' + expect(page).to have_content 'Time spent' + expect(page).to have_content 'Spent at' end end @@ -123,24 +131,6 @@ RSpec.shared_examples 'issuable time tracker' do |issuable_type| expect(page).to have_content '1d' end end - - it 'hides the help state when close icon is clicked' do - page.within time_tracker_selector do - find('[data-testid="helpButton"]').click - find('[data-testid="closeHelpButton"]').click - - expect(page).not_to have_content 'Track time with quick actions' - expect(page).not_to have_content 'Learn more' - end - end - - it 'displays the correct help url' do - page.within time_tracker_selector do - find('[data-testid="helpButton"]').click - - expect(find_link('Learn more')[:href]).to have_content('/help/user/project/time_tracking.md') - end - end end def submit_time(quick_action) diff --git a/spec/support/shared_examples/services/timelogs/create_service_shared_examples.rb b/spec/support/shared_examples/services/timelogs/create_service_shared_examples.rb index 53c42ec0e00..00d4224f021 100644 --- a/spec/support/shared_examples/services/timelogs/create_service_shared_examples.rb +++ b/spec/support/shared_examples/services/timelogs/create_service_shared_examples.rb @@ -1,6 +1,12 @@ # frozen_string_literal: true RSpec.shared_examples 'issuable supports timelog creation service' do + let_it_be(:time_spent) { 3600 } + let_it_be(:spent_at) { Time.now } + let_it_be(:summary) { "Test summary" } + + let(:service) { described_class.new(issuable, time_spent, spent_at, summary, user) } + shared_examples 'success_response' do it 'sucessfully saves the timelog' do is_expected.to be_success @@ -9,7 +15,7 @@ RSpec.shared_examples 'issuable supports timelog creation service' do expect(timelog).to be_persisted expect(timelog.time_spent).to eq(time_spent) - expect(timelog.spent_at).to eq('Fri, 08 Jul 2022 00:00:00.000000000 UTC +00:00') + expect(timelog.spent_at).to eq(spent_at) expect(timelog.summary).to eq(summary) expect(timelog.issuable).to eq(issuable) end @@ -34,6 +40,39 @@ RSpec.shared_examples 'issuable supports timelog creation service' do users_container.add_reporter(user) end + context 'when spent_at is in the future' do + let_it_be(:spent_at) { Time.now + 2.hours } + + it 'returns an error' do + is_expected.to be_error + + expect(subject.message).to eq("Spent at can't be a future date and time.") + expect(subject.http_status).to eq(404) + end + end + + context 'when time_spent is zero' do + let_it_be(:time_spent) { 0 } + + it 'returns an error' do + is_expected.to be_error + + expect(subject.message).to eq("Time spent can't be zero.") + expect(subject.http_status).to eq(404) + end + end + + context 'when time_spent is nil' do + let_it_be(:time_spent) { nil } + + it 'returns an error' do + is_expected.to be_error + + expect(subject.message).to eq("Time spent can't be blank") + expect(subject.http_status).to eq(404) + end + end + context 'when the timelog save fails' do before do allow_next_instance_of(Timelog) do |timelog| @@ -54,6 +93,12 @@ RSpec.shared_examples 'issuable supports timelog creation service' do end RSpec.shared_examples 'issuable does not support timelog creation service' do + let_it_be(:time_spent) { 3600 } + let_it_be(:spent_at) { Time.now } + let_it_be(:summary) { "Test summary" } + + let(:service) { described_class.new(issuable, time_spent, spent_at, summary, user) } + shared_examples 'error_response' do it 'returns an error' do is_expected.to be_error diff --git a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb index ece0c5053cb..02190201986 100644 --- a/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb +++ b/spec/workers/concerns/gitlab/github_import/object_importer_spec.rb @@ -194,6 +194,43 @@ RSpec.describe Gitlab::GithubImport::ObjectImporter, :aggregate_failures do .to raise_error(NoMethodError, /^undefined method `github_identifiers/) end end + + context 'when the record is invalid' do + it 'logs an error' do + expect(Gitlab::GithubImport::Logger) + .to receive(:info) + .with( + { + github_identifiers: github_identifiers, + message: 'starting importer', + project_id: project.id, + importer: 'klass_name' + } + ) + + expect(importer_class) + .to receive(:new) + .with(instance_of(MockRepresantation), project, client) + .and_return(importer_instance) + + exception = ActiveRecord::RecordInvalid.new + expect(importer_instance) + .to receive(:execute) + .and_raise(exception) + + expect(Gitlab::Import::ImportFailureService) + .to receive(:track) + .with( + project_id: project.id, + exception: exception, + error_source: 'klass_name', + fail_import: false + ) + .and_call_original + + worker.import(project, client, { 'number' => 10, 'github_id' => 1 }) + end + end end describe '#increment_object_counter?' do |