diff options
Diffstat (limited to 'spec')
25 files changed, 405 insertions, 55 deletions
diff --git a/spec/features/admin/admin_settings_spec.rb b/spec/features/admin/admin_settings_spec.rb index ed05ad60ff0..8eb15bb6bf5 100644 --- a/spec/features/admin/admin_settings_spec.rb +++ b/spec/features/admin/admin_settings_spec.rb @@ -489,7 +489,6 @@ describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_moc end def check_all_events - page.check('Active') page.check('Push') page.check('Issue') page.check('Confidential issue') diff --git a/spec/features/dashboard/snippets_spec.rb b/spec/features/dashboard/snippets_spec.rb index db5e56bdde0..287af7e7b11 100644 --- a/spec/features/dashboard/snippets_spec.rb +++ b/spec/features/dashboard/snippets_spec.rb @@ -3,8 +3,10 @@ require 'spec_helper' describe 'Dashboard snippets' do + let_it_be(:user) { create(:user) } + context 'when the project has snippets' do - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, creator: user) } let!(:snippets) { create_list(:project_snippet, 2, :public, author: project.owner, project: project) } before do @@ -22,7 +24,7 @@ describe 'Dashboard snippets' do end context 'when there are no project snippets', :js do - let(:project) { create(:project, :public) } + let(:project) { create(:project, :public, creator: user) } before do sign_in(project.owner) @@ -47,9 +49,49 @@ describe 'Dashboard snippets' do end end + context 'rendering file names' do + let_it_be(:snippet) { create(:personal_snippet, :public, author: user, file_name: 'foo.txt') } + let_it_be(:versioned_snippet) { create(:personal_snippet, :repository, :public, author: user, file_name: 'bar.txt') } + + before do + sign_in(user) + end + + context 'when feature flag :version_snippets is disabled' do + before do + stub_feature_flags(version_snippets: false) + + visit dashboard_snippets_path + end + + it 'contains the snippet file names from the DB' do + aggregate_failures do + expect(page).to have_content 'foo.txt' + expect(page).to have_content('bar.txt') + expect(page).not_to have_content('.gitattributes') + end + end + end + + context 'when feature flag :version_snippets is enabled' do + before do + stub_feature_flags(version_snippets: true) + + visit dashboard_snippets_path + end + + it 'contains both the versioned and non-versioned filenames' do + aggregate_failures do + expect(page).to have_content 'foo.txt' + expect(page).to have_content('.gitattributes') + expect(page).not_to have_content('bar.txt') + end + end + end + end + context 'filtering by visibility' do - let(:user) { create(:user) } - let!(:snippets) do + let_it_be(:snippets) do [ create(:personal_snippet, :public, author: user), create(:personal_snippet, :internal, author: user), @@ -99,7 +141,7 @@ describe 'Dashboard snippets' do end context 'as an external user' do - let(:user) { create(:user, :external) } + let_it_be(:user) { create(:user, :external) } before do sign_in(user) diff --git a/spec/features/projects/services/user_activates_issue_tracker_spec.rb b/spec/features/projects/services/user_activates_issue_tracker_spec.rb index 0b0a3362043..4f3fb6ac3bf 100644 --- a/spec/features/projects/services/user_activates_issue_tracker_spec.rb +++ b/spec/features/projects/services/user_activates_issue_tracker_spec.rb @@ -9,7 +9,7 @@ describe 'User activates issue tracker', :js do let(:url) { 'http://tracker.example.com' } def fill_short_form(disabled: false) - uncheck 'Active' if disabled + find('input[name="service[active]"] + button').click if disabled fill_in 'service_project_url', with: url fill_in 'service_issues_url', with: "#{url}/:id" diff --git a/spec/features/projects/services/user_activates_jira_spec.rb b/spec/features/projects/services/user_activates_jira_spec.rb index 557615f8872..fb9628032b2 100644 --- a/spec/features/projects/services/user_activates_jira_spec.rb +++ b/spec/features/projects/services/user_activates_jira_spec.rb @@ -10,7 +10,7 @@ describe 'User activates Jira', :js do let(:test_url) { 'http://jira.example.com/rest/api/2/serverInfo' } def fill_form(disabled: false) - uncheck 'Active' if disabled + find('input[name="service[active]"] + button').click if disabled fill_in 'service_url', with: url fill_in 'service_username', with: 'username' @@ -53,7 +53,6 @@ describe 'User activates Jira', :js do it 'shows errors when some required fields are not filled in' do click_link('Jira') - check 'Active' fill_in 'service_password', with: 'password' click_button('Test settings and save changes') diff --git a/spec/features/projects/services/user_activates_mattermost_slash_command_spec.rb b/spec/features/projects/services/user_activates_mattermost_slash_command_spec.rb index 2eaa2d24c4b..ac9cb00be84 100644 --- a/spec/features/projects/services/user_activates_mattermost_slash_command_spec.rb +++ b/spec/features/projects/services/user_activates_mattermost_slash_command_spec.rb @@ -5,14 +5,13 @@ require 'spec_helper' describe 'Set up Mattermost slash commands', :js do let(:user) { create(:user) } let(:project) { create(:project) } - let(:service) { project.create_mattermost_slash_commands_service } let(:mattermost_enabled) { true } before do stub_mattermost_setting(enabled: mattermost_enabled) project.add_maintainer(user) sign_in(user) - visit edit_project_service_path(project, service) + visit edit_project_service_path(project, :mattermost_slash_commands) end describe 'user visits the mattermost slash command config page' do @@ -30,6 +29,7 @@ describe 'Set up Mattermost slash commands', :js do token = ('a'..'z').to_a.join fill_in 'service_token', with: token + find('input[name="service[active]"] + button').click click_on 'Save changes' expect(current_path).to eq(project_settings_integrations_path(project)) @@ -40,7 +40,6 @@ describe 'Set up Mattermost slash commands', :js do token = ('a'..'z').to_a.join fill_in 'service_token', with: token - check 'service_active' click_on 'Save changes' expect(current_path).to eq(project_settings_integrations_path(project)) diff --git a/spec/features/projects/services/user_activates_slack_slash_command_spec.rb b/spec/features/projects/services/user_activates_slack_slash_command_spec.rb index 752ef8d592d..4ce1acd9377 100644 --- a/spec/features/projects/services/user_activates_slack_slash_command_spec.rb +++ b/spec/features/projects/services/user_activates_slack_slash_command_spec.rb @@ -5,12 +5,11 @@ require 'spec_helper' describe 'Slack slash commands' do let(:user) { create(:user) } let(:project) { create(:project) } - let(:service) { project.create_slack_slash_commands_service } before do project.add_maintainer(user) sign_in(user) - visit edit_project_service_path(project, service) + visit edit_project_service_path(project, :slack_slash_commands) end it 'shows a token placeholder' do @@ -23,17 +22,17 @@ describe 'Slack slash commands' do expect(page).to have_content('This service allows users to perform common') end - it 'redirects to the integrations page after saving but not activating' do + it 'redirects to the integrations page after saving but not activating', :js do fill_in 'service_token', with: 'token' + find('input[name="service[active]"] + button').click click_on 'Save' expect(current_path).to eq(project_settings_integrations_path(project)) expect(page).to have_content('Slack slash commands settings saved, but not activated.') end - it 'redirects to the integrations page after activating' do + it 'redirects to the integrations page after activating', :js do fill_in 'service_token', with: 'token' - check 'service_active' click_on 'Save' expect(current_path).to eq(project_settings_integrations_path(project)) diff --git a/spec/features/projects/services/user_activates_youtrack_spec.rb b/spec/features/projects/services/user_activates_youtrack_spec.rb index 2f6aad1d736..26734766ff0 100644 --- a/spec/features/projects/services/user_activates_youtrack_spec.rb +++ b/spec/features/projects/services/user_activates_youtrack_spec.rb @@ -9,7 +9,7 @@ describe 'User activates issue tracker', :js do let(:url) { 'http://tracker.example.com' } def fill_form(disabled: false) - uncheck 'Active' if disabled + find('input[name="service[active]"] + button').click if disabled fill_in 'service_project_url', with: url fill_in 'service_issues_url', with: "#{url}/:id" diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml b/spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml index 638ecbcc11f..b460a031486 100644 --- a/spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml @@ -8,6 +8,7 @@ panel_groups: type: "area-chart" y_label: "y_label" weight: 1 + max_value: 1 metrics: - id: metric_a1 query_range: 'query' diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panels.json b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panels.json index fe2da16c9b7..20595cc0d73 100644 --- a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panels.json +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/panels.json @@ -11,6 +11,7 @@ "type": { "type": "string" }, "y_label": { "type": "string" }, "y_axis": { "$ref": "axis.json" }, + "max_value": { "type": "number" }, "weight": { "type": "number" }, "metrics": { "type": "array", diff --git a/spec/frontend/integrations/edit/components/active_toggle_spec.js b/spec/frontend/integrations/edit/components/active_toggle_spec.js new file mode 100644 index 00000000000..8a11c200c15 --- /dev/null +++ b/spec/frontend/integrations/edit/components/active_toggle_spec.js @@ -0,0 +1,65 @@ +import { mount } from '@vue/test-utils'; +import ActiveToggle from '~/integrations/edit/components/active_toggle.vue'; +import { GlToggle } from '@gitlab/ui'; + +const GL_TOGGLE_ACTIVE_CLASS = 'is-checked'; + +describe('ActiveToggle', () => { + let wrapper; + + const defaultProps = { + initialActivated: true, + disabled: false, + }; + + const createComponent = props => { + wrapper = mount(ActiveToggle, { + propsData: Object.assign({}, defaultProps, props), + }); + }; + + afterEach(() => { + if (wrapper) wrapper.destroy(); + }); + + const findGlToggle = () => wrapper.find(GlToggle); + const findButtonInToggle = () => findGlToggle().find('button'); + const findInputInToggle = () => findGlToggle().find('input'); + + describe('template', () => { + describe('initialActivated is false', () => { + it('renders GlToggle as inactive', () => { + createComponent({ + initialActivated: false, + }); + + expect(findGlToggle().exists()).toBe(true); + expect(findButtonInToggle().classes()).not.toContain(GL_TOGGLE_ACTIVE_CLASS); + expect(findInputInToggle().attributes('value')).toBe('false'); + }); + }); + + describe('initialActivated is true', () => { + beforeEach(() => { + createComponent(); + }); + + it('renders GlToggle as active', () => { + expect(findGlToggle().exists()).toBe(true); + expect(findButtonInToggle().classes()).toContain(GL_TOGGLE_ACTIVE_CLASS); + expect(findInputInToggle().attributes('value')).toBe('true'); + }); + + describe('on toggle click', () => { + it('switches the form value', () => { + findButtonInToggle().trigger('click'); + + wrapper.vm.$nextTick(() => { + expect(findButtonInToggle().classes()).not.toContain(GL_TOGGLE_ACTIVE_CLASS); + expect(findInputInToggle().attributes('value')).toBe('false'); + }); + }); + }); + }); + }); +}); diff --git a/spec/frontend/notes/components/note_header_spec.js b/spec/frontend/notes/components/note_header_spec.js index 642ab5138dc..d477de69716 100644 --- a/spec/frontend/notes/components/note_header_spec.js +++ b/spec/frontend/notes/components/note_header_spec.js @@ -16,7 +16,9 @@ describe('NoteHeader component', () => { const findActionsWrapper = () => wrapper.find({ ref: 'discussionActions' }); const findChevronIcon = () => wrapper.find({ ref: 'chevronIcon' }); const findActionText = () => wrapper.find({ ref: 'actionText' }); + const findTimestampLink = () => wrapper.find({ ref: 'noteTimestampLink' }); const findTimestamp = () => wrapper.find({ ref: 'noteTimestamp' }); + const findSpinner = () => wrapper.find({ ref: 'spinner' }); const author = { avatar_url: null, @@ -33,11 +35,7 @@ describe('NoteHeader component', () => { store: new Vuex.Store({ actions, }), - propsData: { - ...props, - actionTextHtml: '', - noteId: '1394', - }, + propsData: { ...props }, }); }; @@ -108,17 +106,18 @@ describe('NoteHeader component', () => { createComponent(); expect(findActionText().exists()).toBe(false); - expect(findTimestamp().exists()).toBe(false); + expect(findTimestampLink().exists()).toBe(false); }); describe('when createdAt is passed as a prop', () => { it('renders action text and a timestamp', () => { createComponent({ createdAt: '2017-08-02T10:51:58.559Z', + noteId: 123, }); expect(findActionText().exists()).toBe(true); - expect(findTimestamp().exists()).toBe(true); + expect(findTimestampLink().exists()).toBe(true); }); it('renders correct actionText if passed', () => { @@ -133,8 +132,9 @@ describe('NoteHeader component', () => { it('calls an action when timestamp is clicked', () => { createComponent({ createdAt: '2017-08-02T10:51:58.559Z', + noteId: 123, }); - findTimestamp().trigger('click'); + findTimestampLink().trigger('click'); expect(actions.setTargetNoteHash).toHaveBeenCalled(); }); @@ -153,4 +153,30 @@ describe('NoteHeader component', () => { expect(wrapper.find(GitlabTeamMemberBadge).exists()).toBe(expected); }, ); + + describe('loading spinner', () => { + it('shows spinner when showSpinner is true', () => { + createComponent(); + expect(findSpinner().exists()).toBe(true); + }); + + it('does not show spinner when showSpinner is false', () => { + createComponent({ showSpinner: false }); + expect(findSpinner().exists()).toBe(false); + }); + }); + + describe('timestamp', () => { + it('shows timestamp as a link if a noteId was provided', () => { + createComponent({ createdAt: new Date().toISOString(), noteId: 123 }); + expect(findTimestampLink().exists()).toBe(true); + expect(findTimestamp().exists()).toBe(false); + }); + + it('shows timestamp as plain text if a noteId was not provided', () => { + createComponent({ createdAt: new Date().toISOString() }); + expect(findTimestampLink().exists()).toBe(false); + expect(findTimestamp().exists()).toBe(true); + }); + }); }); diff --git a/spec/helpers/snippets_helper_spec.rb b/spec/helpers/snippets_helper_spec.rb index 6fdf4f5cfb4..b5b431b5818 100644 --- a/spec/helpers/snippets_helper_spec.rb +++ b/spec/helpers/snippets_helper_spec.rb @@ -151,4 +151,35 @@ describe SnippetsHelper do "<input type=\"text\" readonly=\"readonly\" class=\"js-snippet-url-area snippet-embed-input form-control\" data-url=\"#{url}\" value=\"<script src="#{url}.js"></script>\" autocomplete=\"off\"></input>" end end + + describe '#snippet_file_name' do + subject { helper.snippet_file_name(snippet) } + + where(:snippet_type, :flag_enabled, :trait, :filename) do + [ + [:personal_snippet, false, nil, 'foo.txt'], + [:personal_snippet, true, nil, 'foo.txt'], + [:personal_snippet, false, :repository, 'foo.txt'], + [:personal_snippet, true, :repository, '.gitattributes'], + + [:project_snippet, false, nil, 'foo.txt'], + [:project_snippet, true, nil, 'foo.txt'], + [:project_snippet, false, :repository, 'foo.txt'], + [:project_snippet, true, :repository, '.gitattributes'] + ] + end + + with_them do + let(:snippet) { create(snippet_type, trait, file_name: 'foo.txt') } + + before do + allow(helper).to receive(:current_user).and_return(snippet.author) + stub_feature_flags(version_snippets: flag_enabled) + end + + it 'returns the correct filename' do + expect(subject).to eq filename + end + end + end end diff --git a/spec/javascripts/integrations/integration_settings_form_spec.js b/spec/javascripts/integrations/integration_settings_form_spec.js index 82d1f815ca8..72d04be822f 100644 --- a/spec/javascripts/integrations/integration_settings_form_spec.js +++ b/spec/javascripts/integrations/integration_settings_form_spec.js @@ -23,9 +23,9 @@ describe('IntegrationSettingsForm', () => { // Form Reference expect(integrationSettingsForm.$form).toBeDefined(); expect(integrationSettingsForm.$form.prop('nodeName')).toEqual('FORM'); + expect(integrationSettingsForm.formActive).toBeDefined(); // Form Child Elements - expect(integrationSettingsForm.$serviceToggle).toBeDefined(); expect(integrationSettingsForm.$submitBtn).toBeDefined(); expect(integrationSettingsForm.$submitBtnLoader).toBeDefined(); expect(integrationSettingsForm.$submitBtnLabel).toBeDefined(); @@ -45,13 +45,15 @@ describe('IntegrationSettingsForm', () => { }); it('should remove `novalidate` attribute to form when called with `true`', () => { - integrationSettingsForm.toggleServiceState(true); + integrationSettingsForm.formActive = true; + integrationSettingsForm.toggleServiceState(); expect(integrationSettingsForm.$form.attr('novalidate')).not.toBeDefined(); }); it('should set `novalidate` attribute to form when called with `false`', () => { - integrationSettingsForm.toggleServiceState(false); + integrationSettingsForm.formActive = false; + integrationSettingsForm.toggleServiceState(); expect(integrationSettingsForm.$form.attr('novalidate')).toBeDefined(); }); @@ -66,8 +68,9 @@ describe('IntegrationSettingsForm', () => { it('should set Save button label to "Test settings and save changes" when serviceActive & canTestService are `true`', () => { integrationSettingsForm.canTestService = true; + integrationSettingsForm.formActive = true; - integrationSettingsForm.toggleSubmitBtnLabel(true); + integrationSettingsForm.toggleSubmitBtnLabel(); expect(integrationSettingsForm.$submitBtnLabel.text()).toEqual( 'Test settings and save changes', @@ -76,18 +79,22 @@ describe('IntegrationSettingsForm', () => { it('should set Save button label to "Save changes" when either serviceActive or canTestService (or both) is `false`', () => { integrationSettingsForm.canTestService = false; + integrationSettingsForm.formActive = false; - integrationSettingsForm.toggleSubmitBtnLabel(false); + integrationSettingsForm.toggleSubmitBtnLabel(); expect(integrationSettingsForm.$submitBtnLabel.text()).toEqual('Save changes'); - integrationSettingsForm.toggleSubmitBtnLabel(true); + integrationSettingsForm.formActive = true; + + integrationSettingsForm.toggleSubmitBtnLabel(); expect(integrationSettingsForm.$submitBtnLabel.text()).toEqual('Save changes'); integrationSettingsForm.canTestService = true; + integrationSettingsForm.formActive = false; - integrationSettingsForm.toggleSubmitBtnLabel(false); + integrationSettingsForm.toggleSubmitBtnLabel(); expect(integrationSettingsForm.$submitBtnLabel.text()).toEqual('Save changes'); }); diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 515d72add92..5d5e2fe2a33 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -58,6 +58,7 @@ notes: - system_note_metadata - note_diff_file - suggestions +- diff_note_positions - review label_links: - target @@ -134,6 +135,7 @@ merge_requests: - pipelines_for_merge_request - merge_request_assignees - suggestions +- diff_note_positions - unresolved_notes - assignees - reviews @@ -517,6 +519,8 @@ error_tracking_setting: - project suggestions: - note +diff_note_positions: +- note metrics_setting: - project protected_environments: diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index 1eac580bc5e..80ae9a08257 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -25,7 +25,7 @@ describe Gitlab::ImportExport::Project::TreeRestorer, quarantine: { flaky: 'http @project = create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project') @shared = @project.import_export_shared - allow(Feature).to receive(:enabled?).and_call_original + allow(Feature).to receive(:enabled?) { true } stub_feature_flags(project_import_ndjson: ndjson_enabled) setup_import_export_config('complex') @@ -34,6 +34,7 @@ describe Gitlab::ImportExport::Project::TreeRestorer, quarantine: { flaky: 'http allow_any_instance_of(Repository).to receive(:fetch_source_branch!).and_return(true) allow_any_instance_of(Gitlab::Git::Repository).to receive(:branch_exists?).and_return(false) + expect(@shared).not_to receive(:error) expect_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch).with('feature', 'DCBA') allow_any_instance_of(Gitlab::Git::Repository).to receive(:create_branch) diff --git a/spec/lib/gitlab/import_export/project/tree_saver_spec.rb b/spec/lib/gitlab/import_export/project/tree_saver_spec.rb index ded57b1d576..8adc360026d 100644 --- a/spec/lib/gitlab/import_export/project/tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_saver_spec.rb @@ -29,12 +29,11 @@ describe Gitlab::ImportExport::Project::TreeSaver do before_all do RSpec::Mocks.with_temporary_scope do - allow(Feature).to receive(:enabled?).and_call_original + allow(Feature).to receive(:enabled?) { true } stub_feature_flags(project_export_as_ndjson: ndjson_enabled) project.add_maintainer(user) - stub_feature_flags(project_export_as_ndjson: ndjson_enabled) project_tree_saver = described_class.new(project: project, current_user: user, shared: shared) project_tree_saver.save diff --git a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb index 3cb02a8bcb3..b2fca0b5954 100644 --- a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb @@ -15,7 +15,8 @@ describe Gitlab::Metrics::Dashboard::Processor do Gitlab::Metrics::Dashboard::Stages::CustomMetricsDetailsInserter, Gitlab::Metrics::Dashboard::Stages::EndpointInserter, Gitlab::Metrics::Dashboard::Stages::Sorter, - Gitlab::Metrics::Dashboard::Stages::AlertsInserter + Gitlab::Metrics::Dashboard::Stages::AlertsInserter, + Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter ] end @@ -28,6 +29,12 @@ describe Gitlab::Metrics::Dashboard::Processor do end end + it 'includes an id for each dashboard panel' do + expect(all_panels).to satisfy_all do |panel| + panel[:id].present? + end + end + it 'includes boolean to indicate if panel group has custom metrics' do expect(dashboard[:panel_groups]).to all(include( { has_custom_metrics: boolean } )) end @@ -199,9 +206,11 @@ describe Gitlab::Metrics::Dashboard::Processor do private def all_metrics - dashboard[:panel_groups].flat_map do |group| - group[:panels].flat_map { |panel| panel[:metrics] } - end + all_panels.flat_map { |panel| panel[:metrics] } + end + + def all_panels + dashboard[:panel_groups].flat_map { |group| group[:panels] } end def get_metric_details(metric) diff --git a/spec/lib/gitlab/metrics/dashboard/stages/panel_ids_inserter_spec.rb b/spec/lib/gitlab/metrics/dashboard/stages/panel_ids_inserter_spec.rb index 426a54bea78..6124f471e39 100644 --- a/spec/lib/gitlab/metrics/dashboard/stages/panel_ids_inserter_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/stages/panel_ids_inserter_spec.rb @@ -63,5 +63,24 @@ describe Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter do ) end end + + context 'when dashboard panels has unknown schema attributes' do + before do + error = ActiveModel::UnknownAttributeError.new(double, 'unknown_panel_attribute') + allow(::PerformanceMonitoring::PrometheusPanel).to receive(:new).and_raise(error) + end + + it 'no panel has assigned id' do + transform! + + expect(fetch_panel_ids(dashboard)).to all be_nil + end + + it 'logs the failure' do + expect(Gitlab::ErrorTracking).to receive(:log_exception) + + transform! + end + end end end diff --git a/spec/models/diff_note_position_spec.rb b/spec/models/diff_note_position_spec.rb index a00ba35feef..dedb8a8da4d 100644 --- a/spec/models/diff_note_position_spec.rb +++ b/spec/models/diff_note_position_spec.rb @@ -3,14 +3,35 @@ require 'spec_helper' describe DiffNotePosition, type: :model do - it 'has a position attribute' do - diff_position = build(:diff_position) - line_code = 'bd4b7bfff3a247ccf6e3371c41ec018a55230bcc_534_521' - diff_note_position = build(:diff_note_position, line_code: line_code, position: diff_position) - - expect(diff_note_position.position).to eq(diff_position) - expect(diff_note_position.line_code).to eq(line_code) - expect(diff_note_position.diff_content_type).to eq('text') + describe '.create_or_update_by' do + context 'when a diff note' do + let(:note) { create(:diff_note_on_merge_request) } + let(:diff_position) { build(:diff_position) } + let(:line_code) { 'bd4b7bfff3a247ccf6e3371c41ec018a55230bcc_534_521' } + let(:diff_note_position) { note.diff_note_positions.first } + let(:params) { { diff_type: :head, line_code: line_code, position: diff_position } } + + context 'does not have a diff note position' do + it 'creates a diff note position' do + described_class.create_or_update_for(note, params) + + expect(diff_note_position.position).to eq(diff_position) + expect(diff_note_position.line_code).to eq(line_code) + expect(diff_note_position.diff_content_type).to eq('text') + end + end + + context 'has a diff note position' do + it 'updates the existing diff note position' do + create(:diff_note_position, note: note) + described_class.create_or_update_for(note, params) + + expect(note.diff_note_positions.size).to eq(1) + expect(diff_note_position.position).to eq(diff_position) + expect(diff_note_position.line_code).to eq(line_code) + end + end + end end it 'unique by note_id and diff type' do diff --git a/spec/services/discussions/capture_diff_note_position_service_spec.rb b/spec/services/discussions/capture_diff_note_position_service_spec.rb new file mode 100644 index 00000000000..fced2eb7fce --- /dev/null +++ b/spec/services/discussions/capture_diff_note_position_service_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Discussions::CaptureDiffNotePositionService do + context 'image note on diff' do + let!(:note) { create(:image_diff_note_on_merge_request) } + + subject { described_class.new(note.noteable, ['files/images/any_image.png']) } + + it 'is note affected by the service' do + expect(Gitlab::Diff::PositionTracer).not_to receive(:new) + + expect(subject.execute(note.discussion)).to eq(nil) + expect(note.diff_note_positions).to be_empty + end + end + + context 'when empty paths are passed as a param' do + let!(:note) { create(:diff_note_on_merge_request) } + + subject { described_class.new(note.noteable, []) } + + it 'does not calculate positons' do + expect(Gitlab::Diff::PositionTracer).not_to receive(:new) + + expect(subject.execute(note.discussion)).to eq(nil) + expect(note.diff_note_positions).to be_empty + end + end +end diff --git a/spec/services/discussions/capture_diff_note_positions_service_spec.rb b/spec/services/discussions/capture_diff_note_positions_service_spec.rb new file mode 100644 index 00000000000..7b1e207f3eb --- /dev/null +++ b/spec/services/discussions/capture_diff_note_positions_service_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Discussions::CaptureDiffNotePositionsService do + context 'when merge request has a discussion' do + let(:source_branch) { 'compare-with-merge-head-source' } + let(:target_branch) { 'compare-with-merge-head-target' } + let(:merge_request) { create(:merge_request, source_branch: source_branch, target_branch: target_branch) } + let(:project) { merge_request.project } + + let(:offset) { 30 } + let(:first_new_line) { 508 } + let(:second_new_line) { 521 } + + let(:service) { described_class.new(merge_request) } + + def build_position(new_line, diff_refs) + path = 'files/markdown/ruby-style-guide.md' + Gitlab::Diff::Position.new(old_path: path, new_path: path, + new_line: new_line, diff_refs: diff_refs) + end + + def note_for(new_line) + position = build_position(new_line, merge_request.diff_refs) + create(:diff_note_on_merge_request, project: project, position: position, noteable: merge_request) + end + + def verify_diff_note_position!(note, line) + id, old_line, new_line = note.line_code.split('_') + + expect(new_line).to eq(line.to_s) + expect(note.diff_note_positions.size).to eq(1) + + diff_position = note.diff_note_positions.last + diff_refs = Gitlab::Diff::DiffRefs.new( + base_sha: merge_request.target_branch_sha, + start_sha: merge_request.target_branch_sha, + head_sha: merge_request.merge_ref_head.sha) + + expect(diff_position.line_code).to eq("#{id}_#{old_line.to_i - offset}_#{new_line}") + expect(diff_position.position).to eq(build_position(new_line.to_i, diff_refs)) + end + + let!(:first_discussion_note) { note_for(first_new_line) } + let!(:second_discussion_note) { note_for(second_new_line) } + let!(:second_discussion_another_note) do + create(:diff_note_on_merge_request, + project: project, + position: second_discussion_note.position, + discussion_id: second_discussion_note.discussion_id, + noteable: merge_request) + end + + context 'and position of the discussion changed on target branch head' do + it 'diff positions are created for the first notes of the discussions' do + MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request) + service.execute + + verify_diff_note_position!(first_discussion_note, first_new_line) + verify_diff_note_position!(second_discussion_note, second_new_line) + + expect(second_discussion_another_note.diff_note_positions).to be_empty + end + end + end +end diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb index 8f17e8083e3..45519ddf3d3 100644 --- a/spec/services/merge_requests/mergeability_check_service_spec.rb +++ b/spec/services/merge_requests/mergeability_check_service_spec.rb @@ -33,6 +33,24 @@ describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shared_sta expect(merge_request.merge_status).to eq('can_be_merged') end + it 'update diff discussion positions' do + expect_next_instance_of(Discussions::CaptureDiffNotePositionsService) do |service| + expect(service).to receive(:execute) + end + + subject + end + + context 'when merge_ref_head_comments is disabled' do + it 'does not update diff discussion positions' do + stub_feature_flags(merge_ref_head_comments: false) + + expect(Discussions::CaptureDiffNotePositionsService).not_to receive(:new) + + subject + end + end + it 'updates the merge ref' do expect { subject }.to change(merge_request, :merge_ref_head).from(nil) end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index a03b78a9a7a..c461dd700ec 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -143,10 +143,21 @@ describe Notes::CreateService do end it 'note is associated with a note diff file' do + MergeRequests::MergeToRefService.new(merge_request.project, merge_request.author).execute(merge_request) + note = described_class.new(project_with_repo, user, new_opts).execute expect(note).to be_persisted expect(note.note_diff_file).to be_present + expect(note.diff_note_positions).to be_present + end + + it 'does not create diff positions merge_ref_head_comments is disabled' do + stub_feature_flags(merge_ref_head_comments: false) + + expect(Discussions::CaptureDiffNotePositionService).not_to receive(:new) + + described_class.new(project_with_repo, user, new_opts).execute end end @@ -160,6 +171,8 @@ describe Notes::CreateService do end it 'note is not associated with a note diff file' do + expect(Discussions::CaptureDiffNotePositionService).not_to receive(:new) + note = described_class.new(project_with_repo, user, new_opts).execute expect(note).to be_persisted diff --git a/spec/support/helpers/test_env.rb b/spec/support/helpers/test_env.rb index fd41c5c8fe3..47d69ca1f6a 100644 --- a/spec/support/helpers/test_env.rb +++ b/spec/support/helpers/test_env.rb @@ -73,7 +73,9 @@ module TestEnv 'submodule_inside_folder' => 'b491b92', 'png-lfs' => 'fe42f41', 'sha-starting-with-large-number' => '8426165', - 'invalid-utf8-diff-paths' => '99e4853' + 'invalid-utf8-diff-paths' => '99e4853', + 'compare-with-merge-head-source' => 'b5f4399', + 'compare-with-merge-head-target' => '2f1e176' }.freeze # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily diff --git a/spec/support/import_export/common_util.rb b/spec/support/import_export/common_util.rb index 4e287f36648..1a5668946c6 100644 --- a/spec/support/import_export/common_util.rb +++ b/spec/support/import_export/common_util.rb @@ -38,15 +38,12 @@ module ImportExport end def setup_reader(reader) - case reader - when :legacy_reader - allow_any_instance_of(Gitlab::ImportExport::JSON::LegacyReader::File).to receive(:exist?).and_return(true) - allow_any_instance_of(Gitlab::ImportExport::JSON::NdjsonReader).to receive(:exist?).and_return(false) - when :ndjson_reader + if reader == :ndjson_reader && Feature.enabled?(:project_import_ndjson) allow_any_instance_of(Gitlab::ImportExport::JSON::LegacyReader::File).to receive(:exist?).and_return(false) allow_any_instance_of(Gitlab::ImportExport::JSON::NdjsonReader).to receive(:exist?).and_return(true) else - raise "invalid reader #{reader}. Supported readers: :legacy_reader, :ndjson_reader" + allow_any_instance_of(Gitlab::ImportExport::JSON::LegacyReader::File).to receive(:exist?).and_return(true) + allow_any_instance_of(Gitlab::ImportExport::JSON::NdjsonReader).to receive(:exist?).and_return(false) end end |