diff options
Diffstat (limited to 'spec')
13 files changed, 132 insertions, 97 deletions
diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index f4fd78730b6..4de724fd6d6 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -213,7 +213,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, latest_diff: true, only_context_commits: false, - merge_conflicts_in_diff: true, merge_ref_head_diff: false } end @@ -281,7 +280,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, latest_diff: true, only_context_commits: false, - merge_conflicts_in_diff: true, merge_ref_head_diff: nil } end @@ -303,33 +301,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: merge_request.diff_head_commit, latest_diff: nil, only_context_commits: false, - merge_conflicts_in_diff: true, - merge_ref_head_diff: nil - } - end - end - end - - context 'when display_merge_conflicts_in_diff is disabled' do - subject { go } - - before do - stub_feature_flags(display_merge_conflicts_in_diff: false) - end - - it_behaves_like 'serializes diffs metadata with expected arguments' do - let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiff } - let(:expected_options) do - { - merge_request: merge_request, - merge_request_diff: merge_request.merge_request_diff, - merge_request_diffs: merge_request.merge_request_diffs, - start_version: nil, - start_sha: nil, - commit: nil, - latest_diff: true, - only_context_commits: false, - merge_conflicts_in_diff: false, merge_ref_head_diff: nil } end @@ -498,7 +469,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do commit: nil, diff_view: :inline, merge_ref_head_diff: nil, - merge_conflicts_in_diff: true, pagination_data: { total_pages: nil }.merge(pagination_data) @@ -617,21 +587,6 @@ RSpec.describe Projects::MergeRequests::DiffsController do it_behaves_like 'successful request' end - context 'when display_merge_conflicts_in_diff is disabled' do - before do - stub_feature_flags(display_merge_conflicts_in_diff: false) - end - - subject { go } - - it_behaves_like 'serializes diffs with expected arguments' do - let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch } - let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_conflicts_in_diff: false) } - end - - it_behaves_like 'successful request' - end - it_behaves_like 'forked project with submodules' it_behaves_like 'cached diff collection' diff --git a/spec/finders/ci/runners_finder_spec.rb b/spec/finders/ci/runners_finder_spec.rb index a8ef99eeaec..5b0c88e711d 100644 --- a/spec/finders/ci/runners_finder_spec.rb +++ b/spec/finders/ci/runners_finder_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::RunnersFinder do +RSpec.describe Ci::RunnersFinder, feature_category: :runner_fleet do context 'admin' do let_it_be(:admin) { create(:user, :admin) } diff --git a/spec/lib/gitlab/observability_spec.rb b/spec/lib/gitlab/observability_spec.rb index 2b1d22d9019..8068d2f8ec9 100644 --- a/spec/lib/gitlab/observability_spec.rb +++ b/spec/lib/gitlab/observability_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' RSpec.describe Gitlab::Observability do describe '.observability_url' do @@ -30,4 +30,39 @@ RSpec.describe Gitlab::Observability do it { is_expected.to eq(observe_url) } end end + + describe '.observability_enabled?' do + let_it_be(:group) { build(:user) } + let_it_be(:user) { build(:group) } + + subject do + described_class.observability_enabled?(user, group) + end + + it 'checks if read_observability ability is allowed for the given user and group' do + allow(Ability).to receive(:allowed?).and_return(true) + + subject + + expect(Ability).to have_received(:allowed?).with(user, :read_observability, group) + end + + it 'returns true if the read_observability ability is allowed' do + allow(Ability).to receive(:allowed?).and_return(true) + + expect(subject).to eq(true) + end + + it 'returns false if the read_observability ability is not allowed' do + allow(Ability).to receive(:allowed?).and_return(false) + + expect(subject).to eq(false) + end + + it 'returns false if observability url is missing' do + allow(described_class).to receive(:observability_url).and_return("") + + expect(subject).to eq(false) + end + end end diff --git a/spec/lib/sidebars/groups/menus/observability_menu_spec.rb b/spec/lib/sidebars/groups/menus/observability_menu_spec.rb index 3a91b1aea2f..5b993cd6f28 100644 --- a/spec/lib/sidebars/groups/menus/observability_menu_spec.rb +++ b/spec/lib/sidebars/groups/menus/observability_menu_spec.rb @@ -20,23 +20,25 @@ RSpec.describe Sidebars::Groups::Menus::ObservabilityMenu do allow(menu).to receive(:can?).and_call_original end - context 'when user can :read_observability' do + context 'when observability is enabled' do before do - allow(menu).to receive(:can?).with(user, :read_observability, group).and_return(true) + allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(true) end it 'returns true' do expect(menu.render?).to eq true + expect(Gitlab::Observability).to have_received(:observability_enabled?).with(user, group) end end - context 'when user cannot :read_observability' do + context 'when observability is disabled' do before do - allow(menu).to receive(:can?).with(user, :read_observability, group).and_return(false) + allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(false) end it 'returns false' do expect(menu.render?).to eq false + expect(Gitlab::Observability).to have_received(:observability_enabled?).with(user, group) end end end diff --git a/spec/requests/projects/issues_controller_spec.rb b/spec/requests/projects/issues_controller_spec.rb index a943bd6449c..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 @@ -60,7 +62,10 @@ RSpec.describe Projects::IssuesController, feature_category: :team_planning do 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) } @@ -114,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 @@ -122,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/context_commit_diffs_spec.rb b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb index 10e57970704..60223a30d28 100644 --- a/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb +++ b/spec/requests/projects/merge_requests/context_commit_diffs_spec.rb @@ -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..e299d711cb1 100644 --- a/spec/requests/projects/merge_requests/creations_spec.rb +++ b/spec/requests/projects/merge_requests/creations_spec.rb @@ -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..dfdd372f8ad 100644 --- a/spec/requests/projects/merge_requests/diffs_spec.rb +++ b/spec/requests/projects/merge_requests/diffs_spec.rb @@ -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 aa7a32ef2cf..2e16529b20d 100644 --- a/spec/requests/projects/merge_requests_controller_spec.rb +++ b/spec/requests/projects/merge_requests_controller_spec.rb @@ -8,6 +8,12 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code let_it_be(:user) { merge_request.author } describe 'GET #show' do + let_it_be(:group) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:project) { create :project, group: group } + + let_it_be(:merge_request) { create :merge_request, source_project: project, author: user } + before do login_as(user) end @@ -43,7 +49,10 @@ RSpec.describe Projects::MergeRequestsController, feature_category: :source_code 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) } @@ -113,7 +122,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/serializers/diffs_entity_spec.rb b/spec/serializers/diffs_entity_spec.rb index ba40d538ccb..aa8e7275870 100644 --- a/spec/serializers/diffs_entity_spec.rb +++ b/spec/serializers/diffs_entity_spec.rb @@ -9,13 +9,11 @@ RSpec.describe DiffsEntity do let(:request) { EntityRequest.new(project: project, current_user: user) } let(:merge_request_diffs) { merge_request.merge_request_diffs } - let(:merge_conflicts_in_diff) { false } let(:options) do { request: request, merge_request: merge_request, - merge_request_diffs: merge_request_diffs, - merge_conflicts_in_diff: merge_conflicts_in_diff + merge_request_diffs: merge_request_diffs } end @@ -101,10 +99,9 @@ RSpec.describe DiffsEntity do subject[:diff_files] end - context 'when merge_conflicts_in_diff is true' do + context 'when there are conflicts' do let(:conflict_file) { double(path: diff_files.first.new_path, conflict_type: :both_modified) } let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) } - let(:merge_conflicts_in_diff) { true } before do allow(merge_request).to receive(:cannot_be_merged?).and_return(true) diff --git a/spec/serializers/diffs_metadata_entity_spec.rb b/spec/serializers/diffs_metadata_entity_spec.rb index 04db576ffb5..415a0d8e450 100644 --- a/spec/serializers/diffs_metadata_entity_spec.rb +++ b/spec/serializers/diffs_metadata_entity_spec.rb @@ -9,7 +9,6 @@ RSpec.describe DiffsMetadataEntity do let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } let(:merge_request_diffs) { merge_request.merge_request_diffs } let(:merge_request_diff) { merge_request_diffs.last } - let(:merge_conflicts_in_diff) { false } let(:options) { {} } let(:entity) do @@ -18,8 +17,7 @@ RSpec.describe DiffsMetadataEntity do options.merge( request: request, merge_request: merge_request, - merge_request_diffs: merge_request_diffs, - merge_conflicts_in_diff: merge_conflicts_in_diff + merge_request_diffs: merge_request_diffs ) ) end @@ -67,10 +65,9 @@ RSpec.describe DiffsMetadataEntity do subject[:diff_files] end - context 'when merge_conflicts_in_diff is true' do + context 'when there are conflicts' do let(:conflict_file) { double(path: raw_diff_files.first.new_path, conflict_type: :both_modified) } let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) } - let(:merge_conflicts_in_diff) { true } before do allow(merge_request).to receive(:cannot_be_merged?).and_return(true) diff --git a/spec/serializers/paginated_diff_entity_spec.rb b/spec/serializers/paginated_diff_entity_spec.rb index 3d77beb9abc..29484d170f8 100644 --- a/spec/serializers/paginated_diff_entity_spec.rb +++ b/spec/serializers/paginated_diff_entity_spec.rb @@ -7,13 +7,11 @@ RSpec.describe PaginatedDiffEntity do let(:request) { double('request', current_user: user) } let(:merge_request) { create(:merge_request) } let(:diff_batch) { merge_request.merge_request_diff.diffs_in_batch(2, 3, diff_options: nil) } - let(:merge_conflicts_in_diff) { false } let(:options) do { request: request, merge_request: merge_request, - pagination_data: diff_batch.pagination_data, - merge_conflicts_in_diff: merge_conflicts_in_diff + pagination_data: diff_batch.pagination_data } end @@ -43,10 +41,9 @@ RSpec.describe PaginatedDiffEntity do subject[:diff_files] end - context 'when merge_conflicts_in_diff is true' do + context 'when there are conflicts' do let(:conflict_file) { double(path: diff_files.first.new_path, conflict_type: :both_modified) } let(:conflicts) { double(conflicts: double(files: [conflict_file]), can_be_resolved_in_ui?: false) } - let(:merge_conflicts_in_diff) { true } before do allow(merge_request).to receive(:cannot_be_merged?).and_return(true) diff --git a/spec/support/shared_examples/observability/csp_shared_examples.rb b/spec/support/shared_examples/observability/csp_shared_examples.rb index 868d7023d14..0cd211f69eb 100644 --- a/spec/support/shared_examples/observability/csp_shared_examples.rb +++ b/spec/support/shared_examples/observability/csp_shared_examples.rb @@ -2,9 +2,17 @@ # Verifies that the proper CSP rules for Observabilty UI are applied to a given controller/path # -# The path under test needs to be declared with `let(:tested_path) { .. }` in the context including this example +# It requires the following variables declared in the context including this example: +# +# - `tested_path`: the path under test +# - `user`: the test user +# - `group`: the test group +# +# e.g. # # ``` +# let_it_be(:group) { create(:group) } +# let_it_be(:user) { create(:user) } # it_behaves_like "observability csp policy" do # let(:tested_path) { ....the path under test } # end @@ -33,6 +41,9 @@ RSpec.shared_examples 'observability csp policy' do |controller_class = describe before do setup_csp_for_controller(controller_class, csp, any_time: true) + group.add_developer(user) + login_as(user) + allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(true) end subject do @@ -48,6 +59,40 @@ RSpec.shared_examples 'observability csp policy' do |controller_class = describe end end + context 'when observability is disabled' do + let(:csp) do + ActionDispatch::ContentSecurityPolicy.new do |p| + p.frame_src 'https://something.test' + end + end + + before do + allow(Gitlab::Observability).to receive(:observability_enabled?).and_return(false) + end + + it 'does not add observability urls to the csp header' do + expect(subject).to include("frame-src https://something.test") + expect(subject).not_to include("#{observability_url} #{signin_url} #{oauth_url}") + end + end + + context 'when checking if observability is enabled' do + let(:csp) do + ActionDispatch::ContentSecurityPolicy.new do |p| + p.frame_src 'https://something.test' + end + end + + it 'check access for a given user and group' do + allow(Gitlab::Observability).to receive(:observability_enabled?) + + get tested_path + + expect(Gitlab::Observability).to have_received(:observability_enabled?) + .with(user, group).at_least(:once) + end + end + context 'when frame-src exists in the CSP config' do let(:csp) do ActionDispatch::ContentSecurityPolicy.new do |p| |