From 991a295378736c60d419c794f0accbf0336987fc Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 11 Nov 2019 00:06:26 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../merge_requests/diffs_controller.rb | 5 ++- app/serializers/diff_file_entity.rb | 20 +++++++++-- .../error_tracking/list_issues_service.rb | 8 ++--- .../merge_requests/diffs_controller_spec.rb | 14 +++++++- .../maintainer_edits_fork_spec.rb | 4 ++- .../user_comments_on_diff_spec.rb | 3 ++ .../user_creates_image_diff_notes_spec.rb | 3 ++ .../merge_request/user_expands_diff_spec.rb | 4 +++ .../user_posts_diff_notes_spec.rb | 3 ++ .../user_resolves_conflicts_spec.rb | 5 ++- ...diff_notes_and_discussions_resolve_spec.rb | 6 ++++ .../user_sees_avatar_on_diff_notes_spec.rb | 7 ++-- .../merge_request/user_sees_diff_spec.rb | 6 ++++ ...sees_mr_with_deleted_source_branch_spec.rb | 3 ++ .../merge_request/user_sees_versions_spec.rb | 4 +++ .../user_suggests_changes_on_diff_spec.rb | 3 ++ .../user_toggles_whitespace_changes_spec.rb | 3 ++ .../merge_request/user_views_diffs_spec.rb | 3 ++ spec/features/projects/blobs/edit_spec.rb | 14 ++++---- spec/features/projects/view_on_env_spec.rb | 10 ++++-- spec/lib/gitlab/auth/ldap/auth_hash_spec.rb | 4 ++- spec/lib/gitlab/auth/ldap/person_spec.rb | 4 ++- spec/serializers/diff_file_entity_spec.rb | 7 ++-- .../list_issues_service_spec.rb | 8 +++++ spec/support/capybara.rb | 6 +++- ...equests_rendering_a_single_diff_version.rb | 17 ++++++++++ .../serializers/diff_file_entity_examples.rb | 33 +++++++++++++++++-- 27 files changed, 179 insertions(+), 28 deletions(-) create mode 100644 spec/support/shared_examples/merge_requests_rendering_a_single_diff_version.rb diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 4a37dfe5c19..42f9c0522a3 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -31,6 +31,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic options = { merge_request: @merge_request, + diff_view: diff_view, pagination_data: diffs.pagination_data } @@ -60,7 +61,9 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic render: ->(partial, locals) { view_to_html_string(partial, locals) } } - render json: DiffsSerializer.new(request).represent(@diffs, additional_attributes) + options = additional_attributes.merge(diff_view: diff_view) + + render json: DiffsSerializer.new(request).represent(@diffs, options) end def define_diff_vars diff --git a/app/serializers/diff_file_entity.rb b/app/serializers/diff_file_entity.rb index 2a5121a2266..af7d1172f17 100644 --- a/app/serializers/diff_file_entity.rb +++ b/app/serializers/diff_file_entity.rb @@ -53,7 +53,7 @@ class DiffFileEntity < DiffFileBaseEntity end # Used for inline diffs - expose :highlighted_diff_lines, using: DiffLineEntity, if: -> (diff_file, _) { diff_file.text? } do |diff_file| + expose :highlighted_diff_lines, using: DiffLineEntity, if: -> (diff_file, options) { inline_diff_view?(options) && diff_file.text? } do |diff_file| diff_file.diff_lines_for_serializer end @@ -62,5 +62,21 @@ class DiffFileEntity < DiffFileBaseEntity end # Used for parallel diffs - expose :parallel_diff_lines, using: DiffLineParallelEntity, if: -> (diff_file, _) { diff_file.text? } + expose :parallel_diff_lines, using: DiffLineParallelEntity, if: -> (diff_file, options) { parallel_diff_view?(options) && diff_file.text? } + + private + + def parallel_diff_view?(options) + return true unless Feature.enabled?(:single_mr_diff_view) + + # If we're not rendering inline, we must be rendering parallel + !inline_diff_view?(options) + end + + def inline_diff_view?(options) + return true unless Feature.enabled?(:single_mr_diff_view) + + # If nothing is present, inline will be the default. + options.fetch(:diff_view, :inline).to_sym == :inline + end end diff --git a/app/services/error_tracking/list_issues_service.rb b/app/services/error_tracking/list_issues_service.rb index e09f4345308..2e8c401b8ef 100644 --- a/app/services/error_tracking/list_issues_service.rb +++ b/app/services/error_tracking/list_issues_service.rb @@ -5,6 +5,10 @@ module ErrorTracking DEFAULT_ISSUE_STATUS = 'unresolved' DEFAULT_LIMIT = 20 + def external_url + project_error_tracking_setting&.sentry_external_url + end + private def fetch @@ -15,10 +19,6 @@ module ErrorTracking { issues: response[:issues] } end - def external_url - project_error_tracking_setting&.sentry_external_url - end - def issue_status params[:issue_status] || DEFAULT_ISSUE_STATUS end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 5c02e8d6461..4f6a7de5ae7 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -34,6 +34,16 @@ describe Projects::MergeRequests::DiffsController do it 'saves the preferred diff view in a cookie' do expect(response.cookies['diff_view']).to eq('parallel') end + + it 'only renders the required view', :aggregate_failures do + diff_files_without_deletions = json_response['diff_files'].reject { |f| f['deleted_file'] } + have_no_inline_diff_lines = satisfy('have no inline diff lines') do |diff_file| + !diff_file.has_key?('highlighted_diff_lines') + end + + expect(diff_files_without_deletions).to all(have_key('parallel_diff_lines')) + expect(diff_files_without_deletions).to all(have_no_inline_diff_lines) + end end context 'when the user cannot view the merge request' do @@ -259,7 +269,7 @@ describe Projects::MergeRequests::DiffsController do it 'only renders the diffs for the path given' do diff_for_path(old_path: existing_path, new_path: existing_path) - paths = json_response["diff_files"].map { |file| file['new_path'] } + paths = json_response['diff_files'].map { |file| file['new_path'] } expect(paths).to include(existing_path) end @@ -344,6 +354,7 @@ describe Projects::MergeRequests::DiffsController do let(:expected_options) do { merge_request: merge_request, + diff_view: :inline, pagination_data: { current_page: 1, next_page: nil, @@ -367,6 +378,7 @@ describe Projects::MergeRequests::DiffsController do let(:expected_options) do { merge_request: merge_request, + diff_view: :inline, pagination_data: { current_page: 2, next_page: 3, diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index fa53682b024..4e161d530d3 100644 --- a/spec/features/merge_request/maintainer_edits_fork_spec.rb +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -20,7 +20,7 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js end before do - stub_feature_flags(web_ide_default: false) + stub_feature_flags(web_ide_default: false, single_mr_diff_view: false) target_project.add_maintainer(user) sign_in(user) @@ -32,6 +32,8 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js wait_for_requests end + it_behaves_like 'rendering a single diff version' + it 'mentions commits will go to the source branch' do expect(page).to have_content('Your changes can be committed to fix because a merge request is open.') end diff --git a/spec/features/merge_request/user_comments_on_diff_spec.rb b/spec/features/merge_request/user_comments_on_diff_spec.rb index 19b8a7f74b7..6a23b6cdf60 100644 --- a/spec/features/merge_request/user_comments_on_diff_spec.rb +++ b/spec/features/merge_request/user_comments_on_diff_spec.rb @@ -13,12 +13,15 @@ describe 'User comments on a diff', :js do let(:user) { create(:user) } before do + stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in(user) visit(diffs_project_merge_request_path(project, merge_request)) end + it_behaves_like 'rendering a single diff version' + context 'when viewing comments' do context 'when toggling inline comments' do context 'in a single file' do diff --git a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb index e0724a04ea3..e6634a8ff39 100644 --- a/spec/features/merge_request/user_creates_image_diff_notes_spec.rb +++ b/spec/features/merge_request/user_creates_image_diff_notes_spec.rb @@ -9,6 +9,7 @@ describe 'Merge request > User creates image diff notes', :js do let(:user) { project.creator } before do + stub_feature_flags(single_mr_diff_view: false) sign_in(user) # Stub helper to return any blob file as image from public app folder. @@ -17,6 +18,8 @@ describe 'Merge request > User creates image diff notes', :js do allow_any_instance_of(DiffHelper).to receive(:diff_file_old_blob_raw_url).and_return('/favicon.png') end + it_behaves_like 'rendering a single diff version' + context 'create commit diff notes' do commit_id = '2f63565e7aac07bcdadb654e253078b727143ec4' diff --git a/spec/features/merge_request/user_expands_diff_spec.rb b/spec/features/merge_request/user_expands_diff_spec.rb index f7317ec5ca7..ba7abd3af2c 100644 --- a/spec/features/merge_request/user_expands_diff_spec.rb +++ b/spec/features/merge_request/user_expands_diff_spec.rb @@ -7,6 +7,8 @@ describe 'User expands diff', :js do let(:merge_request) { create(:merge_request, source_branch: 'expand-collapse-files', source_project: project, target_project: project) } before do + stub_feature_flags(single_mr_diff_view: false) + allow(Gitlab::Git::Diff).to receive(:size_limit).and_return(100.kilobytes) allow(Gitlab::Git::Diff).to receive(:collapse_limit).and_return(10.kilobytes) @@ -15,6 +17,8 @@ describe 'User expands diff', :js do wait_for_requests end + it_behaves_like 'rendering a single diff version' + it 'allows user to expand diff' do page.within find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd"]') do click_link 'Click to expand it.' diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index 8b16760606c..6328c0a5133 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -14,12 +14,15 @@ describe 'Merge request > User posts diff notes', :js do let(:test_note_comment) { 'this is a test note!' } before do + stub_feature_flags(single_mr_diff_view: false) set_cookie('sidebar_collapsed', 'true') project.add_developer(user) sign_in(user) end + it_behaves_like 'rendering a single diff version' + context 'when hovering over a parallel view diff file' do before do visit diffs_project_merge_request_path(project, merge_request, view: 'parallel') diff --git a/spec/features/merge_request/user_resolves_conflicts_spec.rb b/spec/features/merge_request/user_resolves_conflicts_spec.rb index e3ee80a47d7..f0949fefa3b 100644 --- a/spec/features/merge_request/user_resolves_conflicts_spec.rb +++ b/spec/features/merge_request/user_resolves_conflicts_spec.rb @@ -9,6 +9,7 @@ describe 'Merge request > User resolves conflicts', :js do before do # In order to have the diffs collapsed, we need to disable the increase feature stub_feature_flags(gitlab_git_diff_size_limit_increase: false) + stub_feature_flags(single_mr_diff_view: false) end def create_merge_request(source_branch) @@ -17,7 +18,9 @@ describe 'Merge request > User resolves conflicts', :js do end end - shared_examples "conflicts are resolved in Interactive mode" do + it_behaves_like 'rendering a single diff version' + + shared_examples 'conflicts are resolved in Interactive mode' do it 'conflicts are resolved in Interactive mode' do within find('.files-wrapper .diff-file', text: 'files/ruby/popen.rb') do click_button 'Use ours' diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index 8b41ef86791..7cb46d90092 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -19,6 +19,12 @@ describe 'Merge request > User resolves diff notes and threads', :js do ) end + before do + stub_feature_flags(single_mr_diff_view: false) + end + + it_behaves_like 'rendering a single diff version' + context 'no threads' do before do project.add_maintainer(user) diff --git a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb index baef831c40e..e882b401122 100644 --- a/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb +++ b/spec/features/merge_request/user_sees_avatar_on_diff_notes_spec.rb @@ -7,8 +7,8 @@ describe 'Merge request > User sees avatars on diff notes', :js do let(:project) { create(:project, :public, :repository) } let(:user) { project.creator } - let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user, title: "Bug NS-04") } - let(:path) { "files/ruby/popen.rb" } + let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user, title: 'Bug NS-04') } + let(:path) { 'files/ruby/popen.rb' } let(:position) do Gitlab::Diff::Position.new( old_path: path, @@ -21,12 +21,15 @@ describe 'Merge request > User sees avatars on diff notes', :js do let!(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) } before do + stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in user set_cookie('sidebar_collapsed', 'true') end + it_behaves_like 'rendering a single diff version' + context 'discussion tab' do before do visit project_merge_request_path(project, merge_request) diff --git a/spec/features/merge_request/user_sees_diff_spec.rb b/spec/features/merge_request/user_sees_diff_spec.rb index d9783f3388c..82dd779577c 100644 --- a/spec/features/merge_request/user_sees_diff_spec.rb +++ b/spec/features/merge_request/user_sees_diff_spec.rb @@ -9,6 +9,12 @@ describe 'Merge request > User sees diff', :js do let(:project) { create(:project, :public, :repository) } let(:merge_request) { create(:merge_request, source_project: project) } + before do + stub_feature_flags(single_mr_diff_view: false) + end + + it_behaves_like 'rendering a single diff version' + context 'when linking to note' do describe 'with unresolved note' do let(:note) { create :diff_note_on_merge_request, project: project, noteable: merge_request } diff --git a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb index db0d632cdf2..3d25611e1ea 100644 --- a/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb +++ b/spec/features/merge_request/user_sees_mr_with_deleted_source_branch_spec.rb @@ -11,11 +11,14 @@ describe 'Merge request > User sees MR with deleted source branch', :js do let(:user) { project.creator } before do + stub_feature_flags(single_mr_diff_view: false) merge_request.update!(source_branch: 'this-branch-does-not-exist') sign_in(user) visit project_merge_request_path(project, merge_request) end + it_behaves_like 'rendering a single diff version' + it 'shows a message about missing source branch' do expect(page).to have_content('Source branch does not exist.') end diff --git a/spec/features/merge_request/user_sees_versions_spec.rb b/spec/features/merge_request/user_sees_versions_spec.rb index 62abcff7bda..c3fce9761df 100644 --- a/spec/features/merge_request/user_sees_versions_spec.rb +++ b/spec/features/merge_request/user_sees_versions_spec.rb @@ -16,11 +16,15 @@ describe 'Merge request > User sees versions', :js do let!(:params) { {} } before do + stub_feature_flags(single_mr_diff_view: false) + project.add_maintainer(user) sign_in(user) visit diffs_project_merge_request_path(project, merge_request, params) end + it_behaves_like 'rendering a single diff version' + shared_examples 'allows commenting' do |file_id:, line_code:, comment:| it do diff_file_selector = ".diff-file[id='#{file_id}']" diff --git a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb index 3d26ff3ed94..e2bcdfd1e2b 100644 --- a/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb +++ b/spec/features/merge_request/user_suggests_changes_on_diff_spec.rb @@ -25,12 +25,15 @@ describe 'User comments on a diff', :js do let(:user) { create(:user) } before do + stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in(user) visit(diffs_project_merge_request_path(project, merge_request)) end + it_behaves_like 'rendering a single diff version' + context 'single suggestion note' do it 'hides suggestion popover' do click_diff_line(find("[id='#{sample_compare.changes[1][:line_code]}']")) diff --git a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb index 4db067a4e41..5e59bc87e68 100644 --- a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb +++ b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb @@ -8,6 +8,7 @@ describe 'Merge request > User toggles whitespace changes', :js do let(:user) { project.creator } before do + stub_feature_flags(single_mr_diff_view: false) project.add_maintainer(user) sign_in(user) visit diffs_project_merge_request_path(project, merge_request) @@ -15,6 +16,8 @@ describe 'Merge request > User toggles whitespace changes', :js do find('.js-show-diff-settings').click end + it_behaves_like 'rendering a single diff version' + it 'has a button to toggle whitespace changes' do expect(page).to have_content 'Show whitespace changes' end diff --git a/spec/features/merge_request/user_views_diffs_spec.rb b/spec/features/merge_request/user_views_diffs_spec.rb index 2d1eb260236..5a29477e597 100644 --- a/spec/features/merge_request/user_views_diffs_spec.rb +++ b/spec/features/merge_request/user_views_diffs_spec.rb @@ -9,6 +9,7 @@ describe 'User views diffs', :js do let(:project) { create(:project, :public, :repository) } before do + stub_feature_flags(single_mr_diff_view: false) visit(diffs_project_merge_request_path(project, merge_request)) wait_for_requests @@ -16,6 +17,8 @@ describe 'User views diffs', :js do find('.js-toggle-tree-list').click end + it_behaves_like 'rendering a single diff version' + shared_examples 'unfold diffs' do it 'unfolds diffs upwards' do first('.js-unfold').click diff --git a/spec/features/projects/blobs/edit_spec.rb b/spec/features/projects/blobs/edit_spec.rb index e02a4579095..b81f52a541b 100644 --- a/spec/features/projects/blobs/edit_spec.rb +++ b/spec/features/projects/blobs/edit_spec.rb @@ -12,9 +12,11 @@ describe 'Editing file blob', :js do let(:readme_file_path) { 'README.md' } before do - stub_feature_flags(web_ide_default: false) + stub_feature_flags(web_ide_default: false, single_mr_diff_view: false) end + it_behaves_like 'rendering a single diff version' + context 'as a developer' do let(:user) { create(:user) } let(:role) { :developer } @@ -27,14 +29,14 @@ describe 'Editing file blob', :js do def edit_and_commit(commit_changes: true) wait_for_requests find('.js-edit-blob').click - fill_editor(content: "class NextFeature\\nend\\n") + fill_editor(content: 'class NextFeature\\nend\\n') if commit_changes click_button 'Commit changes' end end - def fill_editor(content: "class NextFeature\\nend\\n") + def fill_editor(content: 'class NextFeature\\nend\\n') wait_for_requests find('#editor') execute_script("ace.edit('editor').setValue('#{content}')") @@ -95,13 +97,13 @@ describe 'Editing file blob', :js do context 'when rendering the preview' do it 'renders content with CommonMark' do visit project_edit_blob_path(project, tree_join(branch, readme_file_path)) - fill_editor(content: "1. one\\n - sublist\\n") + fill_editor(content: '1. one\\n - sublist\\n') click_link 'Preview' wait_for_requests # the above generates two separate lists (not embedded) in CommonMark - expect(page).to have_content("sublist") - expect(page).not_to have_xpath("//ol//li//ul") + expect(page).to have_content('sublist') + expect(page).not_to have_xpath('//ol//li//ul') end end end diff --git a/spec/features/projects/view_on_env_spec.rb b/spec/features/projects/view_on_env_spec.rb index beb32104809..832985f1a30 100644 --- a/spec/features/projects/view_on_env_spec.rb +++ b/spec/features/projects/view_on_env_spec.rb @@ -9,9 +9,13 @@ describe 'View on environment', :js do let(:user) { project.creator } before do + stub_feature_flags(single_mr_diff_view: false) + project.add_maintainer(user) end + it_behaves_like 'rendering a single diff version' + context 'when the branch has a route map' do let(:route_map) do <<-MAP.strip_heredoc @@ -26,7 +30,7 @@ describe 'View on environment', :js do user, start_branch: branch_name, branch_name: branch_name, - commit_message: "Add .gitlab/route-map.yml", + commit_message: 'Add .gitlab/route-map.yml', file_path: '.gitlab/route-map.yml', file_content: route_map ).execute @@ -37,9 +41,9 @@ describe 'View on environment', :js do user, start_branch: branch_name, branch_name: branch_name, - commit_message: "Update feature", + commit_message: 'Update feature', file_path: file_path, - file_content: "# Noop" + file_content: '# Noop' ).execute end diff --git a/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb b/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb index 05541972f87..adb8e138ca7 100644 --- a/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb +++ b/spec/lib/gitlab/auth/ldap/auth_hash_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::Auth::LDAP::AuthHash do @@ -91,7 +93,7 @@ describe Gitlab::Auth::LDAP::AuthHash do let(:given_uid) { 'uid=John Smith,ou=People,dc=example,dc=com' } before do - raw_info[:uid] = ['JOHN'] + raw_info[:uid] = [+'JOHN'] end it 'enabled the username attribute is lower cased' do diff --git a/spec/lib/gitlab/auth/ldap/person_spec.rb b/spec/lib/gitlab/auth/ldap/person_spec.rb index 1527fe60fb9..985732e69f9 100644 --- a/spec/lib/gitlab/auth/ldap/person_spec.rb +++ b/spec/lib/gitlab/auth/ldap/person_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'spec_helper' describe Gitlab::Auth::LDAP::Person do @@ -135,7 +137,7 @@ describe Gitlab::Auth::LDAP::Person do let(:username_attribute) { 'uid' } before do - entry[username_attribute] = 'JOHN' + entry[username_attribute] = +'JOHN' @person = described_class.new(entry, 'ldapmain') end diff --git a/spec/serializers/diff_file_entity_spec.rb b/spec/serializers/diff_file_entity_spec.rb index 0c2e7c1e3eb..65b62f8aa16 100644 --- a/spec/serializers/diff_file_entity_spec.rb +++ b/spec/serializers/diff_file_entity_spec.rb @@ -11,7 +11,8 @@ describe DiffFileEntity do let(:diff_refs) { commit.diff_refs } let(:diff) { commit.raw_diffs.first } let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) } - let(:entity) { described_class.new(diff_file, request: {}) } + let(:options) { {} } + let(:entity) { described_class.new(diff_file, options.reverse_merge(request: {})) } subject { entity.as_json } @@ -23,7 +24,7 @@ describe DiffFileEntity do let(:user) { create(:user) } let(:request) { EntityRequest.new(project: project, current_user: user) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - let(:entity) { described_class.new(diff_file, request: request, merge_request: merge_request) } + let(:entity) { described_class.new(diff_file, options.merge(request: request, merge_request: merge_request)) } let(:exposed_urls) { %i(edit_path view_path context_lines_path) } it_behaves_like 'diff file entity' @@ -49,6 +50,8 @@ describe DiffFileEntity do end context '#parallel_diff_lines' do + let(:options) { { diff_view: :parallel } } + it 'exposes parallel diff lines correctly' do response = subject diff --git a/spec/services/error_tracking/list_issues_service_spec.rb b/spec/services/error_tracking/list_issues_service_spec.rb index fce790f708d..5b73bc91478 100644 --- a/spec/services/error_tracking/list_issues_service_spec.rb +++ b/spec/services/error_tracking/list_issues_service_spec.rb @@ -45,4 +45,12 @@ describe ErrorTracking::ListIssuesService do include_examples 'error tracking service unauthorized user' include_examples 'error tracking service disabled' end + + describe '#external_url' do + it 'calls the project setting sentry_external_url' do + expect(error_tracking_setting).to receive(:sentry_external_url).and_return(sentry_url) + + expect(subject.external_url).to eql sentry_url + end + end end diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 7b3b966bd50..2bd4750dffa 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -101,8 +101,12 @@ RSpec.configure do |config| config.after(:example, :js) do |example| # when a test fails, display any messages in the browser's console - if example.exception + # but fail don't add the message if the failure is a pending test that got + # fixed. If we raised the `JSException` the fixed test would be marked as + # failed again. + if example.exception && !example.exception.is_a?(RSpec::Core::Pending::PendingExampleFixedError) console = page.driver.browser.manage.logs.get(:browser)&.reject { |log| log.message =~ JS_CONSOLE_FILTER } + if console.present? message = "Unexpected browser console output:\n" + console.map(&:message).join("\n") raise JSConsoleError, message diff --git a/spec/support/shared_examples/merge_requests_rendering_a_single_diff_version.rb b/spec/support/shared_examples/merge_requests_rendering_a_single_diff_version.rb new file mode 100644 index 00000000000..80120629a32 --- /dev/null +++ b/spec/support/shared_examples/merge_requests_rendering_a_single_diff_version.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# This pending test can be removed when `single_mr_diff_view` is enabled by default +# disabling the feature flag above is then not needed anymore. +RSpec.shared_examples 'rendering a single diff version' do |attribute| + pending 'allows editing diff settings single_mr_diff_view is enabled' do + project = create(:project, :repository) + user = project.creator + merge_request = create(:merge_request, source_project: project) + stub_feature_flags(single_mr_diff_view: true) + sign_in(user) + + visit(diffs_project_merge_request_path(project, merge_request)) + + expect(page).to have_selector('.js-show-diff-settings') + end +end diff --git a/spec/support/shared_examples/serializers/diff_file_entity_examples.rb b/spec/support/shared_examples/serializers/diff_file_entity_examples.rb index 96cb71be737..d2c269c597c 100644 --- a/spec/support/shared_examples/serializers/diff_file_entity_examples.rb +++ b/spec/support/shared_examples/serializers/diff_file_entity_examples.rb @@ -31,14 +31,43 @@ shared_examples 'diff file entity' do it 'exposes correct attributes' do expect(subject).to include(:added_lines, :removed_lines, - :context_lines_path, :highlighted_diff_lines, - :parallel_diff_lines) + :context_lines_path) end it 'includes viewer' do expect(subject[:viewer].with_indifferent_access) .to match_schema('entities/diff_viewer') end + + context 'diff files' do + context 'when diff_view is parallel' do + let(:options) { { diff_view: :parallel } } + + it 'contains only the parallel diff lines', :aggregate_failures do + expect(subject).to include(:parallel_diff_lines) + expect(subject).not_to include(:highlighted_diff_lines) + end + end + + context 'when diff_view is parallel' do + let(:options) { { diff_view: :inline } } + + it 'contains only the inline diff lines', :aggregate_failures do + expect(subject).not_to include(:parallel_diff_lines) + expect(subject).to include(:highlighted_diff_lines) + end + end + + context 'when the `single_mr_diff_view` feature is disabled' do + before do + stub_feature_flags(single_mr_diff_view: false) + end + + it 'contains both kinds of diffs' do + expect(subject).to include(:highlighted_diff_lines, :parallel_diff_lines) + end + end + end end shared_examples 'diff file discussion entity' do -- GitLab