diff options
Diffstat (limited to 'spec')
98 files changed, 2638 insertions, 657 deletions
diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index 298a7ff179c..d20e7368086 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -266,5 +266,19 @@ describe Projects::BranchesController do expect(parsed_response.first).to eq 'master' end end + + context 'show_all = true' do + it 'returns all the branches name' do + get :index, + namespace_id: project.namespace, + project_id: project, + format: :json, + show_all: true + + parsed_response = JSON.parse(response.body) + + expect(parsed_response.length).to eq(project.repository.branches.count) + end + end end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 46c758b4654..6ceaf96f78f 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -104,7 +104,16 @@ describe Projects::IssuesController do project_with_repository.team << [user, :developer] mr = create(:merge_request_with_diff_notes, source_project: project_with_repository) - get :new, namespace_id: project_with_repository.namespace, project_id: project_with_repository, merge_request_for_resolving_discussions: mr.iid + get :new, namespace_id: project_with_repository.namespace, project_id: project_with_repository, merge_request_to_resolve_discussions_of: mr.iid + + expect(assigns(:issue).title).not_to be_empty + expect(assigns(:issue).description).not_to be_empty + end + + it 'fills in an issue for a discussion' do + note = create(:note_on_merge_request, project: project) + + get :new, namespace_id: project.namespace.path, project_id: project, merge_request_to_resolve_discussions_of: note.noteable.iid, discussion_to_resolve: note.discussion_id expect(assigns(:issue).title).not_to be_empty expect(assigns(:issue).description).not_to be_empty @@ -462,11 +471,11 @@ describe Projects::IssuesController do end let(:merge_request_params) do - { merge_request_for_resolving_discussions: merge_request.iid } + { merge_request_to_resolve_discussions_of: merge_request.iid } end - def post_issue(issue_params) - post :create, namespace_id: project.namespace.to_param, project_id: project, issue: issue_params, merge_request_for_resolving_discussions: merge_request.iid + def post_issue(issue_params, other_params: {}) + post :create, { namespace_id: project.namespace.to_param, project_id: project, issue: issue_params, merge_request_to_resolve_discussions_of: merge_request.iid }.merge(other_params) end it 'creates an issue for the project' do @@ -485,6 +494,27 @@ describe Projects::IssuesController do expect(discussion.resolved?).to eq(true) end + + it 'sets a flash message' do + post_issue(title: 'Hello') + + expect(flash[:notice]).to eq('Resolved all discussions.') + end + + describe "resolving a single discussion" do + before do + post_issue({ title: 'Hello' }, other_params: { discussion_to_resolve: discussion.id }) + end + it 'resolves a single discussion' do + discussion.first_note.reload + + expect(discussion.resolved?).to eq(true) + end + + it 'sets a flash message that one discussion was resolved' do + expect(flash[:notice]).to eq('Resolved 1 discussion.') + end + end end context 'Akismet is enabled' do diff --git a/spec/controllers/projects/raw_controller_spec.rb b/spec/controllers/projects/raw_controller_spec.rb index 4cebe3884bf..952071af57f 100644 --- a/spec/controllers/projects/raw_controller_spec.rb +++ b/spec/controllers/projects/raw_controller_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe Projects::RawController do let(:public_project) { create(:project, :public, :repository) } - describe "#show" do + describe '#show' do context 'regular filename' do let(:id) { 'master/README.md' } @@ -16,8 +16,8 @@ describe Projects::RawController do expect(response).to have_http_status(200) expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8') expect(response.header['Content-Disposition']). - to eq("inline") - expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-blob:") + to eq('inline') + expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:') end end @@ -32,7 +32,7 @@ describe Projects::RawController do expect(response).to have_http_status(200) expect(response.header['Content-Type']).to eq('image/jpeg') - expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("git-blob:") + expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:') end end @@ -40,32 +40,57 @@ describe Projects::RawController do let(:id) { 'be93687/files/lfs/lfs_object.iso' } let!(:lfs_object) { create(:lfs_object, oid: '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897', size: '1575078') } - context 'when project has access' do + context 'when lfs is enabled' do before do - public_project.lfs_objects << lfs_object - allow_any_instance_of(LfsObjectUploader).to receive(:exists?).and_return(true) - allow(controller).to receive(:send_file) { controller.head :ok } + allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(true) end - it 'serves the file' do - expect(controller).to receive(:send_file).with("#{Gitlab.config.shared.path}/lfs-objects/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", filename: "lfs_object.iso", disposition: 'attachment') - get(:show, - namespace_id: public_project.namespace.to_param, - project_id: public_project, - id: id) + context 'when project has access' do + before do + public_project.lfs_objects << lfs_object + allow_any_instance_of(LfsObjectUploader).to receive(:exists?).and_return(true) + allow(controller).to receive(:send_file) { controller.head :ok } + end - expect(response).to have_http_status(200) + it 'serves the file' do + expect(controller).to receive(:send_file).with("#{Gitlab.config.shared.path}/lfs-objects/91/ef/f75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897", filename: 'lfs_object.iso', disposition: 'attachment') + get(:show, + namespace_id: public_project.namespace.to_param, + project_id: public_project, + id: id) + + expect(response).to have_http_status(200) + end + end + + context 'when project does not have access' do + it 'does not serve the file' do + get(:show, + namespace_id: public_project.namespace.to_param, + project_id: public_project, + id: id) + + expect(response).to have_http_status(404) + end end end - context 'when project does not have access' do - it 'does not serve the file' do + context 'when lfs is not enabled' do + before do + allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(false) + end + + it 'delivers ASCII file' do get(:show, namespace_id: public_project.namespace.to_param, project_id: public_project, id: id) - expect(response).to have_http_status(404) + expect(response).to have_http_status(200) + expect(response.header['Content-Type']).to eq('text/plain; charset=utf-8') + expect(response.header['Content-Disposition']). + to eq('inline') + expect(response.header[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with('git-blob:') end end end diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb index 77404f46c92..b67c96bc00d 100644 --- a/spec/factories/ci/pipelines.rb +++ b/spec/factories/ci/pipelines.rb @@ -40,6 +40,14 @@ FactoryGirl.define do trait :invalid do config(rspec: nil) end + + trait :blocked do + status :manual + end + + trait :success do + status :success + end end end end diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb index 5c50cd7f4ad..fe19a404e16 100644 --- a/spec/factories/notes.rb +++ b/spec/factories/notes.rb @@ -26,12 +26,17 @@ FactoryGirl.define do factory :diff_note_on_merge_request, traits: [:on_merge_request], class: DiffNote do association :project, :repository + + transient do + line_number 14 + end + position do Gitlab::Diff::Position.new( old_path: "files/ruby/popen.rb", new_path: "files/ruby/popen.rb", old_line: nil, - new_line: 14, + new_line: line_number, diff_refs: noteable.diff_refs ) end diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb index c6f91e05d83..0db2fe04edd 100644 --- a/spec/factories/projects.rb +++ b/spec/factories/projects.rb @@ -38,7 +38,7 @@ FactoryGirl.define do trait :empty_repo do after(:create) do |project| - project.create_repository + raise "Failed to create repository!" unless project.create_repository # We delete hooks so that gitlab-shell will not try to authenticate with # an API that isn't running @@ -48,7 +48,7 @@ FactoryGirl.define do trait :broken_repo do after(:create) do |project| - project.create_repository + raise "Failed to create repository!" unless project.create_repository FileUtils.rm_r(File.join(project.repository_storage_path, "#{project.path_with_namespace}.git", 'refs')) end diff --git a/spec/features/dashboard/user_filters_projects_spec.rb b/spec/features/dashboard/user_filters_projects_spec.rb index c2e0612aef8..34d6257f5fd 100644 --- a/spec/features/dashboard/user_filters_projects_spec.rb +++ b/spec/features/dashboard/user_filters_projects_spec.rb @@ -1,26 +1,45 @@ require 'spec_helper' -describe "Dashboard > User filters projects", feature: true do +describe 'Dashboard > User filters projects', :feature do + let(:user) { create(:user) } + let(:project) { create(:project, name: 'Victorialand', namespace: user.namespace) } + let(:user2) { create(:user) } + let(:project2) { create(:project, name: 'Treasure', namespace: user2.namespace) } + + before do + project.team << [user, :master] + + login_as(user) + end + describe 'filtering personal projects' do before do - user = create(:user) - project = create(:project, name: "Victorialand", namespace: user.namespace) - project.team << [user, :master] - - user2 = create(:user) - project2 = create(:project, name: "Treasure", namespace: user2.namespace) project2.team << [user, :developer] - login_as(user) visit dashboard_projects_path end it 'filters by projects "Owned by me"' do - click_link "Owned by me" + click_link 'Owned by me' expect(page).to have_css('.is-active', text: 'Owned by me') expect(page).to have_content('Victorialand') expect(page).not_to have_content('Treasure') end end + + describe 'filtering starred projects', :js do + before do + user.toggle_star(project) + + visit dashboard_projects_path + end + + it 'returns message when starred projects fitler returns no results' do + fill_in 'project-filter-form-field', with: 'Beta\n' + + expect(page).to have_content('No projects found') + expect(page).not_to have_content('You don\'t have starred projects yet') + end + end end diff --git a/spec/features/dashboard_issues_spec.rb b/spec/features/dashboard_issues_spec.rb index aa75e1140f6..8c61cdebc4b 100644 --- a/spec/features/dashboard_issues_spec.rb +++ b/spec/features/dashboard_issues_spec.rb @@ -48,7 +48,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do it 'updates atom feed link' do visit_issues(milestone_title: '', assignee_id: user.id) - link = find('.nav-controls a', text: 'Subscribe') + link = find('.nav-controls a[title="Subscribe"]') params = CGI.parse(URI.parse(link[:href]).query) auto_discovery_link = find('link[type="application/atom+xml"]', visible: false) auto_discovery_params = CGI.parse(URI.parse(auto_discovery_link[:href]).query) diff --git a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb index 762cab0c0e1..572bca3de21 100644 --- a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb +++ b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb @@ -1,76 +1,93 @@ require 'rails_helper' -feature 'Resolving all open discussions in a merge request from an issue', feature: true do +feature 'Resolving all open discussions in a merge request from an issue', feature: true, js: true do let(:user) { create(:user) } - let(:project) { create(:project, only_allow_merge_if_all_discussions_are_resolved: true) } + let(:project) { create(:project) } let(:merge_request) { create(:merge_request, source_project: project) } let!(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request, noteable: merge_request, project: project)]).first } - before do - project.team << [user, :master] - login_as user - end - - context 'with the internal tracker disabled' do + describe 'as a user with access to the project' do before do - project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) + project.team << [user, :master] + login_as user visit namespace_project_merge_request_path(project.namespace, project, merge_request) end - it 'does not show a link to create a new issue' do - expect(page).not_to have_link 'open an issue to resolve them later' - end - end - - context 'merge request has discussions that need to be resolved' do - before do - visit namespace_project_merge_request_path(project.namespace, project, merge_request) + it 'shows a button to resolve all discussions by creating a new issue' do + within('li#resolve-count-app') do + expect(page).to have_link "Resolve all discussions in new issue", href: new_namespace_project_issue_path(project.namespace, project, merge_request_to_resolve_discussions_of: merge_request.iid) + end end - it 'shows a warning that the merge request contains unresolved discussions' do - expect(page).to have_content 'This merge request has unresolved discussions' - end + context 'resolving the discussion' do + before do + click_button 'Resolve discussion' + end - it 'has a link to resolve all discussions by creating an issue' do - page.within '.mr-widget-body' do - expect(page).to have_link 'open an issue to resolve them later', href: new_namespace_project_issue_path(project.namespace, project, merge_request_for_resolving_discussions: merge_request.iid) + it 'hides the link for creating a new issue' do + expect(page).not_to have_link "Resolve all discussions in new issue", href: new_namespace_project_issue_path(project.namespace, project, merge_request_to_resolve_discussions_of: merge_request.iid) end end context 'creating an issue for discussions' do before do - page.click_link 'open an issue to resolve them later', href: new_namespace_project_issue_path(project.namespace, project, merge_request_for_resolving_discussions: merge_request.iid) + click_link "Resolve all discussions in new issue", href: new_namespace_project_issue_path(project.namespace, project, merge_request_to_resolve_discussions_of: merge_request.iid) end - it 'shows an issue with the title filled in' do - title_field = page.find_field('issue[title]') + it_behaves_like 'creating an issue for a discussion' + end - expect(title_field.value).to include(merge_request.title) + context 'for a project where all discussions need to be resolved before merging' do + before do + project.update_attribute(:only_allow_merge_if_all_discussions_are_resolved, true) end - it 'has a mention of the discussion in the description' do - description_field = page.find_field('issue[description]') + context 'with the internal tracker disabled' do + before do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end - expect(description_field.value).to include(discussion.first_note.note) + it 'does not show a link to create a new issue' do + expect(page).not_to have_link 'open an issue to resolve them later' + end end - it 'has a hidden field for the merge request' do - merge_request_field = find('#merge_request_for_resolving_discussions', visible: false) - - expect(merge_request_field.value).to eq(merge_request.iid.to_s) - end + context 'merge request has discussions that need to be resolved' do + before do + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end - it 'can create a new issue for the project' do - expect { click_button 'Submit issue' }.to change { project.issues.reload.size }.by(1) - end + it 'shows a warning that the merge request contains unresolved discussions' do + expect(page).to have_content 'This merge request has unresolved discussions' + end - it 'resolves the discussion in the merge request' do - click_button 'Submit issue' + it 'has a link to resolve all discussions by creating an issue' do + page.within '.mr-widget-body' do + expect(page).to have_link 'open an issue to resolve them later', href: new_namespace_project_issue_path(project.namespace, project, merge_request_to_resolve_discussions_of: merge_request.iid) + end + end - discussion.first_note.reload + context 'creating an issue for discussions' do + before do + page.click_link 'open an issue to resolve them later', href: new_namespace_project_issue_path(project.namespace, project, merge_request_to_resolve_discussions_of: merge_request.iid) + end - expect(discussion.resolved?).to eq(true) + it_behaves_like 'creating an issue for a discussion' + end end end end + + describe 'as a reporter' do + before do + project.team << [user, :reporter] + login_as user + visit new_namespace_project_issue_path(project.namespace, project, merge_request_to_resolve_discussions_of: merge_request.iid) + end + + it 'Shows a notice to ask someone else to resolve the discussions' do + expect(page).to have_content("The discussions at #{merge_request.to_reference} will stay unresolved. Ask someone with permission to resolve them.") + end + end end diff --git a/spec/features/issues/create_issue_for_single_discussion_in_merge_request.rb b/spec/features/issues/create_issue_for_single_discussion_in_merge_request.rb new file mode 100644 index 00000000000..88e2cc60d79 --- /dev/null +++ b/spec/features/issues/create_issue_for_single_discussion_in_merge_request.rb @@ -0,0 +1,81 @@ +require 'rails_helper' + +feature 'Resolve an open discussion in a merge request by creating an issue', feature: true do + let(:user) { create(:user) } + let(:project) { create(:project, only_allow_merge_if_all_discussions_are_resolved: true) } + let(:merge_request) { create(:merge_request, source_project: project) } + let!(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request, noteable: merge_request, project: project)]).first } + + describe 'As a user with access to the project' do + before do + project.team << [user, :master] + login_as user + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + context 'with the internal tracker disabled' do + before do + project.project_feature.update_attribute(:issues_access_level, ProjectFeature::DISABLED) + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'does not show a link to create a new issue' do + expect(page).not_to have_link 'Resolve this discussion in a new issue' + end + end + + context 'resolving the discussion', js: true do + before do + click_button 'Resolve discussion' + end + + it 'hides the link for creating a new issue' do + expect(page).not_to have_link 'Resolve this discussion in a new issue' + end + + it 'shows the link for creating a new issue when unresolving a discussion' do + page.within '.diff-content' do + click_button 'Unresolve discussion' + end + + expect(page).to have_link 'Resolve this discussion in a new issue' + end + end + + it 'has a link to create a new issue for a discussion' do + new_issue_link = new_namespace_project_issue_path(project.namespace, project, discussion_to_resolve: discussion.id, merge_request_to_resolve_discussions_of: merge_request.iid) + + expect(page).to have_link 'Resolve this discussion in a new issue', href: new_issue_link + end + + context 'creating the issue' do + before do + click_link 'Resolve this discussion in a new issue', href: new_namespace_project_issue_path(project.namespace, project, discussion_to_resolve: discussion.id, merge_request_to_resolve_discussions_of: merge_request.iid) + end + + it 'has a hidden field for the discussion' do + discussion_field = find('#discussion_to_resolve', visible: false) + + expect(discussion_field.value).to eq(discussion.id.to_s) + end + + it_behaves_like 'creating an issue for a discussion' + end + end + + describe 'as a reporter' do + before do + project.team << [user, :reporter] + login_as user + visit new_namespace_project_issue_path(project.namespace, project, + merge_request_to_resolve_discussions_of: merge_request.iid, + discussion_to_resolve: discussion.id) + end + + it 'Shows a notice to ask someone else to resolve the discussions' do + expect(page).to have_content("The discussion at #{merge_request.to_reference}"\ + "(discussion #{discussion.first_note.id}) will stay unresolved."\ + "Ask someone with permission to resolve it.") + end + end +end diff --git a/spec/features/issues/filtered_search/dropdown_milestone_spec.rb b/spec/features/issues/filtered_search/dropdown_milestone_spec.rb index 0324fcad0a0..85ffffe4b6d 100644 --- a/spec/features/issues/filtered_search/dropdown_milestone_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_milestone_spec.rb @@ -1,8 +1,7 @@ require 'rails_helper' -describe 'Dropdown milestone', js: true, feature: true do +describe 'Dropdown milestone', :feature, :js do include FilteredSearchHelpers - include WaitForAjax let!(:project) { create(:empty_project) } let!(:user) { create(:user) } @@ -15,18 +14,10 @@ describe 'Dropdown milestone', js: true, feature: true do let(:filtered_search) { find('.filtered-search') } let(:js_dropdown_milestone) { '#js-dropdown-milestone' } - - def send_keys_to_filtered_search(input) - input.split("").each do |i| - filtered_search.send_keys(i) - sleep 3 - wait_for_ajax - sleep 3 - end - end + let(:filter_dropdown) { find("#{js_dropdown_milestone} .filter-dropdown") } def dropdown_milestone_size - page.all('#js-dropdown-milestone .filter-dropdown .filter-dropdown-item').size + filter_dropdown.all('.filter-dropdown-item').size end def click_milestone(text) @@ -65,13 +56,14 @@ describe 'Dropdown milestone', js: true, feature: true do end it 'should hide loading indicator when loaded' do - send_keys_to_filtered_search('milestone:') + filtered_search.set('milestone:') - expect(page).not_to have_css('#js-dropdown-milestone .filter-dropdown-loading') + expect(find(js_dropdown_milestone)).to have_css('.filter-dropdown-loading') + expect(find(js_dropdown_milestone)).not_to have_css('.filter-dropdown-loading') end it 'should load all the milestones when opened' do - send_keys_to_filtered_search('milestone:') + filtered_search.set('milestone:') expect(dropdown_milestone_size).to be > 0 end @@ -79,41 +71,48 @@ describe 'Dropdown milestone', js: true, feature: true do describe 'filtering' do before do - filtered_search.set('milestone') + filtered_search.set('milestone:') + + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(milestone.title) + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(uppercase_milestone.title) + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(two_words_milestone.title) + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(wont_fix_milestone.title) + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(special_milestone.title) + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(long_milestone.title) end it 'filters by name' do - send_keys_to_filtered_search(':v1') + filtered_search.send_keys('v1') expect(dropdown_milestone_size).to eq(1) end it 'filters by case insensitive name' do - send_keys_to_filtered_search(':V1') + filtered_search.send_keys('V1') expect(dropdown_milestone_size).to eq(1) end it 'filters by name with symbol' do - send_keys_to_filtered_search(':%v1') + filtered_search.send_keys('%v1') expect(dropdown_milestone_size).to eq(1) end it 'filters by case insensitive name with symbol' do - send_keys_to_filtered_search(':%V1') + filtered_search.send_keys('%V1') expect(dropdown_milestone_size).to eq(1) end it 'filters by special characters' do - send_keys_to_filtered_search(':(+') + filtered_search.send_keys('(+') expect(dropdown_milestone_size).to eq(1) end it 'filters by special characters with symbol' do - send_keys_to_filtered_search(':%(+') + filtered_search.send_keys('%(+') expect(dropdown_milestone_size).to eq(1) end @@ -122,6 +121,13 @@ describe 'Dropdown milestone', js: true, feature: true do describe 'selecting from dropdown' do before do filtered_search.set('milestone:') + + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(milestone.title) + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(uppercase_milestone.title) + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(two_words_milestone.title) + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(wont_fix_milestone.title) + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(special_milestone.title) + expect(find("#{js_dropdown_milestone} .filter-dropdown")).to have_content(long_milestone.title) end it 'fills in the milestone name when the milestone has not been filled' do @@ -133,7 +139,7 @@ describe 'Dropdown milestone', js: true, feature: true do end it 'fills in the milestone name when the milestone is partially filled' do - send_keys_to_filtered_search('v') + filtered_search.send_keys('v') click_milestone(milestone.title) expect(page).to have_css(js_dropdown_milestone, visible: false) @@ -232,16 +238,14 @@ describe 'Dropdown milestone', js: true, feature: true do describe 'caching requests' do it 'caches requests after the first load' do - filtered_search.set('milestone') - send_keys_to_filtered_search(':') + filtered_search.set('milestone:') initial_size = dropdown_milestone_size expect(initial_size).to be > 0 create(:milestone, project: project) find('.filtered-search-input-container .clear-search').click - filtered_search.set('milestone') - send_keys_to_filtered_search(':') + filtered_search.set('milestone:') expect(dropdown_milestone_size).to eq(initial_size) end diff --git a/spec/features/issues/filtered_search/visual_tokens_spec.rb b/spec/features/issues/filtered_search/visual_tokens_spec.rb index a78dbaf53ed..96e87c82d2c 100644 --- a/spec/features/issues/filtered_search/visual_tokens_spec.rb +++ b/spec/features/issues/filtered_search/visual_tokens_spec.rb @@ -242,6 +242,23 @@ describe 'Visual tokens', js: true, feature: true do end end + describe 'editing a search term while editing another filter token' do + before do + input_filtered_search('author assignee:', submit: false) + first('.tokens-container .filtered-search-term').double_click + end + + it 'opens hint dropdown' do + expect(page).to have_css('#js-dropdown-hint', visible: true) + end + + it 'opens author dropdown' do + find('#js-dropdown-hint .filter-dropdown .filter-dropdown-item', text: 'author').click + + expect(page).to have_css('#js-dropdown-author', visible: true) + end + end + describe 'add new token after editing existing token' do before do input_filtered_search('author:@root assignee:none', submit: false) @@ -319,4 +336,17 @@ describe 'Visual tokens', js: true, feature: true do expect(token.find('.name').text).to eq('Author') end end + + describe 'search using incomplete visual tokens' do + before do + input_filtered_search('author:@root assignee:none', extra_space: false) + end + + it 'tokenizes the search term to complete visual token' do + expect_tokens([ + { name: 'author', value: '@root' }, + { name: 'assignee', value: 'none' } + ]) + end + end end diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index d4e0ef91856..755992069ff 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe 'New/edit issue', feature: true, js: true do + include GitlabRoutingHelper + let!(:project) { create(:project) } let!(:user) { create(:user)} let!(:user2) { create(:user)} @@ -78,6 +80,14 @@ describe 'New/edit issue', feature: true, js: true do expect(page).to have_content label2.title end end + + page.within '.issuable-meta' do + issue = Issue.find_by(title: 'title') + + expect(page).to have_text("Issue #{issue.to_reference}") + # compare paths because the host differ in test + expect(find_link(issue.to_reference)[:href]).to end_with(issue_path(issue)) + end end it 'correctly updates the dropdown toggle when removing a label' do diff --git a/spec/features/login_spec.rb b/spec/features/login_spec.rb index ae609160e18..f32d1f78b40 100644 --- a/spec/features/login_spec.rb +++ b/spec/features/login_spec.rb @@ -48,6 +48,18 @@ feature 'Login', feature: true do end end + describe 'with the ghost user' do + it 'disallows login' do + login_with(User.ghost) + + expect(page).to have_content('Invalid Login or password.') + end + + it 'does not update Devise trackable attributes' do + expect { login_with(User.ghost) }.not_to change { User.ghost.reload.sign_in_count } + end + end + describe 'with two-factor authentication' do def enter_code(code) fill_in 'user_otp_attempt', with: code diff --git a/spec/features/merge_requests/diff_notes_avatars_spec.rb b/spec/features/merge_requests/diff_notes_avatars_spec.rb new file mode 100644 index 00000000000..a6c72b0b3ac --- /dev/null +++ b/spec/features/merge_requests/diff_notes_avatars_spec.rb @@ -0,0 +1,186 @@ +require 'spec_helper' + +feature 'Diff note avatars', feature: true, js: true do + include WaitForAjax + + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + 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, + new_path: path, + old_line: nil, + new_line: 9, + diff_refs: merge_request.diff_refs + ) + end + let!(:note) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) } + + before do + project.team << [user, :master] + login_as user + end + + context 'discussion tab' do + before do + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + end + + it 'does not show avatars on discussion tab' do + expect(page).not_to have_selector('.js-avatar-container') + expect(page).not_to have_selector('.diff-comment-avatar-holders') + end + + it 'does not render avatars after commening on discussion tab' do + click_button 'Reply...' + + page.within('.js-discussion-note-form') do + find('.note-textarea').native.send_keys('Test comment') + + click_button 'Comment' + end + + expect(page).to have_content('Test comment') + expect(page).not_to have_selector('.js-avatar-container') + expect(page).not_to have_selector('.diff-comment-avatar-holders') + end + end + + context 'commit view' do + before do + visit namespace_project_commit_path(project.namespace, project, merge_request.commits.first.id) + end + + it 'does not render avatar after commenting' do + first('.diff-line-num').trigger('mouseover') + find('.js-add-diff-note-button').click + + page.within('.js-discussion-note-form') do + find('.note-textarea').native.send_keys('test comment') + + click_button 'Comment' + + wait_for_ajax + end + + visit namespace_project_merge_request_path(project.namespace, project, merge_request) + + expect(page).to have_content('test comment') + expect(page).not_to have_selector('.js-avatar-container') + expect(page).not_to have_selector('.diff-comment-avatar-holders') + end + end + + %w(inline parallel).each do |view| + context "#{view} view" do + before do + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: view) + + wait_for_ajax + end + + it 'shows note avatar' do + page.within find("[id='#{position.line_code(project.repository)}']") do + find('.diff-notes-collapse').click + + expect(page).to have_selector('img.js-diff-comment-avatar', count: 1) + end + end + + it 'shows comment on note avatar' do + page.within find("[id='#{position.line_code(project.repository)}']") do + find('.diff-notes-collapse').click + + expect(first('img.js-diff-comment-avatar')["title"]).to eq("#{note.author.name}: #{note.note.truncate(17)}") + end + end + + it 'toggles comments when clicking avatar' do + page.within find("[id='#{position.line_code(project.repository)}']") do + find('.diff-notes-collapse').click + end + + expect(page).to have_selector('.notes_holder', visible: false) + + page.within find("[id='#{position.line_code(project.repository)}']") do + first('img.js-diff-comment-avatar').click + end + + expect(page).to have_selector('.notes_holder') + end + + it 'removes avatar when note is deleted' do + page.within find(".note-row-#{note.id}") do + find('.js-note-delete').click + end + + wait_for_ajax + + page.within find("[id='#{position.line_code(project.repository)}']") do + expect(page).not_to have_selector('img.js-diff-comment-avatar') + end + end + + it 'adds avatar when commenting' do + click_button 'Reply...' + + page.within '.js-discussion-note-form' do + find('.js-note-text').native.send_keys('Test') + + click_button 'Comment' + + wait_for_ajax + end + + page.within find("[id='#{position.line_code(project.repository)}']") do + find('.diff-notes-collapse').click + + expect(page).to have_selector('img.js-diff-comment-avatar', count: 2) + end + end + + it 'adds multiple comments' do + 3.times do + click_button 'Reply...' + + page.within '.js-discussion-note-form' do + find('.js-note-text').native.send_keys('Test') + + find('.js-comment-button').trigger 'click' + + wait_for_ajax + end + end + + page.within find("[id='#{position.line_code(project.repository)}']") do + find('.diff-notes-collapse').click + + expect(page).to have_selector('img.js-diff-comment-avatar', count: 3) + expect(find('.diff-comments-more-count')).to have_content '+1' + end + end + + context 'multiple comments' do + before do + create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) + create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) + create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: position) + + visit diffs_namespace_project_merge_request_path(project.namespace, project, merge_request, view: view) + + wait_for_ajax + end + + it 'shows extra comment count' do + page.within find("[id='#{position.line_code(project.repository)}']") do + find('.diff-notes-collapse').click + + expect(find('.diff-comments-more-count')).to have_content '+1' + end + end + end + end + end +end diff --git a/spec/features/merge_requests/form_spec.rb b/spec/features/merge_requests/form_spec.rb index 1ecdb8b5983..f8518f450dc 100644 --- a/spec/features/merge_requests/form_spec.rb +++ b/spec/features/merge_requests/form_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' describe 'New/edit merge request', feature: true, js: true do + include GitlabRoutingHelper + let!(:project) { create(:project, visibility_level: Gitlab::VisibilityLevel::PUBLIC) } let(:fork_project) { create(:project, forked_from_project: project) } let!(:user) { create(:user)} @@ -84,6 +86,15 @@ describe 'New/edit merge request', feature: true, js: true do expect(page).to have_content label2.title end end + + page.within '.issuable-meta' do + merge_request = MergeRequest.find_by(source_branch: 'fix') + + expect(page).to have_text("Merge Request #{merge_request.to_reference}") + # compare paths because the host differ in test + expect(find_link(merge_request.to_reference)[:href]) + .to end_with(merge_request_path(merge_request)) + end end end diff --git a/spec/features/projects/blobs/blob_line_permalink_updater_spec.rb b/spec/features/projects/blobs/blob_line_permalink_updater_spec.rb new file mode 100644 index 00000000000..d94204230f6 --- /dev/null +++ b/spec/features/projects/blobs/blob_line_permalink_updater_spec.rb @@ -0,0 +1,97 @@ +require 'spec_helper' + +feature 'Blob button line permalinks (BlobLinePermalinkUpdater)', feature: true, js: true do + include TreeHelper + + let(:project) { create(:project, :public, :repository) } + let(:path) { 'CHANGELOG' } + let(:sha) { project.repository.commit.sha } + + describe 'On a file(blob)' do + def get_absolute_url(path = "") + "http://#{page.server.host}:#{page.server.port}#{path}" + end + + def visit_blob(fragment = nil) + visit namespace_project_blob_path(project.namespace, project, tree_join('master', path), anchor: fragment) + end + + describe 'Click "Permalink" button' do + it 'works with no initial line number fragment hash' do + visit_blob + + expect(find('.js-data-file-blob-permalink-url')['href']).to eq(get_absolute_url(namespace_project_blob_path(project.namespace, project, tree_join(sha, path)))) + end + + it 'maintains intitial fragment hash' do + fragment = "L3" + + visit_blob(fragment) + + expect(find('.js-data-file-blob-permalink-url')['href']).to eq(get_absolute_url(namespace_project_blob_path(project.namespace, project, tree_join(sha, path), anchor: fragment))) + end + + it 'changes fragment hash if line number clicked' do + ending_fragment = "L5" + + visit_blob + + find('#L3').click + find("##{ending_fragment}").click + + expect(find('.js-data-file-blob-permalink-url')['href']).to eq(get_absolute_url(namespace_project_blob_path(project.namespace, project, tree_join(sha, path), anchor: ending_fragment))) + end + + it 'with initial fragment hash, changes fragment hash if line number clicked' do + fragment = "L1" + ending_fragment = "L5" + + visit_blob(fragment) + + find('#L3').click + find("##{ending_fragment}").click + + expect(find('.js-data-file-blob-permalink-url')['href']).to eq(get_absolute_url(namespace_project_blob_path(project.namespace, project, tree_join(sha, path), anchor: ending_fragment))) + end + end + + describe 'Click "Blame" button' do + it 'works with no initial line number fragment hash' do + visit_blob + + expect(find('.js-blob-blame-link')['href']).to eq(get_absolute_url(namespace_project_blame_path(project.namespace, project, tree_join('master', path)))) + end + + it 'maintains intitial fragment hash' do + fragment = "L3" + + visit_blob(fragment) + + expect(find('.js-blob-blame-link')['href']).to eq(get_absolute_url(namespace_project_blame_path(project.namespace, project, tree_join('master', path), anchor: fragment))) + end + + it 'changes fragment hash if line number clicked' do + ending_fragment = "L5" + + visit_blob + + find('#L3').click + find("##{ending_fragment}").click + + expect(find('.js-blob-blame-link')['href']).to eq(get_absolute_url(namespace_project_blame_path(project.namespace, project, tree_join('master', path), anchor: ending_fragment))) + end + + it 'with initial fragment hash, changes fragment hash if line number clicked' do + fragment = "L1" + ending_fragment = "L5" + + visit_blob(fragment) + + find('#L3').click + find("##{ending_fragment}").click + + expect(find('.js-blob-blame-link')['href']).to eq(get_absolute_url(namespace_project_blame_path(project.namespace, project, tree_join('master', path), anchor: ending_fragment))) + end + end + end +end diff --git a/spec/features/projects/blobs/user_create_spec.rb b/spec/features/projects/blobs/user_create_spec.rb new file mode 100644 index 00000000000..03d08c12612 --- /dev/null +++ b/spec/features/projects/blobs/user_create_spec.rb @@ -0,0 +1,107 @@ +require 'spec_helper' + +feature 'New blob creation', feature: true, js: true do + include WaitForAjax + + given(:user) { create(:user) } + given(:role) { :developer } + given(:project) { create(:project) } + given(:content) { 'class NextFeature\nend\n' } + + background do + login_as(user) + project.team << [user, role] + visit namespace_project_new_blob_path(project.namespace, project, 'master') + end + + def edit_file + wait_for_ajax + fill_in 'file_name', with: 'feature.rb' + execute_script("ace.edit('editor').setValue('#{content}')") + end + + def select_branch_index(index) + first('button.js-target-branch').click + wait_for_ajax + all('a[data-group="Branches"]')[index].click + end + + def create_new_branch(name) + first('button.js-target-branch').click + click_link 'Create new branch' + fill_in 'new_branch_name', with: name + click_button 'Create' + end + + def commit_file + click_button 'Commit Changes' + end + + context 'with default target branch' do + background do + edit_file + commit_file + end + + scenario 'creates the blob in the default branch' do + expect(page).to have_content 'master' + expect(page).to have_content 'successfully created' + expect(page).to have_content 'NextFeature' + end + end + + context 'with different target branch' do + background do + edit_file + select_branch_index(0) + commit_file + end + + scenario 'creates the blob in the different branch' do + expect(page).to have_content 'test' + expect(page).to have_content 'successfully created' + end + end + + context 'with a new target branch' do + given(:new_branch_name) { 'new-feature' } + + background do + edit_file + create_new_branch(new_branch_name) + commit_file + end + + scenario 'creates the blob in the new branch' do + expect(page).to have_content new_branch_name + expect(page).to have_content 'successfully created' + end + scenario 'returns you to the mr' do + expect(page).to have_content 'New Merge Request' + expect(page).to have_content "From #{new_branch_name} into master" + expect(page).to have_content 'Add new file' + end + end + + context 'the file already exist in the source branch' do + background do + Files::CreateService.new( + project, + user, + start_branch: 'master', + target_branch: 'master', + commit_message: 'Create file', + file_path: 'feature.rb', + file_content: content + ).execute + edit_file + commit_file + end + + scenario 'shows error message' do + expect(page).to have_content('Your changes could not be committed because a file with the same name already exists') + expect(page).to have_content('New File') + expect(page).to have_content('NextFeature') + end + end +end diff --git a/spec/features/projects/edit_spec.rb b/spec/features/projects/edit_spec.rb index a1643fd1f43..7c319af893b 100644 --- a/spec/features/projects/edit_spec.rb +++ b/spec/features/projects/edit_spec.rb @@ -21,36 +21,28 @@ feature 'Project edit', feature: true, js: true do expect(page).to have_selector('.merge-requests-feature', visible: false) end - it 'hides merge requests section after save' do - select('Disabled', from: 'project_project_feature_attributes_merge_requests_access_level') - - expect(page).to have_selector('.merge-requests-feature', visible: false) - - click_button 'Save changes' + context 'given project with merge_requests_disabled access level' do + let(:project) { create(:project, :merge_requests_disabled) } - wait_for_ajax - - expect(page).to have_selector('.merge-requests-feature', visible: false) + it 'hides merge requests section' do + expect(page).to have_selector('.merge-requests-feature', visible: false) + end end end context 'builds select' do - it 'hides merge requests section' do + it 'hides builds select section' do select('Disabled', from: 'project_project_feature_attributes_builds_access_level') expect(page).to have_selector('.builds-feature', visible: false) end - it 'hides merge requests section after save' do - select('Disabled', from: 'project_project_feature_attributes_builds_access_level') - - expect(page).to have_selector('.builds-feature', visible: false) + context 'given project with builds_disabled access level' do + let(:project) { create(:project, :builds_disabled) } - click_button 'Save changes' - - wait_for_ajax - - expect(page).to have_selector('.builds-feature', visible: false) + it 'hides builds select section' do + expect(page).to have_selector('.builds-feature', visible: false) + end end end end diff --git a/spec/features/projects/environments/environments_spec.rb b/spec/features/projects/environments/environments_spec.rb index 25f31b423b8..641e2cf7402 100644 --- a/spec/features/projects/environments/environments_spec.rb +++ b/spec/features/projects/environments/environments_spec.rb @@ -111,10 +111,8 @@ feature 'Environments page', :feature, :js do find('.js-dropdown-play-icon-container').click expect(page).to have_content(action.name.humanize) - expect { click_link(action.name.humanize) } + expect { find('.js-manual-action-link').click } .not_to change { Ci::Pipeline.count } - - expect(action.reload).to be_pending end scenario 'does show build name and id' do @@ -158,12 +156,6 @@ feature 'Environments page', :feature, :js do expect(page).to have_selector('.stop-env-link') end - scenario 'starts build when stop button clicked' do - find('.stop-env-link').click - - expect(page).to have_content('close_app') - end - context 'for reporter' do let(:role) { :reporter } diff --git a/spec/features/projects/files/browse_files_spec.rb b/spec/features/projects/files/browse_files_spec.rb index 69295e450d0..d281043caa3 100644 --- a/spec/features/projects/files/browse_files_spec.rb +++ b/spec/features/projects/files/browse_files_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'user checks git blame', feature: true do +feature 'user browses project', feature: true do let(:project) { create(:project) } let(:user) { create(:user) } @@ -18,4 +18,16 @@ feature 'user checks git blame', feature: true do expect(page).to have_content "Dmitriy Zaporozhets" expect(page).to have_content "Initial commit" end + + scenario 'can see raw content of LFS pointer with LFS disabled' do + allow_any_instance_of(Project).to receive(:lfs_enabled?).and_return(false) + click_link 'files' + click_link 'lfs' + click_link 'lfs_object.iso' + + expect(page).not_to have_content 'Download (1.5 MB)' + expect(page).to have_content 'version https://git-lfs.github.com/spec/v1' + expect(page).to have_content 'oid sha256:91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897' + expect(page).to have_content 'size 1575078' + end end diff --git a/spec/features/projects/members/user_requests_access_spec.rb b/spec/features/projects/members/user_requests_access_spec.rb index 0b4dcaa39c6..b64c15e0adc 100644 --- a/spec/features/projects/members/user_requests_access_spec.rb +++ b/spec/features/projects/members/user_requests_access_spec.rb @@ -57,6 +57,12 @@ feature 'Projects > Members > User requests access', feature: true do end def open_project_settings_menu - find('#project-settings-button').click + page.within('.layout-nav .nav-links') do + click_link('Settings') + end + + page.within('.page-with-layout-nav .sub-nav') do + click_link('Members') + end end end diff --git a/spec/features/tags/master_deletes_tag_spec.rb b/spec/features/tags/master_deletes_tag_spec.rb index 0f30f562539..ccfafe6db7d 100644 --- a/spec/features/tags/master_deletes_tag_spec.rb +++ b/spec/features/tags/master_deletes_tag_spec.rb @@ -10,16 +10,12 @@ feature 'Master deletes tag', feature: true do visit namespace_project_tags_path(project.namespace, project) end - context 'from the tags list page' do + context 'from the tags list page', js: true do scenario 'deletes the tag' do expect(page).to have_content 'v1.1.0' - page.within('.content') do - first('.btn-remove').click - end + delete_first_tag - expect(current_path).to eq( - namespace_project_tags_path(project.namespace, project)) expect(page).not_to have_content 'v1.1.0' end end @@ -37,4 +33,23 @@ feature 'Master deletes tag', feature: true do expect(page).not_to have_content 'v1.0.0' end end + + context 'when pre-receive hook fails', js: true do + before do + allow_any_instance_of(GitHooksService).to receive(:execute) + .and_raise(GitHooksService::PreReceiveError, 'Do not delete tags') + end + + scenario 'shows the error message' do + delete_first_tag + + expect(page).to have_content('Do not delete tags') + end + end + + def delete_first_tag + page.within('.content') do + first('.btn-remove').click + end + end end diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index 3495091a0d5..5c2df949ac5 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -38,7 +38,9 @@ describe 'Dashboard Todos', feature: true do shared_examples 'deleting the todo' do before do - first('.js-done-todo').click + within first('.todo') do + click_link 'Done' + end end it 'is marked as done-reversible in the list' do @@ -62,9 +64,11 @@ describe 'Dashboard Todos', feature: true do shared_examples 'deleting and restoring the todo' do before do - first('.js-done-todo').click - wait_for_ajax - first('.js-undo-todo').click + within first('.todo') do + click_link 'Done' + wait_for_ajax + click_link 'Undo' + end end it 'is marked back as pending in the list' do @@ -97,6 +101,35 @@ describe 'Dashboard Todos', feature: true do end end + context 'User has done todos', js: true do + before do + create(:todo, :mentioned, :done, user: user, project: project, target: issue, author: author) + login_as(user) + visit dashboard_todos_path(state: :done) + end + + it 'has the done todo present' do + expect(page).to have_selector('.todos-list .todo.todo-done', count: 1) + end + + describe 'restoring the todo' do + before do + within first('.todo') do + click_link 'Add todo' + end + end + + it 'is removed from the list' do + expect(page).not_to have_selector('.todos-list .todo.todo-done') + end + + it 'updates todo count' do + expect(page).to have_content 'To do 1' + expect(page).to have_content 'Done 0' + end + end + end + context 'User has Todos with labels spanning multiple projects' do before do label1 = create(:label, project: project) @@ -143,7 +176,7 @@ describe 'Dashboard Todos', feature: true do describe 'mark all as done', js: true do before do visit dashboard_todos_path - click_link('Mark all as done') + click_link 'Mark all as done' end it 'shows "All done" message!' do diff --git a/spec/finders/members_finder_spec.rb b/spec/finders/members_finder_spec.rb new file mode 100644 index 00000000000..cf691cf684b --- /dev/null +++ b/spec/finders/members_finder_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe MembersFinder, '#execute' do + let(:group) { create(:group) } + let(:nested_group) { create(:group, :access_requestable, parent: group) } + let(:project) { create(:project, namespace: nested_group) } + let(:user1) { create(:user) } + let(:user2) { create(:user) } + let(:user3) { create(:user) } + let(:user4) { create(:user) } + + it 'returns members for project and parent groups' do + nested_group.request_access(user1) + member1 = group.add_master(user2) + member2 = nested_group.add_master(user3) + member3 = project.add_master(user4) + + result = described_class.new(project, user2).execute + + expect(result.to_a).to eq([member3, member2, member1]) + end +end diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb index 88d853935c7..f0554cc068d 100644 --- a/spec/helpers/issues_helper_spec.rb +++ b/spec/helpers/issues_helper_spec.rb @@ -131,4 +131,36 @@ describe IssuesHelper do expect(options).to have_selector('option', text: milestone2.title) end end + + describe "#link_to_discussions_to_resolve" do + describe "passing only a merge request" do + let(:merge_request) { create(:merge_request) } + + it "links just the merge request" do + expected_path = namespace_project_merge_request_path(merge_request.project.namespace, merge_request.project, merge_request) + + expect(link_to_discussions_to_resolve(merge_request, nil)).to include(expected_path) + end + + it "containst the reference to the merge request" do + expect(link_to_discussions_to_resolve(merge_request, nil)).to include(merge_request.to_reference) + end + end + + describe "when passing a discussion" do + let(:diff_note) { create(:diff_note_on_merge_request) } + let(:merge_request) { diff_note.noteable } + let(:discussion) { Discussion.new([diff_note]) } + + it "links to the merge request with first note if a single discussion was passed" do + expected_path = Gitlab::UrlBuilder.build(diff_note) + + expect(link_to_discussions_to_resolve(merge_request, discussion)).to include(expected_path) + end + + it "contains both the reference to the merge request and a mention of the discussion" do + expect(link_to_discussions_to_resolve(merge_request, discussion)).to include("#{merge_request.to_reference} (discussion #{diff_note.id})") + end + end + end end diff --git a/spec/initializers/6_validations_spec.rb b/spec/initializers/6_validations_spec.rb index cf182e6d221..374517fec37 100644 --- a/spec/initializers/6_validations_spec.rb +++ b/spec/initializers/6_validations_spec.rb @@ -12,63 +12,77 @@ describe '6_validations', lib: true do FileUtils.rm_rf('tmp/tests/paths') end - context 'with correct settings' do - before do - mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/d' }) - end + describe 'validate_storages_config' do + context 'with correct settings' do + before do + mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/d' }) + end - it 'passes through' do - expect { validate_storages }.not_to raise_error + it 'passes through' do + expect { validate_storages_config }.not_to raise_error + end end - end - context 'with invalid storage names' do - before do - mock_storages('name with spaces' => { 'path' => 'tmp/tests/paths/a/b/c' }) - end + context 'with invalid storage names' do + before do + mock_storages('name with spaces' => { 'path' => 'tmp/tests/paths/a/b/c' }) + end - it 'throws an error' do - expect { validate_storages }.to raise_error('"name with spaces" is not a valid storage name. Please fix this in your gitlab.yml before starting GitLab.') + it 'throws an error' do + expect { validate_storages_config }.to raise_error('"name with spaces" is not a valid storage name. Please fix this in your gitlab.yml before starting GitLab.') + end end - end - context 'with nested storage paths' do - before do - mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/c/d' }) - end + context 'with incomplete settings' do + before do + mock_storages('foo' => {}) + end - it 'throws an error' do - expect { validate_storages }.to raise_error('bar is a nested path of foo. Nested paths are not supported for repository storages. Please fix this in your gitlab.yml before starting GitLab.') + it 'throws an error suggesting the user to update its settings' do + expect { validate_storages_config }.to raise_error('foo is not a valid storage, because it has no `path` key. Refer to gitlab.yml.example for an updated example. Please fix this in your gitlab.yml before starting GitLab.') + end end - end - context 'with similar but un-nested storage paths' do - before do - mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/c2' }) - end + context 'with deprecated settings structure' do + before do + mock_storages('foo' => 'tmp/tests/paths/a/b/c') + end - it 'passes through' do - expect { validate_storages }.not_to raise_error + it 'throws an error suggesting the user to update its settings' do + expect { validate_storages_config }.to raise_error("foo is not a valid storage, because it has no `path` key. It may be configured as:\n\nfoo:\n path: tmp/tests/paths/a/b/c\n\nFor source installations, update your config/gitlab.yml Refer to gitlab.yml.example for an updated example.\n\nIf you're using the Gitlab Development Kit, you can update your configuration running `gdk reconfigure`.\n") + end end end - context 'with incomplete settings' do - before do - mock_storages('foo' => {}) - end + describe 'validate_storages_paths' do + context 'with correct settings' do + before do + mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/d' }) + end - it 'throws an error suggesting the user to update its settings' do - expect { validate_storages }.to raise_error('foo is not a valid storage, because it has no `path` key. Refer to gitlab.yml.example for an updated example. Please fix this in your gitlab.yml before starting GitLab.') + it 'passes through' do + expect { validate_storages_paths }.not_to raise_error + end end - end - context 'with deprecated settings structure' do - before do - mock_storages('foo' => 'tmp/tests/paths/a/b/c') + context 'with nested storage paths' do + before do + mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/c/d' }) + end + + it 'throws an error' do + expect { validate_storages_paths }.to raise_error('bar is a nested path of foo. Nested paths are not supported for repository storages. Please fix this in your gitlab.yml before starting GitLab.') + end end - it 'throws an error suggesting the user to update its settings' do - expect { validate_storages }.to raise_error("foo is not a valid storage, because it has no `path` key. It may be configured as:\n\nfoo:\n path: tmp/tests/paths/a/b/c\n\nRefer to gitlab.yml.example for an updated example. Please fix this in your gitlab.yml before starting GitLab.") + context 'with similar but un-nested storage paths' do + before do + mock_storages('foo' => { 'path' => 'tmp/tests/paths/a/b/c' }, 'bar' => { 'path' => 'tmp/tests/paths/a/b/c2' }) + end + + it 'passes through' do + expect { validate_storages_paths }.not_to raise_error + end end end diff --git a/spec/javascripts/awards_handler_spec.js b/spec/javascripts/awards_handler_spec.js index f4b1d777203..0a6e042b700 100644 --- a/spec/javascripts/awards_handler_spec.js +++ b/spec/javascripts/awards_handler_spec.js @@ -1,8 +1,7 @@ /* eslint-disable space-before-function-paren, no-var, one-var, one-var-declaration-per-line, no-unused-expressions, comma-dangle, new-parens, no-unused-vars, quotes, jasmine/no-spec-dupes, prefer-template, max-len */ -require('es6-promise').polyfill(); - -const AwardsHandler = require('~/awards_handler'); +import Cookies from 'js-cookie'; +import AwardsHandler from '~/awards_handler'; (function() { var awardsHandler, lazyAssert, urlRoot, openAndWaitForEmojiMenu; @@ -46,9 +45,6 @@ const AwardsHandler = require('~/awards_handler'); isEmojiMenuBuilt = true; resolve(); }); - - // Fail after 1 second - setTimeout(reject, 1000); } }); }; @@ -210,8 +206,8 @@ const AwardsHandler = require('~/awards_handler'); expect($('[data-name=alien]').is(':visible')).toBe(true); }) .then(done) - .catch(() => { - done.fail('Failed to open and build emoji menu'); + .catch((err) => { + done.fail(`Failed to open and build emoji menu: ${err.message}`); }); }); }); @@ -234,8 +230,8 @@ const AwardsHandler = require('~/awards_handler'); it('should add selected emoji to awards block', function(done) { return openEmojiMenuAndAddEmoji() .then(done) - .catch(() => { - done.fail('Failed to open and build emoji menu'); + .catch((err) => { + done.fail(`Failed to open and build emoji menu: ${err.message}`); }); }); it('should remove already selected emoji', function(done) { @@ -249,7 +245,46 @@ const AwardsHandler = require('~/awards_handler'); }) .then(done) .catch((err) => { - done.fail('Failed to open and build emoji menu'); + done.fail(`Failed to open and build emoji menu: ${err.message}`); + }); + }); + }); + + describe('frequently used emojis', function() { + beforeEach(() => { + // Clear it out + Cookies.set('frequently_used_emojis', ''); + }); + + it('shouldn\'t have any "Frequently used" heading if no frequently used emojis', function(done) { + return openAndWaitForEmojiMenu() + .then(() => { + const emojiMenu = document.querySelector('.emoji-menu'); + Array.prototype.forEach.call(emojiMenu.querySelectorAll('.emoji-menu-title'), (title) => { + expect(title.textContent.trim().toLowerCase()).not.toBe('frequently used'); + }); + }) + .then(done) + .catch((err) => { + done.fail(`Failed to open and build emoji menu: ${err.message}`); + }); + }); + + it('should have any frequently used section when there are frequently used emojis', function(done) { + awardsHandler.addEmojiToFrequentlyUsedList('8ball'); + + return openAndWaitForEmojiMenu() + .then(() => { + const emojiMenu = document.querySelector('.emoji-menu'); + const hasFrequentlyUsedHeading = Array.prototype.some.call(emojiMenu.querySelectorAll('.emoji-menu-title'), title => + title.textContent.trim().toLowerCase() === 'frequently used' + ); + + expect(hasFrequentlyUsedHeading).toBe(true); + }) + .then(done) + .catch((err) => { + done.fail(`Failed to open and build emoji menu: ${err.message}`); }); }); }); diff --git a/spec/javascripts/blob/create_branch_dropdown_spec.js b/spec/javascripts/blob/create_branch_dropdown_spec.js new file mode 100644 index 00000000000..c1179e572ae --- /dev/null +++ b/spec/javascripts/blob/create_branch_dropdown_spec.js @@ -0,0 +1,107 @@ +require('~/gl_dropdown'); +require('~/lib/utils/type_utility'); +require('~/blob/create_branch_dropdown'); +require('~/blob/target_branch_dropdown'); + +describe('CreateBranchDropdown', () => { + const fixtureTemplate = 'static/target_branch_dropdown.html.raw'; + // selectors + const createBranchSel = '.js-new-branch-btn'; + const backBtnSel = '.dropdown-menu-back'; + const cancelBtnSel = '.js-cancel-branch-btn'; + const branchNameSel = '#new_branch_name'; + const branchName = 'new_name'; + let dropdown; + + function createDropdown() { + const dropdownEl = document.querySelector('.js-project-branches-dropdown'); + const projectBranches = getJSONFixture('project_branches.json'); + dropdown = new gl.TargetBranchDropDown(dropdownEl); + dropdown.cachedRefs = projectBranches; + return dropdown; + } + + function createBranchBtn() { + return document.querySelector(createBranchSel); + } + + function backBtn() { + return document.querySelector(backBtnSel); + } + + function cancelBtn() { + return document.querySelector(cancelBtnSel); + } + + function branchNameEl() { + return document.querySelector(branchNameSel); + } + + function changeBranchName(text) { + branchNameEl().value = text; + branchNameEl().dispatchEvent(new Event('change')); + } + + preloadFixtures(fixtureTemplate); + + beforeEach(() => { + loadFixtures(fixtureTemplate); + createDropdown(); + }); + + it('disable submit when branch name is empty', () => { + expect(createBranchBtn()).toBeDisabled(); + }); + + it('enable submit when branch name is present', () => { + changeBranchName(branchName); + + expect(createBranchBtn()).not.toBeDisabled(); + }); + + it('resets the form when cancel btn is clicked and triggers dropdownback', () => { + const spyBackEvent = spyOnEvent(backBtnSel, 'click'); + changeBranchName(branchName); + + cancelBtn().click(); + + expect(branchNameEl()).toHaveValue(''); + expect(spyBackEvent).toHaveBeenTriggered(); + }); + + it('resets the form when back btn is clicked', () => { + changeBranchName(branchName); + + backBtn().click(); + + expect(branchNameEl()).toHaveValue(''); + }); + + describe('new branch creation', () => { + beforeEach(() => { + changeBranchName(branchName); + }); + it('sets the new branch name and updates the dropdown', () => { + spyOn(dropdown, 'setNewBranch'); + + createBranchBtn().click(); + + expect(dropdown.setNewBranch).toHaveBeenCalledWith(branchName); + }); + + it('resets the form', () => { + createBranchBtn().click(); + + expect(branchNameEl()).toHaveValue(''); + }); + + it('is triggered with enter keypress', () => { + spyOn(dropdown, 'setNewBranch'); + const enterEvent = new Event('keydown'); + enterEvent.which = 13; + branchNameEl().dispatchEvent(enterEvent); + + expect(dropdown.setNewBranch).toHaveBeenCalledWith(branchName); + }); + }); +}); diff --git a/spec/javascripts/blob/target_branch_dropdown_spec.js b/spec/javascripts/blob/target_branch_dropdown_spec.js new file mode 100644 index 00000000000..4fb79663c51 --- /dev/null +++ b/spec/javascripts/blob/target_branch_dropdown_spec.js @@ -0,0 +1,119 @@ +require('~/gl_dropdown'); +require('~/lib/utils/type_utility'); +require('~/blob/create_branch_dropdown'); +require('~/blob/target_branch_dropdown'); + +describe('TargetBranchDropdown', () => { + const fixtureTemplate = 'static/target_branch_dropdown.html.raw'; + let dropdown; + + function createDropdown() { + const projectBranches = getJSONFixture('project_branches.json'); + const dropdownEl = document.querySelector('.js-project-branches-dropdown'); + dropdown = new gl.TargetBranchDropDown(dropdownEl); + dropdown.cachedRefs = projectBranches; + dropdown.refreshData(); + return dropdown; + } + + function submitBtn() { + return document.querySelector('button[type="submit"]'); + } + + function searchField() { + return document.querySelector('.dropdown-page-one .dropdown-input-field'); + } + + function element() { + return document.querySelectorAll('div.dropdown-content li a'); + } + + function elementAtIndex(index) { + return element()[index]; + } + + function clickElementAtIndex(index) { + elementAtIndex(index).click(); + } + + preloadFixtures(fixtureTemplate); + + beforeEach(() => { + loadFixtures(fixtureTemplate); + createDropdown(); + }); + + it('disable submit when branch is not selected', () => { + document.querySelector('input[name="target_branch"]').value = null; + clickElementAtIndex(1); + + expect(submitBtn().getAttribute('disabled')).toEqual(''); + }); + + it('enable submit when a branch is selected', () => { + clickElementAtIndex(1); + + expect(submitBtn().getAttribute('disabled')).toBe(null); + }); + + it('triggers change.branch event on a branch click', () => { + spyOnEvent(dropdown.$dropdown, 'change.branch'); + clickElementAtIndex(0); + + expect('change.branch').toHaveBeenTriggeredOn(dropdown.$dropdown); + }); + + describe('#dropdownData', () => { + it('cache the refs', () => { + const refs = dropdown.cachedRefs; + dropdown.cachedRefs = null; + + dropdown.dropdownData(refs); + + expect(dropdown.cachedRefs).toEqual(refs); + }); + + it('returns the Branches with the newBranch and defaultBranch', () => { + const refs = dropdown.cachedRefs; + dropdown.branchInput.value = 'master'; + dropdown.newBranch = { id: 'new_branch', text: 'new_branch', title: 'new_branch' }; + + const branches = dropdown.dropdownData(refs).Branches; + + expect(branches.length).toEqual(4); + expect(branches[0]).toEqual(dropdown.newBranch); + expect(branches[1]).toEqual({ id: 'master', text: 'master', title: 'master' }); + expect(branches[2]).toEqual({ id: 'development', text: 'development', title: 'development' }); + expect(branches[3]).toEqual({ id: 'staging', text: 'staging', title: 'staging' }); + }); + }); + + describe('#setNewBranch', () => { + it('adds the new branch and select it', () => { + const branchName = 'new_branch'; + + dropdown.setNewBranch(branchName); + + expect(elementAtIndex(0)).toHaveClass('is-active'); + expect(elementAtIndex(0)).toContainHtml(branchName); + }); + + it("doesn't add a new branch if already exists in the list", () => { + const branchName = elementAtIndex(0).text; + const initialLength = element().length; + + dropdown.setNewBranch(branchName); + + expect(element().length).toEqual(initialLength); + }); + + it('clears the search filter', () => { + const branchName = elementAtIndex(0).text; + searchField().value = 'searching'; + + dropdown.setNewBranch(branchName); + + expect(searchField().value).toEqual(''); + }); + }); +}); diff --git a/spec/javascripts/boards/board_new_issue_spec.js b/spec/javascripts/boards/board_new_issue_spec.js index 22c9f12951b..4999933c0c1 100644 --- a/spec/javascripts/boards/board_new_issue_spec.js +++ b/spec/javascripts/boards/board_new_issue_spec.js @@ -8,7 +8,6 @@ import boardNewIssue from '~/boards/components/board_new_issue'; require('~/boards/models/list'); require('./mock_data'); -require('es6-promise').polyfill(); describe('Issue boards new issue form', () => { let vm; diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js index 49a2ca4a78f..1d1069600fc 100644 --- a/spec/javascripts/boards/boards_store_spec.js +++ b/spec/javascripts/boards/boards_store_spec.js @@ -15,7 +15,6 @@ require('~/boards/models/user'); require('~/boards/services/board_service'); require('~/boards/stores/boards_store'); require('./mock_data'); -require('es6-promise').polyfill(); describe('Store', () => { beforeEach(() => { diff --git a/spec/javascripts/extensions/jquery_spec.js b/spec/javascripts/bootstrap_jquery_spec.js index 096d3272eac..48994b7c523 100644 --- a/spec/javascripts/extensions/jquery_spec.js +++ b/spec/javascripts/bootstrap_jquery_spec.js @@ -1,9 +1,9 @@ /* eslint-disable space-before-function-paren, no-var */ -require('~/extensions/jquery'); +import '~/commons/bootstrap'; (function() { - describe('jQuery extensions', function() { + describe('Bootstrap jQuery extensions', function() { describe('disable', function() { beforeEach(function() { return setFixtures('<input type="text" />'); diff --git a/spec/javascripts/diff_comments_store_spec.js b/spec/javascripts/diff_comments_store_spec.js index f956394ef53..84cf98c930a 100644 --- a/spec/javascripts/diff_comments_store_spec.js +++ b/spec/javascripts/diff_comments_store_spec.js @@ -7,7 +7,16 @@ require('~/diff_notes/stores/comments'); (() => { function createDiscussion(noteId = 1, resolved = true) { - CommentsStore.create('a', noteId, true, resolved, 'test'); + CommentsStore.create({ + discussionId: 'a', + noteId, + canResolve: true, + resolved, + resolvedBy: 'test', + authorName: 'test', + authorAvatar: 'test', + noteTruncated: 'test...', + }); } beforeEach(() => { diff --git a/spec/javascripts/environments/environment_actions_spec.js b/spec/javascripts/environments/environment_actions_spec.js index d50d45d295e..85b73f1d4e2 100644 --- a/spec/javascripts/environments/environment_actions_spec.js +++ b/spec/javascripts/environments/environment_actions_spec.js @@ -1,14 +1,16 @@ -const ActionsComponent = require('~/environments/components/environment_actions'); +import Vue from 'vue'; +import actionsComp from '~/environments/components/environment_actions'; describe('Actions Component', () => { - preloadFixtures('static/environments/element.html.raw'); + let ActionsComponent; + let actionsMock; + let spy; + let component; beforeEach(() => { - loadFixtures('static/environments/element.html.raw'); - }); + ActionsComponent = Vue.extend(actionsComp); - it('should render a dropdown with the provided actions', () => { - const actionsMock = [ + actionsMock = [ { name: 'bar', play_path: 'https://gitlab.com/play', @@ -19,18 +21,27 @@ describe('Actions Component', () => { }, ]; - const component = new ActionsComponent({ - el: document.querySelector('.test-dom-element'), + spy = jasmine.createSpy('spy').and.returnValue(Promise.resolve()); + component = new ActionsComponent({ propsData: { actions: actionsMock, + service: { + postAction: spy, + }, }, - }); + }).$mount(); + }); + it('should render a dropdown with the provided actions', () => { expect( component.$el.querySelectorAll('.dropdown-menu li').length, ).toEqual(actionsMock.length); - expect( - component.$el.querySelector('.dropdown-menu li a').getAttribute('href'), - ).toEqual(actionsMock[0].play_path); + }); + + it('should call the service when an action is clicked', () => { + component.$el.querySelector('.dropdown').click(); + component.$el.querySelector('.js-manual-action-link').click(); + + expect(spy).toHaveBeenCalledWith(actionsMock[0].play_path); }); }); diff --git a/spec/javascripts/environments/environment_external_url_spec.js b/spec/javascripts/environments/environment_external_url_spec.js index 393dbb5aae0..9af218a27ff 100644 --- a/spec/javascripts/environments/environment_external_url_spec.js +++ b/spec/javascripts/environments/environment_external_url_spec.js @@ -1,19 +1,20 @@ -const ExternalUrlComponent = require('~/environments/components/environment_external_url'); +import Vue from 'vue'; +import externalUrlComp from '~/environments/components/environment_external_url'; describe('External URL Component', () => { - preloadFixtures('static/environments/element.html.raw'); + let ExternalUrlComponent; + beforeEach(() => { - loadFixtures('static/environments/element.html.raw'); + ExternalUrlComponent = Vue.extend(externalUrlComp); }); it('should link to the provided externalUrl prop', () => { const externalURL = 'https://gitlab.com'; const component = new ExternalUrlComponent({ - el: document.querySelector('.test-dom-element'), propsData: { externalUrl: externalURL, }, - }); + }).$mount(); expect(component.$el.getAttribute('href')).toEqual(externalURL); expect(component.$el.querySelector('fa-external-link')).toBeDefined(); diff --git a/spec/javascripts/environments/environment_item_spec.js b/spec/javascripts/environments/environment_item_spec.js index 7fea80ed799..4d42de4d549 100644 --- a/spec/javascripts/environments/environment_item_spec.js +++ b/spec/javascripts/environments/environment_item_spec.js @@ -1,10 +1,12 @@ -window.timeago = require('timeago.js'); -const EnvironmentItem = require('~/environments/components/environment_item'); +import 'timeago.js'; +import Vue from 'vue'; +import environmentItemComp from '~/environments/components/environment_item'; describe('Environment item', () => { - preloadFixtures('static/environments/table.html.raw'); + let EnvironmentItem; + beforeEach(() => { - loadFixtures('static/environments/table.html.raw'); + EnvironmentItem = Vue.extend(environmentItemComp); }); describe('When item is folder', () => { @@ -21,13 +23,13 @@ describe('Environment item', () => { }; component = new EnvironmentItem({ - el: document.querySelector('tr#environment-row'), propsData: { model: mockItem, canCreateDeployment: false, canReadEnvironment: true, + service: {}, }, - }); + }).$mount(); }); it('Should render folder icon and name', () => { @@ -109,13 +111,13 @@ describe('Environment item', () => { }; component = new EnvironmentItem({ - el: document.querySelector('tr#environment-row'), propsData: { model: environment, canCreateDeployment: true, canReadEnvironment: true, + service: {}, }, - }); + }).$mount(); }); it('should render environment name', () => { diff --git a/spec/javascripts/environments/environment_rollback_spec.js b/spec/javascripts/environments/environment_rollback_spec.js index 4a596baad09..7cb39d9df03 100644 --- a/spec/javascripts/environments/environment_rollback_spec.js +++ b/spec/javascripts/environments/environment_rollback_spec.js @@ -1,47 +1,59 @@ -const RollbackComponent = require('~/environments/components/environment_rollback'); +import Vue from 'vue'; +import rollbackComp from '~/environments/components/environment_rollback'; describe('Rollback Component', () => { - preloadFixtures('static/environments/element.html.raw'); - const retryURL = 'https://gitlab.com/retry'; + let RollbackComponent; + let spy; beforeEach(() => { - loadFixtures('static/environments/element.html.raw'); + RollbackComponent = Vue.extend(rollbackComp); + spy = jasmine.createSpy('spy').and.returnValue(Promise.resolve()); }); - it('Should link to the provided retryUrl', () => { + it('Should render Re-deploy label when isLastDeployment is true', () => { const component = new RollbackComponent({ el: document.querySelector('.test-dom-element'), propsData: { retryUrl: retryURL, isLastDeployment: true, + service: { + postAction: spy, + }, }, - }); + }).$mount(); - expect(component.$el.getAttribute('href')).toEqual(retryURL); + expect(component.$el.querySelector('span').textContent).toContain('Re-deploy'); }); - it('Should render Re-deploy label when isLastDeployment is true', () => { + it('Should render Rollback label when isLastDeployment is false', () => { const component = new RollbackComponent({ el: document.querySelector('.test-dom-element'), propsData: { retryUrl: retryURL, - isLastDeployment: true, + isLastDeployment: false, + service: { + postAction: spy, + }, }, - }); + }).$mount(); - expect(component.$el.querySelector('span').textContent).toContain('Re-deploy'); + expect(component.$el.querySelector('span').textContent).toContain('Rollback'); }); - it('Should render Rollback label when isLastDeployment is false', () => { + it('should call the service when the button is clicked', () => { const component = new RollbackComponent({ - el: document.querySelector('.test-dom-element'), propsData: { retryUrl: retryURL, isLastDeployment: false, + service: { + postAction: spy, + }, }, - }); + }).$mount(); - expect(component.$el.querySelector('span').textContent).toContain('Rollback'); + component.$el.click(); + + expect(spy).toHaveBeenCalledWith(retryURL); }); }); diff --git a/spec/javascripts/environments/environment_spec.js b/spec/javascripts/environments/environment_spec.js index edd0cad32d0..9601575577e 100644 --- a/spec/javascripts/environments/environment_spec.js +++ b/spec/javascripts/environments/environment_spec.js @@ -1,7 +1,7 @@ -const Vue = require('vue'); -require('~/flash'); -const EnvironmentsComponent = require('~/environments/components/environment'); -const { environment } = require('./mock_data'); +import Vue from 'vue'; +import '~/flash'; +import EnvironmentsComponent from '~/environments/components/environment'; +import { environment } from './mock_data'; describe('Environment', () => { preloadFixtures('static/environments/environments.html.raw'); diff --git a/spec/javascripts/environments/environment_stop_spec.js b/spec/javascripts/environments/environment_stop_spec.js index 5ca65b1debc..8f79b88f3df 100644 --- a/spec/javascripts/environments/environment_stop_spec.js +++ b/spec/javascripts/environments/environment_stop_spec.js @@ -1,28 +1,34 @@ -const StopComponent = require('~/environments/components/environment_stop'); +import Vue from 'vue'; +import stopComp from '~/environments/components/environment_stop'; describe('Stop Component', () => { - preloadFixtures('static/environments/element.html.raw'); - - let stopURL; + let StopComponent; let component; + let spy; + const stopURL = '/stop'; beforeEach(() => { - loadFixtures('static/environments/element.html.raw'); + StopComponent = Vue.extend(stopComp); + spy = jasmine.createSpy('spy').and.returnValue(Promise.resolve()); + spyOn(window, 'confirm').and.returnValue(true); - stopURL = '/stop'; component = new StopComponent({ - el: document.querySelector('.test-dom-element'), propsData: { stopUrl: stopURL, + service: { + postAction: spy, + }, }, - }); + }).$mount(); }); - it('should link to the provided URL', () => { - expect(component.$el.getAttribute('href')).toEqual(stopURL); + it('should render a button to stop the environment', () => { + expect(component.$el.tagName).toEqual('BUTTON'); + expect(component.$el.getAttribute('title')).toEqual('Stop Environment'); }); - it('should have a data-confirm attribute', () => { - expect(component.$el.getAttribute('data-confirm')).toEqual('Are you sure you want to stop this environment?'); + it('should call the service when an action is clicked', () => { + component.$el.click(); + expect(spy).toHaveBeenCalled(); }); }); diff --git a/spec/javascripts/environments/environment_table_spec.js b/spec/javascripts/environments/environment_table_spec.js index be4330b5012..3df967848a7 100644 --- a/spec/javascripts/environments/environment_table_spec.js +++ b/spec/javascripts/environments/environment_table_spec.js @@ -1,4 +1,5 @@ -const EnvironmentTable = require('~/environments/components/environments_table'); +import Vue from 'vue'; +import environmentTableComp from '~/environments/components/environments_table'; describe('Environment item', () => { preloadFixtures('static/environments/element.html.raw'); @@ -16,14 +17,17 @@ describe('Environment item', () => { }, }; + const EnvironmentTable = Vue.extend(environmentTableComp); + const component = new EnvironmentTable({ el: document.querySelector('.test-dom-element'), propsData: { environments: [{ mockItem }], canCreateDeployment: false, canReadEnvironment: true, + service: {}, }, - }); + }).$mount(); expect(component.$el.tagName).toEqual('TABLE'); }); diff --git a/spec/javascripts/environments/environment_terminal_button_spec.js b/spec/javascripts/environments/environment_terminal_button_spec.js new file mode 100644 index 00000000000..b07aa4e1745 --- /dev/null +++ b/spec/javascripts/environments/environment_terminal_button_spec.js @@ -0,0 +1,24 @@ +import Vue from 'vue'; +import terminalComp from '~/environments/components/environment_terminal_button'; + +describe('Stop Component', () => { + let TerminalComponent; + let component; + const terminalPath = '/path'; + + beforeEach(() => { + TerminalComponent = Vue.extend(terminalComp); + + component = new TerminalComponent({ + propsData: { + terminalPath, + }, + }).$mount(); + }); + + it('should render a link to open a web terminal with the provided path', () => { + expect(component.$el.tagName).toEqual('A'); + expect(component.$el.getAttribute('title')).toEqual('Open web terminal'); + expect(component.$el.getAttribute('href')).toEqual(terminalPath); + }); +}); diff --git a/spec/javascripts/environments/environments_store_spec.js b/spec/javascripts/environments/environments_store_spec.js index 77e182b3830..115d84b50f5 100644 --- a/spec/javascripts/environments/environments_store_spec.js +++ b/spec/javascripts/environments/environments_store_spec.js @@ -1,5 +1,5 @@ -const Store = require('~/environments/stores/environments_store'); -const { environmentsList, serverData } = require('./mock_data'); +import Store from '~/environments/stores/environments_store'; +import { environmentsList, serverData } from './mock_data'; (() => { describe('Store', () => { diff --git a/spec/javascripts/environments/folder/environments_folder_view_spec.js b/spec/javascripts/environments/folder/environments_folder_view_spec.js index d1335b5b304..43a217a67f5 100644 --- a/spec/javascripts/environments/folder/environments_folder_view_spec.js +++ b/spec/javascripts/environments/folder/environments_folder_view_spec.js @@ -1,7 +1,7 @@ -const Vue = require('vue'); -require('~/flash'); -const EnvironmentsFolderViewComponent = require('~/environments/folder/environments_folder_view'); -const { environmentsList } = require('../mock_data'); +import Vue from 'vue'; +import '~/flash'; +import EnvironmentsFolderViewComponent from '~/environments/folder/environments_folder_view'; +import { environmentsList } from '../mock_data'; describe('Environments Folder View', () => { preloadFixtures('static/environments/environments_folder_view.html.raw'); diff --git a/spec/javascripts/environments/mock_data.js b/spec/javascripts/environments/mock_data.js index 5c395c6b2d8..30861481cc5 100644 --- a/spec/javascripts/environments/mock_data.js +++ b/spec/javascripts/environments/mock_data.js @@ -1,4 +1,4 @@ -const environmentsList = [ +export const environmentsList = [ { name: 'DEV', size: 1, @@ -30,7 +30,7 @@ const environmentsList = [ }, ]; -const serverData = [ +export const serverData = [ { name: 'DEV', size: 1, @@ -67,7 +67,7 @@ const serverData = [ }, ]; -const environment = { +export const environment = { name: 'DEV', size: 1, latest: { @@ -84,9 +84,3 @@ const environment = { updated_at: '2017-01-31T10:53:46.894Z', }, }; - -module.exports = { - environmentsList, - environment, - serverData, -}; diff --git a/spec/javascripts/extensions/array_spec.js b/spec/javascripts/extensions/array_spec.js index 60f6b9b78e3..4b871fe967d 100644 --- a/spec/javascripts/extensions/array_spec.js +++ b/spec/javascripts/extensions/array_spec.js @@ -18,28 +18,5 @@ require('~/extensions/array'); return expect(arr.last()).toBe(5); }); }); - - describe('find', function () { - beforeEach(() => { - this.arr = [0, 1, 2, 3, 4, 5]; - }); - - it('returns the item that first passes the predicate function', () => { - expect(this.arr.find(item => item === 2)).toBe(2); - }); - - it('returns undefined if no items pass the predicate function', () => { - expect(this.arr.find(item => item === 6)).not.toBeDefined(); - }); - - it('error when called on undefined or null', () => { - expect(Array.prototype.find.bind(undefined, item => item === 1)).toThrow(); - expect(Array.prototype.find.bind(null, item => item === 1)).toThrow(); - }); - - it('error when predicate is not a function', () => { - expect(Array.prototype.find.bind(this.arr, 1)).toThrow(); - }); - }); }); }).call(window); diff --git a/spec/javascripts/extensions/element_spec.js b/spec/javascripts/extensions/element_spec.js deleted file mode 100644 index 2d8a128ed33..00000000000 --- a/spec/javascripts/extensions/element_spec.js +++ /dev/null @@ -1,38 +0,0 @@ -require('~/extensions/element'); - -(() => { - describe('Element extensions', function () { - beforeEach(() => { - this.element = document.createElement('ul'); - }); - - describe('matches', () => { - it('returns true if element matches the selector', () => { - expect(this.element.matches('ul')).toBeTruthy(); - }); - - it("returns false if element doesn't match the selector", () => { - expect(this.element.matches('.not-an-element')).toBeFalsy(); - }); - }); - - describe('closest', () => { - beforeEach(() => { - this.childElement = document.createElement('li'); - this.element.appendChild(this.childElement); - }); - - it('returns the closest parent that matches the selector', () => { - expect(this.childElement.closest('ul').toString()).toBe(this.element.toString()); - }); - - it('returns itself if it matches the selector', () => { - expect(this.childElement.closest('li').toString()).toBe(this.childElement.toString()); - }); - - it('returns undefined if nothing matches the selector', () => { - expect(this.childElement.closest('.no-an-element')).toBeFalsy(); - }); - }); - }); -})(); diff --git a/spec/javascripts/extensions/object_spec.js b/spec/javascripts/extensions/object_spec.js deleted file mode 100644 index 2467ed78459..00000000000 --- a/spec/javascripts/extensions/object_spec.js +++ /dev/null @@ -1,25 +0,0 @@ -require('~/extensions/object'); - -describe('Object extensions', () => { - describe('assign', () => { - it('merges source object into target object', () => { - const targetObj = {}; - const sourceObj = { - foo: 'bar', - }; - Object.assign(targetObj, sourceObj); - expect(targetObj.foo).toBe('bar'); - }); - - it('merges object with the same properties', () => { - const targetObj = { - foo: 'bar', - }; - const sourceObj = { - foo: 'baz', - }; - Object.assign(targetObj, sourceObj); - expect(targetObj.foo).toBe('baz'); - }); - }); -}); diff --git a/spec/javascripts/filtered_search/filtered_search_manager_spec.js b/spec/javascripts/filtered_search/filtered_search_manager_spec.js index 81c1d81d181..ae9c263d1d7 100644 --- a/spec/javascripts/filtered_search/filtered_search_manager_spec.js +++ b/spec/javascripts/filtered_search/filtered_search_manager_spec.js @@ -41,7 +41,6 @@ const FilteredSearchSpecHelper = require('../helpers/filtered_search_spec_helper </div> `); - spyOn(gl.FilteredSearchManager.prototype, 'cleanup').and.callFake(() => {}); spyOn(gl.FilteredSearchManager.prototype, 'loadSearchParamsFromURL').and.callFake(() => {}); spyOn(gl.FilteredSearchManager.prototype, 'tokenChange').and.callFake(() => {}); spyOn(gl.FilteredSearchDropdownManager.prototype, 'setDropdown').and.callFake(() => {}); @@ -54,6 +53,10 @@ const FilteredSearchSpecHelper = require('../helpers/filtered_search_spec_helper manager = new gl.FilteredSearchManager(); }); + afterEach(() => { + manager.cleanup(); + }); + describe('search', () => { const defaultParams = '?scope=all&utf8=✓&state=opened'; diff --git a/spec/javascripts/fixtures/project_branches.json b/spec/javascripts/fixtures/project_branches.json new file mode 100644 index 00000000000..a96a4c0c095 --- /dev/null +++ b/spec/javascripts/fixtures/project_branches.json @@ -0,0 +1,5 @@ +[ + "master", + "development", + "staging" +] diff --git a/spec/javascripts/fixtures/target_branch_dropdown.html.haml b/spec/javascripts/fixtures/target_branch_dropdown.html.haml new file mode 100644 index 00000000000..821fb7940a0 --- /dev/null +++ b/spec/javascripts/fixtures/target_branch_dropdown.html.haml @@ -0,0 +1,28 @@ +%form.js-edit-blob-form + %input{type: 'hidden', name: 'target_branch', value: 'master'} + %div + .dropdown + %button.dropdown-menu-toggle.js-project-branches-dropdown.js-target-branch{type: 'button', data: {toggle: 'dropdown', selected: 'master', field_name: 'target_branch', form_id: '.js-edit-blob-form'}} + .dropdown-menu.dropdown-menu-selectable.dropdown-menu-paging + .dropdown-page-one + .dropdown-title 'Select branch' + .dropdown-input + %input.dropdown-input-field{type: 'search', value: ''} + %i.fa.fa-search.dropdown-input-search + %i.fa.fa-times-dropdown-input-clear.js-dropdown-input-clear{role: 'button'} + .dropdown-content + .dropdown-footer + %ul.dropdown-footer-list + %li + %a.create-new-branch.dropdown-toggle-page{href: "#"} + Create new branch + .dropdown-page-two.dropdown-new-branch + %button.dropdown-title-button.dropdown-menu-back{type: 'button'} + .dropdown_title 'Create new branch' + .dropdown_content + %input#new_branch_name.default-dropdown-input{ type: "text", placeholder: "Name new branch" } + %button.btn.btn-primary.pull-left.js-new-branch-btn{ type: "button" } + Create + %button.btn.btn-default.pull-right.js-cancel-branch-btn{ type: "button" } + Cancel + %button{type: 'submit'} diff --git a/spec/javascripts/gl_emoji_spec.js b/spec/javascripts/gl_emoji_spec.js index e94e220b19f..9b44b25980c 100644 --- a/spec/javascripts/gl_emoji_spec.js +++ b/spec/javascripts/gl_emoji_spec.js @@ -1,16 +1,12 @@ - -require('~/extensions/string'); -require('~/extensions/array'); - -const glEmoji = require('~/behaviors/gl_emoji'); - -const glEmojiTag = glEmoji.glEmojiTag; -const isEmojiUnicodeSupported = glEmoji.isEmojiUnicodeSupported; -const isFlagEmoji = glEmoji.isFlagEmoji; -const isKeycapEmoji = glEmoji.isKeycapEmoji; -const isSkinToneComboEmoji = glEmoji.isSkinToneComboEmoji; -const isHorceRacingSkinToneComboEmoji = glEmoji.isHorceRacingSkinToneComboEmoji; -const isPersonZwjEmoji = glEmoji.isPersonZwjEmoji; +import { glEmojiTag } from '~/behaviors/gl_emoji'; +import { + isEmojiUnicodeSupported, + isFlagEmoji, + isKeycapEmoji, + isSkinToneComboEmoji, + isHorceRacingSkinToneComboEmoji, + isPersonZwjEmoji, +} from '~/behaviors/gl_emoji/is_emoji_unicode_supported'; const emptySupportMap = { personZwj: false, diff --git a/spec/javascripts/issue_spec.js b/spec/javascripts/issue_spec.js index e7530f61385..8d25500b9fd 100644 --- a/spec/javascripts/issue_spec.js +++ b/spec/javascripts/issue_spec.js @@ -1,10 +1,9 @@ /* eslint-disable space-before-function-paren, no-var, one-var, one-var-declaration-per-line, no-use-before-define, comma-dangle, max-len */ -/* global Issue */ +import Issue from '~/issue'; require('~/lib/utils/text_utility'); -require('~/issue'); -(function() { +describe('Issue', function() { var INVALID_URL = 'http://goesnowhere.nothing/whereami'; var $boxClosed, $boxOpen, $btnClose, $btnReopen; @@ -59,28 +58,26 @@ require('~/issue'); expect($btnReopen).toHaveText('Reopen issue'); } - describe('Issue', function() { - describe('task lists', function() { - beforeEach(function() { - loadFixtures('issues/issue-with-task-list.html.raw'); - this.issue = new Issue(); - }); - - it('modifies the Markdown field', function() { - spyOn(jQuery, 'ajax').and.stub(); - $('input[type=checkbox]').attr('checked', true).trigger('change'); - expect($('.js-task-list-field').val()).toBe('- [x] Task List Item'); - }); + describe('task lists', function() { + beforeEach(function() { + loadFixtures('issues/issue-with-task-list.html.raw'); + this.issue = new Issue(); + }); - it('submits an ajax request on tasklist:changed', function() { - spyOn(jQuery, 'ajax').and.callFake(function(req) { - expect(req.type).toBe('PATCH'); - expect(req.url).toBe(gl.TEST_HOST + '/frontend-fixtures/issues-project/issues/1.json'); // eslint-disable-line prefer-template - expect(req.data.issue.description).not.toBe(null); - }); + it('modifies the Markdown field', function() { + spyOn(jQuery, 'ajax').and.stub(); + $('input[type=checkbox]').attr('checked', true).trigger('change'); + expect($('.js-task-list-field').val()).toBe('- [x] Task List Item'); + }); - $('.js-task-list-field').trigger('tasklist:changed'); + it('submits an ajax request on tasklist:changed', function() { + spyOn(jQuery, 'ajax').and.callFake(function(req) { + expect(req.type).toBe('PATCH'); + expect(req.url).toBe(gl.TEST_HOST + '/frontend-fixtures/issues-project/issues/1.json'); // eslint-disable-line prefer-template + expect(req.data.issue.description).not.toBe(null); }); + + $('.js-task-list-field').trigger('tasklist:changed'); }); }); @@ -165,4 +162,4 @@ require('~/issue'); expect($('.issue_counter')).toHaveText(1); }); }); -}).call(window); +}); diff --git a/spec/javascripts/line_highlighter_spec.js b/spec/javascripts/line_highlighter_spec.js index a0b2ebc221b..a1fd2d38968 100644 --- a/spec/javascripts/line_highlighter_spec.js +++ b/spec/javascripts/line_highlighter_spec.js @@ -7,16 +7,12 @@ require('~/line_highlighter'); describe('LineHighlighter', function() { var clickLine; preloadFixtures('static/line_highlighter.html.raw'); - clickLine = function(number, eventData) { - var e; - if (eventData == null) { - eventData = {}; - } + clickLine = function(number, eventData = {}) { if ($.isEmptyObject(eventData)) { - return $("#L" + number).mousedown().click(); + return $("#L" + number).click(); } else { - e = $.Event('mousedown', eventData); - return $("#L" + number).trigger(e).click(); + const e = $.Event('click', eventData); + return $("#L" + number).trigger(e); } }; beforeEach(function() { @@ -63,12 +59,6 @@ require('~/line_highlighter'); }); }); describe('#clickHandler', function() { - it('discards the mousedown event', function() { - var spy; - spy = spyOnEvent('a[data-line-number]', 'mousedown'); - clickLine(13); - return expect(spy).toHaveBeenPrevented(); - }); it('handles clicking on a child icon element', function() { var spy; spy = spyOn(this["class"], 'setHash').and.callThrough(); diff --git a/spec/javascripts/mini_pipeline_graph_dropdown_spec.js b/spec/javascripts/mini_pipeline_graph_dropdown_spec.js index 7cdade01e00..e504d41d4d4 100644 --- a/spec/javascripts/mini_pipeline_graph_dropdown_spec.js +++ b/spec/javascripts/mini_pipeline_graph_dropdown_spec.js @@ -1,7 +1,7 @@ /* eslint-disable no-new */ -require('~/flash'); -require('~/mini_pipeline_graph_dropdown'); +import MiniPipelineGraph from '~/mini_pipeline_graph_dropdown'; +import '~/flash'; (() => { describe('Mini Pipeline Graph Dropdown', () => { @@ -13,7 +13,7 @@ require('~/mini_pipeline_graph_dropdown'); describe('When is initialized', () => { it('should initialize without errors when no options are given', () => { - const miniPipelineGraph = new window.gl.MiniPipelineGraph(); + const miniPipelineGraph = new MiniPipelineGraph(); expect(miniPipelineGraph.dropdownListSelector).toEqual('.js-builds-dropdown-container'); }); @@ -21,7 +21,7 @@ require('~/mini_pipeline_graph_dropdown'); it('should set the container as the given prop', () => { const container = '.foo'; - const miniPipelineGraph = new window.gl.MiniPipelineGraph({ container }); + const miniPipelineGraph = new MiniPipelineGraph({ container }); expect(miniPipelineGraph.container).toEqual(container); }); @@ -29,9 +29,9 @@ require('~/mini_pipeline_graph_dropdown'); describe('When dropdown is clicked', () => { it('should call getBuildsList', () => { - const getBuildsListSpy = spyOn(gl.MiniPipelineGraph.prototype, 'getBuildsList').and.callFake(function () {}); + const getBuildsListSpy = spyOn(MiniPipelineGraph.prototype, 'getBuildsList').and.callFake(function () {}); - new gl.MiniPipelineGraph({ container: '.js-builds-dropdown-tests' }).bindEvents(); + new MiniPipelineGraph({ container: '.js-builds-dropdown-tests' }).bindEvents(); document.querySelector('.js-builds-dropdown-button').click(); @@ -41,11 +41,32 @@ require('~/mini_pipeline_graph_dropdown'); it('should make a request to the endpoint provided in the html', () => { const ajaxSpy = spyOn($, 'ajax').and.callFake(function () {}); - new gl.MiniPipelineGraph({ container: '.js-builds-dropdown-tests' }).bindEvents(); + new MiniPipelineGraph({ container: '.js-builds-dropdown-tests' }).bindEvents(); document.querySelector('.js-builds-dropdown-button').click(); expect(ajaxSpy.calls.allArgs()[0][0].url).toEqual('foobar'); }); + + it('should not close when user uses cmd/ctrl + click', () => { + spyOn($, 'ajax').and.callFake(function (params) { + params.success({ + html: `<li> + <a class="mini-pipeline-graph-dropdown-item" href="#"> + <span class="ci-status-icon ci-status-icon-failed"></span> + <span class="ci-build-text">build</span> + </a> + <a class="ci-action-icon-wrapper js-ci-action-icon" href="#"></a> + </li>`, + }); + }); + new MiniPipelineGraph({ container: '.js-builds-dropdown-tests' }).bindEvents(); + + document.querySelector('.js-builds-dropdown-button').click(); + + document.querySelector('a.mini-pipeline-graph-dropdown-item').click(); + + expect($('.js-builds-dropdown-list').is(':visible')).toEqual(true); + }); }); }); })(); diff --git a/spec/javascripts/monitoring/prometheus_graph_spec.js b/spec/javascripts/monitoring/prometheus_graph_spec.js index 823b4bab7fc..a3c1c5e1b7c 100644 --- a/spec/javascripts/monitoring/prometheus_graph_spec.js +++ b/spec/javascripts/monitoring/prometheus_graph_spec.js @@ -1,11 +1,8 @@ import 'jquery'; -import es6Promise from 'es6-promise'; import '~/lib/utils/common_utils'; import PrometheusGraph from '~/monitoring/prometheus_graph'; import { prometheusMockData } from './prometheus_mock_data'; -es6Promise.polyfill(); - describe('PrometheusGraph', () => { const fixtureName = 'static/environments/metrics.html.raw'; const prometheusGraphContainer = '.prometheus-graph'; diff --git a/spec/javascripts/pager_spec.js b/spec/javascripts/pager_spec.js new file mode 100644 index 00000000000..d966226909b --- /dev/null +++ b/spec/javascripts/pager_spec.js @@ -0,0 +1,90 @@ +/* global fixture */ + +require('~/pager'); + +describe('pager', () => { + const Pager = window.Pager; + + it('is defined on window', () => { + expect(window.Pager).toBeDefined(); + }); + + describe('init', () => { + const originalHref = window.location.href; + + beforeEach(() => { + setFixtures('<div class="content_list"></div><div class="loading"></div>'); + spyOn($, 'ajax'); + }); + + afterEach(() => { + window.history.replaceState({}, null, originalHref); + }); + + it('should use data-href attribute from list element', () => { + const href = `${gl.TEST_HOST}/some_list.json`; + setFixtures(`<div class="content_list" data-href="${href}"></div>`); + Pager.init(); + expect(Pager.url).toBe(href); + }); + + it('should use current url if data-href attribute not provided', () => { + const href = `${gl.TEST_HOST}/some_list`; + spyOn(gl.utils, 'removeParams').and.returnValue(href); + Pager.init(); + expect(Pager.url).toBe(href); + }); + + it('should get initial offset from query parameter', () => { + window.history.replaceState({}, null, '?offset=100'); + Pager.init(); + expect(Pager.offset).toBe(100); + }); + + it('keeps extra query parameters from url', () => { + window.history.replaceState({}, null, '?filter=test&offset=100'); + const href = `${gl.TEST_HOST}/some_list?filter=test`; + spyOn(gl.utils, 'removeParams').and.returnValue(href); + Pager.init(); + expect(gl.utils.removeParams).toHaveBeenCalledWith(['limit', 'offset']); + expect(Pager.url).toEqual(href); + }); + }); + + describe('getOld', () => { + beforeEach(() => { + setFixtures('<div class="content_list" data-href="/some_list"></div><div class="loading"></div>'); + Pager.init(); + }); + + it('shows loader while loading next page', () => { + spyOn(Pager.loading, 'show'); + Pager.getOld(); + expect(Pager.loading.show).toHaveBeenCalled(); + }); + + it('hides loader on success', () => { + spyOn($, 'ajax').and.callFake(options => options.success({})); + spyOn(Pager.loading, 'hide'); + Pager.getOld(); + expect(Pager.loading.hide).toHaveBeenCalled(); + }); + + it('hides loader on error', () => { + spyOn($, 'ajax').and.callFake(options => options.error()); + spyOn(Pager.loading, 'hide'); + Pager.getOld(); + expect(Pager.loading.hide).toHaveBeenCalled(); + }); + + it('sends request to url with offset and limit params', () => { + spyOn($, 'ajax'); + Pager.offset = 100; + Pager.limit = 20; + Pager.getOld(); + const [{ data, url }] = $.ajax.calls.argsFor(0); + expect(data).toBe('limit=20&offset=100'); + expect(url).toBe('/some_list'); + }); + }); +}); diff --git a/spec/javascripts/polyfills/element_spec.js b/spec/javascripts/polyfills/element_spec.js new file mode 100644 index 00000000000..ecaaf1907ea --- /dev/null +++ b/spec/javascripts/polyfills/element_spec.js @@ -0,0 +1,36 @@ +import '~/commons/polyfills/element'; + +describe('Element polyfills', function () { + beforeEach(() => { + this.element = document.createElement('ul'); + }); + + describe('matches', () => { + it('returns true if element matches the selector', () => { + expect(this.element.matches('ul')).toBeTruthy(); + }); + + it("returns false if element doesn't match the selector", () => { + expect(this.element.matches('.not-an-element')).toBeFalsy(); + }); + }); + + describe('closest', () => { + beforeEach(() => { + this.childElement = document.createElement('li'); + this.element.appendChild(this.childElement); + }); + + it('returns the closest parent that matches the selector', () => { + expect(this.childElement.closest('ul').toString()).toBe(this.element.toString()); + }); + + it('returns itself if it matches the selector', () => { + expect(this.childElement.closest('li').toString()).toBe(this.childElement.toString()); + }); + + it('returns undefined if nothing matches the selector', () => { + expect(this.childElement.closest('.no-an-element')).toBeFalsy(); + }); + }); +}); diff --git a/spec/javascripts/project_title_spec.js b/spec/javascripts/project_title_spec.js index 69d9587771f..3a1d4e2440f 100644 --- a/spec/javascripts/project_title_spec.js +++ b/spec/javascripts/project_title_spec.js @@ -26,7 +26,7 @@ require('~/project'); var fakeAjaxResponse = function fakeAjaxResponse(req) { var d; expect(req.url).toBe('/api/v3/projects.json?simple=true'); - expect(req.data).toEqual({ search: '', order_by: 'last_activity_at', per_page: 20 }); + expect(req.data).toEqual({ search: '', order_by: 'last_activity_at', per_page: 20, membership: true }); d = $.Deferred(); d.resolve(this.projects_data); return d.promise(); diff --git a/spec/javascripts/right_sidebar_spec.js b/spec/javascripts/right_sidebar_spec.js index 4ac7e911740..285b7940174 100644 --- a/spec/javascripts/right_sidebar_spec.js +++ b/spec/javascripts/right_sidebar_spec.js @@ -1,8 +1,8 @@ /* eslint-disable space-before-function-paren, no-var, one-var, one-var-declaration-per-line, new-parens, no-return-assign, new-cap, vars-on-top, max-len */ /* global Sidebar */ -require('~/right_sidebar'); -require('~/extensions/jquery.js'); +import '~/commons/bootstrap'; +import '~/right_sidebar'; (function() { var $aside, $icon, $labelsIcon, $page, $toggle, assertSidebarState; diff --git a/spec/javascripts/shortcuts_issuable_spec.js b/spec/javascripts/shortcuts_issuable_spec.js index ffff643e371..9e19dabd0e3 100644 --- a/spec/javascripts/shortcuts_issuable_spec.js +++ b/spec/javascripts/shortcuts_issuable_spec.js @@ -31,13 +31,9 @@ require('~/shortcuts_issuable'); this.shortcut.replyWithSelectedText(); expect($(this.selector).val()).toBe(''); }); - it('triggers `input`', function() { - var focused = false; - $(this.selector).on('focus', function() { - focused = true; - }); + it('triggers `focus`', function() { this.shortcut.replyWithSelectedText(); - expect(focused).toBe(true); + expect(document.activeElement).toBe(document.querySelector(this.selector)); }); }); describe('with any selection', function() { diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb index cadfbadca10..e22f88b7a32 100644 --- a/spec/lib/gitlab/checks/change_access_spec.rb +++ b/spec/lib/gitlab/checks/change_access_spec.rb @@ -12,8 +12,16 @@ describe Gitlab::Checks::ChangeAccess, lib: true do ref: 'refs/heads/master' } end - - subject { described_class.new(changes, project: project, user_access: user_access).exec } + let(:protocol) { 'ssh' } + + subject do + described_class.new( + changes, + project: project, + user_access: user_access, + protocol: protocol + ).exec + end before { allow(user_access).to receive(:can_do_action?).with(:push_code).and_return(true) } diff --git a/spec/lib/gitlab/ci/status/pipeline/blocked_spec.rb b/spec/lib/gitlab/ci/status/pipeline/blocked_spec.rb new file mode 100644 index 00000000000..1a2b952d374 --- /dev/null +++ b/spec/lib/gitlab/ci/status/pipeline/blocked_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +describe Gitlab::Ci::Status::Pipeline::Blocked do + let(:pipeline) { double('pipeline') } + + subject do + described_class.new(pipeline) + end + + describe '#text' do + it 'overrides status text' do + expect(subject.text).to eq 'blocked' + end + end + + describe '#label' do + it 'overrides status label' do + expect(subject.label).to eq 'waiting for manual action' + end + end + + describe '.matches?' do + let(:user) { double('user') } + subject { described_class.matches?(pipeline, user) } + + context 'when pipeline is blocked' do + let(:pipeline) { create(:ci_pipeline, :blocked) } + + it 'is a correct match' do + expect(subject).to be true + end + end + + context 'when pipeline is not blocked' do + let(:pipeline) { create(:ci_pipeline, :success) } + + it 'does not match' do + expect(subject).to be false + end + end + end +end diff --git a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb index b10a447c27a..dd754b849b2 100644 --- a/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb +++ b/spec/lib/gitlab/ci/status/pipeline/factory_spec.rb @@ -11,7 +11,8 @@ describe Gitlab::Ci::Status::Pipeline::Factory do end context 'when pipeline has a core status' do - HasStatus::AVAILABLE_STATUSES.each do |simple_status| + (HasStatus::AVAILABLE_STATUSES - [HasStatus::BLOCKED_STATUS]) + .each do |simple_status| context "when core status is #{simple_status}" do let(:pipeline) { create(:ci_pipeline, status: simple_status) } @@ -23,7 +24,7 @@ describe Gitlab::Ci::Status::Pipeline::Factory do expect(factory.core_status).to be_a expected_status end - it 'does not matche extended statuses' do + it 'does not match extended statuses' do expect(factory.extended_statuses).to be_empty end @@ -39,6 +40,27 @@ describe Gitlab::Ci::Status::Pipeline::Factory do end end end + + context "when core status is manual" do + let(:pipeline) { create(:ci_pipeline, status: :manual) } + + it "matches manual core status" do + expect(factory.core_status) + .to be_a Gitlab::Ci::Status::Manual + end + + it 'matches a correct extended statuses' do + expect(factory.extended_statuses) + .to eq [Gitlab::Ci::Status::Pipeline::Blocked] + end + + it 'extends core status with common pipeline methods' do + expect(status).to have_details + expect(status).not_to have_action + expect(status.details_path) + .to include "pipelines/#{pipeline.id}" + end + end end context 'when pipeline has warnings' do diff --git a/spec/lib/gitlab/diff/parser_spec.rb b/spec/lib/gitlab/diff/parser_spec.rb index b983d73f8be..e76128ecd87 100644 --- a/spec/lib/gitlab/diff/parser_spec.rb +++ b/spec/lib/gitlab/diff/parser_spec.rb @@ -91,6 +91,54 @@ eos end end + describe '\ No newline at end of file' do + it "parses nonewline in one file correctly" do + first_nonewline_diff = <<~END + --- a/test + +++ b/test + @@ -1,2 +1,2 @@ + +ipsum + lorem + -ipsum + \\ No newline at end of file + END + lines = parser.parse(first_nonewline_diff.lines).to_a + + expect(lines[0].type).to eq('new') + expect(lines[0].text).to eq('+ipsum') + expect(lines[2].type).to eq('old') + expect(lines[3].type).to eq('old-nonewline') + expect(lines[1].old_pos).to eq(1) + expect(lines[1].new_pos).to eq(2) + end + + it "parses nonewline in two files correctly" do + both_nonewline_diff = <<~END + --- a/test + +++ b/test + @@ -1,2 +1,2 @@ + -lorem + -ipsum + \\ No newline at end of file + +ipsum + +lorem + \\ No newline at end of file + END + lines = parser.parse(both_nonewline_diff.lines).to_a + + expect(lines[0].type).to eq('old') + expect(lines[1].type).to eq('old') + expect(lines[2].type).to eq('old-nonewline') + expect(lines[5].type).to eq('new-nonewline') + expect(lines[3].text).to eq('+ipsum') + expect(lines[3].old_pos).to eq(3) + expect(lines[3].new_pos).to eq(1) + expect(lines[4].text).to eq('+lorem') + expect(lines[4].old_pos).to eq(3) + expect(lines[4].new_pos).to eq(2) + end + end + context 'when lines is empty' do it { expect(parser.parse([])).to eq([]) } it { expect(parser.parse(nil)).to eq([]) } diff --git a/spec/lib/gitlab/github_import/branch_formatter_spec.rb b/spec/lib/gitlab/github_import/branch_formatter_spec.rb index 36e7d739f7e..3a31f93efa5 100644 --- a/spec/lib/gitlab/github_import/branch_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/branch_formatter_spec.rb @@ -6,27 +6,27 @@ describe Gitlab::GithubImport::BranchFormatter, lib: true do let(:repo) { double } let(:raw) do { - ref: 'feature', + ref: 'branch-merged', repo: repo, sha: commit.id } end describe '#exists?' do - it 'returns true when both branch, and commit exists' do + it 'returns true when branch exists and commit is part of the branch' do branch = described_class.new(project, double(raw)) expect(branch.exists?).to eq true end - it 'returns false when branch does not exist' do - branch = described_class.new(project, double(raw.merge(ref: 'removed-branch'))) + it 'returns false when branch exists and commit is not part of the branch' do + branch = described_class.new(project, double(raw.merge(ref: 'feature'))) expect(branch.exists?).to eq false end - it 'returns false when commit does not exist' do - branch = described_class.new(project, double(raw.merge(sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b'))) + it 'returns false when branch does not exist' do + branch = described_class.new(project, double(raw.merge(ref: 'removed-branch'))) expect(branch.exists?).to eq false end diff --git a/spec/lib/gitlab/github_import/importer_spec.rb b/spec/lib/gitlab/github_import/importer_spec.rb index 33d83d6d2f1..8b867fbe322 100644 --- a/spec/lib/gitlab/github_import/importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer_spec.rb @@ -55,9 +55,6 @@ describe Gitlab::GithubImport::Importer, lib: true do allow_any_instance_of(Octokit::Client).to receive(:releases).and_return([release1, release2]) end - let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } - let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } - let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } let(:label1) do double( name: 'Bug', @@ -127,32 +124,6 @@ describe Gitlab::GithubImport::Importer, lib: true do ) end - let!(:user) { create(:user, email: octocat.email) } - let(:repository) { double(id: 1, fork: false) } - let(:source_sha) { create(:commit, project: project).id } - let(:source_branch) { double(ref: 'feature', repo: repository, sha: source_sha) } - let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } - let(:target_branch) { double(ref: 'master', repo: repository, sha: target_sha) } - let(:pull_request) do - double( - number: 1347, - milestone: nil, - state: 'open', - title: 'New feature', - body: 'Please pull these awesome changes', - head: source_branch, - base: target_branch, - assignee: nil, - user: octocat, - created_at: created_at, - updated_at: updated_at, - closed_at: nil, - merged_at: nil, - url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", - labels: [double(name: 'Label #2')] - ) - end - let(:release1) do double( tag_name: 'v1.0.0', @@ -177,12 +148,14 @@ describe Gitlab::GithubImport::Importer, lib: true do ) end + subject { described_class.new(project) } + it 'returns true' do - expect(described_class.new(project).execute).to eq true + expect(subject.execute).to eq true end it 'does not raise an error' do - expect { described_class.new(project).execute }.not_to raise_error + expect { subject.execute }.not_to raise_error end it 'stores error messages' do @@ -205,15 +178,93 @@ describe Gitlab::GithubImport::Importer, lib: true do end end + shared_examples 'Gitlab::GithubImport unit-testing' do + describe '#clean_up_restored_branches' do + subject { described_class.new(project) } + + before do + allow(gh_pull_request).to receive(:source_branch_exists?).at_least(:once) { false } + allow(gh_pull_request).to receive(:target_branch_exists?).at_least(:once) { false } + end + + context 'when pull request stills open' do + let(:gh_pull_request) { Gitlab::GithubImport::PullRequestFormatter.new(project, pull_request) } + + it 'does not remove branches' do + expect(subject).not_to receive(:remove_branch) + subject.send(:clean_up_restored_branches, gh_pull_request) + end + end + + context 'when pull request is closed' do + let(:gh_pull_request) { Gitlab::GithubImport::PullRequestFormatter.new(project, closed_pull_request) } + + it 'does remove branches' do + expect(subject).to receive(:remove_branch).at_least(2).times + subject.send(:clean_up_restored_branches, gh_pull_request) + end + end + end + end + let(:project) { create(:project, :wiki_disabled, import_url: "#{repo_root}/octocat/Hello-World.git") } + let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } let(:credentials) { { user: 'joe' } } + let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } + let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } + let(:repository) { double(id: 1, fork: false) } + let(:source_sha) { create(:commit, project: project).id } + let(:source_branch) { double(ref: 'branch-merged', repo: repository, sha: source_sha) } + let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } + let(:target_branch) { double(ref: 'master', repo: repository, sha: target_sha) } + let(:pull_request) do + double( + number: 1347, + milestone: nil, + state: 'open', + title: 'New feature', + body: 'Please pull these awesome changes', + head: source_branch, + base: target_branch, + assignee: nil, + user: octocat, + created_at: created_at, + updated_at: updated_at, + closed_at: nil, + merged_at: nil, + url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", + labels: [double(name: 'Label #2')] + ) + end + let(:closed_pull_request) do + double( + number: 1347, + milestone: nil, + state: 'closed', + title: 'New feature', + body: 'Please pull these awesome changes', + head: source_branch, + base: target_branch, + assignee: nil, + user: octocat, + created_at: created_at, + updated_at: updated_at, + closed_at: updated_at, + merged_at: nil, + url: "#{api_root}/repos/octocat/Hello-World/pulls/1347", + labels: [double(name: 'Label #2')] + ) + end + context 'when importing a GitHub project' do let(:api_root) { 'https://api.github.com' } let(:repo_root) { 'https://github.com' } + subject { described_class.new(project) } it_behaves_like 'Gitlab::GithubImport::Importer#execute' it_behaves_like 'Gitlab::GithubImport::Importer#execute an error occurs' + it_behaves_like 'Gitlab::GithubImport unit-testing' describe '#client' do it 'instantiates a Client' do @@ -223,7 +274,7 @@ describe Gitlab::GithubImport::Importer, lib: true do {} ) - described_class.new(project).client + subject.client end end end @@ -231,6 +282,8 @@ describe Gitlab::GithubImport::Importer, lib: true do context 'when importing a Gitea project' do let(:api_root) { 'https://try.gitea.io/api/v1' } let(:repo_root) { 'https://try.gitea.io' } + subject { described_class.new(project) } + before do project.update(import_type: 'gitea', import_url: "#{repo_root}/foo/group/project.git") end @@ -239,6 +292,7 @@ describe Gitlab::GithubImport::Importer, lib: true do let(:expected_not_called) { [:import_releases] } end it_behaves_like 'Gitlab::GithubImport::Importer#execute an error occurs' + it_behaves_like 'Gitlab::GithubImport unit-testing' describe '#client' do it 'instantiates a Client' do @@ -248,7 +302,7 @@ describe Gitlab::GithubImport::Importer, lib: true do { host: "#{repo_root}:443/foo", api_version: 'v1' } ) - described_class.new(project).client + subject.client end end end diff --git a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb index e46be18aa99..44423917944 100644 --- a/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb +++ b/spec/lib/gitlab/github_import/pull_request_formatter_spec.rb @@ -7,10 +7,12 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do let(:target_sha) { create(:commit, project: project, git_commit: RepoHelpers.another_sample_commit).id } let(:repository) { double(id: 1, fork: false) } let(:source_repo) { repository } - let(:source_branch) { double(ref: 'feature', repo: source_repo, sha: source_sha) } + let(:source_branch) { double(ref: 'branch-merged', repo: source_repo, sha: source_sha) } + let(:forked_source_repo) { double(id: 2, fork: true, name: 'otherproject', full_name: 'company/otherproject') } let(:target_repo) { repository } let(:target_branch) { double(ref: 'master', repo: target_repo, sha: target_sha) } let(:removed_branch) { double(ref: 'removed-branch', repo: source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') } + let(:forked_branch) { double(ref: 'master', repo: forked_source_repo, sha: '2e5d3239642f9161dcbbc4b70a211a68e5e45e2b') } let(:octocat) { double(id: 123456, login: 'octocat', email: 'octocat@example.com') } let(:created_at) { DateTime.strptime('2011-01-26T19:01:12Z') } let(:updated_at) { DateTime.strptime('2011-01-27T19:01:12Z') } @@ -49,7 +51,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do title: 'New feature', description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, - source_branch: 'feature', + source_branch: 'branch-merged', source_branch_sha: source_sha, target_project: project, target_branch: 'master', @@ -75,7 +77,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do title: 'New feature', description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, - source_branch: 'feature', + source_branch: 'branch-merged', source_branch_sha: source_sha, target_project: project, target_branch: 'master', @@ -102,7 +104,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do title: 'New feature', description: "*Created by: octocat*\n\nPlease pull these awesome changes", source_project: project, - source_branch: 'feature', + source_branch: 'branch-merged', source_branch_sha: source_sha, target_project: project, target_branch: 'master', @@ -194,7 +196,7 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do let(:raw_data) { double(base_data) } it 'returns branch ref' do - expect(pull_request.source_branch_name).to eq 'feature' + expect(pull_request.source_branch_name).to eq 'branch-merged' end end @@ -205,10 +207,18 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do expect(pull_request.source_branch_name).to eq 'pull/1347/removed-branch' end end + + context 'when source branch is from a fork' do + let(:raw_data) { double(base_data.merge(head: forked_branch)) } + + it 'prefixes branch name with pull request number and project with namespace to avoid collision' do + expect(pull_request.source_branch_name).to eq 'pull/1347/company/otherproject/master' + end + end end shared_examples 'Gitlab::GithubImport::PullRequestFormatter#target_branch_name' do - context 'when source branch exists' do + context 'when target branch exists' do let(:raw_data) { double(base_data) } it 'returns branch ref' do @@ -271,6 +281,24 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do end end + describe '#cross_project?' do + context 'when source and target repositories are different' do + let(:raw_data) { double(base_data.merge(head: forked_branch)) } + + it 'returns true' do + expect(pull_request.cross_project?).to eq true + end + end + + context 'when source and target repositories are the same' do + let(:raw_data) { double(base_data.merge(head: source_branch)) } + + it 'returns false' do + expect(pull_request.cross_project?).to eq false + end + end + end + describe '#url' do let(:raw_data) { double(base_data) } @@ -278,4 +306,12 @@ describe Gitlab::GithubImport::PullRequestFormatter, lib: true do expect(pull_request.url).to eq 'https://api.github.com/repos/octocat/Hello-World/pulls/1347' end end + + describe '#opened?' do + let(:raw_data) { double(base_data.merge(state: 'open')) } + + it 'returns true when state is "open"' do + expect(pull_request.opened?).to be_truthy + end + end end diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index 2cb74629da8..3fd361de458 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -70,4 +70,12 @@ describe Gitlab::UrlSanitizer, lib: true do expect(sanitizer.full_url).to eq('user@server:project.git') end end + + describe '.valid?' do + it 'validates url strings' do + expect(described_class.valid?(nil)).to be(false) + expect(described_class.valid?('valid@project:url.git')).to be(true) + expect(described_class.valid?('123://invalid:url')).to be(false) + end + end end diff --git a/spec/migrations/rename_more_reserved_project_names_spec.rb b/spec/migrations/rename_more_reserved_project_names_spec.rb new file mode 100644 index 00000000000..36e82729c23 --- /dev/null +++ b/spec/migrations/rename_more_reserved_project_names_spec.rb @@ -0,0 +1,47 @@ +# encoding: utf-8 + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170313133418_rename_more_reserved_project_names.rb') + +# This migration uses multiple threads, and thus different transactions. This +# means data created in this spec may not be visible to some threads. To work +# around this we use the TRUNCATE cleaning strategy. +describe RenameMoreReservedProjectNames, truncate: true do + let(:migration) { described_class.new } + let!(:project) { create(:empty_project) } + + before do + project.path = 'artifacts' + project.save!(validate: false) + end + + describe '#up' do + context 'when project repository exists' do + before { project.create_repository } + + context 'when no exception is raised' do + it 'renames project with reserved names' do + migration.up + + expect(project.reload.path).to eq('artifacts0') + end + end + + context 'when exception is raised during rename' do + before do + allow(project).to receive(:rename_repo).and_raise(StandardError) + end + + it 'captures exception from project rename' do + expect { migration.up }.not_to raise_error + end + end + end + + context 'when project repository does not exist' do + it 'does not raise error' do + expect { migration.up }.not_to raise_error + end + end + end +end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index 30f8fdf91b2..92d70cfc64c 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -1,6 +1,12 @@ require 'spec_helper' describe Ability, lib: true do + context 'using a nil subject' do + it 'is always empty' do + expect(Ability.allowed(nil, nil).to_set).to be_empty + end + end + describe '.can_edit_note?' do let(:project) { create(:empty_project) } let(:note) { create(:note_on_issue, project: project) } diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 03d02b4d382..94c25a454aa 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -70,6 +70,8 @@ describe Blob do end describe '#to_partial_path' do + let(:project) { double(lfs_enabled?: true) } + def stubbed_blob(overrides = {}) overrides.reverse_merge!( image?: false, @@ -84,34 +86,35 @@ describe Blob do end end - it 'handles LFS pointers' do - blob = stubbed_blob(lfs_pointer?: true) + it 'handles LFS pointers with LFS enabled' do + blob = stubbed_blob(lfs_pointer?: true, text?: true) + expect(blob.to_partial_path(project)).to eq 'download' + end - expect(blob.to_partial_path).to eq 'download' + it 'handles LFS pointers with LFS disabled' do + blob = stubbed_blob(lfs_pointer?: true, text?: true) + project = double(lfs_enabled?: false) + expect(blob.to_partial_path(project)).to eq 'text' end it 'handles SVGs' do blob = stubbed_blob(text?: true, svg?: true) - - expect(blob.to_partial_path).to eq 'image' + expect(blob.to_partial_path(project)).to eq 'image' end it 'handles images' do blob = stubbed_blob(image?: true) - - expect(blob.to_partial_path).to eq 'image' + expect(blob.to_partial_path(project)).to eq 'image' end it 'handles text' do blob = stubbed_blob(text?: true) - - expect(blob.to_partial_path).to eq 'text' + expect(blob.to_partial_path(project)).to eq 'text' end it 'defaults to download' do blob = stubbed_blob - - expect(blob.to_partial_path).to eq 'download' + expect(blob.to_partial_path(project)).to eq 'download' end end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index dd5f7098d06..9962c987110 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -197,6 +197,24 @@ describe Ci::Pipeline, models: true do end end end + + context 'when there is a stage with warnings' do + before do + create(:commit_status, pipeline: pipeline, + stage: 'deploy', + name: 'prod:2', + stage_idx: 2, + status: 'failed', + allow_failure: true) + end + + it 'populates stage with correct number of warnings' do + deploy_stage = pipeline.stages.third + + expect(deploy_stage).not_to receive(:statuses) + expect(deploy_stage).to have_warnings + end + end end describe '#stages_count' do @@ -647,7 +665,7 @@ describe Ci::Pipeline, models: true do let(:pipeline) { create(:ci_pipeline, status: :manual) } it 'returns detailed status for blocked pipeline' do - expect(subject.text).to eq 'manual' + expect(subject.text).to eq 'blocked' end end diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index c4a9743a4e2..c38faf32f7d 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -170,22 +170,31 @@ describe Ci::Stage, models: true do context 'when stage has warnings' do context 'when using memoized warnings flag' do context 'when there are warnings' do - let(:stage) { build(:ci_stage, warnings: true) } + let(:stage) { build(:ci_stage, warnings: 2) } - it 'has memoized warnings' do + it 'returns true using memoized value' do expect(stage).not_to receive(:statuses) expect(stage).to have_warnings end end context 'when there are no warnings' do - let(:stage) { build(:ci_stage, warnings: false) } + let(:stage) { build(:ci_stage, warnings: 0) } - it 'has memoized warnings' do + it 'returns false using memoized value' do expect(stage).not_to receive(:statuses) expect(stage).not_to have_warnings end end + + context 'when number of warnings is not a valid value' do + let(:stage) { build(:ci_stage, warnings: true) } + + it 'calculates statuses using database queries' do + expect(stage).to receive(:statuses).and_call_original + expect(stage).not_to have_warnings + end + end end context 'when calculating warnings from statuses' do diff --git a/spec/models/concerns/relative_positioning_spec.rb b/spec/models/concerns/relative_positioning_spec.rb index 69906382545..255b584a85e 100644 --- a/spec/models/concerns/relative_positioning_spec.rb +++ b/spec/models/concerns/relative_positioning_spec.rb @@ -12,12 +12,6 @@ describe Issue, 'RelativePositioning' do end end - describe '#min_relative_position' do - it 'returns maximum position' do - expect(issue.min_relative_position).to eq issue.relative_position - end - end - describe '#max_relative_position' do it 'returns maximum position' do expect(issue.max_relative_position).to eq issue1.relative_position @@ -29,8 +23,8 @@ describe Issue, 'RelativePositioning' do expect(issue1.prev_relative_position).to eq issue.relative_position end - it 'returns minimum position if there is no issue above' do - expect(issue.prev_relative_position).to eq RelativePositioning::MIN_POSITION + it 'returns nil if there is no issue above' do + expect(issue.prev_relative_position).to eq nil end end @@ -39,8 +33,8 @@ describe Issue, 'RelativePositioning' do expect(issue.next_relative_position).to eq issue1.relative_position end - it 'returns next position if there is no issue below' do - expect(issue1.next_relative_position).to eq RelativePositioning::MAX_POSITION + it 'returns nil if there is no issue below' do + expect(issue1.next_relative_position).to eq nil end end @@ -72,6 +66,34 @@ describe Issue, 'RelativePositioning' do end end + describe '#shift_after?' do + it 'returns true' do + issue.update(relative_position: issue1.relative_position - 1) + + expect(issue.shift_after?).to be_truthy + end + + it 'returns false' do + issue.update(relative_position: issue1.relative_position - 2) + + expect(issue.shift_after?).to be_falsey + end + end + + describe '#shift_before?' do + it 'returns true' do + issue.update(relative_position: issue1.relative_position + 1) + + expect(issue.shift_before?).to be_truthy + end + + it 'returns false' do + issue.update(relative_position: issue1.relative_position + 2) + + expect(issue.shift_before?).to be_falsey + end + end + describe '#move_between' do it 'positions issue between two other' do new_issue.move_between(issue, issue1) @@ -100,5 +122,83 @@ describe Issue, 'RelativePositioning' do expect(new_issue.relative_position).to be > issue.relative_position expect(issue.relative_position).to be < issue1.relative_position end + + it 'positions issues between other two if distance is 1' do + issue1.update relative_position: issue.relative_position + 1 + + new_issue.move_between(issue, issue1) + + expect(new_issue.relative_position).to be > issue.relative_position + expect(issue.relative_position).to be < issue1.relative_position + end + + it 'positions issue in the middle of other two if distance is big enough' do + issue.update relative_position: 6000 + issue1.update relative_position: 10000 + + new_issue.move_between(issue, issue1) + + expect(new_issue.relative_position).to eq(8000) + end + + it 'positions issue closer to the middle if we are at the very top' do + issue1.update relative_position: 6000 + + new_issue.move_between(nil, issue1) + + expect(new_issue.relative_position).to eq(6000 - RelativePositioning::IDEAL_DISTANCE) + end + + it 'positions issue closer to the middle if we are at the very bottom' do + issue.update relative_position: 6000 + issue1.update relative_position: nil + + new_issue.move_between(issue, nil) + + expect(new_issue.relative_position).to eq(6000 + RelativePositioning::IDEAL_DISTANCE) + end + + it 'positions issue in the middle of other two if distance is not big enough' do + issue.update relative_position: 100 + issue1.update relative_position: 400 + + new_issue.move_between(issue, issue1) + + expect(new_issue.relative_position).to eq(250) + end + + it 'positions issue in the middle of other two is there is no place' do + issue.update relative_position: 100 + issue1.update relative_position: 101 + + new_issue.move_between(issue, issue1) + + expect(new_issue.relative_position).to be_between(issue.relative_position, issue1.relative_position) + end + + it 'uses rebalancing if there is no place' do + issue.update relative_position: 100 + issue1.update relative_position: 101 + issue2 = create(:issue, relative_position: 102, project: project) + new_issue.update relative_position: 103 + + new_issue.move_between(issue1, issue2) + new_issue.save! + + expect(new_issue.relative_position).to be_between(issue1.relative_position, issue2.relative_position) + expect(issue.reload.relative_position).not_to eq(100) + end + + it 'positions issue right if we pass none-sequential parameters' do + issue.update relative_position: 99 + issue1.update relative_position: 101 + issue2 = create(:issue, relative_position: 102, project: project) + new_issue.update relative_position: 103 + + new_issue.move_between(issue, issue2) + new_issue.save! + + expect(new_issue.relative_position).to be(100) + end end end diff --git a/spec/models/global_milestone_spec.rb b/spec/models/global_milestone_spec.rb index cacbab8bcb1..55b87d1c48a 100644 --- a/spec/models/global_milestone_spec.rb +++ b/spec/models/global_milestone_spec.rb @@ -92,6 +92,41 @@ describe GlobalMilestone, models: true do end end + describe '.states_count' do + context 'when the projects have milestones' do + before do + create(:closed_milestone, title: 'Active Group Milestone', project: project3) + create(:active_milestone, title: 'Active Group Milestone', project: project1) + create(:active_milestone, title: 'Active Group Milestone', project: project2) + create(:closed_milestone, title: 'Closed Group Milestone', project: project1) + create(:closed_milestone, title: 'Closed Group Milestone', project: project2) + create(:closed_milestone, title: 'Closed Group Milestone', project: project3) + end + + it 'returns the quantity of global milestones in each possible state' do + expected_count = { opened: 1, closed: 2, all: 2 } + + count = GlobalMilestone.states_count(Project.all) + + expect(count).to eq(expected_count) + end + end + + context 'when the projects do not have milestones' do + before do + project1 + end + + it 'returns 0 as the quantity of global milestones in each state' do + expected_count = { opened: 0, closed: 0, all: 0 } + + count = GlobalMilestone.states_count(Project.all) + + expect(count).to eq(expected_count) + end + end + end + describe '#initialize' do let(:milestone1_project1) { create(:milestone, title: "Milestone v1.2", project: project1) } let(:milestone1_project2) { create(:milestone, title: "Milestone v1.2", project: project2) } @@ -127,4 +162,32 @@ describe GlobalMilestone, models: true do expect(global_milestone.safe_title).to eq('git-test') end end + + describe '#state' do + context 'when at least one milestone is active' do + it 'returns active' do + title = 'Active Group Milestone' + milestones = [ + create(:active_milestone, title: title), + create(:closed_milestone, title: title) + ] + global_milestone = GlobalMilestone.new(title, milestones) + + expect(global_milestone.state).to eq('active') + end + end + + context 'when all milestones are closed' do + it 'returns closed' do + title = 'Closed Group Milestone' + milestones = [ + create(:closed_milestone, title: title), + create(:closed_milestone, title: title) + ] + global_milestone = GlobalMilestone.new(title, milestones) + + expect(global_milestone.state).to eq('closed') + end + end + end end diff --git a/spec/models/project_services/issue_tracker_service_spec.rb b/spec/models/project_services/issue_tracker_service_spec.rb new file mode 100644 index 00000000000..fbe6f344a98 --- /dev/null +++ b/spec/models/project_services/issue_tracker_service_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe IssueTrackerService, models: true do + describe 'Validations' do + let(:project) { create :project } + + describe 'only one issue tracker per project' do + let(:service) { RedmineService.new(project: project, active: true) } + + before do + create(:service, project: project, active: true, category: 'issue_tracker') + end + + context 'when service is changed manually by user' do + it 'executes the validation' do + valid = service.valid?(:manual_change) + + expect(valid).to be_falsey + expect(service.errors[:base]).to include( + 'Another issue tracker is already in use. Only one issue tracker service can be active at a time' + ) + end + end + + context 'when service is changed internally' do + it 'does not execute the validation' do + expect(service.valid?).to be_truthy + end + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index adb5b538922..9da4140f3ce 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -210,22 +210,6 @@ describe User, models: true do end end end - - describe 'ghost users' do - it 'does not allow a non-blocked ghost user' do - user = build(:user, :ghost) - user.state = 'active' - - expect(user).to be_invalid - end - - it 'allows a blocked ghost user' do - user = build(:user, :ghost) - user.state = 'blocked' - - expect(user).to be_valid - end - end end describe "scopes" do @@ -713,8 +697,9 @@ describe User, models: true do describe '.search_with_secondary_emails' do delegate :search_with_secondary_emails, to: :described_class - let!(:user) { create(:user) } - let!(:email) { create(:email) } + let!(:user) { create(:user, name: 'John Doe', username: 'john.doe', email: 'john.doe@example.com' ) } + let!(:another_user) { create(:user, name: 'Albert Smith', username: 'albert.smith', email: 'albert.smith@example.com' ) } + let!(:email) { create(:email, user: another_user) } it 'returns users with a matching name' do expect(search_with_secondary_emails(user.name)).to eq([user]) diff --git a/spec/policies/base_policy_spec.rb b/spec/policies/base_policy_spec.rb index 63acc0b68cd..02acdcb36df 100644 --- a/spec/policies/base_policy_spec.rb +++ b/spec/policies/base_policy_spec.rb @@ -1,17 +1,19 @@ require 'spec_helper' describe BasePolicy, models: true do - let(:build) { Ci::Build.new } - describe '.class_for' do it 'detects policy class based on the subject ancestors' do - expect(described_class.class_for(build)).to eq(Ci::BuildPolicy) + expect(described_class.class_for(GenericCommitStatus.new)).to eq(CommitStatusPolicy) end it 'detects policy class for a presented subject' do - presentee = Ci::BuildPresenter.new(build) + presentee = Ci::BuildPresenter.new(Ci::Build.new) expect(described_class.class_for(presentee)).to eq(Ci::BuildPolicy) end + + it 'uses GlobalPolicy when :global is given' do + expect(described_class.class_for(:global)).to eq(GlobalPolicy) + end end end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index a89676fec93..988a57a80ea 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -436,7 +436,7 @@ describe API::Helpers, api: true do context 'current_user is present' do before do - expect_any_instance_of(self.class).to receive(:current_user).and_return(true) + expect_any_instance_of(self.class).to receive(:current_user).at_least(:once).and_return(User.new) end it 'does not raise an error' do diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 2fc11a3b782..de7dbca0b22 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -928,29 +928,34 @@ describe API::Issues, api: true do ]) end - context 'resolving issues in a merge request' do + context 'resolving discussions' do let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first } let(:merge_request) { discussion.noteable } let(:project) { merge_request.source_project } + before do project.team << [user, :master] - post api("/projects/#{project.id}/issues", user), - title: 'New Issue', - merge_request_for_resolving_discussions: merge_request.iid - end - - it 'creates a new project issue' do - expect(response).to have_http_status(:created) end - it 'resolves the discussions in a merge request' do - discussion.first_note.reload + context 'resolving all discussions in a merge request' do + before do + post api("/projects/#{project.id}/issues", user), + title: 'New Issue', + merge_request_to_resolve_discussions_of: merge_request.iid + end - expect(discussion.resolved?).to be(true) + it_behaves_like 'creating an issue resolving discussions through the API' end - it 'assigns a description to the issue mentioning the merge request' do - expect(json_response['description']).to include(merge_request.to_reference) + context 'resolving a single discussion' do + before do + post api("/projects/#{project.id}/issues", user), + title: 'New Issue', + merge_request_to_resolve_discussions_of: merge_request.iid, + discussion_to_resolve: discussion.id + end + + it_behaves_like 'creating an issue resolving discussions through the API' end end diff --git a/spec/requests/api/milestones_spec.rb b/spec/requests/api/milestones_spec.rb index 3bb8b6fdbeb..7fb728fed6f 100644 --- a/spec/requests/api/milestones_spec.rb +++ b/spec/requests/api/milestones_spec.rb @@ -243,8 +243,8 @@ describe API::Milestones, api: true do describe 'confidential issues' do let(:public_project) { create(:empty_project, :public) } let(:milestone) { create(:milestone, project: public_project) } - let(:issue) { create(:issue, project: public_project) } - let(:confidential_issue) { create(:issue, confidential: true, project: public_project) } + let(:issue) { create(:issue, project: public_project, position: 2) } + let(:confidential_issue) { create(:issue, confidential: true, project: public_project, position: 1) } before do public_project.team << [user, :developer] @@ -283,11 +283,24 @@ describe API::Milestones, api: true do expect(json_response.size).to eq(1) expect(json_response.map { |issue| issue['id'] }).to include(issue.id) end + + it 'returns issues ordered by position asc' do + get api("/projects/#{public_project.id}/milestones/#{milestone.id}/issues", user) + + expect(response).to have_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(2) + expect(json_response.first['id']).to eq(confidential_issue.id) + expect(json_response.second['id']).to eq(issue.id) + end end end describe 'GET /projects/:id/milestones/:milestone_id/merge_requests' do - let(:merge_request) { create(:merge_request, source_project: project) } + let(:merge_request) { create(:merge_request, source_project: project, position: 2) } + let(:another_merge_request) { create(:merge_request, :simple, source_project: project, position: 1) } + before do milestone.merge_requests << merge_request end @@ -320,5 +333,18 @@ describe API::Milestones, api: true do expect(response).to have_http_status(401) end + + it 'returns merge_requests ordered by position asc' do + milestone.merge_requests << another_merge_request + + get api("/projects/#{project.id}/milestones/#{milestone.id}/merge_requests", user) + + expect(response).to have_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.size).to eq(2) + expect(json_response.first['id']).to eq(another_merge_request.id) + expect(json_response.second['id']).to eq(merge_request.id) + end end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 77f79cd5bc7..c481b7e72b1 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1,9 +1,9 @@ # -*- coding: utf-8 -*- require 'spec_helper' -describe API::Projects, api: true do - include ApiHelpers +describe API::Projects, :api do include Gitlab::CurrentSettings + let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } @@ -424,6 +424,14 @@ describe API::Projects, api: true do expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to be_truthy end + it 'ignores import_url when it is nil' do + project = attributes_for(:project, { import_url: nil }) + + post api('/projects', user), project + + expect(response).to have_http_status(201) + end + context 'when a visibility level is restricted' do let(:project_param) { attributes_for(:project, visibility: 'public') } diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 15d458e0795..d50fe80b36a 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -39,6 +39,7 @@ describe API::Runner do expect(json_response['id']).to eq(runner.id) expect(json_response['token']).to eq(runner.token) expect(runner.run_untagged).to be true + expect(runner.token).not_to eq(registration_token) end context 'when project token is used' do @@ -49,6 +50,8 @@ describe API::Runner do expect(response).to have_http_status 201 expect(project.runners.size).to eq(1) + expect(Ci::Runner.first.token).not_to eq(registration_token) + expect(Ci::Runner.first.token).not_to eq(project.runners_token) end end end diff --git a/spec/requests/ci/api/runners_spec.rb b/spec/requests/ci/api/runners_spec.rb index 8719313783e..d50cdfdc2d6 100644 --- a/spec/requests/ci/api/runners_spec.rb +++ b/spec/requests/ci/api/runners_spec.rb @@ -18,6 +18,7 @@ describe Ci::API::Runners do it 'creates runner with default values' do expect(response).to have_http_status 201 expect(Ci::Runner.first.run_untagged).to be true + expect(Ci::Runner.first.token).not_to eq(registration_token) end end @@ -74,6 +75,8 @@ describe Ci::API::Runners do it 'creates runner' do expect(response).to have_http_status 201 expect(project.runners.size).to eq(1) + expect(Ci::Runner.first.token).not_to eq(registration_token) + expect(Ci::Runner.first.token).not_to eq(project.runners_token) end end diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index 09807e5d35b..1dd53236fbd 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -8,24 +8,34 @@ describe Issues::BuildService, services: true do project.team << [user, :developer] end + context 'for a single discussion' do + describe '#execute' do + let(:merge_request) { create(:merge_request, title: "Hello world", source_project: project) } + let(:discussion) { Discussion.new([create(:diff_note_on_merge_request, project: project, noteable: merge_request, note: "Almost done")]) } + let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid, discussion_to_resolve: discussion.id) } + + it 'references the noteable title in the issue title' do + issue = service.execute + + expect(issue.title).to include('Hello world') + end + + it 'adds the note content to the description' do + issue = service.execute + + expect(issue.description).to include('Almost done') + end + end + end + context 'for discussions in a merge request' do let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) } - let(:issue) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request).execute } - - def position_on_line(line_number) - Gitlab::Diff::Position.new( - old_path: "files/ruby/popen.rb", - new_path: "files/ruby/popen.rb", - old_line: nil, - new_line: line_number, - diff_refs: merge_request.diff_refs - ) - end + let(:issue) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute } describe '#items_for_discussions' do it 'has an item for each discussion' do - create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.source_project, position: position_on_line(13)) - service = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request) + create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.source_project, line_number: 13) + service = described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) service.execute @@ -34,7 +44,7 @@ describe Issues::BuildService, services: true do end describe '#item_for_discussion' do - let(:service) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request) } + let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } it 'mentions the author of the note' do discussion = Discussion.new([create(:diff_note_on_merge_request, author: create(:user, username: 'author'))]) @@ -47,11 +57,11 @@ describe Issues::BuildService, services: true do "with a blockquote\n"\ "> That has a quote\n"\ ">>>\n" - note_result = "This is a string\n"\ - "> with a blockquote\n"\ - "> > That has a quote\n" + note_result = " > This is a string\n"\ + " > > with a blockquote\n"\ + " > > > That has a quote\n" discussion = Discussion.new([create(:diff_note_on_merge_request, note: note_text)]) - expect(service.item_for_discussion(discussion)).to include(">>>\n#{note_result}\n>>>") + expect(service.item_for_discussion(discussion)).to include(note_result) end end @@ -66,7 +76,7 @@ describe Issues::BuildService, services: true do it 'does not assign title when a title was given' do issue = described_class.new(project, user, - merge_request_for_resolving_discussions: merge_request, + merge_request_to_resolve_discussions_of: merge_request, title: 'What an issue').execute expect(issue.title).to eq('What an issue') @@ -74,7 +84,7 @@ describe Issues::BuildService, services: true do it 'does not assign description when a description was given' do issue = described_class.new(project, user, - merge_request_for_resolving_discussions: merge_request, + merge_request_to_resolve_discussions_of: merge_request, description: 'Fix at your earliest conveignance').execute expect(issue.description).to eq('Fix at your earliest conveignance') @@ -82,7 +92,7 @@ describe Issues::BuildService, services: true do describe 'with multiple discussions' do before do - create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.target_project, position: position_on_line(15)) + create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.target_project, line_number: 15) end it 'mentions all the authors in the description' do @@ -99,7 +109,7 @@ describe Issues::BuildService, services: true do end it 'mentions additional notes' do - create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, position: position_on_line(15)) + create_list(:diff_note_on_merge_request, 2, noteable: merge_request, project: merge_request.target_project, line_number: 15) expect(issue.description).to include('(+2 comments)') end @@ -112,7 +122,7 @@ describe Issues::BuildService, services: true do describe '#execute' do it 'mentions the merge request in the description' do - issue = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request).execute + issue = described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid).execute expect(issue.description).to include("Review the conversation in #{merge_request.to_reference}") end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 6045d00ff09..776cbc4296b 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -140,46 +140,85 @@ describe Issues::CreateService, services: true do it_behaves_like 'new issuable record that supports slash commands' - context 'for a merge request' do + context 'resolving discussions' do let(:discussion) { Discussion.for_diff_notes([create(:diff_note_on_merge_request)]).first } let(:merge_request) { discussion.noteable } let(:project) { merge_request.source_project } - let(:opts) { { merge_request_for_resolving_discussions: merge_request } } before do project.team << [user, :master] end - it 'resolves the discussion for the merge request' do - described_class.new(project, user, opts).execute - discussion.first_note.reload + describe 'for a single discussion' do + let(:opts) { { discussion_to_resolve: discussion.id, merge_request_to_resolve_discussions_of: merge_request.iid } } - expect(discussion.resolved?).to be(true) - end + it 'resolves the discussion' do + described_class.new(project, user, opts).execute + discussion.first_note.reload - it 'added a system note to the discussion' do - described_class.new(project, user, opts).execute + expect(discussion.resolved?).to be(true) + end - reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first + it 'added a system note to the discussion' do + described_class.new(project, user, opts).execute - expect(reloaded_discussion.last_note.system).to eq(true) - end + reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first + + expect(reloaded_discussion.last_note.system).to eq(true) + end + + it 'assigns the title and description for the issue' do + issue = described_class.new(project, user, opts).execute + + expect(issue.title).not_to be_nil + expect(issue.description).not_to be_nil + end - it 'assigns the title and description for the issue' do - issue = described_class.new(project, user, opts).execute + it 'can set nil explicitly to the title and description' do + issue = described_class.new(project, user, + merge_request_to_resolve_discussions_of: merge_request, + description: nil, + title: nil).execute - expect(issue.title).not_to be_nil - expect(issue.description).not_to be_nil + expect(issue.description).to be_nil + expect(issue.title).to be_nil + end end - it 'can set nil explicityly to the title and description' do - issue = described_class.new(project, user, - merge_request_for_resolving_discussions: merge_request, - description: nil, - title: nil).execute + describe 'for a merge request' do + let(:opts) { { merge_request_to_resolve_discussions_of: merge_request.iid } } + + it 'resolves the discussion' do + described_class.new(project, user, opts).execute + discussion.first_note.reload - expect(issue.description).to be_nil - expect(issue.title).to be_nil + expect(discussion.resolved?).to be(true) + end + + it 'added a system note to the discussion' do + described_class.new(project, user, opts).execute + + reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first + + expect(reloaded_discussion.last_note.system).to eq(true) + end + + it 'assigns the title and description for the issue' do + issue = described_class.new(project, user, opts).execute + + expect(issue.title).not_to be_nil + expect(issue.description).not_to be_nil + end + + it 'can set nil explicitly to the title and description' do + issue = described_class.new(project, user, + merge_request_to_resolve_discussions_of: merge_request, + description: nil, + title: nil).execute + + expect(issue.description).to be_nil + expect(issue.title).to be_nil + end end end diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb new file mode 100644 index 00000000000..6cc738aec08 --- /dev/null +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -0,0 +1,106 @@ +require 'spec_helper.rb' + +class DummyService < Issues::BaseService + include ::Issues::ResolveDiscussions + + def initialize(*args) + super + filter_resolve_discussion_params + end +end + +describe DummyService, services: true do + let(:project) { create(:project) } + let(:user) { create(:user) } + + before do + project.team << [user, :developer] + end + + describe "for resolving discussions" do + let(:discussion) { Discussion.new([create(:diff_note_on_merge_request, project: project, note: "Almost done")]) } + let(:merge_request) { discussion.noteable } + let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "other") } + + describe "#merge_request_for_resolving_discussion" do + let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } + + it "finds the merge request" do + expect(service.merge_request_to_resolve_discussions_of).to eq(merge_request) + end + + it "only queries for the merge request once" do + fake_finder = double + fake_results = double + + expect(fake_finder).to receive(:execute).and_return(fake_results).exactly(1) + expect(fake_results).to receive(:find_by).exactly(1) + expect(MergeRequestsFinder).to receive(:new).and_return(fake_finder).exactly(1) + + 2.times { service.merge_request_to_resolve_discussions_of } + end + end + + describe "#discussions_to_resolve" do + it "contains a single discussion when matching merge request and discussion are passed" do + service = described_class.new( + project, + user, + discussion_to_resolve: discussion.id, + merge_request_to_resolve_discussions_of: merge_request.iid + ) + # We need to compare discussion id's because the Discussion-objects are rebuilt + # which causes the object-id's not to be different. + discussion_ids = service.discussions_to_resolve.map(&:id) + + expect(discussion_ids).to contain_exactly(discussion.id) + end + + it "contains all discussions when only a merge request is passed" do + second_discussion = Discussion.new([create(:diff_note_on_merge_request, + noteable: merge_request, + project: merge_request.target_project, + line_number: 15)]) + service = described_class.new( + project, + user, + merge_request_to_resolve_discussions_of: merge_request.iid + ) + # We need to compare discussion id's because the Discussion-objects are rebuilt + # which causes the object-id's not to be different. + discussion_ids = service.discussions_to_resolve.map(&:id) + + expect(discussion_ids).to contain_exactly(discussion.id, second_discussion.id) + end + + it "contains only unresolved discussions" do + _second_discussion = Discussion.new([create(:diff_note_on_merge_request, :resolved, + noteable: merge_request, + project: merge_request.target_project, + line_number: 15, + )]) + service = described_class.new( + project, + user, + merge_request_to_resolve_discussions_of: merge_request.iid + ) + # We need to compare discussion id's because the Discussion-objects are rebuilt + # which causes the object-id's not to be different. + discussion_ids = service.discussions_to_resolve.map(&:id) + + expect(discussion_ids).to contain_exactly(discussion.id) + end + + it "is empty when a discussion and another merge request are passed" do + service = described_class.new( + project, + user, + discussion_to_resolve: discussion.id, + merge_request_to_resolve_discussions_of: other_merge_request.iid + ) + + expect(service.discussions_to_resolve).to be_empty + end + end + end +end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index ff367f54d2a..92729f68e5f 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -58,16 +58,16 @@ describe MergeRequests::RefreshService, services: true do it 'executes hooks with update action' do expect(refresh_service).to have_received(:execute_hooks). with(@merge_request, 'update', @oldrev) - end - it { expect(@merge_request.notes).not_to be_empty } - it { expect(@merge_request).to be_open } - it { expect(@merge_request.merge_when_pipeline_succeeds).to be_falsey } - it { expect(@merge_request.diff_head_sha).to eq(@newrev) } - it { expect(@fork_merge_request).to be_open } - it { expect(@fork_merge_request.notes).to be_empty } - it { expect(@build_failed_todo).to be_done } - it { expect(@fork_build_failed_todo).to be_done } + expect(@merge_request.notes).not_to be_empty + expect(@merge_request).to be_open + expect(@merge_request.merge_when_pipeline_succeeds).to be_falsey + expect(@merge_request.diff_head_sha).to eq(@newrev) + expect(@fork_merge_request).to be_open + expect(@fork_merge_request.notes).to be_empty + expect(@build_failed_todo).to be_done + expect(@fork_build_failed_todo).to be_done + end end context 'push to origin repo target branch' do @@ -76,12 +76,14 @@ describe MergeRequests::RefreshService, services: true do reload_mrs end - it { expect(@merge_request.notes.last.note).to include('merged') } - it { expect(@merge_request).to be_merged } - it { expect(@fork_merge_request).to be_merged } - it { expect(@fork_merge_request.notes.last.note).to include('merged') } - it { expect(@build_failed_todo).to be_done } - it { expect(@fork_build_failed_todo).to be_done } + it 'updates the merge state' do + expect(@merge_request.notes.last.note).to include('merged') + expect(@merge_request).to be_merged + expect(@fork_merge_request).to be_merged + expect(@fork_merge_request.notes.last.note).to include('merged') + expect(@build_failed_todo).to be_done + expect(@fork_build_failed_todo).to be_done + end end context 'manual merge of source branch' do @@ -95,13 +97,15 @@ describe MergeRequests::RefreshService, services: true do reload_mrs end - it { expect(@merge_request.notes.last.note).to include('merged') } - it { expect(@merge_request).to be_merged } - it { expect(@merge_request.diffs.size).to be > 0 } - it { expect(@fork_merge_request).to be_merged } - it { expect(@fork_merge_request.notes.last.note).to include('merged') } - it { expect(@build_failed_todo).to be_done } - it { expect(@fork_build_failed_todo).to be_done } + it 'updates the merge state' do + expect(@merge_request.notes.last.note).to include('merged') + expect(@merge_request).to be_merged + expect(@merge_request.diffs.size).to be > 0 + expect(@fork_merge_request).to be_merged + expect(@fork_merge_request.notes.last.note).to include('merged') + expect(@build_failed_todo).to be_done + expect(@fork_build_failed_todo).to be_done + end end context 'push to fork repo source branch' do @@ -117,14 +121,14 @@ describe MergeRequests::RefreshService, services: true do it 'executes hooks with update action' do expect(refresh_service).to have_received(:execute_hooks). with(@fork_merge_request, 'update', @oldrev) - end - it { expect(@merge_request.notes).to be_empty } - it { expect(@merge_request).to be_open } - it { expect(@fork_merge_request.notes.last.note).to include('added 28 commits') } - it { expect(@fork_merge_request).to be_open } - it { expect(@build_failed_todo).to be_pending } - it { expect(@fork_build_failed_todo).to be_pending } + expect(@merge_request.notes).to be_empty + expect(@merge_request).to be_open + expect(@fork_merge_request.notes.last.note).to include('added 28 commits') + expect(@fork_merge_request).to be_open + expect(@build_failed_todo).to be_pending + expect(@fork_build_failed_todo).to be_pending + end end context 'closed fork merge request' do @@ -139,12 +143,14 @@ describe MergeRequests::RefreshService, services: true do expect(refresh_service).not_to have_received(:execute_hooks) end - it { expect(@merge_request.notes).to be_empty } - it { expect(@merge_request).to be_open } - it { expect(@fork_merge_request.notes).to be_empty } - it { expect(@fork_merge_request).to be_closed } - it { expect(@build_failed_todo).to be_pending } - it { expect(@fork_build_failed_todo).to be_pending } + it 'updates merge request to closed state' do + expect(@merge_request.notes).to be_empty + expect(@merge_request).to be_open + expect(@fork_merge_request.notes).to be_empty + expect(@fork_merge_request).to be_closed + expect(@build_failed_todo).to be_pending + expect(@fork_build_failed_todo).to be_pending + end end end @@ -155,12 +161,14 @@ describe MergeRequests::RefreshService, services: true do reload_mrs end - it { expect(@merge_request.notes).to be_empty } - it { expect(@merge_request).to be_open } - it { expect(@fork_merge_request.notes).to be_empty } - it { expect(@fork_merge_request).to be_open } - it { expect(@build_failed_todo).to be_pending } - it { expect(@fork_build_failed_todo).to be_pending } + it 'updates the merge request state' do + expect(@merge_request.notes).to be_empty + expect(@merge_request).to be_open + expect(@fork_merge_request.notes).to be_empty + expect(@fork_merge_request).to be_open + expect(@build_failed_todo).to be_pending + expect(@fork_build_failed_todo).to be_pending + end end describe 'merge request diff' do @@ -179,12 +187,14 @@ describe MergeRequests::RefreshService, services: true do reload_mrs end - it { expect(@merge_request.notes.last.note).to include('merged') } - it { expect(@merge_request).to be_merged } - it { expect(@fork_merge_request).to be_open } - it { expect(@fork_merge_request.notes).to be_empty } - it { expect(@build_failed_todo).to be_done } - it { expect(@fork_build_failed_todo).to be_done } + it 'updates the merge request state' do + expect(@merge_request.notes.last.note).to include('merged') + expect(@merge_request).to be_merged + expect(@fork_merge_request).to be_open + expect(@fork_merge_request.notes).to be_empty + expect(@build_failed_todo).to be_done + expect(@fork_build_failed_todo).to be_done + end end context 'push new branch that exists in a merge request' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5fda7c63cdb..ceb3209331f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -43,14 +43,27 @@ RSpec.configure do |config| config.include ActiveSupport::Testing::TimeHelpers config.include StubGitlabCalls config.include StubGitlabData + config.include ApiHelpers, :api config.infer_spec_type_from_file_location! + + config.define_derived_metadata(file_path: %r{/spec/requests/(ci/)?api/}) do |metadata| + metadata[:api] = true + end + config.raise_errors_for_deprecations! config.before(:suite) do TestEnv.init end + if ENV['CI'] + # Retry only on feature specs that use JS + config.around :each, :js do |ex| + ex.run_with_retry retry: 3 + end + end + config.around(:each, :caching) do |example| caching_store = Rails.cache Rails.cache = ActiveSupport::Cache::MemoryStore.new if example.metadata[:caching] diff --git a/spec/support/api/issues_resolving_discussions_shared_examples.rb b/spec/support/api/issues_resolving_discussions_shared_examples.rb new file mode 100644 index 00000000000..d26d279363c --- /dev/null +++ b/spec/support/api/issues_resolving_discussions_shared_examples.rb @@ -0,0 +1,15 @@ +shared_examples 'creating an issue resolving discussions through the API' do + it 'creates a new project issue' do + expect(response).to have_http_status(:created) + end + + it 'resolves the discussions in a merge request' do + discussion.first_note.reload + + expect(discussion.resolved?).to be(true) + end + + it 'assigns a description to the issue mentioning the merge request' do + expect(json_response['description']).to include(merge_request.to_reference) + end +end diff --git a/spec/support/api_helpers.rb b/spec/support/api_helpers.rb index ae6e708cf87..35d1e1cfc7d 100644 --- a/spec/support/api_helpers.rb +++ b/spec/support/api_helpers.rb @@ -49,8 +49,4 @@ module ApiHelpers '' end end - - def json_response - @_json_response ||= JSON.parse(response.body) - end end diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 16d5f2bf0b8..aa14709bc9c 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -1,9 +1,10 @@ require 'capybara/rails' require 'capybara/rspec' require 'capybara/poltergeist' +require 'capybara-screenshot/rspec' # Give CI some extra time -timeout = (ENV['CI'] || ENV['CI_SERVER']) ? 90 : 10 +timeout = (ENV['CI'] || ENV['CI_SERVER']) ? 30 : 10 Capybara.javascript_driver = :poltergeist Capybara.register_driver :poltergeist do |app| @@ -21,12 +22,8 @@ end Capybara.default_max_wait_time = timeout Capybara.ignore_hidden_elements = true -unless ENV['CI'] || ENV['CI_SERVER'] - require 'capybara-screenshot/rspec' - - # Keep only the screenshots generated from the last failing test suite - Capybara::Screenshot.prune_strategy = :keep_last_run -end +# Keep only the screenshots generated from the last failing test suite +Capybara::Screenshot.prune_strategy = :keep_last_run RSpec.configure do |config| config.before(:suite) do diff --git a/spec/support/features/resolving_discussions_in_issues_shared_examples.rb b/spec/support/features/resolving_discussions_in_issues_shared_examples.rb new file mode 100644 index 00000000000..4a946995f84 --- /dev/null +++ b/spec/support/features/resolving_discussions_in_issues_shared_examples.rb @@ -0,0 +1,41 @@ +shared_examples 'creating an issue for a discussion' do + it 'shows an issue with the title filled in' do + title_field = page.find_field('issue[title]') + + expect(title_field.value).to include(merge_request.title) + end + + it 'has a mention of the discussion in the description' do + description_field = page.find_field('issue[description]') + + expect(description_field.value).to include(discussion.first_note.note) + end + + it 'can create a new issue for the project' do + expect { click_button 'Submit issue' }.to change { project.issues.reload.size }.by(1) + end + + it 'resolves the discussion in the merge request' do + click_button 'Submit issue' + + discussion.first_note.reload + + expect(discussion.resolved?).to eq(true) + end + + it 'shows a flash messaage after resolving a discussion' do + click_button 'Submit issue' + + page.within '.flash-notice' do + # Only check for the word 'Resolved' since the spec might have resolved + # multiple discussions + expect(page).to have_content('Resolved') + end + end + + it 'has a hidden field for the merge request' do + merge_request_field = find('#merge_request_to_resolve_discussions_of', visible: false) + + expect(merge_request_field.value).to eq(merge_request.iid.to_s) + end +end diff --git a/spec/support/filtered_search_helpers.rb b/spec/support/filtered_search_helpers.rb index f21b85ec10b..6b009b132b6 100644 --- a/spec/support/filtered_search_helpers.rb +++ b/spec/support/filtered_search_helpers.rb @@ -4,9 +4,14 @@ module FilteredSearchHelpers end # Enables input to be set (similar to copy and paste) - def input_filtered_search(search_term, submit: true) - # Add an extra space to engage visual tokens - filtered_search.set("#{search_term} ") + def input_filtered_search(search_term, submit: true, extra_space: true) + search = search_term + if extra_space + # Add an extra space to engage visual tokens + search = "#{search_term} " + end + + filtered_search.set(search) if submit filtered_search.send_keys(:enter) diff --git a/spec/support/json_response_helpers.rb b/spec/support/json_response_helpers.rb new file mode 100644 index 00000000000..e8d2ef2d7f0 --- /dev/null +++ b/spec/support/json_response_helpers.rb @@ -0,0 +1,9 @@ +shared_context 'JSON response' do + let(:json_response) { JSON.parse(response.body) } +end + +RSpec.configure do |config| + config.include_context 'JSON response', type: :controller + config.include_context 'JSON response', type: :request + config.include_context 'JSON response', :api +end diff --git a/spec/views/projects/commit/_commit_box.html.haml_spec.rb b/spec/views/projects/commit/_commit_box.html.haml_spec.rb index e741e3cf9b6..f2919f20e85 100644 --- a/spec/views/projects/commit/_commit_box.html.haml_spec.rb +++ b/spec/views/projects/commit/_commit_box.html.haml_spec.rb @@ -3,11 +3,13 @@ require 'spec_helper' describe 'projects/commit/_commit_box.html.haml' do include Devise::Test::ControllerHelpers + let(:user) { create(:user) } let(:project) { create(:project) } before do assign(:project, project) assign(:commit, project.commit) + allow(view).to receive(:can_collaborate_with_project?).and_return(false) end it 'shows the commit SHA' do @@ -25,4 +27,30 @@ describe 'projects/commit/_commit_box.html.haml' do expect(rendered).to have_text("Pipeline ##{third_pipeline.id} for #{Commit.truncate_sha(project.commit.sha)} failed") end + + context 'viewing a commit' do + context 'as a developer' do + before do + expect(view).to receive(:can_collaborate_with_project?).and_return(true) + end + + it 'has a link to create a new tag' do + render + + expect(rendered).to have_link('Tag') + end + end + + context 'as a non-developer' do + before do + expect(view).to receive(:can_collaborate_with_project?).and_return(false) + end + + it 'does not have a link to create a new tag' do + render + + expect(rendered).not_to have_link('Tag') + end + end + end end diff --git a/spec/workers/authorized_projects_worker_spec.rb b/spec/workers/authorized_projects_worker_spec.rb index 97c4bfcd248..bd5cc651c2b 100644 --- a/spec/workers/authorized_projects_worker_spec.rb +++ b/spec/workers/authorized_projects_worker_spec.rb @@ -1,12 +1,10 @@ require 'spec_helper' describe AuthorizedProjectsWorker do - let(:worker) { described_class.new } + let(:project) { create(:empty_project) } describe '.bulk_perform_and_wait' do it 'schedules the ids and waits for the jobs to complete' do - project = create(:project) - project.owner.project_authorizations.delete_all described_class.bulk_perform_and_wait([[project.owner.id]]) @@ -15,20 +13,37 @@ describe AuthorizedProjectsWorker do end end + describe '.bulk_perform_async' do + it "uses it's respective sidekiq queue" do + args = [[project.owner.id]] + push_bulk_args = { + 'class' => described_class, + 'queue' => described_class.sidekiq_options['queue'], + 'args' => args + } + + expect(Sidekiq::Client).to receive(:push_bulk).with(push_bulk_args).once + + described_class.bulk_perform_async(args) + end + end + describe '#perform' do + subject { described_class.new } + it "refreshes user's authorized projects" do user = create(:user) expect_any_instance_of(User).to receive(:refresh_authorized_projects) - worker.perform(user.id) + subject.perform(user.id) end context "when the user is not found" do it "does nothing" do expect_any_instance_of(User).not_to receive(:refresh_authorized_projects) - described_class.new.perform(-1) + subject.perform(-1) end end end |