diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-20 15:26:25 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-20 15:26:25 +0300 |
commit | a09983ae35713f5a2bbb100981116d31ce99826e (patch) | |
tree | 2ee2af7bd104d57086db360a7e6d8c9d5d43667a /spec/features/merge_request | |
parent | 18c5ab32b738c0b6ecb4d0df3994000482f34bd8 (diff) |
Add latest changes from gitlab-org/gitlab@13-2-stable-ee
Diffstat (limited to 'spec/features/merge_request')
14 files changed, 315 insertions, 27 deletions
diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb index 7573553b3fb..be12f774c29 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 @@ RSpec.describe 'a maintainer edits files on a source-branch of an MR from a fork end before do - stub_feature_flags(web_ide_default: false, single_mr_diff_view: false) + stub_feature_flags(single_mr_diff_view: false) target_project.add_maintainer(user) sign_in(user) @@ -37,7 +37,7 @@ RSpec.describe 'a maintainer edits files on a source-branch of an MR from a fork end it 'allows committing to the source branch' do - find('.ace_text-input', visible: false).send_keys('Updated the readme') + execute_script("monaco.editor.getModels()[0].setValue('Updated the readme')") click_button 'Commit changes' wait_for_requests diff --git a/spec/features/merge_request/user_approves_spec.rb b/spec/features/merge_request/user_approves_spec.rb new file mode 100644 index 00000000000..d319fdcb87b --- /dev/null +++ b/spec/features/merge_request/user_approves_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Merge request > User approves', :js do + let(:user) { create(:user) } + let(:project) { create(:project, :public, :repository) } + let(:merge_request) { create(:merge_request, source_project: project) } + + before do + project.add_developer(user) + + sign_in(user) + + visit project_merge_request_path(project, merge_request) + end + + it 'approves merge request' do + click_approval_button('Approve') + expect(page).to have_content('Merge request approved') + + verify_approvals_count_on_index! + + click_approval_button('Revoke approval') + expect(page).to have_content('No approval required; you can still approve') + end + + def verify_approvals_count_on_index! + visit(project_merge_requests_path(project, state: :all)) + expect(page.all('li').any? { |item| item["title"] == "1 approver (you've approved)"}).to be true + visit project_merge_request_path(project, merge_request) + end + + def click_approval_button(action) + page.within('.mr-state-widget') do + click_button(action) + end + + wait_for_requests + end +end diff --git a/spec/features/merge_request/user_closes_merge_request_spec.rb b/spec/features/merge_request/user_closes_merge_request_spec.rb index 669bd989f4f..e6b6778c76e 100644 --- a/spec/features/merge_request/user_closes_merge_request_spec.rb +++ b/spec/features/merge_request/user_closes_merge_request_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'User closes a merge requests', :js do end it 'closes a merge request' do - click_link('Close merge request', match: :first) + click_button('Close merge request', match: :first) expect(page).to have_content(merge_request.title) expect(page).to have_content('Closed by') 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 9cd3c7eaf76..30bf82e3665 100644 --- a/spec/features/merge_request/user_comments_on_diff_spec.rb +++ b/spec/features/merge_request/user_comments_on_diff_spec.rb @@ -117,9 +117,58 @@ RSpec.describe 'User comments on a diff', :js do context 'when adding multiline comments' do it 'saves a multiline comment' do click_diff_line(find("[id='#{sample_commit.line_code}']")) + add_comment('-13', '+14') + end + + context 'when in side-by-side view' do + before do + visit(diffs_project_merge_request_path(project, merge_request, view: 'parallel')) + end + + # In `files/ruby/popen.rb` + it 'allows comments for changes involving both sides' do + # click +15, select -13 add and verify comment + click_diff_line(find('div[data-path="files/ruby/popen.rb"] .new_line a[data-linenumber="15"]').find(:xpath, '../..'), 'right') + add_comment('-13', '+15') + end + + it 'allows comments to start above hidden lines and end below' do + # click +28, select 21 add and verify comment + click_diff_line(find('div[data-path="files/ruby/popen.rb"] .new_line a[data-linenumber="28"]').find(:xpath, '../..'), 'right') + add_comment('21', '+28') + end + + it 'allows comments on previously hidden lines at the top of a file' do + # Click -9, expand up, select 1 add and verify comment + page.within('[data-path="files/ruby/popen.rb"]') do + all('.js-unfold-all')[0].click + end + click_diff_line(find('div[data-path="files/ruby/popen.rb"] .old_line a[data-linenumber="9"]').find(:xpath, '../..'), 'left') + add_comment('1', '-9') + end + + it 'allows comments on previously hidden lines the middle of a file' do + # Click 27, expand up, select 18, add and verify comment + page.within('[data-path="files/ruby/popen.rb"]') do + all('.js-unfold-all')[1].click + end + click_diff_line(find('div[data-path="files/ruby/popen.rb"] .old_line a[data-linenumber="21"]').find(:xpath, '../..'), 'left') + add_comment('18', '21') + end + + it 'allows comments on previously hidden lines at the bottom of a file' do + # Click +28, expand down, select 37 add and verify comment + page.within('[data-path="files/ruby/popen.rb"]') do + all('.js-unfold-down')[1].click + end + click_diff_line(find('div[data-path="files/ruby/popen.rb"] .old_line a[data-linenumber="30"]').find(:xpath, '../..'), 'left') + add_comment('+28', '37') + end + end + def add_comment(start_line, end_line) page.within('.discussion-form') do - find('#comment-line-start option', text: '-13').select_option + find('#comment-line-start option', exact_text: start_line).select_option end page.within('.js-discussion-note-form') do @@ -131,7 +180,7 @@ RSpec.describe 'User comments on a diff', :js do page.within('.notes_holder') do expect(page).to have_content('Line is wrong') - expect(page).to have_content('Comment on lines -13 to +14') + expect(page).to have_content("Comment on lines #{start_line} to #{end_line}") end visit(merge_request_path(merge_request)) 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 34eaca24a01..3cd23764382 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 @@ -88,7 +88,7 @@ RSpec.describe 'Merge request > User creates image diff notes', :js do create_image_diff_note end - it 'shows indicator and avatar badges, and allows collapsing/expanding the discussion notes', :quarantine do + it 'shows indicator and avatar badges, and allows collapsing/expanding the discussion notes', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/27950' do indicator = find('.js-image-badge', match: :first) badge = find('.user-avatar-link .badge', match: :first) diff --git a/spec/features/merge_request/user_edits_merge_request_spec.rb b/spec/features/merge_request/user_edits_merge_request_spec.rb index 84ecd6537dc..6c5f508c8c6 100644 --- a/spec/features/merge_request/user_edits_merge_request_spec.rb +++ b/spec/features/merge_request/user_edits_merge_request_spec.rb @@ -16,6 +16,75 @@ RSpec.describe 'User edits a merge request', :js do visit(edit_project_merge_request_path(project, merge_request)) end + describe 'Squash commits' do + it 'override MR setting if "Required" is saved' do + merge_request.update!(squash: false) + + project.project_setting.update!(squash_option: 'always') + visit(edit_project_merge_request_path(project, merge_request)) + click_button('Save changes') + + project.project_setting.update!(squash_option: 'default_off') + visit(edit_project_merge_request_path(project, merge_request)) + + expect(find("#merge_request_squash").selected?).to be(true) + end + + it 'recovers MR squash setting if "Required" is not saved' do + merge_request.update!(squash: false) + + project.project_setting.update!(squash_option: 'always') + visit(edit_project_merge_request_path(project, merge_request)) + + project.project_setting.update!(squash_option: 'default_on') + visit(edit_project_merge_request_path(project, merge_request)) + + expect(find("#merge_request_squash").selected?).to be(false) + end + + it 'does not override MR squash setting if "Do not allow" is saved' do + merge_request.update!(squash: true) + + project.project_setting.update!(squash_option: 'never') + visit(edit_project_merge_request_path(project, merge_request)) + click_button('Save changes') + + expect(merge_request.squash).to be true + end + + it 'displays "Required in this project" for "Required" project setting squash option' do + project.project_setting.update!(squash_option: 'always') + visit(edit_project_merge_request_path(project, merge_request)) + + expect(page).to have_content('Squash commits when merge request is accepted.') + expect(page).to have_content("Required in this project") + end + + it 'does not display message for "Allow" project setting squash option' do + project.project_setting.update!(squash_option: 'default_off') + visit(edit_project_merge_request_path(project, merge_request)) + + expect(page).to have_content('Squash commits when merge request is accepted.') + expect(page).not_to have_content("Required in this project") + end + + it 'does not display message for "Encourage" project setting squash option' do + project.project_setting.update!(squash_option: 'default_on') + visit(edit_project_merge_request_path(project, merge_request)) + + expect(page).to have_content('Squash commits when merge request is accepted.') + expect(page).not_to have_content("Required in this project") + end + + it 'is hidden for "Do not allow" project setting squash option' do + project.project_setting.update!(squash_option: 'never') + visit(edit_project_merge_request_path(project, merge_request)) + + expect(page).not_to have_content('Squash commits when merge request is accepted.') + expect(page).not_to have_css('#merge_request_squash') + end + end + it 'changes the target branch' do expect(page).to have_content('From master into feature') 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 dbad2f191a1..6ecffb05009 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -46,7 +46,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do end context 'with an old line on the left and a new line on the right' do - it 'allows commenting on the left side', quarantine: 'https://gitlab.com/gitlab-org/gitlab/issues/199050' do + it 'allows commenting on the left side', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/199050' do should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'left') end @@ -56,7 +56,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do end context 'with an unchanged line on the left and an unchanged line on the right' do - it 'allows commenting on the left side', quarantine: 'https://gitlab.com/gitlab-org/gitlab/issues/196826' do + it 'allows commenting on the left side', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/196826' do should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]', match: :first).find(:xpath, '..'), 'left') end diff --git a/spec/features/merge_request/user_reopens_merge_request_spec.rb b/spec/features/merge_request/user_reopens_merge_request_spec.rb index 020929dc416..7866ece84ac 100644 --- a/spec/features/merge_request/user_reopens_merge_request_spec.rb +++ b/spec/features/merge_request/user_reopens_merge_request_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'User reopens a merge requests', :js do end it 'reopens a merge request' do - click_link('Reopen merge request', match: :first) + click_button('Reopen merge request', match: :first) page.within('.status-box') do expect(page).to have_content('Open') diff --git a/spec/features/merge_request/user_sees_diff_spec.rb b/spec/features/merge_request/user_sees_diff_spec.rb index 2229b242d5b..d067fc0ada4 100644 --- a/spec/features/merge_request/user_sees_diff_spec.rb +++ b/spec/features/merge_request/user_sees_diff_spec.rb @@ -72,7 +72,7 @@ RSpec.describe 'Merge request > User sees diff', :js do end context 'as user who needs to fork' do - it 'shows fork/cancel confirmation', :sidekiq_might_not_need_inline, quarantine: 'https://gitlab.com/gitlab-org/gitlab/issues/196749' do + it 'shows fork/cancel confirmation', :sidekiq_might_not_need_inline, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/196749' do sign_in(user) visit diffs_project_merge_request_path(project, merge_request) diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index bd140a0643d..ce49e9f4141 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -268,7 +268,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do end end - context 'view merge request where project has CI set up but no CI status' do + context 'view merge request where there is no pipeline yet' do before do pipeline = create(:ci_pipeline, project: project, sha: merge_request.diff_head_sha, @@ -278,11 +278,11 @@ RSpec.describe 'Merge request > User sees merge widget', :js do visit project_merge_request_path(project, merge_request) end - it 'has pipeline error text' do + it 'has pipeline loading state' do # Wait for the `ci_status` and `merge_check` requests wait_for_requests - expect(page).to have_text("Could not retrieve the pipeline status. For troubleshooting steps, read the documentation.") + expect(page).to have_text("Checking pipeline status") end end @@ -889,9 +889,9 @@ RSpec.describe 'Merge request > User sees merge widget', :js do visit project_merge_request_path(project, merge_request) end - it 'renders a CI pipeline error' do + it 'renders a CI pipeline loading state' do within '.ci-widget' do - expect(page).to have_content('Could not retrieve the pipeline status.') + expect(page).to have_content('Checking pipeline status') end end end diff --git a/spec/features/merge_request/user_sees_pipelines_spec.rb b/spec/features/merge_request/user_sees_pipelines_spec.rb index 2d125087cb6..d693eec91c8 100644 --- a/spec/features/merge_request/user_sees_pipelines_spec.rb +++ b/spec/features/merge_request/user_sees_pipelines_spec.rb @@ -38,14 +38,6 @@ RSpec.describe 'Merge request > User sees pipelines', :js do expect(page).to have_selector('.stage-cell') end - it 'pipeline sha does not equal last commit sha' do - pipeline.update_attribute(:sha, '19e2e9b4ef76b422ce1154af39a91323ccc57434') - visit project_merge_request_path(project, merge_request) - wait_for_requests - - expect(page.find('.ci-widget')).to have_text("Could not retrieve the pipeline status. For troubleshooting steps, read the documentation.") - end - context 'with a detached merge request pipeline' do let(:merge_request) { create(:merge_request, :with_detached_merge_request_pipeline) } @@ -92,6 +84,111 @@ RSpec.describe 'Merge request > User sees pipelines', :js do end end + describe 'fork MRs in parent project', :sidekiq_inline do + include ProjectForksHelper + + let_it_be(:parent_project) { create(:project, :public, :repository) } + let_it_be(:forked_project) { fork_project(parent_project, developer_in_fork, repository: true, target_project: create(:project, :public, :repository)) } + let_it_be(:developer_in_parent) { create(:user) } + let_it_be(:developer_in_fork) { create(:user) } + let_it_be(:reporter_in_parent_and_developer_in_fork) { create(:user) } + + let(:merge_request) do + create(:merge_request, :with_detached_merge_request_pipeline, + source_project: forked_project, source_branch: 'feature', + target_project: parent_project, target_branch: 'master') + end + + let(:config) do + { test: { script: 'test', rules: [{ if: '$CI_MERGE_REQUEST_ID' }] } } + end + + before_all do + parent_project.add_developer(developer_in_parent) + parent_project.add_reporter(reporter_in_parent_and_developer_in_fork) + forked_project.add_developer(developer_in_fork) + forked_project.add_developer(reporter_in_parent_and_developer_in_fork) + end + + before do + stub_ci_pipeline_yaml_file(YAML.dump(config)) + sign_in(actor) + end + + after do + parent_project.all_pipelines.delete_all + forked_project.all_pipelines.delete_all + end + + context 'when actor is a developer in parent project' do + let(:actor) { developer_in_parent } + + it 'creates a pipeline in the parent project' do + visit project_merge_request_path(parent_project, merge_request) + + create_merge_request_pipeline + + check_pipeline(expected_project: parent_project) + check_head_pipeline(expected_project: parent_project) + end + end + + context 'when actor is a developer in fork project' do + let(:actor) { developer_in_fork } + + it 'creates a pipeline in the fork project' do + visit project_merge_request_path(parent_project, merge_request) + + create_merge_request_pipeline + + check_pipeline(expected_project: forked_project) + check_head_pipeline(expected_project: forked_project) + end + end + + context 'when actor is a reporter in parent project and a developer in fork project' do + let(:actor) { reporter_in_parent_and_developer_in_fork } + + it 'creates a pipeline in the fork project' do + visit project_merge_request_path(parent_project, merge_request) + + create_merge_request_pipeline + + check_pipeline(expected_project: forked_project) + check_head_pipeline(expected_project: forked_project) + end + end + + def create_merge_request_pipeline + page.within('.merge-request-tabs') { click_link('Pipelines') } + click_button('Run Pipeline') + end + + def check_pipeline(expected_project:) + page.within('.ci-table') do + expect(page).to have_selector('.commit', count: 2) + + page.within(first('.commit')) do + page.within('.pipeline-tags') do + expect(page.find('.js-pipeline-url-link')[:href]).to include(expected_project.full_path) + expect(page).to have_content('detached') + end + page.within('.pipeline-triggerer') do + expect(page).to have_link(href: user_path(actor)) + end + end + end + end + + def check_head_pipeline(expected_project:) + page.within('.merge-request-tabs') { click_link('Overview') } + + page.within('.ci-widget-content') do + expect(page.find('.pipeline-id')[:href]).to include(expected_project.full_path) + end + end + end + describe 'race condition' do let(:project) { create(:project, :repository) } let(:user) { create(:user) } 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 b81c0e49538..0506d190487 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 @@ -155,7 +155,7 @@ RSpec.describe 'User comments on a diff', :js do end end - it 'can apply multiple suggestions as a batch' do + it 'can apply multiple suggestions as a batch', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/224100' do files.each_with_index do |file, index| page.within("[id='#{file[:hash]}']") do find("button[title='Show full file']").click @@ -188,8 +188,7 @@ RSpec.describe 'User comments on a diff', :js do end context 'multiple suggestions in expanded lines' do - # https://gitlab.com/gitlab-org/gitlab/issues/38277 - it 'suggestions are appliable', :quarantine do + it 'suggestions are appliable', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/38277' do diff_file = merge_request.diffs(paths: ['files/ruby/popen.rb']).diff_files.first hash = Digest::SHA1.hexdigest(diff_file.file_path) 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 fab500f47bf..05f4c16ef60 100644 --- a/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb +++ b/spec/features/merge_request/user_toggles_whitespace_changes_spec.rb @@ -27,7 +27,7 @@ RSpec.describe 'Merge request > User toggles whitespace changes', :js do find('.js-show-diff-settings').click - expect(find('#show-whitespace')).to be_checked + expect(find('#show-whitespace')).not_to be_checked end end end diff --git a/spec/features/merge_request/user_views_diffs_file_by_file_spec.rb b/spec/features/merge_request/user_views_diffs_file_by_file_spec.rb new file mode 100644 index 00000000000..c254a142349 --- /dev/null +++ b/spec/features/merge_request/user_views_diffs_file_by_file_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'User views diffs file-by-file', :js do + let(:merge_request) do + create(:merge_request_with_diffs, source_project: project, target_project: project, source_branch: 'merge-test') + end + let(:project) { create(:project, :repository) } + let(:user) { create(:user, view_diffs_file_by_file: true) } + + before do + project.add_developer(user) + + sign_in(user) + + visit(diffs_project_merge_request_path(project, merge_request)) + + wait_for_requests + end + + it 'shows diffs file-by-file' do + page.within('#diffs') do + expect(page).to have_selector('.file-holder', count: 1) + expect(page).to have_selector('.diff-file .file-title', text: '.DS_Store') + + click_button 'Next' + + expect(page).to have_selector('.file-holder', count: 1) + expect(page).to have_selector('.diff-file .file-title', text: '.gitignore') + end + end +end |