diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-16 21:08:46 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-01-16 21:08:46 +0300 |
commit | aa0f0e992153e84e1cdec8a1c7310d5eb93a9f8f (patch) | |
tree | 4a662bc77fb43e1d1deec78cc7a95d911c0da1c5 /spec | |
parent | d47f9d2304dbc3a23bba7fe7a5cd07218eeb41cd (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec')
58 files changed, 1605 insertions, 229 deletions
diff --git a/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb b/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb index 6ae37fe6d2f..1c29b68dc24 100644 --- a/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb +++ b/spec/controllers/projects/performance_monitoring/dashboards_controller_spec.rb @@ -37,144 +37,72 @@ describe Projects::PerformanceMonitoring::DashboardsController do end context 'valid parameters' do - it 'delegates commit creation to service' do + it 'delegates cloning to ::Metrics::Dashboard::CloneDashboardService' do allow(controller).to receive(:repository).and_return(repository) allow(repository).to receive(:find_branch).and_return(branch) dashboard_attrs = { + dashboard: dashboard, + file_name: file_name, commit_message: commit_message, - branch_name: branch_name, - start_branch: 'master', - encoding: 'text', - file_path: '.gitlab/dashboards/custom_dashboard.yml', - file_content: File.read('config/prometheus/common_metrics.yml') + branch: branch_name } - service_instance = instance_double(::Files::CreateService) - expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance) - expect(service_instance).to receive(:execute).and_return(status: :success) + service_instance = instance_double(::Metrics::Dashboard::CloneDashboardService) + expect(::Metrics::Dashboard::CloneDashboardService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance) + expect(service_instance).to receive(:execute).and_return(status: :success, http_status: :created, dashboard: { path: 'dashboard/path' }) post :create, params: params end - it 'extends dashboard template path to absolute url' do - allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success })) - allow(controller).to receive(:repository).and_return(repository) - allow(repository).to receive(:find_branch).and_return(branch) - - expect(File).to receive(:read).with(Rails.root.join('config/prometheus/common_metrics.yml')).and_return('') - - post :create, params: params - end - - context 'selected branch already exists' do - it 'responds with :created status code', :aggregate_failures do - repository.add_branch(user, branch_name, 'master') - - post :create, params: params - - expect(response).to have_gitlab_http_status :created - end - end - context 'request format json' do - it 'returns path to new file' do - allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success })) + it 'returns services response' do + allow(::Metrics::Dashboard::CloneDashboardService).to receive(:new).and_return(double(execute: { status: :success, dashboard: { path: ".gitlab/dashboards/#{file_name}" }, http_status: :created })) allow(controller).to receive(:repository).and_return(repository) - - expect(repository).to receive(:find_branch).with(branch_name).and_return(branch) + allow(repository).to receive(:find_branch).and_return(branch) post :create, params: params expect(response).to have_gitlab_http_status :created - expect(json_response).to eq('redirect_to' => "/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}") + expect(response).to set_flash[:notice].to eq("Your dashboard has been copied. You can <a href=\"/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}\">edit it here</a>.") + expect(json_response).to eq('status' => 'success', 'dashboard' => { 'path' => ".gitlab/dashboards/#{file_name}" }) end - context 'files create service failure' do - it 'returns json with failure message' do - allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: false, message: 'something went wrong' })) + context 'Metrics::Dashboard::CloneDashboardService failure' do + it 'returns json with failure message', :aggregate_failures do + allow(::Metrics::Dashboard::CloneDashboardService).to receive(:new).and_return(double(execute: { status: :error, message: 'something went wrong', http_status: :bad_request })) post :create, params: params expect(response).to have_gitlab_http_status :bad_request - expect(response).to set_flash[:alert].to eq('something went wrong') expect(json_response).to eq('error' => 'something went wrong') end end - end - context 'request format html' do - before do - params.delete(:format) - end + %w(commit_message file_name dashboard).each do |param| + context "param #{param} is missing" do + let(param.to_s) { nil } - it 'redirects to ide with new file' do - allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success })) - allow(controller).to receive(:repository).and_return(repository) + it 'responds with bad request status and error message', :aggregate_failures do + post :create, params: params - expect(repository).to receive(:find_branch).with(branch_name).and_return(branch) - - post :create, params: params - - expect(response).to redirect_to "/-/ide/project/#{namespace.path}/#{project.name}/edit/#{branch_name}/-/.gitlab/dashboards/#{file_name}" + expect(response).to have_gitlab_http_status :bad_request + expect(json_response).to eq('error' => "Request parameter #{param} is missing.") + end + end end - context 'files create service failure' do - it 'redirects back and sets alert' do - allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: false, message: 'something went wrong' })) - allow(controller).to receive(:repository).and_return(repository) - allow(repository).to receive(:find_branch).and_return(branch) + context "param branch_name is missing" do + let(:branch_name) { nil } + it 'responds with bad request status and error message', :aggregate_failures do post :create, params: params - expect(response).to set_flash[:alert].to eq('something went wrong') - expect(response).to redirect_to namespace_project_environments_path + expect(response).to have_gitlab_http_status :bad_request + expect(json_response).to eq('error' => "Request parameter branch is missing.") end end end end - - context 'invalid dashboard template' do - let(:dashboard) { 'config/database.yml' } - - it 'responds 404 not found' do - post :create, params: params - - expect(response).to have_gitlab_http_status :not_found - end - end - - context 'missing commit message' do - before do - params.delete(:commit_message) - end - - it 'use default commit message' do - allow(controller).to receive(:repository).and_return(repository) - allow(repository).to receive(:find_branch).and_return(branch) - dashboard_attrs = { - commit_message: 'Create custom dashboard custom_dashboard.yml', - branch_name: branch_name, - start_branch: 'master', - encoding: 'text', - file_path: ".gitlab/dashboards/custom_dashboard.yml", - file_content: File.read('config/prometheus/common_metrics.yml') - } - - service_instance = instance_double(::Files::CreateService) - expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance) - expect(service_instance).to receive(:execute).and_return(status: :success) - - post :create, params: params - end - end - - context 'missing branch' do - let(:branch_name) { nil } - - it 'raises ActionController::ParameterMissing' do - expect { post :create, params: params }.to raise_error ActionController::ParameterMissing - end - end end context 'without rights to push to repository' do diff --git a/spec/fixtures/sentry/issue_link_sample_response.json b/spec/fixtures/sentry/issue_link_sample_response.json new file mode 100644 index 00000000000..f7f3220e83d --- /dev/null +++ b/spec/fixtures/sentry/issue_link_sample_response.json @@ -0,0 +1,7 @@ +{ + "url": "https://gitlab.com/test/tanuki-inc/issues/3", + "integrationId": 44444, + "displayName": "test/tanuki-inc#3", + "id": 140319, + "key": "gitlab.com/test:test/tanuki-inc#3" +} diff --git a/spec/fixtures/sentry/repos_sample_response.json b/spec/fixtures/sentry/repos_sample_response.json new file mode 100644 index 00000000000..fe389035fe3 --- /dev/null +++ b/spec/fixtures/sentry/repos_sample_response.json @@ -0,0 +1,15 @@ +[ + { + "status": "active", + "integrationId": "48066", + "externalSlug": 139, + "name": "test / tanuki-inc", + "provider": { + "id": "integrations:gitlab", + "name": "Gitlab" + }, + "url": "https://gitlab.com/test/tanuki-inc", + "id": "52480", + "dateCreated": "2020-01-08T21:15:17.181520Z" + } +] diff --git a/spec/frontend/issuable_suggestions/components/app_spec.js b/spec/frontend/issuable_suggestions/components/app_spec.js index 7a2d2df78c4..20930be8667 100644 --- a/spec/frontend/issuable_suggestions/components/app_spec.js +++ b/spec/frontend/issuable_suggestions/components/app_spec.js @@ -11,7 +11,6 @@ describe('Issuable suggestions app component', () => { search, projectPath: 'project', }, - attachToDocument: true, }); } diff --git a/spec/frontend/issuable_suggestions/components/item_spec.js b/spec/frontend/issuable_suggestions/components/item_spec.js index 4c89bb5fa81..6c3c30fcbb0 100644 --- a/spec/frontend/issuable_suggestions/components/item_spec.js +++ b/spec/frontend/issuable_suggestions/components/item_spec.js @@ -16,7 +16,6 @@ describe('Issuable suggestions suggestion component', () => { ...suggestion, }, }, - attachToDocument: true, }); } diff --git a/spec/frontend/issuables_list/components/issuable_spec.js b/spec/frontend/issuables_list/components/issuable_spec.js index b721fe61ace..81f6b60ae25 100644 --- a/spec/frontend/issuables_list/components/issuable_spec.js +++ b/spec/frontend/issuables_list/components/issuable_spec.js @@ -44,7 +44,6 @@ describe('Issuable component', () => { baseUrl: TEST_BASE_URL, ...props, }, - attachToDocument: true, }); }; diff --git a/spec/frontend/issuables_list/components/issuables_list_app_spec.js b/spec/frontend/issuables_list/components/issuables_list_app_spec.js index 3a01dc3a364..eafc4d83d87 100644 --- a/spec/frontend/issuables_list/components/issuables_list_app_spec.js +++ b/spec/frontend/issuables_list/components/issuables_list_app_spec.js @@ -45,7 +45,6 @@ describe('Issuables list component', () => { emptySvgPath: TEST_EMPTY_SVG_PATH, ...props, }, - attachToDocument: true, }); }; diff --git a/spec/frontend/monitoring/components/dashboard_spec.js b/spec/frontend/monitoring/components/dashboard_spec.js index efae8b941ee..030b68bcd5a 100644 --- a/spec/frontend/monitoring/components/dashboard_spec.js +++ b/spec/frontend/monitoring/components/dashboard_spec.js @@ -6,6 +6,8 @@ import axios from '~/lib/utils/axios_utils'; import statusCodes from '~/lib/utils/http_status'; import { metricStates } from '~/monitoring/constants'; import Dashboard from '~/monitoring/components/dashboard.vue'; + +import DashboardsDropdown from '~/monitoring/components/dashboards_dropdown.vue'; import DateTimePicker from '~/monitoring/components/date_time_picker/date_time_picker.vue'; import GroupEmptyState from '~/monitoring/components/group_empty_state.vue'; import { createStore } from '~/monitoring/stores'; @@ -465,7 +467,7 @@ describe('Dashboard', () => { wrapper.vm .$nextTick() .then(() => { - const dashboardDropdown = wrapper.find('.js-dashboards-dropdown'); + const dashboardDropdown = wrapper.find(DashboardsDropdown); expect(dashboardDropdown.exists()).toBe(true); done(); diff --git a/spec/frontend/monitoring/components/dashboards_dropdown_spec.js b/spec/frontend/monitoring/components/dashboards_dropdown_spec.js new file mode 100644 index 00000000000..6af5ab4ba75 --- /dev/null +++ b/spec/frontend/monitoring/components/dashboards_dropdown_spec.js @@ -0,0 +1,249 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlDropdownItem, GlModal, GlLoadingIcon, GlAlert } from '@gitlab/ui'; +import waitForPromises from 'helpers/wait_for_promises'; + +import DashboardsDropdown from '~/monitoring/components/dashboards_dropdown.vue'; +import DuplicateDashboardForm from '~/monitoring/components/duplicate_dashboard_form.vue'; + +import { dashboardGitResponse } from '../mock_data'; + +const defaultBranch = 'master'; + +function createComponent(props, opts = {}) { + const storeOpts = { + methods: { + duplicateSystemDashboard: jest.fn(), + }, + computed: { + allDashboards: () => dashboardGitResponse, + }, + }; + + return shallowMount(DashboardsDropdown, { + propsData: { + ...props, + defaultBranch, + }, + sync: false, + ...storeOpts, + ...opts, + }); +} + +describe('DashboardsDropdown', () => { + let wrapper; + + const findItems = () => wrapper.findAll(GlDropdownItem); + const findItemAt = i => wrapper.findAll(GlDropdownItem).at(i); + + describe('when it receives dashboards data', () => { + beforeEach(() => { + wrapper = createComponent(); + }); + it('displays an item for each dashboard', () => { + expect(wrapper.findAll(GlDropdownItem).length).toEqual(dashboardGitResponse.length); + }); + + it('displays items with the dashboard display name', () => { + expect(findItemAt(0).text()).toBe(dashboardGitResponse[0].display_name); + expect(findItemAt(1).text()).toBe(dashboardGitResponse[1].display_name); + expect(findItemAt(2).text()).toBe(dashboardGitResponse[2].display_name); + }); + }); + + describe('when a system dashboard is selected', () => { + let duplicateDashboardAction; + let modalDirective; + + beforeEach(() => { + modalDirective = jest.fn(); + duplicateDashboardAction = jest.fn().mockResolvedValue(); + + wrapper = createComponent( + { + selectedDashboard: dashboardGitResponse[0], + }, + { + directives: { + GlModal: modalDirective, + }, + methods: { + // Mock vuex actions + duplicateSystemDashboard: duplicateDashboardAction, + }, + }, + ); + + wrapper.vm.$refs.duplicateDashboardModal.hide = jest.fn(); + }); + + it('displays an item for each dashboard plus a "duplicate dashboard" item', () => { + const item = wrapper.findAll({ ref: 'duplicateDashboardItem' }); + + expect(findItems().length).toEqual(dashboardGitResponse.length + 1); + expect(item.length).toBe(1); + }); + + describe('modal form', () => { + let okEvent; + + const findModal = () => wrapper.find(GlModal); + const findAlert = () => wrapper.find(GlAlert); + + beforeEach(() => { + okEvent = { + preventDefault: jest.fn(), + }; + }); + + it('exists and contains a form to duplicate a dashboard', () => { + expect(findModal().exists()).toBe(true); + expect(findModal().contains(DuplicateDashboardForm)).toBe(true); + }); + + it('saves a new dashboard', done => { + findModal().vm.$emit('ok', okEvent); + + waitForPromises() + .then(() => { + expect(okEvent.preventDefault).toHaveBeenCalled(); + + expect(wrapper.find(GlLoadingIcon).exists()).toBe(false); + expect(wrapper.vm.$refs.duplicateDashboardModal.hide).toHaveBeenCalled(); + expect(wrapper.emitted().selectDashboard).toBeTruthy(); + expect(findAlert().exists()).toBe(false); + done(); + }) + .catch(done.fail); + }); + + describe('when a new dashboard is saved succesfully', () => { + const newDashboard = { + can_edit: true, + default: false, + display_name: 'A new dashboard', + system_dashboard: false, + }; + + const submitForm = formVals => { + duplicateDashboardAction.mockResolvedValueOnce(newDashboard); + findModal() + .find(DuplicateDashboardForm) + .vm.$emit('change', { + dashboard: 'common_metrics.yml', + commitMessage: 'A commit message', + ...formVals, + }); + findModal().vm.$emit('ok', okEvent); + }; + + it('to the default branch, redirects to the new dashboard', done => { + submitForm({ + branch: defaultBranch, + }); + + waitForPromises() + .then(() => { + expect(wrapper.emitted().selectDashboard[0][0]).toEqual(newDashboard); + done(); + }) + .catch(done.fail); + }); + + it('to a new branch refreshes in the current dashboard', done => { + submitForm({ + branch: 'another-branch', + }); + + waitForPromises() + .then(() => { + expect(wrapper.emitted().selectDashboard[0][0]).toEqual(dashboardGitResponse[0]); + done(); + }) + .catch(done.fail); + }); + }); + + it('handles error when a new dashboard is not saved', done => { + const errMsg = 'An error occurred'; + + duplicateDashboardAction.mockRejectedValueOnce(errMsg); + findModal().vm.$emit('ok', okEvent); + + waitForPromises() + .then(() => { + expect(okEvent.preventDefault).toHaveBeenCalled(); + + expect(findAlert().exists()).toBe(true); + expect(findAlert().text()).toBe(errMsg); + + expect(wrapper.find(GlLoadingIcon).exists()).toBe(false); + expect(wrapper.vm.$refs.duplicateDashboardModal.hide).not.toHaveBeenCalled(); + + done(); + }) + .catch(done.fail); + }); + + it('id is correct, as the value of modal directive binding matches modal id', () => { + expect(modalDirective).toHaveBeenCalledTimes(1); + + // Binding's second argument contains the modal id + expect(modalDirective.mock.calls[0][1]).toEqual( + expect.objectContaining({ + value: findModal().props('modalId'), + }), + ); + }); + + it('updates the form on changes', () => { + const formVals = { + dashboard: 'common_metrics.yml', + commitMessage: 'A commit message', + }; + + findModal() + .find(DuplicateDashboardForm) + .vm.$emit('change', formVals); + + // Binding's second argument contains the modal id + expect(wrapper.vm.form).toEqual(formVals); + }); + }); + }); + + describe('when a custom dashboard is selected', () => { + const findModal = () => wrapper.find(GlModal); + + beforeEach(() => { + wrapper = createComponent({ + selectedDashboard: dashboardGitResponse[1], + }); + }); + + it('displays an item for each dashboard', () => { + const item = wrapper.findAll({ ref: 'duplicateDashboardItem' }); + + expect(findItems().length).toEqual(dashboardGitResponse.length); + expect(item.length).toBe(0); + }); + + it('modal form does not exist and contains a form to duplicate a dashboard', () => { + expect(findModal().exists()).toBe(false); + }); + }); + + describe('when a dashboard gets selected by the user', () => { + beforeEach(() => { + wrapper = createComponent(); + findItemAt(1).vm.$emit('click'); + }); + + it('emits a "selectDashboard" event', () => { + expect(wrapper.emitted().selectDashboard).toBeTruthy(); + }); + it('emits a "selectDashboard" event with dashboard information', () => { + expect(wrapper.emitted().selectDashboard[0]).toEqual([dashboardGitResponse[1]]); + }); + }); +}); diff --git a/spec/frontend/monitoring/components/duplicate_dashboard_form_spec.js b/spec/frontend/monitoring/components/duplicate_dashboard_form_spec.js new file mode 100644 index 00000000000..75a488b5c7b --- /dev/null +++ b/spec/frontend/monitoring/components/duplicate_dashboard_form_spec.js @@ -0,0 +1,153 @@ +import { mount } from '@vue/test-utils'; +import DuplicateDashboardForm from '~/monitoring/components/duplicate_dashboard_form.vue'; + +import { dashboardGitResponse } from '../mock_data'; + +describe('DuplicateDashboardForm', () => { + let wrapper; + + const defaultBranch = 'master'; + + const findByRef = ref => wrapper.find({ ref }); + const setValue = (ref, val) => { + findByRef(ref).setValue(val); + }; + const setChecked = value => { + const input = wrapper.find(`.form-check-input[value="${value}"]`); + input.element.checked = true; + input.trigger('click'); + input.trigger('change'); + }; + + beforeEach(() => { + // Use `mount` to render native input elements + wrapper = mount(DuplicateDashboardForm, { + propsData: { + dashboard: dashboardGitResponse[0], + defaultBranch, + }, + sync: false, + }); + }); + + it('renders correctly', () => { + expect(wrapper.exists()).toEqual(true); + }); + + it('renders form elements', () => { + expect(findByRef('fileName').exists()).toEqual(true); + expect(findByRef('branchName').exists()).toEqual(true); + expect(findByRef('branchOption').exists()).toEqual(true); + expect(findByRef('commitMessage').exists()).toEqual(true); + }); + + describe('validates the file name', () => { + const findInvalidFeedback = () => findByRef('fileNameFormGroup').find('.invalid-feedback'); + + it('when is empty', done => { + setValue('fileName', ''); + wrapper.vm.$nextTick(() => { + expect(findByRef('fileNameFormGroup').is('.is-valid')).toBe(true); + expect(findInvalidFeedback().exists()).toBe(false); + done(); + }); + }); + + it('when is valid', done => { + setValue('fileName', 'my_dashboard.yml'); + wrapper.vm.$nextTick(() => { + expect(findByRef('fileNameFormGroup').is('.is-valid')).toBe(true); + expect(findInvalidFeedback().exists()).toBe(false); + done(); + }); + }); + + it('when is not valid', done => { + setValue('fileName', 'my_dashboard.exe'); + wrapper.vm.$nextTick(() => { + expect(findByRef('fileNameFormGroup').is('.is-invalid')).toBe(true); + expect(findInvalidFeedback().text()).toBeTruthy(); + done(); + }); + }); + }); + + describe('emits `change` event', () => { + const lastChange = () => + wrapper.vm.$nextTick().then(() => { + wrapper.find('form').trigger('change'); + + // Resolves to the last emitted change + const changes = wrapper.emitted().change; + return changes[changes.length - 1][0]; + }); + + it('with the inital form values', () => { + expect(wrapper.emitted().change).toHaveLength(1); + expect(lastChange()).resolves.toEqual({ + branch: '', + commitMessage: expect.any(String), + dashboard: dashboardGitResponse[0].path, + fileName: 'common_metrics.yml', + }); + }); + + it('containing an inputted file name', () => { + setValue('fileName', 'my_dashboard.yml'); + + expect(lastChange()).resolves.toMatchObject({ + fileName: 'my_dashboard.yml', + }); + }); + + it('containing a default commit message when no message is set', () => { + setValue('commitMessage', ''); + + expect(lastChange()).resolves.toMatchObject({ + commitMessage: expect.stringContaining('Create custom dashboard'), + }); + }); + + it('containing an inputted commit message', () => { + setValue('commitMessage', 'My commit message'); + + expect(lastChange()).resolves.toMatchObject({ + commitMessage: expect.stringContaining('My commit message'), + }); + }); + + it('containing an inputted branch name', () => { + setValue('branchName', 'a-new-branch'); + + expect(lastChange()).resolves.toMatchObject({ + branch: 'a-new-branch', + }); + }); + + it('when a `default` branch option is set, branch input is invisible and ignored', done => { + setChecked(wrapper.vm.$options.radioVals.DEFAULT); + setValue('branchName', 'a-new-branch'); + + expect(lastChange()).resolves.toMatchObject({ + branch: defaultBranch, + }); + wrapper.vm.$nextTick(() => { + expect(findByRef('branchName').isVisible()).toBe(false); + done(); + }); + }); + + it('when `new` branch option is chosen, focuses on the branch name input', done => { + setChecked(wrapper.vm.$options.radioVals.NEW); + + wrapper.vm + .$nextTick() + .then(() => { + wrapper.find('form').trigger('change'); + expect(findByRef('branchName').is(':focus')).toBe(true); + }) + .then(done) + .catch(done.fail); + }); + }); +}); diff --git a/spec/frontend/monitoring/init_utils.js b/spec/frontend/monitoring/init_utils.js index 10db8b902b5..5f229cb6ee5 100644 --- a/spec/frontend/monitoring/init_utils.js +++ b/spec/frontend/monitoring/init_utils.js @@ -15,6 +15,7 @@ export const propsData = { clustersPath: '/path/to/clusters', tagsPath: '/path/to/tags', projectPath: '/path/to/project', + defaultBranch: 'master', metricsEndpoint: mockApiEndpoint, deploymentsEndpoint: null, emptyGettingStartedSvgPath: '/path/to/getting-started.svg', diff --git a/spec/frontend/monitoring/mock_data.js b/spec/frontend/monitoring/mock_data.js index 77c92d0eca6..8ed0e232775 100644 --- a/spec/frontend/monitoring/mock_data.js +++ b/spec/frontend/monitoring/mock_data.js @@ -522,6 +522,7 @@ export const dashboardGitResponse = [ default: true, display_name: 'Default', can_edit: false, + system_dashboard: true, project_blob_path: null, path: 'config/prometheus/common_metrics.yml', }, @@ -529,6 +530,7 @@ export const dashboardGitResponse = [ default: false, display_name: 'Custom Dashboard 1', can_edit: true, + system_dashboard: false, project_blob_path: `${mockProjectDir}/blob/master/dashboards/.gitlab/dashboards/dashboard_1.yml`, path: '.gitlab/dashboards/dashboard_1.yml', }, @@ -536,6 +538,7 @@ export const dashboardGitResponse = [ default: false, display_name: 'Custom Dashboard 2', can_edit: true, + system_dashboard: false, project_blob_path: `${mockProjectDir}/blob/master/dashboards/.gitlab/dashboards/dashboard_2.yml`, path: '.gitlab/dashboards/dashboard_2.yml', }, diff --git a/spec/frontend/monitoring/store/actions_spec.js b/spec/frontend/monitoring/store/actions_spec.js index c1ad59ac95b..975bdd3a27a 100644 --- a/spec/frontend/monitoring/store/actions_spec.js +++ b/spec/frontend/monitoring/store/actions_spec.js @@ -18,6 +18,7 @@ import { fetchPrometheusMetric, setEndpoints, setGettingStartedEmptyState, + duplicateSystemDashboard, } from '~/monitoring/stores/actions'; import storeState from '~/monitoring/stores/state'; import { @@ -544,4 +545,85 @@ describe('Monitoring store actions', () => { }); }); }); + + describe('duplicateSystemDashboard', () => { + let state; + + beforeEach(() => { + state = storeState(); + state.dashboardsEndpoint = '/dashboards.json'; + }); + + it('Succesful POST request resolves', done => { + mock.onPost(state.dashboardsEndpoint).reply(statusCodes.CREATED, { + dashboard: dashboardGitResponse[1], + }); + + testAction(duplicateSystemDashboard, {}, state, [], []) + .then(() => { + expect(mock.history.post).toHaveLength(1); + done(); + }) + .catch(done.fail); + }); + + it('Succesful POST request resolves to a dashboard', done => { + const mockCreatedDashboard = dashboardGitResponse[1]; + + const params = { + dashboard: 'my-dashboard', + fileName: 'file-name.yml', + branch: 'my-new-branch', + commitMessage: 'A new commit message', + }; + + const expectedPayload = JSON.stringify({ + dashboard: 'my-dashboard', + file_name: 'file-name.yml', + branch: 'my-new-branch', + commit_message: 'A new commit message', + }); + + mock.onPost(state.dashboardsEndpoint).reply(statusCodes.CREATED, { + dashboard: mockCreatedDashboard, + }); + + testAction(duplicateSystemDashboard, params, state, [], []) + .then(result => { + expect(mock.history.post).toHaveLength(1); + expect(mock.history.post[0].data).toEqual(expectedPayload); + expect(result).toEqual(mockCreatedDashboard); + + done(); + }) + .catch(done.fail); + }); + + it('Failed POST request throws an error', done => { + mock.onPost(state.dashboardsEndpoint).reply(statusCodes.BAD_REQUEST); + + testAction(duplicateSystemDashboard, {}, state, [], []).catch(err => { + expect(mock.history.post).toHaveLength(1); + expect(err).toEqual(expect.any(String)); + + done(); + }); + }); + + it('Failed POST request throws an error with a description', done => { + const backendErrorMsg = 'This file already exists!'; + + mock.onPost(state.dashboardsEndpoint).reply(statusCodes.BAD_REQUEST, { + error: backendErrorMsg, + }); + + testAction(duplicateSystemDashboard, {}, state, [], []).catch(err => { + expect(mock.history.post).toHaveLength(1); + expect(err).toEqual(expect.any(String)); + expect(err).toEqual(expect.stringContaining(backendErrorMsg)); + + done(); + }); + }); + }); }); diff --git a/spec/frontend/vue_mr_widget/deployment/deployment_spec.js b/spec/frontend/vue_mr_widget/deployment/deployment_spec.js index 78e086e473d..2902c8280dd 100644 --- a/spec/frontend/vue_mr_widget/deployment/deployment_spec.js +++ b/spec/frontend/vue_mr_widget/deployment/deployment_spec.js @@ -134,7 +134,7 @@ describe('Deployment component', () => { if (status === SUCCESS) { expect(wrapper.find(DeploymentViewButton).text()).toContain('View app'); } else { - expect(wrapper.find(DeploymentViewButton).text()).toContain('View previous app'); + expect(wrapper.find(DeploymentViewButton).text()).toContain('View latest app'); } }); } diff --git a/spec/frontend/vue_mr_widget/deployment/deployment_view_button_spec.js b/spec/frontend/vue_mr_widget/deployment/deployment_view_button_spec.js index b48c97341b2..5e0f38459b0 100644 --- a/spec/frontend/vue_mr_widget/deployment/deployment_view_button_spec.js +++ b/spec/frontend/vue_mr_widget/deployment/deployment_view_button_spec.js @@ -3,6 +3,11 @@ import DeploymentViewButton from '~/vue_merge_request_widget/components/deployme import ReviewAppLink from '~/vue_merge_request_widget/components/review_app_link.vue'; import deploymentMockData from './deployment_mock_data'; +const appButtonText = { + text: 'View app', + tooltip: 'View the latest successful deployment to this environment', +}; + describe('Deployment View App button', () => { let wrapper; @@ -16,7 +21,7 @@ describe('Deployment View App button', () => { factory({ propsData: { deployment: deploymentMockData, - isCurrent: true, + appButtonText, }, }); }); @@ -26,25 +31,8 @@ describe('Deployment View App button', () => { }); describe('text', () => { - describe('when app is current', () => { - it('shows View app', () => { - expect(wrapper.find(ReviewAppLink).text()).toContain('View app'); - }); - }); - - describe('when app is not current', () => { - beforeEach(() => { - factory({ - propsData: { - deployment: deploymentMockData, - isCurrent: false, - }, - }); - }); - - it('shows View Previous app', () => { - expect(wrapper.find(ReviewAppLink).text()).toContain('View previous app'); - }); + it('renders text as passed', () => { + expect(wrapper.find(ReviewAppLink).text()).toContain(appButtonText.text); }); }); @@ -53,7 +41,7 @@ describe('Deployment View App button', () => { factory({ propsData: { deployment: { ...deploymentMockData, changes: null }, - isCurrent: false, + appButtonText, }, }); }); @@ -68,7 +56,7 @@ describe('Deployment View App button', () => { factory({ propsData: { deployment: { ...deploymentMockData, changes: [deploymentMockData.changes[0]] }, - isCurrent: false, + appButtonText, }, }); }); @@ -91,7 +79,7 @@ describe('Deployment View App button', () => { factory({ propsData: { deployment: deploymentMockData, - isCurrent: false, + appButtonText, }, }); }); diff --git a/spec/frontend/vue_shared/components/changed_file_icon_spec.js b/spec/frontend/vue_shared/components/changed_file_icon_spec.js index 3a52941a06e..02c4dabeffc 100644 --- a/spec/frontend/vue_shared/components/changed_file_icon_spec.js +++ b/spec/frontend/vue_shared/components/changed_file_icon_spec.js @@ -18,7 +18,6 @@ describe('Changed file icon', () => { showTooltip: true, ...props, }, - attachToDocument: true, }); }; diff --git a/spec/frontend/vue_shared/components/clipboard_button_spec.js b/spec/frontend/vue_shared/components/clipboard_button_spec.js index 233088b8d32..37f71867ab9 100644 --- a/spec/frontend/vue_shared/components/clipboard_button_spec.js +++ b/spec/frontend/vue_shared/components/clipboard_button_spec.js @@ -9,7 +9,6 @@ describe('clipboard button', () => { const createWrapper = propsData => { wrapper = shallowMount(ClipboardButton, { propsData, - attachToDocument: true, }); }; diff --git a/spec/frontend/vue_shared/components/commit_spec.js b/spec/frontend/vue_shared/components/commit_spec.js index 81c67b30a11..3510c9b699d 100644 --- a/spec/frontend/vue_shared/components/commit_spec.js +++ b/spec/frontend/vue_shared/components/commit_spec.js @@ -17,7 +17,6 @@ describe('Commit component', () => { const createComponent = propsData => { wrapper = shallowMount(CommitComponent, { propsData, - attachToDocument: true, }); }; diff --git a/spec/frontend/vue_shared/components/issue/issue_assignees_spec.js b/spec/frontend/vue_shared/components/issue/issue_assignees_spec.js index 00245c68342..b00261ae067 100644 --- a/spec/frontend/vue_shared/components/issue/issue_assignees_spec.js +++ b/spec/frontend/vue_shared/components/issue/issue_assignees_spec.js @@ -17,7 +17,6 @@ describe('IssueAssigneesComponent', () => { assignees: mockAssigneesList, ...props, }, - attachToDocument: true, }); vm = wrapper.vm; // eslint-disable-line }; diff --git a/spec/frontend/vue_shared/components/issue/issue_milestone_spec.js b/spec/frontend/vue_shared/components/issue/issue_milestone_spec.js index ef752743fa9..4c654e01f74 100644 --- a/spec/frontend/vue_shared/components/issue/issue_milestone_spec.js +++ b/spec/frontend/vue_shared/components/issue/issue_milestone_spec.js @@ -13,7 +13,6 @@ const createComponent = (milestone = mockMilestone) => { propsData: { milestone, }, - attachToDocument: true, }); }; diff --git a/spec/frontend/vue_shared/components/issue/related_issuable_item_spec.js b/spec/frontend/vue_shared/components/issue/related_issuable_item_spec.js index e895fe27095..f7b1f041ef2 100644 --- a/spec/frontend/vue_shared/components/issue/related_issuable_item_spec.js +++ b/spec/frontend/vue_shared/components/issue/related_issuable_item_spec.js @@ -31,7 +31,6 @@ describe('RelatedIssuableItem', () => { beforeEach(() => { wrapper = mount(RelatedIssuableItem, { slots, - attachToDocument: true, propsData: props, }); }); diff --git a/spec/frontend/vue_shared/components/markdown/header_spec.js b/spec/frontend/vue_shared/components/markdown/header_spec.js index 8eec48749c9..551d781d296 100644 --- a/spec/frontend/vue_shared/components/markdown/header_spec.js +++ b/spec/frontend/vue_shared/components/markdown/header_spec.js @@ -12,7 +12,6 @@ describe('Markdown field header component', () => { previewMarkdown: false, ...props, }, - attachToDocument: true, }); }; diff --git a/spec/frontend/vue_shared/components/markdown/suggestion_diff_header_spec.js b/spec/frontend/vue_shared/components/markdown/suggestion_diff_header_spec.js index 0450166a468..9b9c3d559e3 100644 --- a/spec/frontend/vue_shared/components/markdown/suggestion_diff_header_spec.js +++ b/spec/frontend/vue_shared/components/markdown/suggestion_diff_header_spec.js @@ -17,7 +17,6 @@ describe('Suggestion Diff component', () => { ...DEFAULT_PROPS, ...props, }, - attachToDocument: true, }); }; diff --git a/spec/frontend/vue_shared/components/modal_copy_button_spec.js b/spec/frontend/vue_shared/components/modal_copy_button_spec.js index 1c048560212..e5a8860f42e 100644 --- a/spec/frontend/vue_shared/components/modal_copy_button_spec.js +++ b/spec/frontend/vue_shared/components/modal_copy_button_spec.js @@ -16,7 +16,6 @@ describe('modal copy button', () => { text: 'copy me', title: 'Copy this value', }, - attachToDocument: true, }); }); diff --git a/spec/frontend/vue_shared/components/notes/system_note_spec.js b/spec/frontend/vue_shared/components/notes/system_note_spec.js index 91b68ce1c6f..d5eac7c2aa3 100644 --- a/spec/frontend/vue_shared/components/notes/system_note_spec.js +++ b/spec/frontend/vue_shared/components/notes/system_note_spec.js @@ -33,7 +33,6 @@ describe('system note component', () => { vm = mount(IssueSystemNote, { store, propsData: props, - attachToDocument: true, }); }); diff --git a/spec/frontend/vue_shared/components/paginated_list_spec.js b/spec/frontend/vue_shared/components/paginated_list_spec.js index 3b064410274..46e45296c37 100644 --- a/spec/frontend/vue_shared/components/paginated_list_spec.js +++ b/spec/frontend/vue_shared/components/paginated_list_spec.js @@ -26,7 +26,6 @@ describe('Pagination links component', () => { list: [{ id: 'foo' }, { id: 'bar' }], props, }, - attachToDocument: true, }); [glPaginatedList] = wrapper.vm.$children; diff --git a/spec/frontend/vue_shared/components/resizable_chart_container_spec.js b/spec/frontend/vue_shared/components/resizable_chart_container_spec.js index 552cfade7b6..3a5514ef318 100644 --- a/spec/frontend/vue_shared/components/resizable_chart_container_spec.js +++ b/spec/frontend/vue_shared/components/resizable_chart_container_spec.js @@ -14,7 +14,6 @@ describe('Resizable Chart Container', () => { beforeEach(() => { wrapper = mount(ResizableChartContainer, { - attachToDocument: true, scopedSlots: { default: ` <div class="slot" slot-scope="{ width, height }"> diff --git a/spec/frontend/vue_shared/components/sidebar/labels_select/base_spec.js b/spec/frontend/vue_shared/components/sidebar/labels_select/base_spec.js index 7dd1be24360..d90fafb6bf7 100644 --- a/spec/frontend/vue_shared/components/sidebar/labels_select/base_spec.js +++ b/spec/frontend/vue_shared/components/sidebar/labels_select/base_spec.js @@ -12,7 +12,6 @@ import { const createComponent = (config = mockConfig) => shallowMount(BaseComponent, { propsData: config, - attachToDocument: true, }); describe('BaseComponent', () => { diff --git a/spec/frontend/vue_shared/components/sidebar/labels_select/dropdown_value_spec.js b/spec/frontend/vue_shared/components/sidebar/labels_select/dropdown_value_spec.js index 76f6ff96f82..54ad96073c8 100644 --- a/spec/frontend/vue_shared/components/sidebar/labels_select/dropdown_value_spec.js +++ b/spec/frontend/vue_shared/components/sidebar/labels_select/dropdown_value_spec.js @@ -24,7 +24,6 @@ const createComponent = ( labelFilterBasePath, enableScopedLabels: true, }, - attachToDocument: true, }); }; diff --git a/spec/frontend/vue_shared/components/time_ago_tooltip_spec.js b/spec/frontend/vue_shared/components/time_ago_tooltip_spec.js index a1db72c9c73..46fcb92455b 100644 --- a/spec/frontend/vue_shared/components/time_ago_tooltip_spec.js +++ b/spec/frontend/vue_shared/components/time_ago_tooltip_spec.js @@ -7,7 +7,6 @@ describe('Time ago with tooltip component', () => { const buildVm = (propsData = {}) => { vm = shallowMount(TimeAgoTooltip, { - attachToDocument: true, propsData, }); }; diff --git a/spec/frontend/vue_shared/components/user_avatar/user_avatar_link_spec.js b/spec/frontend/vue_shared/components/user_avatar/user_avatar_link_spec.js index 2bbbab17bce..2f68e15b0d7 100644 --- a/spec/frontend/vue_shared/components/user_avatar/user_avatar_link_spec.js +++ b/spec/frontend/vue_shared/components/user_avatar/user_avatar_link_spec.js @@ -26,7 +26,6 @@ describe('User Avatar Link Component', () => { ...defaultProps, ...props, }, - attachToDocument: true, }); }; diff --git a/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js b/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js index b1c9f8b505b..a8bbc80d2df 100644 --- a/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js +++ b/spec/frontend/vue_shared/components/user_popover/user_popover_spec.js @@ -59,7 +59,6 @@ describe('User Popover Component', () => { status: null, }, }, - attachToDocument: true, }, ); }); diff --git a/spec/graphql/types/environment_type_spec.rb b/spec/graphql/types/environment_type_spec.rb new file mode 100644 index 00000000000..cf30893b3ca --- /dev/null +++ b/spec/graphql/types/environment_type_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe GitlabSchema.types['Environment'] do + it { expect(described_class.graphql_name).to eq('Environment') } + + it 'has the expected fields' do + expected_fields = %w[ + name id + ] + + is_expected.to have_graphql_fields(*expected_fields) + end + + it { is_expected.to require_graphql_authorizations(:read_environment) } +end diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb index 7bed1f72e0b..ac2d2d6f7f0 100644 --- a/spec/graphql/types/project_type_spec.rb +++ b/spec/graphql/types/project_type_spec.rb @@ -23,7 +23,7 @@ describe GitlabSchema.types['Project'] do only_allow_merge_if_all_discussions_are_resolved printing_merge_request_link_enabled namespace group statistics repository merge_requests merge_request issues issue pipelines removeSourceBranchAfterMerge sentryDetailedError snippets - grafanaIntegration autocloseReferencedIssues suggestion_commit_message + grafanaIntegration autocloseReferencedIssues suggestion_commit_message environments ] is_expected.to include_graphql_fields(*expected_fields) @@ -70,4 +70,11 @@ describe GitlabSchema.types['Project'] do it { is_expected.to have_graphql_type(Types::GrafanaIntegrationType) } it { is_expected.to have_graphql_resolver(Resolvers::Projects::GrafanaIntegrationResolver) } end + + describe 'environments field' do + subject { described_class.fields['environments'] } + + it { is_expected.to have_graphql_type(Types::EnvironmentType.connection_type) } + it { is_expected.to have_graphql_resolver(Resolvers::EnvironmentsResolver) } + end end diff --git a/spec/helpers/environments_helper_spec.rb b/spec/helpers/environments_helper_spec.rb index a50c8e9bf8e..b7a6cd4db74 100644 --- a/spec/helpers/environments_helper_spec.rb +++ b/spec/helpers/environments_helper_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' describe EnvironmentsHelper do - set(:environment) { create(:environment) } - set(:project) { environment.project } set(:user) { create(:user) } + set(:project) { create(:project, :repository) } + set(:environment) { create(:environment, project: project) } describe '#metrics_data' do before do @@ -28,6 +28,7 @@ describe EnvironmentsHelper do 'empty-unable-to-connect-svg-path' => match_asset_path('/assets/illustrations/monitoring/unable_to_connect.svg'), 'metrics-endpoint' => additional_metrics_project_environment_path(project, environment, format: :json), 'deployments-endpoint' => project_environment_deployments_path(project, environment, format: :json), + 'default-branch' => 'master', 'environments-endpoint': project_environments_path(project, format: :json), 'project-path' => project_path(project), 'tags-path' => project_tags_path(project), diff --git a/spec/javascripts/monitoring/components/dashboard_resize_spec.js b/spec/javascripts/monitoring/components/dashboard_resize_spec.js index 4eab398e3ab..46a6679da18 100644 --- a/spec/javascripts/monitoring/components/dashboard_resize_spec.js +++ b/spec/javascripts/monitoring/components/dashboard_resize_spec.js @@ -22,6 +22,7 @@ const propsData = { clustersPath: '/path/to/clusters', tagsPath: '/path/to/tags', projectPath: '/path/to/project', + defaultBranch: 'master', metricsEndpoint: mockApiEndpoint, deploymentsEndpoint: null, emptyGettingStartedSvgPath: '/path/to/getting-started.svg', diff --git a/spec/javascripts/vue_mr_widget/components/review_app_link_spec.js b/spec/javascripts/vue_mr_widget/components/review_app_link_spec.js index bd481f93413..242193c7b3d 100644 --- a/spec/javascripts/vue_mr_widget/components/review_app_link_spec.js +++ b/spec/javascripts/vue_mr_widget/components/review_app_link_spec.js @@ -8,7 +8,10 @@ describe('review app link', () => { const props = { link: '/review', cssClass: 'js-link', - isCurrent: true, + display: { + text: 'View app', + tooltip: '', + }, }; let vm; let el; diff --git a/spec/lib/gitlab/danger/changelog_spec.rb b/spec/lib/gitlab/danger/changelog_spec.rb index 689957993ec..64f87ec8cd3 100644 --- a/spec/lib/gitlab/danger/changelog_spec.rb +++ b/spec/lib/gitlab/danger/changelog_spec.rb @@ -106,18 +106,6 @@ describe Gitlab::Danger::Changelog do end end - describe '#sanitized_mr_title' do - subject { changelog.sanitized_mr_title } - - [ - 'WIP: My MR title', - 'My MR title' - ].each do |mr_title| - let(:mr_json) { { "title" => mr_title } } - it { is_expected.to eq("My MR title") } - end - end - describe '#ee_changelog?' do context 'is ee changelog' do [ diff --git a/spec/lib/gitlab/danger/commit_linter_spec.rb b/spec/lib/gitlab/danger/commit_linter_spec.rb new file mode 100644 index 00000000000..0cf7ac64e43 --- /dev/null +++ b/spec/lib/gitlab/danger/commit_linter_spec.rb @@ -0,0 +1,315 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rspec-parameterized' +require_relative 'danger_spec_helper' + +require 'gitlab/danger/commit_linter' + +describe Gitlab::Danger::CommitLinter do + using RSpec::Parameterized::TableSyntax + + let(:total_files_changed) { 2 } + let(:total_lines_changed) { 10 } + let(:stats) { { total: { files: total_files_changed, lines: total_lines_changed } } } + let(:diff_parent) { Struct.new(:stats).new(stats) } + let(:commit_class) do + Struct.new(:message, :sha, :diff_parent) + end + let(:commit_message) { 'A commit message' } + let(:commit_sha) { 'abcd1234' } + let(:commit) { commit_class.new(commit_message, commit_sha, diff_parent) } + + subject(:commit_linter) { described_class.new(commit) } + + describe '#fixup?' do + where(:commit_message, :is_fixup) do + 'A commit message' | false + 'fixup!' | true + 'fixup! A commit message' | true + 'squash!' | true + 'squash! A commit message' | true + end + + with_them do + it 'is true when commit message starts with "fixup!" or "squash!"' do + expect(commit_linter.fixup?).to be(is_fixup) + end + end + end + + describe '#suggestion?' do + where(:commit_message, :is_suggestion) do + 'A commit message' | false + 'Apply suggestion to' | true + 'Apply suggestion to "A commit message"' | true + end + + with_them do + it 'is true when commit message starts with "Apply suggestion to"' do + expect(commit_linter.suggestion?).to be(is_suggestion) + end + end + end + + describe '#merge?' do + where(:commit_message, :is_merge) do + 'A commit message' | false + 'Merge branch' | true + 'Merge branch "A commit message"' | true + end + + with_them do + it 'is true when commit message starts with "Merge branch"' do + expect(commit_linter.merge?).to be(is_merge) + end + end + end + + describe '#revert?' do + where(:commit_message, :is_revert) do + 'A commit message' | false + 'Revert' | false + 'Revert "' | true + 'Revert "A commit message"' | true + end + + with_them do + it 'is true when commit message starts with "Revert \""' do + expect(commit_linter.revert?).to be(is_revert) + end + end + end + + describe '#multi_line?' do + where(:commit_message, :is_multi_line) do + "A commit message" | false + "A commit message\n" | false + "A commit message\n\n" | false + "A commit message\n\nWith details" | true + end + + with_them do + it 'is true when commit message contains details' do + expect(commit_linter.multi_line?).to be(is_multi_line) + end + end + end + + describe '#failed?' do + context 'with no failures' do + it { expect(commit_linter).not_to be_failed } + end + + context 'with failures' do + before do + commit_linter.add_problem(:details_line_too_long) + end + + it { expect(commit_linter).to be_failed } + end + end + + describe '#add_problem' do + it 'stores messages in #failures' do + commit_linter.add_problem(:details_line_too_long) + + expect(commit_linter.problems).to eq({ details_line_too_long: described_class::PROBLEMS[:details_line_too_long] }) + end + end + + shared_examples 'a valid commit' do + it 'does not have any problem' do + commit_linter.lint + + expect(commit_linter.problems).to be_empty + end + end + + describe '#lint' do + describe 'subject' do + context 'when subject valid' do + it_behaves_like 'a valid commit' + end + + context 'when subject is too short' do + let(:commit_message) { 'A B' } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:subject_too_short, described_class::DEFAULT_SUBJECT_DESCRIPTION) + + commit_linter.lint + end + end + + context 'when subject is too long' do + let(:commit_message) { 'A B ' + 'C' * described_class::MAX_LINE_LENGTH } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:subject_too_long, described_class::DEFAULT_SUBJECT_DESCRIPTION) + + commit_linter.lint + end + end + + context 'when subject is too short and too long' do + let(:commit_message) { 'A ' + 'B' * described_class::MAX_LINE_LENGTH } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:subject_too_short, described_class::DEFAULT_SUBJECT_DESCRIPTION) + expect(commit_linter).to receive(:add_problem).with(:subject_too_long, described_class::DEFAULT_SUBJECT_DESCRIPTION) + + commit_linter.lint + end + end + + context 'when subject is above warning' do + let(:commit_message) { 'A B ' + 'C' * described_class::WARN_SUBJECT_LENGTH } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:subject_above_warning, described_class::DEFAULT_SUBJECT_DESCRIPTION) + + commit_linter.lint + end + end + + context 'when subject starts with lowercase' do + let(:commit_message) { 'a B C' } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:subject_starts_with_lowercase, described_class::DEFAULT_SUBJECT_DESCRIPTION) + + commit_linter.lint + end + end + + context 'when subject ands with a period' do + let(:commit_message) { 'A B C.' } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:subject_ends_with_a_period, described_class::DEFAULT_SUBJECT_DESCRIPTION) + + commit_linter.lint + end + end + end + + describe 'separator' do + context 'when separator is missing' do + let(:commit_message) { "A B C\n" } + + it_behaves_like 'a valid commit' + end + + context 'when separator is a blank line' do + let(:commit_message) { "A B C\n\nMore details." } + + it_behaves_like 'a valid commit' + end + + context 'when separator is missing' do + let(:commit_message) { "A B C\nMore details." } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:separator_missing) + + commit_linter.lint + end + end + end + + describe 'details' do + context 'when details are valid' do + let(:commit_message) { "A B C\n\nMore details." } + + it_behaves_like 'a valid commit' + end + + context 'when no details are given and many files are changed' do + let(:total_files_changed) { described_class::MAX_CHANGED_FILES_IN_COMMIT + 1 } + + it_behaves_like 'a valid commit' + end + + context 'when no details are given and many lines are changed' do + let(:total_lines_changed) { described_class::MAX_CHANGED_LINES_IN_COMMIT + 1 } + + it_behaves_like 'a valid commit' + end + + context 'when no details are given and many files and lines are changed' do + let(:total_files_changed) { described_class::MAX_CHANGED_FILES_IN_COMMIT + 1 } + let(:total_lines_changed) { described_class::MAX_CHANGED_LINES_IN_COMMIT + 1 } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:details_too_many_changes) + + commit_linter.lint + end + end + + context 'when details exceeds the max line length' do + let(:commit_message) { "A B C\n\n" + 'D' * (described_class::MAX_LINE_LENGTH + 1) } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:details_line_too_long) + + commit_linter.lint + end + end + + context 'when details exceeds the max line length including a URL' do + let(:commit_message) { "A B C\n\nhttps://gitlab.com" + 'D' * described_class::MAX_LINE_LENGTH } + + it_behaves_like 'a valid commit' + end + end + + describe 'message' do + context 'when message includes a text emoji' do + let(:commit_message) { "A commit message :+1:" } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:message_contains_text_emoji) + + commit_linter.lint + end + end + + context 'when message includes a unicode emoji' do + let(:commit_message) { "A commit message 🚀" } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:message_contains_unicode_emoji) + + commit_linter.lint + end + end + + context 'when message includes a short reference' do + [ + 'A commit message to fix #1234', + 'A commit message to fix !1234', + 'A commit message to fix &1234', + 'A commit message to fix %1234', + 'A commit message to fix gitlab#1234', + 'A commit message to fix gitlab!1234', + 'A commit message to fix gitlab&1234', + 'A commit message to fix gitlab%1234', + 'A commit message to fix gitlab-org/gitlab#1234', + 'A commit message to fix gitlab-org/gitlab!1234', + 'A commit message to fix gitlab-org/gitlab&1234', + 'A commit message to fix gitlab-org/gitlab%1234' + ].each do |message| + let(:commit_message) { message } + + it 'adds a problem' do + expect(commit_linter).to receive(:add_problem).with(:message_contains_short_reference) + + commit_linter.lint + end + end + end + end + end +end diff --git a/spec/lib/gitlab/danger/emoji_checker_spec.rb b/spec/lib/gitlab/danger/emoji_checker_spec.rb new file mode 100644 index 00000000000..0cdc18ce626 --- /dev/null +++ b/spec/lib/gitlab/danger/emoji_checker_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'rspec-parameterized' + +require 'gitlab/danger/emoji_checker' + +describe Gitlab::Danger::EmojiChecker do + using RSpec::Parameterized::TableSyntax + + describe '#includes_text_emoji?' do + where(:text, :includes_emoji) do + 'Hello World!' | false + ':+1:' | true + 'Hello World! :+1:' | true + end + + with_them do + it 'is true when text includes a text emoji' do + expect(subject.includes_text_emoji?(text)).to be(includes_emoji) + end + end + end + + describe '#includes_unicode_emoji?' do + where(:text, :includes_emoji) do + 'Hello World!' | false + '🚀' | true + 'Hello World! 🚀' | true + end + + with_them do + it 'is true when text includes a text emoji' do + expect(subject.includes_unicode_emoji?(text)).to be(includes_emoji) + end + end + end +end diff --git a/spec/lib/gitlab/danger/helper_spec.rb b/spec/lib/gitlab/danger/helper_spec.rb index edcd020a10f..ae0fcf443c5 100644 --- a/spec/lib/gitlab/danger/helper_spec.rb +++ b/spec/lib/gitlab/danger/helper_spec.rb @@ -313,6 +313,19 @@ describe Gitlab::Danger::Helper do end end + describe '#sanitize_mr_title' do + where(:mr_title, :expected_mr_title) do + 'My MR title' | 'My MR title' + 'WIP: My MR title' | 'My MR title' + end + + with_them do + subject { helper.sanitize_mr_title(mr_title) } + + it { is_expected.to eq(expected_mr_title) } + end + end + describe '#security_mr?' do it 'returns false when `gitlab_helper` is unavailable' do expect(helper).to receive(:gitlab_helper).and_return(nil) diff --git a/spec/lib/gitlab/data_builder/note_spec.rb b/spec/lib/gitlab/data_builder/note_spec.rb index 3c26daba5a5..4b799c23de8 100644 --- a/spec/lib/gitlab/data_builder/note_spec.rb +++ b/spec/lib/gitlab/data_builder/note_spec.rb @@ -137,7 +137,7 @@ describe Gitlab::DataBuilder::Note do it 'returns the note and project snippet data' do expect(data).to have_key(:snippet) expect(data[:snippet].except('updated_at')) - .to eq(snippet.reload.hook_attrs.except('updated_at')) + .to eq(snippet.hook_attrs.except('updated_at')) expect(data[:snippet]['updated_at']) .to be >= snippet.hook_attrs['updated_at'] end diff --git a/spec/lib/gitlab/import_export/import_test_coverage_spec.rb b/spec/lib/gitlab/import_export/import_test_coverage_spec.rb new file mode 100644 index 00000000000..d94abe613e8 --- /dev/null +++ b/spec/lib/gitlab/import_export/import_test_coverage_spec.rb @@ -0,0 +1,125 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# We want to test Import on "complete" data set, +# which means that every relation (as in our Import/Export definition) is covered. +# Fixture JSONs we use for testing Import such as +# `spec/fixtures/lib/gitlab/import_export/complex/project.json` +# should include these relations being non-empty. +describe 'Test coverage of the Project Import' do + include ConfigurationHelper + + # `MUTED_RELATIONS` is a technical debt. + # This list expected to be empty or used as a workround + # in case this spec blocks an important urgent MR. + # It is also expected that adding a relation in the list should lead to + # opening a follow-up issue to fix this. + MUTED_RELATIONS = %w[ + project.milestones.events.push_event_payload + project.issues.events.push_event_payload + project.issues.notes.events + project.issues.notes.events.push_event_payload + project.issues.milestone.events.push_event_payload + project.issues.issue_milestones + project.issues.issue_milestones.milestone + project.issues.resource_label_events.label.priorities + project.issues.designs.notes + project.issues.designs.notes.author + project.issues.designs.notes.events + project.issues.designs.notes.events.push_event_payload + project.merge_requests.metrics + project.merge_requests.notes.events.push_event_payload + project.merge_requests.events.push_event_payload + project.merge_requests.timelogs + project.merge_requests.label_links + project.merge_requests.label_links.label + project.merge_requests.label_links.label.priorities + project.merge_requests.milestone + project.merge_requests.milestone.events + project.merge_requests.milestone.events.push_event_payload + project.merge_requests.merge_request_milestones + project.merge_requests.merge_request_milestones.milestone + project.merge_requests.resource_label_events.label + project.merge_requests.resource_label_events.label.priorities + project.ci_pipelines.notes.events + project.ci_pipelines.notes.events.push_event_payload + project.protected_branches.unprotect_access_levels + project.prometheus_metrics + project.metrics_setting + project.boards.lists.label.priorities + project.service_desk_setting + ].freeze + + # A list of JSON fixture files we use to test Import. + # Note that we use separate fixture to test ee-only features. + # Most of the relations are present in `complex/project.json` + # which is our main fixture. + PROJECT_JSON_FIXTURES = [ + 'spec/fixtures/lib/gitlab/import_export/complex/project.json', + 'spec/fixtures/lib/gitlab/import_export/group/project.json', + 'spec/fixtures/lib/gitlab/import_export/light/project.json', + 'spec/fixtures/lib/gitlab/import_export/milestone-iid/project.json', + 'ee/spec/fixtures/lib/gitlab/import_export/designs/project.json' + ].freeze + + it 'ensures that all imported/exported relations are present in test JSONs' do + not_tested_relations = (relations_from_config - tested_relations) - MUTED_RELATIONS + + expect(not_tested_relations).to be_empty, failure_message(not_tested_relations) + end + + def relations_from_config + relation_paths_for(:project) + .map { |relation_names| relation_names.join(".") } + .to_set + end + + def tested_relations + PROJECT_JSON_FIXTURES.flat_map(&method(:relations_from_json)).to_set + end + + def relations_from_json(json_file) + json = ActiveSupport::JSON.decode(IO.read(json_file)) + + Gitlab::ImportExport::RelationRenameService.rename(json) + + [].tap {|res| gather_relations({ project: json }, res, [])} + .map {|relation_names| relation_names.join('.')} + end + + def gather_relations(item, res, path) + case item + when Hash + item.each do |k, v| + if (v.is_a?(Array) || v.is_a?(Hash)) && v.present? + new_path = path + [k] + res << new_path + gather_relations(v, res, new_path) + end + end + when Array + item.each {|i| gather_relations(i, res, path)} + end + end + + def failure_message(not_tested_relations) + <<~MSG + These relations seem to be added recenty and + they expected to be covered in our Import specs: #{not_tested_relations}. + + To do that, expand one of the files listed in `PROJECT_JSON_FIXTURES` + (or expand the list if you consider adding a new fixture file). + + After that, add a new spec into + `spec/lib/gitlab/import_export/project_tree_restorer_spec.rb` + to check that the relation is being imported correctly. + + In case the spec breaks the master or there is a sense of urgency, + you could include the relations into the `MUTED_RELATIONS` list. + + Muting relations is considered to be a temporary solution, so please + open a follow-up issue and try to fix that when it is possible. + MSG + end +end diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb index eb071d38eb7..5704f823b93 100644 --- a/spec/lib/gitlab/import_export/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb @@ -213,6 +213,10 @@ describe Gitlab::ImportExport::RelationFactory do attr_accessor :service_id, :moved_to_id, :namespace_id, :ci_id, :random_project_id, :random_id, :milestone_id, :project_id end + before do + allow(HazardousFooModel).to receive(:reflect_on_association).and_return(nil) + end + it 'does not preserve any foreign key IDs' do expect(created_object.values).not_to include(99) end @@ -247,6 +251,10 @@ describe Gitlab::ImportExport::RelationFactory do attr_accessor(*Gitlab::ImportExport::RelationFactory::PROJECT_REFERENCES) end + before do + allow(ProjectFooModel).to receive(:reflect_on_association).and_return(nil) + end + it 'does not preserve any project foreign key IDs' do expect(created_object.values).not_to include(99) end diff --git a/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb new file mode 100644 index 00000000000..6516016e67f --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/client_metrics_spec.rb @@ -0,0 +1,113 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::SidekiqMiddleware::ClientMetrics do + context "with worker attribution" do + subject { described_class.new } + + let(:queue) { :test } + let(:worker_class) { worker.class } + let(:job) { {} } + let(:default_labels) { { queue: queue.to_s, boundary: "", external_dependencies: "no", feature_category: "", latency_sensitive: "no" } } + + shared_examples "a metrics client middleware" do + context "with mocked prometheus" do + let(:enqueued_jobs_metric) { double('enqueued jobs metric', increment: true) } + + before do + allow(Gitlab::Metrics).to receive(:counter).with(described_class::ENQUEUED, anything).and_return(enqueued_jobs_metric) + end + + describe '#call' do + it 'yields block' do + expect { |b| subject.call(worker, job, :test, double, &b) }.to yield_control.once + end + + it 'increments enqueued jobs metric' do + expect(enqueued_jobs_metric).to receive(:increment).with(labels, 1) + + subject.call(worker, job, :test, double) { nil } + end + end + end + end + + context "when workers are not attributed" do + class TestNonAttributedWorker + include Sidekiq::Worker + end + + it_behaves_like "a metrics client middleware" do + let(:worker) { TestNonAttributedWorker.new } + let(:labels) { default_labels } + end + end + + context "when workers are attributed" do + def create_attributed_worker_class(latency_sensitive, external_dependencies, resource_boundary, category) + Class.new do + include Sidekiq::Worker + include WorkerAttributes + + latency_sensitive_worker! if latency_sensitive + worker_has_external_dependencies! if external_dependencies + worker_resource_boundary resource_boundary unless resource_boundary == :unknown + feature_category category unless category.nil? + end + end + + let(:latency_sensitive) { false } + let(:external_dependencies) { false } + let(:resource_boundary) { :unknown } + let(:feature_category) { nil } + let(:worker_class) { create_attributed_worker_class(latency_sensitive, external_dependencies, resource_boundary, feature_category) } + let(:worker) { worker_class.new } + + context "latency sensitive" do + it_behaves_like "a metrics client middleware" do + let(:latency_sensitive) { true } + let(:labels) { default_labels.merge(latency_sensitive: "yes") } + end + end + + context "external dependencies" do + it_behaves_like "a metrics client middleware" do + let(:external_dependencies) { true } + let(:labels) { default_labels.merge(external_dependencies: "yes") } + end + end + + context "cpu boundary" do + it_behaves_like "a metrics client middleware" do + let(:resource_boundary) { :cpu } + let(:labels) { default_labels.merge(boundary: "cpu") } + end + end + + context "memory boundary" do + it_behaves_like "a metrics client middleware" do + let(:resource_boundary) { :memory } + let(:labels) { default_labels.merge(boundary: "memory") } + end + end + + context "feature category" do + it_behaves_like "a metrics client middleware" do + let(:feature_category) { :authentication } + let(:labels) { default_labels.merge(feature_category: "authentication") } + end + end + + context "combined" do + it_behaves_like "a metrics client middleware" do + let(:latency_sensitive) { true } + let(:external_dependencies) { true } + let(:resource_boundary) { :cpu } + let(:feature_category) { :authentication } + let(:labels) { default_labels.merge(latency_sensitive: "yes", external_dependencies: "yes", boundary: "cpu", feature_category: "authentication") } + end + end + end + end +end diff --git a/spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb index 36c6f377bde..65a961b34f8 100644 --- a/spec/lib/gitlab/sidekiq_middleware/metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' -describe Gitlab::SidekiqMiddleware::Metrics do +describe Gitlab::SidekiqMiddleware::ServerMetrics do context "with worker attribution" do subject { described_class.new } diff --git a/spec/lib/gitlab/sidekiq_middleware_spec.rb b/spec/lib/gitlab/sidekiq_middleware_spec.rb index ef4a898bdb6..473d85c0143 100644 --- a/spec/lib/gitlab/sidekiq_middleware_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware_spec.rb @@ -41,7 +41,7 @@ describe Gitlab::SidekiqMiddleware do Labkit::Middleware::Sidekiq::Server, Gitlab::SidekiqMiddleware::InstrumentationLogger, Gitlab::SidekiqStatus::ServerMiddleware, - Gitlab::SidekiqMiddleware::Metrics, + Gitlab::SidekiqMiddleware::ServerMetrics, Gitlab::SidekiqMiddleware::ArgumentsLogger, Gitlab::SidekiqMiddleware::MemoryKiller, Gitlab::SidekiqMiddleware::RequestStoreMiddleware @@ -74,7 +74,7 @@ describe Gitlab::SidekiqMiddleware do let(:request_store) { false } let(:disabled_sidekiq_middlewares) do [ - Gitlab::SidekiqMiddleware::Metrics, + Gitlab::SidekiqMiddleware::ServerMetrics, Gitlab::SidekiqMiddleware::ArgumentsLogger, Gitlab::SidekiqMiddleware::MemoryKiller, Gitlab::SidekiqMiddleware::RequestStoreMiddleware diff --git a/spec/lib/sentry/client/issue_link_spec.rb b/spec/lib/sentry/client/issue_link_spec.rb new file mode 100644 index 00000000000..35a69be6de5 --- /dev/null +++ b/spec/lib/sentry/client/issue_link_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Sentry::Client::IssueLink do + include SentryClientHelpers + + let(:error_tracking_setting) { create(:project_error_tracking_setting, api_url: sentry_url) } + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:client) { error_tracking_setting.sentry_client } + + let(:issue_link_sample_response) { JSON.parse(fixture_file('sentry/issue_link_sample_response.json')) } + + describe '#create_issue_link' do + let(:integration_id) { 44444 } + let(:sentry_issue_id) { 11111111 } + let(:issue) { create(:issue, project: error_tracking_setting.project) } + + let(:sentry_issue_link_url) { "https://sentrytest.gitlab.com/api/0/groups/#{sentry_issue_id}/integrations/#{integration_id}/" } + let(:sentry_api_response) { issue_link_sample_response } + let!(:sentry_api_request) { stub_sentry_request(sentry_issue_link_url, :put, body: sentry_api_response, status: 201) } + + subject { client.create_issue_link(integration_id, sentry_issue_id, issue) } + + it_behaves_like 'calls sentry api' + + it { is_expected.to be_present } + + context 'redirects' do + let(:sentry_api_url) { sentry_issue_link_url } + + it_behaves_like 'no Sentry redirects', :put + end + + context 'when exception is raised' do + let(:sentry_request_url) { sentry_issue_link_url } + + it_behaves_like 'maps Sentry exceptions', :put + end + end +end diff --git a/spec/lib/sentry/client/repo_spec.rb b/spec/lib/sentry/client/repo_spec.rb new file mode 100644 index 00000000000..7bc2811ef03 --- /dev/null +++ b/spec/lib/sentry/client/repo_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Sentry::Client::Repo do + include SentryClientHelpers + + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + let(:client) { Sentry::Client.new(sentry_url, token) } + let(:repos_sample_response) { JSON.parse(fixture_file('sentry/repos_sample_response.json')) } + + describe '#repos' do + let(:organization_slug) { 'gitlab' } + let(:sentry_repos_url) { "https://sentrytest.gitlab.com/api/0/organizations/#{organization_slug}/repos/" } + let(:sentry_api_response) { repos_sample_response } + let!(:sentry_api_request) { stub_sentry_request(sentry_repos_url, body: sentry_api_response) } + + subject { client.repos(organization_slug) } + + it_behaves_like 'calls sentry api' + + it { is_expected.to all( be_a(Gitlab::ErrorTracking::Repo)) } + + it { expect(subject.length).to eq(1) } + + context 'redirects' do + let(:sentry_api_url) { sentry_repos_url } + + it_behaves_like 'no Sentry redirects' + end + + context 'when exception is raised' do + let(:sentry_request_url) { sentry_repos_url } + + it_behaves_like 'maps Sentry exceptions' + end + end +end diff --git a/spec/lib/sentry/client_spec.rb b/spec/lib/sentry/client_spec.rb index 409e8be3198..e2da4564ca1 100644 --- a/spec/lib/sentry/client_spec.rb +++ b/spec/lib/sentry/client_spec.rb @@ -12,4 +12,6 @@ describe Sentry::Client do it { is_expected.to respond_to :list_issues } it { is_expected.to respond_to :issue_details } it { is_expected.to respond_to :issue_latest_event } + it { is_expected.to respond_to :repos } + it { is_expected.to respond_to :create_issue_link } end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index ce01765bb8c..7c20bb415e1 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -1183,6 +1183,38 @@ describe Ci::Pipeline, :mailer do end end + describe 'auto devops pipeline metrics' do + using RSpec::Parameterized::TableSyntax + + let(:pipeline) { create(:ci_empty_pipeline, config_source: config_source) } + let(:config_source) { :auto_devops_source } + + where(:action, :status) do + :succeed | 'success' + :drop | 'failed' + :skip | 'skipped' + :cancel | 'canceled' + end + + with_them do + context "when pipeline receives action '#{params[:action]}'" do + subject { pipeline.public_send(action) } + + it { expect { subject }.to change { auto_devops_pipelines_completed_total(status) }.by(1) } + + context 'when not auto_devops_source?' do + let(:config_source) { :repository_source } + + it { expect { subject }.not_to change { auto_devops_pipelines_completed_total(status) } } + end + end + end + + def auto_devops_pipelines_completed_total(status) + Gitlab::Metrics.counter(:auto_devops_pipelines_completed_total, 'Number of completed auto devops pipelines').get(status: status) + end + end + def create_build(name, *traits, queued_at: current, started_from: 0, **opts) create(:ci_build, *traits, name: name, diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8ca167934f4..110f7a5af65 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -633,6 +633,27 @@ describe User, :do_not_mock_admin_mode do end end end + + describe '.active_without_ghosts' do + let_it_be(:user1) { create(:user, :external) } + let_it_be(:user2) { create(:user, state: 'blocked') } + let_it_be(:user3) { create(:user, ghost: true) } + let_it_be(:user4) { create(:user) } + + it 'returns all active users but ghost users' do + expect(described_class.active_without_ghosts).to match_array([user1, user4]) + end + end + + describe '.without_ghosts' do + let_it_be(:user1) { create(:user, :external) } + let_it_be(:user2) { create(:user, state: 'blocked') } + let_it_be(:user3) { create(:user, ghost: true) } + + it 'returns users without ghosts users' do + expect(described_class.without_ghosts).to match_array([user1, user2]) + end + end end describe "Respond to" do @@ -1252,7 +1273,7 @@ describe User, :do_not_mock_admin_mode do let(:user) { double } it 'filters by active users by default' do - expect(described_class).to receive(:active).and_return([user]) + expect(described_class).to receive(:active_without_ghosts).and_return([user]) expect(described_class.filter_items(nil)).to include user end diff --git a/spec/services/metrics/dashboard/clone_dashboard_service_spec.rb b/spec/services/metrics/dashboard/clone_dashboard_service_spec.rb new file mode 100644 index 00000000000..274d594fd68 --- /dev/null +++ b/spec/services/metrics/dashboard/clone_dashboard_service_spec.rb @@ -0,0 +1,197 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Metrics::Dashboard::CloneDashboardService, :use_clean_rails_memory_store_caching do + include MetricsDashboardHelpers + + set(:user) { create(:user) } + set(:project) { create(:project, :repository) } + set(:environment) { create(:environment, project: project) } + + describe '#execute' do + subject(:service_call) { described_class.new(project, user, params).execute } + + let(:commit_message) { 'test' } + let(:branch) { "dashboard_new_branch" } + let(:dashboard) { 'config/prometheus/common_metrics.yml' } + let(:file_name) { 'custom_dashboard.yml' } + let(:params) do + { + dashboard: dashboard, + file_name: file_name, + commit_message: commit_message, + branch: branch + } + end + + let(:dashboard_attrs) do + { + commit_message: commit_message, + branch_name: branch, + start_branch: project.default_branch, + encoding: 'text', + file_path: ".gitlab/dashboards/#{file_name}", + file_content: File.read(dashboard) + } + end + + context 'user does not have push right to repository' do + it_behaves_like 'misconfigured dashboard service response', :forbidden, %q(You can't commit to this project) + end + + context 'with rights to push to the repository' do + before do + project.add_maintainer(user) + end + + context 'wrong target file extension' do + let(:file_name) { 'custom_dashboard.txt' } + + it_behaves_like 'misconfigured dashboard service response', :bad_request, 'The file name should have a .yml extension' + end + + context 'wrong source dashboard file' do + let(:dashboard) { 'config/prometheus/common_metrics_123.yml' } + + it_behaves_like 'misconfigured dashboard service response', :not_found, 'Not found.' + end + + context 'path traversal attack attempt' do + let(:dashboard) { 'config/prometheus/../database.yml' } + + it_behaves_like 'misconfigured dashboard service response', :not_found, 'Not found.' + end + + context 'path traversal attack attempt on target file' do + let(:file_name) { '../../custom_dashboard.yml' } + let(:dashboard_attrs) do + { + commit_message: commit_message, + branch_name: branch, + start_branch: project.default_branch, + encoding: 'text', + file_path: ".gitlab/dashboards/custom_dashboard.yml", + file_content: File.read(dashboard) + } + end + + it 'strips target file name to safe value', :aggregate_failures do + service_instance = instance_double(::Files::CreateService) + expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance) + expect(service_instance).to receive(:execute).and_return(status: :success) + + service_call + end + end + + context 'valid parameters' do + it 'delegates commit creation to Files::CreateService', :aggregate_failures do + service_instance = instance_double(::Files::CreateService) + expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance) + expect(service_instance).to receive(:execute).and_return(status: :success) + + service_call + end + + context 'selected branch already exists' do + let(:branch) { 'existing_branch' } + + before do + project.repository.add_branch(user, branch, 'master') + end + + it_behaves_like 'misconfigured dashboard service response', :bad_request, "There was an error creating the dashboard, branch named: existing_branch already exists." + + # temporary not available function for first iteration + # follow up issue https://gitlab.com/gitlab-org/gitlab/issues/196237 which + # require this feature + # it 'pass correct params to Files::CreateService', :aggregate_failures do + # project.repository.add_branch(user, branch, 'master') + # + # service_instance = instance_double(::Files::CreateService) + # expect(::Files::CreateService).to receive(:new).with(project, user, dashboard_attrs).and_return(service_instance) + # expect(service_instance).to receive(:execute).and_return(status: :success) + # + # service_call + # end + end + + context 'blank branch name' do + let(:branch) { '' } + + it_behaves_like 'misconfigured dashboard service response', :bad_request, 'There was an error creating the dashboard, branch name is invalid.' + end + + context 'dashboard file already exists' do + let(:branch) { 'custom_dashboard' } + + before do + Files::CreateService.new( + project, + user, + commit_message: 'Create custom dashboard custom_dashboard.yml', + branch_name: 'master', + start_branch: 'master', + file_path: ".gitlab/dashboards/custom_dashboard.yml", + file_content: File.read('config/prometheus/common_metrics.yml') + ).execute + end + + it_behaves_like 'misconfigured dashboard service response', :bad_request, "A file with 'custom_dashboard.yml' already exists in custom_dashboard branch" + end + + it 'extends dashboard template path to absolute url' do + allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success })) + + expect(File).to receive(:read).with(Rails.root.join('config/prometheus/common_metrics.yml')).and_return('') + + service_call + end + + context 'Files::CreateService success' do + before do + allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :success })) + end + + it 'clears dashboards cache' do + expect(project.repository).to receive(:refresh_method_caches).with([:metrics_dashboard]) + + service_call + end + + it 'returns success', :aggregate_failures do + result = service_call + dashboard_details = { + path: '.gitlab/dashboards/custom_dashboard.yml', + display_name: 'custom_dashboard.yml', + default: false, + system_dashboard: false + } + + expect(result[:status]).to be :success + expect(result[:http_status]).to be :created + expect(result[:dashboard]).to match dashboard_details + end + end + + context 'Files::CreateService fails' do + before do + allow(::Files::CreateService).to receive(:new).and_return(double(execute: { status: :error })) + end + + it 'does NOT clear dashboards cache' do + expect(project.repository).not_to receive(:refresh_method_caches) + + service_call + end + + it 'returns error' do + result = service_call + expect(result[:status]).to be :error + end + end + end + end + end +end diff --git a/spec/support/helpers/metrics_dashboard_helpers.rb b/spec/support/helpers/metrics_dashboard_helpers.rb index 5b425d0964d..908a3e1fb09 100644 --- a/spec/support/helpers/metrics_dashboard_helpers.rb +++ b/spec/support/helpers/metrics_dashboard_helpers.rb @@ -29,54 +29,4 @@ module MetricsDashboardHelpers def business_metric_title PrometheusMetricEnums.group_details[:business][:group_title] end - - shared_examples_for 'misconfigured dashboard service response' do |status_code| - it 'returns an appropriate message and status code' do - result = service_call - - expect(result.keys).to contain_exactly(:message, :http_status, :status) - expect(result[:status]).to eq(:error) - expect(result[:http_status]).to eq(status_code) - end - end - - shared_examples_for 'valid dashboard service response for schema' do - it 'returns a json representation of the dashboard' do - result = service_call - - expect(result.keys).to contain_exactly(:dashboard, :status) - expect(result[:status]).to eq(:success) - - expect(JSON::Validator.fully_validate(dashboard_schema, result[:dashboard])).to be_empty - end - end - - shared_examples_for 'valid dashboard service response' do - let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/dashboard.json')) } - - it_behaves_like 'valid dashboard service response for schema' - end - - shared_examples_for 'caches the unprocessed dashboard for subsequent calls' do - it do - expect(YAML).to receive(:safe_load).once.and_call_original - - described_class.new(*service_params).get_dashboard - described_class.new(*service_params).get_dashboard - end - end - - shared_examples_for 'valid embedded dashboard service response' do - let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/embedded_dashboard.json')) } - - it_behaves_like 'valid dashboard service response for schema' - end - - shared_examples_for 'raises error for users with insufficient permissions' do - context 'when the user does not have sufficient access' do - let(:user) { build(:user) } - - it_behaves_like 'misconfigured dashboard service response', :unauthorized - end - end end diff --git a/spec/support/redis/redis_shared_examples.rb b/spec/support/redis/redis_shared_examples.rb index e079c32d6ae..1e2d11a66cb 100644 --- a/spec/support/redis/redis_shared_examples.rb +++ b/spec/support/redis/redis_shared_examples.rb @@ -116,9 +116,9 @@ RSpec.shared_examples "redis_shared_examples" do clear_pool end - context 'when running not on sidekiq workers' do + context 'when running on single-threaded runtime' do before do - allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(false) + allow(Gitlab::Runtime).to receive(:multi_threaded?).and_return(false) end it 'instantiates a connection pool with size 5' do @@ -128,10 +128,10 @@ RSpec.shared_examples "redis_shared_examples" do end end - context 'when running on sidekiq workers' do + context 'when running on multi-threaded runtime' do before do - allow(Gitlab::Runtime).to receive(:sidekiq?).and_return(true) - allow(Sidekiq).to receive(:options).and_return({ concurrency: 18 }) + allow(Gitlab::Runtime).to receive(:multi_threaded?).and_return(true) + allow(Gitlab::Runtime).to receive(:max_threads).and_return(18) end it 'instantiates a connection pool with a size based on the concurrency of the worker' do diff --git a/spec/support/shared_examples/lib/sentry/client_shared_examples.rb b/spec/support/shared_examples/lib/sentry/client_shared_examples.rb index 76b71ebd3c5..4221708b55c 100644 --- a/spec/support/shared_examples/lib/sentry/client_shared_examples.rb +++ b/spec/support/shared_examples/lib/sentry/client_shared_examples.rb @@ -10,7 +10,7 @@ RSpec.shared_examples 'calls sentry api' do end # Requires sentry_api_url and subject to be defined -RSpec.shared_examples 'no Sentry redirects' do +RSpec.shared_examples 'no Sentry redirects' do |http_method| let(:redirect_to) { 'https://redirected.example.com' } let(:other_url) { 'https://other.example.org' } @@ -19,6 +19,7 @@ RSpec.shared_examples 'no Sentry redirects' do let!(:redirect_req_stub) do stub_sentry_request( sentry_api_url, + http_method || :get, status: 302, headers: { location: redirect_to } ) @@ -31,7 +32,7 @@ RSpec.shared_examples 'no Sentry redirects' do end end -RSpec.shared_examples 'maps Sentry exceptions' do +RSpec.shared_examples 'maps Sentry exceptions' do |http_method| exceptions = { Gitlab::HTTP::Error => 'Error when connecting to Sentry', Net::OpenTimeout => 'Connection to Sentry timed out', @@ -44,7 +45,10 @@ RSpec.shared_examples 'maps Sentry exceptions' do exceptions.each do |exception, message| context "#{exception}" do before do - stub_request(:get, sentry_request_url).to_raise(exception) + stub_request( + http_method || :get, + sentry_request_url + ).to_raise(exception) end it do diff --git a/spec/support/shared_examples/requests/api/status_shared_examples.rb b/spec/support/shared_examples/requests/api/status_shared_examples.rb index eebed7e42c1..ed9964fa108 100644 --- a/spec/support/shared_examples/requests/api/status_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/status_shared_examples.rb @@ -59,8 +59,9 @@ shared_examples_for '412 response' do delete request, params: params, headers: { 'HTTP_IF_UNMODIFIED_SINCE' => '1990-01-12T00:00:48-0600' } end - it 'returns 412' do + it 'returns 412 with a JSON error' do expect(response).to have_gitlab_http_status(412) + expect(json_response).to eq('message' => '412 Precondition Failed') end end @@ -69,8 +70,9 @@ shared_examples_for '412 response' do delete request, params: params, headers: { 'HTTP_IF_UNMODIFIED_SINCE' => Time.now } end - it 'returns accepted' do + it 'returns 204 with an empty body' do expect(response).to have_gitlab_http_status(success_status) + expect(response.body).to eq('') if success_status == 204 end end end diff --git a/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb b/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb new file mode 100644 index 00000000000..30d91346df3 --- /dev/null +++ b/spec/support/shared_examples/services/metrics/dashboard_shared_examples.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +shared_examples_for 'misconfigured dashboard service response' do |status_code, message = nil| + it 'returns an appropriate message and status code', :aggregate_failures do + result = service_call + + expect(result.keys).to contain_exactly(:message, :http_status, :status) + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(status_code) + expect(result[:message]).to eq(message) if message + end +end + +shared_examples_for 'valid dashboard service response for schema' do + it 'returns a json representation of the dashboard' do + result = service_call + + expect(result.keys).to contain_exactly(:dashboard, :status) + expect(result[:status]).to eq(:success) + + expect(JSON::Validator.fully_validate(dashboard_schema, result[:dashboard])).to be_empty + end +end + +shared_examples_for 'valid dashboard service response' do + let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/dashboard.json')) } + + it_behaves_like 'valid dashboard service response for schema' +end + +shared_examples_for 'caches the unprocessed dashboard for subsequent calls' do + it do + expect(YAML).to receive(:safe_load).once.and_call_original + + described_class.new(*service_params).get_dashboard + described_class.new(*service_params).get_dashboard + end +end + +shared_examples_for 'valid embedded dashboard service response' do + let(:dashboard_schema) { JSON.parse(fixture_file('lib/gitlab/metrics/dashboard/schemas/embedded_dashboard.json')) } + + it_behaves_like 'valid dashboard service response for schema' +end + +shared_examples_for 'raises error for users with insufficient permissions' do + context 'when the user does not have sufficient access' do + let(:user) { build(:user) } + + it_behaves_like 'misconfigured dashboard service response', :unauthorized + end +end |