diff options
Diffstat (limited to 'spec/features/merge_request')
11 files changed, 156 insertions, 90 deletions
diff --git a/spec/features/merge_request/batch_comments_spec.rb b/spec/features/merge_request/batch_comments_spec.rb index fafaea8ac68..bccdc3c4c62 100644 --- a/spec/features/merge_request/batch_comments_spec.rb +++ b/spec/features/merge_request/batch_comments_spec.rb @@ -13,6 +13,8 @@ RSpec.describe 'Merge request > Batch comments', :js do end before do + stub_feature_flags(moved_mr_sidebar: false) + project.add_maintainer(user) sign_in(user) 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 f21929e5275..fd33731cb7b 100644 --- a/spec/features/merge_request/user_comments_on_diff_spec.rb +++ b/spec/features/merge_request/user_comments_on_diff_spec.rb @@ -14,6 +14,7 @@ RSpec.describe 'User comments on a diff', :js do let(:user) { create(:user) } before do + stub_feature_flags(remove_user_attributes_projects: false) project.add_maintainer(user) sign_in(user) @@ -256,9 +257,7 @@ RSpec.describe 'User comments on a diff', :js do click_button('Delete comment', match: :first) end - page.within('.merge-request-tabs') do - find('.notes-tab').click - end + find('.notes-tab', visible: true).click wait_for_requests diff --git a/spec/features/merge_request/user_comments_on_merge_request_spec.rb b/spec/features/merge_request/user_comments_on_merge_request_spec.rb index dbcfc2b968f..ec1e2fea851 100644 --- a/spec/features/merge_request/user_comments_on_merge_request_spec.rb +++ b/spec/features/merge_request/user_comments_on_merge_request_spec.rb @@ -10,6 +10,7 @@ RSpec.describe 'User comments on a merge request', :js do let(:user) { create(:user) } before do + stub_feature_flags(remove_user_attributes_projects: false) project.add_maintainer(user) sign_in(user) diff --git a/spec/features/merge_request/user_creates_merge_request_spec.rb b/spec/features/merge_request/user_creates_merge_request_spec.rb index c8b22bb3125..0ae4ef18649 100644 --- a/spec/features/merge_request/user_creates_merge_request_spec.rb +++ b/spec/features/merge_request/user_creates_merge_request_spec.rb @@ -1,111 +1,164 @@ # frozen_string_literal: true -require "spec_helper" +require 'spec_helper' -RSpec.describe "User creates a merge request", :js do +RSpec.describe 'User creates a merge request', :js do include ProjectForksHelper - let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { create(:user) } - - let(:title) { "Some feature" } - - before do - project.add_maintainer(user) - sign_in(user) - end - - it "creates a merge request" do - visit(project_new_merge_request_path(project)) + shared_examples 'creates a merge request' do + specify do + visit(project_new_merge_request_path(project)) - find(".js-source-branch").click - click_link("fix") + compare_source_and_target('fix', 'feature') - find(".js-target-branch").click - click_link("feature") + page.within('.merge-request-form') do + expect(page.find('#merge_request_description')['placeholder']).to eq 'Describe the goal of the changes and what reviewers should be aware of.' + end - click_button("Compare branches") + fill_in('Title', with: title) + click_button('Create merge request') - page.within('.merge-request-form') do - expect(page.find('#merge_request_description')['placeholder']).to eq 'Describe the goal of the changes and what reviewers should be aware of.' + page.within('.merge-request') do + expect(page).to have_content(title) + end end + end - fill_in("Title", with: title) - click_button("Create merge request") + shared_examples 'renders not found' do + specify do + visit project_new_merge_request_path(project) - page.within(".merge-request") do - expect(page).to have_content(title) + expect(page).to have_title('Not Found') + expect(page).to have_content('Page Not Found') end end - context "XSS branch name exists" do + context 'when user is a direct project member' do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + + let(:title) { 'Some feature' } + before do - project.repository.create_branch("<img/src='x'/onerror=alert('oops')>", "master") + project.add_maintainer(user) + sign_in(user) end - it "doesn't execute the dodgy branch name" do - visit(project_new_merge_request_path(project)) + it_behaves_like 'creates a merge request' - find(".js-source-branch").click - click_link("<img/src='x'/onerror=alert('oops')>") + context 'with XSS branch name' do + before do + project.repository.create_branch("<img/src='x'/onerror=alert('oops')>", 'master') + end - find(".js-target-branch").click - click_link("feature") + it 'does not execute the suspicious branch name' do + visit(project_new_merge_request_path(project)) - click_button("Compare branches") + compare_source_and_target("<img/src='x'/onerror=alert('oops')>", 'feature') - expect { page.driver.browser.switch_to.alert }.to raise_error(Selenium::WebDriver::Error::NoSuchAlertError) + expect { page.driver.browser.switch_to.alert }.to raise_error(Selenium::WebDriver::Error::NoSuchAlertError) + end end - end - context "to a forked project" do - let(:forked_project) { fork_project(project, user, namespace: user.namespace, repository: true) } + context 'to a forked project' do + let(:forked_project) { fork_project(project, user, namespace: user.namespace, repository: true) } + + it 'creates a merge request', :sidekiq_might_not_need_inline do + visit(project_new_merge_request_path(forked_project)) + + expect(page).to have_content('Source branch').and have_content('Target branch') + expect(find('#merge_request_target_project_id', visible: false).value).to eq(project.id.to_s) - it "creates a merge request", :sidekiq_might_not_need_inline do - visit(project_new_merge_request_path(forked_project)) + click_button('Compare branches and continue') - expect(page).to have_content("Source branch").and have_content("Target branch") - expect(find("#merge_request_target_project_id", visible: false).value).to eq(project.id.to_s) + expect(page).to have_content('You must select source and target branch') - click_button("Compare branches and continue") + first('.js-source-project').click + first('.dropdown-source-project a', text: forked_project.full_path) - expect(page).to have_content("You must select source and target branch") + first('.js-target-project').click + first('.dropdown-target-project a', text: project.full_path) - first(".js-source-project").click - first(".dropdown-source-project a", text: forked_project.full_path) + first('.js-source-branch').click - first(".js-target-project").click - first(".dropdown-target-project a", text: project.full_path) + wait_for_requests - first(".js-source-branch").click + source_branch = 'fix' - wait_for_requests + first('.js-source-branch-dropdown .dropdown-content a', text: source_branch).click - source_branch = "fix" + click_button('Compare branches and continue') - first(".js-source-branch-dropdown .dropdown-content a", text: source_branch).click + expect(page).to have_text _('New merge request') - click_button("Compare branches and continue") + page.within('form#new_merge_request') do + fill_in('Title', with: title) + end - expect(page).to have_text _('New merge request') + expect(find('.js-assignee-search')['data-project-id']).to eq(project.id.to_s) + find('.js-assignee-search').click - page.within("form#new_merge_request") do - fill_in("Title", with: title) + page.within('.dropdown-menu-user') do + expect(page).to have_content('Unassigned') + .and have_content(user.name) + .and have_content(project.users.first.name) + end + find('.js-assignee-search').click + + click_button('Create merge request') + + expect(page).to have_content(title).and have_content("requested to merge #{forked_project.full_path}:#{source_branch} into master") + end + end + end + + context 'when user is an inherited member from the group' do + let_it_be(:group) { create(:group, :public) } + + let(:user) { create(:user) } + + context 'when project is public and merge requests are private' do + let_it_be(:project) do + create(:project, + :public, + :repository, + group: group, + merge_requests_access_level: ProjectFeature::DISABLED) end - expect(find(".js-assignee-search")["data-project-id"]).to eq(project.id.to_s) - find('.js-assignee-search').click + context 'and user is a guest' do + before do + group.add_guest(user) + sign_in(user) + end - page.within(".dropdown-menu-user") do - expect(page).to have_content("Unassigned") - .and have_content(user.name) - .and have_content(project.users.first.name) + it_behaves_like 'renders not found' end - find('.js-assignee-search').click + end + + context 'when project is private' do + let_it_be(:project) { create(:project, :private, :repository, group: group) } - click_button("Create merge request") + context 'and user is a guest' do + before do + group.add_guest(user) + sign_in(user) + end - expect(page).to have_content(title).and have_content("requested to merge #{forked_project.full_path}:#{source_branch} into master") + it_behaves_like 'renders not found' + end end end + + private + + def compare_source_and_target(source_branch, target_branch) + find('.js-source-branch').click + click_link(source_branch) + + find('.js-target-branch').click + click_link(target_branch) + + click_button('Compare branches') + end end 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 0b4b9d7452a..4ac25ea7ae0 100644 --- a/spec/features/merge_request/user_edits_merge_request_spec.rb +++ b/spec/features/merge_request/user_edits_merge_request_spec.rb @@ -3,8 +3,6 @@ require 'spec_helper' RSpec.describe 'User edits a merge request', :js do - include Select2Helper - let(:project) { create(:project, :repository) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } let(:user) { create(:user) } @@ -89,7 +87,12 @@ RSpec.describe 'User edits a merge request', :js do it 'allows user to change target branch' do expect(page).to have_content('From master into feature') - select2('merge-test', from: '#merge_request_target_branch') + first('.js-target-branch').click + + wait_for_requests + + first('.js-target-branch-dropdown a', text: 'merge-test').click + click_button('Save changes') expect(page).to have_content("requested to merge #{merge_request.source_branch} into merge-test") @@ -101,7 +104,7 @@ RSpec.describe 'User edits a merge request', :js do it 'does not allow user to change target branch' do expect(page).to have_content('From master into feature') - expect(page).not_to have_selector('.select2-container') + expect(page).not_to have_selector('.js-target-branch.js-compare-dropdown') end end end diff --git a/spec/features/merge_request/user_manages_subscription_spec.rb b/spec/features/merge_request/user_manages_subscription_spec.rb index c64c761b8d1..9fb85957979 100644 --- a/spec/features/merge_request/user_manages_subscription_spec.rb +++ b/spec/features/merge_request/user_manages_subscription_spec.rb @@ -45,21 +45,21 @@ RSpec.describe 'User manages subscription', :js do click_button 'Toggle dropdown' - expect(page).to have_content('Turn on notifications') - click_button 'Turn on notifications' + expect(page).to have_selector('.gl-toggle:not(.is-checked)') + find('[data-testid="notifications-toggle"] .gl-toggle').click wait_for_requests click_button 'Toggle dropdown' - expect(page).to have_content('Turn off notifications') - click_button 'Turn off notifications' + expect(page).to have_selector('.gl-toggle.is-checked') + find('[data-testid="notifications-toggle"] .gl-toggle').click wait_for_requests click_button 'Toggle dropdown' - expect(page).to have_content('Turn on notifications') + expect(page).to have_selector('.gl-toggle:not(.is-checked)') end end end 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 e09ec11f095..332426de07e 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 @@ -15,6 +15,10 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do diff_refs: merge_request.diff_refs) end + before do + stub_feature_flags(moved_mr_sidebar: false) + end + context 'no threads' do before do project.add_maintainer(user) @@ -62,7 +66,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to mark thread as resolved' do page.within '.diff-content' do - find('button[data-qa-selector="resolve_discussion_button"]').click # rubocop:disable QA/SelectorUsage + find('button[data-testid="resolve-discussion-button"]').click end expect(page).to have_selector('.discussion-body', visible: false) @@ -78,7 +82,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to unresolve thread' do page.within '.diff-content' do - find('button[data-qa-selector="resolve_discussion_button"]').click # rubocop:disable QA/SelectorUsage + find('button[data-testid="resolve-discussion-button"]').click click_button 'Unresolve thread' end @@ -90,7 +94,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do describe 'resolved thread' do before do page.within '.diff-content' do - find('button[data-qa-selector="resolve_discussion_button"]').click # rubocop:disable QA/SelectorUsage + find('button[data-testid="resolve-discussion-button"]').click end visit_merge_request @@ -190,7 +194,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to resolve from reply form without a comment' do page.within '.diff-content' do - find('button[data-qa-selector="resolve_discussion_button"]').click # rubocop:disable QA/SelectorUsage + find('button[data-testid="resolve-discussion-button"]').click end page.within '.discussions-counter' do @@ -225,7 +229,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'hides jump to next button when all resolved' do page.within '.diff-content' do - find('button[data-qa-selector="resolve_discussion_button"]').click # rubocop:disable QA/SelectorUsage + find('button[data-testid="resolve-discussion-button"]').click end expect(page).to have_selector('.discussion-next-btn', visible: false) @@ -320,7 +324,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to mark all threads as resolved' do page.all('.discussion-reply-holder', count: 2).each do |reply_holder| page.within reply_holder do - find('button[data-qa-selector="resolve_discussion_button"]').click # rubocop:disable QA/SelectorUsage + find('button[data-testid="resolve-discussion-button"]').click end end @@ -331,7 +335,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to quickly scroll to next unresolved thread' do page.within('.discussion-reply-holder', match: :first) do - find('button[data-qa-selector="resolve_discussion_button"]').click # rubocop:disable QA/SelectorUsage + find('button[data-testid="resolve-discussion-button"]').click end page.within '.discussions-counter' do @@ -402,7 +406,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to mark thread as resolved' do page.within '.diff-content' do - find('button[data-qa-selector="resolve_discussion_button"]').click # rubocop:disable QA/SelectorUsage + find('button[data-testid="resolve-discussion-button"]').click end page.within '.diff-content .note' do @@ -416,7 +420,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to unresolve thread' do page.within '.diff-content' do - find('button[data-qa-selector="resolve_discussion_button"]').click # rubocop:disable QA/SelectorUsage + find('button[data-testid="resolve-discussion-button"]').click click_button 'Unresolve thread' end @@ -443,7 +447,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do it 'allows user to comment & unresolve thread' do page.within '.diff-content' do - find('button[data-qa-selector="resolve_discussion_button"]').click # rubocop:disable QA/SelectorUsage + find('button[data-testid="resolve-discussion-button"]').click find_field('Reply…').click diff --git a/spec/features/merge_request/user_sees_deployment_widget_spec.rb b/spec/features/merge_request/user_sees_deployment_widget_spec.rb index c02149eed87..63ac7862b06 100644 --- a/spec/features/merge_request/user_sees_deployment_widget_spec.rb +++ b/spec/features/merge_request/user_sees_deployment_widget_spec.rb @@ -111,7 +111,7 @@ RSpec.describe 'Merge request > User sees deployment widget', :js do context 'with stop action' do let(:manual) do create(:ci_build, :manual, pipeline: pipeline, - name: 'close_app', environment: environment.name) + name: 'close_app', environment: environment.name) end before do 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 1d3effd4a2a..c2a0e528ea7 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -219,7 +219,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do shared_examples 'pipeline widget' do it 'shows head pipeline information', :sidekiq_might_not_need_inline do within '.ci-widget-content' do - expect(page).to have_content("Detached merge request pipeline ##{pipeline.id} pending for #{pipeline.short_sha}") + expect(page).to have_content("Merge request pipeline ##{pipeline.id} pending for #{pipeline.short_sha}") end end end diff --git a/spec/features/merge_request/user_uses_quick_actions_spec.rb b/spec/features/merge_request/user_uses_quick_actions_spec.rb index 563120fc8b7..ca102913369 100644 --- a/spec/features/merge_request/user_uses_quick_actions_spec.rb +++ b/spec/features/merge_request/user_uses_quick_actions_spec.rb @@ -7,7 +7,7 @@ require 'spec_helper' # for example, adding quick actions when creating the issue and checking DateTime formats on UI. # Because this kind of spec takes more time to run there is no need to add new ones # for each existing quick action unless they test something not tested by existing tests. -RSpec.describe 'Merge request > User uses quick actions', :js do +RSpec.describe 'Merge request > User uses quick actions', :js, :use_clean_rails_redis_caching do include Spec::Support::Helpers::Features::NotesHelpers let(:project) { create(:project, :public, :repository) } diff --git a/spec/features/merge_request/user_views_user_status_on_merge_request_spec.rb b/spec/features/merge_request/user_views_user_status_on_merge_request_spec.rb index b214486b3c1..d3ea8b955f2 100644 --- a/spec/features/merge_request/user_views_user_status_on_merge_request_spec.rb +++ b/spec/features/merge_request/user_views_user_status_on_merge_request_spec.rb @@ -8,6 +8,10 @@ RSpec.describe 'Project > Merge request > View user status' do create(:merge_request, source_project: project, target_project: project, author: create(:user)) end + before do + stub_feature_flags(remove_user_attributes_projects: false) + end + subject { visit merge_request_path(merge_request) } context 'for notes', :js do |