diff options
Diffstat (limited to 'spec/controllers')
31 files changed, 646 insertions, 256 deletions
diff --git a/spec/controllers/admin/identities_controller_spec.rb b/spec/controllers/admin/identities_controller_spec.rb index c131d22a30a..a29853bf8df 100644 --- a/spec/controllers/admin/identities_controller_spec.rb +++ b/spec/controllers/admin/identities_controller_spec.rb @@ -2,7 +2,10 @@ require 'spec_helper' describe Admin::IdentitiesController do let(:admin) { create(:admin) } - before { sign_in(admin) } + + before do + sign_in(admin) + end describe 'UPDATE identity' do let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') } diff --git a/spec/controllers/admin/services_controller_spec.rb b/spec/controllers/admin/services_controller_spec.rb index c94616d8508..4ca0cfc74e9 100644 --- a/spec/controllers/admin/services_controller_spec.rb +++ b/spec/controllers/admin/services_controller_spec.rb @@ -3,7 +3,9 @@ require 'spec_helper' describe Admin::ServicesController do let(:admin) { create(:admin) } - before { sign_in(admin) } + before do + sign_in(admin) + end describe 'GET #edit' do let!(:project) { create(:empty_project) } diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 2c9d1ffc9c2..b40f647644d 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -170,27 +170,39 @@ describe AutocompleteController do end context 'author of issuable included' do - before do - sign_in(user) - end - let(:body) { JSON.parse(response.body) } - it 'includes the author' do - get(:users, author_id: non_member.id) + context 'authenticated' do + before do + sign_in(user) + end - expect(body.first["username"]).to eq non_member.username + it 'includes the author' do + get(:users, author_id: non_member.id) + + expect(body.first["username"]).to eq non_member.username + end + + it 'rejects non existent user ids' do + get(:users, author_id: 99999) + + expect(body.collect { |u| u['id'] }).not_to include(99999) + end end - it 'rejects non existent user ids' do - get(:users, author_id: 99999) + context 'without authenticating' do + it 'returns empty result' do + get(:users, author_id: non_member.id) - expect(body.collect { |u| u['id'] }).not_to include(99999) + expect(body).to be_empty + end end end context 'skip_users parameter included' do - before { sign_in(user) } + before do + sign_in(user) + end it 'skips the user IDs passed' do get(:users, skip_users: [user, user2].map(&:id)) diff --git a/spec/controllers/dashboard/milestones_controller_spec.rb b/spec/controllers/dashboard/milestones_controller_spec.rb new file mode 100644 index 00000000000..424f39fd3b8 --- /dev/null +++ b/spec/controllers/dashboard/milestones_controller_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Dashboard::MilestonesController do + let(:project) { create(:empty_project) } + let(:user) { create(:user) } + let(:project_milestone) { create(:milestone, project: project) } + let(:milestone) do + DashboardMilestone.build( + [project], + project_milestone.title + ) + end + let(:issue) { create(:issue, project: project, milestone: project_milestone) } + let!(:label) { create(:label, project: project, title: 'Issue Label', issues: [issue]) } + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: project_milestone) } + let(:milestone_path) { dashboard_milestone_path(milestone.safe_title, title: milestone.title) } + + before do + sign_in(user) + project.team << [user, :master] + end + + it_behaves_like 'milestone tabs' + + describe "#show" do + render_views + + def view_milestone + get :show, id: milestone.safe_title, title: milestone.title + end + + it 'shows milestone page' do + view_milestone + + expect(response).to have_http_status(200) + end + end +end diff --git a/spec/controllers/groups/group_members_controller_spec.rb b/spec/controllers/groups/group_members_controller_spec.rb index 60db0192dfd..cce53f6697c 100644 --- a/spec/controllers/groups/group_members_controller_spec.rb +++ b/spec/controllers/groups/group_members_controller_spec.rb @@ -16,10 +16,14 @@ describe Groups::GroupMembersController do describe 'POST create' do let(:group_user) { create(:user) } - before { sign_in(user) } + before do + sign_in(user) + end context 'when user does not have enough rights' do - before { group.add_developer(user) } + before do + group.add_developer(user) + end it 'returns 403' do post :create, group_id: group, @@ -32,7 +36,9 @@ describe Groups::GroupMembersController do end context 'when user has enough rights' do - before { group.add_owner(user) } + before do + group.add_owner(user) + end it 'adds user to members' do post :create, group_id: group, @@ -59,7 +65,9 @@ describe Groups::GroupMembersController do describe 'DELETE destroy' do let(:member) { create(:group_member, :developer, group: group) } - before { sign_in(user) } + before do + sign_in(user) + end context 'when member is not found' do it 'returns 403' do @@ -71,7 +79,9 @@ describe Groups::GroupMembersController do context 'when member is found' do context 'when user does not have enough rights' do - before { group.add_developer(user) } + before do + group.add_developer(user) + end it 'returns 403' do delete :destroy, group_id: group, id: member @@ -82,7 +92,9 @@ describe Groups::GroupMembersController do end context 'when user has enough rights' do - before { group.add_owner(user) } + before do + group.add_owner(user) + end it '[HTML] removes user from members' do delete :destroy, group_id: group, id: member @@ -103,7 +115,9 @@ describe Groups::GroupMembersController do end describe 'DELETE leave' do - before { sign_in(user) } + before do + sign_in(user) + end context 'when member is not found' do it 'returns 404' do @@ -115,7 +129,9 @@ describe Groups::GroupMembersController do context 'when member is found' do context 'and is not an owner' do - before { group.add_developer(user) } + before do + group.add_developer(user) + end it 'removes user from members' do delete :leave, group_id: group @@ -124,10 +140,19 @@ describe Groups::GroupMembersController do expect(response).to redirect_to(dashboard_groups_path) expect(group.users).not_to include user end + + it 'supports json request' do + delete :leave, group_id: group, format: :json + + expect(response).to have_http_status(200) + expect(json_response['notice']).to eq "You left the \"#{group.name}\" group." + end end context 'and is an owner' do - before { group.add_owner(user) } + before do + group.add_owner(user) + end it 'cannot removes himself from the group' do delete :leave, group_id: group @@ -137,7 +162,9 @@ describe Groups::GroupMembersController do end context 'and is a requester' do - before { group.request_access(user) } + before do + group.request_access(user) + end it 'removes user from members' do delete :leave, group_id: group @@ -152,7 +179,9 @@ describe Groups::GroupMembersController do end describe 'POST request_access' do - before { sign_in(user) } + before do + sign_in(user) + end it 'creates a new GroupMember that is not a team member' do post :request_access, group_id: group @@ -167,7 +196,9 @@ describe Groups::GroupMembersController do describe 'POST approve_access_request' do let(:member) { create(:group_member, :access_request, group: group) } - before { sign_in(user) } + before do + sign_in(user) + end context 'when member is not found' do it 'returns 403' do @@ -179,7 +210,9 @@ describe Groups::GroupMembersController do context 'when member is found' do context 'when user does not have enough rights' do - before { group.add_developer(user) } + before do + group.add_developer(user) + end it 'returns 403' do post :approve_access_request, group_id: group, id: member @@ -190,7 +223,9 @@ describe Groups::GroupMembersController do end context 'when user has enough rights' do - before { group.add_owner(user) } + before do + group.add_owner(user) + end it 'adds user to members' do post :approve_access_request, group_id: group, id: member diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb index b8b6e0c3a88..e7c19b47a6a 100644 --- a/spec/controllers/health_controller_spec.rb +++ b/spec/controllers/health_controller_spec.rb @@ -54,43 +54,4 @@ describe HealthController do end end end - - describe '#metrics' do - context 'authorization token provided' do - before do - request.headers['TOKEN'] = token - end - - it 'returns DB ping metrics' do - get :metrics - expect(response.body).to match(/^db_ping_timeout 0$/) - expect(response.body).to match(/^db_ping_success 1$/) - expect(response.body).to match(/^db_ping_latency [0-9\.]+$/) - end - - it 'returns Redis ping metrics' do - get :metrics - expect(response.body).to match(/^redis_ping_timeout 0$/) - expect(response.body).to match(/^redis_ping_success 1$/) - expect(response.body).to match(/^redis_ping_latency [0-9\.]+$/) - end - - it 'returns file system check metrics' do - get :metrics - expect(response.body).to match(/^filesystem_access_latency{shard="default"} [0-9\.]+$/) - expect(response.body).to match(/^filesystem_accessible{shard="default"} 1$/) - expect(response.body).to match(/^filesystem_write_latency{shard="default"} [0-9\.]+$/) - expect(response.body).to match(/^filesystem_writable{shard="default"} 1$/) - expect(response.body).to match(/^filesystem_read_latency{shard="default"} [0-9\.]+$/) - expect(response.body).to match(/^filesystem_readable{shard="default"} 1$/) - end - end - - context 'without authorization token' do - it 'returns proper response' do - get :metrics - expect(response.status).to eq(404) - end - end - end end diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb new file mode 100644 index 00000000000..044c9f179ed --- /dev/null +++ b/spec/controllers/metrics_controller_spec.rb @@ -0,0 +1,70 @@ +require 'spec_helper' + +describe MetricsController do + include StubENV + + let(:token) { current_application_settings.health_check_access_token } + let(:json_response) { JSON.parse(response.body) } + let(:metrics_multiproc_dir) { Dir.mktmpdir } + + before do + stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + stub_env('prometheus_multiproc_dir', metrics_multiproc_dir) + allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(true) + end + + describe '#index' do + context 'authorization token provided' do + before do + request.headers['TOKEN'] = token + end + + it 'returns DB ping metrics' do + get :index + + expect(response.body).to match(/^db_ping_timeout 0$/) + expect(response.body).to match(/^db_ping_success 1$/) + expect(response.body).to match(/^db_ping_latency [0-9\.]+$/) + end + + it 'returns Redis ping metrics' do + get :index + + expect(response.body).to match(/^redis_ping_timeout 0$/) + expect(response.body).to match(/^redis_ping_success 1$/) + expect(response.body).to match(/^redis_ping_latency [0-9\.]+$/) + end + + it 'returns file system check metrics' do + get :index + + expect(response.body).to match(/^filesystem_access_latency{shard="default"} [0-9\.]+$/) + expect(response.body).to match(/^filesystem_accessible{shard="default"} 1$/) + expect(response.body).to match(/^filesystem_write_latency{shard="default"} [0-9\.]+$/) + expect(response.body).to match(/^filesystem_writable{shard="default"} 1$/) + expect(response.body).to match(/^filesystem_read_latency{shard="default"} [0-9\.]+$/) + expect(response.body).to match(/^filesystem_readable{shard="default"} 1$/) + end + + context 'prometheus metrics are disabled' do + before do + allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(false) + end + + it 'returns proper response' do + get :index + + expect(response.status).to eq(404) + end + end + end + + context 'without authorization token' do + it 'returns proper response' do + get :index + + expect(response.status).to eq(404) + end + end + end +end diff --git a/spec/controllers/notification_settings_controller_spec.rb b/spec/controllers/notification_settings_controller_spec.rb index 9e3a31e1a6b..6b690407ce3 100644 --- a/spec/controllers/notification_settings_controller_spec.rb +++ b/spec/controllers/notification_settings_controller_spec.rb @@ -58,7 +58,10 @@ describe NotificationSettingsController do expect(response.status).to eq 200 expect(notification_setting.level).to eq("custom") - expect(notification_setting.events).to eq(custom_events) + + custom_events.each do |event, value| + expect(notification_setting.event_enabled?(event)).to eq(value) + end end end end @@ -86,7 +89,10 @@ describe NotificationSettingsController do expect(response.status).to eq 200 expect(notification_setting.level).to eq("custom") - expect(notification_setting.events).to eq(custom_events) + + custom_events.each do |event, value| + expect(notification_setting.event_enabled?(event)).to eq(value) + end end end end @@ -94,7 +100,10 @@ describe NotificationSettingsController do context 'not authorized' do let(:private_project) { create(:empty_project, :private) } - before { sign_in(user) } + + before do + sign_in(user) + end it 'returns 404' do post :create, @@ -120,7 +129,9 @@ describe NotificationSettingsController do end context 'when authorized' do - before{ sign_in(user) } + before do + sign_in(user) + end it 'returns success' do put :update, @@ -152,7 +163,9 @@ describe NotificationSettingsController do context 'not authorized' do let(:other_user) { create(:user) } - before { sign_in(other_user) } + before do + sign_in(other_user) + end it 'returns 404' do put :update, diff --git a/spec/controllers/profiles/personal_access_tokens_controller_spec.rb b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb index 98a43e278b2..ed08a4c1bf2 100644 --- a/spec/controllers/profiles/personal_access_tokens_controller_spec.rb +++ b/spec/controllers/profiles/personal_access_tokens_controller_spec.rb @@ -4,7 +4,9 @@ describe Profiles::PersonalAccessTokensController do let(:user) { create(:user) } let(:token_attributes) { attributes_for(:personal_access_token) } - before { sign_in(user) } + before do + sign_in(user) + end describe '#create' do def created_token @@ -38,7 +40,9 @@ describe Profiles::PersonalAccessTokensController do let!(:inactive_personal_access_token) { create(:personal_access_token, :revoked, user: user) } let!(:impersonation_personal_access_token) { create(:personal_access_token, :impersonation, user: user) } - before { get :index } + before do + get :index + end it "retrieves active personal access tokens" do expect(assigns(:active_personal_access_tokens)).to include(active_personal_access_token) diff --git a/spec/controllers/profiles_controller_spec.rb b/spec/controllers/profiles_controller_spec.rb new file mode 100644 index 00000000000..9d60dab12d1 --- /dev/null +++ b/spec/controllers/profiles_controller_spec.rb @@ -0,0 +1,31 @@ +require('spec_helper') + +describe ProfilesController do + describe "PUT update" do + it "allows an email update from a user without an external email address" do + user = create(:user) + sign_in(user) + + put :update, + user: { email: "john@gmail.com", name: "John" } + + user.reload + + expect(response.status).to eq(302) + expect(user.unconfirmed_email).to eq('john@gmail.com') + end + + it "ignores an email update from a user with an external email address" do + ldap_user = create(:omniauth_user, external_email: true) + sign_in(ldap_user) + + put :update, + user: { email: "john@gmail.com", name: "John" } + + ldap_user.reload + + expect(response.status).to eq(302) + expect(ldap_user.unconfirmed_email).not_to eq('john@gmail.com') + end + end +end diff --git a/spec/controllers/projects/boards/lists_controller_spec.rb b/spec/controllers/projects/boards/lists_controller_spec.rb index 432f3c53c90..0f2664262e8 100644 --- a/spec/controllers/projects/boards/lists_controller_spec.rb +++ b/spec/controllers/projects/boards/lists_controller_spec.rb @@ -27,7 +27,7 @@ describe Projects::Boards::ListsController do parsed_response = JSON.parse(response.body) expect(response).to match_response_schema('lists') - expect(parsed_response.length).to eq 2 + expect(parsed_response.length).to eq 3 end context 'with unauthorized user' do diff --git a/spec/controllers/projects/branches_controller_spec.rb b/spec/controllers/projects/branches_controller_spec.rb index f285e5333d6..f9e21f9d8f6 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -367,19 +367,5 @@ describe Projects::BranchesController do expect(parsed_response.first).to eq 'master' end end - - context 'show_all = true' do - it 'returns all the branches name' do - get :index, - namespace_id: project.namespace, - project_id: project, - format: :json, - show_all: true - - parsed_response = JSON.parse(response.body) - - expect(parsed_response.length).to eq(project.repository.branches.count) - end - end end end diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 69e4706dc71..7fb08df1950 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -281,7 +281,9 @@ describe Projects::CommitController do end context 'when the path does not exist in the diff' do - before { diff_for_path(id: commit.id, old_path: existing_path.succ, new_path: existing_path.succ) } + before do + diff_for_path(id: commit.id, old_path: existing_path.succ, new_path: existing_path.succ) + end it 'returns a 404' do expect(response).to have_http_status(404) @@ -302,7 +304,9 @@ describe Projects::CommitController do end context 'when the commit does not exist' do - before { diff_for_path(id: commit.id.succ, old_path: existing_path, new_path: existing_path) } + before do + diff_for_path(id: commit.id.succ, old_path: existing_path, new_path: existing_path) + end it 'returns a 404' do expect(response).to have_http_status(404) diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 15ac4e0925a..8f4694c9854 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -128,7 +128,9 @@ describe Projects::CompareController do end context 'when the path does not exist in the diff' do - before { diff_for_path(from: ref_from, to: ref_to, old_path: existing_path.succ, new_path: existing_path.succ) } + before do + diff_for_path(from: ref_from, to: ref_to, old_path: existing_path.succ, new_path: existing_path.succ) + end it 'returns a 404' do expect(response).to have_http_status(404) @@ -149,7 +151,9 @@ describe Projects::CompareController do end context 'when the from ref does not exist' do - before { diff_for_path(from: ref_from.succ, to: ref_to, old_path: existing_path, new_path: existing_path) } + before do + diff_for_path(from: ref_from.succ, to: ref_to, old_path: existing_path, new_path: existing_path) + end it 'returns a 404' do expect(response).to have_http_status(404) @@ -157,7 +161,9 @@ describe Projects::CompareController do end context 'when the to ref does not exist' do - before { diff_for_path(from: ref_from, to: ref_to.succ, old_path: existing_path, new_path: existing_path) } + before do + diff_for_path(from: ref_from, to: ref_to.succ, old_path: existing_path, new_path: existing_path) + end it 'returns a 404' do expect(response).to have_http_status(404) diff --git a/spec/controllers/projects/forks_controller_spec.rb b/spec/controllers/projects/forks_controller_spec.rb index 8282d79298f..dc8290c438e 100644 --- a/spec/controllers/projects/forks_controller_spec.rb +++ b/spec/controllers/projects/forks_controller_spec.rb @@ -14,7 +14,9 @@ describe Projects::ForksController do end context 'when fork is public' do - before { forked_project.update_attribute(:visibility_level, Project::PUBLIC) } + before do + forked_project.update_attribute(:visibility_level, Project::PUBLIC) + end it 'is visible for non logged in users' do get_forks @@ -35,7 +37,9 @@ describe Projects::ForksController do end context 'when user is logged in' do - before { sign_in(project.creator) } + before do + sign_in(project.creator) + end context 'when user is not a Project member neither a group member' do it 'does not see the Project listed' do @@ -46,7 +50,9 @@ describe Projects::ForksController do end context 'when user is a member of the Project' do - before { forked_project.team << [project.creator, :developer] } + before do + forked_project.team << [project.creator, :developer] + end it 'sees the project listed' do get_forks @@ -56,7 +62,9 @@ describe Projects::ForksController do end context 'when user is a member of the Group' do - before { forked_project.group.add_developer(project.creator) } + before do + forked_project.group.add_developer(project.creator) + end it 'sees the project listed' do get_forks diff --git a/spec/controllers/projects/group_links_controller_spec.rb b/spec/controllers/projects/group_links_controller_spec.rb index ca4a8e871c0..b5435357f53 100644 --- a/spec/controllers/projects/group_links_controller_spec.rb +++ b/spec/controllers/projects/group_links_controller_spec.rb @@ -22,7 +22,10 @@ describe Projects::GroupLinksController do end context 'when user has access to group he want to link project to' do - before { group.add_developer(user) } + before do + group.add_developer(user) + end + include_context 'link project to group' it 'links project with selected group' do diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index a38ae2eb990..f853bfe370c 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -212,7 +212,9 @@ describe Projects::IssuesController do let(:another_project) { create(:empty_project, :private) } context 'when user has access to move issue' do - before { another_project.team << [user, :reporter] } + before do + another_project.team << [user, :reporter] + end it 'moves issue to another project' do move_issue @@ -250,16 +252,21 @@ describe Projects::IssuesController do end context 'when an issue is identified as spam' do - before { allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) } + before do + allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + end context 'when captcha is not verified' do def update_spam_issue update_issue(title: 'Spam Title', description: 'Spam lives here') end - before { allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) } + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) + end it 'rejects an issue recognized as a spam' do + expect(Gitlab::Recaptcha).to receive(:load_configurations!).and_return(true) expect { update_spam_issue }.not_to change{ issue.reload.title } end @@ -619,14 +626,18 @@ describe Projects::IssuesController do end context 'when an issue is identified as spam' do - before { allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) } + before do + allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) + end context 'when captcha is not verified' do def post_spam_issue post_new_issue(title: 'Spam Title', description: 'Spam lives here') end - before { allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) } + before do + allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false) + end it 'rejects an issue recognized as a spam' do expect { post_spam_issue }.not_to change(Issue, :count) @@ -738,7 +749,10 @@ describe Projects::IssuesController do describe "DELETE #destroy" do context "when the user is a developer" do - before { sign_in(user) } + before do + sign_in(user) + end + it "rejects a developer to destroy an issue" do delete :destroy, namespace_id: project.namespace, project_id: project, id: issue.iid expect(response).to have_http_status(404) @@ -750,7 +764,9 @@ describe Projects::IssuesController do let(:namespace) { create(:namespace, owner: owner) } let(:project) { create(:empty_project, namespace: namespace) } - before { sign_in(owner) } + before do + sign_in(owner) + end it "deletes the issue" do delete :destroy, namespace_id: project.namespace, project_id: project, id: issue.iid diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 7211acc53dc..472e5fc51a0 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -28,7 +28,7 @@ describe Projects::JobsController do get_index(scope: 'running') end - it 'has only running builds' do + it 'has only running jobs' do expect(response).to have_http_status(:ok) expect(assigns(:builds).first.status).to eq('running') end @@ -41,7 +41,7 @@ describe Projects::JobsController do get_index(scope: 'finished') end - it 'has only finished builds' do + it 'has only finished jobs' do expect(response).to have_http_status(:ok) expect(assigns(:builds).first.status).to eq('success') end @@ -67,23 +67,16 @@ describe Projects::JobsController do context 'number of queries' do before do Ci::Build::AVAILABLE_STATUSES.each do |status| - create_build(status, status) + create_job(status, status) end - - RequestStore.begin! - end - - after do - RequestStore.end! - RequestStore.clear! end - it "verifies number of queries" do + it 'verifies number of queries', :request_store do recorded = ActiveRecord::QueryRecorder.new { get_index } - expect(recorded.count).to be_within(5).of(8) + expect(recorded.count).to be_within(5).of(7) end - def create_build(name, status) + def create_job(name, status) pipeline = create(:ci_pipeline, project: project) create(:ci_build, :tags, :triggered, :artifacts, pipeline: pipeline, name: name, status: status) @@ -101,21 +94,21 @@ describe Projects::JobsController do end describe 'GET show' do - let!(:build) { create(:ci_build, :failed, pipeline: pipeline) } + let!(:job) { create(:ci_build, :failed, pipeline: pipeline) } context 'when requesting HTML' do - context 'when build exists' do + context 'when job exists' do before do - get_show(id: build.id) + get_show(id: job.id) end - it 'has a build' do + it 'has a job' do expect(response).to have_http_status(:ok) - expect(assigns(:build).id).to eq(build.id) + expect(assigns(:build).id).to eq(job.id) end end - context 'when build does not exist' do + context 'when job does not exist' do before do get_show(id: 1234) end @@ -135,12 +128,12 @@ describe Projects::JobsController do allow_any_instance_of(Ci::Build).to receive(:merge_request).and_return(merge_request) - get_show(id: build.id, format: :json) + get_show(id: job.id, format: :json) end it 'exposes needed information' do expect(response).to have_http_status(:ok) - expect(json_response['raw_path']).to match(/builds\/\d+\/raw\z/) + expect(json_response['raw_path']).to match(/jobs\/\d+\/raw\z/) expect(json_response.dig('merge_request', 'path')).to match(/merge_requests\/\d+\z/) expect(json_response['new_issue_path']) .to include('/issues/new') @@ -162,35 +155,35 @@ describe Projects::JobsController do get_trace end - context 'when build has a trace' do - let(:build) { create(:ci_build, :trace, pipeline: pipeline) } + context 'when job has a trace' do + let(:job) { create(:ci_build, :trace, pipeline: pipeline) } it 'returns a trace' do expect(response).to have_http_status(:ok) - expect(json_response['id']).to eq build.id - expect(json_response['status']).to eq build.status + expect(json_response['id']).to eq job.id + expect(json_response['status']).to eq job.status expect(json_response['html']).to eq('BUILD TRACE') end end - context 'when build has no traces' do - let(:build) { create(:ci_build, pipeline: pipeline) } + context 'when job has no traces' do + let(:job) { create(:ci_build, pipeline: pipeline) } it 'returns no traces' do expect(response).to have_http_status(:ok) - expect(json_response['id']).to eq build.id - expect(json_response['status']).to eq build.status + expect(json_response['id']).to eq job.id + expect(json_response['status']).to eq job.status expect(json_response['html']).to be_nil end end - context 'when build has a trace with ANSI sequence and Unicode' do - let(:build) { create(:ci_build, :unicode_trace, pipeline: pipeline) } + context 'when job has a trace with ANSI sequence and Unicode' do + let(:job) { create(:ci_build, :unicode_trace, pipeline: pipeline) } it 'returns a trace with Unicode' do expect(response).to have_http_status(:ok) - expect(json_response['id']).to eq build.id - expect(json_response['status']).to eq build.status + expect(json_response['id']).to eq job.id + expect(json_response['status']).to eq job.status expect(json_response['html']).to include("ヾ(´༎ຶД༎ຶ`)ノ") end end @@ -198,23 +191,23 @@ describe Projects::JobsController do def get_trace get :trace, namespace_id: project.namespace, project_id: project, - id: build.id, + id: job.id, format: :json end end describe 'GET status.json' do - let(:build) { create(:ci_build, pipeline: pipeline) } - let(:status) { build.detailed_status(double('user')) } + let(:job) { create(:ci_build, pipeline: pipeline) } + let(:status) { job.detailed_status(double('user')) } before do get :status, namespace_id: project.namespace, project_id: project, - id: build.id, + id: job.id, format: :json end - it 'return a detailed build status in json' do + it 'return a detailed job status in json' do expect(response).to have_http_status(:ok) expect(json_response['text']).to eq status.text expect(json_response['label']).to eq status.label @@ -231,17 +224,17 @@ describe Projects::JobsController do post_retry end - context 'when build is retryable' do - let(:build) { create(:ci_build, :retryable, pipeline: pipeline) } + context 'when job is retryable' do + let(:job) { create(:ci_build, :retryable, pipeline: pipeline) } - it 'redirects to the retried build page' do + it 'redirects to the retried job page' do expect(response).to have_http_status(:found) expect(response).to redirect_to(namespace_project_job_path(id: Ci::Build.last.id)) end end - context 'when build is not retryable' do - let(:build) { create(:ci_build, pipeline: pipeline) } + context 'when job is not retryable' do + let(:job) { create(:ci_build, pipeline: pipeline) } it 'renders unprocessable_entity' do expect(response).to have_http_status(:unprocessable_entity) @@ -251,7 +244,7 @@ describe Projects::JobsController do def post_retry post :retry, namespace_id: project.namespace, project_id: project, - id: build.id + id: job.id end end @@ -267,21 +260,21 @@ describe Projects::JobsController do post_play end - context 'when build is playable' do - let(:build) { create(:ci_build, :playable, pipeline: pipeline) } + context 'when job is playable' do + let(:job) { create(:ci_build, :playable, pipeline: pipeline) } - it 'redirects to the played build page' do + it 'redirects to the played job page' do expect(response).to have_http_status(:found) - expect(response).to redirect_to(namespace_project_job_path(id: build.id)) + expect(response).to redirect_to(namespace_project_job_path(id: job.id)) end it 'transits to pending' do - expect(build.reload).to be_pending + expect(job.reload).to be_pending end end - context 'when build is not playable' do - let(:build) { create(:ci_build, pipeline: pipeline) } + context 'when job is not playable' do + let(:job) { create(:ci_build, pipeline: pipeline) } it 'renders unprocessable_entity' do expect(response).to have_http_status(:unprocessable_entity) @@ -291,7 +284,7 @@ describe Projects::JobsController do def post_play post :play, namespace_id: project.namespace, project_id: project, - id: build.id + id: job.id end end @@ -303,21 +296,21 @@ describe Projects::JobsController do post_cancel end - context 'when build is cancelable' do - let(:build) { create(:ci_build, :cancelable, pipeline: pipeline) } + context 'when job is cancelable' do + let(:job) { create(:ci_build, :cancelable, pipeline: pipeline) } - it 'redirects to the canceled build page' do + it 'redirects to the canceled job page' do expect(response).to have_http_status(:found) - expect(response).to redirect_to(namespace_project_job_path(id: build.id)) + expect(response).to redirect_to(namespace_project_job_path(id: job.id)) end it 'transits to canceled' do - expect(build.reload).to be_canceled + expect(job.reload).to be_canceled end end - context 'when build is not cancelable' do - let(:build) { create(:ci_build, :canceled, pipeline: pipeline) } + context 'when job is not cancelable' do + let(:job) { create(:ci_build, :canceled, pipeline: pipeline) } it 'returns unprocessable_entity' do expect(response).to have_http_status(:unprocessable_entity) @@ -327,7 +320,7 @@ describe Projects::JobsController do def post_cancel post :cancel, namespace_id: project.namespace, project_id: project, - id: build.id + id: job.id end end @@ -337,7 +330,7 @@ describe Projects::JobsController do sign_in(user) end - context 'when builds are cancelable' do + context 'when jobs are cancelable' do before do create_list(:ci_build, 2, :cancelable, pipeline: pipeline) @@ -354,7 +347,7 @@ describe Projects::JobsController do end end - context 'when builds are not cancelable' do + context 'when jobs are not cancelable' do before do create_list(:ci_build, 2, :canceled, pipeline: pipeline) @@ -381,26 +374,26 @@ describe Projects::JobsController do post_erase end - context 'when build is erasable' do - let(:build) { create(:ci_build, :erasable, :trace, pipeline: pipeline) } + context 'when job is erasable' do + let(:job) { create(:ci_build, :erasable, :trace, pipeline: pipeline) } - it 'redirects to the erased build page' do + it 'redirects to the erased job page' do expect(response).to have_http_status(:found) - expect(response).to redirect_to(namespace_project_job_path(id: build.id)) + expect(response).to redirect_to(namespace_project_job_path(id: job.id)) end it 'erases artifacts' do - expect(build.artifacts_file.exists?).to be_falsey - expect(build.artifacts_metadata.exists?).to be_falsey + expect(job.artifacts_file.exists?).to be_falsey + expect(job.artifacts_metadata.exists?).to be_falsey end it 'erases trace' do - expect(build.trace.exist?).to be_falsey + expect(job.trace.exist?).to be_falsey end end - context 'when build is not erasable' do - let(:build) { create(:ci_build, :erased, pipeline: pipeline) } + context 'when job is not erasable' do + let(:job) { create(:ci_build, :erased, pipeline: pipeline) } it 'returns unprocessable_entity' do expect(response).to have_http_status(:unprocessable_entity) @@ -410,7 +403,7 @@ describe Projects::JobsController do def post_erase post :erase, namespace_id: project.namespace, project_id: project, - id: build.id + id: job.id end end @@ -419,8 +412,8 @@ describe Projects::JobsController do get_raw end - context 'when build has a trace file' do - let(:build) { create(:ci_build, :trace, pipeline: pipeline) } + context 'when job has a trace file' do + let(:job) { create(:ci_build, :trace, pipeline: pipeline) } it 'send a trace file' do expect(response).to have_http_status(:ok) @@ -429,8 +422,8 @@ describe Projects::JobsController do end end - context 'when build does not have a trace file' do - let(:build) { create(:ci_build, pipeline: pipeline) } + context 'when job does not have a trace file' do + let(:job) { create(:ci_build, pipeline: pipeline) } it 'returns not_found' do expect(response).to have_http_status(:not_found) @@ -440,7 +433,7 @@ describe Projects::JobsController do def get_raw post :raw, namespace_id: project.namespace, project_id: project, - id: build.id + id: job.id end end end diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index 130b0b744b5..bf1776eb320 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -117,7 +117,7 @@ describe Projects::LabelsController do let!(:promoted_label_name) { "Promoted Label" } let!(:label_1) { create(:label, title: promoted_label_name, project: project) } - context 'not group owner' do + context 'not group reporters' do it 'denies access' do post :promote, namespace_id: project.namespace.to_param, project_id: project, id: label_1.to_param @@ -125,9 +125,9 @@ describe Projects::LabelsController do end end - context 'group owner' do + context 'group reporter' do before do - GroupMember.add_users(group, [user], :owner) + group.add_reporter(user) end it 'gives access' do diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 08024a2148b..d8a3a510f97 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -19,7 +19,10 @@ describe Projects::MergeRequestsController do render_views let(:fork_project) { create(:forked_project_with_submodules) } - before { fork_project.team << [user, :master] } + + before do + fork_project.team << [user, :master] + end context 'when rendering HTML response' do it 'renders new merge request widget template' do @@ -119,14 +122,14 @@ describe Projects::MergeRequestsController do end end - context 'number of queries' do + context 'number of queries', :request_store do it 'verifies number of queries' do # pre-create objects merge_request recorded = ActiveRecord::QueryRecorder.new { go(format: :json) } - expect(recorded.count).to be_within(5).of(50) + expect(recorded.count).to be_within(5).of(30) expect(recorded.cached_count).to eq(0) end end @@ -328,7 +331,9 @@ describe Projects::MergeRequestsController do end context 'when the sha parameter does not match the source SHA' do - before { post :merge, base_params.merge(sha: 'foo') } + before do + post :merge, base_params.merge(sha: 'foo') + end it 'returns :sha_mismatch' do expect(json_response).to eq('status' => 'sha_mismatch') @@ -473,7 +478,9 @@ describe Projects::MergeRequestsController do let(:namespace) { create(:namespace, owner: owner) } let(:project) { create(:project, namespace: namespace) } - before { sign_in owner } + before do + sign_in owner + end it "deletes the merge request" do delete :destroy, namespace_id: project.namespace, project_id: project, id: merge_request.iid @@ -505,7 +512,9 @@ describe Projects::MergeRequestsController do context 'with default params' do context 'as html' do - before { go(format: 'html') } + before do + go(format: 'html') + end it 'renders the diff template' do expect(response).to render_template('diffs') @@ -513,7 +522,9 @@ describe Projects::MergeRequestsController do end context 'as json' do - before { go(format: 'json') } + before do + go(format: 'json') + end it 'renders the diffs template to a string' do expect(response).to render_template('projects/merge_requests/show/_diffs') @@ -544,7 +555,9 @@ describe Projects::MergeRequestsController do context 'with ignore_whitespace_change' do context 'as html' do - before { go(format: 'html', w: 1) } + before do + go(format: 'html', w: 1) + end it 'renders the diff template' do expect(response).to render_template('diffs') @@ -552,7 +565,9 @@ describe Projects::MergeRequestsController do end context 'as json' do - before { go(format: 'json', w: 1) } + before do + go(format: 'json', w: 1) + end it 'renders the diffs template to a string' do expect(response).to render_template('projects/merge_requests/show/_diffs') @@ -562,7 +577,9 @@ describe Projects::MergeRequestsController do end context 'with view' do - before { go(view: 'parallel') } + before do + go(view: 'parallel') + end it 'saves the preferred diff view in a cookie' do expect(response.cookies['diff_view']).to eq('parallel') @@ -605,7 +622,9 @@ describe Projects::MergeRequestsController do end context 'when the path does not exist in the diff' do - before { diff_for_path(id: merge_request.iid, old_path: 'files/ruby/nopen.rb', new_path: 'files/ruby/nopen.rb') } + before do + diff_for_path(id: merge_request.iid, old_path: 'files/ruby/nopen.rb', new_path: 'files/ruby/nopen.rb') + end it 'returns a 404' do expect(response).to have_http_status(404) @@ -626,7 +645,9 @@ describe Projects::MergeRequestsController do end context 'when the merge request does not exist' do - before { diff_for_path(id: merge_request.iid.succ, old_path: existing_path, new_path: existing_path) } + before do + diff_for_path(id: merge_request.iid.succ, old_path: existing_path, new_path: existing_path) + end it 'returns a 404' do expect(response).to have_http_status(404) @@ -670,7 +691,9 @@ describe Projects::MergeRequestsController do context 'when the source branch is in a different project to the target' do let(:other_project) { create(:project) } - before { other_project.team << [user, :master] } + before do + other_project.team << [user, :master] + end context 'when the path exists in the diff' do it 'disables diff notes' do @@ -690,7 +713,9 @@ describe Projects::MergeRequestsController do end context 'when the path does not exist in the diff' do - before { diff_for_path(old_path: 'files/ruby/nopen.rb', new_path: 'files/ruby/nopen.rb', merge_request: { source_project: other_project, source_branch: 'feature', target_branch: 'master' }) } + before do + diff_for_path(old_path: 'files/ruby/nopen.rb', new_path: 'files/ruby/nopen.rb', merge_request: { source_project: other_project, source_branch: 'feature', target_branch: 'master' }) + end it 'returns a 404' do expect(response).to have_http_status(404) @@ -913,7 +938,9 @@ describe Projects::MergeRequestsController do end context 'when the file does not exist cannot be resolved in the UI' do - before { conflict_for_path('files/ruby/regexp.rb') } + before do + conflict_for_path('files/ruby/regexp.rb') + end it 'returns a 404 status code' do expect(response).to have_http_status(:not_found) @@ -923,7 +950,9 @@ describe Projects::MergeRequestsController do context 'with an existing file' do let(:path) { 'files/ruby/regex.rb' } - before { conflict_for_path(path) } + before do + conflict_for_path(path) + end it 'returns a 200 status code' do expect(response).to have_http_status(:ok) @@ -1195,7 +1224,9 @@ describe Projects::MergeRequestsController do end context 'when head_pipeline does not exist' do - before { get_pipeline_status } + before do + get_pipeline_status + end it 'return empty' do expect(response).to have_http_status(:ok) diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index c880da1e36a..734532668d3 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -5,9 +5,12 @@ describe Projects::PipelinesController do let(:user) { create(:user) } let(:project) { create(:empty_project, :public) } + let(:feature) { ProjectFeature::DISABLED } before do project.add_developer(user) + project.project_feature.update( + builds_access_level: feature) sign_in(user) end @@ -49,21 +52,14 @@ describe Projects::PipelinesController do expect(json_response['details']).to have_key 'stages' end - context 'when the pipeline has multiple stages and groups' do + context 'when the pipeline has multiple stages and groups', :request_store do before do - RequestStore.begin! - create_build('build', 0, 'build') create_build('test', 1, 'rspec 0') create_build('deploy', 2, 'production') create_build('post deploy', 3, 'pages 0') end - after do - RequestStore.end! - RequestStore.clear! - end - let(:project) { create(:project) } let(:pipeline) do create(:ci_empty_pipeline, project: project, user: user, sha: project.commit.id) @@ -160,16 +156,26 @@ describe Projects::PipelinesController do format: :json end - it 'retries a pipeline without returning any content' do - expect(response).to have_http_status(:no_content) - expect(build.reload).to be_retried + context 'when builds are enabled' do + let(:feature) { ProjectFeature::ENABLED } + + it 'retries a pipeline without returning any content' do + expect(response).to have_http_status(:no_content) + expect(build.reload).to be_retried + end + end + + context 'when builds are disabled' do + it 'fails to retry pipeline' do + expect(response).to have_http_status(:not_found) + end end end describe 'POST cancel.json' do let!(:pipeline) { create(:ci_pipeline, project: project) } let!(:build) { create(:ci_build, :running, pipeline: pipeline) } - + before do post :cancel, namespace_id: project.namespace, project_id: project, @@ -177,9 +183,19 @@ describe Projects::PipelinesController do format: :json end - it 'cancels a pipeline without returning any content' do - expect(response).to have_http_status(:no_content) - expect(pipeline.reload).to be_canceled + context 'when builds are enabled' do + let(:feature) { ProjectFeature::ENABLED } + + it 'cancels a pipeline without returning any content' do + expect(response).to have_http_status(:no_content) + expect(pipeline.reload).to be_canceled + end + end + + context 'when builds are disabled' do + it 'fails to retry pipeline' do + expect(response).to have_http_status(:not_found) + end end end end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 2294d5df581..f2b59ba82ca 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -16,10 +16,14 @@ describe Projects::ProjectMembersController do describe 'POST create' do let(:project_user) { create(:user) } - before { sign_in(user) } + before do + sign_in(user) + end context 'when user does not have enough rights' do - before { project.team << [user, :developer] } + before do + project.team << [user, :developer] + end it 'returns 404' do post :create, namespace_id: project.namespace, @@ -33,7 +37,9 @@ describe Projects::ProjectMembersController do end context 'when user has enough rights' do - before { project.team << [user, :master] } + before do + project.team << [user, :master] + end it 'adds user to members' do expect_any_instance_of(Members::CreateService).to receive(:execute).and_return(status: :success) @@ -64,7 +70,9 @@ describe Projects::ProjectMembersController do describe 'DELETE destroy' do let(:member) { create(:project_member, :developer, project: project) } - before { sign_in(user) } + before do + sign_in(user) + end context 'when member is not found' do it 'returns 404' do @@ -78,7 +86,9 @@ describe Projects::ProjectMembersController do context 'when member is found' do context 'when user does not have enough rights' do - before { project.team << [user, :developer] } + before do + project.team << [user, :developer] + end it 'returns 404' do delete :destroy, namespace_id: project.namespace, @@ -91,7 +101,9 @@ describe Projects::ProjectMembersController do end context 'when user has enough rights' do - before { project.team << [user, :master] } + before do + project.team << [user, :master] + end it '[HTML] removes user from members' do delete :destroy, namespace_id: project.namespace, @@ -117,7 +129,9 @@ describe Projects::ProjectMembersController do end describe 'DELETE leave' do - before { sign_in(user) } + before do + sign_in(user) + end context 'when member is not found' do it 'returns 404' do @@ -130,7 +144,9 @@ describe Projects::ProjectMembersController do context 'when member is found' do context 'and is not an owner' do - before { project.team << [user, :developer] } + before do + project.team << [user, :developer] + end it 'removes user from members' do delete :leave, namespace_id: project.namespace, @@ -145,7 +161,9 @@ describe Projects::ProjectMembersController do context 'and is an owner' do let(:project) { create(:empty_project, namespace: user.namespace) } - before { project.team << [user, :master] } + before do + project.team << [user, :master] + end it 'cannot remove himself from the project' do delete :leave, namespace_id: project.namespace, @@ -156,7 +174,9 @@ describe Projects::ProjectMembersController do end context 'and is a requester' do - before { project.request_access(user) } + before do + project.request_access(user) + end it 'removes user from members' do delete :leave, namespace_id: project.namespace, @@ -172,7 +192,9 @@ describe Projects::ProjectMembersController do end describe 'POST request_access' do - before { sign_in(user) } + before do + sign_in(user) + end it 'creates a new ProjectMember that is not a team member' do post :request_access, namespace_id: project.namespace, @@ -190,7 +212,9 @@ describe Projects::ProjectMembersController do describe 'POST approve' do let(:member) { create(:project_member, :access_request, project: project) } - before { sign_in(user) } + before do + sign_in(user) + end context 'when member is not found' do it 'returns 404' do @@ -204,7 +228,9 @@ describe Projects::ProjectMembersController do context 'when member is found' do context 'when user does not have enough rights' do - before { project.team << [user, :developer] } + before do + project.team << [user, :developer] + end it 'returns 404' do post :approve_access_request, namespace_id: project.namespace, @@ -217,7 +243,9 @@ describe Projects::ProjectMembersController do end context 'when user has enough rights' do - before { project.team << [user, :master] } + before do + project.team << [user, :master] + end it 'adds user to members' do post :approve_access_request, namespace_id: project.namespace, @@ -252,7 +280,10 @@ describe Projects::ProjectMembersController do end context 'when user can access source project members' do - before { another_project.team << [user, :guest] } + before do + another_project.team << [user, :guest] + end + include_context 'import applied' it 'imports source project members' do diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index 24a59caff4e..2434f822c6f 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -46,7 +46,9 @@ describe Projects::SnippetsController do end context 'when signed in as the author' do - before { sign_in(user) } + before do + sign_in(user) + end it 'renders the snippet' do get :index, namespace_id: project.namespace, project_id: project @@ -57,7 +59,9 @@ describe Projects::SnippetsController do end context 'when signed in as a project member' do - before { sign_in(user2) } + before do + sign_in(user2) + end it 'renders the snippet' do get :index, namespace_id: project.namespace, project_id: project @@ -78,8 +82,18 @@ describe Projects::SnippetsController do post :create, { namespace_id: project.namespace.to_param, project_id: project, - project_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params) + project_snippet: { title: 'Title', content: 'Content', description: 'Description' }.merge(snippet_params) }.merge(additional_params) + + Snippet.last + end + + it 'creates the snippet correctly' do + snippet = create_snippet(project, visibility_level: Snippet::PRIVATE) + + expect(snippet.title).to eq('Title') + expect(snippet.content).to eq('Content') + expect(snippet.description).to eq('Description') end context 'when the snippet is spam' do @@ -307,7 +321,9 @@ describe Projects::SnippetsController do end context 'when signed in as the author' do - before { sign_in(user) } + before do + sign_in(user) + end it 'renders the snippet' do get action, namespace_id: project.namespace, project_id: project, id: project_snippet.to_param @@ -318,7 +334,9 @@ describe Projects::SnippetsController do end context 'when signed in as a project member' do - before { sign_in(user2) } + before do + sign_in(user2) + end it 'renders the snippet' do get action, namespace_id: project.namespace, project_id: project, id: project_snippet.to_param @@ -339,7 +357,9 @@ describe Projects::SnippetsController do end context 'when signed in' do - before { sign_in(user) } + before do + sign_in(user) + end it 'responds with status 404' do get action, namespace_id: project.namespace, project_id: project, id: 42 diff --git a/spec/controllers/projects/tags_controller_spec.rb b/spec/controllers/projects/tags_controller_spec.rb index fc97bac64cd..c48f41ca12e 100644 --- a/spec/controllers/projects/tags_controller_spec.rb +++ b/spec/controllers/projects/tags_controller_spec.rb @@ -6,7 +6,9 @@ describe Projects::TagsController do let!(:invalid_release) { create(:release, project: project, tag: 'does-not-exist') } describe 'GET index' do - before { get :index, namespace_id: project.namespace.to_param, project_id: project } + before do + get :index, namespace_id: project.namespace.to_param, project_id: project + end it 'returns the tags for the page' do expect(assigns(:tags).map(&:name)).to eq(['v1.1.0', 'v1.0.0']) @@ -19,7 +21,9 @@ describe Projects::TagsController do end describe 'GET show' do - before { get :show, namespace_id: project.namespace.to_param, project_id: project, id: id } + before do + get :show, namespace_id: project.namespace.to_param, project_id: project, id: id + end context "valid tag" do let(:id) { 'v1.0.0' } diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 4f6fc6691be..240a81367d0 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -29,7 +29,9 @@ describe ProjectsController do describe "GET show" do context "user not project member" do - before { sign_in(user) } + before do + sign_in(user) + end context "user does not have access to project" do let(:private_project) { create(:empty_project, :private) } @@ -108,7 +110,9 @@ describe ProjectsController do context "project with empty repo" do let(:empty_project) { create(:project_empty_repo, :public) } - before { sign_in(user) } + before do + sign_in(user) + end User.project_views.keys.each do |project_view| context "with #{project_view} view set" do @@ -128,7 +132,9 @@ describe ProjectsController do context "project with broken repo" do let(:empty_project) { create(:project_broken_repo, :public) } - before { sign_in(user) } + before do + sign_in(user) + end User.project_views.keys.each do |project_view| context "with #{project_view} view set" do diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 3173aae664c..a3708ad0908 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -18,7 +18,9 @@ describe SearchController do context 'on restricted projects' do context 'when signed out' do - before { sign_out(user) } + before do + sign_out(user) + end it "doesn't expose comments on issues" do project = create(:empty_project, :public, :issues_private) diff --git a/spec/controllers/sent_notifications_controller_spec.rb b/spec/controllers/sent_notifications_controller_spec.rb index 954fc2eaf21..0cc8a3b68eb 100644 --- a/spec/controllers/sent_notifications_controller_spec.rb +++ b/spec/controllers/sent_notifications_controller_spec.rb @@ -14,7 +14,9 @@ describe SentNotificationsController, type: :controller do describe 'GET unsubscribe' do context 'when the user is not logged in' do context 'when the force param is passed' do - before { get(:unsubscribe, id: sent_notification.reply_key, force: true) } + before do + get(:unsubscribe, id: sent_notification.reply_key, force: true) + end it 'unsubscribes the user' do expect(issue.subscribed?(user, project)).to be_falsey @@ -30,7 +32,9 @@ describe SentNotificationsController, type: :controller do end context 'when the force param is not passed' do - before { get(:unsubscribe, id: sent_notification.reply_key) } + before do + get(:unsubscribe, id: sent_notification.reply_key) + end it 'does not unsubscribe the user' do expect(issue.subscribed?(user, project)).to be_truthy @@ -47,10 +51,14 @@ describe SentNotificationsController, type: :controller do end context 'when the user is logged in' do - before { sign_in(user) } + before do + sign_in(user) + end context 'when the ID passed does not exist' do - before { get(:unsubscribe, id: sent_notification.reply_key.reverse) } + before do + get(:unsubscribe, id: sent_notification.reply_key.reverse) + end it 'does not unsubscribe the user' do expect(issue.subscribed?(user, project)).to be_truthy @@ -66,7 +74,9 @@ describe SentNotificationsController, type: :controller do end context 'when the force param is passed' do - before { get(:unsubscribe, id: sent_notification.reply_key, force: true) } + before do + get(:unsubscribe, id: sent_notification.reply_key, force: true) + end it 'unsubscribes the user' do expect(issue.subscribed?(user, project)).to be_falsey @@ -89,7 +99,10 @@ describe SentNotificationsController, type: :controller do end end let(:sent_notification) { create(:sent_notification, project: project, noteable: merge_request, recipient: user) } - before { get(:unsubscribe, id: sent_notification.reply_key) } + + before do + get(:unsubscribe, id: sent_notification.reply_key) + end it 'unsubscribes the user' do expect(merge_request.subscribed?(user, project)).to be_falsey diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index e87e24a33a1..03f4b0ba343 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -142,7 +142,9 @@ describe SessionsController do end context 'when OTP is invalid' do - before { authenticate_2fa(otp_attempt: 'invalid') } + before do + authenticate_2fa(otp_attempt: 'invalid') + end it 'does not authenticate' do expect(subject.current_user).not_to eq user @@ -169,7 +171,9 @@ describe SessionsController do end context 'when OTP is invalid' do - before { authenticate_2fa(otp_attempt: 'invalid') } + before do + authenticate_2fa(otp_attempt: 'invalid') + end it 'does not authenticate' do expect(subject.current_user).not_to eq user diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 930415a4778..b1339b2a185 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -171,12 +171,50 @@ describe SnippetsController do sign_in(user) post :create, { - personal_snippet: { title: 'Title', content: 'Content' }.merge(snippet_params) + personal_snippet: { title: 'Title', content: 'Content', description: 'Description' }.merge(snippet_params) }.merge(additional_params) Snippet.last end + it 'creates the snippet correctly' do + snippet = create_snippet(visibility_level: Snippet::PRIVATE) + + expect(snippet.title).to eq('Title') + expect(snippet.content).to eq('Content') + expect(snippet.description).to eq('Description') + end + + context 'when the snippet description contains a file' do + let(:picture_file) { '/temp/secret56/picture.jpg' } + let(:text_file) { '/temp/secret78/text.txt' } + let(:description) do + "Description with picture: ![picture](/uploads#{picture_file}) and "\ + "text: [text.txt](/uploads#{text_file})" + end + + before do + allow(FileUtils).to receive(:mkdir_p) + allow(FileUtils).to receive(:move) + end + + subject { create_snippet({ description: description }, { files: [picture_file, text_file] }) } + + it 'creates the snippet' do + expect { subject }.to change { Snippet.count }.by(1) + end + + it 'stores the snippet description correctly' do + snippet = subject + + expected_description = "Description with picture: "\ + "![picture](/uploads/personal_snippet/#{snippet.id}/secret56/picture.jpg) and "\ + "text: [text.txt](/uploads/personal_snippet/#{snippet.id}/secret78/text.txt)" + + expect(snippet.description).to eq(expected_description) + end + end + context 'when the snippet is spam' do before do allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true) @@ -399,7 +437,9 @@ describe SnippetsController do end context 'when signed in user is the author' do - before { get :raw, id: personal_snippet.to_param } + before do + get :raw, id: personal_snippet.to_param + end it 'responds with status 200' do expect(assigns(:snippet)).to eq(personal_snippet) diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index 8000c9dec61..01a0659479b 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -92,6 +92,40 @@ describe UploadsController do end end end + + context 'temporal with valid image' do + subject do + post :create, model: 'personal_snippet', file: jpg, format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + subject + + expect(response.body).to match '\"alt\":\"rails_sample\"' + expect(response.body).to match "\"url\":\"/uploads/temp" + end + + it 'does not create an Upload record' do + expect { subject }.not_to change { Upload.count } + end + end + + context 'temporal with valid non-image file' do + subject do + post :create, model: 'personal_snippet', file: txt, format: :json + end + + it 'returns a content with original filename, new link, and correct type.' do + subject + + expect(response.body).to match '\"alt\":\"doc_sample.txt\"' + expect(response.body).to match "\"url\":\"/uploads/temp" + end + + it 'does not create an Upload record' do + expect { subject }.not_to change { Upload.count } + end + end end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index d33e2ba1e53..842d82cdbe9 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -43,7 +43,9 @@ describe UsersController do end context 'when logged in' do - before { sign_in(user) } + before do + sign_in(user) + end it 'renders show' do get :show, username: user.username @@ -62,7 +64,9 @@ describe UsersController do end context 'when logged in' do - before { sign_in(user) } + before do + sign_in(user) + end it 'renders 404' do get :show, username: 'nonexistent' |