diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-03 18:07:39 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2023-02-03 18:07:39 +0300 |
commit | 3cda3d43aef1e92e2eedf7383122c6db9c61149f (patch) | |
tree | 1b4dd068a449d050afafa403de54a00a63e9bfc4 /spec | |
parent | 83916cf0a2f9254455a08a723961db34f0840df4 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
24 files changed, 752 insertions, 383 deletions
diff --git a/spec/controllers/projects/refs_controller_spec.rb b/spec/controllers/projects/refs_controller_spec.rb index a7a8361ae20..a0d119baf16 100644 --- a/spec/controllers/projects/refs_controller_spec.rb +++ b/spec/controllers/projects/refs_controller_spec.rb @@ -22,65 +22,30 @@ RSpec.describe Projects::RefsController, feature_category: :source_code_manageme subject { get :switch, params: params } - context 'when the use_ref_type_parameter feature flag is not enabled' do - before do - stub_feature_flags(use_ref_type_parameter: false) - end - - where(:destination, :ref_type, :redirected_to) do - 'tree' | nil | lazy { project_tree_path(project, id) } - 'tree' | 'heads' | lazy { project_tree_path(project, id) } - 'blob' | nil | lazy { project_blob_path(project, id) } - 'blob' | 'heads' | lazy { project_blob_path(project, id) } - 'graph' | nil | lazy { project_network_path(project, id) } - 'graph' | 'heads' | lazy { project_network_path(project, id) } - 'graphs' | nil | lazy { project_graph_path(project, id) } - 'graphs' | 'heads' | lazy { project_graph_path(project, id) } - 'find_file' | nil | lazy { project_find_file_path(project, id) } - 'find_file' | 'heads' | lazy { project_find_file_path(project, id) } - 'graphs_commits' | nil | lazy { commits_project_graph_path(project, id) } - 'graphs_commits' | 'heads' | lazy { commits_project_graph_path(project, id) } - 'badges' | nil | lazy { project_settings_ci_cd_path(project, ref: id) } - 'badges' | 'heads' | lazy { project_settings_ci_cd_path(project, ref: id) } - 'commits' | nil | lazy { project_commits_path(project, id) } - 'commits' | 'heads' | lazy { project_commits_path(project, id) } - 'somethingelse' | nil | lazy { project_commits_path(project, id) } - 'somethingelse' | 'heads' | lazy { project_commits_path(project, id) } - end - - with_them do - it 'redirects to destination' do - expect(subject).to redirect_to(redirected_to) - end - end + where(:destination, :ref_type, :redirected_to) do + 'tree' | nil | lazy { project_tree_path(project, id) } + 'tree' | 'heads' | lazy { project_tree_path(project, id) } + 'blob' | nil | lazy { project_blob_path(project, id) } + 'blob' | 'heads' | lazy { project_blob_path(project, id) } + 'graph' | nil | lazy { project_network_path(project, id) } + 'graph' | 'heads' | lazy { project_network_path(project, id, ref_type: 'heads') } + 'graphs' | nil | lazy { project_graph_path(project, id) } + 'graphs' | 'heads' | lazy { project_graph_path(project, id, ref_type: 'heads') } + 'find_file' | nil | lazy { project_find_file_path(project, id) } + 'find_file' | 'heads' | lazy { project_find_file_path(project, id) } + 'graphs_commits' | nil | lazy { commits_project_graph_path(project, id) } + 'graphs_commits' | 'heads' | lazy { commits_project_graph_path(project, id) } + 'badges' | nil | lazy { project_settings_ci_cd_path(project, ref: id) } + 'badges' | 'heads' | lazy { project_settings_ci_cd_path(project, ref: id) } + 'commits' | nil | lazy { project_commits_path(project, id) } + 'commits' | 'heads' | lazy { project_commits_path(project, id, ref_type: 'heads') } + nil | nil | lazy { project_commits_path(project, id) } + nil | 'heads' | lazy { project_commits_path(project, id, ref_type: 'heads') } end - context 'when the use_ref_type_parameter feature flag is enabled' do - where(:destination, :ref_type, :redirected_to) do - 'tree' | nil | lazy { project_tree_path(project, id) } - 'tree' | 'heads' | lazy { project_tree_path(project, id) } - 'blob' | nil | lazy { project_blob_path(project, id) } - 'blob' | 'heads' | lazy { project_blob_path(project, id) } - 'graph' | nil | lazy { project_network_path(project, id) } - 'graph' | 'heads' | lazy { project_network_path(project, id, ref_type: 'heads') } - 'graphs' | nil | lazy { project_graph_path(project, id) } - 'graphs' | 'heads' | lazy { project_graph_path(project, id, ref_type: 'heads') } - 'find_file' | nil | lazy { project_find_file_path(project, id) } - 'find_file' | 'heads' | lazy { project_find_file_path(project, id) } - 'graphs_commits' | nil | lazy { commits_project_graph_path(project, id) } - 'graphs_commits' | 'heads' | lazy { commits_project_graph_path(project, id) } - 'badges' | nil | lazy { project_settings_ci_cd_path(project, ref: id) } - 'badges' | 'heads' | lazy { project_settings_ci_cd_path(project, ref: id) } - 'commits' | nil | lazy { project_commits_path(project, id) } - 'commits' | 'heads' | lazy { project_commits_path(project, id, ref_type: 'heads') } - nil | nil | lazy { project_commits_path(project, id) } - nil | 'heads' | lazy { project_commits_path(project, id, ref_type: 'heads') } - end - - with_them do - it 'redirects to destination' do - expect(subject).to redirect_to(redirected_to) - end + with_them do + it 'redirects to destination' do + expect(subject).to redirect_to(redirected_to) end end end diff --git a/spec/frontend/ide/init_gitlab_web_ide_spec.js b/spec/frontend/ide/init_gitlab_web_ide_spec.js index 97254ab680b..b5ad38addbd 100644 --- a/spec/frontend/ide/init_gitlab_web_ide_spec.js +++ b/spec/frontend/ide/init_gitlab_web_ide_spec.js @@ -32,6 +32,9 @@ const TEST_START_REMOTE_PARAMS = { remotePath: '/test/projects/f oo', connectionToken: '123abc', }; +const TEST_EDITOR_FONT_SRC_URL = 'http://gitlab.test/assets/jetbrains-mono/JetBrainsMono.woff2'; +const TEST_EDITOR_FONT_FORMAT = 'woff2'; +const TEST_EDITOR_FONT_FAMILY = 'JebBrains Mono'; describe('ide/init_gitlab_web_ide', () => { let resolveConfirm; @@ -49,6 +52,9 @@ describe('ide/init_gitlab_web_ide', () => { el.dataset.userPreferencesPath = TEST_USER_PREFERENCES_PATH; el.dataset.mergeRequest = TEST_MR_ID; el.dataset.filePath = TEST_FILE_PATH; + el.dataset.editorFontSrcUrl = TEST_EDITOR_FONT_SRC_URL; + el.dataset.editorFontFormat = TEST_EDITOR_FONT_FORMAT; + el.dataset.editorFontFamily = TEST_EDITOR_FONT_FAMILY; document.body.append(el); }; @@ -103,6 +109,11 @@ describe('ide/init_gitlab_web_ide', () => { userPreferences: TEST_USER_PREFERENCES_PATH, feedbackIssue: GITLAB_WEB_IDE_FEEDBACK_ISSUE, }, + editorFont: { + srcUrl: TEST_EDITOR_FONT_SRC_URL, + fontFamily: TEST_EDITOR_FONT_FAMILY, + format: TEST_EDITOR_FONT_FORMAT, + }, handleStartRemote: expect.any(Function), }); }); diff --git a/spec/frontend/token_access/opt_in_jwt_spec.js b/spec/frontend/token_access/opt_in_jwt_spec.js index a25a480f889..3a68f247aa6 100644 --- a/spec/frontend/token_access/opt_in_jwt_spec.js +++ b/spec/frontend/token_access/opt_in_jwt_spec.js @@ -1,10 +1,11 @@ -import { GlLoadingIcon, GlToggle } from '@gitlab/ui'; +import { GlLink, GlLoadingIcon, GlToggle, GlSprintf } from '@gitlab/ui'; import Vue from 'vue'; import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; import { mountExtended, shallowMountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import { createAlert } from '~/flash'; +import { OPT_IN_JWT_HELP_LINK } from '~/token_access/constants'; import OptInJwt from '~/token_access/components/opt_in_jwt.vue'; import getOptInJwtSettingQuery from '~/token_access/graphql/queries/get_opt_in_jwt_setting.query.graphql'; import updateOptInJwtMutation from '~/token_access/graphql/mutations/update_opt_in_jwt.mutation.graphql'; @@ -25,6 +26,7 @@ describe('OptInJwt component', () => { const disabledOptInJwtHandler = jest.fn().mockResolvedValue(optInJwtQueryResponse(false)); const updateOptInJwtHandler = jest.fn().mockResolvedValue(optInJwtMutationResponse(true)); + const findHelpLink = () => wrapper.findComponent(GlLink); const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findToggle = () => wrapper.findComponent(GlToggle); const findOptInJwtExpandedSection = () => wrapper.findByTestId('opt-in-jwt-expanded-section'); @@ -33,7 +35,7 @@ describe('OptInJwt component', () => { return createMockApollo(requestHandlers); }; - const createComponent = (requestHandlers, mountFn = shallowMountExtended) => { + const createComponent = (requestHandlers, mountFn = shallowMountExtended, options = {}) => { wrapper = mountFn(OptInJwt, { provide: { fullPath: 'root/my-repo', @@ -44,12 +46,18 @@ describe('OptInJwt component', () => { targetProjectPath: 'root/test', }; }, + ...options, }); }; + const createShallowComponent = (requestHandlers, options = {}) => + createComponent(requestHandlers, shallowMountExtended, options); + const createFullComponent = (requestHandlers, options = {}) => + createComponent(requestHandlers, mountExtended, options); + describe('loading state', () => { it('shows loading state and hides toggle while waiting on query to resolve', async () => { - createComponent([[getOptInJwtSettingQuery, enabledOptInJwtHandler]]); + createShallowComponent([[getOptInJwtSettingQuery, enabledOptInJwtHandler]]); expect(findLoadingIcon().exists()).toBe(true); expect(findToggle().exists()).toBe(false); @@ -61,9 +69,25 @@ describe('OptInJwt component', () => { }); }); + describe('template', () => { + it('renders help link', async () => { + createShallowComponent([[getOptInJwtSettingQuery, enabledOptInJwtHandler]], { + stubs: { + GlToggle, + GlSprintf, + GlLink, + }, + }); + await waitForPromises(); + + expect(findHelpLink().exists()).toBe(true); + expect(findHelpLink().attributes('href')).toBe(OPT_IN_JWT_HELP_LINK); + }); + }); + describe('toggle JWT token access', () => { it('code instruction is visible when toggle is enabled', async () => { - createComponent([[getOptInJwtSettingQuery, enabledOptInJwtHandler]]); + createShallowComponent([[getOptInJwtSettingQuery, enabledOptInJwtHandler]]); await waitForPromises(); @@ -72,7 +96,7 @@ describe('OptInJwt component', () => { }); it('code instruction is hidden when toggle is disabled', async () => { - createComponent([[getOptInJwtSettingQuery, disabledOptInJwtHandler]]); + createShallowComponent([[getOptInJwtSettingQuery, disabledOptInJwtHandler]]); await waitForPromises(); @@ -82,13 +106,10 @@ describe('OptInJwt component', () => { describe('update JWT token access', () => { it('calls updateOptInJwtMutation with correct arguments', async () => { - createComponent( - [ - [getOptInJwtSettingQuery, disabledOptInJwtHandler], - [updateOptInJwtMutation, updateOptInJwtHandler], - ], - mountExtended, - ); + createFullComponent([ + [getOptInJwtSettingQuery, disabledOptInJwtHandler], + [updateOptInJwtMutation, updateOptInJwtHandler], + ]); await waitForPromises(); @@ -103,13 +124,10 @@ describe('OptInJwt component', () => { }); it('handles update error', async () => { - createComponent( - [ - [getOptInJwtSettingQuery, enabledOptInJwtHandler], - [updateOptInJwtMutation, failureHandler], - ], - mountExtended, - ); + createFullComponent([ + [getOptInJwtSettingQuery, enabledOptInJwtHandler], + [updateOptInJwtMutation, failureHandler], + ]); await waitForPromises(); diff --git a/spec/frontend/work_items/components/work_item_created_updated_spec.js b/spec/frontend/work_items/components/work_item_created_updated_spec.js new file mode 100644 index 00000000000..fe31c01df36 --- /dev/null +++ b/spec/frontend/work_items/components/work_item_created_updated_spec.js @@ -0,0 +1,104 @@ +import { GlAvatarLink, GlSprintf } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import waitForPromises from 'helpers/wait_for_promises'; +import WorkItemCreatedUpdated from '~/work_items/components/work_item_created_updated.vue'; +import workItemQuery from '~/work_items/graphql/work_item.query.graphql'; +import workItemByIidQuery from '~/work_items/graphql/work_item_by_iid.query.graphql'; +import { workItemResponseFactory, mockAssignees } from '../mock_data'; + +describe('WorkItemCreatedUpdated component', () => { + let wrapper; + let successHandler; + let successByIidHandler; + + Vue.use(VueApollo); + + const findCreatedAt = () => wrapper.find('[data-testid="work-item-created"]'); + const findUpdatedAt = () => wrapper.find('[data-testid="work-item-updated"]'); + + const findCreatedAtText = () => findCreatedAt().text().replace(/\s+/g, ' '); + + const createComponent = async ({ + workItemId = 'gid://gitlab/WorkItem/1', + workItemIid = '1', + fetchByIid = false, + author = null, + updatedAt, + } = {}) => { + const workItemQueryResponse = workItemResponseFactory({ + author, + updatedAt, + }); + const byIidResponse = { + data: { + workspace: { + id: 'gid://gitlab/Project/1', + workItems: { + nodes: [workItemQueryResponse.data.workItem], + }, + }, + }, + }; + + successHandler = jest.fn().mockResolvedValue(workItemQueryResponse); + successByIidHandler = jest.fn().mockResolvedValue(byIidResponse); + + const handlers = [ + [workItemQuery, successHandler], + [workItemByIidQuery, successByIidHandler], + ]; + + wrapper = shallowMount(WorkItemCreatedUpdated, { + apolloProvider: createMockApollo(handlers), + propsData: { workItemId, workItemIid, fetchByIid, fullPath: '/some/project' }, + stubs: { + GlAvatarLink, + GlSprintf, + }, + }); + + await waitForPromises(); + }; + + describe.each([true, false])('fetchByIid is %s', (fetchByIid) => { + describe('work item id and iid undefined', () => { + beforeEach(async () => { + await createComponent({ workItemId: null, workItemIid: null, fetchByIid }); + }); + + it('skips the work item query', () => { + expect(successHandler).not.toHaveBeenCalled(); + expect(successByIidHandler).not.toHaveBeenCalled(); + }); + }); + + it('shows author name and link', async () => { + const author = mockAssignees[0]; + + await createComponent({ fetchByIid, author }); + + expect(findCreatedAtText()).toEqual(`Created by ${author.name}`); + }); + + it('shows created time when author is null', async () => { + await createComponent({ fetchByIid, author: null }); + + expect(findCreatedAtText()).toEqual('Created'); + }); + + it('shows updated time', async () => { + await createComponent({ fetchByIid }); + + expect(findUpdatedAt().exists()).toBe(true); + }); + + it('does not show updated time for new work items', async () => { + await createComponent({ fetchByIid, updatedAt: null }); + + expect(findUpdatedAt().exists()).toBe(false); + }); + }); +}); diff --git a/spec/frontend/work_items/components/work_item_detail_spec.js b/spec/frontend/work_items/components/work_item_detail_spec.js index b3b64c4fd40..cc4a4416253 100644 --- a/spec/frontend/work_items/components/work_item_detail_spec.js +++ b/spec/frontend/work_items/components/work_item_detail_spec.js @@ -16,6 +16,7 @@ import { stubComponent } from 'helpers/stub_component'; import WorkItemDetail from '~/work_items/components/work_item_detail.vue'; import WorkItemActions from '~/work_items/components/work_item_actions.vue'; import WorkItemDescription from '~/work_items/components/work_item_description.vue'; +import WorkItemCreatedUpdated from '~/work_items/components/work_item_created_updated.vue'; import WorkItemDueDate from '~/work_items/components/work_item_due_date.vue'; import WorkItemState from '~/work_items/components/work_item_state.vue'; import WorkItemTitle from '~/work_items/components/work_item_title.vue'; @@ -74,6 +75,7 @@ describe('WorkItemDetail component', () => { const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon); const findWorkItemActions = () => wrapper.findComponent(WorkItemActions); const findWorkItemTitle = () => wrapper.findComponent(WorkItemTitle); + const findCreatedUpdated = () => wrapper.findComponent(WorkItemCreatedUpdated); const findWorkItemState = () => wrapper.findComponent(WorkItemState); const findWorkItemDescription = () => wrapper.findComponent(WorkItemDescription); const findWorkItemDueDate = () => wrapper.findComponent(WorkItemDueDate); @@ -764,4 +766,18 @@ describe('WorkItemDetail component', () => { expect(findNotesWidget().exists()).toBe(true); }); }); + + it('does not render created/updated by default', async () => { + createComponent(); + await waitForPromises(); + + expect(findCreatedUpdated().exists()).toBe(false); + }); + + it('renders created/updated when the work_items_mvc flag is on', async () => { + createComponent({ workItemsMvcEnabled: true }); + await waitForPromises(); + + expect(findCreatedUpdated().exists()).toBe(true); + }); }); diff --git a/spec/frontend/work_items/mock_data.js b/spec/frontend/work_items/mock_data.js index bd5d0302ad9..5b331c016a9 100644 --- a/spec/frontend/work_items/mock_data.js +++ b/spec/frontend/work_items/mock_data.js @@ -57,7 +57,16 @@ export const workItemQueryResponse = { description: 'description', confidential: false, createdAt: '2022-08-03T12:41:54Z', + updatedAt: null, closedAt: null, + author: { + avatarUrl: 'http://127.0.0.1:3000/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + id: 'gid://gitlab/User/1', + name: 'Administrator', + username: 'root', + webUrl: 'http://127.0.0.1:3000/root', + __typename: 'UserCore', + }, project: { __typename: 'Project', id: '1', @@ -153,7 +162,11 @@ export const updateWorkItemMutationResponse = { description: 'description', confidential: false, createdAt: '2022-08-03T12:41:54Z', + updatedAt: '2022-08-08T12:41:54Z', closedAt: null, + author: { + ...mockAssignees[0], + }, project: { __typename: 'Project', id: '1', @@ -281,6 +294,9 @@ export const workItemResponseFactory = ({ withCheckboxes = false, parent = mockParent.parent, workItemType = taskType, + author = mockAssignees[0], + createdAt = '2022-08-03T12:41:54Z', + updatedAt = '2022-08-08T12:32:54Z', } = {}) => ({ data: { workItem: { @@ -291,8 +307,10 @@ export const workItemResponseFactory = ({ state: 'OPEN', description: 'description', confidential, - createdAt: '2022-08-03T12:41:54Z', + createdAt, + updatedAt, closedAt: null, + author, project: { __typename: 'Project', id: '1', @@ -473,7 +491,11 @@ export const createWorkItemMutationResponse = { description: 'description', confidential: false, createdAt: '2022-08-03T12:41:54Z', + updatedAt: null, closedAt: null, + author: { + ...mockAssignees[0], + }, project: { __typename: 'Project', id: '1', @@ -1048,11 +1070,15 @@ export const workItemObjectiveWithChild = { deleteWorkItem: true, updateWorkItem: true, }, + author: { + ...mockAssignees[0], + }, title: 'Objective', description: 'Objective description', state: 'OPEN', confidential: false, createdAt: '2022-08-03T12:41:54Z', + updatedAt: null, closedAt: null, widgets: [ { @@ -1193,7 +1219,11 @@ export const changeWorkItemParentMutationResponse = { title: 'Foo', confidential: false, createdAt: '2022-08-03T12:41:54Z', + updatedAt: null, closedAt: null, + author: { + ...mockAssignees[0], + }, project: { __typename: 'Project', id: '1', diff --git a/spec/helpers/ide_helper_spec.rb b/spec/helpers/ide_helper_spec.rb index 29b2784412e..e2ee4f33eee 100644 --- a/spec/helpers/ide_helper_spec.rb +++ b/spec/helpers/ide_helper_spec.rb @@ -15,16 +15,12 @@ RSpec.describe IdeHelper, feature_category: :web_ide do context 'with vscode_web_ide=true and instance vars set' do before do stub_feature_flags(vscode_web_ide: true) - - self.instance_variable_set(:@branch, 'master') - self.instance_variable_set(:@project, project) - self.instance_variable_set(:@path, 'foo/README.md') - self.instance_variable_set(:@merge_request, '7') end it 'returns hash' do - expect(helper.ide_data) - .to eq( + expect(helper.ide_data(project: project, branch: 'master', path: 'foo/README.md', merge_request: '7', +fork_info: nil)) + .to match( 'can-use-new-web-ide' => 'true', 'use-new-web-ide' => 'true', 'user-preferences-path' => profile_preferences_path, @@ -35,6 +31,9 @@ RSpec.describe IdeHelper, feature_category: :web_ide do 'csp-nonce' => 'test-csp-nonce', 'ide-remote-path' => ide_remote_path(remote_host: ':remote_host', remote_path: ':remote_path'), 'file-path' => 'foo/README.md', + 'editor-font-family' => 'JetBrains Mono', + 'editor-font-format' => 'woff2', + 'editor-font-src-url' => a_string_matching(%r{jetbrains-mono/JetBrainsMono}), 'merge-request' => '7', 'fork-info' => nil ) @@ -43,7 +42,8 @@ RSpec.describe IdeHelper, feature_category: :web_ide do it 'does not use new web ide if user.use_legacy_web_ide' do allow(user).to receive(:use_legacy_web_ide).and_return(true) - expect(helper.ide_data).to include('use-new-web-ide' => 'false') + expect(helper.ide_data(project: project, branch: nil, path: nil, merge_request: nil, +fork_info: nil)).to include('use-new-web-ide' => 'false') end end @@ -52,9 +52,9 @@ RSpec.describe IdeHelper, feature_category: :web_ide do stub_feature_flags(vscode_web_ide: false) end - context 'when instance vars are not set' do + context 'when instance vars and parameters are not set' do it 'returns instance data in the hash as nil' do - expect(helper.ide_data) + expect(helper.ide_data(project: nil, branch: nil, path: nil, merge_request: nil, fork_info: nil)) .to include( 'can-use-new-web-ide' => 'false', 'use-new-web-ide' => 'false', @@ -73,15 +73,10 @@ RSpec.describe IdeHelper, feature_category: :web_ide do it 'returns instance data in the hash' do fork_info = { ide_path: '/test/ide/path' } - self.instance_variable_set(:@branch, 'master') - self.instance_variable_set(:@path, 'foo/bar') - self.instance_variable_set(:@merge_request, '1') - self.instance_variable_set(:@fork_info, fork_info) - self.instance_variable_set(:@project, project) - serialized_project = API::Entities::Project.represent(project, current_user: project.creator).to_json - expect(helper.ide_data) + expect(helper.ide_data(project: project, branch: 'master', path: 'foo/bar', merge_request: '1', +fork_info: fork_info)) .to include( 'branch-name' => 'master', 'file-path' => 'foo/bar', @@ -96,12 +91,12 @@ RSpec.describe IdeHelper, feature_category: :web_ide do context 'environments guidance experiment', :experiment do before do stub_experiments(in_product_guidance_environments_webide: :candidate) - self.instance_variable_set(:@project, project) end context 'when project has no enviornments' do it 'enables environment guidance' do - expect(helper.ide_data).to include('enable-environments-guidance' => 'true') + expect(helper.ide_data(project: project, branch: nil, path: nil, merge_request: nil, +fork_info: nil)).to include('enable-environments-guidance' => 'true') end context 'and the callout has been dismissed' do @@ -109,7 +104,8 @@ RSpec.describe IdeHelper, feature_category: :web_ide do callout = create(:callout, feature_name: :web_ide_ci_environments_guidance, user: project.creator) callout.update!(dismissed_at: Time.now - 1.week) allow(helper).to receive(:current_user).and_return(User.find(project.creator.id)) - expect(helper.ide_data).to include('enable-environments-guidance' => 'false') + expect(helper.ide_data(project: project, branch: nil, path: nil, merge_request: nil, +fork_info: nil)).to include('enable-environments-guidance' => 'false') end end end @@ -118,7 +114,8 @@ RSpec.describe IdeHelper, feature_category: :web_ide do it 'disables environment guidance' do create(:environment, project: project) - expect(helper.ide_data).to include('enable-environments-guidance' => 'false') + expect(helper.ide_data(project: project, branch: nil, path: nil, merge_request: nil, +fork_info: nil)).to include('enable-environments-guidance' => 'false') end end end diff --git a/spec/lib/backup/database_spec.rb b/spec/lib/backup/database_spec.rb index bb7f8c63ee5..c70d47e4940 100644 --- a/spec/lib/backup/database_spec.rb +++ b/spec/lib/backup/database_spec.rb @@ -2,11 +2,20 @@ require 'spec_helper' +RSpec.configure do |rspec| + rspec.expect_with :rspec do |c| + c.max_formatted_output_length = nil + end +end + RSpec.describe Backup::Database, feature_category: :backup_restore do let(:progress) { StringIO.new } let(:output) { progress.string } let(:one_db_configured?) { Gitlab::Database.database_base_models.one? } let(:database_models_for_backup) { Gitlab::Database.database_base_models_with_gitlab_shared } + let(:timeout_service) do + instance_double(Gitlab::Database::TransactionTimeoutSettings, restore_timeouts: nil, disable_timeouts: nil) + end before(:all) do Rake::Task.define_task(:environment) @@ -26,6 +35,7 @@ RSpec.describe Backup::Database, feature_category: :backup_restore do before do database_models_for_backup.each do |database_name, base_model| base_model.connection.rollback_transaction unless base_model.connection.open_transactions.zero? + allow(base_model.connection).to receive(:execute).and_call_original end end @@ -55,6 +65,18 @@ RSpec.describe Backup::Database, feature_category: :backup_restore do end end + it 'disables transaction time out' do + number_of_databases = Gitlab::Database.database_base_models_with_gitlab_shared.count + expect(Gitlab::Database::TransactionTimeoutSettings) + .to receive(:new).exactly(2 * number_of_databases).times.and_return(timeout_service) + expect(timeout_service).to receive(:disable_timeouts).exactly(number_of_databases).times + expect(timeout_service).to receive(:restore_timeouts).exactly(number_of_databases).times + + Dir.mktmpdir do |dir| + subject.dump(dir, backup_id) + end + end + describe 'pg_dump arguments' do let(:snapshot_id) { 'fake_id' } let(:pg_args) do @@ -104,6 +126,23 @@ RSpec.describe Backup::Database, feature_category: :backup_restore do end end end + + context 'when a StandardError (or descendant) is raised' do + before do + allow(FileUtils).to receive(:mkdir_p).and_raise(StandardError) + end + + it 'restores timeouts' do + Dir.mktmpdir do |dir| + number_of_databases = Gitlab::Database.database_base_models_with_gitlab_shared.count + expect(Gitlab::Database::TransactionTimeoutSettings) + .to receive(:new).exactly(number_of_databases).times.and_return(timeout_service) + expect(timeout_service).to receive(:restore_timeouts).exactly(number_of_databases).times + + expect { subject.dump(dir, backup_id) }.to raise_error StandardError + end + end + end end describe '#restore' do diff --git a/spec/lib/extracts_ref_spec.rb b/spec/lib/extracts_ref_spec.rb index ca8af9413f3..93a09bf5a0a 100644 --- a/spec/lib/extracts_ref_spec.rb +++ b/spec/lib/extracts_ref_spec.rb @@ -48,13 +48,11 @@ RSpec.describe ExtractsRef do context 'when a ref_type parameter is provided' do let(:params) { ActionController::Parameters.new(path: path, ref: ref, ref_type: 'tags') } - context 'and the use_ref_type_parameter feature flag is enabled' do - it 'sets a fully_qualified_ref variable' do - fully_qualified_ref = "refs/tags/#{ref}" - expect(container.repository).to receive(:commit).with(fully_qualified_ref) - assign_ref_vars - expect(@fully_qualified_ref).to eq(fully_qualified_ref) - end + it 'sets a fully_qualified_ref variable' do + fully_qualified_ref = "refs/tags/#{ref}" + expect(container.repository).to receive(:commit).with(fully_qualified_ref) + assign_ref_vars + expect(@fully_qualified_ref).to eq(fully_qualified_ref) end end end diff --git a/spec/lib/gitlab/database/tables_locker_spec.rb b/spec/lib/gitlab/database/tables_locker_spec.rb new file mode 100644 index 00000000000..f4050f15e29 --- /dev/null +++ b/spec/lib/gitlab/database/tables_locker_spec.rb @@ -0,0 +1,214 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::TablesLocker, :reestablished_active_record_base, :delete, :silence_stdout, + :suppress_gitlab_schemas_validate_connection, feature_category: :pods do + let(:main_connection) { ApplicationRecord.connection } + let(:ci_connection) { Ci::ApplicationRecord.connection } + let!(:user) { create(:user) } + let!(:ci_build) { create(:ci_build) } + + let(:detached_partition_table) { '_test_gitlab_main_part_20220101' } + + before do + described_class.new.unlock_writes + end + + before(:all) do + create_detached_partition_sql = <<~SQL + CREATE TABLE IF NOT EXISTS gitlab_partitions_dynamic._test_gitlab_main_part_20220101 ( + id bigserial primary key not null + ) + SQL + + ApplicationRecord.connection.execute(create_detached_partition_sql) + Ci::ApplicationRecord.connection.execute(create_detached_partition_sql) + + Gitlab::Database::SharedModel.using_connection(ApplicationRecord.connection) do + Postgresql::DetachedPartition.create!( + table_name: '_test_gitlab_main_part_20220101', + drop_after: Time.current + ) + end + end + + after(:all) do + described_class.new.unlock_writes + + drop_detached_partition_sql = <<~SQL + DROP TABLE IF EXISTS gitlab_partitions_dynamic._test_gitlab_main_part_20220101 + SQL + + ApplicationRecord.connection.execute(drop_detached_partition_sql) + Ci::ApplicationRecord.connection.execute(drop_detached_partition_sql) + + Gitlab::Database::SharedModel.using_connection(ApplicationRecord.connection) do + Postgresql::DetachedPartition.delete_all + end + end + + context 'when running on single database' do + before do + skip_if_multiple_databases_are_setup + end + + describe '#lock_writes' do + subject { described_class.new.lock_writes } + + it 'does not add any triggers to the main schema tables' do + expect { subject }.not_to change { number_of_triggers(main_connection) } + end + + it 'will be still able to modify tables that belong to the main two schemas' do + subject + + expect do + User.last.touch + Ci::Build.last.touch + end.not_to raise_error + end + end + end + + context 'when running on multiple databases' do + before do + skip_if_multiple_databases_not_setup + + Gitlab::Database::SharedModel.using_connection(ci_connection) do + Postgresql::DetachedPartition.create!( + table_name: detached_partition_table, + drop_after: Time.zone.now + ) + end + end + + describe '#lock_writes' do + subject { described_class.new.lock_writes } + + it 'still allows writes on the tables with the correct connections' do + User.touch_all + Ci::Build.touch_all + end + + it 'still allows writing to gitlab_shared schema on any connection' do + connections = [main_connection, ci_connection] + connections.each do |connection| + Gitlab::Database::SharedModel.using_connection(connection) do + LooseForeignKeys::DeletedRecord.create!( + fully_qualified_table_name: "public.users", + primary_key_value: 1, + cleanup_attempts: 0 + ) + end + end + end + + it 'prevents writes on the main tables on the ci database' do + subject + + expect do + ci_connection.execute("delete from users") + end.to raise_error(ActiveRecord::StatementInvalid, /Table: "users" is write protected/) + end + + it 'prevents writes on the ci tables on the main database' do + subject + + expect do + main_connection.execute("delete from ci_builds") + end.to raise_error(ActiveRecord::StatementInvalid, /Table: "ci_builds" is write protected/) + end + + it 'prevents truncating a ci table on the main database' do + subject + + expect do + main_connection.execute("truncate ci_build_needs") + end.to raise_error(ActiveRecord::StatementInvalid, /Table: "ci_build_needs" is write protected/) + end + + it 'prevents writes to detached partitions' do + subject + + expect do + ci_connection.execute("INSERT INTO gitlab_partitions_dynamic.#{detached_partition_table} DEFAULT VALUES") + end.to raise_error(ActiveRecord::StatementInvalid, /Table: "#{detached_partition_table}" is write protected/) + end + + context 'when running in dry_run mode' do + subject { described_class.new(dry_run: true).lock_writes } + + it 'allows writes on the main tables on the ci database' do + subject + + expect do + ci_connection.execute("delete from users") + end.not_to raise_error + end + + it 'allows writes on the ci tables on the main database' do + subject + + expect do + main_connection.execute("delete from ci_builds") + end.not_to raise_error + end + end + + context 'when running on multiple shared databases' do + before do + allow(::Gitlab::Database).to receive(:db_config_share_with).and_return(nil) + ci_db_config = Ci::ApplicationRecord.connection_db_config + allow(::Gitlab::Database).to receive(:db_config_share_with).with(ci_db_config).and_return('main') + end + + it 'does not lock any tables if the ci database is shared with main database' do + subject { described_class.new.lock_writes } + + expect do + ApplicationRecord.connection.execute("delete from ci_builds") + Ci::ApplicationRecord.connection.execute("delete from users") + end.not_to raise_error + end + end + end + end + + context 'when geo database is configured' do + let(:lock_writes_manager) do + instance_double(Gitlab::Database::LockWritesManager, lock_writes: nil, unlock_writes: nil) + end + + let(:geo_table) do + Gitlab::Database::GitlabSchema + .tables_to_schema.filter_map { |table_name, schema| table_name if schema == :gitlab_geo } + .first + end + + subject { described_class.new.unlock_writes } + + before do + skip "Geo database is not configured" unless Gitlab::Database.has_config?(:geo) + + allow(Gitlab::Database::LockWritesManager).to receive(:new).with(any_args).and_return(lock_writes_manager) + end + + it 'does not lock table in geo database' do + expect(Gitlab::Database::LockWritesManager).not_to receive(:new).with( + table_name: geo_table, + connection: anything, + database_name: 'geo', + with_retries: true, + logger: anything, + dry_run: anything + ) + + subject + end + end +end + +def number_of_triggers(connection) + connection.select_value("SELECT count(*) FROM information_schema.triggers") +end diff --git a/spec/lib/gitlab/database/transaction_timeout_settings_spec.rb b/spec/lib/gitlab/database/transaction_timeout_settings_spec.rb new file mode 100644 index 00000000000..5b68f9a3757 --- /dev/null +++ b/spec/lib/gitlab/database/transaction_timeout_settings_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::TransactionTimeoutSettings, feature_category: :pods do + let(:connection) { ActiveRecord::Base.connection } + + subject { described_class.new(connection) } + + after(:all) do + described_class.new(ActiveRecord::Base.connection).restore_timeouts + end + + describe '#disable_timeouts' do + it 'sets timeouts to 0' do + subject.disable_timeouts + + expect(current_timeout).to eq("0") + end + end + + describe '#restore_timeouts' do + before do + subject.disable_timeouts + end + + it 'resets value' do + expect(connection).to receive(:execute).with('RESET idle_in_transaction_session_timeout').and_call_original + + subject.restore_timeouts + end + end + + def current_timeout + connection.execute("show idle_in_transaction_session_timeout").first['idle_in_transaction_session_timeout'] + end +end diff --git a/spec/lib/gitlab/redis/repository_cache_spec.rb b/spec/lib/gitlab/redis/repository_cache_spec.rb index b11e9ebf1f3..56f77782778 100644 --- a/spec/lib/gitlab/redis/repository_cache_spec.rb +++ b/spec/lib/gitlab/redis/repository_cache_spec.rb @@ -14,6 +14,9 @@ RSpec.describe Gitlab::Redis::RepositoryCache, feature_category: :scalability do before do allow(described_class).to receive(:config_file_name).and_return(config_new_format_host) + + # Override rails root to avoid having our fixtures overwritten by `redis.yml` if it exists + allow(Gitlab::Redis::Cache).to receive(:rails_root).and_return(mktmpdir) allow(Gitlab::Redis::Cache).to receive(:config_file_name).and_return(config_new_format_socket) end diff --git a/spec/lib/gitlab/redis/sidekiq_status_spec.rb b/spec/lib/gitlab/redis/sidekiq_status_spec.rb index e7cf229b494..bbfec13e6c8 100644 --- a/spec/lib/gitlab/redis/sidekiq_status_spec.rb +++ b/spec/lib/gitlab/redis/sidekiq_status_spec.rb @@ -14,10 +14,15 @@ RSpec.describe Gitlab::Redis::SidekiqStatus do describe '#pool' do let(:config_new_format_host) { "spec/fixtures/config/redis_new_format_host.yml" } let(:config_new_format_socket) { "spec/fixtures/config/redis_new_format_socket.yml" } + let(:rails_root) { mktmpdir } subject { described_class.pool } before do + # Override rails root to avoid having our fixtures overwritten by `redis.yml` if it exists + allow(Gitlab::Redis::SharedState).to receive(:rails_root).and_return(rails_root) + allow(Gitlab::Redis::Queues).to receive(:rails_root).and_return(rails_root) + allow(Gitlab::Redis::SharedState).to receive(:config_file_name).and_return(config_new_format_host) allow(Gitlab::Redis::Queues).to receive(:config_file_name).and_return(config_new_format_socket) end diff --git a/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb index cb7970cacec..f8a4603c1f8 100644 --- a/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/editor_unique_counter_spec.rb @@ -65,17 +65,4 @@ RSpec.describe Gitlab::UsageDataCounters::EditorUniqueCounter, :clean_gitlab_red end end end - - it 'can return the count of actions per user deduplicated' do - described_class.track_web_ide_edit_action(author: user1, project: project) - described_class.track_snippet_editor_edit_action(author: user1, project: project) - described_class.track_sfe_edit_action(author: user1, project: project) - described_class.track_web_ide_edit_action(author: user2, time: time - 2.days, project: project) - described_class.track_web_ide_edit_action(author: user3, time: time - 3.days, project: project) - described_class.track_snippet_editor_edit_action(author: user3, time: time - 3.days, project: project) - described_class.track_sfe_edit_action(author: user3, time: time - 3.days, project: project) - - expect(described_class.count_edit_using_editor(date_from: time, date_to: Date.today)).to eq(1) - expect(described_class.count_edit_using_editor(date_from: time - 5.days, date_to: Date.tomorrow)).to eq(3) - end end diff --git a/spec/lib/gitlab/usage_data_spec.rb b/spec/lib/gitlab/usage_data_spec.rb index ffa34acef5c..5325ef5b5dd 100644 --- a/spec/lib/gitlab/usage_data_spec.rb +++ b/spec/lib/gitlab/usage_data_spec.rb @@ -1099,8 +1099,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures, feature_category: :servic { action_monthly_active_users_web_ide_edit: 2, action_monthly_active_users_sfe_edit: 2, - action_monthly_active_users_snippet_editor_edit: 2, - action_monthly_active_users_ide_edit: 3 + action_monthly_active_users_snippet_editor_edit: 2 } ) end diff --git a/spec/lib/sidebars/projects/menus/repository_menu_spec.rb b/spec/lib/sidebars/projects/menus/repository_menu_spec.rb index e7aa2b7edca..40ca2107698 100644 --- a/spec/lib/sidebars/projects/menus/repository_menu_spec.rb +++ b/spec/lib/sidebars/projects/menus/repository_menu_spec.rb @@ -48,20 +48,13 @@ RSpec.describe Sidebars::Projects::Menus::RepositoryMenu, feature_category: :sou ref_type: ref_type) end - where(:feature_flag_enabled, :ref_type, :link) do - true | nil | lazy { "#{route}?ref_type=heads" } - true | 'heads' | lazy { "#{route}?ref_type=heads" } - true | 'tags' | lazy { "#{route}?ref_type=tags" } - false | nil | lazy { route } - false | 'heads' | lazy { route } - false | 'tags' | lazy { route } + where(:ref_type, :link) do + nil | lazy { "#{route}?ref_type=heads" } + 'heads' | lazy { "#{route}?ref_type=heads" } + 'tags' | lazy { "#{route}?ref_type=tags" } end with_them do - before do - stub_feature_flags(use_ref_type_parameter: feature_flag_enabled) - end - it 'has a link with the fully qualifed ref route' do expect(subject).to eq(link) end diff --git a/spec/models/work_item_spec.rb b/spec/models/work_item_spec.rb index 52155c3b936..6aacaa3c119 100644 --- a/spec/models/work_item_spec.rb +++ b/spec/models/work_item_spec.rb @@ -21,9 +21,8 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do .with_foreign_key('work_item_id') end - it 'has many `work_item_children_by_created_at`' do - is_expected.to have_many(:work_item_children_by_created_at) - .order(created_at: :asc) + it 'has many `work_item_children_by_relative_position`' do + is_expected.to have_many(:work_item_children_by_relative_position) .class_name('WorkItem') .with_foreign_key('work_item_id') end @@ -35,6 +34,49 @@ RSpec.describe WorkItem, feature_category: :portfolio_management do end end + describe '.work_item_children_by_relative_position' do + subject { parent_item.reload.work_item_children_by_relative_position } + + let_it_be(:parent_item) { create(:work_item, :objective, project: reusable_project) } + let_it_be(:oldest_item) { create(:work_item, :objective, created_at: 5.hours.ago, project: reusable_project) } + let_it_be(:middle_item) { create(:work_item, :objective, project: reusable_project) } + let_it_be(:newest_item) { create(:work_item, :objective, created_at: 5.hours.from_now, project: reusable_project) } + + let_it_be_with_reload(:link_to_oldest_item) do + create(:parent_link, work_item_parent: parent_item, work_item: oldest_item) + end + + let_it_be_with_reload(:link_to_middle_item) do + create(:parent_link, work_item_parent: parent_item, work_item: middle_item) + end + + let_it_be_with_reload(:link_to_newest_item) do + create(:parent_link, work_item_parent: parent_item, work_item: newest_item) + end + + context 'when ordered by relative position and created_at' do + using RSpec::Parameterized::TableSyntax + + where(:oldest_item_position, :middle_item_position, :newest_item_position, :expected_order) do + nil | nil | nil | lazy { [oldest_item, middle_item, newest_item] } + nil | nil | 1 | lazy { [newest_item, oldest_item, middle_item] } + nil | 1 | 2 | lazy { [middle_item, newest_item, oldest_item] } + 2 | 3 | 1 | lazy { [newest_item, oldest_item, middle_item] } + 1 | 2 | 3 | lazy { [oldest_item, middle_item, newest_item] } + end + + with_them do + before do + link_to_oldest_item.update!(relative_position: oldest_item_position) + link_to_middle_item.update!(relative_position: middle_item_position) + link_to_newest_item.update!(relative_position: newest_item_position) + end + + it { is_expected.to eq(expected_order) } + end + end + end + describe '#noteable_target_type_name' do it 'returns `issue` as the target name' do work_item = build(:work_item) diff --git a/spec/models/work_items/widgets/hierarchy_spec.rb b/spec/models/work_items/widgets/hierarchy_spec.rb index 43670b30645..7ff3088d9ec 100644 --- a/spec/models/work_items/widgets/hierarchy_spec.rb +++ b/spec/models/work_items/widgets/hierarchy_spec.rb @@ -36,14 +36,40 @@ RSpec.describe WorkItems::Widgets::Hierarchy, feature_category: :team_planning d it { is_expected.to contain_exactly(parent_link1.work_item, parent_link2.work_item) } - context 'with default order by created_at' do + context 'when ordered by relative position and created_at' do let_it_be(:oldest_child) { create(:work_item, :task, project: project, created_at: 5.minutes.ago) } + let_it_be(:newest_child) { create(:work_item, :task, project: project, created_at: 5.minutes.from_now) } let_it_be_with_reload(:link_to_oldest_child) do create(:parent_link, work_item_parent: work_item_parent, work_item: oldest_child) end - it { is_expected.to eq([link_to_oldest_child, parent_link1, parent_link2].map(&:work_item)) } + let_it_be_with_reload(:link_to_newest_child) do + create(:parent_link, work_item_parent: work_item_parent, work_item: newest_child) + end + + let(:parent_links_ordered) { [link_to_oldest_child, parent_link1, parent_link2, link_to_newest_child] } + + context 'when children relative positions are nil' do + it 'orders by created_at' do + is_expected.to eq(parent_links_ordered.map(&:work_item)) + end + end + + context 'when children relative positions are present' do + let(:first_position) { 10 } + let(:second_position) { 20 } + let(:parent_links_ordered) { [link_to_oldest_child, link_to_newest_child, parent_link1, parent_link2] } + + before do + link_to_oldest_child.update!(relative_position: first_position) + link_to_newest_child.update!(relative_position: second_position) + end + + it 'orders by relative_position and by created_at' do + is_expected.to eq(parent_links_ordered.map(&:work_item)) + end + end end end end diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index 1f321d1dec3..0fad4f4ff3a 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -215,6 +215,20 @@ RSpec.describe 'Query.work_item(id)', feature_category: :team_planning do it 'places the newest child item to the end of the children list' do expect(hierarchy_children.last['id']).to eq(newest_child.to_gid.to_s) end + + context 'when relative position is set' do + let_it_be(:first_child) { create(:work_item, :task, project: project, created_at: 5.minutes.from_now) } + + let_it_be(:first_link) do + create(:parent_link, work_item_parent: work_item, work_item: first_child, relative_position: 1) + end + + it 'places children according to relative_position at the beginning of the children list' do + ordered_list = [first_child, oldest_child, child_item1, child_item2, newest_child] + + expect(hierarchy_children.pluck('id')).to eq(ordered_list.map(&:to_gid).map(&:to_s)) + end + end end end diff --git a/spec/requests/projects/network_controller_spec.rb b/spec/requests/projects/network_controller_spec.rb index 954f9655558..dee95c6e70e 100644 --- a/spec/requests/projects/network_controller_spec.rb +++ b/spec/requests/projects/network_controller_spec.rb @@ -35,17 +35,6 @@ RSpec.describe Projects::NetworkController, feature_category: :source_code_manag subject expect(assigns(:url)).to eq(project_network_path(project, ref, format: :json, ref_type: 'heads')) end - - context 'when the use_ref_type_parameter flag is disabled' do - before do - stub_feature_flags(use_ref_type_parameter: false) - end - - it 'assigns url without ref_type' do - subject - expect(assigns(:url)).to eq(project_network_path(project, ref, format: :json)) - end - end end it 'assigns url' do diff --git a/spec/support/redis/redis_new_instance_shared_examples.rb b/spec/support/redis/redis_new_instance_shared_examples.rb index 0f2de78b2cb..435d342fcca 100644 --- a/spec/support/redis/redis_new_instance_shared_examples.rb +++ b/spec/support/redis/redis_new_instance_shared_examples.rb @@ -27,38 +27,34 @@ RSpec.shared_examples "redis_new_instance_shared_examples" do |name, fallback_cl FileUtils.mkdir_p(File.join(rails_root, 'config')) end - context 'when there is only a resque.yml' do + context 'and there is a global env override' do before do - FileUtils.touch(File.join(rails_root, 'config/resque.yml')) + stub_env('GITLAB_REDIS_CONFIG_FILE', 'global override') end - it { expect(subject).to eq("#{rails_root}/config/resque.yml") } + it { expect(subject).to eq('global override') } - context 'and there is a global env override' do - before do - stub_env('GITLAB_REDIS_CONFIG_FILE', 'global override') - end - - it { expect(subject).to eq('global override') } + context "and #{fallback_class.name.demodulize} has a different config file" do + let(:fallback_config_file) { 'fallback config file' } - context "and #{fallback_class.name.demodulize} has a different config file" do - let(:fallback_config_file) { 'fallback config file' } - - it { expect(subject).to eq('fallback config file') } - end + it { expect(subject).to eq('fallback config file') } end end end describe '#fetch_config' do - context 'when redis.yml exists' do - subject { described_class.new('test').send(:fetch_config) } + subject { described_class.new('test').send(:fetch_config) } + + before do + FileUtils.mkdir_p(File.join(rails_root, 'config')) + + allow(described_class).to receive(:rails_root).and_return(rails_root) + end + context 'when redis.yml exists' do before do allow(described_class).to receive(:config_file_name).and_call_original allow(described_class).to receive(:redis_yml_path).and_call_original - allow(described_class).to receive(:rails_root).and_return(rails_root) - FileUtils.mkdir_p(File.join(rails_root, 'config')) end context 'when the fallback has a redis.yml entry' do @@ -93,5 +89,23 @@ RSpec.shared_examples "redis_new_instance_shared_examples" do |name, fallback_cl end end end + + context 'when no redis config file exsits' do + it 'returns nil' do + expect(subject).to eq(nil) + end + + context 'when resque.yml exists' do + before do + File.write(File.join(rails_root, 'config/resque.yml'), { + 'test' => { 'foobar' => 123 } + }.to_json) + end + + it 'returns the config from resque.yml' do + expect(subject).to eq({ 'foobar' => 123 }) + end + end + end end end diff --git a/spec/support/redis/redis_shared_examples.rb b/spec/support/redis/redis_shared_examples.rb index 43c118a362d..6e20f6028be 100644 --- a/spec/support/redis/redis_shared_examples.rb +++ b/spec/support/redis/redis_shared_examples.rb @@ -40,42 +40,30 @@ RSpec.shared_examples "redis_shared_examples" do context 'when there is no config file anywhere' do it { expect(subject).to be_nil } - context 'but resque.yml exists' do + context 'and there is a global env override' do before do - FileUtils.touch(File.join(rails_root, 'config', 'resque.yml')) + stub_env('GITLAB_REDIS_CONFIG_FILE', 'global override') end - it { expect(subject).to eq("#{rails_root}/config/resque.yml") } - - it 'returns a path that exists' do - expect(File.file?(subject)).to eq(true) - end + it { expect(subject).to eq('global override') } - context 'and there is a global env override' do + context 'and there is an instance specific config file' do before do - stub_env('GITLAB_REDIS_CONFIG_FILE', 'global override') + FileUtils.touch(File.join(rails_root, instance_specific_config_file)) end - it { expect(subject).to eq('global override') } - - context 'and there is an instance specific config file' do - before do - FileUtils.touch(File.join(rails_root, instance_specific_config_file)) - end + it { expect(subject).to eq("#{rails_root}/#{instance_specific_config_file}") } - it { expect(subject).to eq("#{rails_root}/#{instance_specific_config_file}") } + it 'returns a path that exists' do + expect(File.file?(subject)).to eq(true) + end - it 'returns a path that exists' do - expect(File.file?(subject)).to eq(true) + context 'and there is a specific env override' do + before do + stub_env(environment_config_file_name, 'instance specific override') end - context 'and there is a specific env override' do - before do - stub_env(environment_config_file_name, 'instance specific override') - end - - it { expect(subject).to eq('instance specific override') } - end + it { expect(subject).to eq('instance specific override') } end end end @@ -402,6 +390,12 @@ RSpec.shared_examples "redis_shared_examples" do end describe '#fetch_config' do + before do + FileUtils.mkdir_p(File.join(rails_root, 'config')) + + allow(described_class).to receive(:rails_root).and_return(rails_root) + end + it 'raises an exception when the config file contains invalid yaml' do Tempfile.open('bad.yml') do |file| file.write('{"not":"yaml"') @@ -424,8 +418,6 @@ RSpec.shared_examples "redis_shared_examples" do before do allow(described_class).to receive(:config_file_name).and_call_original allow(described_class).to receive(:redis_yml_path).and_call_original - allow(described_class).to receive(:rails_root).and_return(rails_root) - FileUtils.mkdir_p(File.join(rails_root, 'config')) end it 'uses config/redis.yml' do @@ -436,6 +428,27 @@ RSpec.shared_examples "redis_shared_examples" do expect(subject).to eq({ 'foobar' => 123 }) end end + + context 'when no config file exsits' do + subject { described_class.new('test').send(:fetch_config) } + + it 'returns nil' do + expect(subject).to eq(nil) + end + + context 'but resque.yml exists' do + before do + FileUtils.mkdir_p(File.join(rails_root, 'config')) + File.write(File.join(rails_root, 'config/resque.yml'), { + 'test' => { 'foobar' => 123 } + }.to_json) + end + + it 'returns the config from resque.yml' do + expect(subject).to eq({ 'foobar' => 123 }) + end + end + end end def clear_pool diff --git a/spec/tasks/gitlab/db/lock_writes_rake_spec.rb b/spec/tasks/gitlab/db/lock_writes_rake_spec.rb index a0a99b65767..24a2c0a48f7 100644 --- a/spec/tasks/gitlab/db/lock_writes_rake_spec.rb +++ b/spec/tasks/gitlab/db/lock_writes_rake_spec.rb @@ -2,8 +2,7 @@ require 'rake_helper' -RSpec.describe 'gitlab:db:lock_writes', :silence_stdout, :reestablished_active_record_base, :delete, - :suppress_gitlab_schemas_validate_connection, feature_category: :pods do +RSpec.describe 'gitlab:db:lock_writes', :reestablished_active_record_base, feature_category: :pods do before :all do Rake.application.rake_require 'active_record/railties/databases' Rake.application.rake_require 'tasks/seed_fu' @@ -14,193 +13,77 @@ RSpec.describe 'gitlab:db:lock_writes', :silence_stdout, :reestablished_active_r Rake::Task.define_task :environment end - let(:main_connection) { ApplicationRecord.connection } - let(:ci_connection) { Ci::ApplicationRecord.connection } - let!(:user) { create(:user) } - let!(:ci_build) { create(:ci_build) } - - let(:detached_partition_table) { '_test_gitlab_main_part_20220101' } + let(:table_locker) { instance_double(Gitlab::Database::TablesLocker) } + let(:logger) { instance_double(Logger) } before do - create_detached_partition_sql = <<~SQL - CREATE TABLE IF NOT EXISTS gitlab_partitions_dynamic._test_gitlab_main_part_20220101 ( - id bigserial primary key not null - ) - SQL - - main_connection.execute(create_detached_partition_sql) - ci_connection.execute(create_detached_partition_sql) - - Gitlab::Database::SharedModel.using_connection(main_connection) do - Postgresql::DetachedPartition.create!( - table_name: detached_partition_table, - drop_after: Time.current - ) - end + allow(Logger).to receive(:new).with($stdout).and_return(logger) + allow(Gitlab::Database::TablesLocker).to receive(:new).with( + logger: logger, dry_run: dry_run + ).and_return(table_locker) end - after do - run_rake_task('gitlab:db:unlock_writes') - end - - after(:all) do - drop_detached_partition_sql = <<~SQL - DROP TABLE IF EXISTS gitlab_partitions_dynamic._test_gitlab_main_part_20220101 - SQL - - ApplicationRecord.connection.execute(drop_detached_partition_sql) - Ci::ApplicationRecord.connection.execute(drop_detached_partition_sql) + shared_examples "call table locker" do |method| + it "creates TablesLocker with dry run set and calls #{method}" do + expect(table_locker).to receive(method) - Gitlab::Database::SharedModel.using_connection(ApplicationRecord.connection) do - Postgresql::DetachedPartition.delete_all + run_rake_task("gitlab:db:#{method}") end end - context 'single database' do - before do - skip_if_multiple_databases_are_setup - end - - context 'when locking writes' do - it 'does not add any triggers to the main schema tables' do - expect do - run_rake_task('gitlab:db:lock_writes') - end.to change { - number_of_triggers(main_connection) - }.by(0) - end + describe 'lock_writes' do + context 'when environment sets DRY_RUN to true' do + let(:dry_run) { true } - it 'will be still able to modify tables that belong to the main two schemas' do - run_rake_task('gitlab:db:lock_writes') - expect do - User.last.touch - Ci::Build.last.touch - end.not_to raise_error + before do + stub_env('DRY_RUN', 'true') end - end - end - - context 'multiple databases' do - before do - skip_if_multiple_databases_not_setup - Gitlab::Database::SharedModel.using_connection(ci_connection) do - Postgresql::DetachedPartition.create!( - table_name: detached_partition_table, - drop_after: Time.current - ) - end + include_examples "call table locker", :lock_writes end - context 'when locking writes' do - it 'still allows writes on the tables with the correct connections' do - User.update_all(updated_at: Time.now) - Ci::Build.update_all(updated_at: Time.now) - end + context 'when environment sets DRY_RUN to false' do + let(:dry_run) { false } - it 'still allows writing to gitlab_shared schema on any connection' do - connections = [main_connection, ci_connection] - connections.each do |connection| - Gitlab::Database::SharedModel.using_connection(connection) do - LooseForeignKeys::DeletedRecord.create!( - fully_qualified_table_name: "public.users", - primary_key_value: 1, - cleanup_attempts: 0 - ) - end - end + before do + stub_env('DRY_RUN', 'false') end - it 'prevents writes on the main tables on the ci database' do - run_rake_task('gitlab:db:lock_writes') - expect do - ci_connection.execute("delete from users") - end.to raise_error(ActiveRecord::StatementInvalid, /Table: "users" is write protected/) - end + include_examples "call table locker", :lock_writes + end - it 'prevents writes on the ci tables on the main database' do - run_rake_task('gitlab:db:lock_writes') - expect do - main_connection.execute("delete from ci_builds") - end.to raise_error(ActiveRecord::StatementInvalid, /Table: "ci_builds" is write protected/) - end + context 'when environment does not define DRY_RUN' do + let(:dry_run) { false } - it 'prevents truncating a ci table on the main database' do - run_rake_task('gitlab:db:lock_writes') - expect do - main_connection.execute("truncate ci_build_needs") - end.to raise_error(ActiveRecord::StatementInvalid, /Table: "ci_build_needs" is write protected/) - end - - it 'prevents writes to detached partitions' do - run_rake_task('gitlab:db:lock_writes') - expect do - ci_connection.execute("INSERT INTO gitlab_partitions_dynamic.#{detached_partition_table} DEFAULT VALUES") - end.to raise_error(ActiveRecord::StatementInvalid, /Table: "#{detached_partition_table}" is write protected/) - end + include_examples "call table locker", :lock_writes end + end + + describe 'unlock_writes' do + context 'when environment sets DRY_RUN to true' do + let(:dry_run) { true } - context 'when running in dry_run mode' do before do stub_env('DRY_RUN', 'true') end - it 'allows writes on the main tables on the ci database' do - run_rake_task('gitlab:db:lock_writes') - expect do - ci_connection.execute("delete from users") - end.not_to raise_error - end - - it 'allows writes on the ci tables on the main database' do - run_rake_task('gitlab:db:lock_writes') - expect do - main_connection.execute("delete from ci_builds") - end.not_to raise_error - end + include_examples "call table locker", :unlock_writes end - context 'multiple shared databases' do - before do - allow(::Gitlab::Database).to receive(:db_config_share_with).and_return(nil) - ci_db_config = Ci::ApplicationRecord.connection_db_config - allow(::Gitlab::Database).to receive(:db_config_share_with).with(ci_db_config).and_return('main') - end - - it 'does not lock any tables if the ci database is shared with main database' do - run_rake_task('gitlab:db:lock_writes') + context 'when environment sets DRY_RUN to false' do + let(:dry_run) { false } - expect do - ApplicationRecord.connection.execute("delete from ci_builds") - Ci::ApplicationRecord.connection.execute("delete from users") - end.not_to raise_error - end - end - - context 'when unlocking writes' do before do - run_rake_task('gitlab:db:lock_writes') + stub_env('DRY_RUN', 'false') end - it 'allows writes again on the gitlab_ci tables on the main database' do - run_rake_task('gitlab:db:unlock_writes') - - expect do - main_connection.execute("delete from ci_builds") - end.not_to raise_error - end + include_examples "call table locker", :unlock_writes + end - it 'allows writes again to detached partitions' do - run_rake_task('gitlab:db:unlock_writes') + context 'when environment does not define DRY_RUN' do + let(:dry_run) { false } - expect do - ci_connection.execute("INSERT INTO gitlab_partitions_dynamic._test_gitlab_main_part_20220101 DEFAULT VALUES") - end.not_to raise_error - end + include_examples "call table locker", :unlock_writes end end - - def number_of_triggers(connection) - connection.select_value("SELECT count(*) FROM information_schema.triggers") - end end diff --git a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb index 82584155de5..cddff276317 100644 --- a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb +++ b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb @@ -83,24 +83,10 @@ RSpec.describe 'layouts/nav/sidebar/_project', feature_category: :navigation do end describe 'Commits' do - context 'when the use_ref_type_parameter flag is not enabled' do - before do - stub_feature_flags(use_ref_type_parameter: false) - end - - it 'has a link to the project commits path' do - render - - expect(rendered).to have_link('Commits', href: project_commits_path(project, current_ref), id: 'js-onboarding-commits-link') - end - end - - context 'when the use_ref_type_parameter flag is enabled' do - it 'has a link to the fully qualified project commits path' do - render + it 'has a link to the fully qualified project commits path' do + render - expect(rendered).to have_link('Commits', href: project_commits_path(project, current_ref, ref_type: 'heads'), id: 'js-onboarding-commits-link') - end + expect(rendered).to have_link('Commits', href: project_commits_path(project, current_ref, ref_type: 'heads'), id: 'js-onboarding-commits-link') end end @@ -121,24 +107,10 @@ RSpec.describe 'layouts/nav/sidebar/_project', feature_category: :navigation do end describe 'Contributors' do - context 'and the use_ref_type_parameter flag is disabled' do - before do - stub_feature_flags(use_ref_type_parameter: false) - end - - it 'has a link to the project contributors path' do - render - - expect(rendered).to have_link('Contributors', href: project_graph_path(project, current_ref)) - end - end - - context 'and the use_ref_type_parameter flag is enabled' do - it 'has a link to the project contributors path' do - render + it 'has a link to the project contributors path' do + render - expect(rendered).to have_link('Contributors', href: project_graph_path(project, current_ref, ref_type: 'heads')) - end + expect(rendered).to have_link('Contributors', href: project_graph_path(project, current_ref, ref_type: 'heads')) end end |