diff options
Diffstat (limited to 'spec')
21 files changed, 621 insertions, 24 deletions
diff --git a/spec/controllers/root_controller_spec.rb b/spec/controllers/root_controller_spec.rb index 7622f82afa7..49841aa61d7 100644 --- a/spec/controllers/root_controller_spec.rb +++ b/spec/controllers/root_controller_spec.rb @@ -68,6 +68,18 @@ RSpec.describe RootController do end end + context 'who has customized their dashboard setting for followed user activities' do + before do + user.dashboard = 'followed_user_activity' + end + + it 'redirects to the activity list' do + get :index + + expect(response).to redirect_to activity_dashboard_path(filter: 'followed') + end + end + context 'who has customized their dashboard setting for groups' do before do user.dashboard = 'groups' diff --git a/spec/frontend/design_management/components/delete_button_spec.js b/spec/frontend/design_management/components/delete_button_spec.js index 8f7d8e0b214..f5a841d35b8 100644 --- a/spec/frontend/design_management/components/delete_button_spec.js +++ b/spec/frontend/design_management/components/delete_button_spec.js @@ -36,7 +36,7 @@ describe('Batch delete button component', () => { expect(findButton().attributes('disabled')).toBeTruthy(); }); - it('emits `deleteSelectedDesigns` event on modal ok click', () => { + it('emits `delete-selected-designs` event on modal ok click', () => { createComponent(); findButton().vm.$emit('click'); return wrapper.vm @@ -46,7 +46,7 @@ describe('Batch delete button component', () => { return wrapper.vm.$nextTick(); }) .then(() => { - expect(wrapper.emitted().deleteSelectedDesigns).toBeTruthy(); + expect(wrapper.emitted('delete-selected-designs')).toBeTruthy(); }); }); diff --git a/spec/frontend/design_management/components/toolbar/index_spec.js b/spec/frontend/design_management/components/toolbar/index_spec.js index 44c865d976d..009ffe57744 100644 --- a/spec/frontend/design_management/components/toolbar/index_spec.js +++ b/spec/frontend/design_management/components/toolbar/index_spec.js @@ -106,11 +106,11 @@ describe('Design management toolbar component', () => { }); }); - it('emits `delete` event on deleteButton `deleteSelectedDesigns` event', () => { + it('emits `delete` event on deleteButton `delete-selected-designs` event', () => { createComponent(); return wrapper.vm.$nextTick().then(() => { - wrapper.find(DeleteButton).vm.$emit('deleteSelectedDesigns'); + wrapper.find(DeleteButton).vm.$emit('delete-selected-designs'); expect(wrapper.emitted().delete).toBeTruthy(); }); }); diff --git a/spec/frontend/design_management/components/upload/__snapshots__/button_spec.js.snap b/spec/frontend/design_management/components/upload/__snapshots__/button_spec.js.snap index 2f857247303..904bb2022ca 100644 --- a/spec/frontend/design_management/components/upload/__snapshots__/button_spec.js.snap +++ b/spec/frontend/design_management/components/upload/__snapshots__/button_spec.js.snap @@ -19,7 +19,7 @@ exports[`Design management upload button component renders inverted upload desig <input accept="image/*" - class="hide" + class="gl-display-none" multiple="multiple" name="design_file" type="file" @@ -44,7 +44,7 @@ exports[`Design management upload button component renders upload design button <input accept="image/*" - class="hide" + class="gl-display-none" multiple="multiple" name="design_file" type="file" diff --git a/spec/frontend/design_management/pages/index_spec.js b/spec/frontend/design_management/pages/index_spec.js index 4f162ca8e7f..95cb1ac943c 100644 --- a/spec/frontend/design_management/pages/index_spec.js +++ b/spec/frontend/design_management/pages/index_spec.js @@ -97,7 +97,7 @@ describe('Design management index page', () => { let moveDesignHandler; const findDesignCheckboxes = () => wrapper.findAll('.design-checkbox'); - const findSelectAllButton = () => wrapper.find('.js-select-all'); + const findSelectAllButton = () => wrapper.find('[data-testid="select-all-designs-button"'); const findToolbar = () => wrapper.find('.qa-selector-toolbar'); const findDesignCollectionIsCopying = () => wrapper.find('[data-testid="design-collection-is-copying"'); @@ -542,7 +542,9 @@ describe('Design management index page', () => { await nextTick(); expect(findDeleteButton().exists()).toBe(true); expect(findSelectAllButton().text()).toBe('Deselect all'); - findDeleteButton().vm.$emit('deleteSelectedDesigns'); + + findDeleteButton().vm.$emit('delete-selected-designs'); + const [{ variables }] = mutate.mock.calls[0]; expect(variables.filenames).toStrictEqual([mockDesigns[0].filename, mockDesigns[1].filename]); }); diff --git a/spec/frontend/pipelines/pipeline_triggerer_spec.js b/spec/frontend/pipelines/pipeline_triggerer_spec.js index 467a97d95c7..ffb2721f159 100644 --- a/spec/frontend/pipelines/pipeline_triggerer_spec.js +++ b/spec/frontend/pipelines/pipeline_triggerer_spec.js @@ -35,8 +35,8 @@ describe('Pipelines Triggerer', () => { wrapper.destroy(); }); - it('should render a table cell', () => { - expect(wrapper.find('.table-section').exists()).toBe(true); + it('should render pipeline triggerer table cell', () => { + expect(wrapper.find('[data-testid="pipeline-triggerer"]').exists()).toBe(true); }); it('should pass triggerer information when triggerer is provided', () => { diff --git a/spec/frontend/pipelines/pipeline_url_spec.js b/spec/frontend/pipelines/pipeline_url_spec.js index 4997e9cf3ec..367c7f2b2f6 100644 --- a/spec/frontend/pipelines/pipeline_url_spec.js +++ b/spec/frontend/pipelines/pipeline_url_spec.js @@ -7,6 +7,7 @@ const projectPath = 'test/test'; describe('Pipeline Url Component', () => { let wrapper; + const findTableCell = () => wrapper.find('[data-testid="pipeline-url-table-cell"]'); const findPipelineUrlLink = () => wrapper.find('[data-testid="pipeline-url-link"]'); const findScheduledTag = () => wrapper.find('[data-testid="pipeline-url-scheduled"]'); const findLatestTag = () => wrapper.find('[data-testid="pipeline-url-latest"]'); @@ -43,10 +44,10 @@ describe('Pipeline Url Component', () => { wrapper = null; }); - it('should render a table cell', () => { + it('should render pipeline url table cell', () => { createComponent(); - expect(wrapper.attributes('class')).toContain('table-section'); + expect(findTableCell().exists()).toBe(true); }); it('should render a link the provided path and id', () => { diff --git a/spec/frontend/pipelines/pipelines_spec.js b/spec/frontend/pipelines/pipelines_spec.js index aeca210b8ce..2116d022603 100644 --- a/spec/frontend/pipelines/pipelines_spec.js +++ b/spec/frontend/pipelines/pipelines_spec.js @@ -1,4 +1,4 @@ -import { GlFilteredSearch, GlButton, GlLoadingIcon, GlPagination } from '@gitlab/ui'; +import { GlButton, GlFilteredSearch, GlLoadingIcon, GlPagination } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; import MockAdapter from 'axios-mock-adapter'; import { chunk } from 'lodash'; diff --git a/spec/frontend/pipelines/pipelines_table_spec.js b/spec/frontend/pipelines/pipelines_table_spec.js index 9541461097b..4f4d71e5dab 100644 --- a/spec/frontend/pipelines/pipelines_table_spec.js +++ b/spec/frontend/pipelines/pipelines_table_spec.js @@ -1,7 +1,13 @@ import { GlTable } from '@gitlab/ui'; import { mount } from '@vue/test-utils'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; +import PipelineOperations from '~/pipelines/components/pipelines_list/pipeline_operations.vue'; +import PipelineTriggerer from '~/pipelines/components/pipelines_list/pipeline_triggerer.vue'; +import PipelineUrl from '~/pipelines/components/pipelines_list/pipeline_url.vue'; +import PipelinesStatusBadge from '~/pipelines/components/pipelines_list/pipelines_status_badge.vue'; import PipelinesTable from '~/pipelines/components/pipelines_list/pipelines_table.vue'; +import PipelinesTimeago from '~/pipelines/components/pipelines_list/time_ago.vue'; +import CommitComponent from '~/vue_shared/components/commit.vue'; describe('Pipelines Table', () => { let pipeline; @@ -29,7 +35,22 @@ describe('Pipelines Table', () => { const findRows = () => wrapper.findAll('.commit.gl-responsive-table-row'); const findGlTable = () => wrapper.findComponent(GlTable); - const findLegacyTable = () => wrapper.findByTestId('ci-table'); + const findStatusBadge = () => wrapper.findComponent(PipelinesStatusBadge); + const findPipelineInfo = () => wrapper.findComponent(PipelineUrl); + const findTriggerer = () => wrapper.findComponent(PipelineTriggerer); + const findCommit = () => wrapper.findComponent(CommitComponent); + const findTimeAgo = () => wrapper.findComponent(PipelinesTimeago); + const findActions = () => wrapper.findComponent(PipelineOperations); + + const findLegacyTable = () => wrapper.findByTestId('legacy-ci-table'); + const findTableRows = () => wrapper.findAll('[data-testid="pipeline-table-row"]'); + const findStatusTh = () => wrapper.findByTestId('status-th'); + const findPipelineTh = () => wrapper.findByTestId('pipeline-th'); + const findTriggererTh = () => wrapper.findByTestId('triggerer-th'); + const findCommitTh = () => wrapper.findByTestId('commit-th'); + const findStagesTh = () => wrapper.findByTestId('stages-th'); + const findTimeAgoTh = () => wrapper.findByTestId('timeago-th'); + const findActionsTh = () => wrapper.findByTestId('actions-th'); preloadFixtures(jsonFixtureName); @@ -82,11 +103,82 @@ describe('Pipelines Table', () => { }); describe('table with feature flag on', () => { - it('displays new table', () => { - createComponent(defaultProps, true); + beforeEach(() => { + createComponent({ pipelines: [pipeline], viewType: 'root' }, true); + }); + it('displays new table', () => { expect(findGlTable().exists()).toBe(true); expect(findLegacyTable().exists()).toBe(false); }); + + it('should render table head with correct columns', () => { + expect(findStatusTh().text()).toBe('Status'); + expect(findPipelineTh().text()).toBe('Pipeline'); + expect(findTriggererTh().text()).toBe('Triggerer'); + expect(findCommitTh().text()).toBe('Commit'); + expect(findStagesTh().text()).toBe('Stages'); + expect(findTimeAgoTh().text()).toBe('Duration'); + + // last column should have no text in th + expect(findActionsTh().text()).toBe(''); + }); + + it('should display a table row', () => { + expect(findTableRows()).toHaveLength(1); + }); + + describe('status cell', () => { + it('should render a status badge', () => { + expect(findStatusBadge().exists()).toBe(true); + }); + + it('should render status badge with correct path', () => { + expect(findStatusBadge().attributes('href')).toBe(pipeline.path); + }); + }); + + describe('pipeline cell', () => { + it('should render pipeline information', () => { + expect(findPipelineInfo().exists()).toBe(true); + }); + + it('should display the pipeline id', () => { + expect(findPipelineInfo().text()).toContain(`#${pipeline.id}`); + }); + }); + + describe('triggerer cell', () => { + it('should render the pipeline triggerer', () => { + expect(findTriggerer().exists()).toBe(true); + }); + }); + + describe('commit cell', () => { + it('should render commit information', () => { + expect(findCommit().exists()).toBe(true); + }); + + it('should display and link to commit', () => { + expect(findCommit().text()).toContain(pipeline.commit.short_id); + expect(findCommit().props('commitUrl')).toBe(pipeline.commit.commit_path); + }); + + it('should display the commit author', () => { + expect(findCommit().props('author')).toEqual(pipeline.commit.author); + }); + }); + + describe('duration cell', () => { + it('should render duration information', () => { + expect(findTimeAgo().exists()).toBe(true); + }); + }); + + describe('operations cell', () => { + it('should render pipeline operations', () => { + expect(findActions().exists()).toBe(true); + }); + }); }); }); diff --git a/spec/frontend/ref/components/ref_selector_spec.js b/spec/frontend/ref/components/ref_selector_spec.js index 27ada131ed6..cce365b1949 100644 --- a/spec/frontend/ref/components/ref_selector_spec.js +++ b/spec/frontend/ref/components/ref_selector_spec.js @@ -7,7 +7,13 @@ import { trimText } from 'helpers/text_helper'; import { ENTER_KEY } from '~/lib/utils/keys'; import { sprintf } from '~/locale'; import RefSelector from '~/ref/components/ref_selector.vue'; -import { X_TOTAL_HEADER, DEFAULT_I18N } from '~/ref/constants'; +import { + X_TOTAL_HEADER, + DEFAULT_I18N, + REF_TYPE_BRANCHES, + REF_TYPE_TAGS, + REF_TYPE_COMMITS, +} from '~/ref/constants'; import createStore from '~/ref/stores/'; const localVue = createLocalVue(); @@ -26,6 +32,7 @@ describe('Ref selector component', () => { let branchesApiCallSpy; let tagsApiCallSpy; let commitApiCallSpy; + let requestSpies; const createComponent = (props = {}, attrs = {}) => { wrapper = mount(RefSelector, { @@ -58,6 +65,7 @@ describe('Ref selector component', () => { .mockReturnValue([200, fixtures.branches, { [X_TOTAL_HEADER]: '123' }]); tagsApiCallSpy = jest.fn().mockReturnValue([200, fixtures.tags, { [X_TOTAL_HEADER]: '456' }]); commitApiCallSpy = jest.fn().mockReturnValue([200, fixtures.commit]); + requestSpies = { branchesApiCallSpy, tagsApiCallSpy, commitApiCallSpy }; mock .onGet(`/api/v4/projects/${projectId}/repository/branches`) @@ -592,4 +600,86 @@ describe('Ref selector component', () => { }); }); }); + + describe('with non-default ref types', () => { + it.each` + enabledRefTypes | reqsCalled | reqsNotCalled + ${[REF_TYPE_BRANCHES]} | ${['branchesApiCallSpy']} | ${['tagsApiCallSpy', 'commitApiCallSpy']} + ${[REF_TYPE_TAGS]} | ${['tagsApiCallSpy']} | ${['branchesApiCallSpy', 'commitApiCallSpy']} + ${[REF_TYPE_COMMITS]} | ${[]} | ${['branchesApiCallSpy', 'tagsApiCallSpy', 'commitApiCallSpy']} + ${[REF_TYPE_TAGS, REF_TYPE_COMMITS]} | ${['tagsApiCallSpy']} | ${['branchesApiCallSpy', 'commitApiCallSpy']} + `( + 'only calls $reqsCalled requests when $enabledRefTypes are enabled', + async ({ enabledRefTypes, reqsCalled, reqsNotCalled }) => { + createComponent({ enabledRefTypes }); + + await waitForRequests(); + + reqsCalled.forEach((req) => expect(requestSpies[req]).toHaveBeenCalledTimes(1)); + reqsNotCalled.forEach((req) => expect(requestSpies[req]).not.toHaveBeenCalled()); + }, + ); + + it('only calls commitApiCallSpy when REF_TYPE_COMMITS is enabled', async () => { + createComponent({ enabledRefTypes: [REF_TYPE_COMMITS] }); + updateQuery('abcd1234'); + + await waitForRequests(); + + expect(commitApiCallSpy).toHaveBeenCalledTimes(1); + expect(branchesApiCallSpy).not.toHaveBeenCalled(); + expect(tagsApiCallSpy).not.toHaveBeenCalled(); + }); + + it('triggers another search if enabled ref types change', async () => { + createComponent({ enabledRefTypes: [REF_TYPE_BRANCHES] }); + await waitForRequests(); + + expect(branchesApiCallSpy).toHaveBeenCalledTimes(1); + expect(tagsApiCallSpy).not.toHaveBeenCalled(); + + wrapper.setProps({ + enabledRefTypes: [REF_TYPE_BRANCHES, REF_TYPE_TAGS], + }); + await waitForRequests(); + + expect(branchesApiCallSpy).toHaveBeenCalledTimes(2); + expect(tagsApiCallSpy).toHaveBeenCalledTimes(1); + }); + + it('if a ref type becomes disabled, its section is hidden, even if it had some results in store', async () => { + createComponent({ enabledRefTypes: [REF_TYPE_BRANCHES, REF_TYPE_COMMITS] }); + updateQuery('abcd1234'); + await waitForRequests(); + + expect(findBranchesSection().exists()).toBe(true); + expect(findCommitsSection().exists()).toBe(true); + + wrapper.setProps({ enabledRefTypes: [REF_TYPE_COMMITS] }); + await waitForRequests(); + + expect(findBranchesSection().exists()).toBe(false); + expect(findCommitsSection().exists()).toBe(true); + }); + + it.each` + enabledRefType | findVisibleSection | findHiddenSections + ${REF_TYPE_BRANCHES} | ${findBranchesSection} | ${[findTagsSection, findCommitsSection]} + ${REF_TYPE_TAGS} | ${findTagsSection} | ${[findBranchesSection, findCommitsSection]} + ${REF_TYPE_COMMITS} | ${findCommitsSection} | ${[findBranchesSection, findTagsSection]} + `( + 'hides section headers if a single ref type is enabled', + async ({ enabledRefType, findVisibleSection, findHiddenSections }) => { + createComponent({ enabledRefTypes: [enabledRefType] }); + updateQuery('abcd1234'); + await waitForRequests(); + + expect(findVisibleSection().exists()).toBe(true); + expect(findVisibleSection().find('[data-testid="section-header"]').exists()).toBe(false); + findHiddenSections.forEach((findHiddenSection) => + expect(findHiddenSection().exists()).toBe(false), + ); + }, + ); + }); }); diff --git a/spec/frontend/ref/stores/actions_spec.js b/spec/frontend/ref/stores/actions_spec.js index 11acec27165..099ce062a3a 100644 --- a/spec/frontend/ref/stores/actions_spec.js +++ b/spec/frontend/ref/stores/actions_spec.js @@ -1,4 +1,5 @@ import testAction from 'helpers/vuex_action_helper'; +import { ALL_REF_TYPES, REF_TYPE_BRANCHES, REF_TYPE_TAGS, REF_TYPE_COMMITS } from '~/ref/constants'; import * as actions from '~/ref/stores/actions'; import * as types from '~/ref/stores/mutation_types'; import createState from '~/ref/stores/state'; @@ -25,6 +26,14 @@ describe('Ref selector Vuex store actions', () => { state = createState(); }); + describe('setEnabledRefTypes', () => { + it(`commits ${types.SET_ENABLED_REF_TYPES} with the enabled ref types`, () => { + testAction(actions.setProjectId, ALL_REF_TYPES, state, [ + { type: types.SET_PROJECT_ID, payload: ALL_REF_TYPES }, + ]); + }); + }); + describe('setProjectId', () => { it(`commits ${types.SET_PROJECT_ID} with the new project ID`, () => { const projectId = '4'; @@ -46,12 +55,23 @@ describe('Ref selector Vuex store actions', () => { describe('search', () => { it(`commits ${types.SET_QUERY} with the new search query`, () => { const query = 'hello'; + testAction(actions.search, query, state, [{ type: types.SET_QUERY, payload: query }]); + }); + + it.each` + enabledRefTypes | expectedActions + ${[REF_TYPE_BRANCHES]} | ${['searchBranches']} + ${[REF_TYPE_COMMITS]} | ${['searchCommits']} + ${[REF_TYPE_BRANCHES, REF_TYPE_TAGS, REF_TYPE_COMMITS]} | ${['searchBranches', 'searchTags', 'searchCommits']} + `(`dispatches fetch actions for enabled ref types`, ({ enabledRefTypes, expectedActions }) => { + const query = 'hello'; + state.enabledRefTypes = enabledRefTypes; testAction( actions.search, query, state, [{ type: types.SET_QUERY, payload: query }], - [{ type: 'searchBranches' }, { type: 'searchTags' }, { type: 'searchCommits' }], + expectedActions.map((type) => ({ type })), ); }); }); diff --git a/spec/frontend/ref/stores/mutations_spec.js b/spec/frontend/ref/stores/mutations_spec.js index cda13089766..11d4fe0e206 100644 --- a/spec/frontend/ref/stores/mutations_spec.js +++ b/spec/frontend/ref/stores/mutations_spec.js @@ -1,4 +1,4 @@ -import { X_TOTAL_HEADER } from '~/ref/constants'; +import { X_TOTAL_HEADER, ALL_REF_TYPES } from '~/ref/constants'; import * as types from '~/ref/stores/mutation_types'; import mutations from '~/ref/stores/mutations'; import createState from '~/ref/stores/state'; @@ -13,6 +13,7 @@ describe('Ref selector Vuex store mutations', () => { describe('initial state', () => { it('is created with the correct structure and initial values', () => { expect(state).toEqual({ + enabledRefTypes: [], projectId: null, query: '', @@ -39,6 +40,14 @@ describe('Ref selector Vuex store mutations', () => { }); }); + describe(`${types.SET_ENABLED_REF_TYPES}`, () => { + it('sets the enabled ref types', () => { + mutations[types.SET_ENABLED_REF_TYPES](state, ALL_REF_TYPES); + + expect(state.enabledRefTypes).toBe(ALL_REF_TYPES); + }); + }); + describe(`${types.SET_PROJECT_ID}`, () => { it('updates the project ID', () => { const newProjectId = '4'; diff --git a/spec/helpers/preferences_helper_spec.rb b/spec/helpers/preferences_helper_spec.rb index be0ad5e1a3f..e5420fb6729 100644 --- a/spec/helpers/preferences_helper_spec.rb +++ b/spec/helpers/preferences_helper_spec.rb @@ -29,6 +29,7 @@ RSpec.describe PreferencesHelper do ['Starred Projects', 'stars'], ["Your Projects' Activity", 'project_activity'], ["Starred Projects' Activity", 'starred_project_activity'], + ["Followed Users' Activity", 'followed_user_activity'], ["Your Groups", 'groups'], ["Your To-Do List", 'todos'], ["Assigned Issues", 'issues'], diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb index 23d52c8f106..3bbf348a330 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb @@ -1025,4 +1025,75 @@ RSpec.describe Gitlab::Ci::Pipeline::Seed::Build do end end end + + describe 'applying pipeline variables' do + subject { seed_build } + + let(:pipeline_variables) { [] } + let(:pipeline) do + build(:ci_empty_pipeline, project: project, sha: head_sha, variables: pipeline_variables) + end + + context 'containing variable references' do + let(:pipeline_variables) do + [ + build(:ci_pipeline_variable, key: 'A', value: '$B'), + build(:ci_pipeline_variable, key: 'B', value: '$C') + ] + end + + context 'when FF :variable_inside_variable is enabled' do + before do + stub_feature_flags(variable_inside_variable: [project]) + end + + it "does not have errors" do + expect(subject.errors).to be_empty + end + end + end + + context 'containing cyclic reference' do + let(:pipeline_variables) do + [ + build(:ci_pipeline_variable, key: 'A', value: '$B'), + build(:ci_pipeline_variable, key: 'B', value: '$C'), + build(:ci_pipeline_variable, key: 'C', value: '$A') + ] + end + + context 'when FF :variable_inside_variable is disabled' do + before do + stub_feature_flags(variable_inside_variable: false) + end + + it "does not have errors" do + expect(subject.errors).to be_empty + end + end + + context 'when FF :variable_inside_variable is enabled' do + before do + stub_feature_flags(variable_inside_variable: [project]) + end + + it "returns an error" do + expect(subject.errors).to contain_exactly( + 'rspec: circular variable reference detected: ["A", "B", "C"]') + end + + context 'with job:rules:[if:]' do + let(:attributes) { { name: 'rspec', ref: 'master', rules: [{ if: '$C != null', when: 'always' }] } } + + it "included? does not raise" do + expect { subject.included? }.not_to raise_error + end + + it "included? returns true" do + expect(subject.included?).to eq(true) + end + end + end + end + end end diff --git a/spec/lib/gitlab/cleanup/redis/batch_delete_by_pattern_spec.rb b/spec/lib/gitlab/cleanup/redis/batch_delete_by_pattern_spec.rb new file mode 100644 index 00000000000..6fd95a52eda --- /dev/null +++ b/spec/lib/gitlab/cleanup/redis/batch_delete_by_pattern_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Cleanup::Redis::BatchDeleteByPattern, :clean_gitlab_redis_cache do + subject { described_class.new(patterns) } + + describe 'execute' do + context 'when no patterns are passed' do + before do + expect(Gitlab::Redis::Cache).not_to receive(:with) + end + + context 'with nil patterns' do + let(:patterns) { nil } + + specify { expect { subject }.to raise_error(ArgumentError, 'Argument should be an Array of patterns') } + end + + context 'with empty array patterns' do + let(:patterns) { [] } + + specify { subject.execute } + end + end + + context 'with patterns' do + context 'when key is not found' do + let(:patterns) { ['key'] } + + before do + expect_any_instance_of(Redis).not_to receive(:del) # rubocop:disable RSpec/AnyInstanceOf + end + + specify { subject.execute } + end + + context 'with cache data' do + let(:cache_keys) { %w[key-test1 key-test2 key-test3 key-test4] } + + before do + stub_const("#{described_class}::REDIS_CLEAR_BATCH_SIZE", 2) + + write_to_cache + end + + context 'with one key' do + let(:patterns) { ['key-test1'] } + + it 'deletes the key' do + expect_any_instance_of(Redis).to receive(:del).with(patterns.first).once # rubocop:disable RSpec/AnyInstanceOf + + subject.execute + end + end + + context 'with many keys' do + let(:patterns) { %w[key-test1 key-test2] } + + it 'deletes keys for each pattern separatelly' do + expect_any_instance_of(Redis).to receive(:del).with(patterns.first).once # rubocop:disable RSpec/AnyInstanceOf + expect_any_instance_of(Redis).to receive(:del).with(patterns.last).once # rubocop:disable RSpec/AnyInstanceOf + + subject.execute + end + end + + context 'with cache_keys over batch size' do + let(:patterns) { %w[key-test*] } + + it 'deletes matched keys in batches' do + # redis scan returns the values in random order so just checking it is being called twice meaning + # scan returned results in 2 batches, which is what we expect + key_like = start_with('key-test') + expect_any_instance_of(Redis).to receive(:del).with(key_like, key_like).twice # rubocop:disable RSpec/AnyInstanceOf + + subject.execute + end + end + end + end + end +end + +def write_to_cache + Gitlab::Redis::Cache.with do |redis| + cache_keys.each_with_index do |cache_key, index| + redis.set(cache_key, index) + end + end +end diff --git a/spec/lib/gitlab/cleanup/redis/description_templates_cache_keys_pattern_builder_spec.rb b/spec/lib/gitlab/cleanup/redis/description_templates_cache_keys_pattern_builder_spec.rb new file mode 100644 index 00000000000..4d3fd1f3062 --- /dev/null +++ b/spec/lib/gitlab/cleanup/redis/description_templates_cache_keys_pattern_builder_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Cleanup::Redis::DescriptionTemplatesCacheKeysPatternBuilder, :clean_gitlab_redis_cache do + subject { described_class.new(project_ids).execute } + + describe 'execute' do + context 'when build pattern for all description templates' do + RSpec.shared_examples 'all issue and merge request templates pattern' do + it 'builds pattern to remove all issue and merge request templates keys' do + expect(subject.count).to eq(2) + expect(subject).to match_array(%W[ + #{Gitlab::Redis::Cache::CACHE_NAMESPACE}:issue_template_names_hash:* + #{Gitlab::Redis::Cache::CACHE_NAMESPACE}:merge_request_template_names_hash:* + ]) + end + end + + context 'with project_ids == :all' do + let(:project_ids) { :all } + + it_behaves_like 'all issue and merge request templates pattern' + end + end + + context 'with project_ids' do + let_it_be(:project1) { create(:project, :repository) } + let_it_be(:project2) { create(:project, :repository) } + + context 'with nil project_ids' do + let(:project_ids) { nil } + + specify { expect { subject }.to raise_error(ArgumentError, 'project_ids can either be an array of project IDs or :all') } + end + + context 'with project_ids as string' do + let(:project_ids) { '1' } + + specify { expect { subject }.to raise_error(ArgumentError, 'project_ids can either be an array of project IDs or :all') } + end + + context 'with invalid project_ids as array of strings' do + let(:project_ids) { %w[a b] } + + specify { expect { subject }.to raise_error(ArgumentError, 'Invalid Project ID. Please ensure all passed in project ids values are valid integer project ids.') } + end + + context 'with non existent project id' do + let(:project_ids) { [non_existing_record_id] } + + it 'no patterns are built' do + expect(subject.count).to eq(0) + end + end + + context 'with one project_id' do + let(:project_ids) { [project1.id] } + + it 'builds patterns for the project' do + expect(subject.count).to eq(2) + expect(subject).to match_array(%W[ + #{Gitlab::Redis::Cache::CACHE_NAMESPACE}:issue_template_names_hash:#{project1.full_path}:#{project1.id} + #{Gitlab::Redis::Cache::CACHE_NAMESPACE}:merge_request_template_names_hash:#{project1.full_path}:#{project1.id} + ]) + end + end + + context 'with many project_ids' do + let(:project_ids) { [project1.id, project2.id] } + + RSpec.shared_examples 'builds patterns for the given projects' do + it 'builds patterns for the given projects' do + expect(subject.count).to eq(4) + expect(subject).to match_array(%W[ + #{Gitlab::Redis::Cache::CACHE_NAMESPACE}:issue_template_names_hash:#{project1.full_path}:#{project1.id} + #{Gitlab::Redis::Cache::CACHE_NAMESPACE}:merge_request_template_names_hash:#{project1.full_path}:#{project1.id} + #{Gitlab::Redis::Cache::CACHE_NAMESPACE}:issue_template_names_hash:#{project2.full_path}:#{project2.id} + #{Gitlab::Redis::Cache::CACHE_NAMESPACE}:merge_request_template_names_hash:#{project2.full_path}:#{project2.id} + ]) + end + end + + it_behaves_like 'builds patterns for the given projects' + + context 'with project_ids as string' do + let(:project_ids) { [project1.id.to_s, project2.id.to_s] } + + it_behaves_like 'builds patterns for the given projects' + end + end + end + end +end diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index def3f9f50f8..6367b4bd55f 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -797,6 +797,50 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end + describe 'setting the application context' do + subject { request_job } + + context 'when triggered by a user' do + let(:job) { create(:ci_build, user: user, project: project) } + + subject { request_job(id: job.id) } + + it_behaves_like 'storing arguments in the application context' do + let(:expected_params) { { user: user.username, project: project.full_path } } + end + + it_behaves_like 'not executing any extra queries for the application context', 3 do + # Extra queries: User, Project, Route + let(:subject_proc) { proc { request_job(id: job.id) } } + end + end + + context 'when the runner is of project type' do + it_behaves_like 'storing arguments in the application context' do + let(:expected_params) { { project: project.full_path } } + end + + it_behaves_like 'not executing any extra queries for the application context', 2 do + # Extra queries: Project, Route + let(:subject_proc) { proc { request_job } } + end + end + + context 'when the runner is of group type' do + let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } + + it_behaves_like 'storing arguments in the application context' do + let(:expected_params) { { root_namespace: group.full_path_components.first } } + end + + it_behaves_like 'not executing any extra queries for the application context', 2 do + # Extra queries: Group, Route + let(:subject_proc) { proc { request_job } } + end + end + end + def request_job(token = runner.token, **params) new_params = params.merge(token: token, last_update: last_update) post api('/jobs/request'), params: new_params.to_json, headers: { 'User-Agent' => user_agent, 'Content-Type': 'application/json' } diff --git a/spec/requests/api/ci/runner/runners_post_spec.rb b/spec/requests/api/ci/runner/runners_post_spec.rb index 7c362fae7d2..67163b9329f 100644 --- a/spec/requests/api/ci/runner/runners_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_post_spec.rb @@ -35,6 +35,10 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end context 'when valid token is provided' do + def request + post api('/runners'), params: { token: token } + end + it 'creates runner with default values' do post api('/runners'), params: { token: registration_token } @@ -51,9 +55,10 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do context 'when project token is used' do let(:project) { create(:project) } + let(:token) { project.runners_token } it 'creates project runner' do - post api('/runners'), params: { token: project.runners_token } + request expect(response).to have_gitlab_http_status(:created) expect(project.runners.size).to eq(1) @@ -62,13 +67,24 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(runner.token).not_to eq(project.runners_token) expect(runner).to be_project_type end + + it_behaves_like 'storing arguments in the application context' do + subject { request } + + let(:expected_params) { { project: project.full_path } } + end + + it_behaves_like 'not executing any extra queries for the application context' do + let(:subject_proc) { proc { request } } + end end context 'when group token is used' do let(:group) { create(:group) } + let(:token) { group.runners_token } it 'creates a group runner' do - post api('/runners'), params: { token: group.runners_token } + request expect(response).to have_gitlab_http_status(:created) expect(group.runners.reload.size).to eq(1) @@ -77,6 +93,16 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(runner.token).not_to eq(group.runners_token) expect(runner).to be_group_type end + + it_behaves_like 'storing arguments in the application context' do + subject { request } + + let(:expected_params) { { root_namespace: group.full_path_components.first } } + end + + it_behaves_like 'not executing any extra queries for the application context' do + let(:subject_proc) { proc { request } } + end end end diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index 2d71662b0eb..0b1d4debd03 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -172,8 +172,13 @@ module TestEnv Gitlab::SetupHelper::Gitaly.create_configuration(gitaly_dir, { 'default' => repos_path }, force: true) Gitlab::SetupHelper::Gitaly.create_configuration( gitaly_dir, - { 'default' => repos_path }, force: true, - options: { gitaly_socket: "gitaly2.socket", config_filename: "gitaly2.config.toml" } + { 'default' => repos_path }, + force: true, + options: { + internal_socket_dir: File.join(gitaly_dir, "internal_gitaly2"), + gitaly_socket: "gitaly2.socket", + config_filename: "gitaly2.config.toml" + } ) Gitlab::SetupHelper::Praefect.create_configuration(gitaly_dir, { 'praefect' => repos_path }, force: true) end diff --git a/spec/support/shared_examples/requests/api/logging_application_context_shared_examples.rb b/spec/support/shared_examples/requests/api/logging_application_context_shared_examples.rb index 038ede884c8..4a71b696d57 100644 --- a/spec/support/shared_examples/requests/api/logging_application_context_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/logging_application_context_shared_examples.rb @@ -22,3 +22,19 @@ RSpec.shared_examples 'storing arguments in the application context' do hash.transform_keys! { |key| "meta.#{key}" } end end + +RSpec.shared_examples 'not executing any extra queries for the application context' do |expected_extra_queries = 0| + it 'does not execute more queries than without adding anything to the application context' do + # Call the subject once to memoize all factories being used for the spec, so they won't + # add any queries to the expectation. + subject_proc.call + + expect do + allow(Gitlab::ApplicationContext).to receive(:push).and_call_original + subject_proc.call + end.to issue_same_number_of_queries_as { + allow(Gitlab::ApplicationContext).to receive(:push) + subject_proc.call + }.with_threshold(expected_extra_queries).ignoring_cached_queries + end +end diff --git a/spec/tasks/cache/clear/redis_spec.rb b/spec/tasks/cache/clear/redis_spec.rb index d2de068f254..2c0a5dcd54d 100644 --- a/spec/tasks/cache/clear/redis_spec.rb +++ b/spec/tasks/cache/clear/redis_spec.rb @@ -2,7 +2,7 @@ require 'rake_helper' -RSpec.describe 'clearing redis cache' do +RSpec.describe 'clearing redis cache', :clean_gitlab_redis_cache do before do Rake.application.rake_require 'tasks/cache' end @@ -21,4 +21,27 @@ RSpec.describe 'clearing redis cache' do expect { run_rake_task('cache:clear:redis') }.to change { pipeline_status.has_cache? } end end + + describe 'invoking clear description templates cache rake task' do + using RSpec::Parameterized::TableSyntax + + before do + stub_env('project_ids', project_ids) if project_ids + service = double(:service, execute: true) + + expect(Gitlab::Cleanup::Redis::DescriptionTemplatesCacheKeysPatternBuilder).to receive(:new).with(expected_project_ids).and_return(service) + expect(Gitlab::Cleanup::Redis::BatchDeleteByPattern).to receive(:new).and_return(service) + end + + where(:project_ids, :expected_project_ids) do + nil | [] # this acts as no argument is being passed + '1' | %w[1] + '1, 2, 3' | %w[1 2 3] + '1, 2, some-string, 3' | %w[1 2 some-string 3] + end + + with_them do + specify { run_rake_task('cache:clear:description_templates') } + end + end end |