diff options
Diffstat (limited to 'spec/controllers')
45 files changed, 943 insertions, 273 deletions
diff --git a/spec/controllers/admin/application_settings_controller_spec.rb b/spec/controllers/admin/application_settings_controller_spec.rb index 5ad5f9cdeea..4eb0545eb6c 100644 --- a/spec/controllers/admin/application_settings_controller_spec.rb +++ b/spec/controllers/admin/application_settings_controller_spec.rb @@ -41,7 +41,7 @@ describe Admin::ApplicationSettingsController do it 'returns JSON data' do get :usage_data, format: :json - body = JSON.parse(response.body) + body = json_response expect(body["version"]).to eq(Gitlab::VERSION) expect(body).to include('counts') expect(response.status).to eq(200) diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb index 509d8944e3a..1123563c1e3 100644 --- a/spec/controllers/admin/groups_controller_spec.rb +++ b/spec/controllers/admin/groups_controller_spec.rb @@ -68,5 +68,13 @@ describe Admin::GroupsController do post :update, params: { id: group.to_param, group: { project_creation_level: ::Gitlab::Access::NO_ONE_PROJECT_ACCESS } } end.to change { group.reload.project_creation_level }.to(::Gitlab::Access::NO_ONE_PROJECT_ACCESS) end + + it 'updates the subgroup_creation_level successfully' do + expect do + post :update, + params: { id: group.to_param, + group: { subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS } } + end.to change { group.reload.subgroup_creation_level }.to(::Gitlab::Access::OWNER_SUBGROUP_ACCESS) + end end end diff --git a/spec/controllers/admin/requests_profiles_controller_spec.rb b/spec/controllers/admin/requests_profiles_controller_spec.rb index 10850cb4603..345f7720c25 100644 --- a/spec/controllers/admin/requests_profiles_controller_spec.rb +++ b/spec/controllers/admin/requests_profiles_controller_spec.rb @@ -10,38 +10,63 @@ describe Admin::RequestsProfilesController do end describe '#show' do - let(:basename) { "profile_#{Time.now.to_i}.html" } let(:tmpdir) { Dir.mktmpdir('profiler-test') } let(:test_file) { File.join(tmpdir, basename) } - let(:profile) { Gitlab::RequestProfiler::Profile.new(basename) } - let(:sample_data) do - <<~HTML - <!DOCTYPE html> - <html> - <body> - <h1>My First Heading</h1> - <p>My first paragraph.</p> - </body> - </html> - HTML + + subject do + get :show, params: { name: basename } end before do stub_const('Gitlab::RequestProfiler::PROFILES_DIR', tmpdir) - output = File.open(test_file, 'w') - output.write(sample_data) - output.close + File.write(test_file, sample_data) end after do - File.unlink(test_file) + FileUtils.rm_rf(tmpdir) end - it 'loads an HTML profile' do - get :show, params: { name: basename } + context 'when loading HTML profile' do + let(:basename) { "profile_#{Time.now.to_i}_execution.html" } + + let(:sample_data) do + '<html> <body> <h1>Heading</h1> <p>paragraph.</p> </body> </html>' + end + + it 'renders the data' do + subject + + expect(response).to have_gitlab_http_status(200) + expect(response.body).to eq(sample_data) + end + end + + context 'when loading TXT profile' do + let(:basename) { "profile_#{Time.now.to_i}_memory.txt" } + + let(:sample_data) do + <<~TXT + Total allocated: 112096396 bytes (1080431 objects) + Total retained: 10312598 bytes (53567 objects) + TXT + end + + it 'renders the data' do + subject + + expect(response).to have_gitlab_http_status(200) + expect(response.body).to eq(sample_data) + end + end + + context 'when loading PDF profile' do + let(:basename) { "profile_#{Time.now.to_i}_anything.pdf" } + + let(:sample_data) { 'mocked pdf content' } - expect(response).to have_gitlab_http_status(200) - expect(response.body).to eq(sample_data) + it 'fails to render the data' do + expect { subject }.to raise_error(ActionController::UrlGenerationError, /No route matches.*unmatched constraints:/) + end end end end diff --git a/spec/controllers/admin/runners_controller_spec.rb b/spec/controllers/admin/runners_controller_spec.rb index 78c5e2a2656..bbeda7dae0f 100644 --- a/spec/controllers/admin/runners_controller_spec.rb +++ b/spec/controllers/admin/runners_controller_spec.rb @@ -23,10 +23,11 @@ describe Admin::RunnersController do control_count = ActiveRecord::QueryRecorder.new { get :index }.count - create(:ci_runner, :tagged_only) + create_list(:ci_runner, 5, :tagged_only) # There is still an N+1 query for `runner.builds.count` - expect { get :index }.not_to exceed_query_limit(control_count + 1) + # We also need to add 1 because it takes 2 queries to preload tags + expect { get :index }.not_to exceed_query_limit(control_count + 6) expect(response).to have_gitlab_http_status(200) expect(response.body).to have_content('tag1') diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index 89a0eba66f7..d7428f8b52c 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -279,6 +279,12 @@ describe Admin::UsersController do expect(warden.user).to eq(user) end + it 'logs the beginning of the impersonation event' do + expect(Gitlab::AppLogger).to receive(:info).with("User #{admin.username} has started impersonating #{user.username}").and_call_original + + post :impersonate, params: { id: user.username } + end + it "redirects to root" do post :impersonate, params: { id: user.username } diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 447a12b2fac..84bbbac39b0 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -63,8 +63,6 @@ describe ApplicationController do sign_in user end - let(:json_response) { JSON.parse(response.body) } - controller(described_class) do def index render json: Gon.all_variables diff --git a/spec/controllers/autocomplete_controller_spec.rb b/spec/controllers/autocomplete_controller_spec.rb index 3f1c0ae8ac4..eaa5d6cd073 100644 --- a/spec/controllers/autocomplete_controller_spec.rb +++ b/spec/controllers/autocomplete_controller_spec.rb @@ -295,28 +295,6 @@ describe AutocompleteController do end end - context 'authorized projects with offset' do - before do - authorized_project2 = create(:project) - authorized_project3 = create(:project) - - authorized_project.add_maintainer(user) - authorized_project2.add_maintainer(user) - authorized_project3.add_maintainer(user) - end - - describe 'GET #projects with project ID and offset_id' do - before do - get(:projects, params: { project_id: project.id, offset_id: authorized_project.id }) - end - - it 'returns projects' do - expect(json_response).to be_kind_of(Array) - expect(json_response.size).to eq 2 # Of a total of 3 - end - end - end - context 'authorized projects without admin_issue ability' do before do authorized_project.add_guest(user) diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index c84bb913cad..0db58fbefc1 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -52,10 +52,8 @@ describe Boards::IssuesController do list_issues user: user, board: board, list: list2 - parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('entities/issue_boards') - expect(parsed_response['issues'].length).to eq 2 + expect(json_response['issues'].length).to eq 2 expect(development.issues.map(&:relative_position)).not_to include(nil) end @@ -123,10 +121,8 @@ describe Boards::IssuesController do list_issues user: user, board: board - parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('entities/issue_boards') - expect(parsed_response['issues'].length).to eq 2 + expect(json_response['issues'].length).to eq 2 end end @@ -164,6 +160,215 @@ describe Boards::IssuesController do end end + describe 'PUT bulk_move' do + let(:todo) { create(:group_label, group: group, name: 'Todo') } + let(:development) { create(:group_label, group: group, name: 'Development') } + let(:user) { create(:group_member, :maintainer, user: create(:user), group: group ).user } + let(:guest) { create(:group_member, :guest, user: create(:user), group: group ).user } + let(:project) { create(:project, group: group) } + let(:group) { create(:group) } + let(:board) { create(:board, project: project) } + let(:list1) { create(:list, board: board, label: todo, position: 0) } + let(:list2) { create(:list, board: board, label: development, position: 1) } + let(:issue1) { create(:labeled_issue, project: project, labels: [todo], author: user, relative_position: 10) } + let(:issue2) { create(:labeled_issue, project: project, labels: [todo], author: user, relative_position: 20) } + let(:issue3) { create(:labeled_issue, project: project, labels: [todo], author: user, relative_position: 30) } + let(:issue4) { create(:labeled_issue, project: project, labels: [development], author: user, relative_position: 100) } + + let(:move_params) do + { + board_id: board.id, + ids: [issue1.id, issue2.id, issue3.id], + from_list_id: list1.id, + to_list_id: list2.id, + move_before_id: issue4.id, + move_after_id: nil + } + end + + before do + project.add_maintainer(user) + project.add_guest(guest) + end + + shared_examples 'move issues endpoint provider' do + before do + sign_in(signed_in_user) + end + + it 'responds as expected' do + put :bulk_move, params: move_issues_params + expect(response).to have_gitlab_http_status(expected_status) + + if expected_status == 200 + expect(json_response).to include( + 'count' => move_issues_params[:ids].size, + 'success' => true + ) + + expect(json_response['issues'].pluck('id')).to match_array(move_issues_params[:ids]) + end + end + + it 'moves issues as expected' do + put :bulk_move, params: move_issues_params + expect(response).to have_gitlab_http_status(expected_status) + + list_issues user: requesting_user, board: board, list: list2 + expect(response).to have_gitlab_http_status(200) + + expect(response).to match_response_schema('entities/issue_boards') + + responded_issues = json_response['issues'] + expect(responded_issues.length).to eq expected_issue_count + + ids_in_order = responded_issues.pluck('id') + expect(ids_in_order).to eq(expected_issue_ids_in_order) + end + end + + context 'when items are moved to another list' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) { move_params } + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 4 } + let(:expected_issue_ids_in_order) { [issue4.id, issue1.id, issue2.id, issue3.id] } + end + end + + context 'when moving just one issue' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:ids] = [issue2.id] + end + end + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 2 } + let(:expected_issue_ids_in_order) { [issue4.id, issue2.id] } + end + end + + context 'when user is not allowed to move issue' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { guest } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:ids] = [issue2.id] + end + end + let(:requesting_user) { user } + let(:expected_status) { 403 } + let(:expected_issue_count) { 1 } + let(:expected_issue_ids_in_order) { [issue4.id] } + end + end + + context 'when issues should be moved visually above existing issue in list' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:move_after_id] = issue4.id + hash[:move_before_id] = nil + end + end + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 4 } + let(:expected_issue_ids_in_order) { [issue1.id, issue2.id, issue3.id, issue4.id] } + end + end + + context 'when destination list is empty' do + before do + # Remove issue from list + issue4.labels -= [development] + issue4.save! + end + + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:move_before_id] = nil + end + end + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 3 } + let(:expected_issue_ids_in_order) { [issue1.id, issue2.id, issue3.id] } + end + end + + context 'when no position arguments are given' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:move_before_id] = nil + end + end + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 4 } + let(:expected_issue_ids_in_order) { [issue1.id, issue2.id, issue3.id, issue4.id] } + end + end + + context 'when move_before_id and move_after_id are given' do + let(:issue5) { create(:labeled_issue, project: project, labels: [development], author: user, relative_position: 90) } + + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:move_before_id] = issue5.id + hash[:move_after_id] = issue4.id + end + end + let(:requesting_user) { user } + let(:expected_status) { 200 } + let(:expected_issue_count) { 5 } + let(:expected_issue_ids_in_order) { [issue5.id, issue1.id, issue2.id, issue3.id, issue4.id] } + end + end + + context 'when request contains too many issues' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:ids] = (0..51).to_a + end + end + let(:requesting_user) { user } + let(:expected_status) { 422 } + let(:expected_issue_count) { 1 } + let(:expected_issue_ids_in_order) { [issue4.id] } + end + end + + context 'when request is malformed' do + it_behaves_like 'move issues endpoint provider' do + let(:signed_in_user) { user } + let(:move_issues_params) do + move_params.dup.tap do |hash| + hash[:ids] = 'foobar' + end + end + let(:requesting_user) { user } + let(:expected_status) { 400 } + let(:expected_issue_count) { 1 } + let(:expected_issue_ids_in_order) { [issue4.id] } + end + end + end + def list_issues(user:, board:, list: nil) sign_in(user) diff --git a/spec/controllers/boards/lists_controller_spec.rb b/spec/controllers/boards/lists_controller_spec.rb index e1f75fa3395..418ca6f3210 100644 --- a/spec/controllers/boards/lists_controller_spec.rb +++ b/spec/controllers/boards/lists_controller_spec.rb @@ -26,10 +26,8 @@ describe Boards::ListsController do read_board_list user: user, board: board - parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('lists') - expect(parsed_response.length).to eq 3 + expect(json_response.length).to eq 3 end context 'with unauthorized user' do diff --git a/spec/controllers/chaos_controller_spec.rb b/spec/controllers/chaos_controller_spec.rb new file mode 100644 index 00000000000..bafd4a70862 --- /dev/null +++ b/spec/controllers/chaos_controller_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ChaosController do + describe '#leakmem' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:leak_mem).with(100, 30.seconds) + + get :leakmem + + expect(response).to have_gitlab_http_status(200) + end + + it 'call synchronously with params' do + expect(Gitlab::Chaos).to receive(:leak_mem).with(1, 2.seconds) + + get :leakmem, params: { memory_mb: 1, duration_s: 2 } + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls asynchronously' do + expect(Chaos::LeakMemWorker).to receive(:perform_async).with(100, 30.seconds) + + get :leakmem, params: { async: 1 } + + expect(response).to have_gitlab_http_status(200) + end + end + + describe '#cpu_spin' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:cpu_spin).with(30.seconds) + + get :cpu_spin + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls synchronously with params' do + expect(Gitlab::Chaos).to receive(:cpu_spin).with(3.seconds) + + get :cpu_spin, params: { duration_s: 3 } + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls asynchronously' do + expect(Chaos::CpuSpinWorker).to receive(:perform_async).with(30.seconds) + + get :cpu_spin, params: { async: 1 } + + expect(response).to have_gitlab_http_status(200) + end + end + + describe '#db_spin' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:db_spin).with(30.seconds, 1.second) + + get :db_spin + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls synchronously with params' do + expect(Gitlab::Chaos).to receive(:db_spin).with(4.seconds, 5.seconds) + + get :db_spin, params: { duration_s: 4, interval_s: 5 } + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls asynchronously' do + expect(Chaos::DbSpinWorker).to receive(:perform_async).with(30.seconds, 1.second) + + get :db_spin, params: { async: 1 } + + expect(response).to have_gitlab_http_status(200) + end + end + + describe '#sleep' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:sleep).with(30.seconds) + + get :sleep + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls synchronously with params' do + expect(Gitlab::Chaos).to receive(:sleep).with(5.seconds) + + get :sleep, params: { duration_s: 5 } + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls asynchronously' do + expect(Chaos::SleepWorker).to receive(:perform_async).with(30.seconds) + + get :sleep, params: { async: 1 } + + expect(response).to have_gitlab_http_status(200) + end + end + + describe '#kill' do + it 'calls synchronously' do + expect(Gitlab::Chaos).to receive(:kill).with(no_args) + + get :kill + + expect(response).to have_gitlab_http_status(200) + end + + it 'calls asynchronously' do + expect(Chaos::KillWorker).to receive(:perform_async).with(no_args) + + get :kill, params: { async: 1 } + + expect(response).to have_gitlab_http_status(200) + end + end +end diff --git a/spec/controllers/dashboard/milestones_controller_spec.rb b/spec/controllers/dashboard/milestones_controller_spec.rb index 4de537ae6f8..67939aa4e6a 100644 --- a/spec/controllers/dashboard/milestones_controller_spec.rb +++ b/spec/controllers/dashboard/milestones_controller_spec.rb @@ -47,6 +47,8 @@ describe Dashboard::MilestonesController do describe "#index" do let(:public_group) { create(:group, :public) } let!(:public_milestone) { create(:milestone, group: public_group) } + let!(:closed_group_milestone) { create(:milestone, group: group, state: 'closed') } + let!(:closed_project_milestone) { create(:milestone, project: project, state: 'closed') } render_views @@ -59,6 +61,15 @@ describe Dashboard::MilestonesController do expect(json_response.map { |i| i["group_name"] }.compact).to match_array(group.name) end + it 'returns closed group and project milestones to which the user belongs' do + get :index, params: { state: 'closed' }, format: :json + + expect(response).to have_gitlab_http_status(200) + expect(json_response.size).to eq(2) + expect(json_response.map { |i| i["name"] }).to match_array([closed_group_milestone.name, closed_project_milestone.name]) + expect(json_response.map { |i| i["group_name"] }.compact).to match_array(group.name) + end + it 'searches legacy project milestones by title when search_title is given' do project_milestone = create(:milestone, title: 'Project milestone title', project: project) @@ -77,11 +88,11 @@ describe Dashboard::MilestonesController do expect(response.body).not_to include(project_milestone.title) end - it 'shows counts of group and project milestones to which the user belongs to' do + it 'shows counts of open and closed group and project milestones to which the user belongs to' do get :index expect(response.body).to include("Open\n<span class=\"badge badge-pill\">2</span>") - expect(response.body).to include("Closed\n<span class=\"badge badge-pill\">0</span>") + expect(response.body).to include("Closed\n<span class=\"badge badge-pill\">2</span>") end context 'external authorization' do diff --git a/spec/controllers/dashboard/projects_controller_spec.rb b/spec/controllers/dashboard/projects_controller_spec.rb index ea68eae12ed..6591901a9dc 100644 --- a/spec/controllers/dashboard/projects_controller_spec.rb +++ b/spec/controllers/dashboard/projects_controller_spec.rb @@ -11,8 +11,10 @@ describe Dashboard::ProjectsController do end context 'user logged in' do + let(:user) { create(:user) } + before do - sign_in create(:user) + sign_in(user) end context 'external authorization' do @@ -24,6 +26,20 @@ describe Dashboard::ProjectsController do expect(response).to have_gitlab_http_status(200) end end + + it 'orders the projects by last activity by default' do + project = create(:project) + project.add_developer(user) + project.update!(last_repository_updated_at: 3.days.ago, last_activity_at: 3.days.ago) + + project2 = create(:project) + project2.add_developer(user) + project2.update!(last_repository_updated_at: 10.days.ago, last_activity_at: 10.days.ago) + + get :index + + expect(assigns(:projects)).to eq([project, project2]) + end end end diff --git a/spec/controllers/groups/boards_controller_spec.rb b/spec/controllers/groups/boards_controller_spec.rb index 5e0f64ccca4..e4232c2c1ab 100644 --- a/spec/controllers/groups/boards_controller_spec.rb +++ b/spec/controllers/groups/boards_controller_spec.rb @@ -63,10 +63,8 @@ describe Groups::BoardsController do list_boards format: :json - parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('boards') - expect(parsed_response.length).to eq 1 + expect(json_response.length).to eq 1 end context 'with unauthorized user' do diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index 19b18091aef..bf164aeed38 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -73,7 +73,7 @@ describe Groups::MilestonesController do it 'lists legacy group milestones and group milestones' do get :index, params: { group_id: group.to_param }, format: :json - milestones = JSON.parse(response.body) + milestones = json_response expect(milestones.count).to eq(2) expect(milestones.first["title"]).to eq("group milestone") diff --git a/spec/controllers/groups/uploads_controller_spec.rb b/spec/controllers/groups/uploads_controller_spec.rb index 0f99a957581..60342bf8e3d 100644 --- a/spec/controllers/groups/uploads_controller_spec.rb +++ b/spec/controllers/groups/uploads_controller_spec.rb @@ -10,6 +10,11 @@ describe Groups::UploadsController do { group_id: model } end + let(:other_model) { create(:group, :public) } + let(:other_params) do + { group_id: other_model } + end + it_behaves_like 'handle uploads' do let(:uploader_class) { NamespaceFileUploader } end diff --git a/spec/controllers/health_check_controller_spec.rb b/spec/controllers/health_check_controller_spec.rb index 19d739fcf4f..92f005faf4a 100644 --- a/spec/controllers/health_check_controller_spec.rb +++ b/spec/controllers/health_check_controller_spec.rb @@ -5,7 +5,6 @@ require 'spec_helper' describe HealthCheckController do include StubENV - let(:json_response) { JSON.parse(response.body) } let(:xml_response) { Hash.from_xml(response.body)['hash'] } let(:token) { Gitlab::CurrentSettings.health_check_access_token } let(:whitelisted_ip) { '127.0.0.1' } diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb index fc62a8310aa..e82dcfcdb64 100644 --- a/spec/controllers/health_controller_spec.rb +++ b/spec/controllers/health_controller_spec.rb @@ -5,7 +5,6 @@ require 'spec_helper' describe HealthController do include StubENV - let(:json_response) { JSON.parse(response.body) } let(:token) { Gitlab::CurrentSettings.health_check_access_token } let(:whitelisted_ip) { '127.0.0.1' } let(:not_whitelisted_ip) { '127.0.0.2' } diff --git a/spec/controllers/ide_controller_spec.rb b/spec/controllers/ide_controller_spec.rb new file mode 100644 index 00000000000..0462f9520d5 --- /dev/null +++ b/spec/controllers/ide_controller_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe IdeController do + let(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'increases the views counter' do + expect(Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_views_count) + + get :index + end +end diff --git a/spec/controllers/import/github_controller_spec.rb b/spec/controllers/import/github_controller_spec.rb index 059354870b5..5675798ac33 100644 --- a/spec/controllers/import/github_controller_spec.rb +++ b/spec/controllers/import/github_controller_spec.rb @@ -33,6 +33,16 @@ describe Import::GithubController do expect(response).to have_http_status(200) end + + context 'when importing a CI/CD project' do + it 'always prompts for an access token' do + allow(controller).to receive(:github_import_configured?).and_return(true) + + get :new, params: { ci_cd_only: true } + + expect(response).to render_template(:new) + end + end end describe "GET callback" do diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index ee454a7818c..84027119491 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -5,7 +5,6 @@ require 'spec_helper' describe MetricsController do include StubENV - let(:json_response) { JSON.parse(response.body) } let(:metrics_multiproc_dir) { Dir.mktmpdir } let(:whitelisted_ip) { '127.0.0.1' } let(:whitelisted_ip_range) { '10.0.0.0/24' } diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 44500d3cde3..45aebd1554c 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -160,7 +160,7 @@ describe Projects::BlobController do it 'renders diff context lines Gitlab::Diff::Line array' do do_get(since: 1, to: 2, offset: 0, from_merge_request: true) - lines = JSON.parse(response.body) + lines = json_response expect(lines.size).to eq(diff_lines.size) lines.each do |line| @@ -173,7 +173,7 @@ describe Projects::BlobController do it 'handles full being true' do do_get(full: true, from_merge_request: true) - lines = JSON.parse(response.body) + lines = json_response expect(lines.size).to eq(diff_lines.size) end diff --git a/spec/controllers/projects/boards_controller_spec.rb b/spec/controllers/projects/boards_controller_spec.rb index c07afc57aea..543479d8dd5 100644 --- a/spec/controllers/projects/boards_controller_spec.rb +++ b/spec/controllers/projects/boards_controller_spec.rb @@ -69,10 +69,8 @@ describe Projects::BoardsController do list_boards format: :json - parsed_response = JSON.parse(response.body) - expect(response).to match_response_schema('boards') - expect(parsed_response.length).to eq 2 + expect(json_response.length).to eq 2 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 b30966e70a7..f5bcea4a097 100644 --- a/spec/controllers/projects/branches_controller_spec.rb +++ b/spec/controllers/projects/branches_controller_spec.rb @@ -495,10 +495,8 @@ describe Projects::BranchesController do search: 'master' } - parsed_response = JSON.parse(response.body) - - expect(parsed_response.length).to eq 1 - expect(parsed_response.first).to eq 'master' + expect(json_response.length).to eq 1 + expect(json_response.first).to eq 'master' end end @@ -591,8 +589,7 @@ describe Projects::BranchesController do end it 'returns the commit counts behind and ahead of default branch' do - parsed_response = JSON.parse(response.body) - expect(parsed_response).to eq( + expect(json_response).to eq( "fix" => { "behind" => 29, "ahead" => 2 }, "branch-merged" => { "behind" => 1, "ahead" => 0 }, "add-pdf-file" => { "behind" => 0, "ahead" => 3 } diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index b5c6382a26d..58a1d96d010 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -378,8 +378,8 @@ describe Projects::CommitController do get_pipelines(id: commit.id, format: :json) expect(response).to be_ok - expect(JSON.parse(response.body)['pipelines']).not_to be_empty - expect(JSON.parse(response.body)['count']['all']).to eq 1 + expect(json_response['pipelines']).not_to be_empty + expect(json_response['count']['all']).to eq 1 expect(response).to include_pagination_headers end end diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb index 92380a2bf09..48a92a772dc 100644 --- a/spec/controllers/projects/compare_controller_spec.rb +++ b/spec/controllers/projects/compare_controller_spec.rb @@ -302,8 +302,7 @@ describe Projects::CompareController do signatures_request expect(response).to have_gitlab_http_status(200) - parsed_body = JSON.parse(response.body) - signatures = parsed_body['signatures'] + signatures = json_response['signatures'] expect(signatures.size).to eq(1) expect(signatures.first['commit_sha']).to eq(signature_commit.sha) @@ -332,8 +331,7 @@ describe Projects::CompareController do signatures_request expect(response).to have_gitlab_http_status(200) - parsed_body = JSON.parse(response.body) - expect(parsed_body['signatures']).to be_empty + expect(json_response['signatures']).to be_empty end end @@ -345,8 +343,7 @@ describe Projects::CompareController do signatures_request expect(response).to have_gitlab_http_status(200) - parsed_body = JSON.parse(response.body) - expect(parsed_body['signatures']).to be_empty + expect(json_response['signatures']).to be_empty end end end diff --git a/spec/controllers/projects/cycle_analytics/events_controller_spec.rb b/spec/controllers/projects/cycle_analytics/events_controller_spec.rb new file mode 100644 index 00000000000..8fc3ae0aa32 --- /dev/null +++ b/spec/controllers/projects/cycle_analytics/events_controller_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::CycleAnalytics::EventsController do + let(:project) { create(:project, :repository) } + let(:user) { create(:user) } + + before do + sign_in(user) + project.add_maintainer(user) + end + + describe 'cycle analytics not set up flag' do + context 'with no data' do + it 'is empty' do + get_issue + + expect(response).to be_success + expect(JSON.parse(response.body)['events']).to be_empty + end + end + + context 'with data' do + let(:milestone) { create(:milestone, project: project, created_at: 10.days.ago) } + let(:issue) { create(:issue, project: project, created_at: 9.days.ago) } + + before do + issue.update(milestone: milestone) + end + + it 'is not empty' do + get_issue + + expect(response).to be_success + end + + it 'contains event detais' do + get_issue + + events = JSON.parse(response.body)['events'] + + expect(events).not_to be_empty + expect(events.first).to include('title', 'author', 'iid', 'total_time', 'created_at', 'url') + expect(events.first['title']).to eq(issue.title) + end + + context 'with data older than start date' do + it 'is empty' do + get_issue(additional_params: { cycle_analytics: { start_date: 7 } }) + + expect(response).to be_success + + expect(JSON.parse(response.body)['events']).to be_empty + end + end + end + end + + def get_issue(additional_params: {}) + params = additional_params.merge(namespace_id: project.namespace, project_id: project) + get(:issue, params: params, format: :json) + end +end diff --git a/spec/controllers/projects/deploy_keys_controller_spec.rb b/spec/controllers/projects/deploy_keys_controller_spec.rb index fcd14f13863..ccad76eaddd 100644 --- a/spec/controllers/projects/deploy_keys_controller_spec.rb +++ b/spec/controllers/projects/deploy_keys_controller_spec.rb @@ -52,12 +52,10 @@ describe Projects::DeployKeysController do it 'returns json in a correct format' do get :index, params: params.merge(format: :json) - json = JSON.parse(response.body) - - expect(json.keys).to match_array(%w(enabled_keys available_project_keys public_keys)) - expect(json['enabled_keys'].count).to eq(1) - expect(json['available_project_keys'].count).to eq(1) - expect(json['public_keys'].count).to eq(1) + expect(json_response.keys).to match_array(%w(enabled_keys available_project_keys public_keys)) + expect(json_response['enabled_keys'].count).to eq(1) + expect(json_response['available_project_keys'].count).to eq(1) + expect(json_response['public_keys'].count).to eq(1) end end end diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb index 95417936df4..b9ee69a617b 100644 --- a/spec/controllers/projects/deployments_controller_spec.rb +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -41,34 +41,26 @@ describe Projects::DeploymentsController do describe 'GET #metrics' do let(:deployment) { create(:deployment, :success, project: project, environment: environment) } - before do - allow(controller).to receive(:deployment).and_return(deployment) - end - context 'when metrics are disabled' do - before do - allow(deployment).to receive(:has_metrics?).and_return false - end - it 'responds with not found' do - get :metrics, params: deployment_params(id: deployment.id) + get :metrics, params: deployment_params(id: deployment.to_param) expect(response).to be_not_found end end context 'when metrics are enabled' do - before do - allow(deployment).to receive(:has_metrics?).and_return true - end - context 'when environment has no metrics' do before do - expect(deployment).to receive(:metrics).and_return(nil) + expect_next_instance_of(DeploymentMetrics) do |deployment_metrics| + allow(deployment_metrics).to receive(:has_metrics?).and_return(true) + + expect(deployment_metrics).to receive(:metrics).and_return(nil) + end end it 'returns a empty response 204 resposne' do - get :metrics, params: deployment_params(id: deployment.id) + get :metrics, params: deployment_params(id: deployment.to_param) expect(response).to have_gitlab_http_status(204) expect(response.body).to eq('') end @@ -84,11 +76,15 @@ describe Projects::DeploymentsController do end before do - expect(deployment).to receive(:metrics).and_return(empty_metrics) + expect_next_instance_of(DeploymentMetrics) do |deployment_metrics| + allow(deployment_metrics).to receive(:has_metrics?).and_return(true) + + expect(deployment_metrics).to receive(:metrics).and_return(empty_metrics) + end end it 'returns a metrics JSON document' do - get :metrics, params: deployment_params(id: deployment.id) + get :metrics, params: deployment_params(id: deployment.to_param) expect(response).to be_ok expect(json_response['success']).to be(true) @@ -96,54 +92,32 @@ describe Projects::DeploymentsController do expect(json_response['last_update']).to eq(42) end end - - context 'when metrics service does not implement deployment metrics' do - before do - allow(deployment).to receive(:metrics).and_raise(NotImplementedError) - end - - it 'responds with not found' do - get :metrics, params: deployment_params(id: deployment.id) - - expect(response).to be_not_found - end - end end end describe 'GET #additional_metrics' do let(:deployment) { create(:deployment, :success, project: project, environment: environment) } - before do - allow(controller).to receive(:deployment).and_return(deployment) - end - context 'when metrics are disabled' do - before do - allow(deployment).to receive(:has_metrics?).and_return false - end - it 'responds with not found' do - get :metrics, params: deployment_params(id: deployment.id) + get :metrics, params: deployment_params(id: deployment.to_param) expect(response).to be_not_found end end context 'when metrics are enabled' do - let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } - - before do - allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) - end - context 'when environment has no metrics' do before do - expect(deployment).to receive(:additional_metrics).and_return({}) + expect_next_instance_of(DeploymentMetrics) do |deployment_metrics| + allow(deployment_metrics).to receive(:has_metrics?).and_return(true) + + expect(deployment_metrics).to receive(:additional_metrics).and_return({}) + end end it 'returns a empty response 204 response' do - get :additional_metrics, params: deployment_params(id: deployment.id, format: :json) + get :additional_metrics, params: deployment_params(id: deployment.to_param, format: :json) expect(response).to have_gitlab_http_status(204) expect(response.body).to eq('') end @@ -159,11 +133,15 @@ describe Projects::DeploymentsController do end before do - expect(deployment).to receive(:additional_metrics).and_return(empty_metrics) + expect_next_instance_of(DeploymentMetrics) do |deployment_metrics| + allow(deployment_metrics).to receive(:has_metrics?).and_return(true) + + expect(deployment_metrics).to receive(:additional_metrics).and_return(empty_metrics) + end end it 'returns a metrics JSON document' do - get :additional_metrics, params: deployment_params(id: deployment.id, format: :json) + get :additional_metrics, params: deployment_params(id: deployment.to_param, format: :json) expect(response).to be_ok expect(json_response['success']).to be(true) diff --git a/spec/controllers/projects/discussions_controller_spec.rb b/spec/controllers/projects/discussions_controller_spec.rb index 4c29162cd0f..e30b28a4bd5 100644 --- a/spec/controllers/projects/discussions_controller_spec.rb +++ b/spec/controllers/projects/discussions_controller_spec.rb @@ -112,7 +112,7 @@ describe Projects::DiscussionsController do it "returns the name of the resolving user" do post :resolve, params: request_params - expect(JSON.parse(response.body)['resolved_by']['name']).to eq(user.name) + expect(json_response['resolved_by']['name']).to eq(user.name) end it "returns status 200" do @@ -135,7 +135,7 @@ describe Projects::DiscussionsController do it "returns truncated diff lines" do post :resolve, params: request_params - expect(JSON.parse(response.body)['truncated_diff_lines']).to be_present + expect(json_response['truncated_diff_lines']).to be_present end end end diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb index fdef9bc5638..45328482ad7 100644 --- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb +++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb @@ -176,7 +176,7 @@ describe Projects::Environments::PrometheusApiController do def environment_params(params = {}) { id: environment.id.to_s, - namespace_id: project.namespace.name, + namespace_id: project.namespace.full_path, project_id: project.name, proxy_path: 'query', query: '1' diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 4c2c6160c62..ebbbebf1bc0 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Projects::EnvironmentsController do + include MetricsDashboardHelpers + set(:user) { create(:user) } set(:project) { create(:project) } @@ -445,131 +447,186 @@ describe Projects::EnvironmentsController do end end - describe 'metrics_dashboard' do - context 'when prometheus endpoint is disabled' do - before do - stub_feature_flags(environment_metrics_use_prometheus_endpoint: false) - end + describe 'GET #metrics_dashboard' do + shared_examples_for 'correctly formatted response' do |status_code| + it 'returns a json object with the correct keys' do + get :metrics_dashboard, params: environment_params(dashboard_params) - it 'responds with status code 403' do - get :metrics_dashboard, params: environment_params(format: :json) + # Exlcude `all_dashboards` to handle separately. + found_keys = json_response.keys - ['all_dashboards'] - expect(response).to have_gitlab_http_status(:forbidden) + expect(response).to have_gitlab_http_status(status_code) + expect(found_keys).to contain_exactly(*expected_keys) end end - shared_examples_for '200 response' do |contains_all_dashboards: false| + shared_examples_for '200 response' do let(:expected_keys) { %w(dashboard status) } - before do - expected_keys << 'all_dashboards' if contains_all_dashboards - end - - it 'returns a json representation of the environment dashboard' do - get :metrics_dashboard, params: environment_params(dashboard_params) - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.keys).to contain_exactly(*expected_keys) - expect(json_response['dashboard']).to be_an_instance_of(Hash) - end + it_behaves_like 'correctly formatted response', :ok end - shared_examples_for 'error response' do |status_code, contains_all_dashboards: false| + shared_examples_for 'error response' do |status_code| let(:expected_keys) { %w(message status) } - before do - expected_keys << 'all_dashboards' if contains_all_dashboards - end + it_behaves_like 'correctly formatted response', status_code + end - it 'returns an error response' do + shared_examples_for 'includes all dashboards' do + it 'includes info for all findable dashboard' do get :metrics_dashboard, params: environment_params(dashboard_params) - expect(response).to have_gitlab_http_status(status_code) - expect(json_response.keys).to contain_exactly(*expected_keys) + expect(json_response).to have_key('all_dashboards') + expect(json_response['all_dashboards']).to be_an_instance_of(Array) + expect(json_response['all_dashboards']).to all( include('path', 'default', 'display_name') ) end end - shared_examples_for 'has all dashboards' do - it 'includes an index of all available dashboards' do + shared_examples_for 'the default dashboard' do + all_dashboards = Feature.enabled?(:environment_metrics_show_multiple_dashboards) + + it_behaves_like '200 response' + it_behaves_like 'includes all dashboards' if all_dashboards + + it 'is the default dashboard' do get :metrics_dashboard, params: environment_params(dashboard_params) - expect(json_response.keys).to include('all_dashboards') - expect(json_response['all_dashboards']).to be_an_instance_of(Array) - expect(json_response['all_dashboards']).to all( include('path', 'default') ) + expect(json_response['dashboard']['dashboard']).to eq('Environment metrics') end end - context 'when multiple dashboards is disabled' do - before do - stub_feature_flags(environment_metrics_show_multiple_dashboards: false) - end + shared_examples_for 'the specified dashboard' do |expected_dashboard| + it_behaves_like '200 response' + it_behaves_like 'includes all dashboards' - let(:dashboard_params) { { format: :json } } + it 'has the correct name' do + get :metrics_dashboard, params: environment_params(dashboard_params) - it_behaves_like '200 response' + dashboard_name = json_response['dashboard']['dashboard'] - context 'when the dashboard could not be provided' do + # 'Environment metrics' is the default dashboard. + expect(dashboard_name).not_to eq('Environment metrics') + expect(dashboard_name).to eq(expected_dashboard) + end + + context 'when the dashboard cannot not be processed' do before do allow(YAML).to receive(:safe_load).and_return({}) end it_behaves_like 'error response', :unprocessable_entity end - - context 'when a dashboard param is specified' do - let(:dashboard_params) { { format: :json, dashboard: '.gitlab/dashboards/not_there_dashboard.yml' } } - - it_behaves_like '200 response' - end end - context 'when multiple dashboards is enabled' do - let(:dashboard_params) { { format: :json } } + shared_examples_for 'the default dynamic dashboard' do + it_behaves_like '200 response' - it_behaves_like '200 response', contains_all_dashboards: true - it_behaves_like 'has all dashboards' + it 'contains only the Memory and CPU charts' do + get :metrics_dashboard, params: environment_params(dashboard_params) - context 'when a dashboard could not be provided' do - before do - allow(YAML).to receive(:safe_load).and_return({}) - end + dashboard = json_response['dashboard'] + panel_group = dashboard['panel_groups'].first + titles = panel_group['panels'].map { |panel| panel['title'] } - it_behaves_like 'error response', :unprocessable_entity, contains_all_dashboards: true - it_behaves_like 'has all dashboards' + expect(dashboard['dashboard']).to be_nil + expect(dashboard['panel_groups'].length).to eq 1 + expect(panel_group['group']).to be_nil + expect(titles).to eq ['Memory Usage (Total)', 'Core Usage (Total)'] end + end - context 'when a dashboard param is specified' do - let(:dashboard_params) { { format: :json, dashboard: '.gitlab/dashboards/test.yml' } } + shared_examples_for 'dashboard can be specified' do + context 'when dashboard is specified' do + let(:dashboard_path) { '.gitlab/dashboards/test.yml' } + let(:dashboard_params) { { format: :json, dashboard: dashboard_path } } + + it_behaves_like 'error response', :not_found - context 'when the dashboard is available' do + context 'when the project dashboard is available' do let(:dashboard_yml) { fixture_file('lib/gitlab/metrics/dashboard/sample_dashboard.yml') } - let(:dashboard_file) { { '.gitlab/dashboards/test.yml' => dashboard_yml } } - let(:project) { create(:project, :custom_repo, files: dashboard_file) } + let(:project) { project_with_dashboard(dashboard_path, dashboard_yml) } let(:environment) { create(:environment, name: 'production', project: project) } - it_behaves_like '200 response', contains_all_dashboards: true - it_behaves_like 'has all dashboards' + it_behaves_like 'the specified dashboard', 'Test Dashboard' end - context 'when the dashboard does not exist' do - it_behaves_like 'error response', :not_found, contains_all_dashboards: true - it_behaves_like 'has all dashboards' + context 'when the specified dashboard is the default dashboard' do + let(:dashboard_path) { Gitlab::Metrics::Dashboard::SystemDashboardService::SYSTEM_DASHBOARD_PATH } + + it_behaves_like 'the default dashboard' end end + end - context 'when the dashboard is intended for embedding' do + shared_examples_for 'dashboard can be embedded' do + context 'when the embedded flag is included' do let(:dashboard_params) { { format: :json, embedded: true } } - it_behaves_like '200 response' + it_behaves_like 'the default dynamic dashboard' - context 'when a dashboard path is provided' do - let(:dashboard_params) { { format: :json, dashboard: '.gitlab/dashboards/test.yml', embedded: true } } + context 'when the dashboard is specified' do + let(:dashboard_params) { { format: :json, embedded: true, dashboard: '.gitlab/dashboards/fake.yml' } } - # The dashboard path should simple be ignored. - it_behaves_like '200 response' + # The dashboard param should be ignored. + it_behaves_like 'the default dynamic dashboard' end end end + + shared_examples_for 'dashboard cannot be specified' do + context 'when dashboard is specified' do + let(:dashboard_params) { { format: :json, dashboard: '.gitlab/dashboards/test.yml' } } + + it_behaves_like 'the default dashboard' + end + end + + shared_examples_for 'dashboard cannot be embedded' do + context 'when the embedded flag is included' do + let(:dashboard_params) { { format: :json, embedded: true } } + + it_behaves_like 'the default dashboard' + end + end + + let(:dashboard_params) { { format: :json } } + + it_behaves_like 'the default dashboard' + it_behaves_like 'dashboard can be specified' + it_behaves_like 'dashboard can be embedded' + + context 'when multiple dashboards is enabled and embedding metrics is disabled' do + before do + stub_feature_flags(gfm_embedded_metrics: false) + end + + it_behaves_like 'the default dashboard' + it_behaves_like 'dashboard can be specified' + it_behaves_like 'dashboard cannot be embedded' + end + + context 'when multiple dashboards is disabled and embedding metrics is enabled' do + before do + stub_feature_flags(environment_metrics_show_multiple_dashboards: false) + end + + it_behaves_like 'the default dashboard' + it_behaves_like 'dashboard cannot be specified' + it_behaves_like 'dashboard can be embedded' + end + + context 'when multiple dashboards and embedding metrics are disabled' do + before do + stub_feature_flags( + environment_metrics_show_multiple_dashboards: false, + gfm_embedded_metrics: false + ) + end + + it_behaves_like 'the default dashboard' + it_behaves_like 'dashboard cannot be specified' + it_behaves_like 'dashboard cannot be embedded' + end end describe 'GET #search' do diff --git a/spec/controllers/projects/find_file_controller_spec.rb b/spec/controllers/projects/find_file_controller_spec.rb index 538dbb5ad0b..a493985f8a0 100644 --- a/spec/controllers/projects/find_file_controller_spec.rb +++ b/spec/controllers/projects/find_file_controller_spec.rb @@ -53,10 +53,9 @@ describe Projects::FindFileController do it 'returns an array of file path list' do go - json = JSON.parse(response.body) is_expected.to respond_with(:success) - expect(json).not_to eq(nil) - expect(json.length).to be >= 0 + expect(json_response).not_to eq(nil) + expect(json_response.length).to be >= 0 end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index bc5e0b4671e..32d14dce936 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -444,7 +444,7 @@ describe Projects::IssuesController do it 'renders json with recaptcha_html' do subject - expect(JSON.parse(response.body)).to have_key('recaptcha_html') + expect(json_response).to have_key('recaptcha_html') end end end @@ -484,10 +484,8 @@ describe Projects::IssuesController do it 'returns last edited time' do go(id: issue.iid) - data = JSON.parse(response.body) - - expect(data).to include('updated_at') - expect(data['updated_at']).to eq(issue.last_edited_at.to_time.iso8601) + expect(json_response).to include('updated_at') + expect(json_response['updated_at']).to eq(issue.last_edited_at.to_time.iso8601) end end @@ -520,10 +518,8 @@ describe Projects::IssuesController do it 'returns the necessary data' do go(id: issue.iid) - data = JSON.parse(response.body) - - expect(data).to include('title_text', 'description', 'description_text') - expect(data).to include('task_status', 'lock_version') + expect(json_response).to include('title_text', 'description', 'description_text') + expect(json_response).to include('task_status', 'lock_version') end end end @@ -692,9 +688,7 @@ describe Projects::IssuesController do update_issue(issue_params: { assignee_ids: [assignee.id] }) - body = JSON.parse(response.body) - - expect(body['assignees'].first.keys) + expect(json_response['assignees'].first.keys) .to match_array(%w(id name username avatar_url state web_url)) end end @@ -1314,7 +1308,7 @@ describe Projects::IssuesController do it 'filters notes that the user should not see' do get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } - expect(JSON.parse(response.body).count).to eq(1) + expect(json_response.count).to eq(1) end it 'does not result in N+1 queries' do diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 901402aa5fd..e428aa3c7b7 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -158,7 +158,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do get_show_json json_response.dig('pipeline', 'details', 'stages').tap do |stages| - expect(stages.map(&:keys).flatten) + expect(stages.flat_map(&:keys)) .to eq %w[name title status path dropdown_path] end end diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index 5fefad86ef3..3816e1c7a31 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb @@ -212,4 +212,46 @@ describe Projects::MergeRequests::CreationsController do expect(response).to have_gitlab_http_status(200) end end + + describe 'POST create' do + let(:params) do + { + namespace_id: fork_project.namespace.to_param, + project_id: fork_project, + merge_request: { + title: 'Test merge request', + source_branch: 'remove-submodule', + target_branch: 'master' + } + } + end + + it 'creates merge request' do + expect do + post_request(params) + end.to change { MergeRequest.count }.by(1) + end + + context 'when the merge request is not created from the web ide' do + it 'counter is not increased' do + expect(Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_merge_requests_count) + + post_request(params) + end + end + + context 'when the merge request is created from the web ide' do + let(:nav_source) { { nav_source: 'webide' } } + + it 'counter is increased' do + expect(Gitlab::UsageDataCounters::WebIdeCounter).to receive(:increment_merge_requests_count) + + post_request(params.merge(nav_source)) + end + end + + def post_request(merge_request_params) + post :create, params: merge_request_params + end + end end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 13a28b738ca..d940d226176 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -112,7 +112,7 @@ describe Projects::MergeRequests::DiffsController do it 'only renders the diffs for the path given' do diff_for_path(old_path: existing_path, new_path: existing_path) - paths = JSON.parse(response.body)["diff_files"].map { |file| file['new_path'] } + paths = json_response["diff_files"].map { |file| file['new_path'] } expect(paths).to include(existing_path) end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 64a89205e2f..bdd7322290f 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -242,9 +242,7 @@ describe Projects::MergeRequestsController do update_merge_request({ assignee_ids: [assignee.id] }, format: :json) - body = JSON.parse(response.body) - - expect(body['assignees']).to all(include(*%w(name username avatar_url id state web_url))) + expect(json_response['assignees']).to all(include(*%w(name username avatar_url id state web_url))) end end @@ -968,16 +966,79 @@ describe Projects::MergeRequestsController do expect(control_count).to be <= 137 end - def get_ci_environments_status(extra_params = {}) - params = { - namespace_id: merge_request.project.namespace.to_param, - project_id: merge_request.project, - id: merge_request.iid, - format: 'json' - } + it 'has no N+1 SQL issues for environments', :request_store, retry: 0 do + # First run to insert test data from lets, which does take up some 30 queries + get_ci_environments_status + + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { get_ci_environments_status }.count + + environment2 = create(:environment, project: forked) + create(:deployment, :succeed, environment: environment2, sha: sha, ref: 'master', deployable: build) + + # TODO address the last 11 queries + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63952 (5 queries) + # And https://gitlab.com/gitlab-org/gitlab-ce/issues/64105 (6 queries) + leeway = 11 + expect { get_ci_environments_status }.not_to exceed_all_query_limit(control_count + leeway) + end + end - get :ci_environments_status, params: params.merge(extra_params) + context 'when a merge request has multiple environments with deployments' do + let(:sha) { merge_request.diff_head_sha } + let(:ref) { merge_request.source_branch } + + let!(:build) { create(:ci_build, pipeline: pipeline) } + let!(:pipeline) { create(:ci_pipeline, sha: sha, project: project) } + let!(:environment) { create(:environment, name: 'env_a', project: project) } + let!(:another_environment) { create(:environment, name: 'env_b', project: project) } + + before do + merge_request.update_head_pipeline + + create(:deployment, :succeed, environment: environment, sha: sha, ref: ref, deployable: build) + create(:deployment, :succeed, environment: another_environment, sha: sha, ref: ref, deployable: build) end + + it 'exposes multiple environment statuses' do + get_ci_environments_status + + expect(json_response.count).to eq 2 + end + + context 'when route map is not present in the project' do + it 'does not have N+1 Gitaly requests for environments', :request_store do + expect(merge_request).to be_present + + expect { get_ci_environments_status } + .to change { Gitlab::GitalyClient.get_request_count }.by_at_most(1) + end + end + + context 'when there is route map present in a project' do + before do + allow_any_instance_of(EnvironmentStatus) + .to receive(:has_route_map?) + .and_return(true) + end + + it 'does not have N+1 Gitaly requests for diff files', :request_store do + expect(merge_request.merge_request_diff.merge_request_diff_files).to be_many + + expect { get_ci_environments_status } + .to change { Gitlab::GitalyClient.get_request_count }.by_at_most(1) + end + end + end + + def get_ci_environments_status(extra_params = {}) + params = { + namespace_id: merge_request.project.namespace.to_param, + project_id: merge_request.project, + id: merge_request.iid, + format: 'json' + } + + get :ci_environments_status, params: params.merge(extra_params) end end diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 1db1963476c..98aea9056dc 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -29,7 +29,7 @@ describe Projects::NotesController do } end - let(:parsed_response) { JSON.parse(response.body).with_indifferent_access } + let(:parsed_response) { json_response.with_indifferent_access } let(:note_json) { parsed_response[:notes].first } before do @@ -614,7 +614,7 @@ describe Projects::NotesController do it "returns the name of the resolving user" do post :resolve, params: request_params.merge(html: true) - expect(JSON.parse(response.body)["resolved_by"]).to eq(user.name) + expect(json_response["resolved_by"]).to eq(user.name) end it "returns status 200" do diff --git a/spec/controllers/projects/templates_controller_spec.rb b/spec/controllers/projects/templates_controller_spec.rb index 9e7d34b10c0..d5ef2b0e114 100644 --- a/spec/controllers/projects/templates_controller_spec.rb +++ b/spec/controllers/projects/templates_controller_spec.rb @@ -7,7 +7,6 @@ describe Projects::TemplatesController do let(:user) { create(:user) } let(:file_path_1) { '.gitlab/issue_templates/issue_template.md' } let(:file_path_2) { '.gitlab/merge_request_templates/merge_request_template.md' } - let(:body) { JSON.parse(response.body) } let!(:file_1) { project.repository.create_file(user, file_path_1, 'issue content', message: 'message', branch_name: 'master') } let!(:file_2) { project.repository.create_file(user, file_path_2, 'merge request content', message: 'message', branch_name: 'master') } @@ -17,8 +16,8 @@ describe Projects::TemplatesController do get(:show, params: { namespace_id: project.namespace, template_type: 'issue', key: 'issue_template', project_id: project }, format: :json) expect(response.status).to eq(200) - expect(body['name']).to eq('issue_template') - expect(body['content']).to eq('issue content') + expect(json_response['name']).to eq('issue_template') + expect(json_response['content']).to eq('issue content') end end @@ -27,8 +26,8 @@ describe Projects::TemplatesController do get(:show, params: { namespace_id: project.namespace, template_type: 'merge_request', key: 'merge_request_template', project_id: project }, format: :json) expect(response.status).to eq(200) - expect(body['name']).to eq('merge_request_template') - expect(body['content']).to eq('merge request content') + expect(json_response['name']).to eq('merge_request_template') + expect(json_response['content']).to eq('merge request content') end end diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb index 776c1270977..661ed9840b1 100644 --- a/spec/controllers/projects/uploads_controller_spec.rb +++ b/spec/controllers/projects/uploads_controller_spec.rb @@ -10,6 +10,11 @@ describe Projects::UploadsController do { namespace_id: model.namespace.to_param, project_id: model } end + let(:other_model) { create(:project, :public) } + let(:other_params) do + { namespace_id: other_model.namespace.to_param, project_id: other_model } + end + it_behaves_like 'handle uploads' context 'when the URL the old style, without /-/system' do diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index f2e0b5e5c1d..fbca1d5740f 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -31,6 +31,47 @@ describe Projects::WikisController do end end + describe 'GET #history' do + before do + allow(controller) + .to receive(:can?) + .with(any_args) + .and_call_original + + # The :create_wiki permission is irrelevant to reading history. + expect(controller) + .not_to receive(:can?) + .with(anything, :create_wiki, any_args) + + allow(controller) + .to receive(:can?) + .with(anything, :read_wiki, any_args) + .and_return(allow_read_wiki) + end + + shared_examples 'fetching history' do |expected_status| + before do + get :history, params: { namespace_id: project.namespace, project_id: project, id: wiki_title } + end + + it "returns status #{expected_status}" do + expect(response).to have_http_status(expected_status) + end + end + + it_behaves_like 'fetching history', :ok do + let(:allow_read_wiki) { true } + + it 'assigns @page_versions' do + expect(assigns(:page_versions)).to be_present + end + end + + it_behaves_like 'fetching history', :not_found do + let(:allow_read_wiki) { false } + end + end + describe 'GET #show' do render_views @@ -103,7 +144,7 @@ describe Projects::WikisController do it 'renders json in a correct format' do post :preview_markdown, params: { 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)) + expect(json_response.keys).to match_array(%w(body references)) end end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 8d2412f97ef..083a1c1383a 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -318,6 +318,53 @@ describe ProjectsController do end end + describe '#housekeeping' do + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + let(:housekeeping) { Projects::HousekeepingService.new(project) } + + context 'when authenticated as owner' do + before do + group.add_owner(user) + sign_in(user) + + allow(Projects::HousekeepingService).to receive(:new).with(project, :gc).and_return(housekeeping) + end + + it 'forces a full garbage collection' do + expect(housekeeping).to receive(:execute).once + + post :housekeeping, + params: { + namespace_id: project.namespace.path, + id: project.path + } + + expect(response).to have_gitlab_http_status(302) + end + end + + context 'when authenticated as developer' do + let(:developer) { create(:user) } + + before do + group.add_developer(developer) + end + + it 'does not execute housekeeping' do + expect(housekeeping).not_to receive(:execute) + + post :housekeeping, + params: { + namespace_id: project.namespace.path, + id: project.path + } + + expect(response).to have_gitlab_http_status(302) + end + end + end + describe "#update" do render_views @@ -693,20 +740,18 @@ describe ProjectsController do it 'gets a list of branches and tags' do get :refs, params: { namespace_id: project.namespace, id: project, sort: 'updated_desc' } - parsed_body = JSON.parse(response.body) - expect(parsed_body['Branches']).to include('master') - expect(parsed_body['Tags'].first).to eq('v1.1.0') - expect(parsed_body['Tags'].last).to eq('v1.0.0') - expect(parsed_body['Commits']).to be_nil + expect(json_response['Branches']).to include('master') + expect(json_response['Tags'].first).to eq('v1.1.0') + expect(json_response['Tags'].last).to eq('v1.0.0') + expect(json_response['Commits']).to be_nil end it "gets a list of branches, tags and commits" do get :refs, params: { namespace_id: project.namespace, id: project, ref: "123456" } - parsed_body = JSON.parse(response.body) - expect(parsed_body["Branches"]).to include("master") - expect(parsed_body["Tags"]).to include("v1.0.0") - expect(parsed_body["Commits"]).to include("123456") + expect(json_response["Branches"]).to include("master") + expect(json_response["Tags"]).to include("v1.0.0") + expect(json_response["Commits"]).to include("123456") end context "when preferred language is Japanese" do @@ -718,10 +763,9 @@ describe ProjectsController do it "gets a list of branches, tags and commits" do get :refs, params: { namespace_id: project.namespace, id: project, ref: "123456" } - parsed_body = JSON.parse(response.body) - expect(parsed_body["Branches"]).to include("master") - expect(parsed_body["Tags"]).to include("v1.0.0") - expect(parsed_body["Commits"]).to include("123456") + expect(json_response["Branches"]).to include("master") + expect(json_response["Tags"]).to include("v1.0.0") + expect(json_response["Commits"]).to include("123456") end end @@ -750,7 +794,7 @@ describe ProjectsController do it 'renders json in a correct format' do post :preview_markdown, params: { namespace_id: public_project.namespace, id: public_project, text: '*Markdown* text' } - expect(JSON.parse(response.body).keys).to match_array(%w(body references)) + expect(json_response.keys).to match_array(%w(body references)) end context 'when not authorized' do @@ -774,8 +818,6 @@ describe ProjectsController do text: issue.to_reference } - json_response = JSON.parse(response.body) - expect(json_response['body']).to match(/\##{issue.iid} \(closed\)/) end @@ -786,8 +828,6 @@ describe ProjectsController do text: merge_request.to_reference } - json_response = JSON.parse(response.body) - expect(json_response['body']).to match(/\!#{merge_request.iid} \(closed\)/) end end diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index 586d59c2d09..652533ac49f 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -26,7 +26,7 @@ describe Snippets::NotesController do end it "returns not empty array of notes" do - expect(JSON.parse(response.body)["notes"].empty?).to be_falsey + expect(json_response["notes"].empty?).to be_falsey end end @@ -97,7 +97,7 @@ describe Snippets::NotesController do it "returns 1 note" do get :index, params: { snippet_id: private_snippet } - expect(JSON.parse(response.body)['notes'].count).to eq(1) + expect(json_response['notes'].count).to eq(1) end end end @@ -114,7 +114,7 @@ describe Snippets::NotesController do it "does not return any note" do get :index, params: { snippet_id: public_snippet } - expect(JSON.parse(response.body)['notes'].count).to eq(0) + expect(json_response['notes'].count).to eq(0) end end end diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 3aba02bf3ff..b0092bc8994 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -622,7 +622,7 @@ describe SnippetsController do post :preview_markdown, params: { id: snippet, text: '*Markdown* text' } - expect(JSON.parse(response.body).keys).to match_array(%w(body references)) + expect(json_response.keys).to match_array(%w(body references)) end end end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index c3d6ea9cbcd..8b8d4c57000 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -291,7 +291,7 @@ describe UsersController do it 'response with snippets json data' do get :snippets, params: { username: user.username }, format: :json expect(response).to have_gitlab_http_status(200) - expect(JSON.parse(response.body)).to have_key('html') + expect(json_response).to have_key('html') end end |