From 05f0ebba3a2c8ddf39e436f412dc2ab5bf1353b2 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 18 Jan 2023 19:00:14 +0000 Subject: Add latest changes from gitlab-org/gitlab@15-8-stable-ee --- spec/requests/projects/issues_controller_spec.rb | 47 ++++++++++--- .../projects/merge_requests/content_spec.rb | 2 +- .../merge_requests/context_commit_diffs_spec.rb | 3 +- .../projects/merge_requests/creations_spec.rb | 17 +++-- .../requests/projects/merge_requests/diffs_spec.rb | 14 +--- .../projects/merge_requests_controller_spec.rb | 76 +++++++++++++++------- .../projects/ml/candidates_controller_spec.rb | 8 +-- .../projects/ml/experiments_controller_spec.rb | 51 ++++++++++++--- 8 files changed, 150 insertions(+), 68 deletions(-) (limited to 'spec/requests/projects') diff --git a/spec/requests/projects/issues_controller_spec.rb b/spec/requests/projects/issues_controller_spec.rb index bbf200eaacd..67a73834f2d 100644 --- a/spec/requests/projects/issues_controller_spec.rb +++ b/spec/requests/projects/issues_controller_spec.rb @@ -8,10 +8,14 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do let_it_be(:project) { issue.project } let_it_be(:user) { issue.author } + shared_context 'group project issue' do + let_it_be(:project) { create :project, group: group } + let_it_be(:issue) { create :issue, project: project } + let_it_be(:user) { create(:user) } + end + describe 'GET #new' do - before do - login_as(user) - end + include_context 'group project issue' it_behaves_like "observability csp policy", described_class do let(:tested_path) do @@ -21,9 +25,7 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do end describe 'GET #show' do - before do - login_as(user) - end + include_context 'group project issue' it_behaves_like "observability csp policy", described_class do let(:tested_path) do @@ -32,13 +34,38 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do end end + describe 'GET #index.json' do + let_it_be(:public_project) { create(:project, :public) } + + it_behaves_like 'rate limited endpoint', rate_limit_key: :search_rate_limit do + let_it_be(:current_user) { create(:user) } + + before do + sign_in current_user + end + + def request + get project_issues_path(public_project, format: :json), params: { scope: 'all', search: 'test' } + end + end + + it_behaves_like 'rate limited endpoint', rate_limit_key: :search_rate_limit_unauthenticated do + def request + get project_issues_path(public_project, format: :json), params: { scope: 'all', search: 'test' } + end + end + end + describe 'GET #discussions' do before do login_as(user) end let_it_be(:discussion) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } - let_it_be(:discussion_reply) { create(:discussion_note_on_issue, noteable: issue, project: issue.project, in_reply_to: discussion) } + let_it_be(:discussion_reply) do + create(:discussion_note_on_issue, noteable: issue, project: issue.project, in_reply_to: discussion) + end + let_it_be(:state_event) { create(:resource_state_event, issue: issue) } let_it_be(:discussion_2) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } let_it_be(:discussion_3) { create(:discussion_note_on_issue, noteable: issue, project: issue.project) } @@ -92,7 +119,8 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do context 'when private project' do let_it_be(:private_project) { create(:project, :private) } - it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false, ignore_metrics: true do + it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false, +ignore_metrics: true do let(:url) { project_issues_url(private_project, format: :atom) } before do @@ -100,7 +128,8 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do end end - it_behaves_like 'authenticates sessionless user for the request spec', 'calendar ics', public_resource: false, ignore_metrics: true do + it_behaves_like 'authenticates sessionless user for the request spec', 'calendar ics', public_resource: false, +ignore_metrics: true do let(:url) { project_issues_url(private_project, format: :ics) } before do diff --git a/spec/requests/projects/merge_requests/content_spec.rb b/spec/requests/projects/merge_requests/content_spec.rb index 6c58dcb5722..54066756f3e 100644 --- a/spec/requests/projects/merge_requests/content_spec.rb +++ b/spec/requests/projects/merge_requests/content_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'merge request content spec', feature_category: :code_review do +RSpec.describe 'merge request content spec', feature_category: :code_review_workflow do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } let_it_be(:merge_request) { create(:merge_request, :with_head_pipeline, target_project: project, source_project: project) } diff --git a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb index 10e57970704..24e4dea5cdc 100644 --- a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb +++ b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Merge Requests Context Commit Diffs', feature_category: :code_review do +RSpec.describe 'Merge Requests Context Commit Diffs', feature_category: :code_review_workflow do let_it_be(:sha1) { "33f3729a45c02fc67d00adb1b8bca394b0e761d9" } let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } @@ -35,7 +35,6 @@ RSpec.describe 'Merge Requests Context Commit Diffs', feature_category: :code_re commit: nil, diff_view: :inline, merge_ref_head_diff: nil, - merge_conflicts_in_diff: true, pagination_data: { total_pages: nil }.merge(pagination_data) diff --git a/spec/requests/projects/merge_requests/creations_spec.rb b/spec/requests/projects/merge_requests/creations_spec.rb index 59e2047e1c7..ace6ef0f7b8 100644 --- a/spec/requests/projects/merge_requests/creations_spec.rb +++ b/spec/requests/projects/merge_requests/creations_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'merge requests creations', feature_category: :code_review do +RSpec.describe 'merge requests creations', feature_category: :code_review_workflow do describe 'GET /:namespace/:project/merge_requests/new' do include ProjectForksHelper @@ -26,14 +26,17 @@ RSpec.describe 'merge requests creations', feature_category: :code_review do end it_behaves_like "observability csp policy", Projects::MergeRequests::CreationsController do + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, group: group) } let(:tested_path) do project_new_merge_request_path(project, merge_request: { - title: 'Some feature', - source_branch: 'fix', - target_branch: 'feature', - target_project: project, - source_project: project - }) + title: 'Some feature', + source_branch: 'fix', + target_branch: 'feature', + target_project: project, + source_project: project + }) end end end diff --git a/spec/requests/projects/merge_requests/diffs_spec.rb b/spec/requests/projects/merge_requests/diffs_spec.rb index 858acac7f0d..f98688bf767 100644 --- a/spec/requests/projects/merge_requests/diffs_spec.rb +++ b/spec/requests/projects/merge_requests/diffs_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Merge Requests Diffs', feature_category: :code_review do +RSpec.describe 'Merge Requests Diffs', feature_category: :code_review_workflow do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } let_it_be(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } @@ -33,7 +33,6 @@ RSpec.describe 'Merge Requests Diffs', feature_category: :code_review do commit: nil, diff_view: :inline, merge_ref_head_diff: nil, - merge_conflicts_in_diff: true, pagination_data: { total_pages: nil }.merge(pagination_data) @@ -112,17 +111,6 @@ RSpec.describe 'Merge Requests Diffs', feature_category: :code_review do it_behaves_like 'serializes diffs with expected arguments' end - context 'with disabled display_merge_conflicts_in_diff feature' do - let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch } - let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_conflicts_in_diff: false) } - - before do - stub_feature_flags(display_merge_conflicts_in_diff: false) - end - - it_behaves_like 'serializes diffs with expected arguments' - end - context 'with diff_head option' do subject { go(page: 0, per_page: 5, diff_head: true) } diff --git a/spec/requests/projects/merge_requests_controller_spec.rb b/spec/requests/projects/merge_requests_controller_spec.rb index f5f8b5c2d83..f441438a95a 100644 --- a/spec/requests/projects/merge_requests_controller_spec.rb +++ b/spec/requests/projects/merge_requests_controller_spec.rb @@ -8,20 +8,65 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code let_it_be(:user) { merge_request.author } describe 'GET #show' do - before do - login_as(user) + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :public, group: group) } + + let(:merge_request) { create :merge_request, source_project: project, author: user } + + context 'when logged in' do + before do + login_as(user) + end + + it_behaves_like "observability csp policy", described_class do + let(:tested_path) do + project_merge_request_path(project, merge_request) + end + end + end + + context 'when the author of the merge request is banned', feature_category: :insider_threat do + let_it_be(:user) { create(:user, :banned) } + + subject { response } + + before do + get project_merge_request_path(project, merge_request) + end + + it { is_expected.to have_gitlab_http_status(:not_found) } + end + end + + describe 'GET #index' do + let_it_be(:public_project) { create(:project, :public) } + + it_behaves_like 'rate limited endpoint', rate_limit_key: :search_rate_limit do + let_it_be(:current_user) { user } + + before do + sign_in current_user + end + + def request + get project_merge_requests_path(public_project), params: { scope: 'all', search: 'test' } + end end - it_behaves_like "observability csp policy", described_class do - let(:tested_path) do - project_merge_request_path(project, merge_request) + it_behaves_like 'rate limited endpoint', rate_limit_key: :search_rate_limit_unauthenticated do + def request + get project_merge_requests_path(public_project), params: { scope: 'all', search: 'test' } end end end describe 'GET #discussions' do let_it_be(:discussion) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } - let_it_be(:discussion_reply) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: discussion) } + let_it_be(:discussion_reply) do + create(:discussion_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: discussion) + end + let_it_be(:state_event) { create(:resource_state_event, merge_request: merge_request) } let_it_be(:discussion_2) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } let_it_be(:discussion_3) { create(:diff_note_on_merge_request, noteable: merge_request, project: project) } @@ -60,22 +105,6 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code expect(discussions.count).to eq(1) expect(notes).to match([a_hash_including('id' => discussion_2.id.to_s)]) end - - context 'when paginated_mr_discussions is disabled' do - before do - stub_feature_flags(paginated_mr_discussions: false) - end - - it 'returns all discussions and ignores per_page param' do - get_discussions(per_page: 2) - - discussions = Gitlab::Json.parse(response.body) - notes = discussions.flat_map { |d| d['notes'] } - - expect(discussions.count).to eq(4) - expect(notes.count).to eq(5) - end - end end end @@ -91,7 +120,8 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code context 'when private project' do let_it_be(:private_project) { create(:project, :private) } - it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false, ignore_metrics: true do + it_behaves_like 'authenticates sessionless user for the request spec', 'index atom', public_resource: false, + ignore_metrics: true do let(:url) { project_merge_requests_url(private_project, format: :atom) } before do diff --git a/spec/requests/projects/ml/candidates_controller_spec.rb b/spec/requests/projects/ml/candidates_controller_spec.rb index 4a0fd1ce4f5..d3f9d92bc44 100644 --- a/spec/requests/projects/ml/candidates_controller_spec.rb +++ b/spec/requests/projects/ml/candidates_controller_spec.rb @@ -9,7 +9,6 @@ RSpec.describe Projects::Ml::CandidatesController, feature_category: :mlops do let_it_be(:candidate) { create(:ml_candidates, experiment: experiment, user: user) } let(:ff_value) { true } - let(:threshold) { 4 } let(:candidate_iid) { candidate.iid } before do @@ -40,14 +39,13 @@ RSpec.describe Projects::Ml::CandidatesController, feature_category: :mlops do expect(response).to render_template('projects/ml/candidates/show') end - # MR removing this xit https://gitlab.com/gitlab-org/gitlab/-/merge_requests/104166 - xit 'does not perform N+1 sql queries' do - control_count = ActiveRecord::QueryRecorder.new { show_candidate } + it 'does not perform N+1 sql queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { show_candidate } create_list(:ml_candidate_params, 3, candidate: candidate) create_list(:ml_candidate_metrics, 3, candidate: candidate) - expect { show_candidate }.not_to exceed_all_query_limit(control_count).with_threshold(threshold) + expect { show_candidate }.not_to exceed_all_query_limit(control_count) end context 'when candidate does not exist' do diff --git a/spec/requests/projects/ml/experiments_controller_spec.rb b/spec/requests/projects/ml/experiments_controller_spec.rb index f35f93b1e6c..e8b6f806251 100644 --- a/spec/requests/projects/ml/experiments_controller_spec.rb +++ b/spec/requests/projects/ml/experiments_controller_spec.rb @@ -68,24 +68,59 @@ RSpec.describe Projects::Ml::ExperimentsController, feature_category: :mlops do describe 'GET show' do let(:params) { basic_params.merge(id: experiment.iid) } - before do + it 'renders the template' do show_experiment - end - it 'renders the template' do expect(response).to render_template('projects/ml/experiments/show') end - # MR removing this xit https://gitlab.com/gitlab-org/gitlab/-/merge_requests/104166 - xit 'does not perform N+1 sql queries' do - control_count = ActiveRecord::QueryRecorder.new { show_experiment } + describe 'pagination' do + let_it_be(:candidates) { create_list(:ml_candidates, 5, experiment: experiment) } + + before do + stub_const("Projects::Ml::ExperimentsController::MAX_CANDIDATES_PER_PAGE", 2) + candidates + + show_experiment + end + + context 'when out of bounds' do + let(:params) { basic_params.merge(id: experiment.iid, page: 10000) } + + it 'redirects to last page' do + last_page = (experiment.candidates.size + 1) / 2 + + expect(response).to redirect_to(project_ml_experiment_path(project, experiment.iid, page: last_page)) + end + end + + context 'when bad page' do + let(:params) { basic_params.merge(id: experiment.iid, page: 's') } + + it 'uses first page' do + expect(assigns(:pagination)).to include( + page: 1, + is_last_page: false, + per_page: 2, + total_items: experiment.candidates&.size + ) + end + end + end + + it 'does not perform N+1 sql queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { show_experiment } create_list(:ml_candidates, 2, :with_metrics_and_params, experiment: experiment) - expect { show_experiment }.not_to exceed_all_query_limit(control_count).with_threshold(threshold) + expect { show_experiment }.not_to exceed_all_query_limit(control_count) end - it_behaves_like '404 if feature flag disabled' + it_behaves_like '404 if feature flag disabled' do + before do + show_experiment + end + end end private -- cgit v1.2.3