diff options
author | Stan Hu <stanhu@gmail.com> | 2017-08-31 20:50:05 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2017-08-31 20:50:05 -0700 |
commit | f045903541ace5cf4fd3c6e4a05ecfd264c1c621 (patch) | |
tree | ee6aaf013d92766e7184b3e0772e1e5fa99a20e4 /spec | |
parent | f2c60eba25fc001974a61373bc380528416932a2 (diff) | |
parent | 8713afe61fb1beaff4d550a60b88d274c47006ea (diff) | |
download | gitlab-ce-f045903541ace5cf4fd3c6e4a05ecfd264c1c621.tar.gz |
Merge branch 'master' into sh-headless-chrome-support
Diffstat (limited to 'spec')
184 files changed, 3179 insertions, 1014 deletions
diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 3d21b695af4..aadd3317875 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -150,6 +150,18 @@ describe Admin::UsersController do post :update, params end + context 'when the admin changes his own password' do + it 'updates the password' do + expect { update_password(admin, 'AValidPassword1') } + .to change { admin.reload.encrypted_password } + end + + it 'does not set the new password to expire immediately' do + expect { update_password(admin, 'AValidPassword1') } + .not_to change { admin.reload.password_expires_at } + end + end + context 'when the new password is valid' do it 'redirects to the user' do update_password(user, 'AValidPassword1') @@ -158,15 +170,13 @@ describe Admin::UsersController do end it 'updates the password' do - update_password(user, 'AValidPassword1') - - expect { user.reload }.to change { user.encrypted_password } + expect { update_password(user, 'AValidPassword1') } + .to change { user.reload.encrypted_password } end it 'sets the new password to expire immediately' do - update_password(user, 'AValidPassword1') - - expect { user.reload }.to change { user.password_expires_at }.to(a_value <= Time.now) + expect { update_password(user, 'AValidPassword1') } + .to change { user.reload.password_expires_at }.to be_within(2.seconds).of(Time.now) end end @@ -184,9 +194,8 @@ describe Admin::UsersController do end it 'does not update the password' do - update_password(user, 'invalid') - - expect { user.reload }.not_to change { user.encrypted_password } + expect { update_password(user, 'invalid') } + .not_to change { user.reload.encrypted_password } end end @@ -204,9 +213,8 @@ describe Admin::UsersController do end it 'does not update the password' do - update_password(user, 'AValidPassword1', 'AValidPassword2') - - expect { user.reload }.not_to change { user.encrypted_password } + expect { update_password(user, 'AValidPassword1', 'AValidPassword2') } + .not_to change { user.reload.encrypted_password } end end end diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 2fbab1e4040..572b567cddf 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -339,4 +339,42 @@ describe AutocompleteController do end end end + + context 'GET award_emojis' do + let(:user2) { create(:user) } + let!(:award_emoji1) { create_list(:award_emoji, 2, user: user, name: 'thumbsup') } + let!(:award_emoji2) { create_list(:award_emoji, 1, user: user, name: 'thumbsdown') } + let!(:award_emoji3) { create_list(:award_emoji, 3, user: user, name: 'star') } + let!(:award_emoji4) { create_list(:award_emoji, 1, user: user, name: 'tea') } + + context 'unauthorized user' do + it 'returns empty json' do + get :award_emojis + + expect(json_response).to be_empty + end + end + + context 'sign in as user without award emoji' do + it 'returns empty json' do + sign_in(user2) + get :award_emojis + + expect(json_response).to be_empty + end + end + + context 'sign in as user with award emoji' do + it 'returns json sorted by name count' do + sign_in(user) + get :award_emojis + + expect(json_response.count).to eq 4 + expect(json_response[0]).to match('name' => 'star') + expect(json_response[1]).to match('name' => 'thumbsup') + expect(json_response[2]).to match('name' => 'tea') + expect(json_response[3]).to match('name' => 'thumbsdown') + end + end + end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index b571b11dcac..da8f9e8376e 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -268,7 +268,7 @@ describe Projects::IssuesController do context 'when an issue is not identified as spam' do before do allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(false) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) end it 'normally updates the issue' do @@ -278,7 +278,7 @@ describe Projects::IssuesController do context 'when an issue is identified as spam' do before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when captcha is not verified' do @@ -672,7 +672,7 @@ describe Projects::IssuesController do context 'when an issue is not identified as spam' do before do allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(false) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) end it 'does not create an issue' do @@ -682,7 +682,7 @@ describe Projects::IssuesController do context 'when an issue is identified as spam' do before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when captcha is not verified' do diff --git a/spec/controllers/projects/pages_controller_spec.rb b/spec/controllers/projects/pages_controller_spec.rb index 4d0111302f3..83c7744a231 100644 --- a/spec/controllers/projects/pages_controller_spec.rb +++ b/spec/controllers/projects/pages_controller_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Projects::PagesController do let(:user) { create(:user) } - let(:project) { create(:project, :public, :access_requestable) } + let(:project) { create(:project, :public) } let(:request_params) do { @@ -23,6 +23,17 @@ describe Projects::PagesController do expect(response).to have_http_status(200) end + + context 'when the project is in a subgroup' do + let(:group) { create(:group, :nested) } + let(:project) { create(:project, namespace: group) } + + it 'returns a 404 status code' do + get :show, request_params + + expect(response).to have_http_status(404) + end + end end describe 'DELETE destroy' do diff --git a/spec/controllers/projects/protected_tags_controller_spec.rb b/spec/controllers/projects/protected_tags_controller_spec.rb index 64658988b3f..b6de90039f3 100644 --- a/spec/controllers/projects/protected_tags_controller_spec.rb +++ b/spec/controllers/projects/protected_tags_controller_spec.rb @@ -8,4 +8,21 @@ describe Projects::ProtectedTagsController do get(:index, namespace_id: project.namespace.to_param, project_id: project) end end + + describe "DELETE #destroy" do + let(:project) { create(:project, :repository) } + let(:protected_tag) { create(:protected_tag, :developers_can_create, project: project) } + let(:user) { create(:user) } + + before do + project.add_master(user) + sign_in(user) + end + + it "deletes the protected tag" do + delete(:destroy, namespace_id: project.namespace.to_param, project_id: project, id: protected_tag.id) + + expect { ProtectedTag.find(protected_tag.id) }.to raise_error(ActiveRecord::RecordNotFound) + end + end end diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index cc444f31797..3a1550aa730 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -98,7 +98,7 @@ describe Projects::SnippetsController do context 'when the snippet is spam' do before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do @@ -176,7 +176,7 @@ describe Projects::SnippetsController do context 'when the snippet is spam' do before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index c0e48046937..4459e227fb3 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -7,6 +7,38 @@ describe ProjectsController do let(:jpg) { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') } let(:txt) { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') } + describe 'GET new' do + context 'with an authenticated user' do + let(:group) { create(:group) } + + before do + sign_in(user) + end + + context 'when namespace_id param is present' do + context 'when user has access to the namespace' do + it 'renders the template' do + group.add_owner(user) + + get :new, namespace_id: group.id + + expect(response).to have_http_status(200) + expect(response).to render_template('new') + end + end + + context 'when user does not have access to the namespace' do + it 'responds with status 404' do + get :new, namespace_id: group.id + + expect(response).to have_http_status(404) + expect(response).not_to render_template('new') + end + end + end + end + end + describe 'GET index' do context 'as a user' do it 'redirects to root page' do diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 7c5d059760f..be273acb69b 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -217,7 +217,7 @@ describe SnippetsController do context 'when the snippet is spam' do before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do @@ -289,7 +289,7 @@ describe SnippetsController do context 'when the snippet is spam' do before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do diff --git a/spec/factories/ci/triggers.rb b/spec/factories/ci/triggers.rb index 40c4663c6d8..3734c7040c0 100644 --- a/spec/factories/ci/triggers.rb +++ b/spec/factories/ci/triggers.rb @@ -1,5 +1,7 @@ FactoryGirl.define do factory :ci_trigger_without_token, class: Ci::Trigger do + owner + factory :ci_trigger do sequence(:token) { |n| "token#{n}" } end diff --git a/spec/features/admin/admin_projects_spec.rb b/spec/features/admin/admin_projects_spec.rb index 77710f80036..f4f2505d436 100644 --- a/spec/features/admin/admin_projects_spec.rb +++ b/spec/features/admin/admin_projects_spec.rb @@ -36,6 +36,14 @@ describe "Admin::Projects" do expect(page).to have_content(archived_project.name) expect(page).to have_xpath("//span[@class='label label-warning']", text: 'archived') end + + it 'renders only archived projects', js: true do + find(:css, '#sort-projects-dropdown').click + click_link 'Show archived projects only' + + expect(page).to have_content(archived_project.name) + expect(page).not_to have_content(project.name) + end end describe "GET /admin/projects/:namespace_id/:id" do diff --git a/spec/features/calendar_spec.rb b/spec/features/calendar_spec.rb index 9a597a2d690..4fc6956d111 100644 --- a/spec/features/calendar_spec.rb +++ b/spec/features/calendar_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' feature 'Contributions Calendar', :js do let(:user) { create(:user) } - let(:contributed_project) { create(:project, :public) } + let(:contributed_project) { create(:project, :public, :repository) } let(:issue_note) { create(:note, project: contributed_project) } # Ex/ Sunday Jan 1, 2016 diff --git a/spec/features/dashboard/activity_spec.rb b/spec/features/dashboard/activity_spec.rb index 582868bac1e..bd115785646 100644 --- a/spec/features/dashboard/activity_spec.rb +++ b/spec/features/dashboard/activity_spec.rb @@ -17,7 +17,7 @@ feature 'Dashboard > Activity' do end context 'event filters', :js do - let(:project) { create(:project) } + let(:project) { create(:project, :repository) } let(:merge_request) do create(:merge_request, author: user, source_project: project, target_project: project) diff --git a/spec/features/dashboard/archived_projects_spec.rb b/spec/features/dashboard/archived_projects_spec.rb index 814ec0e59c7..e8d699ff5e0 100644 --- a/spec/features/dashboard/archived_projects_spec.rb +++ b/spec/features/dashboard/archived_projects_spec.rb @@ -26,6 +26,13 @@ RSpec.describe 'Dashboard Archived Project' do expect(page).to have_link(archived_project.name) end + it 'renders only archived projects' do + click_link 'Show archived projects only' + + expect(page).to have_content(archived_project.name) + expect(page).not_to have_content(project.name) + end + it 'searchs archived projects', :js do click_button 'Last updated' click_link 'Show archived projects' diff --git a/spec/features/groups/merge_requests_spec.rb b/spec/features/groups/merge_requests_spec.rb index c2241feb9f7..9ba9f5686f7 100644 --- a/spec/features/groups/merge_requests_spec.rb +++ b/spec/features/groups/merge_requests_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' feature 'Group merge requests page' do + include FilteredSearchHelpers + let(:path) { merge_requests_group_path(group) } let(:issuable) { create(:merge_request, source_project: project, target_project: project, title: 'this is my created issuable') } @@ -33,4 +35,17 @@ feature 'Group merge requests page' do expect(page.find('#state-all span.badge').text).to eq("1") end end + + context 'group filtered search', :js do + let(:access_level) { ProjectFeature::ENABLED } + let(:user) { user_in_group } + let(:user2) { user_outside_group } + + it 'filters by assignee only group users' do + filtered_search.set('assignee:') + + expect(find('#js-dropdown-assignee .filter-dropdown')).to have_content(user.name) + expect(find('#js-dropdown-assignee .filter-dropdown')).not_to have_content(user2.name) + end + end end diff --git a/spec/features/issues/filtered_search/dropdown_assignee_spec.rb b/spec/features/issues/filtered_search/dropdown_assignee_spec.rb index 2cc027aac9e..1c4649d0ba9 100644 --- a/spec/features/issues/filtered_search/dropdown_assignee_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_assignee_spec.rb @@ -204,6 +204,12 @@ describe 'Dropdown assignee', :js do expect(page).to have_css(js_dropdown_assignee, visible: true) end + + it 'opens assignee dropdown with existing my-reaction' do + filtered_search.set('my-reaction:star assignee:') + + expect(page).to have_css(js_dropdown_assignee, visible: true) + end end describe 'caching requests' do diff --git a/spec/features/issues/filtered_search/dropdown_author_spec.rb b/spec/features/issues/filtered_search/dropdown_author_spec.rb index 975dc035f2d..3cec59050ab 100644 --- a/spec/features/issues/filtered_search/dropdown_author_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_author_spec.rb @@ -6,7 +6,7 @@ describe 'Dropdown author', js: true do let!(:project) { create(:project) } let!(:user) { create(:user, name: 'administrator', username: 'root') } let!(:user_john) { create(:user, name: 'John', username: 'th0mas') } - let!(:user_jacob) { create(:user, name: 'Jacob', username: 'otter32') } + let!(:user_jacob) { create(:user, name: 'Jacob', username: 'ooter32') } let(:filtered_search) { find('.filtered-search') } let(:js_dropdown_author) { '#js-dropdown-author' } @@ -82,31 +82,31 @@ describe 'Dropdown author', js: true do end it 'filters by name' do - send_keys_to_filtered_search('ja') + send_keys_to_filtered_search('jac') expect(dropdown_author_size).to eq(1) end it 'filters by case insensitive name' do - send_keys_to_filtered_search('Ja') + send_keys_to_filtered_search('Jac') expect(dropdown_author_size).to eq(1) end it 'filters by username with symbol' do - send_keys_to_filtered_search('@ot') + send_keys_to_filtered_search('@oot') expect(dropdown_author_size).to eq(2) end it 'filters by username without symbol' do - send_keys_to_filtered_search('ot') + send_keys_to_filtered_search('oot') expect(dropdown_author_size).to eq(2) end it 'filters by case insensitive username without symbol' do - send_keys_to_filtered_search('OT') + send_keys_to_filtered_search('OOT') expect(dropdown_author_size).to eq(2) end diff --git a/spec/features/issues/filtered_search/dropdown_emoji_spec.rb b/spec/features/issues/filtered_search/dropdown_emoji_spec.rb new file mode 100644 index 00000000000..44741bcc92d --- /dev/null +++ b/spec/features/issues/filtered_search/dropdown_emoji_spec.rb @@ -0,0 +1,182 @@ +require 'rails_helper' + +describe 'Dropdown emoji', js: true do + include FilteredSearchHelpers + + let!(:project) { create(:project, :public) } + let!(:user) { create(:user, name: 'administrator', username: 'root') } + let!(:issue) { create(:issue, project: project) } + let!(:award_emoji_star) { create(:award_emoji, name: 'star', user: user, awardable: issue) } + let(:filtered_search) { find('.filtered-search') } + let(:js_dropdown_emoji) { '#js-dropdown-my-reaction' } + + def send_keys_to_filtered_search(input) + input.split("").each do |i| + filtered_search.send_keys(i) + end + + sleep 0.5 + wait_for_requests + end + + def dropdown_emoji_size + page.all('#js-dropdown-my-reaction .filter-dropdown .filter-dropdown-item').size + end + + def click_emoji(text) + find('#js-dropdown-my-reaction .filter-dropdown .filter-dropdown-item', text: text).click + end + + before do + project.team << [user, :master] + create_list(:award_emoji, 2, user: user, name: 'thumbsup') + create_list(:award_emoji, 1, user: user, name: 'thumbsdown') + create_list(:award_emoji, 3, user: user, name: 'star') + create_list(:award_emoji, 1, user: user, name: 'tea') + end + + context 'when user not logged in' do + before do + visit project_issues_path(project) + end + + describe 'behavior' do + it 'does not open when the search bar has my-reaction:' do + filtered_search.set('my-reaction:') + + expect(page).not_to have_css(js_dropdown_emoji) + end + end + end + + context 'when user loggged in' do + before do + sign_in(user) + + visit project_issues_path(project) + end + + describe 'behavior' do + it 'opens when the search bar has my-reaction:' do + filtered_search.set('my-reaction:') + + expect(page).to have_css(js_dropdown_emoji, visible: true) + end + + it 'closes when the search bar is unfocused' do + find('body').click() + + expect(page).to have_css(js_dropdown_emoji, visible: false) + end + + it 'should show loading indicator when opened' do + filtered_search.set('my-reaction:') + + expect(page).to have_css('#js-dropdown-my-reaction .filter-dropdown-loading', visible: true) + end + + it 'should hide loading indicator when loaded' do + send_keys_to_filtered_search('my-reaction:') + + expect(page).not_to have_css('#js-dropdown-my-reaction .filter-dropdown-loading') + end + + it 'should load all the emojis when opened' do + send_keys_to_filtered_search('my-reaction:') + + expect(dropdown_emoji_size).to eq(4) + end + + it 'shows the most populated emoji at top of dropdown' do + send_keys_to_filtered_search('my-reaction:') + + expect(first('#js-dropdown-my-reaction li')).to have_content(award_emoji_star.name) + end + end + + describe 'filtering' do + before do + filtered_search.set('my-reaction') + send_keys_to_filtered_search(':') + end + + it 'filters by name' do + send_keys_to_filtered_search('up') + + expect(dropdown_emoji_size).to eq(1) + end + + it 'filters by case insensitive name' do + send_keys_to_filtered_search('Up') + + expect(dropdown_emoji_size).to eq(1) + end + end + + describe 'selecting from dropdown' do + before do + filtered_search.set('my-reaction') + send_keys_to_filtered_search(':') + end + + it 'fills in the my-reaction name' do + click_emoji('thumbsup') + + wait_for_requests + + expect(page).to have_css(js_dropdown_emoji, visible: false) + expect_tokens([emoji_token('thumbsup')]) + expect_filtered_search_input_empty + end + end + + describe 'input has existing content' do + it 'opens my-reaction dropdown with existing search term' do + filtered_search.set('searchTerm my-reaction:') + + expect(page).to have_css(js_dropdown_emoji, visible: true) + end + + it 'opens my-reaction dropdown with existing assignee' do + filtered_search.set('assignee:@user my-reaction:') + + expect(page).to have_css(js_dropdown_emoji, visible: true) + end + + it 'opens my-reaction dropdown with existing label' do + filtered_search.set('label:~bug my-reaction:') + + expect(page).to have_css(js_dropdown_emoji, visible: true) + end + + it 'opens my-reaction dropdown with existing milestone' do + filtered_search.set('milestone:%v1.0 my-reaction:') + + expect(page).to have_css(js_dropdown_emoji, visible: true) + end + + it 'opens my-reaction dropdown with existing my-reaction' do + filtered_search.set('my-reaction:star my-reaction:') + + expect(page).to have_css(js_dropdown_emoji, visible: true) + end + end + + describe 'caching requests' do + it 'caches requests after the first load' do + filtered_search.set('my-reaction') + send_keys_to_filtered_search(':') + initial_size = dropdown_emoji_size + + expect(initial_size).to be > 0 + + create_list(:award_emoji, 1, user: user, name: 'smile') + find('.filtered-search-box .clear-search').click + filtered_search.set('my-reaction') + send_keys_to_filtered_search(':') + + expect(dropdown_emoji_size).to eq(initial_size) + end + end + end +end diff --git a/spec/features/issues/filtered_search/dropdown_hint_spec.rb b/spec/features/issues/filtered_search/dropdown_hint_spec.rb index 04d6dea4b8c..0183495a1db 100644 --- a/spec/features/issues/filtered_search/dropdown_hint_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_hint_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' describe 'Dropdown hint', :js do include FilteredSearchHelpers - let!(:project) { create(:project) } + let!(:project) { create(:project, :public) } let!(:user) { create(:user) } let(:filtered_search) { find('.filtered-search') } let(:js_dropdown_hint) { '#js-dropdown-hint' } @@ -14,165 +14,209 @@ describe 'Dropdown hint', :js do before do project.team << [user, :master] - sign_in(user) create(:issue, project: project) - - visit project_issues_path(project) end - describe 'behavior' do + context 'when user not logged in' do before do - expect(page).to have_css(js_dropdown_hint, visible: false) - filtered_search.click + visit project_issues_path(project) end - it 'opens when the search bar is first focused' do - expect(page).to have_css(js_dropdown_hint, visible: true) - end - - it 'closes when the search bar is unfocused' do - find('body').click - + it 'does not exist my-reaction dropdown item' do expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).not_to have_content('my-reaction') end end - describe 'filtering' do - it 'does not filter `Press Enter or click to search`' do - filtered_search.set('randomtext') - - hint_dropdown = find(js_dropdown_hint) + context 'when user logged in' do + before do + sign_in(user) - expect(hint_dropdown).to have_content('Press Enter or click to search') - expect(hint_dropdown).to have_selector('.filter-dropdown .filter-dropdown-item', count: 0) + visit project_issues_path(project) end - it 'filters with text' do - filtered_search.set('a') + describe 'behavior' do + before do + expect(page).to have_css(js_dropdown_hint, visible: false) + filtered_search.click + end - expect(find(js_dropdown_hint)).to have_selector('.filter-dropdown .filter-dropdown-item', count: 3) - end - end + it 'opens when the search bar is first focused' do + expect(page).to have_css(js_dropdown_hint, visible: true) + end - describe 'selecting from dropdown with no input' do - before do - filtered_search.click - end + it 'closes when the search bar is unfocused' do + find('body').click - it 'opens the author dropdown when you click on author' do - click_hint('author') - - expect(page).to have_css(js_dropdown_hint, visible: false) - expect(page).to have_css('#js-dropdown-author', visible: true) - expect_tokens([{ name: 'author' }]) - expect_filtered_search_input_empty + expect(page).to have_css(js_dropdown_hint, visible: false) + end end - it 'opens the assignee dropdown when you click on assignee' do - click_hint('assignee') + describe 'filtering' do + it 'does not filter `Press Enter or click to search`' do + filtered_search.set('randomtext') - expect(page).to have_css(js_dropdown_hint, visible: false) - expect(page).to have_css('#js-dropdown-assignee', visible: true) - expect_tokens([{ name: 'assignee' }]) - expect_filtered_search_input_empty - end + hint_dropdown = find(js_dropdown_hint) - it 'opens the milestone dropdown when you click on milestone' do - click_hint('milestone') + expect(hint_dropdown).to have_content('Press Enter or click to search') + expect(hint_dropdown).to have_selector('.filter-dropdown .filter-dropdown-item', count: 0) + end - expect(page).to have_css(js_dropdown_hint, visible: false) - expect(page).to have_css('#js-dropdown-milestone', visible: true) - expect_tokens([{ name: 'milestone' }]) - expect_filtered_search_input_empty - end + it 'filters with text' do + filtered_search.set('a') - it 'opens the label dropdown when you click on label' do - click_hint('label') - - expect(page).to have_css(js_dropdown_hint, visible: false) - expect(page).to have_css('#js-dropdown-label', visible: true) - expect_tokens([{ name: 'label' }]) - expect_filtered_search_input_empty + expect(find(js_dropdown_hint)).to have_selector('.filter-dropdown .filter-dropdown-item', count: 4) + end end - end - - describe 'selecting from dropdown with some input' do - it 'opens the author dropdown when you click on author' do - filtered_search.set('auth') - click_hint('author') - expect(page).to have_css(js_dropdown_hint, visible: false) - expect(page).to have_css('#js-dropdown-author', visible: true) - expect_tokens([{ name: 'author' }]) - expect_filtered_search_input_empty - end + describe 'selecting from dropdown with no input' do + before do + filtered_search.click + end - it 'opens the assignee dropdown when you click on assignee' do - filtered_search.set('assign') - click_hint('assignee') + it 'opens the author dropdown when you click on author' do + click_hint('author') - expect(page).to have_css(js_dropdown_hint, visible: false) - expect(page).to have_css('#js-dropdown-assignee', visible: true) - expect_tokens([{ name: 'assignee' }]) - expect_filtered_search_input_empty - end + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-author', visible: true) + expect_tokens([{ name: 'author' }]) + expect_filtered_search_input_empty + end - it 'opens the milestone dropdown when you click on milestone' do - filtered_search.set('mile') - click_hint('milestone') + it 'opens the assignee dropdown when you click on assignee' do + click_hint('assignee') - expect(page).to have_css(js_dropdown_hint, visible: false) - expect(page).to have_css('#js-dropdown-milestone', visible: true) - expect_tokens([{ name: 'milestone' }]) - expect_filtered_search_input_empty - end + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-assignee', visible: true) + expect_tokens([{ name: 'assignee' }]) + expect_filtered_search_input_empty + end - it 'opens the label dropdown when you click on label' do - filtered_search.set('lab') - click_hint('label') + it 'opens the milestone dropdown when you click on milestone' do + click_hint('milestone') - expect(page).to have_css(js_dropdown_hint, visible: false) - expect(page).to have_css('#js-dropdown-label', visible: true) - expect_tokens([{ name: 'label' }]) - expect_filtered_search_input_empty - end - end + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-milestone', visible: true) + expect_tokens([{ name: 'milestone' }]) + expect_filtered_search_input_empty + end - describe 'reselecting from dropdown' do - it 'reuses existing author text' do - filtered_search.send_keys('author:') - filtered_search.send_keys(:backspace) - click_hint('author') + it 'opens the label dropdown when you click on label' do + click_hint('label') - expect_tokens([{ name: 'author' }]) - expect_filtered_search_input_empty - end + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-label', visible: true) + expect_tokens([{ name: 'label' }]) + expect_filtered_search_input_empty + end - it 'reuses existing assignee text' do - filtered_search.send_keys('assignee:') - filtered_search.send_keys(:backspace) - click_hint('assignee') + it 'opens the emoji dropdown when you click on my-reaction' do + click_hint('my-reaction') - expect_tokens([{ name: 'assignee' }]) - expect_filtered_search_input_empty + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-my-reaction', visible: true) + expect_tokens([{ name: 'my-reaction' }]) + expect_filtered_search_input_empty + end end - it 'reuses existing milestone text' do - filtered_search.send_keys('milestone:') - filtered_search.send_keys(:backspace) - click_hint('milestone') - - expect_tokens([{ name: 'milestone' }]) - expect_filtered_search_input_empty - end + describe 'selecting from dropdown with some input' do + it 'opens the author dropdown when you click on author' do + filtered_search.set('auth') + click_hint('author') - it 'reuses existing label text' do - filtered_search.send_keys('label:') - filtered_search.send_keys(:backspace) - click_hint('label') + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-author', visible: true) + expect_tokens([{ name: 'author' }]) + expect_filtered_search_input_empty + end - expect_tokens([{ name: 'label' }]) - expect_filtered_search_input_empty + it 'opens the assignee dropdown when you click on assignee' do + filtered_search.set('assign') + click_hint('assignee') + + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-assignee', visible: true) + expect_tokens([{ name: 'assignee' }]) + expect_filtered_search_input_empty + end + + it 'opens the milestone dropdown when you click on milestone' do + filtered_search.set('mile') + click_hint('milestone') + + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-milestone', visible: true) + expect_tokens([{ name: 'milestone' }]) + expect_filtered_search_input_empty + end + + it 'opens the label dropdown when you click on label' do + filtered_search.set('lab') + click_hint('label') + + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-label', visible: true) + expect_tokens([{ name: 'label' }]) + expect_filtered_search_input_empty + end + + it 'opens the emoji dropdown when you click on my-reaction' do + filtered_search.set('my') + click_hint('my-reaction') + + expect(page).to have_css(js_dropdown_hint, visible: false) + expect(page).to have_css('#js-dropdown-my-reaction', visible: true) + expect_tokens([{ name: 'my-reaction' }]) + expect_filtered_search_input_empty + end + end + + describe 'reselecting from dropdown' do + it 'reuses existing author text' do + filtered_search.send_keys('author:') + filtered_search.send_keys(:backspace) + click_hint('author') + + expect_tokens([{ name: 'author' }]) + expect_filtered_search_input_empty + end + + it 'reuses existing assignee text' do + filtered_search.send_keys('assignee:') + filtered_search.send_keys(:backspace) + click_hint('assignee') + + expect_tokens([{ name: 'assignee' }]) + expect_filtered_search_input_empty + end + + it 'reuses existing milestone text' do + filtered_search.send_keys('milestone:') + filtered_search.send_keys(:backspace) + click_hint('milestone') + + expect_tokens([{ name: 'milestone' }]) + expect_filtered_search_input_empty + end + + it 'reuses existing label text' do + filtered_search.send_keys('label:') + filtered_search.send_keys(:backspace) + click_hint('label') + + expect_tokens([{ name: 'label' }]) + expect_filtered_search_input_empty + end + + it 'reuses existing emoji text' do + filtered_search.send_keys('my-reaction:') + filtered_search.send_keys(:backspace) + click_hint('my-reaction') + + expect_tokens([{ name: 'my-reaction' }]) + expect_filtered_search_input_empty + end end end end diff --git a/spec/features/issues/filtered_search/dropdown_label_spec.rb b/spec/features/issues/filtered_search/dropdown_label_spec.rb index e84b07ec2ef..c46803112a9 100644 --- a/spec/features/issues/filtered_search/dropdown_label_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_label_spec.rb @@ -270,6 +270,12 @@ describe 'Dropdown label', js: true do expect(page).to have_css(js_dropdown_label) end + + it 'opens label dropdown with existing my-reaction' do + filtered_search.set('my-reaction:star label:') + + expect(page).to have_css(js_dropdown_label) + end end describe 'caching requests' do diff --git a/spec/features/issues/filtered_search/dropdown_milestone_spec.rb b/spec/features/issues/filtered_search/dropdown_milestone_spec.rb index 5f99921ae2e..f6c2e952bea 100644 --- a/spec/features/issues/filtered_search/dropdown_milestone_spec.rb +++ b/spec/features/issues/filtered_search/dropdown_milestone_spec.rb @@ -242,6 +242,12 @@ describe 'Dropdown milestone', :js do expect(page).to have_css(js_dropdown_milestone, visible: true) end + + it 'opens milestone dropdown with existing my-reaction' do + filtered_search.set('my-reaction:star milestone:') + + expect(page).to have_css(js_dropdown_milestone, visible: true) + end end describe 'caching requests' do diff --git a/spec/features/issues/filtered_search/filter_issues_spec.rb b/spec/features/issues/filtered_search/filter_issues_spec.rb index 1029a5787b8..22e7becff1a 100644 --- a/spec/features/issues/filtered_search/filter_issues_spec.rb +++ b/spec/features/issues/filtered_search/filter_issues_spec.rb @@ -103,14 +103,6 @@ describe 'Filter issues', js: true do expect_issues_list_count(5) expect_filtered_search_input_empty end - - it 'filters issues by invalid author' do - skip('to be tested, issue #26546') - end - - it 'filters issues by multiple authors' do - skip('to be tested, issue #26546') - end end context 'author with other filters' do @@ -165,10 +157,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input(search_term) end end - - it 'sorting' do - skip('to be tested, issue #26546') - end end describe 'filter issues by assignee' do @@ -190,14 +178,6 @@ describe 'Filter issues', js: true do expect_issues_list_count(8, 1) expect_filtered_search_input_empty end - - it 'filters issues by invalid assignee' do - skip('to be tested, issue #26546') - end - - it 'filters issues by multiple assignees' do - skip('to be tested, issue #26546') - end end context 'assignee with other filters' do @@ -250,12 +230,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input(search_term) end end - - context 'sorting' do - it 'sorts' do - skip('to be tested, issue #26546') - end - end end describe 'filter issues by label' do @@ -278,10 +252,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input_empty end - it 'filters issues by invalid label' do - skip('to be tested, issue #26546') - end - it 'filters issues by multiple labels' do input_filtered_search("label:~#{bug_label.title} label:~#{caps_sensitive_label.title}") @@ -493,12 +463,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input_empty end end - - context 'sorting' do - it 'sorts' do - skip('to be tested, issue #26546') - end - end end describe 'filter issues by milestone' do @@ -535,14 +499,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input_empty end - it 'filters issues by invalid milestones' do - skip('to be tested, issue #26546') - end - - it 'filters issues by multiple milestones' do - skip('to be tested, issue #26546') - end - it 'filters issues by milestone containing special characters' do special_milestone = create(:milestone, title: '!@\#{$%^&*()}', project: project) create(:issue, title: "Issue with special character milestone", project: project, milestone: special_milestone) @@ -618,12 +574,6 @@ describe 'Filter issues', js: true do expect_filtered_search_input(search_term) end end - - context 'sorting' do - it 'sorts' do - skip('to be tested, issue #26546') - end - end end describe 'filter issues by text' do diff --git a/spec/features/issues/filtered_search/search_bar_spec.rb b/spec/features/issues/filtered_search/search_bar_spec.rb index a432d031337..d4dd570fb37 100644 --- a/spec/features/issues/filtered_search/search_bar_spec.rb +++ b/spec/features/issues/filtered_search/search_bar_spec.rb @@ -100,7 +100,7 @@ describe 'Search bar', js: true do find('.filtered-search-box .clear-search').click filtered_search.click - expect(find('#js-dropdown-hint')).to have_selector('.filter-dropdown .filter-dropdown-item', count: 4) + expect(find('#js-dropdown-hint')).to have_selector('.filter-dropdown .filter-dropdown-item', count: 5) expect(get_left_style(find('#js-dropdown-hint')['style'])).to eq(hint_offset) end end diff --git a/spec/features/issues/issue_detail_spec.rb b/spec/features/issues/issue_detail_spec.rb index 28b636f9359..c470cb7c716 100644 --- a/spec/features/issues/issue_detail_spec.rb +++ b/spec/features/issues/issue_detail_spec.rb @@ -40,4 +40,18 @@ feature 'Issue Detail', :js do end end end + + context 'when authored by a user who is later deleted' do + before do + issue.update_attribute(:author_id, nil) + sign_in(user) + visit project_issue_path(project, issue) + end + + it 'shows the issue' do + page.within('.issuable-details') do + expect(find('h2')).to have_content(issue.title) + end + end + end end diff --git a/spec/features/merge_requests/merge_commit_message_toggle_spec.rb b/spec/features/merge_requests/merge_commit_message_toggle_spec.rb index 429bc277d73..08a3bb84aac 100644 --- a/spec/features/merge_requests/merge_commit_message_toggle_spec.rb +++ b/spec/features/merge_requests/merge_commit_message_toggle_spec.rb @@ -19,7 +19,7 @@ feature 'Clicking toggle commit message link', js: true do "Merge branch 'feature' into 'master'", merge_request.title, "Closes #{issue_1.to_reference} and #{issue_2.to_reference}", - "See merge request #{merge_request.to_reference}" + "See merge request #{merge_request.to_reference(full: true)}" ].join("\n\n") end let(:message_with_description) do @@ -27,7 +27,7 @@ feature 'Clicking toggle commit message link', js: true do "Merge branch 'feature' into 'master'", merge_request.title, merge_request.description, - "See merge request #{merge_request.to_reference}" + "See merge request #{merge_request.to_reference(full: true)}" ].join("\n\n") end diff --git a/spec/features/projects/commit/mini_pipeline_graph_spec.rb b/spec/features/projects/commit/mini_pipeline_graph_spec.rb index 2ef74e8857c..807a2189cc4 100644 --- a/spec/features/projects/commit/mini_pipeline_graph_spec.rb +++ b/spec/features/projects/commit/mini_pipeline_graph_spec.rb @@ -1,13 +1,8 @@ require 'rails_helper' feature 'Mini Pipeline Graph in Commit View', :js do - let(:user) { create(:user) } let(:project) { create(:project, :public, :repository) } - before do - sign_in(user) - end - context 'when commit has pipelines' do let(:pipeline) do create(:ci_empty_pipeline, @@ -15,21 +10,14 @@ feature 'Mini Pipeline Graph in Commit View', :js do ref: project.default_branch, sha: project.commit.sha) end + let(:build) { create(:ci_build, pipeline: pipeline) } - let(:build) do - create(:ci_build, pipeline: pipeline) - end - - before do + it 'displays a mini pipeline graph' do build.run visit project_commit_path(project, project.commit.id) - end - it 'should display a mini pipeline graph' do expect(page).to have_selector('.mr-widget-pipeline-graph') - end - it 'should show the builds list when stage is clicked' do first('.mini-pipeline-graph-dropdown-toggle').click wait_for_requests @@ -38,6 +26,8 @@ feature 'Mini Pipeline Graph in Commit View', :js do expect(page).to have_selector('.ci-status-icon-running') expect(page).to have_content(build.stage) end + + build.drop end end diff --git a/spec/features/projects/commit/rss_spec.rb b/spec/features/projects/commits/rss_spec.rb index db958346f06..db958346f06 100644 --- a/spec/features/projects/commit/rss_spec.rb +++ b/spec/features/projects/commits/rss_spec.rb diff --git a/spec/features/projects/commits/user_browses_commits_spec.rb b/spec/features/projects/commits/user_browses_commits_spec.rb new file mode 100644 index 00000000000..41f3c15a94c --- /dev/null +++ b/spec/features/projects/commits/user_browses_commits_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe 'User broweses commits' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository, namespace: user.namespace) } + + before do + project.add_master(user) + sign_in(user) + end + + context 'primary email' do + it 'finds a commit by a primary email' do + user = create(:user, email: 'dmitriy.zaporozhets@gmail.com') + + visit(project_commit_path(project, RepoHelpers.sample_commit.id)) + + check_author_link(RepoHelpers.sample_commit.author_email, user) + end + end + + context 'secondary email' do + it 'finds a commit by a secondary email' do + user = + create(:user) do |user| + create(:email, { user: user, email: 'dmitriy.zaporozhets@gmail.com' }) + end + + visit(project_commit_path(project, RepoHelpers.sample_commit.parent_id)) + + check_author_link(RepoHelpers.sample_commit.author_email, user) + end + end +end + +private + +def check_author_link(email, author) + author_link = find('.commit-author-link') + + expect(author_link['href']).to eq(user_path(author)) + expect(author_link['title']).to eq(email) + expect(find('.commit-author-name').text).to eq(author.name) +end diff --git a/spec/features/projects/files/find_files_spec.rb b/spec/features/projects/files/find_files_spec.rb deleted file mode 100644 index 57d67b28920..00000000000 --- a/spec/features/projects/files/find_files_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -feature 'Find files button in the tree header' do - given(:user) { create(:user) } - given(:project) { create(:project, :repository) } - - background do - sign_in(user) - project.team << [user, :developer] - end - - scenario 'project main screen' do - visit project_path(project) - - expect(page).to have_selector('.tree-controls .shortcuts-find-file') - end - - scenario 'project tree screen' do - visit project_tree_path(project, project.default_branch) - - expect(page).to have_selector('.tree-controls .shortcuts-find-file') - end -end diff --git a/spec/features/projects/files/user_searches_for_files_spec.rb b/spec/features/projects/files/user_searches_for_files_spec.rb new file mode 100644 index 00000000000..a105685bca7 --- /dev/null +++ b/spec/features/projects/files/user_searches_for_files_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe 'User searches for files' do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + + before do + sign_in(user) + end + + describe 'project main screen' do + context 'when project is empty' do + let(:empty_project) { create(:project) } + + before do + empty_project.add_developer(user) + visit project_path(empty_project) + end + + it 'does not show any result' do + fill_in('search', with: 'coffee') + click_button('Go') + + expect(page).to have_content("We couldn't find any") + end + end + + context 'when project is not empty' do + before do + project.add_developer(user) + visit project_path(project) + end + + it 'shows "Find file" button' do + expect(page).to have_selector('.tree-controls .shortcuts-find-file') + end + end + end + + describe 'project tree screen' do + before do + project.add_developer(user) + visit project_tree_path(project, project.default_branch) + end + + it 'shows "Find file" button' do + expect(page).to have_selector('.tree-controls .shortcuts-find-file') + end + + it 'shows found files' do + fill_in('search', with: 'coffee') + click_button('Go') + + expect(page).to have_content('coffee') + expect(page).to have_content('CONTRIBUTING.md') + end + end +end diff --git a/spec/features/projects/import_export/import_file_spec.rb b/spec/features/projects/import_export/import_file_spec.rb index 9f2c86923b7..2eb6fab129d 100644 --- a/spec/features/projects/import_export/import_file_spec.rb +++ b/spec/features/projects/import_export/import_file_spec.rb @@ -99,6 +99,6 @@ feature 'Import/Export - project import integration test', js: true do end def project_hook_exists?(project) - Gitlab::Git::Hook.new('post-receive', project).exists? + Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository).exists? end end diff --git a/spec/features/projects/new_project_spec.rb b/spec/features/projects/new_project_spec.rb index 22fb1223739..cd3dc72d3c6 100644 --- a/spec/features/projects/new_project_spec.rb +++ b/spec/features/projects/new_project_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' feature 'New project' do + include Select2Helper + let(:user) { create(:admin) } before do @@ -68,26 +70,10 @@ feature 'New project' do expect(namespace.text).to eq group.name end - - context 'on validation error' do - before do - fill_in('project_path', with: 'private-group-project') - choose('Internal') - click_button('Create project') - - expect(page).to have_css '.project-edit-errors .alert.alert-danger' - end - - it 'selects the group namespace' do - namespace = find('#project_namespace_id option[selected]') - - expect(namespace.text).to eq group.name - end - end end context 'with subgroup namespace' do - let(:group) { create(:group, :private, owner: user) } + let(:group) { create(:group, owner: user) } let(:subgroup) { create(:group, parent: group) } before do @@ -101,6 +87,41 @@ feature 'New project' do expect(namespace.text).to eq subgroup.full_path end end + + context 'when changing namespaces dynamically', :js do + let(:public_group) { create(:group, :public) } + let(:internal_group) { create(:group, :internal) } + let(:private_group) { create(:group, :private) } + + before do + public_group.add_owner(user) + internal_group.add_owner(user) + private_group.add_owner(user) + visit new_project_path(namespace_id: public_group.id) + end + + it 'enables the correct visibility options' do + select2(user.namespace_id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).not_to be_disabled + + select2(public_group.id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).not_to be_disabled + + select2(internal_group.id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).to be_disabled + + select2(private_group.id, from: '#project_namespace_id') + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PRIVATE}")).not_to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::INTERNAL}")).to be_disabled + expect(find("#project_visibility_level_#{Gitlab::VisibilityLevel::PUBLIC}")).to be_disabled + end + end end context 'Import project options' do diff --git a/spec/features/projects/user_creates_files_spec.rb b/spec/features/projects/user_creates_files_spec.rb index 4b78cc4fc53..3d335687510 100644 --- a/spec/features/projects/user_creates_files_spec.rb +++ b/spec/features/projects/user_creates_files_spec.rb @@ -56,11 +56,10 @@ describe 'User creates files' do find('.add-to-tree').click click_link('New file') + expect(page).to have_selector('.file-editor') end it 'creates and commit a new file', js: true do - expect(page).to have_selector('.file-editor') - execute_script("ace.edit('editor').setValue('*.rbca')") fill_in(:file_name, with: 'not_a_file.md') fill_in(:commit_message, with: 'New commit message', visible: true) diff --git a/spec/features/projects/user_interacts_with_stars_spec.rb b/spec/features/projects/user_interacts_with_stars_spec.rb new file mode 100644 index 00000000000..0ac3f8181fa --- /dev/null +++ b/spec/features/projects/user_interacts_with_stars_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe 'User interacts with project stars' do + let(:project) { create(:project, :public, :repository) } + + context 'when user is signed in', js: true do + let(:user) { create(:user) } + + before do + sign_in(user) + visit(project_path(project)) + end + + it 'toggles the star' do + find('.star-btn').click + + expect(page).to have_css('.star-count', text: 1) + + find('.star-btn').click + + expect(page).to have_css('.star-count', text: 0) + end + end + + context 'when user is not signed in' do + before do + visit(project_path(project)) + end + + it 'does not allow to star a project' do + expect(page).not_to have_content('.toggle-star') + + find('.star-btn').click + + expect(current_path).to eq(new_user_session_path) + end + end +end diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index cac31c34ad1..785cfeb34bd 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -1,149 +1,135 @@ require 'spec_helper' -describe "Runners" do - let(:user) { create(:user) } +feature 'Runners' do + given(:user) { create(:user) } - before do + background do sign_in(user) end - describe "specific runners" do - before do - @project = FactoryGirl.create :project, shared_runners_enabled: false - @project.team << [user, :master] + context 'when a project has enabled shared_runners' do + given(:project) { create(:project) } - @project2 = FactoryGirl.create :project - @project2.team << [user, :master] + background do + project.add_master(user) + end - @project3 = FactoryGirl.create :project - @project3.team << [user, :developer] + context 'when a specific runner is activated on the project' do + given(:specific_runner) { create(:ci_runner, :specific) } - @shared_runner = FactoryGirl.create :ci_runner, :shared - @specific_runner = FactoryGirl.create :ci_runner - @specific_runner2 = FactoryGirl.create :ci_runner - @specific_runner3 = FactoryGirl.create :ci_runner - @project.runners << @specific_runner - @project2.runners << @specific_runner2 - @project3.runners << @specific_runner3 + background do + project.runners << specific_runner + end - visit runners_path(@project) - end + scenario 'user sees the specific runner' do + visit runners_path(project) - before do - expect(page).not_to have_content(@specific_runner3.display_name) - expect(page).not_to have_content(@specific_runner3.display_name) - end + within '.activated-specific-runners' do + expect(page).to have_content(specific_runner.display_name) + end - it "places runners in right places" do - expect(page.find(".available-specific-runners")).to have_content(@specific_runner2.display_name) - expect(page.find(".activated-specific-runners")).to have_content(@specific_runner.display_name) - expect(page.find(".available-shared-runners")).to have_content(@shared_runner.display_name) - end + click_on specific_runner.short_sha - it "enables specific runner for project" do - within ".available-specific-runners" do - click_on "Enable for this project" + expect(page).to have_content(specific_runner.platform) end - expect(page.find(".activated-specific-runners")).to have_content(@specific_runner2.display_name) - end + scenario 'user removes an activated specific runner if this is last project for that runners' do + visit runners_path(project) - it "disables specific runner for project" do - @project2.runners << @specific_runner - visit runners_path(@project) + within '.activated-specific-runners' do + click_on 'Remove Runner' + end - within ".activated-specific-runners" do - click_on "Disable for this project" + expect(page).not_to have_content(specific_runner.display_name) end - expect(page.find(".available-specific-runners")).to have_content(@specific_runner.display_name) - end + context 'when a runner has a tag' do + background do + specific_runner.update(tag_list: ['tag']) + end - it "removes specific runner for project if this is last project for that runners" do - within ".activated-specific-runners" do - click_on "Remove Runner" - end + scenario 'user edits runner not to run untagged jobs' do + visit runners_path(project) - expect(Ci::Runner.exists?(id: @specific_runner)).to be_falsey - end - end + within '.activated-specific-runners' do + first('.edit-runner > a').click + end - describe "shared runners" do - before do - @project = FactoryGirl.create :project, shared_runners_enabled: false - @project.team << [user, :master] - visit runners_path(@project) - end + expect(page.find_field('runner[run_untagged]')).to be_checked - it "enables shared runners" do - click_on "Enable shared Runners" - expect(@project.reload.shared_runners_enabled).to be_truthy - end - end + uncheck 'runner_run_untagged' + click_button 'Save changes' - describe "shared runners description" do - let(:shared_runners_text) { 'custom **shared** runners description' } - let(:shared_runners_html) { 'custom shared runners description' } + expect(page).to have_content 'Can run untagged jobs No' + end + end - before do - stub_application_setting(shared_runners_text: shared_runners_text) - project = FactoryGirl.create :project, shared_runners_enabled: false - project.team << [user, :master] - visit runners_path(project) - end + context 'when a shared runner is activated on the project' do + given!(:shared_runner) { create(:ci_runner, :shared) } - it "sees shared runners description" do - expect(page.find(".shared-runners-description")).to have_content(shared_runners_html) - end - end + scenario 'user sees CI/CD setting page' do + visit runners_path(project) - describe "show page" do - before do - @project = FactoryGirl.create :project - @project.team << [user, :master] - @specific_runner = FactoryGirl.create :ci_runner - @project.runners << @specific_runner + expect(page.find('.available-shared-runners')).to have_content(shared_runner.display_name) + end + end end - it "shows runner information" do - visit runners_path(@project) - click_on @specific_runner.short_sha - expect(page).to have_content(@specific_runner.platform) - end - end + context 'when a specific runner exists in another project' do + given(:another_project) { create(:project) } + given(:specific_runner) { create(:ci_runner, :specific) } - feature 'configuring runners ability to picking untagged jobs' do - given(:project) { create(:project) } - given(:runner) { create(:ci_runner) } + background do + another_project.add_master(user) + another_project.runners << specific_runner + end - background do - project.team << [user, :master] - project.runners << runner - end + scenario 'user enables and disables a specific runner' do + visit runners_path(project) + + within '.available-specific-runners' do + click_on 'Enable for this project' + end - scenario 'user checks default configuration' do - visit project_runner_path(project, runner) + expect(page.find('.activated-specific-runners')).to have_content(specific_runner.display_name) - expect(page).to have_content 'Can run untagged jobs Yes' + within '.activated-specific-runners' do + click_on 'Disable for this project' + end + + expect(page.find('.available-specific-runners')).to have_content(specific_runner.display_name) + end end - context 'when runner has tags' do - before do - runner.update_attribute(:tag_list, ['tag']) + context 'when application settings have shared_runners_text' do + given(:shared_runners_text) { 'custom **shared** runners description' } + given(:shared_runners_html) { 'custom shared runners description' } + + background do + stub_application_setting(shared_runners_text: shared_runners_text) end - scenario 'user wants to prevent runner from running untagged job' do + scenario 'user sees shared runners description' do visit runners_path(project) - page.within('.activated-specific-runners') do - first('small > a').click - end - uncheck 'runner_run_untagged' - click_button 'Save changes' - - expect(page).to have_content 'Can run untagged jobs No' - expect(runner.reload.run_untagged?).to eq false + expect(page.find('.shared-runners-description')).to have_content(shared_runners_html) end end end + + context 'when a project has disabled shared_runners' do + given(:project) { create(:project, shared_runners_enabled: false) } + + background do + project.add_master(user) + end + + scenario 'user enables shared runners' do + visit runners_path(project) + + click_on 'Enable shared Runners' + + expect(page.find('.shared-runners-description')).to have_content('Disable shared Runners') + end + end end diff --git a/spec/features/search_spec.rb b/spec/features/search_spec.rb index c1c8db5155a..979e36e7e86 100644 --- a/spec/features/search_spec.rb +++ b/spec/features/search_spec.rb @@ -281,4 +281,30 @@ describe "Search" do expect(page).to have_selector('.commit-row-description', count: 9) end end + + context 'anonymous user' do + let(:project) { create(:project, :public) } + + before do + sign_out(user) + end + + it 'preserves the group being searched in' do + visit search_path(group_id: project.namespace.id) + + fill_in 'search', with: 'foo' + click_button 'Search' + + expect(find('#group_id').value).to eq(project.namespace.id.to_s) + end + + it 'preserves the project being searched in' do + visit search_path(project_id: project.id) + + fill_in 'search', with: 'foo' + click_button 'Search' + + expect(find('#project_id').value).to eq(project.id.to_s) + end + end end diff --git a/spec/features/snippets/notes_on_personal_snippets_spec.rb b/spec/features/snippets/notes_on_personal_snippets_spec.rb index d089fb5b730..e842af39376 100644 --- a/spec/features/snippets/notes_on_personal_snippets_spec.rb +++ b/spec/features/snippets/notes_on_personal_snippets_spec.rb @@ -88,7 +88,7 @@ describe 'Comments on personal snippets', :js do context 'when editing a note' do it 'changes the text' do - find('.js-note-edit').click + find('.js-note-edit').trigger('click') page.within('.current-note-edit-form') do fill_in 'note[note]', with: 'new content' diff --git a/spec/features/tags/master_deletes_tag_spec.rb b/spec/features/tags/master_deletes_tag_spec.rb index 4d6fc13557f..d6a6b8fc7d5 100644 --- a/spec/features/tags/master_deletes_tag_spec.rb +++ b/spec/features/tags/master_deletes_tag_spec.rb @@ -36,8 +36,8 @@ feature 'Master deletes tag' do 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') + allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) + .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'Do not delete tags') end scenario 'shows the error message' do diff --git a/spec/finders/admin/projects_finder_spec.rb b/spec/finders/admin/projects_finder_spec.rb index 28e36330029..4b67203a0df 100644 --- a/spec/finders/admin/projects_finder_spec.rb +++ b/spec/finders/admin/projects_finder_spec.rb @@ -118,6 +118,12 @@ describe Admin::ProjectsFinder do it { is_expected.to match_array([archived_project, shared_project, public_project, internal_project, private_project]) } end + + context 'archived=only' do + let(:params) { { archived: 'only' } } + + it { is_expected.to eq([archived_project]) } + end end context 'filter by personal' do diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 8769a52863c..0e80df94e18 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -10,6 +10,9 @@ describe IssuesFinder do set(:issue1) { create(:issue, author: user, assignees: [user], project: project1, milestone: milestone, title: 'gitlab', created_at: 1.week.ago) } set(:issue2) { create(:issue, author: user, assignees: [user], project: project2, description: 'gitlab') } set(:issue3) { create(:issue, author: user2, assignees: [user2], project: project2, title: 'tanuki', description: 'tanuki', created_at: 1.week.from_now) } + set(:award_emoji1) { create(:award_emoji, name: 'thumbsup', user: user, awardable: issue1) } + set(:award_emoji2) { create(:award_emoji, name: 'thumbsup', user: user2, awardable: issue2) } + set(:award_emoji3) { create(:award_emoji, name: 'thumbsdown', user: user, awardable: issue3) } describe '#execute' do set(:closed_issue) { create(:issue, author: user2, assignees: [user2], project: project2, state: 'closed') } @@ -26,6 +29,10 @@ describe IssuesFinder do issue1 issue2 issue3 + + award_emoji1 + award_emoji2 + award_emoji3 end context 'scope: all' do @@ -250,6 +257,34 @@ describe IssuesFinder do end end + context 'filtering by reaction name' do + context 'user searches by "thumbsup" reaction' do + let(:params) { { my_reaction_emoji: 'thumbsup' } } + + it 'returns issues that the user thumbsup to' do + expect(issues).to contain_exactly(issue1) + end + end + + context 'user2 searches by "thumbsup" reaction' do + let(:search_user) { user2 } + + let(:params) { { my_reaction_emoji: 'thumbsup' } } + + it 'returns issues that the user2 thumbsup to' do + expect(issues).to contain_exactly(issue2) + end + end + + context 'user searches by "thumbsdown" reaction' do + let(:params) { { my_reaction_emoji: 'thumbsdown' } } + + it 'returns issues that the user thumbsdown to' do + expect(issues).to contain_exactly(issue3) + end + end + end + context 'when the user is unauthorized' do let(:search_user) { nil } diff --git a/spec/finders/projects_finder_spec.rb b/spec/finders/projects_finder_spec.rb index a5de586e869..0dfe6ba9c32 100644 --- a/spec/finders/projects_finder_spec.rb +++ b/spec/finders/projects_finder_spec.rb @@ -123,6 +123,12 @@ describe ProjectsFinder do it { is_expected.to match_array([public_project, internal_project, archived_project]) } end + describe 'filter by archived only' do + let(:params) { { archived: 'only' } } + + it { is_expected.to eq([archived_project]) } + end + describe 'filter by archived for backward compatibility' do let(:params) { { archived: false } } diff --git a/spec/fixtures/api/schemas/public_api/v4/issues.json b/spec/fixtures/api/schemas/public_api/v4/issues.json index bd6bfc03199..8acd9488215 100644 --- a/spec/fixtures/api/schemas/public_api/v4/issues.json +++ b/spec/fixtures/api/schemas/public_api/v4/issues.json @@ -78,7 +78,13 @@ "downvotes": { "type": "integer" }, "due_date": { "type": ["date", "null"] }, "confidential": { "type": "boolean" }, - "web_url": { "type": "uri" } + "web_url": { "type": "uri" }, + "time_stats": { + "time_estimate": { "type": "integer" }, + "total_time_spent": { "type": "integer" }, + "human_time_estimate": { "type": ["string", "null"] }, + "human_total_time_spent": { "type": ["string", "null"] } + } }, "required": [ "id", "iid", "project_id", "title", "description", diff --git a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json index 60aa47c1259..31b3f4ba946 100644 --- a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json +++ b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json @@ -72,7 +72,13 @@ "user_notes_count": { "type": "integer" }, "should_remove_source_branch": { "type": ["boolean", "null"] }, "force_remove_source_branch": { "type": ["boolean", "null"] }, - "web_url": { "type": "uri" } + "web_url": { "type": "uri" }, + "time_stats": { + "time_estimate": { "type": "integer" }, + "total_time_spent": { "type": "integer" }, + "human_time_estimate": { "type": ["string", "null"] }, + "human_total_time_spent": { "type": ["string", "null"] } + } }, "required": [ "id", "iid", "project_id", "title", "description", diff --git a/spec/fixtures/emails/ios_default.eml b/spec/fixtures/emails/ios_default.eml index 8d4d58feb16..fa19475104a 100644 --- a/spec/fixtures/emails/ios_default.eml +++ b/spec/fixtures/emails/ios_default.eml @@ -76,7 +76,7 @@ Content-Transfer-Encoding: 7bit <img src="https://meta-discourse.global.ssl.fastly.net/user_avatar/meta.discourse.org/techapj/45/3281.png" title="techAPJ" style="max-width:100%;" width="45" height="45"> </td> <td> - <a href="https://meta.discourse.org/users/techapj" target="_blank" style="text-decoration: none; font-weight: bold; color: #006699;; font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;color:#3b5998;text-decoration:none;font-weight:bold">techAPJ</a><br> + <a href="https://meta.discourse.org/users/techapj" target="_blank" style="text-decoration: none; font-weight: 600; color: #006699;; font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;color:#3b5998;text-decoration:none;font-weight:bold">techAPJ</a><br> <span style="text-align:right;color:#999999;padding-right:5px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;font-size:11px">November 28</span> </td> </tr> @@ -94,7 +94,7 @@ Content-Transfer-Encoding: 7bit <div style="color:#666;"> - <p>To respond, reply to this email or visit <a href="https://meta.discourse.org/t/testing-default-email-replies/22638/3" style="text-decoration: none; font-weight: bold; color: #006699;; color:#666;">https://meta.discourse.org/t/testing-default-email-replies/22638/3</a> in your browser.</p> + <p>To respond, reply to this email or visit <a href="https://meta.discourse.org/t/testing-default-email-replies/22638/3" style="text-decoration: none; font-weight: 600; color: #006699;; color:#666;">https://meta.discourse.org/t/testing-default-email-replies/22638/3</a> in your browser.</p> </div> <hr style="background-color: #ddd; height: 1px; border: 1px;; background-color: #ddd; height: 1px; border: 1px;"> <h4>Previous Replies</h4> @@ -106,7 +106,7 @@ Content-Transfer-Encoding: 7bit <img src="https://meta-discourse.global.ssl.fastly.net/user_avatar/meta.discourse.org/codinghorror/45/5297.png" title="codinghorror" style="max-width:100%;" width="45" height="45"> </td> <td> - <a href="https://meta.discourse.org/users/codinghorror" target="_blank" style="text-decoration: none; font-weight: bold; color: #006699;; font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;color:#3b5998;text-decoration:none;font-weight:bold">codinghorror</a><br> + <a href="https://meta.discourse.org/users/codinghorror" target="_blank" style="text-decoration: none; font-weight: 600; color: #006699;; font-size:13px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;color:#3b5998;text-decoration:none;font-weight:bold">codinghorror</a><br> <span style="text-align:right;color:#999999;padding-right:5px;font-family:'lucida grande',tahoma,verdana,arial,sans-serif;font-size:11px">November 28</span> </td> </tr> @@ -114,7 +114,7 @@ Content-Transfer-Encoding: 7bit <td style="padding-top:5px;" colspan="2"> <p style="margin-top:0; border: 0;">We're testing the latest GitHub email processing library which we are integrating now.</p> -<p style="margin-top:0; border: 0;"><a href="https://github.com/github/email_reply_parser" target="_blank" style="text-decoration: none; font-weight: bold; color: #006699;">https://github.com/github/email_reply_parser</a></p> +<p style="margin-top:0; border: 0;"><a href="https://github.com/github/email_reply_parser" target="_blank" style="text-decoration: none; font-weight: 600; color: #006699;">https://github.com/github/email_reply_parser</a></p> <p style="margin-top:0; border: 0;">Go ahead and reply to this topic and I'll reply from various email clients for testing.</p> </td> @@ -126,10 +126,10 @@ Content-Transfer-Encoding: 7bit <hr style="background-color: #ddd; height: 1px; border: 1px;; background-color: #ddd; height: 1px; border: 1px;"> <div style="color:#666;"> -<p>To respond, reply to this email or visit <a href="https://meta.discourse.org/t/testing-default-email-replies/22638/3" style="text-decoration: none; font-weight: bold; color: #006699;; color:#666;">https://meta.discourse.org/t/testing-default-email-replies/22638/3</a> in your browser.</p> +<p>To respond, reply to this email or visit <a href="https://meta.discourse.org/t/testing-default-email-replies/22638/3" style="text-decoration: none; font-weight: 600; color: #006699;; color:#666;">https://meta.discourse.org/t/testing-default-email-replies/22638/3</a> in your browser.</p> </div> <div style="color:#666;"> -<p>To unsubscribe from these emails, visit your <a href="https://meta.discourse.org/my/preferences" style="text-decoration: none; font-weight: bold; color: #006699;; color:#666;">user preferences</a>.</p> +<p>To unsubscribe from these emails, visit your <a href="https://meta.discourse.org/my/preferences" style="text-decoration: none; font-weight: 600; color: #006699;; color:#666;">user preferences</a>.</p> </div> </div> </div></blockquote></body></html> diff --git a/spec/fixtures/emails/on_wrote.eml b/spec/fixtures/emails/on_wrote.eml index feb59bd27bb..af6a4e50a49 100644 --- a/spec/fixtures/emails/on_wrote.eml +++ b/spec/fixtures/emails/on_wrote.eml @@ -53,7 +53,7 @@ y > display: inline-block; > font-family: FontAwesome; > font-style: normal; -> font-weight: normal; +> font-weight: 400; > line-height: 1; > -webkit-font-smoothing: antialiased; > -moz-osx-font-smoothing: grayscale; @@ -227,7 +227,7 @@ ding:5px">.fa { display: inline-block; font-family: FontAwesome; font-style: normal; - font-weight: normal; + font-weight: 400; line-height: 1; -webkit-font-smoothing: antialiased; -moz-osx-font-smoothing: grayscale; @@ -274,4 +274,4 @@ ight:bold;color:#006699" target=3D"_blank">user preferences</a>.</p> </div> </blockquote></div><br></div></div> ---001a11c34c389e728f0502aa26a0--
\ No newline at end of file +--001a11c34c389e728f0502aa26a0-- diff --git a/spec/helpers/button_helper_spec.rb b/spec/helpers/button_helper_spec.rb index 250ba239033..4423560ecaa 100644 --- a/spec/helpers/button_helper_spec.rb +++ b/spec/helpers/button_helper_spec.rb @@ -62,4 +62,67 @@ describe ButtonHelper do end end end + + describe 'clipboard_button' do + let(:user) { create(:user) } + let(:project) { build_stubbed(:project) } + + def element(data = {}) + element = helper.clipboard_button(data) + Nokogiri::HTML::DocumentFragment.parse(element).first_element_child + end + + before do + allow(helper).to receive(:current_user).and_return(user) + end + + context 'with default options' do + context 'when no `text` attribute is not provided' do + it 'shows copy to clipboard button with default configuration and no text set to copy' do + expect(element.attr('class')).to eq('btn btn-clipboard btn-transparent') + expect(element.attr('type')).to eq('button') + expect(element.attr('aria-label')).to eq('Copy to clipboard') + expect(element.attr('data-toggle')).to eq('tooltip') + expect(element.attr('data-placement')).to eq('bottom') + expect(element.attr('data-container')).to eq('body') + expect(element.attr('data-clipboard-text')).to eq(nil) + expect(element.inner_text).to eq("") + + expect(element).to have_selector('.fa.fa-clipboard') + end + end + + context 'when `text` attribute is provided' do + it 'shows copy to clipboard button with provided `text` to copy' do + expect(element(text: 'Hello World!').attr('data-clipboard-text')).to eq('Hello World!') + end + end + + context 'when `title` attribute is provided' do + it 'shows copy to clipboard button with provided `title` as tooltip' do + expect(element(title: 'Copy to my clipboard!').attr('aria-label')).to eq('Copy to my clipboard!') + end + end + end + + context 'with `button_text` attribute provided' do + it 'shows copy to clipboard button with provided `button_text` as button label' do + expect(element(button_text: 'Copy text').inner_text).to eq('Copy text') + end + end + + context 'with `hide_tooltip` attribute provided' do + it 'shows copy to clipboard button without tooltip support' do + expect(element(hide_tooltip: true).attr('data-placement')).to eq(nil) + expect(element(hide_tooltip: true).attr('data-toggle')).to eq(nil) + expect(element(hide_tooltip: true).attr('data-container')).to eq(nil) + end + end + + context 'with `hide_button_icon` attribute provided' do + it 'shows copy to clipboard button without tooltip support' do + expect(element(hide_button_icon: true)).not_to have_selector('.fa.fa-clipboard') + end + end + end end diff --git a/spec/helpers/version_check_helper_spec.rb b/spec/helpers/version_check_helper_spec.rb index 5eba03ef576..fa8cfda3b86 100644 --- a/spec/helpers/version_check_helper_spec.rb +++ b/spec/helpers/version_check_helper_spec.rb @@ -4,7 +4,7 @@ describe VersionCheckHelper do describe '#version_status_badge' do it 'should return nil if not dev environment and not enabled' do allow(Rails.env).to receive(:production?) { false } - allow(current_application_settings).to receive(:version_check_enabled) { false } + allow(helper.current_application_settings).to receive(:version_check_enabled) { false } expect(helper.version_status_badge).to be(nil) end @@ -12,7 +12,7 @@ describe VersionCheckHelper do context 'when production and enabled' do before do allow(Rails.env).to receive(:production?) { true } - allow(current_application_settings).to receive(:version_check_enabled) { true } + allow(helper.current_application_settings).to receive(:version_check_enabled) { true } allow_any_instance_of(VersionCheck).to receive(:url) { 'https://version.host.com/check.svg?gitlab_info=xxx' } @image_tag = helper.version_status_badge diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index c3cccbb0d95..5077c89d7b4 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -58,35 +58,82 @@ describe VisibilityLevelHelper do end end - describe "skip_level?" do + describe "disallowed_visibility_level?" do describe "forks" do let(:project) { create(:project, :internal) } let(:fork_project) { create(:project, forked_from_project: project) } - it "skips levels" do - expect(skip_level?(fork_project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy - expect(skip_level?(fork_project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey - expect(skip_level?(fork_project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + it "disallows levels" do + expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey end end describe "non-forked project" do let(:project) { create(:project, :internal) } - it "skips levels" do - expect(skip_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey - expect(skip_level?(project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey - expect(skip_level?(project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + it "disallows levels" do + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey end end - describe "Snippet" do + describe "group" do + let(:group) { create(:group, :internal) } + + it "disallows levels" do + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + end + end + + describe "sub-group" do + let(:group) { create(:group, :private) } + let(:subgroup) { create(:group, :private, parent: group) } + + it "disallows levels" do + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::INTERNAL)).to be_truthy + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + end + end + + describe "snippet" do let(:snippet) { create(:snippet, :internal) } - it "skips levels" do - expect(skip_level?(snippet, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey - expect(skip_level?(snippet, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey - expect(skip_level?(snippet, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + it "disallows levels" do + expect(disallowed_visibility_level?(snippet, Gitlab::VisibilityLevel::PUBLIC)).to be_falsey + expect(disallowed_visibility_level?(snippet, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey + expect(disallowed_visibility_level?(snippet, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey + end + end + end + + describe "disallowed_visibility_level_description" do + let(:group) { create(:group, :internal) } + let!(:subgroup) { create(:group, :internal, parent: group) } + let!(:project) { create(:project, :internal, group: group) } + + describe "project" do + it "provides correct description for disabled levels" do + expect(disallowed_visibility_level?(project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(strip_tags disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, project)) + .to include "the visibility of #{project.group.name} is internal" + end + end + + describe "group" do + it "provides correct description for disabled levels" do + expect(disallowed_visibility_level?(group, Gitlab::VisibilityLevel::PRIVATE)).to be_truthy + expect(disallowed_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, group)) + .to include "it contains projects with higher visibility", "it contains sub-groups with higher visibility" + + expect(disallowed_visibility_level?(subgroup, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy + expect(strip_tags disallowed_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, subgroup)) + .to include "the visibility of #{group.name} is internal" end end end diff --git a/spec/javascripts/api_spec.js b/spec/javascripts/api_spec.js index 867322ce8ae..8c68ceff914 100644 --- a/spec/javascripts/api_spec.js +++ b/spec/javascripts/api_spec.js @@ -17,7 +17,7 @@ describe('Api', () => { beforeEach(() => { originalGon = window.gon; - window.gon = dummyGon; + window.gon = Object.assign({}, dummyGon); }); afterEach(() => { @@ -98,10 +98,11 @@ describe('Api', () => { }); describe('projects', () => { - it('fetches projects', (done) => { + it('fetches projects with membership when logged in', (done) => { const query = 'dummy query'; const options = { unused: 'option' }; const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects.json?simple=true`; + window.gon.current_user_id = 1; const expectedData = Object.assign({ search: query, per_page: 20, @@ -119,6 +120,27 @@ describe('Api', () => { done(); }); }); + + it('fetches projects without membership when not logged in', (done) => { + const query = 'dummy query'; + const options = { unused: 'option' }; + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects.json?simple=true`; + const expectedData = Object.assign({ + search: query, + per_page: 20, + }, options); + spyOn(jQuery, 'ajax').and.callFake((request) => { + expect(request.url).toEqual(expectedUrl); + expect(request.dataType).toEqual('json'); + expect(request.data).toEqual(expectedData); + return sendDummyResponse(); + }); + + Api.projects(query, options, (response) => { + expect(response).toBe(dummyResponse); + done(); + }); + }); }); describe('newLabel', () => { diff --git a/spec/javascripts/droplab/drop_down_spec.js b/spec/javascripts/droplab/drop_down_spec.js index 2bbcebeeac0..1ef494a00b8 100644 --- a/spec/javascripts/droplab/drop_down_spec.js +++ b/spec/javascripts/droplab/drop_down_spec.js @@ -351,14 +351,17 @@ describe('DropDown', function () { describe('render', function () { beforeEach(function () { - this.list = { querySelector: () => {} }; + this.list = { querySelector: () => {}, dispatchEvent: () => {} }; this.dropdown = { renderChildren: () => {}, list: this.list }; this.renderableList = {}; this.data = [0, 1]; + this.customEvent = {}; spyOn(this.dropdown, 'renderChildren').and.callFake(data => data); spyOn(this.list, 'querySelector').and.returnValue(this.renderableList); + spyOn(this.list, 'dispatchEvent'); spyOn(this.data, 'map').and.callThrough(); + spyOn(window, 'CustomEvent').and.returnValue(this.customEvent); DropDown.prototype.render.call(this.dropdown, this.data); }); @@ -375,6 +378,14 @@ describe('DropDown', function () { expect(this.renderableList.innerHTML).toBe('01'); }); + it('should call render.dl', function () { + expect(window.CustomEvent).toHaveBeenCalledWith('render.dl', jasmine.any(Object)); + }); + + it('should call dispatchEvent with the customEvent', function () { + expect(this.list.dispatchEvent).toHaveBeenCalledWith(this.customEvent); + }); + describe('if no data argument is passed', function () { beforeEach(function () { this.data.map.calls.reset(); @@ -394,7 +405,7 @@ describe('DropDown', function () { describe('if no dynamic list is present', function () { beforeEach(function () { - this.list = { querySelector: () => {} }; + this.list = { querySelector: () => {}, dispatchEvent: () => {} }; this.dropdown = { renderChildren: () => {}, list: this.list }; this.data = [0, 1]; diff --git a/spec/javascripts/fly_out_nav_spec.js b/spec/javascripts/fly_out_nav_spec.js index 2e81a1b056b..0847e463577 100644 --- a/spec/javascripts/fly_out_nav_spec.js +++ b/spec/javascripts/fly_out_nav_spec.js @@ -1,4 +1,3 @@ -import Cookies from 'js-cookie'; import { calculateTop, showSubLevelItems, @@ -11,6 +10,7 @@ import { getHideSubItemsInterval, documentMouseMove, getHeaderHeight, + setSidebar, } from '~/fly_out_nav'; import bp from '~/breakpoints'; @@ -113,7 +113,6 @@ describe('Fly out sidebar navigation', () => { clientX: el.getBoundingClientRect().left + 20, clientY: el.getBoundingClientRect().top + 10, }); - console.log(el); expect( getHideSubItemsInterval(), @@ -283,7 +282,7 @@ describe('Fly out sidebar navigation', () => { describe('canShowActiveSubItems', () => { afterEach(() => { - Cookies.remove('sidebar_collapsed'); + setSidebar(null); }); it('returns true by default', () => { @@ -292,36 +291,23 @@ describe('Fly out sidebar navigation', () => { ).toBeTruthy(); }); - it('returns false when cookie is false & element is active', () => { - Cookies.set('sidebar_collapsed', 'false'); + it('returns false when active & expanded sidebar', () => { + const sidebar = document.createElement('div'); el.classList.add('active'); - expect( - canShowActiveSubItems(el), - ).toBeFalsy(); - }); - - it('returns true when cookie is false & element is active', () => { - Cookies.set('sidebar_collapsed', 'true'); - el.classList.add('active'); + setSidebar(sidebar); expect( canShowActiveSubItems(el), - ).toBeTruthy(); + ).toBeFalsy(); }); - it('returns true when element is active & breakpoint is sm', () => { - breakpointSize = 'sm'; + it('returns true when active & collapsed sidebar', () => { + const sidebar = document.createElement('div'); + sidebar.classList.add('sidebar-icons-only'); el.classList.add('active'); - expect( - canShowActiveSubItems(el), - ).toBeTruthy(); - }); - - it('returns true when element is active & breakpoint is md', () => { - breakpointSize = 'md'; - el.classList.add('active'); + setSidebar(sidebar); expect( canShowActiveSubItems(el), diff --git a/spec/javascripts/helpers/vue_mount_component_helper.js b/spec/javascripts/helpers/vue_mount_component_helper.js new file mode 100644 index 00000000000..d7a2e86771c --- /dev/null +++ b/spec/javascripts/helpers/vue_mount_component_helper.js @@ -0,0 +1,4 @@ +export default (Component, props = {}) => new Component({ + propsData: props, +}).$mount(); + diff --git a/spec/javascripts/issue_show/components/edited_spec.js b/spec/javascripts/issue_show/components/edited_spec.js index a0d0750ae34..2061def699b 100644 --- a/spec/javascripts/issue_show/components/edited_spec.js +++ b/spec/javascripts/issue_show/components/edited_spec.js @@ -46,4 +46,14 @@ describe('edited', () => { expect(editedComponent.$el.querySelector('.author_link')).toBeFalsy(); expect(editedComponent.$el.querySelector('time')).toBeTruthy(); }); + + it('renders time ago tooltip at the bottom', () => { + const editedComponent = new EditedComponent({ + propsData: { + updatedAt: '2017-05-15T12:31:04.428Z', + }, + }).$mount(); + + expect(editedComponent.$el.querySelector('time').dataset.placement).toEqual('bottom'); + }); }); diff --git a/spec/javascripts/merge_request_tabs_spec.js b/spec/javascripts/merge_request_tabs_spec.js index dc40244c20e..8830a2d29e5 100644 --- a/spec/javascripts/merge_request_tabs_spec.js +++ b/spec/javascripts/merge_request_tabs_spec.js @@ -295,6 +295,17 @@ import 'vendor/jquery.scrollTo'; this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); }); + it('triggers scroll event when diff already loaded', function () { + spyOn(document, 'dispatchEvent'); + + this.class.diffsLoaded = true; + this.class.loadDiff('/foo/bar/merge_requests/1/diffs'); + + expect( + document.dispatchEvent, + ).toHaveBeenCalledWith(new CustomEvent('scroll')); + }); + describe('with inline diff', () => { let noteId; let noteLineNumId; diff --git a/spec/javascripts/monitoring/monitoring_spec.js b/spec/javascripts/monitoring/dashboard_spec.js index 6c7b691baa4..752fdfb4614 100644 --- a/spec/javascripts/monitoring/monitoring_spec.js +++ b/spec/javascripts/monitoring/dashboard_spec.js @@ -1,21 +1,21 @@ import Vue from 'vue'; -import Monitoring from '~/monitoring/components/monitoring.vue'; +import Dashboard from '~/monitoring/components/dashboard.vue'; import { MonitorMockInterceptor } from './mock_data'; -describe('Monitoring', () => { +describe('Dashboard', () => { const fixtureName = 'environments/metrics/metrics.html.raw'; - let MonitoringComponent; + let DashboardComponent; let component; preloadFixtures(fixtureName); beforeEach(() => { loadFixtures(fixtureName); - MonitoringComponent = Vue.extend(Monitoring); + DashboardComponent = Vue.extend(Dashboard); }); describe('no metrics are available yet', () => { it('shows a getting started empty state when no metrics are present', () => { - component = new MonitoringComponent({ + component = new DashboardComponent({ el: document.querySelector('#prometheus-graphs'), }); @@ -36,7 +36,7 @@ describe('Monitoring', () => { }); it('shows up a loading state', (done) => { - component = new MonitoringComponent({ + component = new DashboardComponent({ el: document.querySelector('#prometheus-graphs'), }); component.$mount(); diff --git a/spec/javascripts/monitoring/monitoring_state_spec.js b/spec/javascripts/monitoring/dashboard_state_spec.js index 4c0c558502f..e8f7042e131 100644 --- a/spec/javascripts/monitoring/monitoring_state_spec.js +++ b/spec/javascripts/monitoring/dashboard_state_spec.js @@ -1,9 +1,9 @@ import Vue from 'vue'; -import MonitoringState from '~/monitoring/components/monitoring_state.vue'; +import EmptyState from '~/monitoring/components/empty_state.vue'; import { statePaths } from './mock_data'; const createComponent = (propsData) => { - const Component = Vue.extend(MonitoringState); + const Component = Vue.extend(EmptyState); return new Component({ propsData, @@ -14,7 +14,7 @@ function getTextFromNode(component, selector) { return component.$el.querySelector(selector).firstChild.nodeValue.trim(); } -describe('MonitoringState', () => { +describe('EmptyState', () => { describe('Computed props', () => { it('currentState', () => { const component = createComponent({ diff --git a/spec/javascripts/monitoring/monitoring_deployment_spec.js b/spec/javascripts/monitoring/graph/deployment_spec.js index 5cc5b514824..c2ff38ffab9 100644 --- a/spec/javascripts/monitoring/monitoring_deployment_spec.js +++ b/spec/javascripts/monitoring/graph/deployment_spec.js @@ -1,9 +1,9 @@ import Vue from 'vue'; -import MonitoringState from '~/monitoring/components/monitoring_deployment.vue'; -import { deploymentData } from './mock_data'; +import GraphDeployment from '~/monitoring/components/graph/deployment.vue'; +import { deploymentData } from '../mock_data'; const createComponent = (propsData) => { - const Component = Vue.extend(MonitoringState); + const Component = Vue.extend(GraphDeployment); return new Component({ propsData, diff --git a/spec/javascripts/monitoring/monitoring_flag_spec.js b/spec/javascripts/monitoring/graph/flag_spec.js index 3861a95ff07..731076a7d2a 100644 --- a/spec/javascripts/monitoring/monitoring_flag_spec.js +++ b/spec/javascripts/monitoring/graph/flag_spec.js @@ -1,8 +1,8 @@ import Vue from 'vue'; -import MonitoringFlag from '~/monitoring/components/monitoring_flag.vue'; +import GraphFlag from '~/monitoring/components/graph/flag.vue'; const createComponent = (propsData) => { - const Component = Vue.extend(MonitoringFlag); + const Component = Vue.extend(GraphFlag); return new Component({ propsData, @@ -14,7 +14,7 @@ function getCoordinate(component, selector, coordinate) { return parseInt(coordinateVal, 10); } -describe('MonitoringFlag', () => { +describe('GraphFlag', () => { it('has a line and a circle located at the currentXCoordinate and currentYCoordinate', () => { const component = createComponent({ currentXCoordinate: 200, diff --git a/spec/javascripts/monitoring/monitoring_legends_spec.js b/spec/javascripts/monitoring/graph/legend_spec.js index 4c69b81e650..e877832dffd 100644 --- a/spec/javascripts/monitoring/monitoring_legends_spec.js +++ b/spec/javascripts/monitoring/graph/legend_spec.js @@ -1,9 +1,9 @@ import Vue from 'vue'; -import MonitoringLegends from '~/monitoring/components/monitoring_legends.vue'; +import GraphLegend from '~/monitoring/components/graph/legend.vue'; import measurements from '~/monitoring/utils/measurements'; const createComponent = (propsData) => { - const Component = Vue.extend(MonitoringLegends); + const Component = Vue.extend(GraphLegend); return new Component({ propsData, @@ -14,7 +14,7 @@ function getTextFromNode(component, selector) { return component.$el.querySelector(selector).firstChild.nodeValue.trim(); } -describe('MonitoringLegends', () => { +describe('GraphLegend', () => { describe('Computed props', () => { it('textTransform', () => { const component = createComponent({ diff --git a/spec/javascripts/monitoring/monitoring_row_spec.js b/spec/javascripts/monitoring/graph_row_spec.js index a82480e8342..dd485473ccf 100644 --- a/spec/javascripts/monitoring/monitoring_row_spec.js +++ b/spec/javascripts/monitoring/graph_row_spec.js @@ -1,16 +1,21 @@ import Vue from 'vue'; -import MonitoringRow from '~/monitoring/components/monitoring_row.vue'; +import GraphRow from '~/monitoring/components/graph_row.vue'; +import MonitoringMixins from '~/monitoring/mixins/monitoring_mixins'; import { deploymentData, singleRowMetrics } from './mock_data'; const createComponent = (propsData) => { - const Component = Vue.extend(MonitoringRow); + const Component = Vue.extend(GraphRow); return new Component({ propsData, }).$mount(); }; -describe('MonitoringRow', () => { +describe('GraphRow', () => { + beforeEach(() => { + spyOn(MonitoringMixins.methods, 'formatDeployments').and.returnValue({}); + }); + describe('Computed props', () => { it('bootstrapClass is set to col-md-6 when rowData is higher/equal to 2', () => { const component = createComponent({ diff --git a/spec/javascripts/monitoring/monitoring_column_spec.js b/spec/javascripts/monitoring/graph_spec.js index c423024dce0..6d6fe410113 100644 --- a/spec/javascripts/monitoring/monitoring_column_spec.js +++ b/spec/javascripts/monitoring/graph_spec.js @@ -1,39 +1,37 @@ import Vue from 'vue'; import _ from 'underscore'; -import MonitoringColumn from '~/monitoring/components/monitoring_column.vue'; +import Graph from '~/monitoring/components/graph.vue'; import MonitoringMixins from '~/monitoring/mixins/monitoring_mixins'; import eventHub from '~/monitoring/event_hub'; import { deploymentData, singleRowMetrics } from './mock_data'; const createComponent = (propsData) => { - const Component = Vue.extend(MonitoringColumn); + const Component = Vue.extend(Graph); return new Component({ propsData, }).$mount(); }; -describe('MonitoringColumn', () => { +describe('Graph', () => { beforeEach(() => { - spyOn(MonitoringMixins.methods, 'formatDeployments').and.callFake(function fakeFormat() { - return {}; - }); + spyOn(MonitoringMixins.methods, 'formatDeployments').and.returnValue({}); }); it('has a title', () => { const component = createComponent({ - columnData: singleRowMetrics[0], + graphData: singleRowMetrics[0], classType: 'col-md-6', updateAspectRatio: false, deploymentData, }); - expect(component.$el.querySelector('.text-center').innerText.trim()).toBe(component.columnData.title); + expect(component.$el.querySelector('.text-center').innerText.trim()).toBe(component.graphData.title); }); it('creates a path for the line and area of the graph', (done) => { const component = createComponent({ - columnData: singleRowMetrics[0], + graphData: singleRowMetrics[0], classType: 'col-md-6', updateAspectRatio: false, deploymentData, @@ -53,7 +51,7 @@ describe('MonitoringColumn', () => { describe('Computed props', () => { it('axisTransform translates an element Y position depending of its height', () => { const component = createComponent({ - columnData: singleRowMetrics[0], + graphData: singleRowMetrics[0], classType: 'col-md-6', updateAspectRatio: false, deploymentData, @@ -66,7 +64,7 @@ describe('MonitoringColumn', () => { it('outterViewBox gets a width and height property based on the DOM size of the element', () => { const component = createComponent({ - columnData: singleRowMetrics[0], + graphData: singleRowMetrics[0], classType: 'col-md-6', updateAspectRatio: false, deploymentData, @@ -81,7 +79,7 @@ describe('MonitoringColumn', () => { it('sends an event to the eventhub when it has finished resizing', (done) => { const component = createComponent({ - columnData: singleRowMetrics[0], + graphData: singleRowMetrics[0], classType: 'col-md-6', updateAspectRatio: false, deploymentData, @@ -97,13 +95,13 @@ describe('MonitoringColumn', () => { it('has a title for the y-axis and the chart legend that comes from the backend', () => { const component = createComponent({ - columnData: singleRowMetrics[0], + graphData: singleRowMetrics[0], classType: 'col-md-6', updateAspectRatio: false, deploymentData, }); - expect(component.yAxisLabel).toEqual(component.columnData.y_label); - expect(component.legendTitle).toEqual(component.columnData.queries[0].label); + expect(component.yAxisLabel).toEqual(component.graphData.y_label); + expect(component.legendTitle).toEqual(component.graphData.queries[0].label); }); }); diff --git a/spec/javascripts/pipelines/navigation_tabs_spec.js b/spec/javascripts/pipelines/navigation_tabs_spec.js new file mode 100644 index 00000000000..53a88e6322f --- /dev/null +++ b/spec/javascripts/pipelines/navigation_tabs_spec.js @@ -0,0 +1,127 @@ +import Vue from 'vue'; +import navigationTabs from '~/pipelines/components/navigation_tabs.vue'; +import mountComponent from '../helpers/vue_mount_component_helper'; + +describe('navigation tabs pipeline component', () => { + let vm; + let Component; + let data; + + beforeEach(() => { + data = { + scope: 'all', + count: { + all: 16, + running: 1, + pending: 10, + finished: 0, + }, + paths: { + allPath: '/gitlab-org/gitlab-ce/pipelines', + pendingPath: '/gitlab-org/gitlab-ce/pipelines?scope=pending', + finishedPath: '/gitlab-org/gitlab-ce/pipelines?scope=finished', + runningPath: '/gitlab-org/gitlab-ce/pipelines?scope=running', + branchesPath: '/gitlab-org/gitlab-ce/pipelines?scope=branches', + tagsPath: '/gitlab-org/gitlab-ce/pipelines?scope=tags', + }, + }; + + Component = Vue.extend(navigationTabs); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('should render tabs with correct paths', () => { + vm = mountComponent(Component, data); + + // All + const allTab = vm.$el.querySelector('.js-pipelines-tab-all a'); + expect(allTab.textContent.trim()).toContain('All'); + expect(allTab.getAttribute('href')).toEqual(data.paths.allPath); + + // Pending + const pendingTab = vm.$el.querySelector('.js-pipelines-tab-pending a'); + expect(pendingTab.textContent.trim()).toContain('Pending'); + expect(pendingTab.getAttribute('href')).toEqual(data.paths.pendingPath); + + // Running + const runningTab = vm.$el.querySelector('.js-pipelines-tab-running a'); + expect(runningTab.textContent.trim()).toContain('Running'); + expect(runningTab.getAttribute('href')).toEqual(data.paths.runningPath); + + // Finished + const finishedTab = vm.$el.querySelector('.js-pipelines-tab-finished a'); + expect(finishedTab.textContent.trim()).toContain('Finished'); + expect(finishedTab.getAttribute('href')).toEqual(data.paths.finishedPath); + + // Branches + const branchesTab = vm.$el.querySelector('.js-pipelines-tab-branches a'); + expect(branchesTab.textContent.trim()).toContain('Branches'); + + // Tags + const tagsTab = vm.$el.querySelector('.js-pipelines-tab-tags a'); + expect(tagsTab.textContent.trim()).toContain('Tags'); + }); + + describe('scope', () => { + it('should render scope provided as active tab', () => { + vm = mountComponent(Component, data); + expect(vm.$el.querySelector('.js-pipelines-tab-all').className).toContain('active'); + }); + }); + + describe('badges', () => { + it('should render provided number', () => { + vm = mountComponent(Component, data); + // All + expect( + vm.$el.querySelector('.js-totalbuilds-count').textContent.trim(), + ).toContain(data.count.all); + + // Pending + expect( + vm.$el.querySelector('.js-pipelines-tab-pending .badge').textContent.trim(), + ).toContain(data.count.pending); + + // Running + expect( + vm.$el.querySelector('.js-pipelines-tab-running .badge').textContent.trim(), + ).toContain(data.count.running); + + // Finished + expect( + vm.$el.querySelector('.js-pipelines-tab-finished .badge').textContent.trim(), + ).toContain(data.count.finished); + }); + + it('should not render badge when number is undefined', () => { + vm = mountComponent(Component, { + scope: 'all', + paths: {}, + count: {}, + }); + + // All + expect( + vm.$el.querySelector('.js-totalbuilds-count'), + ).toEqual(null); + + // Pending + expect( + vm.$el.querySelector('.js-pipelines-tab-pending .badge'), + ).toEqual(null); + + // Running + expect( + vm.$el.querySelector('.js-pipelines-tab-running .badge'), + ).toEqual(null); + + // Finished + expect( + vm.$el.querySelector('.js-pipelines-tab-finished .badge'), + ).toEqual(null); + }); + }); +}); diff --git a/spec/javascripts/pretty_time_spec.js b/spec/javascripts/pretty_time_spec.js index de99e7e3894..0a6c479a95b 100644 --- a/spec/javascripts/pretty_time_spec.js +++ b/spec/javascripts/pretty_time_spec.js @@ -76,6 +76,87 @@ import '~/lib/utils/pretty_time'; expect(aboveOneWeek.days).toBe(3); expect(aboveOneWeek.weeks).toBe(173); }); + + it('should correctly accept a custom param for hoursPerDay', function () { + const parser = prettyTime.parseSeconds; + const config = { hoursPerDay: 24 }; + + const aboveOneHour = parser(4800, config); + + expect(aboveOneHour.minutes).toBe(20); + expect(aboveOneHour.hours).toBe(1); + expect(aboveOneHour.days).toBe(0); + expect(aboveOneHour.weeks).toBe(0); + + const aboveOneDay = parser(110000, config); + + expect(aboveOneDay.minutes).toBe(33); + expect(aboveOneDay.hours).toBe(6); + expect(aboveOneDay.days).toBe(1); + expect(aboveOneDay.weeks).toBe(0); + + const aboveOneWeek = parser(25000000, config); + + expect(aboveOneWeek.minutes).toBe(26); + expect(aboveOneWeek.hours).toBe(8); + expect(aboveOneWeek.days).toBe(4); + + expect(aboveOneWeek.weeks).toBe(57); + }); + + it('should correctly accept a custom param for daysPerWeek', function () { + const parser = prettyTime.parseSeconds; + const config = { daysPerWeek: 7 }; + + const aboveOneHour = parser(4800, config); + + expect(aboveOneHour.minutes).toBe(20); + expect(aboveOneHour.hours).toBe(1); + expect(aboveOneHour.days).toBe(0); + expect(aboveOneHour.weeks).toBe(0); + + const aboveOneDay = parser(110000, config); + + expect(aboveOneDay.minutes).toBe(33); + expect(aboveOneDay.hours).toBe(6); + expect(aboveOneDay.days).toBe(3); + expect(aboveOneDay.weeks).toBe(0); + + const aboveOneWeek = parser(25000000, config); + + expect(aboveOneWeek.minutes).toBe(26); + expect(aboveOneWeek.hours).toBe(0); + expect(aboveOneWeek.days).toBe(0); + + expect(aboveOneWeek.weeks).toBe(124); + }); + + it('should correctly accept custom params for daysPerWeek and hoursPerDay', function () { + const parser = prettyTime.parseSeconds; + const config = { daysPerWeek: 55, hoursPerDay: 14 }; + + const aboveOneHour = parser(4800, config); + + expect(aboveOneHour.minutes).toBe(20); + expect(aboveOneHour.hours).toBe(1); + expect(aboveOneHour.days).toBe(0); + expect(aboveOneHour.weeks).toBe(0); + + const aboveOneDay = parser(110000, config); + + expect(aboveOneDay.minutes).toBe(33); + expect(aboveOneDay.hours).toBe(2); + expect(aboveOneDay.days).toBe(2); + expect(aboveOneDay.weeks).toBe(0); + + const aboveOneWeek = parser(25000000, config); + + expect(aboveOneWeek.minutes).toBe(26); + expect(aboveOneWeek.hours).toBe(0); + expect(aboveOneWeek.days).toBe(1); + + expect(aboveOneWeek.weeks).toBe(9); + }); }); describe('stringifyTime', function () { diff --git a/spec/javascripts/project_select_combo_button_spec.js b/spec/javascripts/project_select_combo_button_spec.js index 021804e0769..dda83645c92 100644 --- a/spec/javascripts/project_select_combo_button_spec.js +++ b/spec/javascripts/project_select_combo_button_spec.js @@ -32,11 +32,6 @@ describe('Project Select Combo Button', function () { this.comboButton = new ProjectSelectComboButton(this.projectSelectInput); }); - it('newItemBtn is disabled', function () { - expect(this.newItemBtn.hasAttribute('disabled')).toBe(true); - expect(this.newItemBtn.classList.contains('disabled')).toBe(true); - }); - it('newItemBtn href is null', function () { expect(this.newItemBtn.getAttribute('href')).toBe(''); }); @@ -53,11 +48,6 @@ describe('Project Select Combo Button', function () { this.comboButton = new ProjectSelectComboButton(this.projectSelectInput); }); - it('newItemBtn is not disabled', function () { - expect(this.newItemBtn.hasAttribute('disabled')).toBe(false); - expect(this.newItemBtn.classList.contains('disabled')).toBe(false); - }); - it('newItemBtn href is correctly set', function () { expect(this.newItemBtn.getAttribute('href')).toBe(this.defaults.projectMeta.url); }); @@ -82,11 +72,6 @@ describe('Project Select Combo Button', function () { .trigger('change'); }); - it('newItemBtn is not disabled', function () { - expect(this.newItemBtn.hasAttribute('disabled')).toBe(false); - expect(this.newItemBtn.classList.contains('disabled')).toBe(false); - }); - it('newItemBtn href is correctly set', function () { expect(this.newItemBtn.getAttribute('href')) .toBe('http://myothercoolproject.com/issues/new'); diff --git a/spec/javascripts/project_title_spec.js b/spec/javascripts/project_title_spec.js index cc336180ff7..3d36bb3e4d4 100644 --- a/spec/javascripts/project_title_spec.js +++ b/spec/javascripts/project_title_spec.js @@ -7,6 +7,7 @@ import '~/project_select'; import '~/project'; describe('Project Title', () => { + const dummyApiVersion = 'v3000'; preloadFixtures('issues/open-issue.html.raw'); loadJSONFixtures('projects.json'); @@ -14,7 +15,7 @@ describe('Project Title', () => { loadFixtures('issues/open-issue.html.raw'); window.gon = {}; - window.gon.api_version = 'v3'; + window.gon.api_version = dummyApiVersion; // eslint-disable-next-line no-new new Project(); @@ -37,9 +38,10 @@ describe('Project Title', () => { it('toggles dropdown', () => { const $menu = $('.js-dropdown-menu-projects'); + window.gon.current_user_id = 1; $('.js-projects-dropdown-toggle').click(); expect($menu).toHaveClass('open'); - expect(reqUrl).toBe('/api/v3/projects.json?simple=true'); + expect(reqUrl).toBe(`/api/${dummyApiVersion}/projects.json?simple=true`); expect(reqData).toEqual({ search: '', order_by: 'last_activity_at', diff --git a/spec/javascripts/repo/components/repo_commit_section_spec.js b/spec/javascripts/repo/components/repo_commit_section_spec.js index 249a2f36fcd..e604dcc152d 100644 --- a/spec/javascripts/repo/components/repo_commit_section_spec.js +++ b/spec/javascripts/repo/components/repo_commit_section_spec.js @@ -1,7 +1,7 @@ import Vue from 'vue'; import repoCommitSection from '~/repo/components/repo_commit_section.vue'; import RepoStore from '~/repo/stores/repo_store'; -import Api from '~/api'; +import RepoService from '~/repo/services/repo_service'; describe('RepoCommitSection', () => { const branch = 'master'; @@ -111,7 +111,7 @@ describe('RepoCommitSection', () => { expect(submitCommit.disabled).toBeFalsy(); spyOn(vm, 'makeCommit').and.callThrough(); - spyOn(Api, 'commitMultiple'); + spyOn(RepoService, 'commitFiles').and.callFake(() => Promise.resolve()); submitCommit.click(); @@ -119,10 +119,9 @@ describe('RepoCommitSection', () => { expect(vm.makeCommit).toHaveBeenCalled(); expect(submitCommit.querySelector('.fa-spinner.fa-spin')).toBeTruthy(); - const args = Api.commitMultiple.calls.allArgs()[0]; - const { commit_message, actions, branch: payloadBranch } = args[1]; + const args = RepoService.commitFiles.calls.allArgs()[0]; + const { commit_message, actions, branch: payloadBranch } = args[0]; - expect(args[0]).toBe(projectId); expect(commit_message).toBe(commitMessage); expect(actions.length).toEqual(2); expect(payloadBranch).toEqual(branch); diff --git a/spec/javascripts/repo/services/repo_service_spec.js b/spec/javascripts/repo/services/repo_service_spec.js index d74e6a67b1e..6f530770525 100644 --- a/spec/javascripts/repo/services/repo_service_spec.js +++ b/spec/javascripts/repo/services/repo_service_spec.js @@ -1,5 +1,7 @@ import axios from 'axios'; import RepoService from '~/repo/services/repo_service'; +import RepoStore from '~/repo/stores/repo_store'; +import Api from '~/api'; describe('RepoService', () => { it('has default json format param', () => { @@ -118,4 +120,52 @@ describe('RepoService', () => { }).catch(done.fail); }); }); + + describe('commitFiles', () => { + it('calls commitMultiple and .then commitFlash', (done) => { + const projectId = 'projectId'; + const payload = {}; + RepoStore.projectId = projectId; + + spyOn(Api, 'commitMultiple').and.returnValue(Promise.resolve()); + spyOn(RepoService, 'commitFlash'); + + const apiPromise = RepoService.commitFiles(payload); + + expect(Api.commitMultiple).toHaveBeenCalledWith(projectId, payload); + + apiPromise.then(() => { + expect(RepoService.commitFlash).toHaveBeenCalled(); + done(); + }).catch(done.fail); + }); + }); + + describe('commitFlash', () => { + it('calls Flash with data.message', () => { + const data = { + message: 'message', + }; + spyOn(window, 'Flash'); + + RepoService.commitFlash(data); + + expect(window.Flash).toHaveBeenCalledWith(data.message); + }); + + it('calls Flash with success string if short_id and stats', () => { + const data = { + short_id: 'short_id', + stats: { + additions: '4', + deletions: '5', + }, + }; + spyOn(window, 'Flash'); + + RepoService.commitFlash(data); + + expect(window.Flash).toHaveBeenCalledWith(`Your changes have been committed. Commit ${data.short_id} with ${data.stats.additions} additions, ${data.stats.deletions} deletions.`, 'notice'); + }); + }); }); diff --git a/spec/javascripts/groups/group_identicon_spec.js b/spec/javascripts/vue_shared/components/identicon_spec.js index 66772327503..4f194e5a64e 100644 --- a/spec/javascripts/groups/group_identicon_spec.js +++ b/spec/javascripts/vue_shared/components/identicon_spec.js @@ -1,22 +1,18 @@ import Vue from 'vue'; -import groupIdenticonComponent from '~/groups/components/group_identicon.vue'; -import GroupsStore from '~/groups/stores/groups_store'; -import { group1 } from './mock_data'; +import identiconComponent from '~/vue_shared/components/identicon.vue'; const createComponent = () => { - const Component = Vue.extend(groupIdenticonComponent); - const store = new GroupsStore(); - const group = store.decorateGroup(group1); + const Component = Vue.extend(identiconComponent); return new Component({ propsData: { - entityId: group.id, - entityName: group.name, + entityId: 1, + entityName: 'entity-name', }, }).$mount(); }; -describe('GroupIdenticonComponent', () => { +describe('IdenticonComponent', () => { let vm; beforeEach(() => { diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index ee28387cd48..c70a4cb55fe 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -344,28 +344,31 @@ module Ci let(:config) { { rspec: { script: "rspec", type: "test", only: only } } } let(:processor) { GitlabCiYamlProcessor.new(YAML.dump(config)) } - shared_examples 'raises an error' do - it do - expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'jobs:rspec:only config should be an array of strings or regexps') - end - end - context 'when it is integer' do let(:only) { 1 } - it_behaves_like 'raises an error' + it do + expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, + 'jobs:rspec:only has to be either an array of conditions or a hash') + end end context 'when it is an array of integers' do let(:only) { [1, 1] } - it_behaves_like 'raises an error' + it do + expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, + 'jobs:rspec:only config should be an array of strings or regexps') + end end context 'when it is invalid regex' do let(:only) { ["/*invalid/"] } - it_behaves_like 'raises an error' + it do + expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, + 'jobs:rspec:only config should be an array of strings or regexps') + end end end end @@ -518,28 +521,31 @@ module Ci let(:config) { { rspec: { script: "rspec", except: except } } } let(:processor) { GitlabCiYamlProcessor.new(YAML.dump(config)) } - shared_examples 'raises an error' do - it do - expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'jobs:rspec:except config should be an array of strings or regexps') - end - end - context 'when it is integer' do let(:except) { 1 } - it_behaves_like 'raises an error' + it do + expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, + 'jobs:rspec:except has to be either an array of conditions or a hash') + end end context 'when it is an array of integers' do let(:except) { [1, 1] } - it_behaves_like 'raises an error' + it do + expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, + 'jobs:rspec:except config should be an array of strings or regexps') + end end context 'when it is invalid regex' do let(:except) { ["/*invalid/"] } - it_behaves_like 'raises an error' + it do + expect { processor }.to raise_error(GitlabCiYamlProcessor::ValidationError, + 'jobs:rspec:except config should be an array of strings or regexps') + end end end end diff --git a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb index f29431b937c..22708687a56 100644 --- a/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb +++ b/spec/lib/gitlab/auth/unique_ips_limiter_spec.rb @@ -41,7 +41,7 @@ describe Gitlab::Auth::UniqueIpsLimiter, :clean_gitlab_redis_shared_state do context 'allow 2 unique ips' do before do - current_application_settings.update!(unique_ips_limit_per_user: 2) + Gitlab::CurrentSettings.current_application_settings.update!(unique_ips_limit_per_user: 2) end it 'blocks user trying to login from third ip' do diff --git a/spec/lib/gitlab/bare_repository_importer_spec.rb b/spec/lib/gitlab/bare_repository_importer_spec.rb index 892f2dafc96..36d1844b5b1 100644 --- a/spec/lib/gitlab/bare_repository_importer_spec.rb +++ b/spec/lib/gitlab/bare_repository_importer_spec.rb @@ -2,67 +2,99 @@ require 'spec_helper' describe Gitlab::BareRepositoryImporter, repository: true do subject(:importer) { described_class.new('default', project_path) } - let(:project_path) { 'a-group/a-sub-group/a-project' } + let!(:admin) { create(:admin) } before do allow(described_class).to receive(:log) end - describe '.execute' do - it 'creates a project for a repository in storage' do - FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project_path}.git")) - fake_importer = double + shared_examples 'importing a repository' do + describe '.execute' do + it 'creates a project for a repository in storage' do + FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project_path}.git")) + fake_importer = double - expect(described_class).to receive(:new).with('default', project_path) - .and_return(fake_importer) - expect(fake_importer).to receive(:create_project_if_needed) + expect(described_class).to receive(:new).with('default', project_path) + .and_return(fake_importer) + expect(fake_importer).to receive(:create_project_if_needed) - described_class.execute - end + described_class.execute + end - it 'skips wiki repos' do - FileUtils.mkdir_p(File.join(TestEnv.repos_path, 'the-group', 'the-project.wiki.git')) + it 'skips wiki repos' do + FileUtils.mkdir_p(File.join(TestEnv.repos_path, 'the-group', 'the-project.wiki.git')) - expect(described_class).to receive(:log).with(' * Skipping wiki repo') - expect(described_class).not_to receive(:new) + expect(described_class).to receive(:log).with(' * Skipping wiki repo') + expect(described_class).not_to receive(:new) - described_class.execute + described_class.execute + end end - end - describe '#initialize' do - context 'without admin users' do - let(:admin) { nil } + describe '#initialize' do + context 'without admin users' do + let(:admin) { nil } - it 'raises an error' do - expect { importer }.to raise_error(Gitlab::BareRepositoryImporter::NoAdminError) + it 'raises an error' do + expect { importer }.to raise_error(Gitlab::BareRepositoryImporter::NoAdminError) + end end end - end - describe '#create_project_if_needed' do - it 'starts an import for a project that did not exist' do - expect(importer).to receive(:create_project) + describe '#create_project_if_needed' do + it 'starts an import for a project that did not exist' do + expect(importer).to receive(:create_project) + + importer.create_project_if_needed + end + + it 'skips importing when the project already exists' do + project = create(:project, path: 'a-project', namespace: existing_group) + + expect(importer).not_to receive(:create_project) + expect(importer).to receive(:log).with(" * #{project.name} (#{project_path}) exists") + + importer.create_project_if_needed + end + + it 'creates a project with the correct path in the database' do + importer.create_project_if_needed - importer.create_project_if_needed + expect(Project.find_by_full_path(project_path)).not_to be_nil + end end + end + + context 'with subgroups', :nested_groups do + let(:project_path) { 'a-group/a-sub-group/a-project' } - it 'skips importing when the project already exists' do + let(:existing_group) do group = create(:group, path: 'a-group') - subgroup = create(:group, path: 'a-sub-group', parent: group) - project = create(:project, path: 'a-project', namespace: subgroup) + create(:group, path: 'a-sub-group', parent: group) + end - expect(importer).not_to receive(:create_project) - expect(importer).to receive(:log).with(" * #{project.name} (a-group/a-sub-group/a-project) exists") + it_behaves_like 'importing a repository' + end - importer.create_project_if_needed - end + context 'without subgroups' do + let(:project_path) { 'a-group/a-project' } + let(:existing_group) { create(:group, path: 'a-group') } - it 'creates a project with the correct path in the database' do - importer.create_project_if_needed + it_behaves_like 'importing a repository' + end + + context 'when subgroups are not available' do + let(:project_path) { 'a-group/a-sub-group/a-project' } + + before do + expect(Group).to receive(:supports_nested_groups?) { false } + end - expect(Project.find_by_full_path(project_path)).not_to be_nil + describe '#create_project_if_needed' do + it 'raises an error' do + expect { importer.create_project_if_needed }.to raise_error('Nested groups are not supported on MySQL') + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/attributable_spec.rb b/spec/lib/gitlab/ci/config/entry/attributable_spec.rb index fde03c51e2c..b028b771375 100644 --- a/spec/lib/gitlab/ci/config/entry/attributable_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/attributable_spec.rb @@ -1,18 +1,21 @@ require 'spec_helper' describe Gitlab::Ci::Config::Entry::Attributable do - let(:node) { Class.new } + let(:node) do + Class.new do + include Gitlab::Ci::Config::Entry::Attributable + end + end + let(:instance) { node.new } before do - node.include(described_class) - node.class_eval do attributes :name, :test end end - context 'config is a hash' do + context 'when config is a hash' do before do allow(instance) .to receive(:config) @@ -29,7 +32,7 @@ describe Gitlab::Ci::Config::Entry::Attributable do end end - context 'config is not a hash' do + context 'when config is not a hash' do before do allow(instance) .to receive(:config) @@ -40,4 +43,18 @@ describe Gitlab::Ci::Config::Entry::Attributable do expect(instance.test).to be_nil end end + + context 'when method is already defined in a superclass' do + it 'raises an error' do + expectation = expect do + Class.new(String) do + include Gitlab::Ci::Config::Entry::Attributable + + attributes :length + end + end + + expectation.to raise_error(ArgumentError, 'Method already defined!') + end + end end diff --git a/spec/lib/gitlab/ci/config/entry/configurable_spec.rb b/spec/lib/gitlab/ci/config/entry/configurable_spec.rb index ae7e628b5b5..088d4b472da 100644 --- a/spec/lib/gitlab/ci/config/entry/configurable_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/configurable_spec.rb @@ -1,40 +1,26 @@ require 'spec_helper' describe Gitlab::Ci::Config::Entry::Configurable do - let(:entry) { Class.new } - - before do - entry.include(described_class) + let(:entry) do + Class.new(Gitlab::Ci::Config::Entry::Node) do + include Gitlab::Ci::Config::Entry::Configurable + end end describe 'validations' do - let(:validator) { entry.validator.new(instance) } - - before do - entry.class_eval do - attr_reader :config + context 'when entry is a hash' do + let(:instance) { entry.new(key: 'value') } - def initialize(config) - @config = config - end + it 'correctly validates an instance' do + expect(instance).to be_valid end - - validator.validate end - context 'when entry validator is invalid' do + context 'when entry is not a hash' do let(:instance) { entry.new('ls') } - it 'returns invalid validator' do - expect(validator).to be_invalid - end - end - - context 'when entry instance is valid' do - let(:instance) { entry.new(key: 'value') } - - it 'returns valid validator' do - expect(validator).to be_valid + it 'invalidates the instance' do + expect(instance).not_to be_valid end end end diff --git a/spec/lib/gitlab/ci/config/entry/policy_spec.rb b/spec/lib/gitlab/ci/config/entry/policy_spec.rb new file mode 100644 index 00000000000..36a84da4a52 --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/policy_spec.rb @@ -0,0 +1,73 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Entry::Policy do + let(:entry) { described_class.new(config) } + + context 'when using simplified policy' do + describe 'validations' do + context 'when entry config value is valid' do + context 'when config is a branch or tag name' do + let(:config) { %w[master feature/branch] } + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + + describe '#value' do + it 'returns key value' do + expect(entry.value).to eq config + end + end + end + + context 'when config is a regexp' do + let(:config) { ['/^issue-.*$/'] } + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + end + + context 'when config is a special keyword' do + let(:config) { %w[tags triggers branches] } + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + end + end + + context 'when entry value is not valid' do + let(:config) { [1] } + + describe '#errors' do + it 'saves errors' do + expect(entry.errors) + .to include /policy config should be an array of strings or regexps/ + end + end + end + end + end + + context 'when policy strategy does not match' do + let(:config) { 'string strategy' } + + it 'returns information about errors' do + expect(entry.errors) + .to include /has to be either an array of conditions or a hash/ + end + end + + describe '.default' do + it 'does not have a default value' do + expect(described_class.default).to be_nil + end + end +end diff --git a/spec/lib/gitlab/ci/config/entry/simplifiable_spec.rb b/spec/lib/gitlab/ci/config/entry/simplifiable_spec.rb new file mode 100644 index 00000000000..395062207a3 --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/simplifiable_spec.rb @@ -0,0 +1,88 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Entry::Simplifiable do + describe '.strategy' do + let(:entry) do + Class.new(described_class) do + strategy :Something, if: -> { 'condition' } + strategy :DifferentOne, if: -> { 'condition' } + end + end + + it 'defines entry strategies' do + expect(entry.strategies.size).to eq 2 + expect(entry.strategies.map(&:name)) + .to eq %i[Something DifferentOne] + end + end + + describe 'setting strategy by a condition' do + let(:first) { double('first strategy') } + let(:second) { double('second strategy') } + let(:unknown) { double('unknown strategy') } + + before do + stub_const("#{described_class.name}::Something", first) + stub_const("#{described_class.name}::DifferentOne", second) + stub_const("#{described_class.name}::UnknownStrategy", unknown) + end + + context 'when first strategy should be used' do + let(:entry) do + Class.new(described_class) do + strategy :Something, if: -> (arg) { arg == 'something' } + strategy :DifferentOne, if: -> (*) { false } + end + end + + it 'attemps to load a first strategy' do + expect(first).to receive(:new).with('something', anything) + + entry.new('something') + end + end + + context 'when second strategy should be used' do + let(:entry) do + Class.new(described_class) do + strategy :Something, if: -> (arg) { arg == 'something' } + strategy :DifferentOne, if: -> (arg) { arg == 'test' } + end + end + + it 'attemps to load a second strategy' do + expect(second).to receive(:new).with('test', anything) + + entry.new('test') + end + end + + context 'when neither one is a valid strategy' do + let(:entry) do + Class.new(described_class) do + strategy :Something, if: -> (*) { false } + strategy :DifferentOne, if: -> (*) { false } + end + end + + it 'instantiates an unknown strategy' do + expect(unknown).to receive(:new).with('test', anything) + + entry.new('test') + end + end + end + + context 'when a unknown strategy class is not defined' do + let(:entry) do + Class.new(described_class) do + strategy :String, if: -> (*) { true } + end + end + + it 'raises an error when being initialized' do + expect { entry.new('something') } + .to raise_error ArgumentError, /UndefinedStrategy not available!/ + end + end +end diff --git a/spec/lib/gitlab/ci/config/entry/trigger_spec.rb b/spec/lib/gitlab/ci/config/entry/trigger_spec.rb deleted file mode 100644 index e4ee44f1274..00000000000 --- a/spec/lib/gitlab/ci/config/entry/trigger_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Ci::Config::Entry::Trigger do - let(:entry) { described_class.new(config) } - - describe 'validations' do - context 'when entry config value is valid' do - context 'when config is a branch or tag name' do - let(:config) { %w[master feature/branch] } - - describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid - end - end - - describe '#value' do - it 'returns key value' do - expect(entry.value).to eq config - end - end - end - - context 'when config is a regexp' do - let(:config) { ['/^issue-.*$/'] } - - describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid - end - end - end - - context 'when config is a special keyword' do - let(:config) { %w[tags triggers branches] } - - describe '#valid?' do - it 'is valid' do - expect(entry).to be_valid - end - end - end - end - - context 'when entry value is not valid' do - let(:config) { [1] } - - describe '#errors' do - it 'saves errors' do - expect(entry.errors) - .to include 'trigger config should be an array of strings or regexps' - end - end - end - end -end diff --git a/spec/lib/gitlab/ci/config/entry/validatable_spec.rb b/spec/lib/gitlab/ci/config/entry/validatable_spec.rb index d1856801827..ae2a7a51ba6 100644 --- a/spec/lib/gitlab/ci/config/entry/validatable_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/validatable_spec.rb @@ -1,10 +1,10 @@ require 'spec_helper' describe Gitlab::Ci::Config::Entry::Validatable do - let(:entry) { Class.new } - - before do - entry.include(described_class) + let(:entry) do + Class.new(Gitlab::Ci::Config::Entry::Node) do + include Gitlab::Ci::Config::Entry::Validatable + end end describe '.validator' do @@ -28,7 +28,7 @@ describe Gitlab::Ci::Config::Entry::Validatable do end context 'when validating entry instance' do - let(:entry_instance) { entry.new } + let(:entry_instance) { entry.new('something') } context 'when attribute is valid' do before do diff --git a/spec/lib/gitlab/ci/config/entry/validator_spec.rb b/spec/lib/gitlab/ci/config/entry/validator_spec.rb index ad7e6f07d3c..172b6b47a4f 100644 --- a/spec/lib/gitlab/ci/config/entry/validator_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/validator_spec.rb @@ -48,7 +48,7 @@ describe Gitlab::Ci::Config::Entry::Validator do validator_instance.validate expect(validator_instance.messages) - .to include "node test attribute can't be blank" + .to include /test attribute can't be blank/ end end end diff --git a/spec/lib/gitlab/database/grant_spec.rb b/spec/lib/gitlab/database/grant_spec.rb new file mode 100644 index 00000000000..651da3e8476 --- /dev/null +++ b/spec/lib/gitlab/database/grant_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe Gitlab::Database::Grant do + describe '.scope_to_current_user' do + it 'scopes the relation to the current user' do + user = Gitlab::Database.username + column = Gitlab::Database.postgresql? ? :grantee : :User + names = described_class.scope_to_current_user.pluck(column).uniq + + expect(names).to eq([user]) + end + end + + describe '.create_and_execute_trigger' do + it 'returns true when the user can create and execute a trigger' do + # We assume the DB/user is set up correctly so that triggers can be + # created, which is necessary anyway for other tests to work. + expect(described_class.create_and_execute_trigger?('users')).to eq(true) + end + + it 'returns false when the user can not create and/or execute a trigger' do + allow(described_class).to receive(:scope_to_current_user) + .and_return(described_class.none) + + result = described_class.create_and_execute_trigger?('kittens') + + expect(result).to eq(false) + end + end +end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index ec2274a70aa..1bcdc369c44 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -2,9 +2,7 @@ require 'spec_helper' describe Gitlab::Database::MigrationHelpers do let(:model) do - ActiveRecord::Migration.new.extend( - described_class - ) + ActiveRecord::Migration.new.extend(described_class) end before do @@ -452,6 +450,8 @@ describe Gitlab::Database::MigrationHelpers do it 'renames a column concurrently' do allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + expect(model).to receive(:check_trigger_permissions!).with(:users) + expect(model).to receive(:install_rename_triggers_for_mysql) .with(trigger_name, 'users', 'old', 'new') @@ -479,6 +479,8 @@ describe Gitlab::Database::MigrationHelpers do it 'renames a column concurrently' do allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + expect(model).to receive(:check_trigger_permissions!).with(:users) + expect(model).to receive(:install_rename_triggers_for_postgresql) .with(trigger_name, 'users', 'old', 'new') @@ -508,6 +510,8 @@ describe Gitlab::Database::MigrationHelpers do it 'cleans up the renaming procedure for PostgreSQL' do allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + expect(model).to receive(:check_trigger_permissions!).with(:users) + expect(model).to receive(:remove_rename_triggers_for_postgresql) .with(:users, /trigger_.{12}/) @@ -519,6 +523,8 @@ describe Gitlab::Database::MigrationHelpers do it 'cleans up the renaming procedure for MySQL' do allow(Gitlab::Database).to receive(:postgresql?).and_return(false) + expect(model).to receive(:check_trigger_permissions!).with(:users) + expect(model).to receive(:remove_rename_triggers_for_mysql) .with(/trigger_.{12}/) @@ -575,8 +581,8 @@ describe Gitlab::Database::MigrationHelpers do describe '#remove_rename_triggers_for_postgresql' do it 'removes the function and trigger' do - expect(model).to receive(:execute).with('DROP TRIGGER foo ON bar') - expect(model).to receive(:execute).with('DROP FUNCTION foo()') + expect(model).to receive(:execute).with('DROP TRIGGER IF EXISTS foo ON bar') + expect(model).to receive(:execute).with('DROP FUNCTION IF EXISTS foo()') model.remove_rename_triggers_for_postgresql('bar', 'foo') end @@ -584,8 +590,8 @@ describe Gitlab::Database::MigrationHelpers do describe '#remove_rename_triggers_for_mysql' do it 'removes the triggers' do - expect(model).to receive(:execute).with('DROP TRIGGER foo_insert') - expect(model).to receive(:execute).with('DROP TRIGGER foo_update') + expect(model).to receive(:execute).with('DROP TRIGGER IF EXISTS foo_insert') + expect(model).to receive(:execute).with('DROP TRIGGER IF EXISTS foo_update') model.remove_rename_triggers_for_mysql('foo') end @@ -845,4 +851,67 @@ describe Gitlab::Database::MigrationHelpers do end end end + + describe 'sidekiq migration helpers', :sidekiq, :redis do + let(:worker) do + Class.new do + include Sidekiq::Worker + sidekiq_options queue: 'test' + end + end + + describe '#sidekiq_queue_length' do + context 'when queue is empty' do + it 'returns zero' do + Sidekiq::Testing.disable! do + expect(model.sidekiq_queue_length('test')).to eq 0 + end + end + end + + context 'when queue contains jobs' do + it 'returns correct size of the queue' do + Sidekiq::Testing.disable! do + worker.perform_async('Something', [1]) + worker.perform_async('Something', [2]) + + expect(model.sidekiq_queue_length('test')).to eq 2 + end + end + end + end + + describe '#migrate_sidekiq_queue' do + it 'migrates jobs from one sidekiq queue to another' do + Sidekiq::Testing.disable! do + worker.perform_async('Something', [1]) + worker.perform_async('Something', [2]) + + expect(model.sidekiq_queue_length('test')).to eq 2 + expect(model.sidekiq_queue_length('new_test')).to eq 0 + + model.sidekiq_queue_migrate('test', to: 'new_test') + + expect(model.sidekiq_queue_length('test')).to eq 0 + expect(model.sidekiq_queue_length('new_test')).to eq 2 + end + end + end + end + + describe '#check_trigger_permissions!' do + it 'does nothing when the user has the correct permissions' do + expect { model.check_trigger_permissions!('users') } + .not_to raise_error(RuntimeError) + end + + it 'raises RuntimeError when the user does not have the correct permissions' do + allow(Gitlab::Database::Grant).to receive(:create_and_execute_trigger?) + .with('kittens') + .and_return(false) + + expect { model.check_trigger_permissions!('kittens') } + .to raise_error(RuntimeError, /Your database user is not allowed/) + end + end end diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index d3d841b0668..c91895cedc3 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -15,6 +15,17 @@ describe Gitlab::Diff::File do it { expect(diff_lines.first).to be_kind_of(Gitlab::Diff::Line) } end + describe '#highlighted_diff_lines' do + it 'highlights the diff and memoises the result' do + expect(Gitlab::Diff::Highlight).to receive(:new) + .with(diff_file, repository: project.repository) + .once + .and_call_original + + diff_file.highlighted_diff_lines + end + end + describe '#mode_changed?' do it { expect(diff_file.mode_changed?).to be_falsey } end @@ -122,8 +133,20 @@ describe Gitlab::Diff::File do let(:commit) { project.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } let(:diff_file) { commit.diffs.diff_file_with_new_path('files/images/6049019_460s.jpg') } - it 'returns true' do - expect(diff_file.content_changed?).to be_truthy + context 'when the blobs are different' do + it 'returns true' do + expect(diff_file.content_changed?).to be_truthy + end + end + + context 'when there are no diff refs' do + before do + allow(diff_file).to receive(:diff_refs).and_return(nil) + end + + it 'returns false' do + expect(diff_file.content_changed?).to be_falsey + end end end @@ -131,8 +154,20 @@ describe Gitlab::Diff::File do let(:commit) { project.commit('570e7b2abdd848b95f2f578043fc23bd6f6fd24d') } let(:diff_file) { commit.diffs.diff_file_with_new_path('files/ruby/popen.rb') } - it 'returns true' do - expect(diff_file.content_changed?).to be_truthy + context 'when the blobs are different' do + it 'returns true' do + expect(diff_file.content_changed?).to be_truthy + end + end + + context 'when there are no diff refs' do + before do + allow(diff_file).to receive(:diff_refs).and_return(nil) + end + + it 'returns true' do + expect(diff_file.content_changed?).to be_truthy + end end end end @@ -270,6 +305,21 @@ describe Gitlab::Diff::File do expect(diff_file.simple_viewer).to be_a(DiffViewer::ModeChanged) end end + + context 'when no other conditions apply' do + before do + allow(diff_file).to receive(:content_changed?).and_return(false) + allow(diff_file).to receive(:new_file?).and_return(false) + allow(diff_file).to receive(:deleted_file?).and_return(false) + allow(diff_file).to receive(:renamed_file?).and_return(false) + allow(diff_file).to receive(:mode_changed?).and_return(false) + allow(diff_file).to receive(:raw_text?).and_return(false) + end + + it 'returns a No Preview viewer' do + expect(diff_file.simple_viewer).to be_a(DiffViewer::NoPreview) + end + end end describe '#rich_viewer' do diff --git a/spec/lib/gitlab/file_finder_spec.rb b/spec/lib/gitlab/file_finder_spec.rb index 3fb6315a39a..07cb10e563e 100644 --- a/spec/lib/gitlab/file_finder_spec.rb +++ b/spec/lib/gitlab/file_finder_spec.rb @@ -7,15 +7,23 @@ describe Gitlab::FileFinder do it 'finds by name' do results = finder.find('files') - expect(results.map(&:first)).to include('files/images/wm.svg') + + filename, blob = results.find { |_, blob| blob.filename == 'files/images/wm.svg' } + expect(filename).to eq('files/images/wm.svg') + expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) + expect(blob.ref).to eq(finder.ref) + expect(blob.data).not_to be_empty end it 'finds by content' do results = finder.find('files') - blob = results.select { |result| result.first == "CHANGELOG" }.flatten.last + filename, blob = results.find { |_, blob| blob.filename == 'CHANGELOG' } - expect(blob.filename).to eq("CHANGELOG") + expect(filename).to eq('CHANGELOG') + expect(blob).to be_a(Gitlab::SearchResults::FoundBlob) + expect(blob.ref).to eq(finder.ref) + expect(blob.data).not_to be_empty end end end diff --git a/spec/lib/gitlab/git/blame_spec.rb b/spec/lib/gitlab/git/blame_spec.rb index 800c245b130..465c2012b05 100644 --- a/spec/lib/gitlab/git/blame_spec.rb +++ b/spec/lib/gitlab/git/blame_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" describe Gitlab::Git::Blame, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:blame) do Gitlab::Git::Blame.new(repository, SeedRepo::Commit::ID, "CONTRIBUTING.md") end diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index dfab0c2fe85..66ba00acb7d 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -3,7 +3,7 @@ require "spec_helper" describe Gitlab::Git::Blob, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } describe 'initialize' do let(:blob) { Gitlab::Git::Blob.new(name: 'test') } diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index cdf1b8beee3..318a7b7a332 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Branch, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } subject { repository.branches } diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index ac33cd8a2c9..14d64d8c4da 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Commit, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } let(:rugged_commit) do repository.rugged.lookup(SeedRepo::Commit::ID) @@ -9,7 +9,7 @@ describe Gitlab::Git::Commit, seed_helper: true do describe "Commit info" do before do - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged @committer = { email: 'mike@smith.com', @@ -59,7 +59,7 @@ describe Gitlab::Git::Commit, seed_helper: true do after do # Erase the new commit so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end end @@ -144,7 +144,7 @@ describe Gitlab::Git::Commit, seed_helper: true do end context 'with broken repo' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_BROKEN_REPO_PATH, '') } it 'returns nil' do expect(described_class.find(repository, SeedRepo::Commit::ID)).to be_nil diff --git a/spec/lib/gitlab/git/committer_spec.rb b/spec/lib/gitlab/git/committer_spec.rb new file mode 100644 index 00000000000..b0ddbb51449 --- /dev/null +++ b/spec/lib/gitlab/git/committer_spec.rb @@ -0,0 +1,22 @@ +require 'spec_helper' + +describe Gitlab::Git::Committer do + let(:name) { 'Jane Doe' } + let(:email) { 'janedoe@example.com' } + let(:gl_id) { 'user-123' } + + subject { described_class.new(name, email, gl_id) } + + describe '#==' do + def eq_other(name, email, gl_id) + eq(described_class.new(name, email, gl_id)) + end + + it { expect(subject).to eq_other(name, email, gl_id) } + + it { expect(subject).not_to eq_other(nil, nil, nil) } + it { expect(subject).not_to eq_other(name + 'x', email, gl_id) } + it { expect(subject).not_to eq_other(name, email + 'x', gl_id) } + it { expect(subject).not_to eq_other(name, email, gl_id + 'x') } + end +end diff --git a/spec/lib/gitlab/git/compare_spec.rb b/spec/lib/gitlab/git/compare_spec.rb index 4c9f4a28f32..b6a42e422b5 100644 --- a/spec/lib/gitlab/git/compare_spec.rb +++ b/spec/lib/gitlab/git/compare_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Compare, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:compare) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: false) } let(:compare_straight) { Gitlab::Git::Compare.new(repository, SeedRepo::BigCommit::ID, SeedRepo::Commit::ID, straight: true) } diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 7ea3386ac2a..dfbdbee48f7 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Diff, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } before do @raw_diff_hash = { diff --git a/spec/lib/gitlab/git/hook_spec.rb b/spec/lib/gitlab/git/hook_spec.rb index 19391a70cf6..ea3e4680b1d 100644 --- a/spec/lib/gitlab/git/hook_spec.rb +++ b/spec/lib/gitlab/git/hook_spec.rb @@ -10,7 +10,8 @@ describe Gitlab::Git::Hook do describe "#trigger" do let(:project) { create(:project, :repository) } - let(:repo_path) { project.repository.path } + let(:repository) { project.repository.raw_repository } + let(:repo_path) { repository.path } let(:user) { create(:user) } let(:gl_id) { Gitlab::GlId.gl_id(user) } @@ -48,7 +49,7 @@ describe Gitlab::Git::Hook do it "returns success with no errors" do create_hook(hook_name) - hook = described_class.new(hook_name, project) + hook = described_class.new(hook_name, repository) blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' @@ -66,7 +67,7 @@ describe Gitlab::Git::Hook do context "when the hook is unsuccessful" do it "returns failure with errors" do create_failing_hook(hook_name) - hook = described_class.new(hook_name, project) + hook = described_class.new(hook_name, repository) blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' @@ -80,7 +81,7 @@ describe Gitlab::Git::Hook do context "when the hook doesn't exist" do it "returns success with no errors" do - hook = described_class.new('unknown_hook', project) + hook = described_class.new('unknown_hook', repository) blank = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' diff --git a/spec/services/git_hooks_service_spec.rb b/spec/lib/gitlab/git/hooks_service_spec.rb index 3ce01a995b4..e9c0209fe3b 100644 --- a/spec/services/git_hooks_service_spec.rb +++ b/spec/lib/gitlab/git/hooks_service_spec.rb @@ -1,16 +1,14 @@ require 'spec_helper' -describe GitHooksService do - include RepoHelpers - - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } +describe Gitlab::Git::HooksService, seed_helper: true do + let(:committer) { Gitlab::Git::Committer.new('Jane Doe', 'janedoe@example.com', 'user-456') } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, 'project-123') } let(:service) { described_class.new } before do @blankrev = Gitlab::Git::BLANK_SHA - @oldrev = sample_commit.parent_id - @newrev = sample_commit.id + @oldrev = SeedRepo::Commit::PARENT_ID + @newrev = SeedRepo::Commit::ID @ref = 'refs/heads/feature' end @@ -20,7 +18,7 @@ describe GitHooksService do hook = double(trigger: [true, nil]) expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) - service.execute(user, project, @blankrev, @newrev, @ref) { } + service.execute(committer, repository, @blankrev, @newrev, @ref) { } end end @@ -30,8 +28,8 @@ describe GitHooksService do expect(service).not_to receive(:run_hook).with('post-receive') expect do - service.execute(user, project, @blankrev, @newrev, @ref) - end.to raise_error(GitHooksService::PreReceiveError) + service.execute(committer, repository, @blankrev, @newrev, @ref) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end @@ -42,8 +40,8 @@ describe GitHooksService do expect(service).not_to receive(:run_hook).with('post-receive') expect do - service.execute(user, project, @blankrev, @newrev, @ref) - end.to raise_error(GitHooksService::PreReceiveError) + service.execute(committer, repository, @blankrev, @newrev, @ref) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end end diff --git a/spec/lib/gitlab/git/index_spec.rb b/spec/lib/gitlab/git/index_spec.rb index 21b71654251..73fbc6a6afa 100644 --- a/spec/lib/gitlab/git/index_spec.rb +++ b/spec/lib/gitlab/git/index_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Gitlab::Git::Index, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:index) { described_class.new(repository) } before do diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 8ec8dfe6acf..4cfb4b7d357 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -17,7 +17,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end end - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } describe "Respond to" do subject { repository } @@ -56,14 +56,14 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#rugged" do describe 'when storage is broken', broken_storage: true do it 'raises a storage exception when storage is not available' do - broken_repo = described_class.new('broken', 'a/path.git') + broken_repo = described_class.new('broken', 'a/path.git', '') expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Storage::Inaccessible) end end it 'raises a no repository exception when there is no repo' do - broken_repo = described_class.new('default', 'a/path.git') + broken_repo = described_class.new('default', 'a/path.git', '') expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Repository::NoRepository) end @@ -257,7 +257,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end describe '#submodule_url_for' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:ref) { 'master' } def submodule_url(path) @@ -295,7 +295,7 @@ describe Gitlab::Git::Repository, seed_helper: true do end context '#submodules' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } context 'where repo has submodules' do let(:submodules) { repository.send(:submodules, 'master') } @@ -391,7 +391,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#delete_branch" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.delete_branch("feature") end @@ -407,7 +407,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#create_branch" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') end it "should create a new branch" do @@ -433,6 +433,40 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#delete_refs' do + before(:all) do + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') + end + + it 'deletes the ref' do + @repo.delete_refs('refs/heads/feature') + + expect(@repo.rugged.references['refs/heads/feature']).to be_nil + end + + it 'deletes all refs' do + refs = %w[refs/heads/wip refs/tags/v1.1.0] + @repo.delete_refs(*refs) + + refs.each do |ref| + expect(@repo.rugged.references[ref]).to be_nil + end + end + + it 'raises an error if it failed' do + expect(Gitlab::Popen).to receive(:popen).and_return(['Error', 1]) + + expect do + @repo.delete_refs('refs/heads/fix') + end.to raise_error(Gitlab::Git::Repository::GitError) + end + + after(:all) do + FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH) + ensure_seeds + end + end + describe "#refs_hash" do let(:refs) { repository.refs_hash } @@ -445,7 +479,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#remote_delete" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.remote_delete("expendable") end @@ -461,7 +495,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#remote_add" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.remote_add("new_remote", SeedHelper::GITLAB_GIT_TEST_REPO_URL) end @@ -477,7 +511,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe "#remote_update" do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.remote_update("expendable", url: TEST_NORMAL_REPO_PATH) end @@ -506,7 +540,7 @@ describe Gitlab::Git::Repository, seed_helper: true do before(:context) do # Add new commits so that there's a renamed file in the commit history - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged @commit_with_old_name_id = new_commit_edit_old_file(repo) @rename_commit_id = new_commit_move_file(repo) @commit_with_new_name_id = new_commit_edit_new_file(repo) @@ -514,7 +548,7 @@ describe Gitlab::Git::Repository, seed_helper: true do after(:context) do # Erase our commits so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH).rugged + repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end @@ -849,7 +883,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#autocrlf' do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.rugged.config['core.autocrlf'] = true end @@ -864,7 +898,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#autocrlf=' do before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH) + @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') @repo.rugged.config['core.autocrlf'] = false end @@ -933,7 +967,7 @@ describe Gitlab::Git::Repository, seed_helper: true do context 'with local and remote branches' do let(:repository) do - Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git')) + Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '') end before do @@ -977,6 +1011,36 @@ describe Gitlab::Git::Repository, seed_helper: true do it 'returns the number of branches' do expect(repository.branch_count).to eq(10) end + + context 'with local and remote branches' do + let(:repository) do + Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '') + end + + before do + create_remote_branch(repository, 'joe', 'remote_branch', 'master') + repository.create_branch('local_branch', 'master') + end + + after do + FileUtils.rm_rf(TEST_MUTABLE_REPO_PATH) + ensure_seeds + end + + it 'returns the count of local branches' do + expect(repository.branch_count).to eq(repository.local_branches.count) + end + + context 'with Gitaly disabled' do + before do + allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false) + end + + it 'returns the count of local branches' do + expect(repository.branch_count).to eq(repository.local_branches.count) + end + end + end end describe "#ls_files" do @@ -1080,6 +1144,34 @@ describe Gitlab::Git::Repository, seed_helper: true do end end + describe '#ref_exists?' do + shared_examples 'checks the existence of refs' do + it 'returns true for an existing tag' do + expect(repository.ref_exists?('refs/heads/master')).to eq(true) + end + + it 'returns false for a non-existing tag' do + expect(repository.ref_exists?('refs/tags/THIS_TAG_DOES_NOT_EXIST')).to eq(false) + end + + it 'raises an ArgumentError for an empty string' do + expect { repository.ref_exists?('') }.to raise_error(ArgumentError) + end + + it 'raises an ArgumentError for an invalid ref' do + expect { repository.ref_exists?('INVALID') }.to raise_error(ArgumentError) + end + end + + context 'when Gitaly ref_exists feature is enabled' do + it_behaves_like 'checks the existence of refs' + end + + context 'when Gitaly ref_exists feature is disabled', skip_gitaly_mock: true do + it_behaves_like 'checks the existence of refs' + end + end + describe '#tag_exists?' do shared_examples 'checks the existence of tags' do it 'returns true for an existing tag' do @@ -1128,7 +1220,7 @@ describe Gitlab::Git::Repository, seed_helper: true do describe '#local_branches' do before(:all) do - @repo = Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git')) + @repo = Gitlab::Git::Repository.new('default', File.join(TEST_MUTABLE_REPO_PATH, '.git'), '') end after(:all) do diff --git a/spec/lib/gitlab/git/tag_spec.rb b/spec/lib/gitlab/git/tag_spec.rb index 78d1e120013..cc10679ef1e 100644 --- a/spec/lib/gitlab/git/tag_spec.rb +++ b/spec/lib/gitlab/git/tag_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tag, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } shared_examples 'Gitlab::Git::Repository#tags' do describe 'first tag' do diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index 98ddd3c3664..c07a2d91768 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -1,7 +1,7 @@ require "spec_helper" describe Gitlab::Git::Tree, seed_helper: true do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH) } + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } context :repo do let(:tree) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID) } diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 80dc49e99cb..295a979da76 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -384,7 +384,7 @@ describe Gitlab::GitAccess do def stub_git_hooks # Running the `pre-receive` hook is expensive, and not necessary for this test. - allow_any_instance_of(GitHooksService).to receive(:execute) do |service, &block| + allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) do |service, &block| block.call(service) end end diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index c10427d798f..5b16fc5d084 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -86,6 +86,10 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(event).not_to be_nil end + it 'has the action' do + expect(event.action).not_to be_nil + end + it 'event belongs to note, belongs to merge request, belongs to a project' do expect(event.note.noteable.project).not_to be_nil end diff --git a/spec/lib/gitlab/import_export/repo_restorer_spec.rb b/spec/lib/gitlab/import_export/repo_restorer_spec.rb index 2786bc92fe5..c49af602a01 100644 --- a/spec/lib/gitlab/import_export/repo_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/repo_restorer_spec.rb @@ -34,7 +34,7 @@ describe Gitlab::ImportExport::RepoRestorer do it 'has the webhooks' do restorer.restore - expect(Gitlab::Git::Hook.new('post-receive', project)).to exist + expect(Gitlab::Git::Hook.new('post-receive', project.repository.raw_repository)).to exist end end end diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index cfadee0bcf5..c7930378240 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -186,22 +186,48 @@ describe Gitlab::Shell do end end - describe '#fetch_remote' do + shared_examples 'fetch_remote' do |gitaly_on| + let(:project2) { create(:project, :repository) } + let(:repository) { project2.repository } + def fetch_remote(ssh_auth = nil) - gitlab_shell.fetch_remote('current/storage', 'project/path', 'new/storage', ssh_auth: ssh_auth) + gitlab_shell.fetch_remote(repository.raw_repository, 'new/storage', ssh_auth: ssh_auth) end - def expect_popen(vars = {}) + def expect_popen(fail = false, vars = {}) popen_args = [ projects_path, 'fetch-remote', - 'current/storage', - 'project/path.git', + TestEnv.repos_path, + repository.relative_path, 'new/storage', Gitlab.config.gitlab_shell.git_timeout.to_s ] - expect(Gitlab::Popen).to receive(:popen).with(popen_args, nil, popen_vars.merge(vars)) + return_value = fail ? ["error", 1] : [nil, 0] + + expect(Gitlab::Popen).to receive(:popen).with(popen_args, nil, popen_vars.merge(vars)).and_return(return_value) + end + + def expect_gitaly_call(fail, vars = {}) + receive_fetch_remote = + if fail + receive(:fetch_remote).and_raise(GRPC::NotFound) + else + receive(:fetch_remote).and_return(true) + end + + expect_any_instance_of(Gitlab::GitalyClient::RepositoryService).to receive_fetch_remote + end + + if gitaly_on + def expect_call(fail, vars = {}) + expect_gitaly_call(fail, vars) + end + else + def expect_call(fail, vars = {}) + expect_popen(fail, vars) + end end def build_ssh_auth(opts = {}) @@ -216,20 +242,20 @@ describe Gitlab::Shell do end it 'returns true when the command succeeds' do - expect_popen.and_return([nil, 0]) + expect_call(false) expect(fetch_remote).to be_truthy end it 'raises an exception when the command fails' do - expect_popen.and_return(["error", 1]) + expect_call(true) - expect { fetch_remote }.to raise_error(Gitlab::Shell::Error, "error") + expect { fetch_remote }.to raise_error(Gitlab::Shell::Error) end context 'SSH auth' do it 'passes the SSH key if specified' do - expect_popen('GITLAB_SHELL_SSH_KEY' => 'foo').and_return([nil, 0]) + expect_call(false, 'GITLAB_SHELL_SSH_KEY' => 'foo') ssh_auth = build_ssh_auth(ssh_key_auth?: true, ssh_private_key: 'foo') @@ -237,7 +263,7 @@ describe Gitlab::Shell do end it 'does not pass an empty SSH key' do - expect_popen.and_return([nil, 0]) + expect_call(false) ssh_auth = build_ssh_auth(ssh_key_auth: true, ssh_private_key: '') @@ -245,7 +271,7 @@ describe Gitlab::Shell do end it 'does not pass the key unless SSH key auth is to be used' do - expect_popen.and_return([nil, 0]) + expect_call(false) ssh_auth = build_ssh_auth(ssh_key_auth: false, ssh_private_key: 'foo') @@ -253,7 +279,7 @@ describe Gitlab::Shell do end it 'passes the known_hosts data if specified' do - expect_popen('GITLAB_SHELL_KNOWN_HOSTS' => 'foo').and_return([nil, 0]) + expect_call(false, 'GITLAB_SHELL_KNOWN_HOSTS' => 'foo') ssh_auth = build_ssh_auth(ssh_known_hosts: 'foo') @@ -261,7 +287,7 @@ describe Gitlab::Shell do end it 'does not pass empty known_hosts data' do - expect_popen.and_return([nil, 0]) + expect_call(false) ssh_auth = build_ssh_auth(ssh_known_hosts: '') @@ -269,7 +295,7 @@ describe Gitlab::Shell do end it 'does not pass known_hosts data unless SSH is to be used' do - expect_popen(popen_vars).and_return([nil, 0]) + expect_call(false, popen_vars) ssh_auth = build_ssh_auth(ssh_import?: false, ssh_known_hosts: 'foo') @@ -278,6 +304,14 @@ describe Gitlab::Shell do end end + describe '#fetch_remote local', skip_gitaly_mock: true do + it_should_behave_like 'fetch_remote', false + end + + describe '#fetch_remote gitaly' do + it_should_behave_like 'fetch_remote', true + end + describe '#import_repository' do it 'returns true when the command succeeds' do expect(Gitlab::Popen).to receive(:popen) diff --git a/spec/lib/gitlab/sql/pattern_spec.rb b/spec/lib/gitlab/sql/pattern_spec.rb new file mode 100644 index 00000000000..9d7b2136dab --- /dev/null +++ b/spec/lib/gitlab/sql/pattern_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Gitlab::SQL::Pattern do + describe '.to_pattern' do + subject(:to_pattern) { User.to_pattern(query) } + + context 'when a query is shorter than 3 chars' do + let(:query) { '12' } + + it 'returns exact matching pattern' do + expect(to_pattern).to eq('12') + end + end + + context 'when a query with a escape character is shorter than 3 chars' do + let(:query) { '_2' } + + it 'returns sanitized exact matching pattern' do + expect(to_pattern).to eq('\_2') + end + end + + context 'when a query is equal to 3 chars' do + let(:query) { '123' } + + it 'returns partial matching pattern' do + expect(to_pattern).to eq('%123%') + end + end + + context 'when a query with a escape character is equal to 3 chars' do + let(:query) { '_23' } + + it 'returns partial matching pattern' do + expect(to_pattern).to eq('%\_23%') + end + end + + context 'when a query is longer than 3 chars' do + let(:query) { '1234' } + + it 'returns partial matching pattern' do + expect(to_pattern).to eq('%1234%') + end + end + + context 'when a query with a escape character is longer than 3 chars' do + let(:query) { '_234' } + + it 'returns sanitized partial matching pattern' do + expect(to_pattern).to eq('%\_234%') + end + end + end +end diff --git a/spec/migrations/migrate_pipeline_sidekiq_queues_spec.rb b/spec/migrations/migrate_pipeline_sidekiq_queues_spec.rb new file mode 100644 index 00000000000..e02bcd2f4da --- /dev/null +++ b/spec/migrations/migrate_pipeline_sidekiq_queues_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170822101017_migrate_pipeline_sidekiq_queues.rb') + +describe MigratePipelineSidekiqQueues, :sidekiq, :redis do + include Gitlab::Database::MigrationHelpers + + context 'when there are jobs in the queues' do + it 'correctly migrates queue when migrating up' do + Sidekiq::Testing.disable! do + stubbed_worker(queue: :pipeline).perform_async('Something', [1]) + stubbed_worker(queue: :build).perform_async('Something', [1]) + + described_class.new.up + + expect(sidekiq_queue_length('pipeline')).to eq 0 + expect(sidekiq_queue_length('build')).to eq 0 + expect(sidekiq_queue_length('pipeline_default')).to eq 2 + end + end + + it 'correctly migrates queue when migrating down' do + Sidekiq::Testing.disable! do + stubbed_worker(queue: :pipeline_default).perform_async('Class', [1]) + stubbed_worker(queue: :pipeline_processing).perform_async('Class', [2]) + stubbed_worker(queue: :pipeline_hooks).perform_async('Class', [3]) + stubbed_worker(queue: :pipeline_cache).perform_async('Class', [4]) + + described_class.new.down + + expect(sidekiq_queue_length('pipeline')).to eq 4 + expect(sidekiq_queue_length('pipeline_default')).to eq 0 + expect(sidekiq_queue_length('pipeline_processing')).to eq 0 + expect(sidekiq_queue_length('pipeline_hooks')).to eq 0 + expect(sidekiq_queue_length('pipeline_cache')).to eq 0 + end + end + end + + context 'when there are no jobs in the queues' do + it 'does not raise error when migrating up' do + expect { described_class.new.up }.not_to raise_error + end + + it 'does not raise error when migrating down' do + expect { described_class.new.down }.not_to raise_error + end + end + + def stubbed_worker(queue:) + Class.new do + include Sidekiq::Worker + sidekiq_options queue: queue + end + end +end diff --git a/spec/migrations/migrate_stages_statuses_spec.rb b/spec/migrations/migrate_stages_statuses_spec.rb index 4102d57e368..094c9bc604e 100644 --- a/spec/migrations/migrate_stages_statuses_spec.rb +++ b/spec/migrations/migrate_stages_statuses_spec.rb @@ -12,7 +12,7 @@ describe MigrateStagesStatuses, :migration do before do stub_const("#{described_class.name}::BATCH_SIZE", 2) - stub_const("#{described_class.name}::RANGE_SIZE", 2) + stub_const("#{described_class.name}::RANGE_SIZE", 1) projects.create!(id: 1, name: 'gitlab1', path: 'gitlab1') projects.create!(id: 2, name: 'gitlab2', path: 'gitlab2') @@ -50,9 +50,10 @@ describe MigrateStagesStatuses, :migration do Timecop.freeze do migrate! - expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1, 2) + expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 1, 1) + expect(described_class::MIGRATION).to be_scheduled_migration(5.minutes, 2, 2) expect(described_class::MIGRATION).to be_scheduled_migration(10.minutes, 3, 3) - expect(BackgroundMigrationWorker.jobs.size).to eq 2 + expect(BackgroundMigrationWorker.jobs.size).to eq 3 end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 767f0ad9e65..0c35ad3c9d8 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -21,6 +21,16 @@ describe Ci::Build do it { is_expected.to respond_to(:has_trace?) } it { is_expected.to respond_to(:trace) } + describe 'callbacks' do + context 'when running after_create callback' do + it 'triggers asynchronous build hooks worker' do + expect(BuildHooksWorker).to receive(:perform_async) + + create(:ci_build) + end + end + end + describe '.manual_actions' do let!(:manual_but_created) { create(:ci_build, :manual, status: :created, pipeline: pipeline) } let!(:manual_but_succeeded) { create(:ci_build, :manual, status: :success, pipeline: pipeline) } @@ -1247,8 +1257,12 @@ describe Ci::Build do context 'when build has user' do let(:user_variables) do - [{ key: 'GITLAB_USER_ID', value: user.id.to_s, public: true }, - { key: 'GITLAB_USER_EMAIL', value: user.email, public: true }] + [ + { key: 'GITLAB_USER_ID', value: user.id.to_s, public: true }, + { key: 'GITLAB_USER_EMAIL', value: user.email, public: true }, + { key: 'GITLAB_USER_LOGIN', value: user.username, public: true }, + { key: 'GITLAB_USER_NAME', value: user.name, public: true } + ] end before do diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index ac75c6501ee..b84e3ff18e8 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Ci::Pipeline, :mailer do let(:user) { create(:user) } - let(:project) { create(:project) } + set(:project) { create(:project) } let(:pipeline) do create(:ci_empty_pipeline, status: :created, project: project) @@ -159,6 +159,18 @@ describe Ci::Pipeline, :mailer do end end + describe '#predefined_variables' do + subject { pipeline.predefined_variables } + + it { is_expected.to be_an(Array) } + + it 'includes the defined keys' do + keys = subject.map { |v| v[:key] } + + expect(keys).to include('CI_PIPELINE_ID', 'CI_CONFIG_PATH', 'CI_PIPELINE_SOURCE') + end + end + describe '#auto_canceled?' do subject { pipeline.auto_canceled? } diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index 74c9d6145e2..586d073eb5e 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -38,6 +38,17 @@ describe Ci::Stage, :models do expect(stage.status).to eq 'success' end end + + context 'when stage status is not defined' do + before do + stage.update_column(:status, nil) + end + + it 'sets the default value' do + expect(described_class.find(stage.id).status) + .to eq 'created' + end + end end describe 'update_status' do diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index 63ad3a3630b..34f923d3f0c 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -12,17 +12,25 @@ describe Awardable do describe "ClassMethods" do let!(:issue2) { create(:issue) } + let!(:award_emoji2) { create(:award_emoji, awardable: issue2) } - before do - create(:award_emoji, awardable: issue2) - end + describe "orders" do + it "orders on upvotes" do + expect(Issue.order_upvotes_desc.to_a).to eq [issue2, issue] + end - it "orders on upvotes" do - expect(Issue.order_upvotes_desc.to_a).to eq [issue2, issue] + it "orders on downvotes" do + expect(Issue.order_downvotes_desc.to_a).to eq [issue, issue2] + end end - it "orders on downvotes" do - expect(Issue.order_downvotes_desc.to_a).to eq [issue, issue2] + describe ".awarded" do + it "filters by user and emoji name" do + expect(Issue.awarded(award_emoji.user, "thumbsup")).to be_empty + expect(Issue.awarded(award_emoji.user, "thumbsdown")).to eq [issue] + expect(Issue.awarded(award_emoji2.user, "thumbsup")).to eq [issue2] + expect(Issue.awarded(award_emoji2.user, "thumbsdown")).to be_empty + end end end diff --git a/spec/models/concerns/editable_spec.rb b/spec/models/concerns/editable_spec.rb index cd73af3b480..49a9a8ebcbc 100644 --- a/spec/models/concerns/editable_spec.rb +++ b/spec/models/concerns/editable_spec.rb @@ -1,11 +1,11 @@ require 'spec_helper' describe Editable do - describe '#is_edited?' do + describe '#edited?' do let(:issue) { create(:issue, last_edited_at: nil) } let(:edited_issue) { create(:issue, created_at: 3.days.ago, last_edited_at: 2.days.ago) } - it { expect(issue.is_edited?).to eq(false) } - it { expect(edited_issue.is_edited?).to eq(true) } + it { expect(issue.edited?).to eq(false) } + it { expect(edited_issue.edited?).to eq(true) } end end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index c5bfae47606..f9cd12c0ff3 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -84,6 +84,83 @@ describe Group do expect(group).not_to be_valid end end + + describe '#visibility_level_allowed_by_parent' do + let(:parent) { create(:group, :internal) } + let(:sub_group) { build(:group, parent_id: parent.id) } + + context 'without a parent' do + it 'is valid' do + sub_group.parent_id = nil + + expect(sub_group).to be_valid + end + end + + context 'with a parent' do + context 'when visibility of sub group is greater than the parent' do + it 'is invalid' do + sub_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(sub_group).to be_invalid + end + end + + context 'when visibility of sub group is lower or equal to the parent' do + [Gitlab::VisibilityLevel::INTERNAL, Gitlab::VisibilityLevel::PRIVATE].each do |level| + it 'is valid' do + sub_group.visibility_level = level + + expect(sub_group).to be_valid + end + end + end + end + end + + describe '#visibility_level_allowed_by_projects' do + let!(:internal_group) { create(:group, :internal) } + let!(:internal_project) { create(:project, :internal, group: internal_group) } + + context 'when group has a lower visibility' do + it 'is invalid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE + + expect(internal_group).to be_invalid + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since this group contains projects with higher visibility.') + end + end + + context 'when group has a higher visibility' do + it 'is valid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(internal_group).to be_valid + end + end + end + + describe '#visibility_level_allowed_by_sub_groups' do + let!(:internal_group) { create(:group, :internal) } + let!(:internal_sub_group) { create(:group, :internal, parent: internal_group) } + + context 'when parent group has a lower visibility' do + it 'is invalid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PRIVATE + + expect(internal_group).to be_invalid + expect(internal_group.errors[:visibility_level]).to include('private is not allowed since there are sub-groups with higher visibility.') + end + end + + context 'when parent group has a higher visibility' do + it 'is valid' do + internal_group.visibility_level = Gitlab::VisibilityLevel::PUBLIC + + expect(internal_group).to be_valid + end + end + end end describe '.visible_to_user' do diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index de86788d142..e547da0cfbe 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -769,4 +769,22 @@ describe Issue do expect(described_class.public_only).to eq([public_issue]) end end + + describe '#update_project_counter_caches?' do + it 'returns true when the state changes' do + subject.state = 'closed' + + expect(subject.update_project_counter_caches?).to eq(true) + end + + it 'returns true when the confidential flag changes' do + subject.confidential = true + + expect(subject.update_project_counter_caches?).to eq(true) + end + + it 'returns false when the state or confidential flag did not change' do + expect(subject.update_project_counter_caches?).to eq(false) + end + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 1c72513520d..09f3b97ec58 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -604,7 +604,7 @@ describe MergeRequest do request = build_stubbed(:merge_request) expect(request.merge_commit_message) - .to match("See merge request #{request.to_reference}") + .to match("See merge request #{request.to_reference(full: true)}") end it 'excludes multiple linebreak runs when description is blank' do @@ -931,6 +931,23 @@ describe MergeRequest do end end + describe '#merge_async' do + it 'enqueues MergeWorker job and updates merge_jid' do + merge_request = create(:merge_request) + user_id = double(:user_id) + params = double(:params) + merge_jid = 'hash-123' + + expect(MergeWorker).to receive(:perform_async).with(merge_request.id, user_id, params) do + merge_jid + end + + merge_request.merge_async(user_id, params) + + expect(merge_request.reload.merge_jid).to eq(merge_jid) + end + end + describe '#check_if_can_be_merged' do let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } @@ -1370,29 +1387,11 @@ describe MergeRequest do end describe '#merge_ongoing?' do - it 'returns true when merge process is ongoing for merge_jid' do - merge_request = create(:merge_request, merge_jid: 'foo') - - allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']).and_return(1) + it 'returns true when merge_id is present and MR is not merged' do + merge_request = build_stubbed(:merge_request, state: :open, merge_jid: 'foo') expect(merge_request.merge_ongoing?).to be(true) end - - it 'returns false when no merge process running for merge_jid' do - merge_request = build(:merge_request, merge_jid: 'foo') - - allow(Gitlab::SidekiqStatus).to receive(:num_running).with(['foo']).and_return(0) - - expect(merge_request.merge_ongoing?).to be(false) - end - - it 'returns false when merge_jid is nil' do - merge_request = build(:merge_request, merge_jid: nil) - - expect(Gitlab::SidekiqStatus).not_to receive(:num_running) - - expect(merge_request.merge_ongoing?).to be(false) - end end describe "#closed_without_fork?" do @@ -1701,4 +1700,16 @@ describe MergeRequest do .to change { project.open_merge_requests_count }.from(1).to(0) end end + + describe '#update_project_counter_caches?' do + it 'returns true when the state changes' do + subject.state = 'closed' + + expect(subject.update_project_counter_caches?).to eq(true) + end + + it 'returns false when the state did not change' do + expect(subject.update_project_counter_caches?).to eq(false) + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 2e613c44357..3621023f4ca 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1563,10 +1563,18 @@ describe Project do describe 'project import state transitions' do context 'state transition: [:started] => [:finished]' do - let(:housekeeping_service) { spy } + let(:after_import_service) { spy(:after_import_service) } + let(:housekeeping_service) { spy(:housekeeping_service) } before do - allow(Projects::HousekeepingService).to receive(:new) { housekeeping_service } + allow(Projects::AfterImportService) + .to receive(:new) { after_import_service } + + allow(after_import_service) + .to receive(:execute) { housekeeping_service.execute } + + allow(Projects::HousekeepingService) + .to receive(:new) { housekeeping_service } end it 'resets project import_error' do @@ -1581,6 +1589,7 @@ describe Project do project.import_finish + expect(after_import_service).to have_received(:execute) expect(housekeeping_service).to have_received(:execute) end @@ -2225,6 +2234,28 @@ describe Project do end end + describe '#pages_available?' do + let(:project) { create(:project, group: group) } + + subject { project.pages_available? } + + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + end + + context 'when the project is in a top level namespace' do + let(:group) { create(:group) } + + it { is_expected.to be(true) } + end + + context 'when the project is in a subgroup' do + let(:group) { create(:group, :nested) } + + it { is_expected.to be(false) } + end + end + describe '#remove_private_deploy_keys' do let!(:project) { create(:project) } diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 4926d5d6c49..34e1a955309 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -8,6 +8,7 @@ describe Repository, models: true do let(:repository) { project.repository } let(:broken_repository) { create(:project, :broken_storage).repository } let(:user) { create(:user) } + let(:committer) { Gitlab::Git::Committer.from_user(user) } let(:commit_options) do author = repository.user_to_committer(user) @@ -846,7 +847,7 @@ describe Repository, models: true do expect do repository.add_branch(user, 'new_feature', 'master') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end it 'does not create the branch' do @@ -854,7 +855,7 @@ describe Repository, models: true do expect do repository.add_branch(user, 'new_feature', 'master') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) expect(repository.find_branch('new_feature')).to be_nil end end @@ -884,8 +885,8 @@ describe Repository, models: true do context 'when pre hooks were successful' do it 'runs without errors' do - expect_any_instance_of(GitHooksService).to receive(:execute) - .with(user, project, old_rev, blank_sha, 'refs/heads/feature') + expect_any_instance_of(Gitlab::Git::HooksService).to receive(:execute) + .with(committer, repository, old_rev, blank_sha, 'refs/heads/feature') expect { repository.rm_branch(user, 'feature') }.not_to raise_error end @@ -905,7 +906,7 @@ describe Repository, models: true do expect do repository.rm_branch(user, 'feature') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end it 'does not delete the branch' do @@ -913,7 +914,7 @@ describe Repository, models: true do expect do repository.rm_branch(user, 'feature') - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) expect(repository.find_branch('feature')).not_to be_nil end end @@ -922,26 +923,29 @@ describe Repository, models: true do describe '#update_branch_with_hooks' do let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature let(:new_rev) { 'a74ae73c1ccde9b974a70e82b901588071dc142a' } # commit whose parent is old_rev + let(:updating_ref) { 'refs/heads/feature' } + let(:target_project) { project } + let(:target_repository) { target_project.repository } context 'when pre hooks were successful' do before do - service = GitHooksService.new - expect(GitHooksService).to receive(:new).and_return(service) + service = Gitlab::Git::HooksService.new + expect(Gitlab::Git::HooksService).to receive(:new).and_return(service) expect(service).to receive(:execute) - .with(user, project, old_rev, new_rev, 'refs/heads/feature') + .with(committer, target_repository, old_rev, new_rev, updating_ref) .and_yield(service).and_return(true) end it 'runs without errors' do expect do - GitOperationService.new(user, repository).with_branch('feature') do + GitOperationService.new(committer, repository).with_branch('feature') do new_rev end end.not_to raise_error end it 'ensures the autocrlf Git option is set to :input' do - service = GitOperationService.new(user, repository) + service = GitOperationService.new(committer, repository) expect(service).to receive(:update_autocrlf_option) @@ -952,13 +956,44 @@ describe Repository, models: true do it 'updates the head' do expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev) - GitOperationService.new(user, repository).with_branch('feature') do + GitOperationService.new(committer, repository).with_branch('feature') do new_rev end expect(repository.find_branch('feature').dereferenced_target.id).to eq(new_rev) end end + + context 'when target project does not have the commit' do + let(:target_project) { create(:project, :empty_repo) } + let(:old_rev) { Gitlab::Git::BLANK_SHA } + let(:new_rev) { project.commit('feature').sha } + let(:updating_ref) { 'refs/heads/master' } + + it 'fetch_ref and create the branch' do + expect(target_project.repository).to receive(:fetch_ref) + .and_call_original + + GitOperationService.new(committer, target_repository) + .with_branch( + 'master', + start_project: project, + start_branch_name: 'feature') { new_rev } + + expect(target_repository.branch_names).to contain_exactly('master') + end + end + + context 'when target project already has the commit' do + let(:target_project) { create(:project, :repository) } + + it 'does not fetch_ref and just pass the commit' do + expect(target_repository).not_to receive(:fetch_ref) + + GitOperationService.new(committer, target_repository) + .with_branch('feature', start_project: project) { new_rev } + end + end end context 'when temporary ref failed to be created from other project' do @@ -974,7 +1009,7 @@ describe Repository, models: true do end expect do - GitOperationService.new(user, target_project.repository) + GitOperationService.new(committer, target_project.repository) .with_branch('feature', start_project: project, &:itself) @@ -996,7 +1031,7 @@ describe Repository, models: true do repository.add_branch(user, branch, old_rev) expect do - GitOperationService.new(user, repository).with_branch(branch) do + GitOperationService.new(committer, repository).with_branch(branch) do new_rev end end.not_to raise_error @@ -1014,7 +1049,7 @@ describe Repository, models: true do # Updating 'master' to new_rev would lose the commits on 'master' that # are not contained in new_rev. This should not be allowed. expect do - GitOperationService.new(user, repository).with_branch(branch) do + GitOperationService.new(committer, repository).with_branch(branch) do new_rev end end.to raise_error(Repository::CommitError) @@ -1026,10 +1061,10 @@ describe Repository, models: true do allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, '']) expect do - GitOperationService.new(user, repository).with_branch('feature') do + GitOperationService.new(committer, repository).with_branch('feature') do new_rev end - end.to raise_error(GitHooksService::PreReceiveError) + end.to raise_error(Gitlab::Git::HooksService::PreReceiveError) end end @@ -1044,7 +1079,7 @@ describe Repository, models: true do expect(repository).not_to receive(:expire_emptiness_caches) expect(repository).to receive(:expire_branches_cache) - GitOperationService.new(user, repository) + GitOperationService.new(committer, repository) .with_branch('new-feature') do new_rev end @@ -2035,23 +2070,23 @@ describe Repository, models: true do end end - describe '#is_ancestor?' do + describe '#ancestor?' do let(:commit) { repository.commit } let(:ancestor) { commit.parents.first } context 'with Gitaly enabled' do it 'it is an ancestor' do - expect(repository.is_ancestor?(ancestor.id, commit.id)).to eq(true) + expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true) end it 'it is not an ancestor' do - expect(repository.is_ancestor?(commit.id, ancestor.id)).to eq(false) + expect(repository.ancestor?(commit.id, ancestor.id)).to eq(false) end it 'returns false on nil-values' do - expect(repository.is_ancestor?(nil, commit.id)).to eq(false) - expect(repository.is_ancestor?(ancestor.id, nil)).to eq(false) - expect(repository.is_ancestor?(nil, nil)).to eq(false) + expect(repository.ancestor?(nil, commit.id)).to eq(false) + expect(repository.ancestor?(ancestor.id, nil)).to eq(false) + expect(repository.ancestor?(nil, nil)).to eq(false) end end @@ -2062,17 +2097,17 @@ describe Repository, models: true do end it 'it is an ancestor' do - expect(repository.is_ancestor?(ancestor.id, commit.id)).to eq(true) + expect(repository.ancestor?(ancestor.id, commit.id)).to eq(true) end it 'it is not an ancestor' do - expect(repository.is_ancestor?(commit.id, ancestor.id)).to eq(false) + expect(repository.ancestor?(commit.id, ancestor.id)).to eq(false) end it 'returns false on nil-values' do - expect(repository.is_ancestor?(nil, commit.id)).to eq(false) - expect(repository.is_ancestor?(ancestor.id, nil)).to eq(false) - expect(repository.is_ancestor?(nil, nil)).to eq(false) + expect(repository.ancestor?(nil, commit.id)).to eq(false) + expect(repository.ancestor?(ancestor.id, nil)).to eq(false) + expect(repository.ancestor?(nil, nil)).to eq(false) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 9a9e255f874..b70ab5581ac 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -789,6 +789,7 @@ describe User do describe '.search' do let!(:user) { create(:user, name: 'user', username: 'usern', email: 'email@gmail.com') } let!(:user2) { create(:user, name: 'user name', username: 'username', email: 'someemail@gmail.com') } + let!(:user3) { create(:user, name: 'us', username: 'se', email: 'foo@gmail.com') } describe 'name matching' do it 'returns users with a matching name with exact match first' do @@ -802,6 +803,14 @@ describe User do it 'returns users with a matching name regardless of the casing' do expect(described_class.search(user2.name.upcase)).to eq([user2]) end + + it 'returns users with a exact matching name shorter than 3 chars' do + expect(described_class.search(user3.name)).to eq([user3]) + end + + it 'returns users with a exact matching name shorter than 3 chars regardless of the casing' do + expect(described_class.search(user3.name.upcase)).to eq([user3]) + end end describe 'email matching' do @@ -830,6 +839,14 @@ describe User do it 'returns users with a matching username regardless of the casing' do expect(described_class.search(user2.username.upcase)).to eq([user2]) end + + it 'returns users with a exact matching username shorter than 3 chars' do + expect(described_class.search(user3.username)).to eq([user3]) + end + + it 'returns users with a exact matching username shorter than 3 chars regardless of the casing' do + expect(described_class.search(user3.username.upcase)).to eq([user3]) + end end end @@ -1356,7 +1373,7 @@ describe User do end it "excludes push event if branch has been deleted" do - allow_any_instance_of(Repository).to receive(:branch_names).and_return(['foo']) + allow_any_instance_of(Repository).to receive(:branch_exists?).with('master').and_return(false) expect(subject.recent_push).to eq(nil) end diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb index 1dd9f3f6ddc..593068b8cd7 100644 --- a/spec/requests/api/award_emoji_spec.rb +++ b/spec/requests/api/award_emoji_spec.rb @@ -253,6 +253,10 @@ describe API::AwardEmoji do expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/issues/#{issue.iid}/award_emoji/#{award_emoji.id}", user) } + end end context 'when the awardable is a Merge Request' do @@ -269,6 +273,10 @@ describe API::AwardEmoji do expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/award_emoji/#{downvote.id}", user) } + end end context 'when the awardable is a Snippet' do @@ -282,6 +290,10 @@ describe API::AwardEmoji do expect(response).to have_http_status(204) end.to change { snippet.award_emoji.count }.from(1).to(0) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/snippets/#{snippet.id}/award_emoji/#{award.id}", user) } + end end end @@ -295,5 +307,9 @@ describe API::AwardEmoji do expect(response).to have_http_status(204) end.to change { note.award_emoji.count }.from(1).to(0) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{note.id}/award_emoji/#{rocket.id}", user) } + end end end diff --git a/spec/requests/api/boards_spec.rb b/spec/requests/api/boards_spec.rb index 43b381c2219..f698d5dddb3 100644 --- a/spec/requests/api/boards_spec.rb +++ b/spec/requests/api/boards_spec.rb @@ -195,6 +195,10 @@ describe API::Boards do expect(response).to have_http_status(204) end + + it_behaves_like '412 response' do + let(:request) { api("#{base_url}/#{dev_list.id}", owner) } + end end end end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index 5a2e1b2cf2d..b1e011de604 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -499,6 +499,10 @@ describe API::Branches do expect(response).to have_gitlab_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/repository/branches/#{branch_name}", user) } + end end describe 'DELETE /projects/:id/repository/merged_branches' do diff --git a/spec/requests/api/broadcast_messages_spec.rb b/spec/requests/api/broadcast_messages_spec.rb index 67989689799..b043a333d33 100644 --- a/spec/requests/api/broadcast_messages_spec.rb +++ b/spec/requests/api/broadcast_messages_spec.rb @@ -171,6 +171,10 @@ describe API::BroadcastMessages do expect(response).to have_http_status(403) end + it_behaves_like '412 response' do + let(:request) { api("/broadcast_messages/#{message.id}", admin) } + end + it 'deletes the broadcast message for admins' do expect do delete api("/broadcast_messages/#{message.id}", admin) diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index e497ec333a2..684877c33c0 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -190,6 +190,10 @@ describe API::DeployKeys do expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/deploy_keys/#{deploy_key.id}", admin) } + end end describe 'POST /projects/:id/deploy_keys/:key_id/enable' do diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index 87716c6fe3a..2361809e0e1 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -138,6 +138,10 @@ describe API::Environments do expect(response).to have_http_status(404) expect(json_response['message']).to eq('404 Not found') end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/environments/#{environment.id}", user) } + end end context 'a non member' do diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index ea97c556430..971eaf837cb 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -125,6 +125,15 @@ describe API::Files do expect(response).to have_http_status(200) end + it 'returns raw file info for files with dots' do + url = route('.gitignore') + "/raw" + expect(Gitlab::Workhorse).to receive(:send_git_blob) + + get api(url, current_user), params + + expect(response).to have_http_status(200) + end + it 'returns file by commit sha' do # This file is deleted on HEAD file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee" diff --git a/spec/requests/api/group_variables_spec.rb b/spec/requests/api/group_variables_spec.rb index 2179790d098..93b9cf85c1d 100644 --- a/spec/requests/api/group_variables_spec.rb +++ b/spec/requests/api/group_variables_spec.rb @@ -200,6 +200,10 @@ describe API::GroupVariables do expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/groups/#{group.id}/variables/#{variable.key}", user) } + end end context 'authorized user with invalid permissions' do diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 313c38cd29c..77c43f92456 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -20,10 +20,15 @@ describe API::Groups do describe "GET /groups" do context "when unauthenticated" do - it "returns authentication error" do + it "returns public groups" do get api("/groups") - expect(response).to have_http_status(401) + expect(response).to have_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.length).to eq(1) + expect(json_response) + .to satisfy_one { |group| group['name'] == group1.name } end end @@ -165,6 +170,18 @@ describe API::Groups do end describe "GET /groups/:id" do + context 'when unauthenticated' do + it 'returns 404 for a private group' do + get api("/groups/#{group2.id}") + expect(response).to have_http_status(404) + end + + it 'returns 200 for a public group' do + get api("/groups/#{group1.id}") + expect(response).to have_http_status(200) + end + end + context "when authenticated as user" do it "returns one of user1's groups" do project = create(:project, namespace: group2, path: 'Foo') @@ -427,6 +444,7 @@ describe API::Groups do expect(json_response["name"]).to eq(group[:name]) expect(json_response["path"]).to eq(group[:path]) expect(json_response["request_access_enabled"]).to eq(group[:request_access_enabled]) + expect(json_response["visibility"]).to eq(Gitlab::VisibilityLevel.string_level(Gitlab::CurrentSettings.current_application_settings.default_group_visibility)) end it "creates a nested group", :nested_groups do @@ -471,6 +489,10 @@ describe API::Groups do expect(response).to have_http_status(204) end + it_behaves_like '412 response' do + let(:request) { api("/groups/#{group1.id}", user1) } + end + it "does not remove a group if not an owner" do user4 = create(:user) group1.add_master(user4) diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb index 7d120e4a234..dee75c96b86 100644 --- a/spec/requests/api/issues_spec.rb +++ b/spec/requests/api/issues_spec.rb @@ -509,6 +509,18 @@ describe API::Issues, :mailer do describe "GET /projects/:id/issues" do let(:base_url) { "/projects/#{project.id}" } + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new do + get api("/projects/#{project.id}/issues", user) + end.count + + create(:issue, author: user, project: project) + + expect do + get api("/projects/#{project.id}/issues", user) + end.not_to exceed_query_limit(control_count) + end + it 'returns 404 when project does not exist' do get api('/projects/1000/issues', non_member) @@ -984,7 +996,7 @@ describe API::Issues, :mailer do describe 'POST /projects/:id/issues with spam filtering' do before do allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) - allow_any_instance_of(AkismetService).to receive_messages(is_spam?: true) + allow_any_instance_of(AkismetService).to receive_messages(spam?: true) end let(:params) do @@ -1114,7 +1126,7 @@ describe API::Issues, :mailer do it "does not create a new project issue" do allow_any_instance_of(SpamService).to receive_messages(check_for_spam?: true) - allow_any_instance_of(AkismetService).to receive_messages(is_spam?: true) + allow_any_instance_of(AkismetService).to receive_messages(spam?: true) put api("/projects/#{project.id}/issues/#{issue.iid}", user), params @@ -1304,6 +1316,10 @@ describe API::Issues, :mailer do expect(response).to have_http_status(204) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/issues/#{issue.iid}", owner) } + end end context 'when issue does not exist' do diff --git a/spec/requests/api/labels_spec.rb b/spec/requests/api/labels_spec.rb index 5a4257d1009..b231fdea2a3 100644 --- a/spec/requests/api/labels_spec.rb +++ b/spec/requests/api/labels_spec.rb @@ -189,6 +189,11 @@ describe API::Labels do delete api("/projects/#{project.id}/labels", user) expect(response).to have_http_status(400) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/labels", user) } + let(:params) { { name: 'label1' } } + end end describe 'PUT /projects/:id/labels' do diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index 06aca698c91..d3bae8d2888 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -284,6 +284,10 @@ describe API::Members do expect(response).to have_http_status(204) end.to change { source.members.count }.by(-1) end + + it_behaves_like '412 response' do + let(:request) { api("/#{source_type.pluralize}/#{source.id}/members/#{developer.id}", master) } + end end it 'returns 404 if member does not exist' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 0db645863fb..9027090aabd 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -698,6 +698,10 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) } + end end end diff --git a/spec/requests/api/notes_spec.rb b/spec/requests/api/notes_spec.rb index 75e5062a99c..f5882c0c74a 100644 --- a/spec/requests/api/notes_spec.rb +++ b/spec/requests/api/notes_spec.rb @@ -390,6 +390,10 @@ describe API::Notes do expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/issues/#{issue.iid}/notes/#{issue_note.id}", user) } + end end context 'when noteable is a Snippet' do @@ -410,6 +414,10 @@ describe API::Notes do expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/snippets/#{snippet.id}/notes/#{snippet_note.id}", user) } + end end context 'when noteable is a Merge Request' do @@ -430,6 +438,10 @@ describe API::Notes do expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/notes/#{merge_request_note.id}", user) } + end end end end diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 1fc0ec528b9..b6a5a7ffbb5 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -267,8 +267,7 @@ describe API::PipelineSchedules do delete api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", master) end.to change { project.pipeline_schedules.count }.by(-1) - expect(response).to have_http_status(:accepted) - expect(response).to match_response_schema('pipeline_schedule') + expect(response).to have_http_status(204) end it 'responds with 404 Not Found if requesting non-existing pipeline_schedule' do @@ -276,6 +275,10 @@ describe API::PipelineSchedules do expect(response).to have_http_status(:not_found) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/pipeline_schedules/#{pipeline_schedule.id}", master) } + end end context 'authenticated user with invalid permissions' do diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 2829c243af3..ac3bab09c4c 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -212,5 +212,9 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(response).to have_http_status(404) expect(WebHook.exists?(hook.id)).to be_truthy end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/hooks/#{hook.id}", user) } + end end end diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 2b541f5719e..db34149eb73 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -117,7 +117,7 @@ describe API::ProjectSnippets do end before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do @@ -179,7 +179,7 @@ describe API::ProjectSnippets do end before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do @@ -228,9 +228,6 @@ describe API::ProjectSnippets do let(:snippet) { create(:project_snippet, author: admin) } it 'deletes snippet' do - admin = create(:admin) - snippet = create(:project_snippet, author: admin) - delete api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin) expect(response).to have_http_status(204) @@ -242,6 +239,10 @@ describe API::ProjectSnippets do expect(response).to have_http_status(404) expect(json_response['message']).to eq('404 Snippet Not Found') end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/", admin) } + end end describe 'GET /projects/:project_id/snippets/:id/raw' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index a89a58ff713..4490e50702b 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1029,6 +1029,10 @@ describe API::Projects do delete api("/projects/#{project.id}/snippets/1234", user) expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/snippets/#{snippet.id}", user) } + end end describe 'GET /projects/:id/snippets/:snippet_id/raw' do @@ -1104,23 +1108,31 @@ describe API::Projects do project_fork_target.group.add_developer user2 end - it 'is forbidden to non-owner users' do - delete api("/projects/#{project_fork_target.id}/fork", user2) - expect(response).to have_http_status(403) - end + context 'for a forked project' do + before do + post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) + project_fork_target.reload + expect(project_fork_target.forked_from_project).not_to be_nil + expect(project_fork_target.forked?).to be_truthy + end - it 'makes forked project unforked' do - post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) - project_fork_target.reload - expect(project_fork_target.forked_from_project).not_to be_nil - expect(project_fork_target.forked?).to be_truthy + it 'makes forked project unforked' do + delete api("/projects/#{project_fork_target.id}/fork", admin) - delete api("/projects/#{project_fork_target.id}/fork", admin) + expect(response).to have_http_status(204) + project_fork_target.reload + expect(project_fork_target.forked_from_project).to be_nil + expect(project_fork_target.forked?).not_to be_truthy + end - expect(response).to have_http_status(204) - project_fork_target.reload - expect(project_fork_target.forked_from_project).to be_nil - expect(project_fork_target.forked?).not_to be_truthy + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project_fork_target.id}/fork", admin) } + end + end + + it 'is forbidden to non-owner users' do + delete api("/projects/#{project_fork_target.id}/fork", user2) + expect(response).to have_http_status(403) end it 'is idempotent if not forked' do @@ -1188,14 +1200,23 @@ describe API::Projects do end describe 'DELETE /projects/:id/share/:group_id' do - it 'returns 204 when deleting a group share' do - group = create(:group, :public) - create(:project_group_link, group: group, project: project) + context 'for a valid group' do + let(:group) { create(:group, :public) } + + before do + create(:project_group_link, group: group, project: project) + end + + it 'returns 204 when deleting a group share' do + delete api("/projects/#{project.id}/share/#{group.id}", user) - delete api("/projects/#{project.id}/share/#{group.id}", user) + expect(response).to have_http_status(204) + expect(project.project_group_links).to be_empty + end - expect(response).to have_http_status(204) - expect(project.project_group_links).to be_empty + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/share/#{group.id}", user) } + end end it 'returns a 400 when group id is not an integer' do @@ -1519,6 +1540,11 @@ describe API::Projects do expect(json_response['message']).to eql('202 Accepted') end + it_behaves_like '412 response' do + let(:success_status) { 202 } + let(:request) { api("/projects/#{project.id}", user) } + end + it 'does not remove a project if not an owner' do user3 = create(:user) project.team << [user3, :developer] @@ -1549,6 +1575,11 @@ describe API::Projects do delete api('/projects/1328', admin) expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:success_status) { 202 } + let(:request) { api("/projects/#{project.id}", admin) } + end end end diff --git a/spec/requests/api/protected_branches_spec.rb b/spec/requests/api/protected_branches_spec.rb index 1aa8a95780e..07d7f96bd70 100644 --- a/spec/requests/api/protected_branches_spec.rb +++ b/spec/requests/api/protected_branches_spec.rb @@ -213,6 +213,10 @@ describe API::ProtectedBranches do expect(response).to have_gitlab_http_status(204) end + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/protected_branches/#{branch_name}", user) } + end + it "returns 404 if branch does not exist" do delete api("/projects/#{project.id}/protected_branches/barfoo", user) diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index e9ee3dd679d..993164aa8fe 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -149,6 +149,11 @@ describe API::Runner do expect(response).to have_http_status 204 expect(Ci::Runner.count).to eq(0) end + + it_behaves_like '412 response' do + let(:request) { api('/runners') } + let(:params) { { token: runner.token } } + end end end diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index c8ff25f70fa..244895a417e 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -279,6 +279,10 @@ describe API::Runners do expect(response).to have_http_status(204) end.to change { Ci::Runner.shared.count }.by(-1) end + + it_behaves_like '412 response' do + let(:request) { api("/runners/#{shared_runner.id}", admin) } + end end context 'when runner is not shared' do @@ -332,6 +336,10 @@ describe API::Runners do expect(response).to have_http_status(204) end.to change { Ci::Runner.specific.count }.by(-1) end + + it_behaves_like '412 response' do + let(:request) { api("/runners/#{specific_runner.id}", user) } + end end end @@ -463,6 +471,10 @@ describe API::Runners do expect(response).to have_http_status(204) end.to change { project.runners.count }.by(-1) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/runners/#{two_projects_runner.id}", user) } + end end context 'when runner have one associated projects' do diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index d09b8bc42f1..d3905f698bd 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -137,7 +137,7 @@ describe API::Snippets do end before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do @@ -209,7 +209,7 @@ describe API::Snippets do end before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do @@ -270,6 +270,10 @@ describe API::Snippets do expect(response).to have_http_status(404) expect(json_response['message']).to eq('404 Snippet Not Found') end + + it_behaves_like '412 response' do + let(:request) { api("/snippets/#{public_snippet.id}", user) } + end end describe "GET /snippets/:id/user_agent_detail" do diff --git a/spec/requests/api/system_hooks_spec.rb b/spec/requests/api/system_hooks_spec.rb index f65b475fe44..216d278ad21 100644 --- a/spec/requests/api/system_hooks_spec.rb +++ b/spec/requests/api/system_hooks_spec.rb @@ -102,5 +102,9 @@ describe API::SystemHooks do expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/hooks/#{hook.id}", admin) } + end end end diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index 9884c1ec206..0bf7863bdc8 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -278,12 +278,16 @@ describe API::Tags do expect(response).to have_gitlab_http_status(204) end + it_behaves_like '412 response' do + let(:request) { api(route, current_user) } + end + context 'when tag does not exist' do let(:tag_name) { 'unknown' } it_behaves_like '404 response' do let(:request) { delete api(route, current_user) } - let(:message) { 'No such tag' } + let(:message) { '404 Tag Not Found' } end end diff --git a/spec/requests/api/triggers_spec.rb b/spec/requests/api/triggers_spec.rb index 572e9a0fd07..922b99a6cba 100644 --- a/spec/requests/api/triggers_spec.rb +++ b/spec/requests/api/triggers_spec.rb @@ -8,8 +8,8 @@ describe API::Triggers do let!(:project) { create(:project, :repository, creator: user) } let!(:master) { create(:project_member, :master, user: user, project: project) } let!(:developer) { create(:project_member, :developer, user: user2, project: project) } - let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } - let!(:trigger2) { create(:ci_trigger, project: project, token: trigger_token_2) } + let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token, owner: user) } + let!(:trigger2) { create(:ci_trigger, project: project, token: trigger_token_2, owner: user2) } let!(:trigger_request) { create(:ci_trigger_request, trigger: trigger, created_at: '2015-01-01 12:13:14') } describe 'POST /projects/:project_id/trigger/pipeline' do @@ -22,7 +22,6 @@ describe API::Triggers do before do stub_ci_pipeline_to_return_yaml_file - trigger.update(owner: user) end context 'Handles errors' do @@ -85,6 +84,22 @@ describe API::Triggers do expect(pipeline.variables.map { |v| { v.key => v.value } }.last).to eq(variables) end end + + context 'when legacy trigger' do + before do + trigger.update(owner: nil) + end + + it 'creates pipeline' do + post api("/projects/#{project.id}/trigger/pipeline"), options.merge(ref: 'master') + + expect(response).to have_http_status(201) + expect(json_response).to include('id' => pipeline.id) + pipeline.builds.reload + expect(pipeline.builds.pending.size).to eq(2) + expect(pipeline.builds.size).to eq(5) + end + end end context 'when triggering a pipeline from a trigger token' do @@ -254,8 +269,6 @@ describe API::Triggers do describe 'POST /projects/:id/triggers/:trigger_id/take_ownership' do context 'authenticated user with valid permissions' do it 'updates owner' do - expect(trigger.owner).to be_nil - post api("/projects/#{project.id}/triggers/#{trigger.id}/take_ownership", user) expect(response).to have_http_status(200) @@ -296,6 +309,10 @@ describe API::Triggers do expect(response).to have_http_status(404) end + + it_behaves_like '412 response' do + let(:request) { api("/projects/#{project.id}/triggers/#{trigger.id}", user) } + end end context 'authenticated user with invalid permissions' do diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 49739a1601a..5fef4437997 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -733,6 +733,10 @@ describe API::Users do end.to change { user.keys.count }.by(-1) end + it_behaves_like '412 response' do + let(:request) { api("/users/#{user.id}/keys/#{key.id}", admin) } + end + it 'returns 404 error if user not found' do user.keys << key user.save @@ -838,6 +842,10 @@ describe API::Users do end.to change { user.emails.count }.by(-1) end + it_behaves_like '412 response' do + let(:request) { api("/users/#{user.id}/emails/#{email.id}", admin) } + end + it 'returns 404 error if user not found' do user.emails << email user.save @@ -876,6 +884,10 @@ describe API::Users do expect { Namespace.find(namespace.id) }.to raise_error ActiveRecord::RecordNotFound end + it_behaves_like '412 response' do + let(:request) { api("/users/#{user.id}", admin) } + end + it "does not delete for unauthenticated user" do Sidekiq::Testing.inline! { delete api("/users/#{user.id}") } expect(response).to have_http_status(401) @@ -1116,6 +1128,10 @@ describe API::Users do end.to change { user.keys.count}.by(-1) end + it_behaves_like '412 response' do + let(:request) { api("/user/keys/#{key.id}", user) } + end + it "returns 404 if key ID not found" do delete api("/user/keys/42", user) @@ -1239,6 +1255,10 @@ describe API::Users do end.to change { user.emails.count}.by(-1) end + it_behaves_like '412 response' do + let(:request) { api("/user/emails/#{email.id}", user) } + end + it "returns 404 if email ID not found" do delete api("/user/emails/42", user) @@ -1551,6 +1571,10 @@ describe API::Users do expect(json_response['message']).to eq('403 Forbidden') end + it_behaves_like '412 response' do + let(:request) { api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", admin) } + end + it 'revokes a impersonation token' do delete api("/users/#{user.id}/impersonation_tokens/#{impersonation_token.id}", admin) diff --git a/spec/requests/api/v3/issues_spec.rb b/spec/requests/api/v3/issues_spec.rb index 9eb538c4b09..9a0e6647ebf 100644 --- a/spec/requests/api/v3/issues_spec.rb +++ b/spec/requests/api/v3/issues_spec.rb @@ -884,7 +884,7 @@ describe API::V3::Issues, :mailer do describe 'POST /projects/:id/issues with spam filtering' do before do allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true) - allow_any_instance_of(AkismetService).to receive_messages(is_spam?: true) + allow_any_instance_of(AkismetService).to receive_messages(spam?: true) end let(:params) do @@ -1016,7 +1016,7 @@ describe API::V3::Issues, :mailer do it "does not create a new project issue" do allow_any_instance_of(SpamService).to receive_messages(check_for_spam?: true) - allow_any_instance_of(AkismetService).to receive_messages(is_spam?: true) + allow_any_instance_of(AkismetService).to receive_messages(spam?: true) put v3_api("/projects/#{project.id}/issues/#{issue.id}", user), params diff --git a/spec/requests/api/v3/project_snippets_spec.rb b/spec/requests/api/v3/project_snippets_spec.rb index 3963924a066..7e88489082a 100644 --- a/spec/requests/api/v3/project_snippets_spec.rb +++ b/spec/requests/api/v3/project_snippets_spec.rb @@ -80,7 +80,7 @@ describe API::ProjectSnippets do end before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do @@ -140,7 +140,7 @@ describe API::ProjectSnippets do end before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do diff --git a/spec/requests/api/v3/snippets_spec.rb b/spec/requests/api/v3/snippets_spec.rb index 9ead3cad8bb..79860725634 100644 --- a/spec/requests/api/v3/snippets_spec.rb +++ b/spec/requests/api/v3/snippets_spec.rb @@ -107,7 +107,7 @@ describe API::V3::Snippets do end before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end context 'when the snippet is private' do diff --git a/spec/requests/api/v3/triggers_spec.rb b/spec/requests/api/v3/triggers_spec.rb index 075de2c0cba..d4648136841 100644 --- a/spec/requests/api/v3/triggers_spec.rb +++ b/spec/requests/api/v3/triggers_spec.rb @@ -7,7 +7,10 @@ describe API::V3::Triggers do let!(:project) { create(:project, :repository, creator: user) } let!(:master) { create(:project_member, :master, user: user, project: project) } let!(:developer) { create(:project_member, :developer, user: user2, project: project) } - let!(:trigger) { create(:ci_trigger, project: project, token: trigger_token) } + + let!(:trigger) do + create(:ci_trigger, project: project, token: trigger_token, owner: user) + end describe 'POST /projects/:project_id/trigger' do let!(:project2) { create(:project) } diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 8465a6f99bd..4ba3dada37c 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -413,14 +413,12 @@ describe Ci::CreatePipelineService do end context 'when trigger belongs to a developer' do - let(:user) {} + let(:user) { create(:user) } + let(:trigger) { create(:ci_trigger, owner: user) } + let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } - let(:trigger_request) do - create(:ci_trigger_request).tap do |request| - user = create(:user) - project.add_developer(user) - request.trigger.update(owner: user) - end + before do + project.add_developer(user) end it 'does not create a pipeline' do @@ -431,17 +429,15 @@ describe Ci::CreatePipelineService do end context 'when trigger belongs to a master' do - let(:user) {} + let(:user) { create(:user) } + let(:trigger) { create(:ci_trigger, owner: user) } + let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } - let(:trigger_request) do - create(:ci_trigger_request).tap do |request| - user = create(:user) - project.add_master(user) - request.trigger.update(owner: user) - end + before do + project.add_master(user) end - it 'does not create a pipeline' do + it 'creates a pipeline' do expect(execute_service(trigger_request: trigger_request)) .to be_persisted expect(Ci::Pipeline.count).to eq(1) @@ -470,7 +466,8 @@ describe Ci::CreatePipelineService do context 'when ref is not protected' do context 'when trigger belongs to no one' do let(:user) {} - let(:trigger_request) { create(:ci_trigger_request) } + let(:trigger) { create(:ci_trigger, owner: nil) } + let(:trigger_request) { create(:ci_trigger_request, trigger: trigger) } it 'creates a pipeline' do expect(execute_service(trigger_request: trigger_request)) diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb index cc950ae6bb3..2b4f8cd42ee 100644 --- a/spec/services/files/update_service_spec.rb +++ b/spec/services/files/update_service_spec.rb @@ -76,7 +76,7 @@ describe Files::UpdateService do let(:branch_name) { "#{project.default_branch}-new" } it 'fires hooks only once' do - expect(GitHooksService).to receive(:new).once.and_call_original + expect(Gitlab::Git::HooksService).to receive(:new).once.and_call_original subject.execute end diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index e3c1bdce300..cc3d4e7da49 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -617,7 +617,7 @@ describe GitPushService, services: true do context 'on the default branch' do before do - allow(service).to receive(:is_default_branch?).and_return(true) + allow(service).to receive(:default_branch?).and_return(true) end it 'flushes the caches of any special files that have been changed' do @@ -638,7 +638,7 @@ describe GitPushService, services: true do context 'on a non-default branch' do before do - allow(service).to receive(:is_default_branch?).and_return(false) + allow(service).to receive(:default_branch?).and_return(false) end it 'does not flush any conditional caches' do diff --git a/spec/services/groups/nested_create_service_spec.rb b/spec/services/groups/nested_create_service_spec.rb index c1526456bac..6491fb34777 100644 --- a/spec/services/groups/nested_create_service_spec.rb +++ b/spec/services/groups/nested_create_service_spec.rb @@ -2,52 +2,87 @@ require 'spec_helper' describe Groups::NestedCreateService do let(:user) { create(:user) } - let(:params) { { group_path: 'a-group/a-sub-group' } } subject(:service) { described_class.new(user, params) } - describe "#execute" do - it 'returns the group if it already existed' do - parent = create(:group, path: 'a-group', owner: user) - child = create(:group, path: 'a-sub-group', parent: parent, owner: user) + shared_examples 'with a visibility level' do + it 'creates the group with correct visibility level' do + allow(Gitlab::CurrentSettings.current_application_settings) + .to receive(:default_group_visibility) { Gitlab::VisibilityLevel::INTERNAL } + + group = service.execute - expect(service.execute).to eq(child) + expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) end - it 'reuses a parent if it already existed' do - parent = create(:group, path: 'a-group') - parent.add_owner(user) + context 'adding a visibility level ' do + it 'overwrites the visibility level' do + service = described_class.new(user, params.merge(visibility_level: Gitlab::VisibilityLevel::PRIVATE)) + + group = service.execute - expect(service.execute.parent).to eq(parent) + expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) + end end + end + + describe 'without subgroups' do + let(:params) { { group_path: 'a-group' } } - it 'creates group and subgroup in the database' do - service.execute + before do + allow(Group).to receive(:supports_nested_groups?) { false } + end - parent = Group.find_by_full_path('a-group') - child = parent.children.find_by(path: 'a-sub-group') + it 'creates the group' do + group = service.execute - expect(parent).not_to be_nil - expect(child).not_to be_nil + expect(group).to be_persisted end - it 'creates the group with correct visibility level' do - allow(Gitlab::CurrentSettings.current_application_settings) - .to receive(:default_group_visibility) { Gitlab::VisibilityLevel::INTERNAL } + it 'returns the group if it already existed' do + existing_group = create(:group, path: 'a-group') - group = service.execute + expect(service.execute).to eq(existing_group) + end - expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL) + it 'raises an error when tring to create a subgroup' do + service = described_class.new(user, group_path: 'a-group/a-sub-group') + + expect { service.execute }.to raise_error('Nested groups are not supported on MySQL') end - context 'adding a visibility level ' do - let(:params) { { group_path: 'a-group/a-sub-group', visibility_level: Gitlab::VisibilityLevel::PRIVATE } } + it_behaves_like 'with a visibility level' + end - it 'overwrites the visibility level' do - group = service.execute + describe 'with subgroups', :nested_groups do + let(:params) { { group_path: 'a-group/a-sub-group' } } - expect(group.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) + describe "#execute" do + it 'returns the group if it already existed' do + parent = create(:group, path: 'a-group', owner: user) + child = create(:group, path: 'a-sub-group', parent: parent, owner: user) + + expect(service.execute).to eq(child) end + + it 'reuses a parent if it already existed' do + parent = create(:group, path: 'a-group') + parent.add_owner(user) + + expect(service.execute.parent).to eq(parent) + end + + it 'creates group and subgroup in the database' do + service.execute + + parent = Group.find_by_full_path('a-group') + child = parent.children.find_by(path: 'a-sub-group') + + expect(parent).not_to be_nil + expect(child).not_to be_nil + end + + it_behaves_like 'with a visibility level' end end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 78b11cd7991..cc3d648c340 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -370,7 +370,7 @@ describe Issues::CreateService do context 'when recaptcha was not verified' do context 'when akismet detects spam' do before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) end it 'marks an issue as a spam ' do @@ -392,7 +392,7 @@ describe Issues::CreateService do context 'when akismet does not detect spam' do before do - allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(false) + allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false) end it 'does not mark an issue as a spam ' do diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index e593bfeeaf7..b60136064b7 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -12,6 +12,38 @@ describe MergeRequests::MergeService do end describe '#execute' do + context 'MergeRequest#merge_jid' do + before do + merge_request.update_column(:merge_jid, 'hash-123') + end + + it 'is cleaned when no error is raised' do + service = described_class.new(project, user, commit_message: 'Awesome message') + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + + it 'is cleaned when expected error is raised' do + service = described_class.new(project, user, commit_message: 'Awesome message') + allow(service).to receive(:commit).and_raise(described_class::MergeError) + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + + it 'is not cleaned when unexpected error is raised' do + service = described_class.new(project, user, commit_message: 'Awesome message') + allow(service).to receive(:commit).and_raise(StandardError) + + expect { service.execute(merge_request) }.to raise_error(StandardError) + + expect(merge_request.reload.merge_jid).to be_present + end + end + context 'valid params' do let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } @@ -217,7 +249,7 @@ describe MergeRequests::MergeService do it 'logs and saves error if there is an PreReceiveError exception' do error_message = 'error message' - allow(service).to receive(:repository).and_raise(GitHooksService::PreReceiveError, error_message) + allow(service).to receive(:repository).and_raise(Gitlab::Git::HooksService::PreReceiveError, error_message) allow(service).to receive(:execute_hooks) service.execute(merge_request) diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 44b2d28d1d4..5b349017c8b 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -130,7 +130,18 @@ describe NotificationService, :mailer do project.add_master(issue.author) project.add_master(assignee) project.add_master(note.author) - create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: '@subscribed_participant cc this guy') + + @u_custom_off = create_user_with_notification(:custom, 'custom_off') + project.add_guest(@u_custom_off) + + create( + :note_on_issue, + author: @u_custom_off, + noteable: issue, + project_id: issue.project_id, + note: 'i think @subscribed_participant should see this' + ) + update_custom_notification(:new_note, @u_guest_custom, resource: project) update_custom_notification(:new_note, @u_custom_global) end @@ -140,8 +151,7 @@ describe NotificationService, :mailer do add_users_with_subscription(note.project, issue) reset_delivered_emails! - # Ensure create SentNotification by noteable = issue 6 times, not noteable = note - expect(SentNotification).to receive(:record).with(issue, any_args).exactly(8).times + expect(SentNotification).to receive(:record).with(issue, any_args).exactly(9).times notification.new_note(note) @@ -153,6 +163,7 @@ describe NotificationService, :mailer do should_email(@subscriber) should_email(@watcher_and_subscriber) should_email(@subscribed_participant) + should_email(@u_custom_off) should_not_email(@u_guest_custom) should_not_email(@u_guest_watcher) should_not_email(note.author) diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb new file mode 100644 index 00000000000..c6678fc1f5c --- /dev/null +++ b/spec/services/projects/after_import_service_spec.rb @@ -0,0 +1,55 @@ +require 'spec_helper' + +describe Projects::AfterImportService do + subject { described_class.new(project) } + + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:sha) { project.commit.sha } + let(:housekeeping_service) { double(:housekeeping_service) } + + describe '#execute' do + before do + allow(Projects::HousekeepingService) + .to receive(:new).with(project).and_return(housekeeping_service) + + allow(housekeeping_service) + .to receive(:execute).and_yield + end + + it 'performs housekeeping' do + subject.execute + + expect(housekeeping_service).to have_received(:execute) + end + + context 'with some refs in refs/pull/**/*' do + before do + repository.write_ref('refs/pull/1/head', sha) + repository.write_ref('refs/pull/1/merge', sha) + + subject.execute + end + + it 'removes refs/pull/**/*' do + expect(repository.rugged.references.map(&:name)) + .not_to include(%r{\Arefs/pull/}) + end + end + + Repository::RESERVED_REFS_NAMES.each do |name| + context "with a ref in refs/#{name}/tmp" do + before do + repository.write_ref("refs/#{name}/tmp", sha) + + subject.execute + end + + it "does not remove refs/#{name}/tmp" do + expect(repository.rugged.references.map(&:name)) + .to include("refs/#{name}/tmp") + end + end + end + end +end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index b0dc7488b5f..088b7b4fc04 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -38,7 +38,7 @@ describe Projects::CreateService, '#execute' do expect(project).to be_persisted expect(project.owner).to eq(user) - expect(project.team.masters).to include(user, admin) + expect(project.team.masters).to contain_exactly(user) expect(project.namespace).to eq(user.namespace) end end diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb index 385f56e447f..9386c110385 100644 --- a/spec/services/projects/housekeeping_service_spec.rb +++ b/spec/services/projects/housekeeping_service_spec.rb @@ -23,6 +23,12 @@ describe Projects::HousekeepingService do expect(project.reload.pushes_since_gc).to eq(0) end + it 'yields the block if given' do + expect do |block| + subject.execute(&block) + end.to yield_with_no_args + end + context 'when no lease can be obtained' do before do expect(subject).to receive(:try_obtain_lease).and_return(false) @@ -39,6 +45,13 @@ describe Projects::HousekeepingService do expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken) end.not_to change { project.pushes_since_gc } end + + it 'does not yield' do + expect do |block| + expect { subject.execute(&block) } + .to raise_error(Projects::HousekeepingService::LeaseTaken) + end.not_to yield_with_no_args + end end end diff --git a/spec/services/spam_service_spec.rb b/spec/services/spam_service_spec.rb index a14dfa3f01f..61312d55b84 100644 --- a/spec/services/spam_service_spec.rb +++ b/spec/services/spam_service_spec.rb @@ -23,7 +23,7 @@ describe SpamService do before do issue.closed_at = Time.zone.now - allow(AkismetService).to receive(:new).and_return(double(is_spam?: true)) + allow(AkismetService).to receive(:new).and_return(double(spam?: true)) end it 'returns false' do @@ -43,7 +43,7 @@ describe SpamService do context 'when indicated as spam by akismet' do before do - allow(AkismetService).to receive(:new).and_return(double(is_spam?: true)) + allow(AkismetService).to receive(:new).and_return(double(spam?: true)) end it 'doesnt check as spam when request is missing' do @@ -71,7 +71,7 @@ describe SpamService do context 'when not indicated as spam by akismet' do before do - allow(AkismetService).to receive(:new).and_return(double(is_spam?: false)) + allow(AkismetService).to receive(:new).and_return(double(spam?: false)) end it 'returns false' do diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 6d36affa9dc..c2d6d7781b9 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -3,9 +3,9 @@ require 'spec_helper' describe SystemNoteService do include Gitlab::Routing - let(:group) { create(:group) } - let(:project) { create(:project, group: group) } - let(:author) { create(:user) } + set(:group) { create(:group) } + set(:project) { create(:project, :repository, group: group) } + set(:author) { create(:user) } let(:noteable) { create(:issue, project: project) } let(:issue) { noteable } @@ -13,45 +13,23 @@ describe SystemNoteService do let(:expected_noteable) { noteable } let(:commit_count) { nil } - it 'is valid' do + it 'has the correct attributes', :aggregate_failures do expect(subject).to be_valid - end + expect(subject).to be_system - it 'sets the noteable model' do expect(subject.noteable).to eq expected_noteable - end - - it 'sets the project' do expect(subject.project).to eq project - end - - it 'sets the author' do expect(subject.author).to eq author - end - - it 'is a system note' do - expect(subject).to be_system - end - - context 'metadata' do - it 'creates a new system note metadata record' do - expect { subject }.to change { SystemNoteMetadata.count }.from(0).to(1) - end - it 'creates a record correctly' do - metadata = subject.system_note_metadata - - expect(metadata.commit_count).to eq(commit_count) - expect(metadata.action).to eq(action) - end + expect(subject.system_note_metadata.action).to eq(action) + expect(subject.system_note_metadata.commit_count).to eq(commit_count) end end describe '.add_commits' do subject { described_class.add_commits(noteable, project, author, new_commits, old_commits, oldrev) } - let(:project) { create(:project, :repository) } - let(:noteable) { create(:merge_request, source_project: project) } + let(:noteable) { create(:merge_request, source_project: project, target_project: project) } let(:new_commits) { noteable.commits } let(:old_commits) { [] } let(:oldrev) { nil } @@ -206,7 +184,7 @@ describe SystemNoteService do describe '.change_label' do subject { described_class.change_label(noteable, project, author, added, removed) } - let(:labels) { create_list(:label, 2) } + let(:labels) { create_list(:label, 2, project: project) } let(:added) { [] } let(:removed) { [] } @@ -315,7 +293,6 @@ describe SystemNoteService do end describe '.merge_when_pipeline_succeeds' do - let(:project) { create(:project, :repository) } let(:pipeline) { build(:ci_pipeline_without_jobs )} let(:noteable) do create(:merge_request, source_project: project, target_project: project) @@ -333,7 +310,6 @@ describe SystemNoteService do end describe '.cancel_merge_when_pipeline_succeeds' do - let(:project) { create(:project, :repository) } let(:noteable) do create(:merge_request, source_project: project, target_project: project) end @@ -411,7 +387,6 @@ describe SystemNoteService do describe '.change_branch' do subject { described_class.change_branch(noteable, project, author, 'target', old_branch, new_branch) } - let(:project) { create(:project, :repository) } let(:old_branch) { 'old_branch'} let(:new_branch) { 'new_branch'} @@ -429,8 +404,6 @@ describe SystemNoteService do describe '.change_branch_presence' do subject { described_class.change_branch_presence(noteable, project, author, :source, 'feature', :delete) } - let(:project) { create(:project, :repository) } - it_behaves_like 'a system note' do let(:action) { 'branch' } end @@ -445,8 +418,6 @@ describe SystemNoteService do describe '.new_issue_branch' do subject { described_class.new_issue_branch(noteable, project, author, "1-mepmep") } - let(:project) { create(:project, :repository) } - it_behaves_like 'a system note' do let(:action) { 'branch' } end @@ -492,7 +463,7 @@ describe SystemNoteService do describe 'note_body' do context 'cross-project' do - let(:project2) { create(:project, :repository) } + let(:project2) { create(:project, :repository) } let(:mentioner) { create(:issue, project: project2) } context 'from Commit' do @@ -512,7 +483,6 @@ describe SystemNoteService do context 'within the same project' do context 'from Commit' do - let(:project) { create(:project, :repository) } let(:mentioner) { project.repository.commit } it 'references the mentioning commit' do @@ -554,7 +524,6 @@ describe SystemNoteService do end context 'when mentioner is a MergeRequest' do - let(:project) { create(:project, :repository) } let(:mentioner) { create(:merge_request, :simple, source_project: project) } let(:noteable) { project.commit } @@ -582,7 +551,6 @@ describe SystemNoteService do end describe '.cross_reference_exists?' do - let(:project) { create(:project, :repository) } let(:commit0) { project.commit } let(:commit1) { project.commit('HEAD~2') } @@ -920,9 +888,8 @@ describe SystemNoteService do end describe '.discussion_continued_in_issue' do - let(:discussion) { create(:diff_note_on_merge_request).to_discussion } + let(:discussion) { create(:diff_note_on_merge_request, project: project).to_discussion } let(:merge_request) { discussion.noteable } - let(:project) { merge_request.source_project } let(:issue) { create(:issue, project: project) } def reloaded_merge_request @@ -1044,7 +1011,6 @@ describe SystemNoteService do end describe '.add_merge_request_wip_from_commit' do - let(:project) { create(:project, :repository) } let(:noteable) do create(:merge_request, source_project: project, target_project: project) end @@ -1099,9 +1065,8 @@ describe SystemNoteService do end describe '.diff_discussion_outdated' do - let(:discussion) { create(:diff_note_on_merge_request).to_discussion } + let(:discussion) { create(:diff_note_on_merge_request, project: project).to_discussion } let(:merge_request) { discussion.noteable } - let(:project) { merge_request.source_project } let(:change_position) { discussion.position } def reloaded_merge_request diff --git a/spec/services/tags/create_service_spec.rb b/spec/services/tags/create_service_spec.rb index 1b31ce29f7a..57013b54560 100644 --- a/spec/services/tags/create_service_spec.rb +++ b/spec/services/tags/create_service_spec.rb @@ -41,7 +41,7 @@ describe Tags::CreateService do it 'returns an error' do expect(repository).to receive(:add_tag) .with(user, 'v1.1.0', 'master', 'Foo') - .and_raise(GitHooksService::PreReceiveError, 'something went wrong') + .and_raise(Gitlab::Git::HooksService::PreReceiveError, 'something went wrong') response = service.execute('v1.1.0', 'master', 'Foo') diff --git a/spec/services/test_hooks/system_service_spec.rb b/spec/services/test_hooks/system_service_spec.rb index 00d89924766..a15708bf82f 100644 --- a/spec/services/test_hooks/system_service_spec.rb +++ b/spec/services/test_hooks/system_service_spec.rb @@ -7,7 +7,6 @@ describe TestHooks::SystemService do let(:project) { create(:project, :repository) } let(:hook) { create(:system_hook) } let(:service) { described_class.new(hook, current_user, trigger) } - let(:sample_data) { { data: 'sample' }} let(:success_result) { { status: :success, http_status: 200, message: 'ok' } } before do @@ -26,18 +25,11 @@ describe TestHooks::SystemService do context 'push_events' do let(:trigger) { 'push_events' } - it 'returns error message if not enough data' do - allow(project).to receive(:empty_repo?).and_return(true) - - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." }) - end - it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) - allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end @@ -45,18 +37,11 @@ describe TestHooks::SystemService do context 'tag_push_events' do let(:trigger) { 'tag_push_events' } - it 'returns error message if not enough data' do - allow(project.repository).to receive(:tags).and_return([]) - - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has tags." }) - end - it 'executes hook' do allow(project.repository).to receive(:tags).and_return(['tag']) - allow(Gitlab::DataBuilder::Push).to receive(:build_sample).and_return(sample_data) + expect(Gitlab::DataBuilder::Push).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Push::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end @@ -64,17 +49,11 @@ describe TestHooks::SystemService do context 'repository_update_events' do let(:trigger) { 'repository_update_events' } - it 'returns error message if not enough data' do - allow(project).to receive(:commit).and_return(nil) - expect(hook).not_to receive(:execute) - expect(service.execute).to include({ status: :error, message: "Ensure project \"#{project.human_name}\" has commits." }) - end - it 'executes hook' do allow(project).to receive(:empty_repo?).and_return(false) - allow(Gitlab::DataBuilder::Repository).to receive(:update).and_return(sample_data) + expect(Gitlab::DataBuilder::Repository).to receive(:sample_data).and_call_original - expect(hook).to receive(:execute).with(sample_data, trigger).and_return(success_result) + expect(hook).to receive(:execute).with(Gitlab::DataBuilder::Repository::SAMPLE_DATA, trigger).and_return(success_result) expect(service.execute).to include(success_result) end end diff --git a/spec/services/user_project_access_changed_service_spec.rb b/spec/services/user_project_access_changed_service_spec.rb index 14a5e40350a..87a90378e2b 100644 --- a/spec/services/user_project_access_changed_service_spec.rb +++ b/spec/services/user_project_access_changed_service_spec.rb @@ -8,5 +8,12 @@ describe UserProjectAccessChangedService do described_class.new([1, 2]).execute end + + it 'permits non-blocking operation' do + expect(AuthorizedProjectsWorker).to receive(:bulk_perform_async) + .with([[1], [2]]) + + described_class.new([1, 2]).execute(blocking: false) + end end end diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 985f6d94876..6ee35a33b2d 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -37,7 +37,10 @@ describe Users::UpdateService do describe '#execute!' do it 'updates the name' do - result = update_user(user, name: 'New Name') + service = described_class.new(user, name: 'New Name') + expect(service).not_to receive(:notify_new_user) + + result = service.execute! expect(result).to be true expect(user.name).to eq('New Name') @@ -49,6 +52,18 @@ describe Users::UpdateService do end.to raise_error(ActiveRecord::RecordInvalid) end + it 'fires system hooks when a new user is saved' do + system_hook_service = spy(:system_hook_service) + user = build(:user) + service = described_class.new(user, name: 'John Doe') + expect(service).to receive(:notify_new_user).and_call_original + expect(service).to receive(:system_hook_service).and_return(system_hook_service) + + service.execute + + expect(system_hook_service).to have_received(:execute_hooks_for).with(user, :create) + end + def update_user(user, opts) described_class.new(user, opts).execute! end diff --git a/spec/support/api_helpers.rb b/spec/support/api_helpers.rb index ac0aaa524b7..01aca74274c 100644 --- a/spec/support/api_helpers.rb +++ b/spec/support/api_helpers.rb @@ -45,18 +45,4 @@ module ApiHelpers oauth_access_token: oauth_access_token ) end - - def ci_api(path, user = nil) - "/ci/api/v1/#{path}" + - - # Normalize query string - (path.index('?') ? '' : '?') + - - # Append private_token if given a User object - if user.respond_to?(:private_token) - "&private_token=#{user.private_token}" - else - '' - end - end end diff --git a/spec/support/cycle_analytics_helpers.rb b/spec/support/cycle_analytics_helpers.rb index 30911e7fa86..39586d37e93 100644 --- a/spec/support/cycle_analytics_helpers.rb +++ b/spec/support/cycle_analytics_helpers.rb @@ -78,6 +78,8 @@ module CycleAnalyticsHelpers @dummy_pipeline ||= Ci::Pipeline.new( sha: project.repository.commit('master').sha, + ref: 'master', + source: :push, project: project) end diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb index b0f520d08e8..edaee03ea6c 100644 --- a/spec/support/db_cleaner.rb +++ b/spec/support/db_cleaner.rb @@ -4,18 +4,18 @@ RSpec.configure do |config| end config.append_after(:context) do - DatabaseCleaner.clean_with(:truncation) + DatabaseCleaner.clean_with(:truncation, cache_tables: false) end config.before(:each) do DatabaseCleaner.strategy = :transaction end - config.before(:each, js: true) do + config.before(:each, :js) do DatabaseCleaner.strategy = :truncation end - config.before(:each, truncate: true) do + config.before(:each, :truncate) do DatabaseCleaner.strategy = :truncation end diff --git a/spec/support/features/reportable_note_shared_examples.rb b/spec/support/features/reportable_note_shared_examples.rb index cb483ae9a5a..5a0e7c3d099 100644 --- a/spec/support/features/reportable_note_shared_examples.rb +++ b/spec/support/features/reportable_note_shared_examples.rb @@ -34,7 +34,7 @@ shared_examples 'reportable note' do end def open_dropdown(dropdown) - dropdown.click + dropdown.find('.more-actions-toggle').trigger('click') dropdown.find('.dropdown-menu li', match: :first) end end diff --git a/spec/support/filtered_search_helpers.rb b/spec/support/filtered_search_helpers.rb index 99b8b6b7ea4..05021ea9054 100644 --- a/spec/support/filtered_search_helpers.rb +++ b/spec/support/filtered_search_helpers.rb @@ -58,11 +58,17 @@ module FilteredSearchHelpers page.all(:css, '.tokens-container li .selectable').each_with_index do |el, index| token_name = tokens[index][:name] token_value = tokens[index][:value] + token_emoji = tokens[index][:emoji_name] expect(el.find('.name')).to have_content(token_name) if token_value expect(el.find('.value')).to have_content(token_value) end + # gl-emoji content is blank when the emoji unicode is not supported + if token_emoji + selector = %(gl-emoji[data-name="#{token_emoji}"]) + expect(el.find('.value')).to have_css(selector) + end end end end @@ -89,6 +95,10 @@ module FilteredSearchHelpers create_token('Label', label_name, symbol) end + def emoji_token(emoji_name = nil) + { name: 'My-Reaction', emoji_name: emoji_name } + end + def default_placeholder 'Search or filter results...' end diff --git a/spec/support/helpers/note_interaction_helpers.rb b/spec/support/helpers/note_interaction_helpers.rb index 551c759133c..86008698692 100644 --- a/spec/support/helpers/note_interaction_helpers.rb +++ b/spec/support/helpers/note_interaction_helpers.rb @@ -2,7 +2,7 @@ module NoteInteractionHelpers def open_more_actions_dropdown(note) note_element = find("#note_#{note.id}") - note_element.find('.more-actions').click + note_element.find('.more-actions-toggle').trigger('click') note_element.find('.more-actions .dropdown-menu li', match: :first) end end diff --git a/spec/support/import_export/export_file_helper.rb b/spec/support/import_export/export_file_helper.rb index 2011408be93..562423afc2a 100644 --- a/spec/support/import_export/export_file_helper.rb +++ b/spec/support/import_export/export_file_helper.rb @@ -35,7 +35,8 @@ module ExportFileHelper project: project, commit_id: ci_pipeline.sha) - create(:event, :created, target: milestone, project: project, author: user) + event = create(:event, :created, target: milestone, project: project, author: user, action: 5) + create(:push_event_payload, event: event) create(:project_member, :master, user: user, project: project) create(:ci_variable, project: project) create(:ci_trigger, project: project) diff --git a/spec/support/migrations_helpers.rb b/spec/support/migrations_helpers.rb index 255b3d96a62..4ca019c1b05 100644 --- a/spec/support/migrations_helpers.rb +++ b/spec/support/migrations_helpers.rb @@ -16,6 +16,8 @@ module MigrationsHelpers end def reset_column_in_migration_models + ActiveRecord::Base.clear_cache! + described_class.constants.sort.each do |name| const = described_class.const_get(name) diff --git a/spec/support/shared_examples/requests/api/status_shared_examples.rb b/spec/support/shared_examples/requests/api/status_shared_examples.rb index 226277411d6..7d7f66adeab 100644 --- a/spec/support/shared_examples/requests/api/status_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/status_shared_examples.rb @@ -40,3 +40,28 @@ shared_examples_for '404 response' do end end end + +shared_examples_for '412 response' do + let(:params) { nil } + let(:success_status) { 204 } + + context 'for a modified ressource' do + before do + delete request, params, { 'HTTP_IF_UNMODIFIED_SINCE' => '1990-01-12T00:00:48-0600' } + end + + it 'returns 412' do + expect(response).to have_gitlab_http_status(412) + end + end + + context 'for an unmodified ressource' do + before do + delete request, params, { 'HTTP_IF_UNMODIFIED_SINCE' => Time.now } + end + + it 'returns accepted' do + expect(response).to have_gitlab_http_status(success_status) + end + end +end diff --git a/spec/support/stub_env.rb b/spec/support/stub_env.rb index b8928867174..19fbe572930 100644 --- a/spec/support/stub_env.rb +++ b/spec/support/stub_env.rb @@ -1,5 +1,7 @@ # Inspired by https://github.com/ljkbennett/stub_env/blob/master/lib/stub_env/helpers.rb module StubENV + include Gitlab::CurrentSettings + def stub_env(key_or_hash, value = nil) init_stub unless env_stubbed? if key_or_hash.is_a? Hash diff --git a/spec/views/admin/dashboard/index.html.haml_spec.rb b/spec/views/admin/dashboard/index.html.haml_spec.rb index df742bf6848..b4359d819a0 100644 --- a/spec/views/admin/dashboard/index.html.haml_spec.rb +++ b/spec/views/admin/dashboard/index.html.haml_spec.rb @@ -9,6 +9,7 @@ describe 'admin/dashboard/index.html.haml' do assign(:groups, create_list(:group, 1)) allow(view).to receive(:admin?).and_return(true) + allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) end it "shows version of GitLab Workhorse" do diff --git a/spec/views/devise/shared/_signin_box.html.haml_spec.rb b/spec/views/devise/shared/_signin_box.html.haml_spec.rb index 9adbb0476be..0870b8f09f9 100644 --- a/spec/views/devise/shared/_signin_box.html.haml_spec.rb +++ b/spec/views/devise/shared/_signin_box.html.haml_spec.rb @@ -5,6 +5,7 @@ describe 'devise/shared/_signin_box' do before do stub_devise assign(:ldap_servers, []) + allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) end it 'is shown when Crowd is enabled' do diff --git a/spec/views/help/index.html.haml_spec.rb b/spec/views/help/index.html.haml_spec.rb index 1f8261cc46b..c030129559e 100644 --- a/spec/views/help/index.html.haml_spec.rb +++ b/spec/views/help/index.html.haml_spec.rb @@ -37,5 +37,6 @@ describe 'help/index' do def stub_helpers allow(view).to receive(:markdown).and_return('') allow(view).to receive(:version_status_badge).and_return('') + allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) end end diff --git a/spec/views/layouts/_head.html.haml_spec.rb b/spec/views/layouts/_head.html.haml_spec.rb index 8020faa1f9c..e8e6d2e7a75 100644 --- a/spec/views/layouts/_head.html.haml_spec.rb +++ b/spec/views/layouts/_head.html.haml_spec.rb @@ -1,6 +1,10 @@ require 'spec_helper' describe 'layouts/_head' do + before do + allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) + end + it 'escapes HTML-safe strings in page_title' do stub_helper_with_safe_string(:page_title) diff --git a/spec/views/projects/commits/_commit.html.haml_spec.rb b/spec/views/projects/commits/_commit.html.haml_spec.rb index 4c247361bd7..00547e433c4 100644 --- a/spec/views/projects/commits/_commit.html.haml_spec.rb +++ b/spec/views/projects/commits/_commit.html.haml_spec.rb @@ -1,6 +1,10 @@ require 'spec_helper' describe 'projects/commits/_commit.html.haml' do + before do + allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) + end + context 'with a singed commit' do let(:project) { create(:project, :repository) } let(:repository) { project.repository } diff --git a/spec/views/projects/edit.html.haml_spec.rb b/spec/views/projects/edit.html.haml_spec.rb index 1af422941d7..c1398629749 100644 --- a/spec/views/projects/edit.html.haml_spec.rb +++ b/spec/views/projects/edit.html.haml_spec.rb @@ -10,7 +10,9 @@ describe 'projects/edit' do assign(:project, project) allow(controller).to receive(:current_user).and_return(user) - allow(view).to receive_messages(current_user: user, can?: true) + allow(view).to receive_messages(current_user: user, + can?: true, + current_application_settings: Gitlab::CurrentSettings.current_application_settings) end context 'LFS enabled setting' do diff --git a/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb b/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb index 5770cf92b4e..9ab105c3238 100644 --- a/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/creations/_new_submit.html.haml_spec.rb @@ -14,6 +14,7 @@ describe 'projects/merge_requests/creations/_new_submit.html.haml' do allow(view).to receive(:can?).and_return(true) allow(view).to receive(:url_for).and_return('#') allow(view).to receive(:current_user).and_return(merge_request.author) + allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) end context 'when there are pipelines for merge request but no pipeline for last commit' do diff --git a/spec/views/projects/merge_requests/show.html.haml_spec.rb b/spec/views/projects/merge_requests/show.html.haml_spec.rb index dc2fcc3e715..6f29d12373a 100644 --- a/spec/views/projects/merge_requests/show.html.haml_spec.rb +++ b/spec/views/projects/merge_requests/show.html.haml_spec.rb @@ -25,7 +25,9 @@ describe 'projects/merge_requests/show.html.haml' do assign(:notes, []) assign(:pipelines, Ci::Pipeline.none) - allow(view).to receive_messages(current_user: user, can?: true) + allow(view).to receive_messages(current_user: user, + can?: true, + current_application_settings: Gitlab::CurrentSettings.current_application_settings) end context 'when the merge request is closed' do diff --git a/spec/views/projects/tree/show.html.haml_spec.rb b/spec/views/projects/tree/show.html.haml_spec.rb index 33eba3e6d3d..3c25e341b39 100644 --- a/spec/views/projects/tree/show.html.haml_spec.rb +++ b/spec/views/projects/tree/show.html.haml_spec.rb @@ -12,6 +12,7 @@ describe 'projects/tree/show' do allow(view).to receive(:can?).and_return(true) allow(view).to receive(:can_collaborate_with_project?).and_return(true) + allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) end context 'for branch names ending on .json' do diff --git a/spec/views/shared/projects/_project.html.haml_spec.rb b/spec/views/shared/projects/_project.html.haml_spec.rb index b500016016a..f0a4f153699 100644 --- a/spec/views/shared/projects/_project.html.haml_spec.rb +++ b/spec/views/shared/projects/_project.html.haml_spec.rb @@ -3,6 +3,10 @@ require 'spec_helper' describe 'shared/projects/_project.html.haml' do let(:project) { create(:project) } + before do + allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) + end + it 'should render creator avatar if project has a creator' do render 'shared/projects/project', use_creator_avatar: true, project: project diff --git a/spec/workers/authorized_projects_worker_spec.rb b/spec/workers/authorized_projects_worker_spec.rb index f8385ae7c72..90ed1309d4a 100644 --- a/spec/workers/authorized_projects_worker_spec.rb +++ b/spec/workers/authorized_projects_worker_spec.rb @@ -3,28 +3,75 @@ require 'spec_helper' describe AuthorizedProjectsWorker do let(:project) { create(:project) } + def build_args_list(*ids, multiply: 1) + args_list = ids.map { |id| [id] } + args_list * multiply + end + describe '.bulk_perform_and_wait' do it 'schedules the ids and waits for the jobs to complete' do + args_list = build_args_list(project.owner.id) + + project.owner.project_authorizations.delete_all + described_class.bulk_perform_and_wait(args_list) + + expect(project.owner.project_authorizations.count).to eq(1) + end + + it 'inlines workloads <= 3 jobs' do + args_list = build_args_list(project.owner.id, multiply: 3) + expect(described_class).to receive(:bulk_perform_inline).with(args_list) + + described_class.bulk_perform_and_wait(args_list) + end + + it 'runs > 3 jobs using sidekiq' do + project.owner.project_authorizations.delete_all + + expect(described_class).to receive(:bulk_perform_async).and_call_original + + args_list = build_args_list(project.owner.id, multiply: 4) + described_class.bulk_perform_and_wait(args_list) + + expect(project.owner.project_authorizations.count).to eq(1) + end + end + + describe '.bulk_perform_inline' do + it 'refreshes the authorizations inline' do project.owner.project_authorizations.delete_all - described_class.bulk_perform_and_wait([[project.owner.id]]) + expect_any_instance_of(described_class).to receive(:perform).and_call_original + + described_class.bulk_perform_inline(build_args_list(project.owner.id)) expect(project.owner.project_authorizations.count).to eq(1) end + + it 'enqueues jobs if an error is raised' do + invalid_id = -1 + args_list = build_args_list(project.owner.id, invalid_id) + + allow_any_instance_of(described_class).to receive(:perform).with(project.owner.id) + allow_any_instance_of(described_class).to receive(:perform).with(invalid_id).and_raise(ArgumentError) + expect(described_class).to receive(:bulk_perform_async).with(build_args_list(invalid_id)) + + described_class.bulk_perform_inline(args_list) + end end describe '.bulk_perform_async' do it "uses it's respective sidekiq queue" do - args = [[project.owner.id]] + args_list = build_args_list(project.owner.id) push_bulk_args = { 'class' => described_class, 'queue' => described_class.sidekiq_options['queue'], - 'args' => args + 'args' => args_list } expect(Sidekiq::Client).to receive(:push_bulk).with(push_bulk_args).once - described_class.bulk_perform_async(args) + described_class.bulk_perform_async(args_list) end end diff --git a/spec/workers/build_finished_worker_spec.rb b/spec/workers/build_finished_worker_spec.rb index 2868167c7d4..8cc3f37ebe8 100644 --- a/spec/workers/build_finished_worker_spec.rb +++ b/spec/workers/build_finished_worker_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe BuildFinishedWorker do describe '#perform' do context 'when build exists' do - let(:build) { create(:ci_build) } + let!(:build) { create(:ci_build) } it 'calculates coverage and calls hooks' do expect(BuildCoverageWorker) diff --git a/spec/workers/concerns/build_queue_spec.rb b/spec/workers/concerns/build_queue_spec.rb deleted file mode 100644 index 6bf955e0be2..00000000000 --- a/spec/workers/concerns/build_queue_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -require 'spec_helper' - -describe BuildQueue do - let(:worker) do - Class.new do - include Sidekiq::Worker - include BuildQueue - end - end - - it 'sets the queue name of a worker' do - expect(worker.sidekiq_options['queue'].to_s).to eq('build') - end -end diff --git a/spec/workers/concerns/pipeline_queue_spec.rb b/spec/workers/concerns/pipeline_queue_spec.rb index 40794d0e42a..eac5a770e5f 100644 --- a/spec/workers/concerns/pipeline_queue_spec.rb +++ b/spec/workers/concerns/pipeline_queue_spec.rb @@ -8,7 +8,17 @@ describe PipelineQueue do end end - it 'sets the queue name of a worker' do - expect(worker.sidekiq_options['queue'].to_s).to eq('pipeline') + it 'sets a default pipelines queue automatically' do + expect(worker.sidekiq_options['queue']) + .to eq 'pipeline_default' + end + + describe '.enqueue_in' do + it 'sets a custom sidekiq queue with prefix and group' do + worker.enqueue_in(group: :processing) + + expect(worker.sidekiq_options['queue']) + .to eq 'pipeline_processing' + end end end diff --git a/spec/workers/merge_worker_spec.rb b/spec/workers/merge_worker_spec.rb index ee51000161a..303193bab9b 100644 --- a/spec/workers/merge_worker_spec.rb +++ b/spec/workers/merge_worker_spec.rb @@ -27,15 +27,4 @@ describe MergeWorker do expect(source_project.repository.branch_names).not_to include('markdown') end end - - it 'persists merge_jid' do - merge_request = create(:merge_request, merge_jid: nil) - user = create(:user) - worker = described_class.new - - allow(worker).to receive(:jid) { '999' } - - expect { worker.perform(merge_request.id, user.id, {}) } - .to change { merge_request.reload.merge_jid }.from(nil).to('999') - end end diff --git a/spec/workers/pipeline_metrics_worker_spec.rb b/spec/workers/pipeline_metrics_worker_spec.rb index ef71125c0b6..896f9e6e7f2 100644 --- a/spec/workers/pipeline_metrics_worker_spec.rb +++ b/spec/workers/pipeline_metrics_worker_spec.rb @@ -2,7 +2,12 @@ require 'spec_helper' describe PipelineMetricsWorker do let(:project) { create(:project, :repository) } - let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref, head_pipeline: pipeline) } + + let!(:merge_request) do + create(:merge_request, source_project: project, + source_branch: pipeline.ref, + head_pipeline: pipeline) + end let(:pipeline) do create(:ci_empty_pipeline, @@ -14,6 +19,8 @@ describe PipelineMetricsWorker do finished_at: Time.now) end + let(:status) { 'pending' } + describe '#perform' do before do described_class.new.perform(pipeline.id) |