diff options
Diffstat (limited to 'spec')
51 files changed, 1581 insertions, 161 deletions
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index f1165c73847..bad7a28556c 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -57,6 +57,10 @@ describe ApplicationController do end describe "#authenticate_user_from_personal_access_token!" do + before do + stub_authentication_activity_metrics(debug: false) + end + controller(described_class) do def index render text: 'authenticated' @@ -67,7 +71,13 @@ describe ApplicationController do context "when the 'personal_access_token' param is populated with the personal access token" do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + get :index, private_token: personal_access_token.token + expect(response).to have_gitlab_http_status(200) expect(response.body).to eq('authenticated') end @@ -75,15 +85,25 @@ describe ApplicationController do context "when the 'PERSONAL_ACCESS_TOKEN' header is populated with the personal access token" do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + @request.headers["PRIVATE-TOKEN"] = personal_access_token.token get :index + expect(response).to have_gitlab_http_status(200) expect(response.body).to eq('authenticated') end end it "doesn't log the user in otherwise" do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + get :index, private_token: "token" + expect(response.status).not_to eq(200) expect(response.body).not_to eq('authenticated') end @@ -174,6 +194,10 @@ describe ApplicationController do end describe '#authenticate_sessionless_user!' do + before do + stub_authentication_activity_metrics(debug: false) + end + describe 'authenticating a user from a feed token' do controller(described_class) do def index @@ -184,7 +208,13 @@ describe ApplicationController do context "when the 'feed_token' param is populated with the feed token" do context 'when the request format is atom' do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + get :index, feed_token: user.feed_token, format: :atom + expect(response).to have_gitlab_http_status 200 expect(response.body).to eq 'authenticated' end @@ -192,7 +222,13 @@ describe ApplicationController do context 'when the request format is ics' do it "logs the user in" do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_sessionless_authentication_counter) + get :index, feed_token: user.feed_token, format: :ics + expect(response).to have_gitlab_http_status 200 expect(response.body).to eq 'authenticated' end @@ -200,7 +236,11 @@ describe ApplicationController do context 'when the request format is neither atom nor ics' do it "doesn't log the user in" do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + get :index, feed_token: user.feed_token + expect(response.status).not_to have_gitlab_http_status 200 expect(response.body).not_to eq 'authenticated' end @@ -209,7 +249,11 @@ describe ApplicationController do context "when the 'feed_token' param is populated with an invalid feed token" do it "doesn't log the user" do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + get :index, feed_token: 'token', format: :atom + expect(response.status).not_to eq 200 expect(response.body).not_to eq 'authenticated' end diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index ce7762691c9..d98e6ff0df8 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -42,7 +42,7 @@ describe Boards::IssuesController do parsed_response = JSON.parse(response.body) expect(response).to match_response_schema('issues') - expect(parsed_response.length).to eq 2 + expect(parsed_response['issues'].length).to eq 2 expect(development.issues.map(&:relative_position)).not_to include(nil) end @@ -80,7 +80,7 @@ describe Boards::IssuesController do parsed_response = JSON.parse(response.body) expect(response).to match_response_schema('issues') - expect(parsed_response.length).to eq 2 + expect(parsed_response['issues'].length).to eq 2 end end diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index fed6677935e..30623b6cb3d 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -2,50 +2,131 @@ require 'spec_helper' describe Projects::WikisController do let(:project) { create(:project, :public, :repository) } - let(:user) { create(:user) } - let(:wiki) { ProjectWiki.new(project, user) } + let(:user) { project.owner } + let(:project_wiki) { ProjectWiki.new(project, user) } + let(:wiki) { project_wiki.wiki } + let(:wiki_title) { 'page-title-test' } - describe 'GET #show' do - let(:wiki_title) { 'page-title-test' } + before do + create_page(wiki_title, 'hello world') + + sign_in(user) + end + + after do + destroy_page(wiki_title) + end + describe 'GET #show' do render_views - before do - create_page(wiki_title, 'hello world') - end + subject { get :show, namespace_id: project.namespace, project_id: project, id: wiki_title } - it 'limits the retrieved pages for the sidebar' do - sign_in(user) + context 'when page content encoding is invalid' do + it 'limits the retrieved pages for the sidebar' do + expect(controller).to receive(:load_wiki).and_return(project_wiki) - expect(controller).to receive(:load_wiki).and_return(wiki) + # empty? call + expect(project_wiki).to receive(:pages).with(limit: 1).and_call_original + # Sidebar entries + expect(project_wiki).to receive(:pages).with(limit: 15).and_call_original - # empty? call - expect(wiki).to receive(:pages).with(limit: 1).and_call_original - # Sidebar entries - expect(wiki).to receive(:pages).with(limit: 15).and_call_original + subject + + expect(response).to have_http_status(:ok) + expect(response.body).to include(wiki_title) + end + end - get :show, namespace_id: project.namespace, project_id: project, id: wiki_title + context 'when page content encoding is invalid' do + it 'sets flash error' do + allow(controller).to receive(:valid_encoding?).and_return(false) - expect(response).to have_http_status(:ok) - expect(response.body).to include(wiki_title) + subject + + expect(response).to have_http_status(:ok) + expect(flash[:notice]).to eq 'The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.' + end end end describe 'POST #preview_markdown' do it 'renders json in a correct format' do - sign_in(user) - post :preview_markdown, namespace_id: project.namespace, project_id: project, id: 'page/path', text: '*Markdown* text' expect(JSON.parse(response.body).keys).to match_array(%w(body references)) end end + describe 'GET #edit' do + subject { get(:edit, namespace_id: project.namespace, project_id: project, id: wiki_title) } + + context 'when page content encoding is invalid' do + it 'redirects to show' do + allow(controller).to receive(:valid_encoding?).and_return(false) + + subject + + expect(response).to redirect_to(project_wiki_path(project, project_wiki.pages.first)) + end + end + + context 'when page content encoding is valid' do + render_views + + it 'shows the edit page' do + subject + + expect(response).to have_http_status(:ok) + expect(response.body).to include('Edit Page') + end + end + end + + describe 'PATCH #update' do + let(:new_title) { 'New title' } + let(:new_content) { 'New content' } + subject do + patch(:update, + namespace_id: project.namespace, + project_id: project, + id: wiki_title, + wiki: { title: new_title, content: new_content }) + end + + context 'when page content encoding is invalid' do + it 'redirects to show' do + allow(controller).to receive(:valid_encoding?).and_return(false) + + subject + expect(response).to redirect_to(project_wiki_path(project, project_wiki.pages.first)) + end + end + + context 'when page content encoding is valid' do + render_views + + it 'updates the page' do + subject + + wiki_page = project_wiki.pages.first + + expect(wiki_page.title).to eq new_title + expect(wiki_page.content).to eq new_content + end + end + end + def create_page(name, content) - project.wiki.wiki.write_page(name, :markdown, content, commit_details(name)) + wiki.write_page(name, :markdown, content, commit_details(name)) end def commit_details(name) Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.email, "created page #{name}") end + + def destroy_page(title, dir = '') + page = wiki.page(title: title, dir: dir) + project_wiki.delete_page(page, "test commit") + end end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 83cb4750741..8bd1f1ae4e0 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -187,6 +187,13 @@ FactoryBot.define do end end + trait :test_reports do + after(:create) do |build| + create(:ci_job_artifact, :junit, job: build) + build.reload + end + end + trait :expired do artifacts_expire_at 1.minute.ago end diff --git a/spec/factories/ci/job_artifacts.rb b/spec/factories/ci/job_artifacts.rb index 3d3287d8168..a6a87782fe7 100644 --- a/spec/factories/ci/job_artifacts.rb +++ b/spec/factories/ci/job_artifacts.rb @@ -4,6 +4,7 @@ FactoryBot.define do factory :ci_job_artifact, class: Ci::JobArtifact do job factory: :ci_build file_type :archive + file_format :zip trait :remote_store do file_store JobArtifactUploader::Store::REMOTE @@ -15,6 +16,7 @@ FactoryBot.define do trait :archive do file_type :archive + file_format :zip after(:build) do |artifact, _| artifact.file = fixture_file_upload( @@ -24,6 +26,7 @@ FactoryBot.define do trait :metadata do file_type :metadata + file_format :gzip after(:build) do |artifact, _| artifact.file = fixture_file_upload( @@ -33,6 +36,7 @@ FactoryBot.define do trait :trace do file_type :trace + file_format :raw after(:build) do |artifact, evaluator| artifact.file = fixture_file_upload( @@ -40,6 +44,16 @@ FactoryBot.define do end end + trait :junit do + file_type :junit + file_format :gzip + + after(:build) do |artifact, evaluator| + artifact.file = fixture_file_upload( + Rails.root.join('spec/fixtures/junit.xml.gz'), 'application/x-gzip') + end + end + trait :correct_checksum do after(:build) do |artifact, evaluator| artifact.file_sha256 = Digest::SHA256.file(artifact.file.path).hexdigest diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb index 0ef7f35f64a..760324adacc 100644 --- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_views_wiki_page_spec.rb @@ -137,6 +137,26 @@ describe 'User views a wiki page' do end end + context 'when page has invalid content encoding' do + let(:content) { 'whatever'.force_encoding('ISO-8859-1') } + + before do + allow(Gitlab::EncodingHelper).to receive(:encode!).and_return(content) + + visit(project_wiki_path(project, wiki_page)) + end + + it 'does not show "Edit" button' do + expect(page).not_to have_selector('a.btn', text: 'Edit') + end + + it 'shows error' do + page.within(:css, '.flash-notice') do + expect(page).to have_content('The content of this page is not encoded in UTF-8. Edits can only be made via the Git repository.') + end + end + end + it 'opens a default wiki page', :js do visit(project_path(project)) diff --git a/spec/features/users/login_spec.rb b/spec/features/users/login_spec.rb index 21891b9ccda..44758f862a8 100644 --- a/spec/features/users/login_spec.rb +++ b/spec/features/users/login_spec.rb @@ -3,28 +3,40 @@ require 'spec_helper' describe 'Login' do include TermsHelper - it 'Successful user signin invalidates password reset token' do - user = create(:user) + before do + stub_authentication_activity_metrics(debug: true) + end + + describe 'password reset token after successful sign in' do + it 'invalidates password reset token' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + user = create(:user) - expect(user.reset_password_token).to be_nil + expect(user.reset_password_token).to be_nil - visit new_user_password_path - fill_in 'user_email', with: user.email - click_button 'Reset password' + visit new_user_password_path + fill_in 'user_email', with: user.email + click_button 'Reset password' - user.reload - expect(user.reset_password_token).not_to be_nil + user.reload + expect(user.reset_password_token).not_to be_nil - find('a[href="#login-pane"]').click - gitlab_sign_in(user) - expect(current_path).to eq root_path + find('a[href="#login-pane"]').click + gitlab_sign_in(user) + expect(current_path).to eq root_path - user.reload - expect(user.reset_password_token).to be_nil + user.reload + expect(user.reset_password_token).to be_nil + end end describe 'initial login after setup' do it 'allows the initial admin to create a password' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + # This behavior is dependent on there only being one user User.delete_all @@ -56,6 +68,11 @@ describe 'Login' do describe 'with a blocked account' do it 'prevents the user from logging in' do + expect(authentication_metrics) + .to increment(:user_blocked_counter) + .and increment(:user_unauthenticated_counter) + .and increment(:user_session_destroyed_counter).twice + user = create(:user, :blocked) gitlab_sign_in(user) @@ -64,6 +81,11 @@ describe 'Login' do end it 'does not update Devise trackable attributes', :clean_gitlab_redis_shared_state do + expect(authentication_metrics) + .to increment(:user_blocked_counter) + .and increment(:user_unauthenticated_counter) + .and increment(:user_session_destroyed_counter).twice + user = create(:user, :blocked) expect { gitlab_sign_in(user) }.not_to change { user.reload.sign_in_count } @@ -72,13 +94,22 @@ describe 'Login' do describe 'with the ghost user' do it 'disallows login' do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + gitlab_sign_in(User.ghost) expect(page).to have_content('Invalid Login or password.') end it 'does not update Devise trackable attributes', :clean_gitlab_redis_shared_state do - expect { gitlab_sign_in(User.ghost) }.not_to change { User.ghost.reload.sign_in_count } + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + + expect { gitlab_sign_in(User.ghost) } + .not_to change { User.ghost.reload.sign_in_count } end end @@ -93,17 +124,30 @@ describe 'Login' do before do gitlab_sign_in(user, remember: true) + expect(page).to have_content('Two-Factor Authentication') end it 'does not show a "You are already signed in." error message' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code(user.current_otp) + expect(page).not_to have_content('You are already signed in.') end context 'using one-time code' do it 'allows login with valid code' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code(user.current_otp) + expect(current_path).to eq root_path end @@ -114,11 +158,20 @@ describe 'Login' do end it 'blocks login with invalid code' do + # TODO invalid 2FA code does not generate any events + # See gitlab-org/gitlab-ce#49785 + enter_code('foo') + expect(page).to have_content('Invalid two-factor code') end it 'allows login with invalid code, then valid code' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code('foo') expect(page).to have_content('Invalid two-factor code') @@ -139,16 +192,33 @@ describe 'Login' do context 'with valid code' do it 'allows login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + enter_code(codes.sample) + expect(current_path).to eq root_path end it 'invalidates the used code' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + expect { enter_code(codes.sample) } .to change { user.reload.otp_backup_codes.size }.by(-1) end it 'invalidates backup codes twice in a row' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter).twice + .and increment(:user_session_override_counter).twice + .and increment(:user_two_factor_authenticated_counter).twice + .and increment(:user_session_destroyed_counter) + random_code = codes.delete(codes.sample) expect { enter_code(random_code) } .to change { user.reload.otp_backup_codes.size }.by(-1) @@ -163,6 +233,9 @@ describe 'Login' do context 'with invalid code' do it 'blocks login' do + # TODO, invalid two factor authentication does not increment + # metrics / counters, see gitlab-org/gitlab-ce#49785 + code = codes.sample expect(user.invalidate_otp_backup_code!(code)).to eq true @@ -176,7 +249,7 @@ describe 'Login' do end end - context 'logging in via OAuth' do + context 'when logging in via OAuth' do let(:user) { create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')} let(:mock_saml_response) do File.read('spec/fixtures/authentication/saml_response.xml') @@ -185,49 +258,80 @@ describe 'Login' do before do stub_omniauth_saml_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [mock_saml_config_with_upstream_two_factor_authn_contexts]) - gitlab_sign_in_via('saml', user, 'my-uid', mock_saml_response) end context 'when authn_context is worth two factors' do let(:mock_saml_response) do File.read('spec/fixtures/authentication/saml_response.xml') - .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') + .gsub('urn:oasis:names:tc:SAML:2.0:ac:classes:Password', + 'urn:oasis:names:tc:SAML:2.0:ac:classes:SecondFactorOTPSMS') end it 'signs user in without prompting for second factor' do + # TODO, OAuth authentication does not fire events, + # see gitlab-org/gitlab-ce#49786 + + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + + sign_in_using_saml! + expect(page).not_to have_content('Two-Factor Authentication') expect(current_path).to eq root_path end end - context 'when authn_context is not worth two factors' do + context 'when two factor authentication is required' do it 'shows 2FA prompt after OAuth login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + + sign_in_using_saml! + expect(page).to have_content('Two-Factor Authentication') + enter_code(user.current_otp) + expect(current_path).to eq root_path end end + + def sign_in_using_saml! + gitlab_sign_in_via('saml', user, 'my-uid', mock_saml_response) + end end end describe 'without two-factor authentication' do - let(:user) { create(:user) } + context 'with correct username and password' do + let(:user) { create(:user) } - it 'allows basic login' do - gitlab_sign_in(user) - expect(current_path).to eq root_path - end + it 'allows basic login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) - it 'does not show a "You are already signed in." error message' do - gitlab_sign_in(user) - expect(page).not_to have_content('You are already signed in.') + gitlab_sign_in(user) + + expect(current_path).to eq root_path + expect(page).not_to have_content('You are already signed in.') + end end - it 'blocks invalid login' do - user = create(:user, password: 'not-the-default') + context 'with invalid username and password' do + let(:user) { create(:user, password: 'not-the-default') } - gitlab_sign_in(user) - expect(page).to have_content('Invalid Login or password.') + it 'blocks invalid login' do + expect(authentication_metrics) + .to increment(:user_unauthenticated_counter) + .and increment(:user_password_invalid_counter) + + gitlab_sign_in(user) + + expect(page).to have_content('Invalid Login or password.') + end end end @@ -243,18 +347,26 @@ describe 'Login' do context 'with grace period defined' do before do stub_application_setting(two_factor_grace_period: 48) - gitlab_sign_in(user) end context 'within the grace period' do it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content('The global settings require you to enable Two-Factor Authentication for your account. You need to do this before ') end it 'allows skipping two-factor configuration', :js do - expect(current_path).to eq profile_two_factor_auth_path + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path click_link 'Configure it later' expect(current_path).to eq root_path end @@ -264,6 +376,11 @@ describe 'Login' do let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) } it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The global settings require you to enable Two-Factor Authentication for your account.' @@ -271,6 +388,11 @@ describe 'Login' do end it 'disallows skipping two-factor configuration', :js do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).not_to have_link('Configure it later') end @@ -280,10 +402,14 @@ describe 'Login' do context 'without grace period defined' do before do stub_application_setting(two_factor_grace_period: 0) - gitlab_sign_in(user) end it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The global settings require you to enable Two-Factor Authentication for your account.' @@ -303,11 +429,15 @@ describe 'Login' do context 'with grace period defined' do before do stub_application_setting(two_factor_grace_period: 48) - gitlab_sign_in(user) end context 'within the grace period' do it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ @@ -316,8 +446,12 @@ describe 'Login' do end it 'allows skipping two-factor configuration', :js do - expect(current_path).to eq profile_two_factor_auth_path + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + gitlab_sign_in(user) + + expect(current_path).to eq profile_two_factor_auth_path click_link 'Configure it later' expect(current_path).to eq root_path end @@ -327,6 +461,11 @@ describe 'Login' do let(:user) { create(:user, otp_grace_period_started_at: 9999.hours.ago) } it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ @@ -335,6 +474,11 @@ describe 'Login' do end it 'disallows skipping two-factor configuration', :js do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).not_to have_link('Configure it later') end @@ -344,10 +488,14 @@ describe 'Login' do context 'without grace period defined' do before do stub_application_setting(two_factor_grace_period: 0) - gitlab_sign_in(user) end it 'redirects to two-factor configuration page' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + + gitlab_sign_in(user) + expect(current_path).to eq profile_two_factor_auth_path expect(page).to have_content( 'The group settings for Group 1 and Group 2 require you to enable ' \ @@ -431,6 +579,9 @@ describe 'Login' do end it 'asks to accept the terms on first login' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -447,6 +598,9 @@ describe 'Login' do end it 'does not ask for terms when the user already accepted them' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + accept_terms(user) visit new_user_session_path @@ -467,6 +621,9 @@ describe 'Login' do context 'when the user did not enable 2FA' do it 'asks to set 2FA before asking to accept the terms' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -495,6 +652,11 @@ describe 'Login' do end it 'asks the user to accept the terms' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + .and increment(:user_two_factor_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -518,6 +680,9 @@ describe 'Login' do end it 'asks the user to accept the terms before setting a new password' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + visit new_user_session_path fill_in 'user_login', with: user.email @@ -546,6 +711,10 @@ describe 'Login' do end it 'asks the user to accept the terms before setting an email' do + expect(authentication_metrics) + .to increment(:user_authenticated_counter) + .and increment(:user_session_override_counter) + gitlab_sign_in_via('saml', user, 'my-uid') expect_to_be_on_terms_page diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index 3b741d51598..a2ac4d238c7 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -81,6 +81,7 @@ "can_revert_on_current_merge_request": { "type": ["boolean", "null"] }, "can_cherry_pick_on_current_merge_request": { "type": ["boolean", "null"] }, "can_create_note": { "type": "boolean" }, + "can_create_issue": { "type": "boolean" }, "can_update": { "type": "boolean" } }, "additionalProperties": false diff --git a/spec/fixtures/junit.xml.gz b/spec/fixtures/junit.xml.gz Binary files differnew file mode 100644 index 00000000000..88b7de6fa61 --- /dev/null +++ b/spec/fixtures/junit.xml.gz diff --git a/spec/javascripts/boards/boards_store_spec.js b/spec/javascripts/boards/boards_store_spec.js index f7af099b3bf..1ee6f4cf680 100644 --- a/spec/javascripts/boards/boards_store_spec.js +++ b/spec/javascripts/boards/boards_store_spec.js @@ -161,6 +161,28 @@ describe('Store', () => { }, 0); }); + it('moves an issue from backlog to a list', (done) => { + const backlog = gl.issueBoards.BoardsStore.addList({ + ...listObj, + list_type: 'backlog', + }); + const listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate); + + expect(gl.issueBoards.BoardsStore.state.lists.length).toBe(2); + + setTimeout(() => { + expect(backlog.issues.length).toBe(1); + expect(listTwo.issues.length).toBe(1); + + gl.issueBoards.BoardsStore.moveIssueToList(backlog, listTwo, backlog.findIssue(1)); + + expect(backlog.issues.length).toBe(0); + expect(listTwo.issues.length).toBe(1); + + done(); + }, 0); + }); + it('moves issue to top of another list', (done) => { const listOne = gl.issueBoards.BoardsStore.addList(listObj); const listTwo = gl.issueBoards.BoardsStore.addList(listObjDuplicate); diff --git a/spec/javascripts/diffs/components/diff_file_header_spec.js b/spec/javascripts/diffs/components/diff_file_header_spec.js index 241ff07026e..92b2004c4d7 100644 --- a/spec/javascripts/diffs/components/diff_file_header_spec.js +++ b/spec/javascripts/diffs/components/diff_file_header_spec.js @@ -11,7 +11,9 @@ const discussionFixture = 'merge_requests/diff_discussion.json'; describe('diff_file_header', () => { let vm; let props; + const diffDiscussionMock = getJSONFixture(discussionFixture)[0]; const Component = Vue.extend(DiffFileHeader); + const store = new Vuex.Store({ modules: { diffs: diffsModule, @@ -20,7 +22,6 @@ describe('diff_file_header', () => { }); beforeEach(() => { - const diffDiscussionMock = getJSONFixture(discussionFixture)[0]; const diffFile = convertObjectPropsToCamelCase(diffDiscussionMock.diff_file, { deep: true }); props = { diffFile, @@ -303,7 +304,7 @@ describe('diff_file_header', () => { const button = vm.$el.querySelector('.btn-clipboard'); expect(button).not.toBe(null); - expect(button.dataset.clipboardText).toBe(props.diffFile.filePath); + expect(button.dataset.clipboardText).toBe('{"text":"files/ruby/popen.rb","gfm":"`files/ruby/popen.rb`"}'); }); describe('file mode', () => { @@ -409,7 +410,7 @@ describe('diff_file_header', () => { }); describe('handles toggle discussions', () => { - it('dispatches toggleFileDiscussions when user clicks on toggle discussions button', () => { + it('renders a disabled button when diff has no discussions', () => { const propsCopy = Object.assign({}, props); propsCopy.diffFile.submodule = false; propsCopy.diffFile.blob = { @@ -428,11 +429,44 @@ describe('diff_file_header', () => { store, }); - spyOn(vm, 'toggleFileDiscussions'); - - vm.$el.querySelector('.js-btn-vue-toggle-comments').click(); - - expect(vm.toggleFileDiscussions).toHaveBeenCalled(); + expect( + vm.$el.querySelector('.js-btn-vue-toggle-comments').getAttribute('disabled'), + ).toEqual('disabled'); + }); + + describe('with discussions', () => { + it('dispatches toggleFileDiscussions when user clicks on toggle discussions button', () => { + const propsCopy = Object.assign({}, props); + propsCopy.diffFile.submodule = false; + propsCopy.diffFile.blob = { + id: '848ed9407c6730ff16edb3dd24485a0eea24292a', + path: 'lib/base.js', + name: 'base.js', + mode: '100644', + readableText: true, + icon: 'file-text-o', + }; + propsCopy.addMergeRequestButtons = true; + propsCopy.diffFile.deletedFile = true; + + const discussionGetter = () => [diffDiscussionMock]; + notesModule.getters.discussions = discussionGetter; + vm = mountComponentWithStore(Component, { + props: propsCopy, + store: new Vuex.Store({ + modules: { + diffs: diffsModule, + notes: notesModule, + }, + }), + }); + + spyOn(vm, 'toggleFileDiscussions'); + + vm.$el.querySelector('.js-btn-vue-toggle-comments').click(); + + expect(vm.toggleFileDiscussions).toHaveBeenCalled(); + }); }); }); }); diff --git a/spec/javascripts/diffs/store/getters_spec.js b/spec/javascripts/diffs/store/getters_spec.js index f5628a01a55..987c4dbcb26 100644 --- a/spec/javascripts/diffs/store/getters_spec.js +++ b/spec/javascripts/diffs/store/getters_spec.js @@ -167,6 +167,24 @@ describe('Diffs Module Getters', () => { }); }); + describe('diffHasDiscussions', () => { + it('returns true when getDiffFileDiscussions returns discussions', () => { + expect( + getters.diffHasDiscussions(localState, { + getDiffFileDiscussions: () => [discussionMock], + })(diffFileMock), + ).toEqual(true); + }); + + it('returns false when getDiffFileDiscussions returns no discussions', () => { + expect( + getters.diffHasDiscussions(localState, { + getDiffFileDiscussions: () => [], + })(diffFileMock), + ).toEqual(false); + }); + }); + describe('getDiffFileDiscussions', () => { it('returns an array with discussions when fileHash matches and the discussion belongs to a diff', () => { discussionMock.diff_file.file_hash = diffFileMock.fileHash; diff --git a/spec/javascripts/ide/components/ide_spec.js b/spec/javascripts/ide/components/ide_spec.js index 708c9fe69af..49b8e934cdd 100644 --- a/spec/javascripts/ide/components/ide_spec.js +++ b/spec/javascripts/ide/components/ide_spec.js @@ -45,6 +45,33 @@ describe('ide component', () => { }); }); + describe('onBeforeUnload', () => { + it('returns undefined when no staged files or changed files', () => { + expect(vm.onBeforeUnload()).toBe(undefined); + }); + + it('returns warning text when their are changed files', () => { + vm.$store.state.changedFiles.push(file()); + + expect(vm.onBeforeUnload()).toBe('Are you sure you want to lose unsaved changes?'); + }); + + it('returns warning text when their are staged files', () => { + vm.$store.state.stagedFiles.push(file()); + + expect(vm.onBeforeUnload()).toBe('Are you sure you want to lose unsaved changes?'); + }); + + it('updates event object', () => { + const event = {}; + vm.$store.state.stagedFiles.push(file()); + + vm.onBeforeUnload(event); + + expect(event.returnValue).toBe('Are you sure you want to lose unsaved changes?'); + }); + }); + describe('file finder', () => { beforeEach(done => { spyOn(vm, 'toggleFileFinder'); diff --git a/spec/javascripts/lib/utils/poll_spec.js b/spec/javascripts/lib/utils/poll_spec.js index 9b8f68f1676..523f4997bc0 100644 --- a/spec/javascripts/lib/utils/poll_spec.js +++ b/spec/javascripts/lib/utils/poll_spec.js @@ -1,4 +1,5 @@ import Poll from '~/lib/utils/poll'; +import { successCodes } from '~/lib/utils/http_status'; const waitForAllCallsToFinish = (service, waitForCount, successCallback) => { const timer = () => { @@ -91,28 +92,32 @@ describe('Poll', () => { }).catch(done.fail); }); - it('starts polling when http status is 200 and interval header is provided', (done) => { - mockServiceCall(service, { status: 200, headers: { 'poll-interval': 1 } }); + describe('for 2xx status code', () => { + successCodes.forEach(httpCode => { + it(`starts polling when http status is ${httpCode} and interval header is provided`, (done) => { + mockServiceCall(service, { status: httpCode, headers: { 'poll-interval': 1 } }); - const Polling = new Poll({ - resource: service, - method: 'fetch', - data: { page: 1 }, - successCallback: callbacks.success, - errorCallback: callbacks.error, - }); + const Polling = new Poll({ + resource: service, + method: 'fetch', + data: { page: 1 }, + successCallback: callbacks.success, + errorCallback: callbacks.error, + }); - Polling.makeRequest(); + Polling.makeRequest(); - waitForAllCallsToFinish(service, 2, () => { - Polling.stop(); + waitForAllCallsToFinish(service, 2, () => { + Polling.stop(); - expect(service.fetch.calls.count()).toEqual(2); - expect(service.fetch).toHaveBeenCalledWith({ page: 1 }); - expect(callbacks.success).toHaveBeenCalled(); - expect(callbacks.error).not.toHaveBeenCalled(); + expect(service.fetch.calls.count()).toEqual(2); + expect(service.fetch).toHaveBeenCalledWith({ page: 1 }); + expect(callbacks.success).toHaveBeenCalled(); + expect(callbacks.error).not.toHaveBeenCalled(); - done(); + done(); + }); + }); }); }); diff --git a/spec/javascripts/vue_shared/components/clipboard_button_spec.js b/spec/javascripts/vue_shared/components/clipboard_button_spec.js index e135690349e..ea525b1e44f 100644 --- a/spec/javascripts/vue_shared/components/clipboard_button_spec.js +++ b/spec/javascripts/vue_shared/components/clipboard_button_spec.js @@ -6,31 +6,47 @@ describe('clipboard button', () => { const Component = Vue.extend(clipboardButton); let vm; - beforeEach(() => { - vm = mountComponent(Component, { - text: 'copy me', - title: 'Copy this value into Clipboard!', - cssClass: 'btn-danger', - }); - }); - afterEach(() => { vm.$destroy(); }); - it('renders a button for clipboard', () => { - expect(vm.$el.tagName).toEqual('BUTTON'); - expect(vm.$el.getAttribute('data-clipboard-text')).toEqual('copy me'); - expect(vm.$el).toHaveSpriteIcon('duplicate'); - }); + describe('without gfm', () => { + beforeEach(() => { + vm = mountComponent(Component, { + text: 'copy me', + title: 'Copy this value into Clipboard!', + cssClass: 'btn-danger', + }); + }); - it('should have a tooltip with default values', () => { - expect(vm.$el.getAttribute('data-original-title')).toEqual('Copy this value into Clipboard!'); - expect(vm.$el.getAttribute('data-placement')).toEqual('top'); - expect(vm.$el.getAttribute('data-container')).toEqual(null); + it('renders a button for clipboard', () => { + expect(vm.$el.tagName).toEqual('BUTTON'); + expect(vm.$el.getAttribute('data-clipboard-text')).toEqual('copy me'); + expect(vm.$el).toHaveSpriteIcon('duplicate'); + }); + + it('should have a tooltip with default values', () => { + expect(vm.$el.getAttribute('data-original-title')).toEqual('Copy this value into Clipboard!'); + expect(vm.$el.getAttribute('data-placement')).toEqual('top'); + expect(vm.$el.getAttribute('data-container')).toEqual(null); + }); + + it('should render provided classname', () => { + expect(vm.$el.classList).toContain('btn-danger'); + }); }); - it('should render provided classname', () => { - expect(vm.$el.classList).toContain('btn-danger'); + describe('with gfm', () => { + it('sets data-clipboard-text with gfm', () => { + vm = mountComponent(Component, { + text: 'copy me', + gfm: '`path/to/file`', + title: 'Copy this value into Clipboard!', + cssClass: 'btn-danger', + }); + expect(vm.$el.getAttribute('data-clipboard-text')).toEqual( + '{"text":"copy me","gfm":"`path/to/file`"}', + ); + }); }); }); diff --git a/spec/lib/gitlab/auth/activity_spec.rb b/spec/lib/gitlab/auth/activity_spec.rb new file mode 100644 index 00000000000..07854cb1eba --- /dev/null +++ b/spec/lib/gitlab/auth/activity_spec.rb @@ -0,0 +1,30 @@ +require 'fast_spec_helper' + +describe Gitlab::Auth::Activity do + describe '.each_counter' do + it 'has all static counters defined' do + described_class.each_counter do |counter| + expect(described_class).to respond_to(counter) + end + end + + it 'has all static incrementers defined' do + described_class.each_counter do |counter| + expect(described_class).to respond_to("#{counter}_increment!") + end + end + + it 'has all counters starting with `user_`' do + described_class.each_counter do |counter| + expect(counter).to start_with('user_') + end + end + + it 'yields counter method, name and description' do + described_class.each_counter do |method, name, description| + expect(method).to eq "#{name}_counter" + expect(description).to start_with('Counter of') + end + end + end +end diff --git a/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb index 43b68e69131..13c09b9cb9b 100644 --- a/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb +++ b/spec/lib/gitlab/auth/blocked_user_tracker_spec.rb @@ -3,24 +3,30 @@ require 'spec_helper' describe Gitlab::Auth::BlockedUserTracker do set(:user) { create(:user) } - describe '.log_if_user_blocked' do + describe '#log_blocked_user_activity!' do it 'does not log if user failed to login due to undefined reason' do expect_any_instance_of(SystemHooksService).not_to receive(:execute_hooks_for) - expect(described_class.log_if_user_blocked({})).to be_nil + tracker = described_class.new({}) + + expect(tracker.user).to be_nil + expect(tracker.user_blocked?).to be_falsey + expect(tracker.log_blocked_user_activity!).to be_nil end it 'gracefully handles malformed environment variables' do - env = { 'warden.options' => 'test' } + tracker = described_class.new({ 'warden.options' => 'test' }) - expect(described_class.log_if_user_blocked(env)).to be_nil + expect(tracker.user).to be_nil + expect(tracker.user_blocked?).to be_falsey + expect(tracker.log_blocked_user_activity!).to be_nil end context 'failed login due to blocked user' do let(:base_env) { { 'warden.options' => { message: User::BLOCKED_MESSAGE } } } let(:env) { base_env.merge(request_env) } - subject { described_class.log_if_user_blocked(env) } + subject { described_class.new(env) } before do expect_any_instance_of(SystemHooksService).to receive(:execute_hooks_for).with(user, :failed_login) @@ -32,14 +38,17 @@ describe Gitlab::Auth::BlockedUserTracker do it 'logs a blocked user' do user.block! - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.user_blocked?).to be true + expect(subject.log_blocked_user_activity!).to be_truthy end it 'logs a blocked user by e-mail' do user.block! env[described_class::ACTIVE_RECORD_REQUEST_PARAMS]['user']['login'] = user.email - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.log_blocked_user_activity!).to be_truthy end end @@ -49,13 +58,17 @@ describe Gitlab::Auth::BlockedUserTracker do it 'logs a blocked user' do user.block! - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.user_blocked?).to be true + expect(subject.log_blocked_user_activity!).to be_truthy end it 'logs a LDAP blocked user' do user.ldap_block! - expect(subject).to be_truthy + expect(subject.user).to be_blocked + expect(subject.user_blocked?).to be true + expect(subject.log_blocked_user_activity!).to be_truthy end end end diff --git a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb index 5c31423fdee..d48aac15f28 100644 --- a/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/artifacts_spec.rb @@ -18,6 +18,14 @@ describe Gitlab::Ci::Config::Entry::Artifacts do expect(entry).to be_valid end end + + context "when value includes 'reports' keyword" do + let(:config) { { paths: %w[public/], reports: { junit: 'junit.xml' } } } + + it 'returns general artifact and report-type artifacts configuration' do + expect(entry.value).to eq config + end + end end context 'when entry value is not correct' do @@ -39,6 +47,15 @@ describe Gitlab::Ci::Config::Entry::Artifacts do .to include 'artifacts config contains unknown keys: test' end end + + context "when 'reports' keyword is not hash" do + let(:config) { { paths: %w[public/], reports: 'junit.xml' } } + + it 'reports error' do + expect(entry.errors) + .to include 'artifacts reports should be a hash' + end + end end end end diff --git a/spec/lib/gitlab/ci/config/entry/commands_spec.rb b/spec/lib/gitlab/ci/config/entry/commands_spec.rb index afa4a089418..8934aeb83db 100644 --- a/spec/lib/gitlab/ci/config/entry/commands_spec.rb +++ b/spec/lib/gitlab/ci/config/entry/commands_spec.rb @@ -41,8 +41,7 @@ describe Gitlab::Ci::Config::Entry::Commands do describe '#errors' do it 'saves errors' do expect(entry.errors) - .to include 'commands config should be a ' \ - 'string or an array of strings' + .to include 'commands config should be an array of strings or a string' end end end diff --git a/spec/lib/gitlab/ci/config/entry/reports_spec.rb b/spec/lib/gitlab/ci/config/entry/reports_spec.rb new file mode 100644 index 00000000000..b3a3a6bee1d --- /dev/null +++ b/spec/lib/gitlab/ci/config/entry/reports_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Entry::Reports do + let(:entry) { described_class.new(config) } + + describe 'validation' do + context 'when entry config value is correct' do + let(:config) { { junit: %w[junit.xml] } } + + describe '#value' do + it 'returns artifacs configuration' do + expect(entry.value).to eq config + end + end + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + + context 'when value is not array' do + let(:config) { { junit: 'junit.xml' } } + + it 'converts to array' do + expect(entry.value).to eq({ junit: ['junit.xml'] } ) + end + end + end + + context 'when entry value is not correct' do + describe '#errors' do + context 'when value of attribute is invalid' do + let(:config) { { junit: 10 } } + + it 'reports error' do + expect(entry.errors) + .to include 'reports junit should be an array of strings or a string' + end + end + + context 'when there is an unknown key present' do + let(:config) { { codeclimate: 'codeclimate.json' } } + + it 'reports error' do + expect(entry.errors) + .to include 'reports config contains unknown keys: codeclimate' + end + end + end + end + end +end diff --git a/spec/lib/gitlab/middleware/basic_health_check_spec.rb b/spec/lib/gitlab/middleware/basic_health_check_spec.rb new file mode 100644 index 00000000000..187d903a5e1 --- /dev/null +++ b/spec/lib/gitlab/middleware/basic_health_check_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe Gitlab::Middleware::BasicHealthCheck do + let(:app) { double(:app) } + let(:middleware) { described_class.new(app) } + let(:env) { {} } + + describe '#call' do + context 'outside IP' do + before do + env['REMOTE_ADDR'] = '8.8.8.8' + end + + it 'returns a 404' do + env['PATH_INFO'] = described_class::HEALTH_PATH + + response = middleware.call(env) + + expect(response[0]).to eq(404) + end + + it 'forwards the call for other paths' do + env['PATH_INFO'] = '/' + + expect(app).to receive(:call) + + middleware.call(env) + end + end + + context 'whitelisted IP' do + before do + env['REMOTE_ADDR'] = '127.0.0.1' + end + + it 'returns 200 response when endpoint is hit' do + env['PATH_INFO'] = described_class::HEALTH_PATH + + expect(app).not_to receive(:call) + + response = middleware.call(env) + + expect(response[0]).to eq(200) + expect(response[1]).to eq({ 'Content-Type' => 'text/plain' }) + expect(response[2]).to eq(['GitLab OK']) + end + + it 'forwards the call for other paths' do + env['PATH_INFO'] = '/-/readiness' + + expect(app).to receive(:call) + + middleware.call(env) + end + end + end +end diff --git a/spec/models/ci/build_runner_session_spec.rb b/spec/models/ci/build_runner_session_spec.rb index 7183957aa50..35622366829 100644 --- a/spec/models/ci/build_runner_session_spec.rb +++ b/spec/models/ci/build_runner_session_spec.rb @@ -29,7 +29,7 @@ describe Ci::BuildRunnerSession, model: true do it 'adds Authorization header if authorization is present' do subject.authorization = 'whatever' - expect(terminal_specification[:headers]).to include(Authorization: 'whatever') + expect(terminal_specification[:headers]).to include(Authorization: ['whatever']) end end end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 67199eb6d26..e4fa04baae6 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -514,6 +514,44 @@ describe Ci::Build do end end + describe '#has_test_reports?' do + subject { build.has_test_reports? } + + context 'when build has a test report' do + let(:build) { create(:ci_build, :test_reports) } + + it { is_expected.to be_truthy } + end + + context 'when build does not have test reports' do + let(:build) { create(:ci_build, :artifacts) } + + it { is_expected.to be_falsy } + end + end + + describe '#erase_test_reports!' do + subject { build.erase_test_reports! } + + context 'when build has a test report' do + let!(:build) { create(:ci_build, :test_reports) } + + it 'removes a test report' do + subject + + expect(build.has_test_reports?).to be_falsy + end + end + + context 'when build does not have test reports' do + let!(:build) { create(:ci_build, :artifacts) } + + it 'does not erase anything' do + expect { subject }.not_to change { Ci::JobArtifact.count } + end + end + end + describe '#has_old_trace?' do subject { build.has_old_trace? } @@ -776,6 +814,10 @@ describe Ci::Build do expect(build.artifacts_metadata.exists?).to be_falsy end + it 'removes test reports' do + expect(build.job_artifacts.test_reports.count).to eq(0) + end + it 'erases build trace in trace file' do expect(build).not_to have_trace end @@ -807,7 +849,7 @@ describe Ci::Build do context 'build is erasable' do context 'new artifacts' do - let!(:build) { create(:ci_build, :trace_artifact, :success, :artifacts) } + let!(:build) { create(:ci_build, :test_reports, :trace_artifact, :success, :artifacts) } describe '#erase' do before do diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index 0fd7612c011..4f34c2e81f8 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -15,6 +15,22 @@ describe Ci::JobArtifact do it { is_expected.to delegate_method(:open).to(:file) } it { is_expected.to delegate_method(:exists?).to(:file) } + describe '.test_reports' do + subject { described_class.test_reports } + + context 'when there is a test report' do + let!(:artifact) { create(:ci_job_artifact, :junit) } + + it { is_expected.to eq([artifact]) } + end + + context 'when there are no test reports' do + let!(:artifact) { create(:ci_job_artifact, :archive) } + + it { is_expected.to be_empty } + end + end + describe 'callbacks' do subject { create(:ci_job_artifact, :archive) } @@ -87,6 +103,40 @@ describe Ci::JobArtifact do end end + describe 'validates file format' do + subject { artifact } + + context 'when archive type with zip format' do + let(:artifact) { build(:ci_job_artifact, :archive, file_format: :zip) } + + it { is_expected.to be_valid } + end + + context 'when archive type with gzip format' do + let(:artifact) { build(:ci_job_artifact, :archive, file_format: :gzip) } + + it { is_expected.not_to be_valid } + end + + context 'when archive type without format specification' do + let(:artifact) { build(:ci_job_artifact, :archive, file_format: nil) } + + it { is_expected.not_to be_valid } + end + + context 'when junit type with zip format' do + let(:artifact) { build(:ci_job_artifact, :junit, file_format: :zip) } + + it { is_expected.not_to be_valid } + end + + context 'when junit type with gzip format' do + let(:artifact) { build(:ci_job_artifact, :junit, file_format: :gzip) } + + it { is_expected.to be_valid } + end + end + describe '#file' do subject { artifact.file } diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index 204d6b47832..55b984faecf 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -310,4 +310,24 @@ describe Milestone do expect(milestone.participants).to eq [user] end end + + describe '.sort_by_attribute' do + set(:milestone_1) { create(:milestone, title: 'Foo') } + set(:milestone_2) { create(:milestone, title: 'Bar') } + set(:milestone_3) { create(:milestone, title: 'Zoo') } + + context 'ordering by name ascending' do + it 'sorts by title ascending' do + expect(described_class.sort_by_attribute('name_asc')) + .to eq([milestone_2, milestone_1, milestone_3]) + end + end + + context 'ordering by name descending' do + it 'sorts by title descending' do + expect(described_class.sort_by_attribute('name_desc')) + .to eq([milestone_3, milestone_1, milestone_2]) + end + end + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index b75ca91b007..ef4167a3912 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3877,6 +3877,16 @@ describe Project do end end + context '#commits_by' do + let(:project) { create(:project, :repository) } + let(:commits) { project.repository.commits('HEAD', limit: 3).commits } + let(:commit_shas) { commits.map(&:id) } + + it 'retrieves several commits from the repository by oid' do + expect(project.commits_by(oids: commit_shas)).to eq commits + end + end + def rugged_config Gitlab::GitalyClient::StorageSettings.allow_disk_access do project.repository.rugged.config diff --git a/spec/models/project_statistics_spec.rb b/spec/models/project_statistics_spec.rb index 38a3590ad12..64c39f09e33 100644 --- a/spec/models/project_statistics_spec.rb +++ b/spec/models/project_statistics_spec.rb @@ -128,6 +128,12 @@ describe ProjectStatistics do .by(13) end + it 'increases also storage size by that amount' do + expect { described_class.increment_statistic(project.id, :build_artifacts_size, 20) } + .to change { statistics.reload.storage_size } + .by(20) + end + context 'when the amount is 0' do it 'does not execute a query' do project diff --git a/spec/presenters/ci/build_runner_presenter_spec.rb b/spec/presenters/ci/build_runner_presenter_spec.rb new file mode 100644 index 00000000000..e7019b990dd --- /dev/null +++ b/spec/presenters/ci/build_runner_presenter_spec.rb @@ -0,0 +1,86 @@ +require 'spec_helper' + +describe Ci::BuildRunnerPresenter do + let(:presenter) { described_class.new(build) } + let(:archive) { { paths: ['sample.txt'] } } + let(:junit) { { junit: ['junit.xml'] } } + + let(:archive_expectation) do + { + artifact_type: :archive, + artifact_format: :zip, + paths: archive[:paths], + untracked: archive[:untracked] + } + end + + let(:junit_expectation) do + { + name: 'junit.xml', + artifact_type: :junit, + artifact_format: :gzip, + paths: ['junit.xml'], + when: 'always' + } + end + + describe '#artifacts' do + context "when option contains archive-type artifacts" do + let(:build) { create(:ci_build, options: { artifacts: archive } ) } + + it 'presents correct hash' do + expect(presenter.artifacts.first).to include(archive_expectation) + end + + context "when untracked is specified" do + let(:archive) { { untracked: true } } + + it 'presents correct hash' do + expect(presenter.artifacts.first).to include(archive_expectation) + end + end + + context "when untracked and paths are missing" do + let(:archive) { { when: 'always' } } + + it 'does not present hash' do + expect(presenter.artifacts).to be_empty + end + end + end + + context "when option has 'junit' keyword" do + let(:build) { create(:ci_build, options: { artifacts: { reports: junit } } ) } + + it 'presents correct hash' do + expect(presenter.artifacts.first).to include(junit_expectation) + end + end + + context "when option has both archive and reports specification" do + let(:build) { create(:ci_build, options: { script: 'echo', artifacts: { **archive, reports: junit } } ) } + + it 'presents correct hash' do + expect(presenter.artifacts.first).to include(archive_expectation) + expect(presenter.artifacts.second).to include(junit_expectation) + end + + context "when archive specifies 'expire_in' keyword" do + let(:archive) { { paths: ['sample.txt'], expire_in: '3 mins 4 sec' } } + + it 'inherits expire_in from archive' do + expect(presenter.artifacts.first).to include({ **archive_expectation, expire_in: '3 mins 4 sec' }) + expect(presenter.artifacts.second).to include({ **junit_expectation, expire_in: '3 mins 4 sec' }) + end + end + end + + context "when option has no artifact keywords" do + let(:build) { create(:ci_build, :no_options) } + + it 'does not present hash' do + expect(presenter.artifacts).to be_nil + end + end + end +end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 8412d0383f7..5814d834572 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -655,13 +655,15 @@ describe API::Jobs do end context 'job is erasable' do - let(:job) { create(:ci_build, :trace_artifact, :artifacts, :success, project: project, pipeline: pipeline) } + let(:job) { create(:ci_build, :trace_artifact, :artifacts, :test_reports, :success, project: project, pipeline: pipeline) } it 'erases job content' do expect(response).to have_gitlab_http_status(201) + expect(job.job_artifacts.count).to eq(0) expect(job.trace.exist?).to be_falsy expect(job.artifacts_file.exists?).to be_falsy expect(job.artifacts_metadata.exists?).to be_falsy + expect(job.has_test_reports?).to be_falsy end it 'updates job' do diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index c621760b6c4..93e1c3a2294 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -231,6 +231,14 @@ describe API::Members do expect(response).to have_gitlab_http_status(409) end + it 'returns 404 when the user_id is not valid' do + post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), + user_id: 0, access_level: Member::MAINTAINER + + expect(response).to have_gitlab_http_status(404) + expect(json_response['message']).to eq('404 User Not Found') + end + it 'returns 400 when user_id is not given' do post api("/#{source_type.pluralize}/#{source.id}/members", maintainer), access_level: Member::MAINTAINER diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index d57993ab454..0f41e46cdd5 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -424,7 +424,9 @@ describe API::Runner, :clean_gitlab_redis_shared_state do 'untracked' => false, 'paths' => %w(out/), 'when' => 'always', - 'expire_in' => '7d' }] + 'expire_in' => '7d', + "artifact_type" => "archive", + "artifact_format" => "zip" }] end let(:expected_cache) do @@ -1420,6 +1422,56 @@ describe API::Runner, :clean_gitlab_redis_shared_state do end end end + + context 'when artifact_type is archive' do + context 'when artifact_format is zip' do + let(:params) { { artifact_type: :archive, artifact_format: :zip } } + + it 'stores junit test report' do + upload_artifacts(file_upload, headers_with_token, params) + + expect(response).to have_gitlab_http_status(201) + expect(job.reload.job_artifacts_archive).not_to be_nil + end + end + + context 'when artifact_format is gzip' do + let(:params) { { artifact_type: :archive, artifact_format: :gzip } } + + it 'returns an error' do + upload_artifacts(file_upload, headers_with_token, params) + + expect(response).to have_gitlab_http_status(400) + expect(job.reload.job_artifacts_archive).to be_nil + end + end + end + + context 'when artifact_type is junit' do + context 'when artifact_format is gzip' do + let(:file_upload) { fixture_file_upload('spec/fixtures/junit.xml.gz') } + let(:params) { { artifact_type: :junit, artifact_format: :gzip } } + + it 'stores junit test report' do + upload_artifacts(file_upload, headers_with_token, params) + + expect(response).to have_gitlab_http_status(201) + expect(job.reload.job_artifacts_junit).not_to be_nil + end + end + + context 'when artifact_format is raw' do + let(:file_upload) { fixture_file_upload('spec/fixtures/junit.xml.gz') } + let(:params) { { artifact_type: :junit, artifact_format: :raw } } + + it 'returns an error' do + upload_artifacts(file_upload, headers_with_token, params) + + expect(response).to have_gitlab_http_status(400) + expect(job.reload.job_artifacts_junit).to be_nil + end + end + end end context 'when artifacts are being stored outside of tmp path' do diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index ecf5d849d3f..18d52082399 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -24,7 +24,7 @@ describe Ci::RetryBuildService do artifacts_file artifacts_metadata artifacts_size created_at updated_at started_at finished_at queued_at erased_by erased_at auto_canceled_by job_artifacts job_artifacts_archive - job_artifacts_metadata job_artifacts_trace].freeze + job_artifacts_metadata job_artifacts_trace job_artifacts_junit].freeze IGNORE_ACCESSORS = %i[type lock_version target_url base_tags trace_sections @@ -38,7 +38,7 @@ describe Ci::RetryBuildService do let(:another_pipeline) { create(:ci_empty_pipeline, project: project) } let(:build) do - create(:ci_build, :failed, :artifacts, :expired, :erased, + create(:ci_build, :failed, :artifacts, :test_reports, :expired, :erased, :queued, :coverage, :tags, :allowed_to_fail, :on_tag, :triggered, :trace_artifact, :teardown_environment, description: 'my-job', stage: 'test', stage_id: stage.id, diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb index 3f62e7e61e1..657afb2b2b5 100644 --- a/spec/services/git_push_service_spec.rb +++ b/spec/services/git_push_service_spec.rb @@ -761,7 +761,7 @@ describe GitPushService, services: true do end it 'does not queue a CreateGpgSignatureWorker' do - expect(CreateGpgSignatureWorker).not_to receive(:perform_async).with(sample_commit.id, project.id) + expect(CreateGpgSignatureWorker).not_to receive(:perform_async) execute_service(project, user, oldrev, newrev, ref) end @@ -769,7 +769,15 @@ describe GitPushService, services: true do context 'when the signature is not yet cached' do it 'queues a CreateGpgSignatureWorker' do - expect(CreateGpgSignatureWorker).to receive(:perform_async).with(sample_commit.id, project.id) + expect(CreateGpgSignatureWorker).to receive(:perform_async).with([sample_commit.id], project.id) + + execute_service(project, user, oldrev, newrev, ref) + end + + it 'can queue several commits to create the gpg signature' do + allow(Gitlab::Git::Commit).to receive(:shas_with_signatures).and_return([sample_commit.id, another_sample_commit.id]) + + expect(CreateGpgSignatureWorker).to receive(:perform_async).with([sample_commit.id, another_sample_commit.id], project.id) execute_service(project, user, oldrev, newrev, ref) end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index fa98d05c61b..5bcfef46b75 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -55,6 +55,8 @@ describe Issues::UpdateService, :mailer do end it 'updates the issue with the given params' do + expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in) + update_issue(opts) expect(issue).to be_valid @@ -74,6 +76,21 @@ describe Issues::UpdateService, :mailer do .to change { project.open_issues_count }.from(1).to(0) end + it 'enqueues ConfidentialIssueWorker when an issue is made confidential' do + expect(TodosDestroyer::ConfidentialIssueWorker).to receive(:perform_in).with(1.hour, issue.id) + + update_issue(confidential: true) + end + + it 'does not enqueue ConfidentialIssueWorker when an issue is made non confidential' do + # set confidentiality to true before the actual update + issue.update!(confidential: true) + + expect(TodosDestroyer::ConfidentialIssueWorker).not_to receive(:perform_in) + + update_issue(confidential: false) + end + it 'updates open issue counter for assignees when issue is reassigned' do update_issue(assignee_ids: [user2.id]) diff --git a/spec/services/members/destroy_service_spec.rb b/spec/services/members/destroy_service_spec.rb index ef47b0a450b..0a5220c7c61 100644 --- a/spec/services/members/destroy_service_spec.rb +++ b/spec/services/members/destroy_service_spec.rb @@ -20,6 +20,11 @@ describe Members::DestroyService do end shared_examples 'a service destroying a member' do + before do + type = member.is_a?(GroupMember) ? 'Group' : 'Project' + expect(TodosDestroyer::EntityLeaveWorker).to receive(:perform_in).with(1.hour, member.user_id, member.source_id, type) + end + it 'destroys the member' do expect { described_class.new(current_user).execute(member, opts) }.to change { member.source.members_and_requesters.count }.by(-1) end diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index a4c103e6f30..36b619ba9be 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -79,7 +79,7 @@ describe Projects::UpdatePagesService do context "for a valid job" do before do create(:ci_job_artifact, file: file, job: build) - create(:ci_job_artifact, file_type: :metadata, file: metadata, job: build) + create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build) build.reload end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index ecf1ba05618..e6871545a0b 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -15,6 +15,8 @@ describe Projects::UpdateService do context 'when changing visibility level' do context 'when visibility_level is INTERNAL' do it 'updates the project to internal' do + expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in) + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) expect(result).to eq({ status: :success }) @@ -24,12 +26,30 @@ describe Projects::UpdateService do context 'when visibility_level is PUBLIC' do it 'updates the project to public' do + expect(TodosDestroyer::ProjectPrivateWorker).not_to receive(:perform_in) + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + expect(result).to eq({ status: :success }) expect(project).to be_public end end + context 'when visibility_level is PRIVATE' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC) + end + + it 'updates the project to private' do + expect(TodosDestroyer::ProjectPrivateWorker).to receive(:perform_in).with(1.hour, project.id) + + result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + expect(result).to eq({ status: :success }) + expect(project).to be_private + end + end + context 'when visibility levels are restricted to PUBLIC only' do before do stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC]) @@ -38,6 +58,7 @@ describe Projects::UpdateService do context 'when visibility_level is INTERNAL' do it 'updates the project to internal' do result = update_project(project, user, visibility_level: Gitlab::VisibilityLevel::INTERNAL) + expect(result).to eq({ status: :success }) expect(project).to be_internal end @@ -54,6 +75,7 @@ describe Projects::UpdateService do context 'when updated by an admin' do it 'updates the project to public' do result = update_project(project, admin, visibility_level: Gitlab::VisibilityLevel::PUBLIC) + expect(result).to eq({ status: :success }) expect(project).to be_public end @@ -166,6 +188,20 @@ describe Projects::UpdateService do end end + context 'when changing feature visibility to private' do + it 'updates the visibility correctly' do + expect(TodosDestroyer::PrivateFeaturesWorker) + .to receive(:perform_in).with(1.hour, project.id) + + result = update_project(project, user, project_feature_attributes: + { issues_access_level: ProjectFeature::PRIVATE } + ) + + expect(result).to eq({ status: :success }) + expect(project.project_feature.issues_access_level).to be(ProjectFeature::PRIVATE) + end + end + context 'when updating a project that contains container images' do before do stub_container_registry_config(enabled: true) diff --git a/spec/services/todos/destroy/confidential_issue_service_spec.rb b/spec/services/todos/destroy/confidential_issue_service_spec.rb new file mode 100644 index 00000000000..54d1d7e83f1 --- /dev/null +++ b/spec/services/todos/destroy/confidential_issue_service_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' + +describe Todos::Destroy::ConfidentialIssueService do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:author) { create(:user) } + let(:assignee) { create(:user) } + let(:guest) { create(:user) } + let(:project_member) { create(:user) } + let(:issue) { create(:issue, project: project, author: author, assignees: [assignee]) } + + let!(:todo_issue_non_member) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_member) { create(:todo, user: project_member, target: issue, project: project) } + let!(:todo_issue_author) { create(:todo, user: author, target: issue, project: project) } + let!(:todo_issue_asignee) { create(:todo, user: assignee, target: issue, project: project) } + let!(:todo_issue_guest) { create(:todo, user: guest, target: issue, project: project) } + let!(:todo_another_non_member) { create(:todo, user: user, project: project) } + + describe '#execute' do + before do + project.add_developer(project_member) + project.add_guest(guest) + end + + subject { described_class.new(issue.id).execute } + + context 'when provided issue is confidential' do + before do + issue.update!(confidential: true) + end + + it 'removes issue todos for a user who is not a project member' do + expect { subject }.to change { Todo.count }.from(6).to(4) + + expect(user.todos).to match_array([todo_another_non_member]) + expect(author.todos).to match_array([todo_issue_author]) + expect(project_member.todos).to match_array([todo_issue_member]) + end + end + + context 'when provided issue is not confidential' do + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + end +end diff --git a/spec/services/todos/destroy/entity_leave_service_spec.rb b/spec/services/todos/destroy/entity_leave_service_spec.rb new file mode 100644 index 00000000000..bad408a314e --- /dev/null +++ b/spec/services/todos/destroy/entity_leave_service_spec.rb @@ -0,0 +1,135 @@ +require 'spec_helper' + +describe Todos::Destroy::EntityLeaveService do + let(:group) { create(:group, :private) } + let(:project) { create(:project, group: group) } + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:mr) { create(:merge_request, source_project: project) } + + let!(:todo_mr_user) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_issue_user) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_user2) { create(:todo, user: user2, target: issue, project: project) } + + describe '#execute' do + context 'when a user leaves a project' do + subject { described_class.new(user.id, project.id, 'Project').execute } + + context 'when project is private' do + it 'removes todos for the provided user' do + expect { subject }.to change { Todo.count }.from(3).to(1) + + expect(user.todos).to be_empty + expect(user2.todos).to match_array([todo_issue_user2]) + end + end + + context 'when project is not private' do + before do + group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end + + context 'when a user is not an author of confidential issue' do + before do + issue.update!(confidential: true) + end + + it 'removes only confidential issues todos' do + expect { subject }.to change { Todo.count }.from(3).to(2) + end + end + + context 'when a user is an author of confidential issue' do + before do + issue.update!(author: user, confidential: true) + end + + it 'removes only confidential issues todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when a user is an assignee of confidential issue' do + before do + issue.update!(confidential: true) + issue.assignees << user + end + + it 'removes only confidential issues todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'feature visibility check' do + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only users issue todos' do + expect { subject }.to change { Todo.count }.from(3).to(2) + end + end + end + end + end + + context 'when a user leaves a group' do + subject { described_class.new(user.id, group.id, 'Group').execute } + + context 'when group is private' do + it 'removes todos for the user' do + expect { subject }.to change { Todo.count }.from(3).to(1) + + expect(user.todos).to be_empty + expect(user2.todos).to match_array([todo_issue_user2]) + end + + context 'with nested groups', :nested_groups do + let(:subgroup) { create(:group, :private, parent: group) } + let(:subproject) { create(:project, group: subgroup) } + + let!(:todo_subproject_user) { create(:todo, user: user, project: subproject) } + let!(:todo_subproject_user2) { create(:todo, user: user2, project: subproject) } + + it 'removes todos for the user including subprojects todos' do + expect { subject }.to change { Todo.count }.from(5).to(2) + + expect(user.todos).to be_empty + expect(user2.todos) + .to match_array([todo_issue_user2, todo_subproject_user2]) + end + end + end + + context 'when group is not private' do + before do + issue.update!(confidential: true) + + group.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + project.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL) + end + + it 'removes only confidential issues todos' do + expect { subject }.to change { Todo.count }.from(3).to(2) + end + end + end + + context 'when entity type is not valid' do + it 'raises an exception' do + expect { described_class.new(user.id, group.id, 'GroupWrongly').execute } + .to raise_error(ArgumentError) + end + end + + context 'when entity was not found' do + it 'does not remove any todos' do + expect { described_class.new(user.id, 999999, 'Group').execute } + .not_to change { Todo.count } + end + end + end +end diff --git a/spec/services/todos/destroy/private_features_service_spec.rb b/spec/services/todos/destroy/private_features_service_spec.rb new file mode 100644 index 00000000000..be8b5bb3979 --- /dev/null +++ b/spec/services/todos/destroy/private_features_service_spec.rb @@ -0,0 +1,143 @@ +require 'spec_helper' + +describe Todos::Destroy::PrivateFeaturesService do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:another_user) { create(:user) } + let(:project_member) { create(:user) } + let(:issue) { create(:issue, project: project) } + let(:mr) { create(:merge_request, source_project: project) } + + let!(:todo_mr_non_member) { create(:todo, user: user, target: mr, project: project) } + let!(:todo_mr_non_member2) { create(:todo, user: another_user, target: mr, project: project) } + let!(:todo_mr_member) { create(:todo, user: project_member, target: mr, project: project) } + let!(:todo_issue_non_member) { create(:todo, user: user, target: issue, project: project) } + let!(:todo_issue_non_member2) { create(:todo, user: another_user, target: issue, project: project) } + let!(:todo_issue_member) { create(:todo, user: project_member, target: issue, project: project) } + let!(:commit_todo_non_member) { create(:on_commit_todo, user: user, project: project) } + let!(:commit_todo_non_member2) { create(:on_commit_todo, user: another_user, project: project) } + let!(:commit_todo_member) { create(:on_commit_todo, user: project_member, project: project) } + + before do + project.add_developer(project_member) + end + + context 'when user_id is provided' do + subject { described_class.new(project.id, user.id).execute } + + context 'when all feaures have same visibility as the project' do + it 'removes only user issue todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members but the user is a member' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.add_developer(user) + end + + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(8) + end + end + + context 'when mrs, builds and repository are visible only to project members' do + before do + # builds and merge requests cannot have higher visibility than repository + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(repository_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user mr and commit todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs are visible only to project members' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user merge request todo' do + expect { subject }.to change { Todo.count }.from(9).to(8) + end + end + + context 'when mrs and issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only user merge request and issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + end + + context 'when user_id is not provided' do + subject { described_class.new(project.id).execute } + + context 'when all feaures have same visibility as the project' do + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + + context 'when issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs, builds and repository are visible only to project members' do + before do + # builds and merge requests cannot have higher visibility than repository + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(repository_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members mr and commit todos' do + expect { subject }.to change { Todo.count }.from(9).to(5) + end + end + + context 'when mrs are visible only to project members' do + before do + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members merge request todos' do + expect { subject }.to change { Todo.count }.from(9).to(7) + end + end + + context 'when mrs and issues are visible only to project members' do + before do + project.project_feature.update!(issues_access_level: ProjectFeature::PRIVATE) + project.project_feature.update!(merge_requests_access_level: ProjectFeature::PRIVATE) + end + + it 'removes only non members merge request and issue todos' do + expect { subject }.to change { Todo.count }.from(9).to(5) + end + end + end +end diff --git a/spec/services/todos/destroy/project_private_service_spec.rb b/spec/services/todos/destroy/project_private_service_spec.rb new file mode 100644 index 00000000000..badf3f913a5 --- /dev/null +++ b/spec/services/todos/destroy/project_private_service_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Todos::Destroy::ProjectPrivateService do + let(:project) { create(:project, :public) } + let(:user) { create(:user) } + let(:project_member) { create(:user) } + + let!(:todo_issue_non_member) { create(:todo, user: user, project: project) } + let!(:todo_issue_member) { create(:todo, user: project_member, project: project) } + let!(:todo_another_non_member) { create(:todo, user: user, project: project) } + + describe '#execute' do + before do + project.add_developer(project_member) + end + + subject { described_class.new(project.id).execute } + + context 'when a project set to private' do + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end + + it 'removes issue todos for a user who is not a member' do + expect { subject }.to change { Todo.count }.from(3).to(1) + + expect(user.todos).to be_empty + expect(project_member.todos).to match_array([todo_issue_member]) + end + end + + context 'when project is not private' do + it 'does not remove any todos' do + expect { subject }.not_to change { Todo.count } + end + end + end +end diff --git a/spec/support/helpers/stub_metrics.rb b/spec/support/helpers/stub_metrics.rb new file mode 100644 index 00000000000..64983fdf222 --- /dev/null +++ b/spec/support/helpers/stub_metrics.rb @@ -0,0 +1,27 @@ +module StubMetrics + def authentication_metrics + Gitlab::Auth::Activity + end + + def stub_authentication_activity_metrics(debug: false) + authentication_metrics.each_counter do |name, metric, description| + allow(authentication_metrics).to receive(name) + .and_return(double("#{metric} - #{description}")) + end + + debug_authentication_activity_metrics if debug + end + + def debug_authentication_activity_metrics + authentication_metrics.tap do |metrics| + metrics.each_counter do |name, metric| + "#{name}_increment!".tap do |incrementer| + allow(metrics).to receive(incrementer).and_wrap_original do |method| + puts "Authentication activity metric incremented: #{name}" + method.call + end + end + end + end + end +end diff --git a/spec/support/matchers/metric_counter_matcher.rb b/spec/support/matchers/metric_counter_matcher.rb new file mode 100644 index 00000000000..22d5cd17e3f --- /dev/null +++ b/spec/support/matchers/metric_counter_matcher.rb @@ -0,0 +1,11 @@ +RSpec::Matchers.define :increment do |counter| + match do |adapter| + expect(adapter.send(counter)) + .to receive(:increment) + .exactly(@exactly || :once) + end + + chain :twice do + @exactly = :twice + end +end diff --git a/spec/support/rspec.rb b/spec/support/rspec.rb index 54b8df7aa19..9b8bcebcb3a 100644 --- a/spec/support/rspec.rb +++ b/spec/support/rspec.rb @@ -1,4 +1,5 @@ require_relative "helpers/stub_configuration" +require_relative "helpers/stub_metrics" require_relative "helpers/stub_object_storage" require_relative "helpers/stub_env" @@ -7,6 +8,7 @@ RSpec.configure do |config| config.raise_errors_for_deprecations! config.include StubConfiguration + config.include StubMetrics config.include StubObjectStorage config.include StubENV diff --git a/spec/support/shared_examples/services/boards/issues_list_service.rb b/spec/support/shared_examples/services/boards/issues_list_service.rb index 3e744323cea..8b879cef084 100644 --- a/spec/support/shared_examples/services/boards/issues_list_service.rb +++ b/spec/support/shared_examples/services/boards/issues_list_service.rb @@ -7,6 +7,16 @@ shared_examples 'issues list service' do described_class.new(parent, user, params).execute end + context '#metadata' do + it 'returns issues count for list' do + params = { board_id: board.id, id: list1.id } + + metadata = described_class.new(parent, user, params).metadata + + expect(metadata[:size]).to eq(3) + end + end + context 'issues are ordered by priority' do it 'returns opened issues when list_id is missing' do params = { board_id: board.id } diff --git a/spec/tasks/gitlab/git_rake_spec.rb b/spec/tasks/gitlab/git_rake_spec.rb index d0263ad9a37..57b006e1a39 100644 --- a/spec/tasks/gitlab/git_rake_spec.rb +++ b/spec/tasks/gitlab/git_rake_spec.rb @@ -2,45 +2,19 @@ require 'rake_helper' describe 'gitlab:git rake tasks' do let(:base_path) { 'tmp/tests/default_storage' } - - before(:all) do - @default_storage_hash = Gitlab.config.repositories.storages.default.to_h - end + let!(:project) { create(:project, :repository) } before do Rake.application.rake_require 'tasks/gitlab/git' - storages = { 'default' => Gitlab::GitalyClient::StorageSettings.new(@default_storage_hash.merge('path' => base_path)) } - - path = Settings.absolute("#{base_path}/@hashed/1/2/test.git") - FileUtils.mkdir_p(path) - Gitlab::Popen.popen(%W[git -C #{path} init --bare]) - allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) allow_any_instance_of(String).to receive(:color) { |string, _color| string } stub_warn_user_is_not_gitlab end - after do - FileUtils.rm_rf(Settings.absolute(base_path)) - end - describe 'fsck' do it 'outputs the integrity check for a repo' do - expect { run_rake_task('gitlab:git:fsck') }.to output(%r{Performed Checking integrity at .*@hashed/1/2/test.git}).to_stdout - end - - it 'errors out about config.lock issues' do - FileUtils.touch(Settings.absolute("#{base_path}/@hashed/1/2/test.git/config.lock")) - - expect { run_rake_task('gitlab:git:fsck') }.to output(/file exists\? ... yes/).to_stdout - end - - it 'errors out about ref lock issues' do - FileUtils.mkdir_p(Settings.absolute("#{base_path}/@hashed/1/2/test.git/refs/heads")) - FileUtils.touch(Settings.absolute("#{base_path}/@hashed/1/2/test.git/refs/heads/blah.lock")) - - expect { run_rake_task('gitlab:git:fsck') }.to output(/Ref lock files exist:/).to_stdout + expect { run_rake_task('gitlab:git:fsck') }.to output(/Performed integrity check for/).to_stdout end end end diff --git a/spec/workers/create_gpg_signature_worker_spec.rb b/spec/workers/create_gpg_signature_worker_spec.rb index aa6c347d738..502765e9786 100644 --- a/spec/workers/create_gpg_signature_worker_spec.rb +++ b/spec/workers/create_gpg_signature_worker_spec.rb @@ -2,21 +2,37 @@ require 'spec_helper' describe CreateGpgSignatureWorker do let(:project) { create(:project, :repository) } + let(:commits) { project.repository.commits('HEAD', limit: 3).commits } + let(:commit_shas) { commits.map(&:id) } context 'when GpgKey is found' do - let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' } + let(:gpg_commit) { instance_double(Gitlab::Gpg::Commit) } + + before do + allow(Project).to receive(:find_by).with(id: project.id).and_return(project) + allow(project).to receive(:commits_by).with(oids: commit_shas).and_return(commits) + end + + subject { described_class.new.perform(commit_shas, project.id) } it 'calls Gitlab::Gpg::Commit#signature' do - commit = instance_double(Commit) - gpg_commit = instance_double(Gitlab::Gpg::Commit) + commits.each do |commit| + expect(Gitlab::Gpg::Commit).to receive(:new).with(commit).and_return(gpg_commit).once + end - allow(Project).to receive(:find_by).with(id: project.id).and_return(project) - allow(project).to receive(:commit).with(commit_sha).and_return(commit) + expect(gpg_commit).to receive(:signature).exactly(commits.size).times + + subject + end + + it 'can recover from exception and continue the signature process' do + allow(gpg_commit).to receive(:signature) + allow(Gitlab::Gpg::Commit).to receive(:new).and_return(gpg_commit) + allow(Gitlab::Gpg::Commit).to receive(:new).with(commits.first).and_raise(StandardError) - expect(Gitlab::Gpg::Commit).to receive(:new).with(commit).and_return(gpg_commit) - expect(gpg_commit).to receive(:signature) + expect(gpg_commit).to receive(:signature).exactly(2).times - described_class.new.perform(commit_sha, project.id) + subject end end @@ -24,7 +40,7 @@ describe CreateGpgSignatureWorker do let(:nonexisting_commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a34' } it 'does not raise errors' do - expect { described_class.new.perform(nonexisting_commit_sha, project.id) }.not_to raise_error + expect { described_class.new.perform([nonexisting_commit_sha], project.id) }.not_to raise_error end end @@ -32,13 +48,13 @@ describe CreateGpgSignatureWorker do let(:nonexisting_project_id) { -1 } it 'does not raise errors' do - expect { described_class.new.perform(anything, nonexisting_project_id) }.not_to raise_error + expect { described_class.new.perform(commit_shas, nonexisting_project_id) }.not_to raise_error end it 'does not call Gitlab::Gpg::Commit#signature' do expect_any_instance_of(Gitlab::Gpg::Commit).not_to receive(:signature) - described_class.new.perform(anything, nonexisting_project_id) + described_class.new.perform(commit_shas, nonexisting_project_id) end end end diff --git a/spec/workers/todos_destroyer/confidential_issue_worker_spec.rb b/spec/workers/todos_destroyer/confidential_issue_worker_spec.rb new file mode 100644 index 00000000000..9d7c0b8f560 --- /dev/null +++ b/spec/workers/todos_destroyer/confidential_issue_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::ConfidentialIssueWorker do + it "calls the Todos::Destroy::ConfidentialIssueService with the params it was given" do + service = double + + expect(::Todos::Destroy::ConfidentialIssueService).to receive(:new).with(100).and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100) + end +end diff --git a/spec/workers/todos_destroyer/entity_leave_worker_spec.rb b/spec/workers/todos_destroyer/entity_leave_worker_spec.rb new file mode 100644 index 00000000000..955447906aa --- /dev/null +++ b/spec/workers/todos_destroyer/entity_leave_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::EntityLeaveWorker do + it "calls the Todos::Destroy::EntityLeaveService with the params it was given" do + service = double + + expect(::Todos::Destroy::EntityLeaveService).to receive(:new).with(100, 5, 'Group').and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100, 5, 'Group') + end +end diff --git a/spec/workers/todos_destroyer/private_features_worker_spec.rb b/spec/workers/todos_destroyer/private_features_worker_spec.rb new file mode 100644 index 00000000000..9599f5ee071 --- /dev/null +++ b/spec/workers/todos_destroyer/private_features_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::PrivateFeaturesWorker do + it "calls the Todos::Destroy::PrivateFeaturesService with the params it was given" do + service = double + + expect(::Todos::Destroy::PrivateFeaturesService).to receive(:new).with(100, nil).and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100) + end +end diff --git a/spec/workers/todos_destroyer/project_private_worker_spec.rb b/spec/workers/todos_destroyer/project_private_worker_spec.rb new file mode 100644 index 00000000000..15d926fa9d5 --- /dev/null +++ b/spec/workers/todos_destroyer/project_private_worker_spec.rb @@ -0,0 +1,12 @@ +require 'spec_helper' + +describe TodosDestroyer::ProjectPrivateWorker do + it "calls the Todos::Destroy::ProjectPrivateService with the params it was given" do + service = double + + expect(::Todos::Destroy::ProjectPrivateService).to receive(:new).with(100).and_return(service) + expect(service).to receive(:execute) + + described_class.new.perform(100) + end +end |