diff options
Diffstat (limited to 'spec')
28 files changed, 649 insertions, 434 deletions
diff --git a/spec/features/projects/navbar_spec.rb b/spec/features/projects/navbar_spec.rb index 94d79d60aeb..0193712aeea 100644 --- a/spec/features/projects/navbar_spec.rb +++ b/spec/features/projects/navbar_spec.rb @@ -12,7 +12,16 @@ RSpec.describe 'Project navbar' do let_it_be(:project) { create(:project, :repository) } before do - stub_licensed_features(service_desk: false) + # TODO - This can be moved into 'project navbar structure' shared + # context when service desk feature gets moved to core. + # More information in: https://gitlab.com/gitlab-org/gitlab/-/issues/215364 + if Gitlab.ee? + insert_after_sub_nav_item( + _('Labels'), + within: _('Issues'), + new_sub_nav_item_name: _('Service Desk') + ) + end project.add_maintainer(user) sign_in(user) diff --git a/spec/finders/merge_requests_finder_spec.rb b/spec/finders/merge_requests_finder_spec.rb index f76110e3d85..e3643698012 100644 --- a/spec/finders/merge_requests_finder_spec.rb +++ b/spec/finders/merge_requests_finder_spec.rb @@ -53,6 +53,21 @@ RSpec.describe MergeRequestsFinder do expect(merge_requests).to be_empty end + context 'filtering by not author ID' do + let(:params) { { not: { author_id: user2.id } } } + + before do + merge_request2.update!(author: user2) + merge_request3.update!(author: user2) + end + + it 'returns merge requests not created by that user' do + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request1, merge_request4, merge_request5) + end + end + it 'filters by projects' do params = { projects: [project2.id, project3.id] } @@ -258,6 +273,11 @@ RSpec.describe MergeRequestsFinder do let(:expected_issuables) { [merge_request1, merge_request2] } end + it_behaves_like 'assignee NOT ID filter' do + let(:params) { { not: { assignee_id: user.id } } } + let(:expected_issuables) { [merge_request3, merge_request4, merge_request5] } + end + it_behaves_like 'assignee username filter' do before do project2.add_developer(user3) @@ -269,6 +289,15 @@ RSpec.describe MergeRequestsFinder do let(:expected_issuables) { [merge_request3] } end + it_behaves_like 'assignee NOT username filter' do + before do + merge_request2.assignees = [user2] + end + + let(:params) { { not: { assignee_username: [user.username, user2.username] } } } + let(:expected_issuables) { [merge_request4, merge_request5] } + end + it_behaves_like 'no assignee filter' do let_it_be(:user3) { create(:user) } let(:expected_issuables) { [merge_request4, merge_request5] } @@ -294,6 +323,16 @@ RSpec.describe MergeRequestsFinder do expect(merge_requests).to contain_exactly(merge_request2, merge_request3) end + + context 'using NOT' do + let(:params) { { not: { milestone_title: group_milestone.title } } } + + it 'returns MRs not assigned to that group milestone' do + merge_requests = described_class.new(user, params).execute + + expect(merge_requests).to contain_exactly(merge_request1, merge_request4, merge_request5) + end + end end end diff --git a/spec/frontend/alert_management/components/alert_management_list_spec.js b/spec/frontend/alert_management/components/alert_management_list_spec.js index 284fd078ab3..e2dc7d79a85 100644 --- a/spec/frontend/alert_management/components/alert_management_list_spec.js +++ b/spec/frontend/alert_management/components/alert_management_list_spec.js @@ -48,6 +48,7 @@ describe('AlertManagementList', () => { const findSeverityColumnHeader = () => wrapper.findAll('th').at(0); const findPagination = () => wrapper.find(GlPagination); const findSearch = () => wrapper.find(GlSearchBoxByType); + const findIssueFields = () => wrapper.findAll('[data-testid="issueField"]'); const alertsCount = { open: 14, triggered: 10, @@ -278,6 +279,37 @@ describe('AlertManagementList', () => { expect(visitUrl).toHaveBeenCalledWith('/1527542/details'); }); + describe('alert issue links', () => { + beforeEach(() => { + mountComponent({ + props: { alertManagementEnabled: true, userCanEnableAlertManagement: true }, + data: { alerts: { list: mockAlerts }, alertsCount, errored: false }, + loading: false, + }); + }); + + it('shows "None" when no link exists', () => { + expect( + findIssueFields() + .at(0) + .text(), + ).toBe('None'); + }); + + it('renders a link when one exists', () => { + expect( + findIssueFields() + .at(1) + .text(), + ).toBe('#1'); + expect( + findIssueFields() + .at(1) + .attributes('href'), + ).toBe('/gitlab-org/gitlab/-/issues/1'); + }); + }); + describe('handle date fields', () => { it('should display time ago dates when values provided', () => { mountComponent({ diff --git a/spec/frontend/alert_management/mocks/alerts.json b/spec/frontend/alert_management/mocks/alerts.json index 312d1756790..34eed7ae024 100644 --- a/spec/frontend/alert_management/mocks/alerts.json +++ b/spec/frontend/alert_management/mocks/alerts.json @@ -20,6 +20,7 @@ "endedAt": "2020-04-17T23:18:14.996Z", "status": "ACKNOWLEDGED", "assignees": { "nodes": [{ "username": "root" }] }, + "issueIid": "1", "notes": { "nodes": [ { diff --git a/spec/frontend/monitoring/components/dashboard_spec.js b/spec/frontend/monitoring/components/dashboard_spec.js index f894b2af5cf..cbcd8305c10 100644 --- a/spec/frontend/monitoring/components/dashboard_spec.js +++ b/spec/frontend/monitoring/components/dashboard_spec.js @@ -26,10 +26,9 @@ import { setMetricResult, setupStoreWithData, setupStoreWithDataForPanelCount, - setupStoreWithVariable, setupStoreWithLinks, } from '../store_utils'; -import { environmentData, dashboardGitResponse } from '../mock_data'; +import { environmentData, dashboardGitResponse, storeVariables } from '../mock_data'; import { metricsDashboardViewModel, metricsDashboardPanelCount, @@ -604,8 +603,7 @@ describe('Dashboard', () => { beforeEach(() => { createShallowWrapper({ hasMetrics: true }); setupStoreWithData(store); - setupStoreWithVariable(store); - + store.state.monitoringDashboard.variables = storeVariables; return wrapper.vm.$nextTick(); }); diff --git a/spec/frontend/monitoring/components/variables/dropdown_field_spec.js b/spec/frontend/monitoring/components/variables/dropdown_field_spec.js index 623125d18f1..cc384aef231 100644 --- a/spec/frontend/monitoring/components/variables/dropdown_field_spec.js +++ b/spec/frontend/monitoring/components/variables/dropdown_field_spec.js @@ -59,7 +59,7 @@ describe('Custom variable component', () => { .vm.$emit('click'); return wrapper.vm.$nextTick(() => { - expect(wrapper.vm.$emit).toHaveBeenCalledWith('onUpdate', 'env', 'canary'); + expect(wrapper.vm.$emit).toHaveBeenCalledWith('input', 'canary'); }); }); }); diff --git a/spec/frontend/monitoring/components/variables/text_field_spec.js b/spec/frontend/monitoring/components/variables/text_field_spec.js index 68bfb8ec695..99c6facac38 100644 --- a/spec/frontend/monitoring/components/variables/text_field_spec.js +++ b/spec/frontend/monitoring/components/variables/text_field_spec.js @@ -40,7 +40,7 @@ describe('Text variable component', () => { findInput().trigger('keyup.enter'); return wrapper.vm.$nextTick(() => { - expect(wrapper.vm.$emit).toHaveBeenCalledWith('onUpdate', 'pod', 'prod-pod'); + expect(wrapper.vm.$emit).toHaveBeenCalledWith('input', 'prod-pod'); }); }); @@ -53,7 +53,7 @@ describe('Text variable component', () => { findInput().trigger('blur'); return wrapper.vm.$nextTick(() => { - expect(wrapper.vm.$emit).toHaveBeenCalledWith('onUpdate', 'pod', 'canary-pod'); + expect(wrapper.vm.$emit).toHaveBeenCalledWith('input', 'canary-pod'); }); }); }); diff --git a/spec/frontend/monitoring/components/variables_section_spec.js b/spec/frontend/monitoring/components/variables_section_spec.js index 9fb93a18e0b..d8e63917eec 100644 --- a/spec/frontend/monitoring/components/variables_section_spec.js +++ b/spec/frontend/monitoring/components/variables_section_spec.js @@ -6,8 +6,7 @@ import TextField from '~/monitoring/components/variables/text_field.vue'; import { updateHistory, mergeUrlParams } from '~/lib/utils/url_utility'; import { createStore } from '~/monitoring/stores'; import { convertVariablesForURL } from '~/monitoring/utils'; -import * as types from '~/monitoring/stores/mutation_types'; -import { mockTemplatingDataResponses } from '../mock_data'; +import { storeVariables } from '../mock_data'; jest.mock('~/lib/utils/url_utility', () => ({ updateHistory: jest.fn(), @@ -17,12 +16,6 @@ jest.mock('~/lib/utils/url_utility', () => ({ describe('Metrics dashboard/variables section component', () => { let store; let wrapper; - const sampleVariables = { - label1: mockTemplatingDataResponses.simpleText.simpleText, - label2: mockTemplatingDataResponses.advText.advText, - label3: mockTemplatingDataResponses.simpleCustom.simpleCustom, - label4: mockTemplatingDataResponses.metricLabelValues.simple, - }; const createShallowWrapper = () => { wrapper = shallowMount(VariablesSection, { @@ -48,22 +41,23 @@ describe('Metrics dashboard/variables section component', () => { describe('when variables are set', () => { beforeEach(() => { + store.state.monitoringDashboard.variables = storeVariables; createShallowWrapper(); - store.commit(`monitoringDashboard/${types.SET_VARIABLES}`, sampleVariables); + return wrapper.vm.$nextTick; }); it('shows the variables section', () => { const allInputs = findTextInputs().length + findCustomInputs().length; - expect(allInputs).toBe(Object.keys(sampleVariables).length); + expect(allInputs).toBe(storeVariables.length); }); it('shows the right custom variable inputs', () => { const customInputs = findCustomInputs(); - expect(customInputs.at(0).props('name')).toBe('label3'); - expect(customInputs.at(1).props('name')).toBe('label4'); + expect(customInputs.at(0).props('name')).toBe('customSimple'); + expect(customInputs.at(1).props('name')).toBe('customAdvanced'); }); }); @@ -77,7 +71,7 @@ describe('Metrics dashboard/variables section component', () => { namespaced: true, state: { showEmptyState: false, - variables: sampleVariables, + variables: storeVariables, }, actions: { updateVariablesAndFetchData, @@ -92,12 +86,12 @@ describe('Metrics dashboard/variables section component', () => { it('merges the url params and refreshes the dashboard when a text-based variables inputs are updated', () => { const firstInput = findTextInputs().at(0); - firstInput.vm.$emit('onUpdate', 'label1', 'test'); + firstInput.vm.$emit('input', 'test'); return wrapper.vm.$nextTick(() => { expect(updateVariablesAndFetchData).toHaveBeenCalled(); expect(mergeUrlParams).toHaveBeenCalledWith( - convertVariablesForURL(sampleVariables), + convertVariablesForURL(storeVariables), window.location.href, ); expect(updateHistory).toHaveBeenCalled(); @@ -107,12 +101,12 @@ describe('Metrics dashboard/variables section component', () => { it('merges the url params and refreshes the dashboard when a custom-based variables inputs are updated', () => { const firstInput = findCustomInputs().at(0); - firstInput.vm.$emit('onUpdate', 'label1', 'test'); + firstInput.vm.$emit('input', 'test'); return wrapper.vm.$nextTick(() => { expect(updateVariablesAndFetchData).toHaveBeenCalled(); expect(mergeUrlParams).toHaveBeenCalledWith( - convertVariablesForURL(sampleVariables), + convertVariablesForURL(storeVariables), window.location.href, ); expect(updateHistory).toHaveBeenCalled(); @@ -122,7 +116,7 @@ describe('Metrics dashboard/variables section component', () => { it('does not merge the url params and refreshes the dashboard if the value entered is not different that is what currently stored', () => { const firstInput = findTextInputs().at(0); - firstInput.vm.$emit('onUpdate', 'label1', 'Simple text'); + firstInput.vm.$emit('input', 'My default value'); expect(updateVariablesAndFetchData).not.toHaveBeenCalled(); expect(mergeUrlParams).not.toHaveBeenCalled(); diff --git a/spec/frontend/monitoring/mock_data.js b/spec/frontend/monitoring/mock_data.js index e5b25a37976..8ccae1228e8 100644 --- a/spec/frontend/monitoring/mock_data.js +++ b/spec/frontend/monitoring/mock_data.js @@ -627,81 +627,79 @@ export const mockLinks = [ }, ]; -const templatingVariableTypes = { +export const templatingVariablesExamples = { text: { - simple: 'Simple text', - advanced: { - label: 'Variable 4', + textSimple: 'My default value', + textAdvanced: { + label: 'Advanced text variable', type: 'text', options: { - default_value: 'default', + default_value: 'A default value', }, }, }, custom: { - simple: ['value1', 'value2', 'value3'], - advanced: { - normal: { - label: 'Advanced Var', - type: 'custom', - options: { - values: [ - { value: 'value1', text: 'Var 1 Option 1' }, - { - value: 'value2', - text: 'Var 1 Option 2', - default: true, - }, - ], - }, - }, - withoutOpts: { - type: 'custom', - options: {}, + customSimple: ['value1', 'value2', 'value3'], + customAdvanced: { + label: 'Advanced Var', + type: 'custom', + options: { + values: [ + { value: 'value1', text: 'Var 1 Option 1' }, + { + value: 'value2', + text: 'Var 1 Option 2', + default: true, + }, + ], }, - withoutLabel: { - type: 'custom', - options: { - values: [ - { value: 'value1', text: 'Var 1 Option 1' }, - { - value: 'value2', - text: 'Var 1 Option 2', - default: true, - }, - ], - }, + }, + customAdvancedWithoutOpts: { + type: 'custom', + options: {}, + }, + customAdvancedWithoutLabel: { + type: 'custom', + options: { + values: [ + { value: 'value1', text: 'Var 1 Option 1' }, + { + value: 'value2', + text: 'Var 1 Option 2', + default: true, + }, + ], }, - withoutType: { - label: 'Variable 2', - options: { - values: [ - { value: 'value1', text: 'Var 1 Option 1' }, - { - value: 'value2', - text: 'Var 1 Option 2', - default: true, - }, - ], - }, + }, + customAdvancedWithoutType: { + label: 'Variable 2', + options: { + values: [ + { value: 'value1', text: 'Var 1 Option 1' }, + { + value: 'value2', + text: 'Var 1 Option 2', + default: true, + }, + ], }, - withoutOptText: { - label: 'Options without text', - type: 'custom', - options: { - values: [ - { value: 'value1' }, - { - value: 'value2', - default: true, - }, - ], - }, + }, + customAdvancedWithoutOptText: { + label: 'Options without text', + type: 'custom', + options: { + values: [ + { value: 'value1' }, + { + value: 'value2', + default: true, + }, + ], }, }, }, metricLabelValues: { - simple: { + metricLabelValuesSimple: { label: 'Metric Label Values', type: 'metric_label_values', options: { @@ -713,205 +711,92 @@ const templatingVariableTypes = { }, }; -const generateMockTemplatingData = data => { - const vars = data - ? { - variables: { - ...data, - }, - } - : {}; - return { - dashboard: { - templating: vars, - }, - }; -}; - -const responseForSimpleTextVariable = { - simpleText: { - label: 'simpleText', +export const storeTextVariables = [ + { type: 'text', - value: 'Simple text', + name: 'textSimple', + label: 'textSimple', + value: 'My default value', }, -}; - -const responseForAdvTextVariable = { - advText: { - label: 'Variable 4', + { type: 'text', - value: 'default', + name: 'textAdvanced', + label: 'Advanced text variable', + value: 'A default value', }, -}; +]; -const responseForSimpleCustomVariable = { - simpleCustom: { - label: 'simpleCustom', - value: 'value1', +export const storeCustomVariables = [ + { + type: 'custom', + name: 'customSimple', + label: 'customSimple', options: { values: [ - { - default: false, - text: 'value1', - value: 'value1', - }, - { - default: false, - text: 'value2', - value: 'value2', - }, - { - default: false, - text: 'value3', - value: 'value3', - }, + { default: false, text: 'value1', value: 'value1' }, + { default: false, text: 'value2', value: 'value2' }, + { default: false, text: 'value3', value: 'value3' }, ], }, - type: 'custom', + value: 'value1', }, -}; - -const responseForAdvancedCustomVariableWithoutOptions = { - advCustomWithoutOpts: { - label: 'advCustomWithoutOpts', + { + type: 'custom', + name: 'customAdvanced', + label: 'Advanced Var', options: { - values: [], + values: [ + { default: false, text: 'Var 1 Option 1', value: 'value1' }, + { default: true, text: 'Var 1 Option 2', value: 'value2' }, + ], }, + value: 'value2', + }, + { type: 'custom', + name: 'customAdvancedWithoutOpts', + label: 'customAdvancedWithoutOpts', + options: { values: [] }, + value: null, }, -}; - -const responseForAdvancedCustomVariableWithoutLabel = { - advCustomWithoutLabel: { - label: 'advCustomWithoutLabel', + { + type: 'custom', + name: 'customAdvancedWithoutLabel', + label: 'customAdvancedWithoutLabel', value: 'value2', options: { values: [ - { - default: false, - text: 'Var 1 Option 1', - value: 'value1', - }, - { - default: true, - text: 'Var 1 Option 2', - value: 'value2', - }, + { default: false, text: 'Var 1 Option 1', value: 'value1' }, + { default: true, text: 'Var 1 Option 2', value: 'value2' }, ], }, - type: 'custom', }, -}; - -const responseForAdvancedCustomVariableWithoutOptText = { - advCustomWithoutOptText: { + { + type: 'custom', + name: 'customAdvancedWithoutOptText', label: 'Options without text', - value: 'value2', options: { values: [ - { - default: false, - text: 'value1', - value: 'value1', - }, - { - default: true, - text: 'value2', - value: 'value2', - }, + { default: false, text: 'value1', value: 'value1' }, + { default: true, text: 'value2', value: 'value2' }, ], }, - type: 'custom', + value: 'value2', }, -}; +]; -const responseForMetricLabelValues = { - simple: { - label: 'Metric Label Values', +export const storeMetricLabelValuesVariables = [ + { type: 'metric_label_values', + name: 'metricLabelValuesSimple', + label: 'Metric Label Values', + options: { prometheusEndpointPath: '/series', label: 'backend', values: [] }, value: null, - options: { - prometheusEndpointPath: '/series', - label: 'backend', - values: [], - }, }, -}; - -const responseForAdvancedCustomVariable = { - ...responseForSimpleCustomVariable, - advCustomNormal: { - label: 'Advanced Var', - value: 'value2', - options: { - values: [ - { - default: false, - text: 'Var 1 Option 1', - value: 'value1', - }, - { - default: true, - text: 'Var 1 Option 2', - value: 'value2', - }, - ], - }, - type: 'custom', - }, -}; - -const responsesForAllVariableTypes = { - ...responseForSimpleTextVariable, - ...responseForAdvTextVariable, - ...responseForSimpleCustomVariable, - ...responseForAdvancedCustomVariable, -}; - -export const mockTemplatingData = { - emptyTemplatingProp: generateMockTemplatingData(), - emptyVariablesProp: generateMockTemplatingData({}), - simpleText: generateMockTemplatingData({ simpleText: templatingVariableTypes.text.simple }), - advText: generateMockTemplatingData({ advText: templatingVariableTypes.text.advanced }), - simpleCustom: generateMockTemplatingData({ simpleCustom: templatingVariableTypes.custom.simple }), - advCustomWithoutOpts: generateMockTemplatingData({ - advCustomWithoutOpts: templatingVariableTypes.custom.advanced.withoutOpts, - }), - advCustomWithoutType: generateMockTemplatingData({ - advCustomWithoutType: templatingVariableTypes.custom.advanced.withoutType, - }), - advCustomWithoutLabel: generateMockTemplatingData({ - advCustomWithoutLabel: templatingVariableTypes.custom.advanced.withoutLabel, - }), - advCustomWithoutOptText: generateMockTemplatingData({ - advCustomWithoutOptText: templatingVariableTypes.custom.advanced.withoutOptText, - }), - simpleAndAdv: generateMockTemplatingData({ - simpleCustom: templatingVariableTypes.custom.simple, - advCustomNormal: templatingVariableTypes.custom.advanced.normal, - }), - metricLabelValues: generateMockTemplatingData({ - simple: templatingVariableTypes.metricLabelValues.simple, - }), - allVariableTypes: generateMockTemplatingData({ - simpleText: templatingVariableTypes.text.simple, - advText: templatingVariableTypes.text.advanced, - simpleCustom: templatingVariableTypes.custom.simple, - advCustomNormal: templatingVariableTypes.custom.advanced.normal, - }), -}; +]; -export const mockTemplatingDataResponses = { - emptyTemplatingProp: {}, - emptyVariablesProp: {}, - simpleText: responseForSimpleTextVariable, - advText: responseForAdvTextVariable, - simpleCustom: responseForSimpleCustomVariable, - advCustomWithoutOpts: responseForAdvancedCustomVariableWithoutOptions, - advCustomWithoutType: {}, - advCustomWithoutLabel: responseForAdvancedCustomVariableWithoutLabel, - advCustomWithoutOptText: responseForAdvancedCustomVariableWithoutOptText, - simpleAndAdv: responseForAdvancedCustomVariable, - allVariableTypes: responsesForAllVariableTypes, - metricLabelValues: responseForMetricLabelValues, -}; +export const storeVariables = [ + ...storeTextVariables, + ...storeCustomVariables, + ...storeMetricLabelValuesVariables, +]; diff --git a/spec/frontend/monitoring/store/actions_spec.js b/spec/frontend/monitoring/store/actions_spec.js index 0e28b70a760..ad01e4c3a9b 100644 --- a/spec/frontend/monitoring/store/actions_spec.js +++ b/spec/frontend/monitoring/store/actions_spec.js @@ -44,7 +44,6 @@ import { deploymentData, environmentData, annotationsData, - mockTemplatingData, dashboardGitResponse, mockDashboardsErrorResponse, } from '../mock_data'; @@ -305,32 +304,6 @@ describe('Monitoring store actions', () => { expect(dispatch).toHaveBeenCalledWith('fetchDashboardData'); }); - it('stores templating variables', () => { - const response = { - ...metricsDashboardResponse.dashboard, - ...mockTemplatingData.allVariableTypes.dashboard, - }; - - receiveMetricsDashboardSuccess( - { state, commit, dispatch }, - { - response: { - ...metricsDashboardResponse, - dashboard: { - ...metricsDashboardResponse.dashboard, - ...mockTemplatingData.allVariableTypes.dashboard, - }, - }, - }, - ); - - expect(commit).toHaveBeenCalledWith( - types.RECEIVE_METRICS_DASHBOARD_SUCCESS, - - response, - ); - }); - it('sets the dashboards loaded from the repository', () => { const params = {}; const response = metricsDashboardResponse; @@ -1144,11 +1117,13 @@ describe('Monitoring store actions', () => { describe('fetchVariableMetricLabelValues', () => { const variable = { type: 'metric_label_values', + name: 'label1', options: { - prometheusEndpointPath: '/series', + prometheusEndpointPath: '/series?match[]=metric_name', label: 'job', }, }; + const defaultQueryParams = { start_time: '2019-08-06T12:40:02.184Z', end_time: '2019-08-06T20:40:02.184Z', @@ -1158,9 +1133,7 @@ describe('Monitoring store actions', () => { state = { ...state, timeRange: defaultTimeRange, - variables: { - label1: variable, - }, + variables: [variable], }; }); @@ -1176,7 +1149,7 @@ describe('Monitoring store actions', () => { }, ]; - mock.onGet('/series').reply(200, { + mock.onGet('/series?match[]=metric_name').reply(200, { status: 'success', data, }); @@ -1196,7 +1169,7 @@ describe('Monitoring store actions', () => { }); it('should notify the user that dynamic options were not loaded', () => { - mock.onGet('/series').reply(500); + mock.onGet('/series?match[]=metric_name').reply(500); return testAction(fetchVariableMetricLabelValues, { defaultQueryParams }, state, [], []).then( () => { diff --git a/spec/frontend/monitoring/store/getters_spec.js b/spec/frontend/monitoring/store/getters_spec.js index 1275686de58..a69f5265ea7 100644 --- a/spec/frontend/monitoring/store/getters_spec.js +++ b/spec/frontend/monitoring/store/getters_spec.js @@ -8,7 +8,7 @@ import { environmentData, metricsResult, dashboardGitResponse, - mockTemplatingDataResponses, + storeVariables, mockLinks, } from '../mock_data'; import { @@ -344,19 +344,21 @@ describe('Monitoring store Getters', () => { }); it('transforms the variables object to an array in the [variable, variable_value] format for all variable types', () => { - mutations[types.SET_VARIABLES](state, mockTemplatingDataResponses.allVariableTypes); + state.variables = storeVariables; const variablesArray = getters.getCustomVariablesParams(state); expect(variablesArray).toEqual({ - 'variables[advCustomNormal]': 'value2', - 'variables[advText]': 'default', - 'variables[simpleCustom]': 'value1', - 'variables[simpleText]': 'Simple text', + 'variables[textSimple]': 'My default value', + 'variables[textAdvanced]': 'A default value', + 'variables[customSimple]': 'value1', + 'variables[customAdvanced]': 'value2', + 'variables[customAdvancedWithoutLabel]': 'value2', + 'variables[customAdvancedWithoutOptText]': 'value2', }); }); it('transforms the variables object to an empty array when no keys are present', () => { - mutations[types.SET_VARIABLES](state, {}); + state.variables = []; const variablesArray = getters.getCustomVariablesParams(state); expect(variablesArray).toEqual({}); diff --git a/spec/frontend/monitoring/store/mutations_spec.js b/spec/frontend/monitoring/store/mutations_spec.js index fb5e6156daf..37da5ea96d9 100644 --- a/spec/frontend/monitoring/store/mutations_spec.js +++ b/spec/frontend/monitoring/store/mutations_spec.js @@ -5,7 +5,7 @@ import * as types from '~/monitoring/stores/mutation_types'; import state from '~/monitoring/stores/state'; import { metricStates } from '~/monitoring/constants'; -import { deploymentData, dashboardGitResponse } from '../mock_data'; +import { deploymentData, dashboardGitResponse, storeTextVariables } from '../mock_data'; import { metricsDashboardPayload } from '../fixture_data'; describe('Monitoring mutations', () => { @@ -427,30 +427,12 @@ describe('Monitoring mutations', () => { }); }); - describe('SET_VARIABLES', () => { - it('stores an empty variables array when no custom variables are given', () => { - mutations[types.SET_VARIABLES](stateCopy, {}); - - expect(stateCopy.variables).toEqual({}); - }); - - it('stores variables in the key key_value format in the array', () => { - mutations[types.SET_VARIABLES](stateCopy, { pod: 'POD', stage: 'main ops' }); - - expect(stateCopy.variables).toEqual({ pod: 'POD', stage: 'main ops' }); - }); - }); - describe('UPDATE_VARIABLE_VALUE', () => { - afterEach(() => { - mutations[types.SET_VARIABLES](stateCopy, {}); - }); - it('updates only the value of the variable in variables', () => { - mutations[types.SET_VARIABLES](stateCopy, { environment: { value: 'prod', type: 'text' } }); - mutations[types.UPDATE_VARIABLE_VALUE](stateCopy, { key: 'environment', value: 'new prod' }); + stateCopy.variables = storeTextVariables; + mutations[types.UPDATE_VARIABLE_VALUE](stateCopy, { name: 'textSimple', value: 'New Value' }); - expect(stateCopy.variables).toEqual({ environment: { value: 'new prod', type: 'text' } }); + expect(stateCopy.variables[0].value).toEqual('New Value'); }); }); diff --git a/spec/frontend/monitoring/store/utils_spec.js b/spec/frontend/monitoring/store/utils_spec.js index 8ee37a529dd..b97948fa1bf 100644 --- a/spec/frontend/monitoring/store/utils_spec.js +++ b/spec/frontend/monitoring/store/utils_spec.js @@ -22,7 +22,7 @@ describe('mapToDashboardViewModel', () => { dashboard: '', panelGroups: [], links: [], - variables: {}, + variables: [], }); }); @@ -52,7 +52,7 @@ describe('mapToDashboardViewModel', () => { expect(mapToDashboardViewModel(response)).toEqual({ dashboard: 'Dashboard Name', links: [], - variables: {}, + variables: [], panelGroups: [ { group: 'Group 1', @@ -424,22 +424,20 @@ describe('mapToDashboardViewModel', () => { urlUtils.queryToObject.mockReturnValueOnce(); - expect(mapToDashboardViewModel(response)).toMatchObject({ - dashboard: 'Dashboard Name', - links: [], - variables: { - pod: { - label: 'pod', - type: 'text', - value: 'kubernetes', - }, - pod_2: { - label: 'pod_2', - type: 'text', - value: 'kubernetes-2', - }, + expect(mapToDashboardViewModel(response).variables).toEqual([ + { + name: 'pod', + label: 'pod', + type: 'text', + value: 'kubernetes', }, - }); + { + name: 'pod_2', + label: 'pod_2', + type: 'text', + value: 'kubernetes-2', + }, + ]); }); it('sets variables as-is from yml file if URL has no matching variables', () => { @@ -458,22 +456,20 @@ describe('mapToDashboardViewModel', () => { 'var-environment': 'POD', }); - expect(mapToDashboardViewModel(response)).toMatchObject({ - dashboard: 'Dashboard Name', - links: [], - variables: { - pod: { - label: 'pod', - type: 'text', - value: 'kubernetes', - }, - pod_2: { - label: 'pod_2', - type: 'text', - value: 'kubernetes-2', - }, + expect(mapToDashboardViewModel(response).variables).toEqual([ + { + label: 'pod', + name: 'pod', + type: 'text', + value: 'kubernetes', }, - }); + { + label: 'pod_2', + name: 'pod_2', + type: 'text', + value: 'kubernetes-2', + }, + ]); }); it('merges variables from URL with the ones from yml file', () => { @@ -494,22 +490,20 @@ describe('mapToDashboardViewModel', () => { 'var-pod_2': 'POD2', }); - expect(mapToDashboardViewModel(response)).toMatchObject({ - dashboard: 'Dashboard Name', - links: [], - variables: { - pod: { - label: 'pod', - type: 'text', - value: 'POD1', - }, - pod_2: { - label: 'pod_2', - type: 'text', - value: 'POD2', - }, + expect(mapToDashboardViewModel(response).variables).toEqual([ + { + label: 'pod', + name: 'pod', + type: 'text', + value: 'POD1', }, - }); + { + label: 'pod_2', + name: 'pod_2', + type: 'text', + value: 'POD2', + }, + ]); }); }); }); diff --git a/spec/frontend/monitoring/store/variable_mapping_spec.js b/spec/frontend/monitoring/store/variable_mapping_spec.js index 390cb2d8eac..de124b0313c 100644 --- a/spec/frontend/monitoring/store/variable_mapping_spec.js +++ b/spec/frontend/monitoring/store/variable_mapping_spec.js @@ -3,29 +3,31 @@ import { mergeURLVariables, optionsFromSeriesData, } from '~/monitoring/stores/variable_mapping'; +import { + templatingVariablesExamples, + storeTextVariables, + storeCustomVariables, + storeMetricLabelValuesVariables, +} from '../mock_data'; import * as urlUtils from '~/lib/utils/url_utility'; -import { mockTemplatingData, mockTemplatingDataResponses } from '../mock_data'; describe('Monitoring variable mapping', () => { describe('parseTemplatingVariables', () => { it.each` - case | input | expected - ${'Returns empty object for no dashboard input'} | ${{}} | ${{}} - ${'Returns empty object for empty dashboard input'} | ${{ dashboard: {} }} | ${{}} - ${'Returns empty object for empty templating prop'} | ${mockTemplatingData.emptyTemplatingProp} | ${{}} - ${'Returns empty object for empty variables prop'} | ${mockTemplatingData.emptyVariablesProp} | ${{}} - ${'Returns parsed object for simple text variable'} | ${mockTemplatingData.simpleText} | ${mockTemplatingDataResponses.simpleText} - ${'Returns parsed object for advanced text variable'} | ${mockTemplatingData.advText} | ${mockTemplatingDataResponses.advText} - ${'Returns parsed object for simple custom variable'} | ${mockTemplatingData.simpleCustom} | ${mockTemplatingDataResponses.simpleCustom} - ${'Returns parsed object for advanced custom variable without options'} | ${mockTemplatingData.advCustomWithoutOpts} | ${mockTemplatingDataResponses.advCustomWithoutOpts} - ${'Returns parsed object for advanced custom variable for option without text'} | ${mockTemplatingData.advCustomWithoutOptText} | ${mockTemplatingDataResponses.advCustomWithoutOptText} - ${'Returns parsed object for advanced custom variable without type'} | ${mockTemplatingData.advCustomWithoutType} | ${{}} - ${'Returns parsed object for advanced custom variable without label'} | ${mockTemplatingData.advCustomWithoutLabel} | ${mockTemplatingDataResponses.advCustomWithoutLabel} - ${'Returns parsed object for simple and advanced custom variables'} | ${mockTemplatingData.simpleAndAdv} | ${mockTemplatingDataResponses.simpleAndAdv} - ${'Returns parsed object for metricLabelValues'} | ${mockTemplatingData.metricLabelValues} | ${mockTemplatingDataResponses.metricLabelValues} - ${'Returns parsed object for all variable types'} | ${mockTemplatingData.allVariableTypes} | ${mockTemplatingDataResponses.allVariableTypes} - `('$case', ({ input, expected }) => { - expect(parseTemplatingVariables(input?.dashboard?.templating)).toEqual(expected); + case | input + ${'For undefined templating object'} | ${undefined} + ${'For empty templating object'} | ${{}} + `('$case, returns an empty array', ({ input }) => { + expect(parseTemplatingVariables(input)).toEqual([]); + }); + + it.each` + case | input | output + ${'Returns parsed object for text variables'} | ${templatingVariablesExamples.text} | ${storeTextVariables} + ${'Returns parsed object for custom variables'} | ${templatingVariablesExamples.custom} | ${storeCustomVariables} + ${'Returns parsed object for metric label value variables'} | ${templatingVariablesExamples.metricLabelValues} | ${storeMetricLabelValuesVariables} + `('$case, returns an empty array', ({ input, output }) => { + expect(parseTemplatingVariables(input)).toEqual(output); }); }); @@ -41,7 +43,7 @@ describe('Monitoring variable mapping', () => { it('returns empty object if variables are not defined in yml or URL', () => { urlUtils.queryToObject.mockReturnValueOnce({}); - expect(mergeURLVariables({})).toEqual({}); + expect(mergeURLVariables([])).toEqual([]); }); it('returns empty object if variables are defined in URL but not in yml', () => { @@ -50,18 +52,24 @@ describe('Monitoring variable mapping', () => { 'var-instance': 'localhost', }); - expect(mergeURLVariables({})).toEqual({}); + expect(mergeURLVariables([])).toEqual([]); }); it('returns yml variables if variables defined in yml but not in the URL', () => { urlUtils.queryToObject.mockReturnValueOnce({}); - const params = { - env: 'one', - instance: 'localhost', - }; + const variables = [ + { + name: 'env', + value: 'one', + }, + { + name: 'instance', + value: 'localhost', + }, + ]; - expect(mergeURLVariables(params)).toEqual(params); + expect(mergeURLVariables(variables)).toEqual(variables); }); it('returns yml variables if variables defined in URL do not match with yml variables', () => { @@ -69,13 +77,19 @@ describe('Monitoring variable mapping', () => { 'var-env': 'one', 'var-instance': 'localhost', }; - const ymlParams = { - pod: { value: 'one' }, - service: { value: 'database' }, - }; + const variables = [ + { + name: 'env', + value: 'one', + }, + { + name: 'service', + value: 'database', + }, + ]; urlUtils.queryToObject.mockReturnValueOnce(urlParams); - expect(mergeURLVariables(ymlParams)).toEqual(ymlParams); + expect(mergeURLVariables(variables)).toEqual(variables); }); it('returns merged yml and URL variables if there is some match', () => { @@ -83,19 +97,29 @@ describe('Monitoring variable mapping', () => { 'var-env': 'one', 'var-instance': 'localhost:8080', }; - const ymlParams = { - instance: { value: 'localhost' }, - service: { value: 'database' }, - }; - - const merged = { - instance: { value: 'localhost:8080' }, - service: { value: 'database' }, - }; + const variables = [ + { + name: 'instance', + value: 'localhost', + }, + { + name: 'service', + value: 'database', + }, + ]; urlUtils.queryToObject.mockReturnValueOnce(urlParams); - expect(mergeURLVariables(ymlParams)).toEqual(merged); + expect(mergeURLVariables(variables)).toEqual([ + { + name: 'instance', + value: 'localhost:8080', + }, + { + name: 'service', + value: 'database', + }, + ]); }); }); diff --git a/spec/frontend/monitoring/store_utils.js b/spec/frontend/monitoring/store_utils.js index 740dbaaa2e3..6c8267e6a3c 100644 --- a/spec/frontend/monitoring/store_utils.js +++ b/spec/frontend/monitoring/store_utils.js @@ -35,12 +35,6 @@ export const setupStoreWithDashboard = store => { ); }; -export const setupStoreWithVariable = store => { - store.commit(`monitoringDashboard/${types.SET_VARIABLES}`, { - label1: 'pod', - }); -}; - export const setupStoreWithLinks = store => { store.commit(`monitoringDashboard/${types.RECEIVE_METRICS_DASHBOARD_SUCCESS}`, { ...metricsDashboardPayload, diff --git a/spec/frontend/monitoring/utils_spec.js b/spec/frontend/monitoring/utils_spec.js index 039cf275eea..1b4df286868 100644 --- a/spec/frontend/monitoring/utils_spec.js +++ b/spec/frontend/monitoring/utils_spec.js @@ -429,14 +429,41 @@ describe('monitoring/utils', () => { describe('convertVariablesForURL', () => { it.each` - input | expected - ${undefined} | ${{}} - ${null} | ${{}} - ${{}} | ${{}} - ${{ env: { value: 'prod' } }} | ${{ 'var-env': 'prod' }} - ${{ 'var-env': { value: 'prod' } }} | ${{ 'var-var-env': 'prod' }} + input | expected + ${[]} | ${{}} + ${[{ name: 'env', value: 'prod' }]} | ${{ 'var-env': 'prod' }} + ${[{ name: 'env1', value: 'prod' }, { name: 'env2', value: null }]} | ${{ 'var-env1': 'prod' }} + ${[{ name: 'var-env', value: 'prod' }]} | ${{ 'var-var-env': 'prod' }} `('convertVariablesForURL returns $expected with input $input', ({ input, expected }) => { expect(monitoringUtils.convertVariablesForURL(input)).toEqual(expected); }); }); + + describe('setCustomVariablesFromUrl', () => { + beforeEach(() => { + jest.spyOn(urlUtils, 'updateHistory'); + }); + + afterEach(() => { + urlUtils.updateHistory.mockRestore(); + }); + + it.each` + input | urlParams + ${[]} | ${''} + ${[{ name: 'env', value: 'prod' }]} | ${'?var-env=prod'} + ${[{ name: 'env1', value: 'prod' }, { name: 'env2', value: null }]} | ${'?var-env=prod&var-env1=prod'} + `( + 'setCustomVariablesFromUrl updates history with query "$urlParams" with input $input', + ({ input, urlParams }) => { + monitoringUtils.setCustomVariablesFromUrl(input); + + expect(urlUtils.updateHistory).toHaveBeenCalledTimes(1); + expect(urlUtils.updateHistory).toHaveBeenCalledWith({ + url: `http://localhost/${urlParams}`, + title: '', + }); + }, + ); + }); }); diff --git a/spec/helpers/notify_helper_spec.rb b/spec/helpers/notify_helper_spec.rb new file mode 100644 index 00000000000..5b2a06b11e9 --- /dev/null +++ b/spec/helpers/notify_helper_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe NotifyHelper do + include ActionView::Helpers::UrlHelper + + describe 'merge_request_reference_link' do + let(:project) { create(:project) } + let(:merge_request) { create(:merge_request, source_project: project) } + + it 'returns link to merge request with the text reference' do + url = "http://test.host/#{project.full_path}/-/merge_requests/#{merge_request.iid}" + + expect(merge_request_reference_link(merge_request)).to eq(reference_link(merge_request, url)) + end + end + + describe 'issue_reference_link' do + let(:project) { create(:project) } + let(:issue) { create(:issue, project: project) } + + it 'returns link to issue with the text reference' do + url = "http://test.host/#{project.full_path}/-/issues/#{issue.iid}" + + expect(issue_reference_link(issue)).to eq(reference_link(issue, url)) + end + end + + def reference_link(entity, url) + "<a href=\"#{url}\">#{entity.to_reference}</a>" + end +end diff --git a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb index f9daff91871..bc2012e83bd 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/command_spec.rb @@ -270,4 +270,29 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Command do it { is_expected. to eq(true) } end end + + describe '#dangling_build?' do + let(:project) { create(:project, :repository) } + let(:command) { described_class.new(project: project, source: source) } + + subject { command.dangling_build? } + + context 'when source is :webide' do + let(:source) { :webide } + + it { is_expected.to eq(true) } + end + + context 'when source is :ondemand_dast_scan' do + let(:source) { :ondemand_dast_scan } + + it { is_expected.to eq(true) } + end + + context 'when source something else' do + let(:source) { :web } + + it { is_expected.to eq(false) } + end + end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/config/content_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/config/content_spec.rb index 6adf2a59b82..42ec9ab6f5d 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/config/content_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/config/content_spec.rb @@ -6,7 +6,8 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Config::Content do let(:project) { create(:project, ci_config_path: ci_config_path) } let(:pipeline) { build(:ci_pipeline, project: project) } let(:content) { nil } - let(:command) { Gitlab::Ci::Pipeline::Chain::Command.new(project: project, content: content) } + let(:source) { :push } + let(:command) { Gitlab::Ci::Pipeline::Chain::Command.new(project: project, content: content, source: source) } subject { described_class.new(pipeline, command) } @@ -143,6 +144,7 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Config::Content do end context 'when config is passed as a parameter' do + let(:source) { :ondemand_dast_scan } let(:ci_config_path) { nil } let(:content) do <<~EOY diff --git a/spec/lib/gitlab/danger/sidekiq_queues_spec.rb b/spec/lib/gitlab/danger/sidekiq_queues_spec.rb index afae22eda20..7dd1a2e6924 100644 --- a/spec/lib/gitlab/danger/sidekiq_queues_spec.rb +++ b/spec/lib/gitlab/danger/sidekiq_queues_spec.rb @@ -62,5 +62,21 @@ RSpec.describe Gitlab::Danger::SidekiqQueues do expect(sidekiq_queues.changed_queue_names).to contain_exactly(:post_receive, :process_commit) end + + it 'ignores removed queues' do + old_queues = { + merge: { name: :merge, urgency: :low }, + post_receive: { name: :post_receive, urgency: :high } + } + + new_queues = { + post_receive: { name: :post_receive, urgency: :low } + } + + allow(sidekiq_queues).to receive(:old_queues).and_return(old_queues) + allow(sidekiq_queues).to receive(:new_queues).and_return(new_queues) + + expect(sidekiq_queues.changed_queue_names).to contain_exactly(:post_receive) + end end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 9e613749bdf..7c1eb66b543 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -105,6 +105,7 @@ RSpec.describe Notify do it 'contains a link to issue author' do is_expected.to have_body_text(issue.author_name) is_expected.to have_body_text 'created an issue' + is_expected.to have_link(issue.to_reference, href: project_issue_url(issue.project, issue)) end it 'contains a link to the issue' do @@ -467,6 +468,7 @@ RSpec.describe Notify do is_expected.to have_body_text(status) is_expected.to have_body_text(current_user_sanitized) is_expected.to have_body_text(project_merge_request_path(project, merge_request)) + is_expected.to have_link(merge_request.to_reference, href: project_merge_request_url(merge_request.target_project, merge_request)) end end end @@ -497,6 +499,7 @@ RSpec.describe Notify do is_expected.to have_referable_subject(merge_request, reply: true) is_expected.to have_body_text('merged') is_expected.to have_body_text(project_merge_request_path(project, merge_request)) + is_expected.to have_link(merge_request.to_reference, href: project_merge_request_url(merge_request.target_project, merge_request)) end end end @@ -534,6 +537,7 @@ RSpec.describe Notify do is_expected.to have_referable_subject(merge_request, reply: true) is_expected.to have_body_text(project_merge_request_path(project, merge_request)) is_expected.to have_body_text('due to conflict.') + is_expected.to have_link(merge_request.to_reference, href: project_merge_request_url(merge_request.target_project, merge_request)) end end end @@ -567,6 +571,7 @@ RSpec.describe Notify do is_expected.to have_referable_subject(merge_request, reply: true) is_expected.to have_body_text("#{push_user.name} pushed new commits") is_expected.to have_body_text(project_merge_request_path(project, merge_request)) + is_expected.to have_link(merge_request.to_reference, href: project_merge_request_url(merge_request.target_project, merge_request)) end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index d71cd843a47..68f1a0f1ba1 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -425,6 +425,73 @@ RSpec.describe API::MergeRequests do end end + context 'NOT params' do + let(:merge_request2) do + create( + :merge_request, + :simple, + milestone: milestone, + author: user, + assignees: [user], + merge_request_context_commits: [merge_request_context_commit], + source_project: project, + target_project: project, + source_branch: 'what', + title: "What", + created_at: base_time + ) + end + + before do + create(:label_link, label: label, target: merge_request) + create(:label_link, label: label2, target: merge_request2) + end + + it 'returns merge requests without any of the labels given', :aggregate_failures do + get api(endpoint_path, user), params: { not: { labels: ["#{label.title}, #{label2.title}"] } } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(3) + json_response.each do |mr| + expect(mr['labels']).not_to include(label2.title, label.title) + end + end + + it 'returns merge requests without any of the milestones given', :aggregate_failures do + get api(endpoint_path, user), params: { not: { milestone: milestone.title } } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(4) + json_response.each do |mr| + expect(mr['milestone']).not_to eq(milestone.title) + end + end + + it 'returns merge requests without the author given', :aggregate_failures do + get api(endpoint_path, user), params: { not: { author_id: user2.id } } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(5) + json_response.each do |mr| + expect(mr['author']['id']).not_to eq(user2.id) + end + end + + it 'returns merge requests without the assignee given', :aggregate_failures do + get api(endpoint_path, user), params: { not: { assignee_id: user2.id } } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(5) + json_response.each do |mr| + expect(mr['assignee']['id']).not_to eq(user2.id) + end + end + end + context 'source_branch param' do it 'returns merge requests with the given source branch' do get api(endpoint_path, user), params: { source_branch: merge_request_closed.source_branch, state: 'all' } diff --git a/spec/services/ci/create_pipeline_service/parameter_content_spec.rb b/spec/services/ci/create_pipeline_service/parameter_content_spec.rb index a0b62ad5194..5157574ea04 100644 --- a/spec/services/ci/create_pipeline_service/parameter_content_spec.rb +++ b/spec/services/ci/create_pipeline_service/parameter_content_spec.rb @@ -28,23 +28,34 @@ RSpec.describe Ci::CreatePipelineService do end describe '#execute' do - subject { service.execute(:web, content: content) } + context 'when source is a dangling build' do + subject { service.execute(:ondemand_dast_scan, content: content) } - context 'parameter config content' do - it 'creates a pipeline' do - expect(subject).to be_persisted - end + context 'parameter config content' do + it 'creates a pipeline' do + expect(subject).to be_persisted + end - it 'creates builds with the correct names' do - expect(subject.builds.pluck(:name)).to match_array %w[dast] - end + it 'creates builds with the correct names' do + expect(subject.builds.pluck(:name)).to match_array %w[dast] + end + + it 'creates stages with the correct names' do + expect(subject.stages.pluck(:name)).to match_array %w[dast] + end - it 'creates stages with the correct names' do - expect(subject.stages.pluck(:name)).to match_array %w[dast] + it 'sets the correct config source' do + expect(subject.config_source).to eq 'parameter_source' + end end + end + + context 'when source is not a dangling build' do + subject { service.execute(:web, content: content) } - it 'sets the correct config source' do - expect(subject.config_source).to eq 'parameter_source' + it 'raises an exception' do + klass = Gitlab::Ci::Pipeline::Chain::Config::Content::Parameter::UnsupportedSourceError + expect { subject }.to raise_error(klass) end end end diff --git a/spec/services/event_create_service_spec.rb b/spec/services/event_create_service_spec.rb index 6a9d8857b32..95ee6fe556c 100644 --- a/spec/services/event_create_service_spec.rb +++ b/spec/services/event_create_service_spec.rb @@ -87,6 +87,18 @@ RSpec.describe EventCreateService do expect { service.reopen_mr(merge_request, merge_request.author) }.to change { ResourceStateEvent.count } end end + + describe '#approve_mr' do + let(:merge_request) { create(:merge_request) } + + it { expect(service.approve_mr(merge_request, user)).to be_truthy } + + it 'creates new event' do + service.approve_mr(merge_request, user) + + change { Event.approved_action.where(target: merge_request).count }.by(1) + end + end end describe 'Milestone' do diff --git a/spec/services/merge_requests/approval_service_spec.rb b/spec/services/merge_requests/approval_service_spec.rb new file mode 100644 index 00000000000..68b9caa30ab --- /dev/null +++ b/spec/services/merge_requests/approval_service_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::ApprovalService do + describe '#execute' do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + let!(:todo) { create(:todo, user: user, project: project, target: merge_request) } + + subject(:service) { described_class.new(project, user) } + + context 'with invalid approval' do + before do + allow(merge_request.approvals).to receive(:new).and_return(double(save: false)) + end + + it 'does not create an approval note' do + expect(SystemNoteService).not_to receive(:approve_mr) + + service.execute(merge_request) + end + + it 'does not mark pending todos as done' do + service.execute(merge_request) + + expect(todo.reload).to be_pending + end + end + + context 'with valid approval' do + it 'creates an approval note and marks pending todos as done' do + expect(SystemNoteService).to receive(:approve_mr).with(merge_request, user) + expect(merge_request.approvals).to receive(:reset) + + service.execute(merge_request) + + expect(todo.reload).to be_done + end + + it 'creates approve MR event' do + expect_next_instance_of(EventCreateService) do |instance| + expect(instance).to receive(:approve_mr) + .with(merge_request, user) + end + + service.execute(merge_request) + end + + context 'with remaining approvals' do + it 'fires an approval webhook' do + expect(service).to receive(:execute_hooks).with(merge_request, 'approved') + + service.execute(merge_request) + end + end + end + end +end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 2a1cb3e0585..4d265258449 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -661,4 +661,14 @@ RSpec.describe SystemNoteService do described_class.design_discussion_added(discussion_note) end end + + describe '.approve_mr' do + it 'calls MergeRequestsService' do + expect_next_instance_of(::SystemNotes::MergeRequestsService) do |service| + expect(service).to receive(:approve_mr) + end + + described_class.approve_mr(noteable, author) + end + end end diff --git a/spec/services/system_notes/merge_requests_service_spec.rb b/spec/services/system_notes/merge_requests_service_spec.rb index 1c378123797..17155969a33 100644 --- a/spec/services/system_notes/merge_requests_service_spec.rb +++ b/spec/services/system_notes/merge_requests_service_spec.rb @@ -261,4 +261,18 @@ RSpec.describe ::SystemNotes::MergeRequestsService do expect(subject.commit_id).to eq(commit_sha) end end + + describe '#approve_mr' do + subject { described_class.new(noteable: noteable, project: project, author: author).approve_mr } + + it_behaves_like 'a system note' do + let(:action) { 'approved' } + end + + context 'when merge request approved' do + it 'sets the note text' do + expect(subject.note).to eq "approved this merge request" + end + end + end end diff --git a/spec/support/shared_contexts/finders/merge_requests_finder_shared_contexts.rb b/spec/support/shared_contexts/finders/merge_requests_finder_shared_contexts.rb index 2b8daa80ab4..07b6b98222f 100644 --- a/spec/support/shared_contexts/finders/merge_requests_finder_shared_contexts.rb +++ b/spec/support/shared_contexts/finders/merge_requests_finder_shared_contexts.rb @@ -23,12 +23,12 @@ RSpec.shared_context 'MergeRequestsFinder multiple projects with merge requests # We cannot use `let_it_be` here otherwise we get: # Failure/Error: allow(RepositoryForkWorker).to receive(:perform_async).and_return(true) # The use of doubles or partial doubles from rspec-mocks outside of the per-test lifecycle is not supported. - let(:project2) do + let!(:project2) do allow_gitaly_n_plus_1 do fork_project(project1, user) end end - let(:project3) do + let!(:project3) do allow_gitaly_n_plus_1 do fork_project(project1, user).tap do |project| project.update!(archived: true) @@ -45,6 +45,9 @@ RSpec.shared_context 'MergeRequestsFinder multiple projects with merge requests allow_gitaly_n_plus_1 { create(:project, group: subgroup) } end + let!(:label) { create(:label, project: project1) } + let!(:label2) { create(:label, project: project1) } + let!(:merge_request1) do create(:merge_request, assignees: [user], author: user, source_project: project2, target_project: project1, @@ -72,6 +75,9 @@ RSpec.shared_context 'MergeRequestsFinder multiple projects with merge requests title: '[WIP]') end + let!(:label_link) { create(:label_link, label: label, target: merge_request2) } + let!(:label_link2) { create(:label_link, label: label2, target: merge_request3) } + before do project1.add_maintainer(user) project2.add_developer(user) |