diff options
Diffstat (limited to 'spec/controllers')
53 files changed, 2037 insertions, 365 deletions
diff --git a/spec/controllers/abuse_reports_controller_spec.rb b/spec/controllers/abuse_reports_controller_spec.rb index e360ab68cf2..e573ef4be49 100644 --- a/spec/controllers/abuse_reports_controller_spec.rb +++ b/spec/controllers/abuse_reports_controller_spec.rb @@ -49,7 +49,9 @@ describe AbuseReportsController do end it 'calls notify' do - expect_any_instance_of(AbuseReport).to receive(:notify) + expect_next_instance_of(AbuseReport) do |instance| + expect(instance).to receive(:notify) + end post :create, params: { abuse_report: attrs } end diff --git a/spec/controllers/admin/clusters_controller_spec.rb b/spec/controllers/admin/clusters_controller_spec.rb index 233710b9fc3..ebae931764d 100644 --- a/spec/controllers/admin/clusters_controller_spec.rb +++ b/spec/controllers/admin/clusters_controller_spec.rb @@ -73,7 +73,7 @@ describe Admin::ClustersController do end describe 'GET #new' do - def get_new(provider: 'gke') + def get_new(provider: 'gcp') get :new, params: { provider: provider } end @@ -227,16 +227,17 @@ describe Admin::ClustersController do describe 'security' do before do - allow_any_instance_of(described_class) - .to receive(:token_in_session).and_return('token') - allow_any_instance_of(described_class) - .to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s) - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:projects_zones_clusters_create) do - OpenStruct.new( - self_link: 'projects/gcp-project-12345/zones/us-central1-a/operations/ope-123', - status: 'RUNNING' - ) + allow_next_instance_of(described_class) do |instance| + allow(instance).to receive(:token_in_session).and_return('token') + allow(instance).to receive(:expires_at_in_session).and_return(1.hour.since.to_i.to_s) + end + allow_next_instance_of(GoogleApi::CloudPlatform::Client) do |instance| + allow(instance).to receive(:projects_zones_clusters_create) do + OpenStruct.new( + self_link: 'projects/gcp-project-12345/zones/us-central1-a/operations/ope-123', + status: 'RUNNING' + ) + end end allow(WaitForClusterCreationWorker).to receive(:perform_in).and_return(nil) @@ -248,6 +249,69 @@ describe Admin::ClustersController do end end + describe 'POST #create_aws' do + let(:params) do + { + cluster: { + name: 'new-cluster', + provider_aws_attributes: { + key_name: 'key', + role_arn: 'arn:role', + region: 'region', + vpc_id: 'vpc', + instance_type: 'instance type', + num_nodes: 3, + security_group_id: 'security group', + subnet_ids: %w(subnet1 subnet2) + } + } + } + end + + def post_create_aws + post :create_aws, params: params + end + + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { post_create_aws }.to change { Clusters::Cluster.count } + .and change { Clusters::Providers::Aws.count } + + cluster = Clusters::Cluster.instance_type.first + + expect(response.status).to eq(201) + expect(response.location).to eq(admin_cluster_path(cluster)) + expect(cluster).to be_aws + expect(cluster).to be_kubernetes + end + + context 'params are invalid' do + let(:params) do + { + cluster: { name: '' } + } + end + + it 'does not create a cluster' do + expect { post_create_aws }.not_to change { Clusters::Cluster.count } + + expect(response.status).to eq(422) + expect(response.content_type).to eq('application/json') + expect(response.body).to include('is invalid') + end + end + + describe 'security' do + before do + allow(WaitForClusterCreationWorker).to receive(:perform_in) + end + + it { expect { post_create_aws }.to be_allowed_for(:admin) } + it { expect { post_create_aws }.to be_denied_for(:user) } + it { expect { post_create_aws }.to be_denied_for(:external) } + end + end + describe 'POST #create_user' do let(:params) do { @@ -318,6 +382,72 @@ describe Admin::ClustersController do end end + describe 'POST authorize AWS role for EKS cluster' do + let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } + let(:role_external_id) { '12345' } + + let(:params) do + { + cluster: { + role_arn: role_arn, + role_external_id: role_external_id + } + } + end + + def go + post :authorize_aws_role, params: params + end + + it 'creates an Aws::Role record' do + expect { go }.to change { Aws::Role.count } + + expect(response.status).to eq 201 + + role = Aws::Role.last + expect(role.user).to eq admin + expect(role.role_arn).to eq role_arn + expect(role.role_external_id).to eq role_external_id + end + + context 'role cannot be created' do + let(:role_arn) { 'invalid-role' } + + it 'does not create a record' do + expect { go }.not_to change { Aws::Role.count } + + expect(response.status).to eq 422 + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + end + + describe 'DELETE revoke AWS role for EKS cluster' do + let!(:role) { create(:aws_role, user: admin) } + + def go + delete :revoke_aws_role + end + + it 'deletes the Aws::Role record' do + expect { go }.to change { Aws::Role.count } + + expect(response.status).to eq 204 + expect(admin.reload_aws_role).to be_nil + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + end + describe 'GET #cluster_status' do let(:cluster) { create(:cluster, :providing_by_gcp, :instance) } @@ -338,7 +468,9 @@ describe Admin::ClustersController do end it 'invokes schedule_status_update on each application' do - expect_any_instance_of(Clusters::Applications::Ingress).to receive(:schedule_status_update) + expect_next_instance_of(Clusters::Applications::Ingress) do |instance| + expect(instance).to receive(:schedule_status_update) + end get_cluster_status end diff --git a/spec/controllers/admin/identities_controller_spec.rb b/spec/controllers/admin/identities_controller_spec.rb index 68695afdb61..256aafe09f8 100644 --- a/spec/controllers/admin/identities_controller_spec.rb +++ b/spec/controllers/admin/identities_controller_spec.rb @@ -13,7 +13,9 @@ describe Admin::IdentitiesController do let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') } it 'repairs ldap blocks' do - expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute) + expect_next_instance_of(RepairLdapBlockedUserService) do |instance| + expect(instance).to receive(:execute) + end put :update, params: { user_id: user.username, id: user.ldap_identity.id, identity: { provider: 'twitter' } } end @@ -23,7 +25,9 @@ describe Admin::IdentitiesController do let(:user) { create(:omniauth_user, provider: 'ldapmain', extern_uid: 'uid=myuser,ou=people,dc=example,dc=com') } it 'repairs ldap blocks' do - expect_any_instance_of(RepairLdapBlockedUserService).to receive(:execute) + expect_next_instance_of(RepairLdapBlockedUserService) do |instance| + expect(instance).to receive(:execute) + end delete :destroy, params: { user_id: user.username, id: user.ldap_identity.id } end diff --git a/spec/controllers/admin/spam_logs_controller_spec.rb b/spec/controllers/admin/spam_logs_controller_spec.rb index 3bc49023357..baf4216dcde 100644 --- a/spec/controllers/admin/spam_logs_controller_spec.rb +++ b/spec/controllers/admin/spam_logs_controller_spec.rb @@ -27,7 +27,7 @@ describe Admin::SpamLogsController do expect(response).to have_gitlab_http_status(200) end - it 'removes user and his spam logs when removing the user' do + it 'removes user and his spam logs when removing the user', :sidekiq_might_not_need_inline do delete :destroy, params: { id: first_spam.id, remove_user: true } expect(flash[:notice]).to eq "User #{user.username} was successfully removed." @@ -39,7 +39,9 @@ describe Admin::SpamLogsController do describe '#mark_as_ham' do before do - allow_any_instance_of(AkismetService).to receive(:submit_ham).and_return(true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive(:submit_ham).and_return(true) + end end it 'submits the log as ham' do post :mark_as_ham, params: { id: first_spam.id } diff --git a/spec/controllers/admin/users_controller_spec.rb b/spec/controllers/admin/users_controller_spec.rb index afe21c8b34a..50ba7418d2c 100644 --- a/spec/controllers/admin/users_controller_spec.rb +++ b/spec/controllers/admin/users_controller_spec.rb @@ -35,7 +35,7 @@ describe Admin::UsersController do end end - describe 'DELETE #user with projects' do + describe 'DELETE #user with projects', :sidekiq_might_not_need_inline do let(:project) { create(:project, namespace: user.namespace) } let!(:issue) { create(:issue, author: user) } diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 993e4020a75..4a10e7b5325 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -96,30 +96,14 @@ describe ApplicationController do request.path = '/-/peek' end - # TODO: - # remove line below once `privacy_policy_update_callout` - # feature flag is removed and `gon` reverts back to - # to not setting any variables. - if Gitlab.ee? - it_behaves_like 'setting gon variables' - else - it_behaves_like 'not setting gon variables' - end + it_behaves_like 'not setting gon variables' end end context 'with json format' do let(:format) { :json } - # TODO: - # remove line below once `privacy_policy_update_callout` - # feature flag is removed and `gon` reverts back to - # to not setting any variables. - if Gitlab.ee? - it_behaves_like 'setting gon variables' - else - it_behaves_like 'not setting gon variables' - end + it_behaves_like 'not setting gon variables' end end @@ -655,7 +639,7 @@ describe ApplicationController do context 'given a 422 error page' do controller do def index - render 'errors/omniauth_error', layout: 'errors', status: 422 + render 'errors/omniauth_error', layout: 'errors', status: :unprocessable_entity end end @@ -669,7 +653,7 @@ describe ApplicationController do context 'given a 500 error page' do controller do def index - render 'errors/omniauth_error', layout: 'errors', status: 500 + render 'errors/omniauth_error', layout: 'errors', status: :internal_server_error end end @@ -683,7 +667,7 @@ describe ApplicationController do context 'given a 200 success page' do controller do def index - render 'errors/omniauth_error', layout: 'errors', status: 200 + render 'errors/omniauth_error', layout: 'errors', status: :ok end end @@ -843,7 +827,7 @@ describe ApplicationController do end end - describe '#require_role' do + describe '#required_signup_info' do controller(described_class) do def index; end end @@ -852,7 +836,7 @@ describe ApplicationController do let(:experiment_enabled) { true } before do - stub_experiment(signup_flow: experiment_enabled) + stub_experiment_for_user(signup_flow: experiment_enabled) end context 'experiment enabled and user with required role' do @@ -865,7 +849,7 @@ describe ApplicationController do it { is_expected.to redirect_to users_sign_up_welcome_path } end - context 'experiment enabled and user without a role' do + context 'experiment enabled and user without a required role' do before do sign_in(user) get :index @@ -874,7 +858,7 @@ describe ApplicationController do it { is_expected.not_to redirect_to users_sign_up_welcome_path } end - context 'experiment disabled and user with required role' do + context 'experiment disabled' do let(:experiment_enabled) { false } before do diff --git a/spec/controllers/concerns/confirm_email_warning_spec.rb b/spec/controllers/concerns/confirm_email_warning_spec.rb index 0c598a360af..25429cdd149 100644 --- a/spec/controllers/concerns/confirm_email_warning_spec.rb +++ b/spec/controllers/concerns/confirm_email_warning_spec.rb @@ -19,7 +19,7 @@ describe ConfirmEmailWarning do RSpec::Matchers.define :set_confirm_warning_for do |email| match do |response| - expect(response).to set_flash.now[:warning].to include("Please check your email (#{email}) to verify that you own this address.") + expect(response).to set_flash.now[:warning].to include("Please check your email (#{email}) to verify that you own this address and unlock the power of CI/CD.") end end diff --git a/spec/controllers/concerns/metrics_dashboard_spec.rb b/spec/controllers/concerns/metrics_dashboard_spec.rb index a71e34fd1ca..ff2b6fbb8ec 100644 --- a/spec/controllers/concerns/metrics_dashboard_spec.rb +++ b/spec/controllers/concerns/metrics_dashboard_spec.rb @@ -3,9 +3,11 @@ require 'spec_helper' describe MetricsDashboard do + include MetricsDashboardHelpers + describe 'GET #metrics_dashboard' do let_it_be(:user) { create(:user) } - let_it_be(:project) { create(:project, :repository) } + let_it_be(:project) { project_with_dashboard('.gitlab/dashboards/test.yml') } let_it_be(:environment) { create(:environment, project: project) } before do @@ -31,11 +33,13 @@ describe MetricsDashboard do end context 'when params are provided' do + let(:params) { { environment: environment } } + before do allow(controller).to receive(:project).and_return(project) allow(controller) .to receive(:metrics_dashboard_params) - .and_return(environment: environment) + .and_return(params) end it 'returns the specified dashboard' do @@ -43,6 +47,15 @@ describe MetricsDashboard do expect(json_response).not_to have_key('all_dashboards') end + context 'when the params are in an alternate format' do + let(:params) { ActionController::Parameters.new({ environment: environment }).permit! } + + it 'returns the specified dashboard' do + expect(json_response['dashboard']['dashboard']).to eq('Environment metrics') + expect(json_response).not_to have_key('all_dashboards') + end + end + context 'when parameters are provided and the list of all dashboards is required' do before do allow(controller).to receive(:include_all_dashboards?).and_return(true) @@ -52,6 +65,36 @@ describe MetricsDashboard do expect(json_response['dashboard']['dashboard']).to eq('Environment metrics') expect(json_response).to have_key('all_dashboards') end + + context 'in all_dashboard list' do + let(:system_dashboard) { json_response['all_dashboards'].find { |dashboard| dashboard["system_dashboard"] == true } } + let(:project_dashboard) { json_response['all_dashboards'].find { |dashboard| dashboard["system_dashboard"] == false } } + + it 'includes project_blob_path only for project dashboards' do + expect(system_dashboard['project_blob_path']).to be_nil + expect(project_dashboard['project_blob_path']).to eq("/#{project.namespace.path}/#{project.name}/blob/master/.gitlab/dashboards/test.yml") + end + + describe 'project permissions' do + using RSpec::Parameterized::TableSyntax + + where(:can_collaborate, :system_can_edit, :project_can_edit) do + false | false | false + true | false | true + end + + with_them do + before do + allow(controller).to receive(:can_collaborate_with_project?).and_return(can_collaborate) + end + + it "sets can_edit appropriately" do + expect(system_dashboard["can_edit"]).to eq(system_can_edit) + expect(project_dashboard["can_edit"]).to eq(project_can_edit) + end + end + end + end end end end diff --git a/spec/controllers/concerns/redirects_for_missing_path_on_tree_spec.rb b/spec/controllers/concerns/redirects_for_missing_path_on_tree_spec.rb new file mode 100644 index 00000000000..903100ba93f --- /dev/null +++ b/spec/controllers/concerns/redirects_for_missing_path_on_tree_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe RedirectsForMissingPathOnTree, type: :controller do + controller(ActionController::Base) do + include Gitlab::Routing.url_helpers + include RedirectsForMissingPathOnTree + + def fake + redirect_to_tree_root_for_missing_path(Project.find(params[:project_id]), params[:ref], params[:file_path]) + end + end + + let(:project) { create(:project) } + + before do + routes.draw { get 'fake' => 'anonymous#fake' } + end + + describe '#redirect_to_root_path' do + it 'redirects to the tree path with a notice' do + long_file_path = ('a/b/' * 30) + 'foo.txt' + truncated_file_path = '...b/' + ('a/b/' * 12) + 'foo.txt' + expected_message = "\"#{truncated_file_path}\" did not exist on \"theref\"" + + get :fake, params: { project_id: project.id, ref: 'theref', file_path: long_file_path } + + expect(response).to redirect_to project_tree_path(project, 'theref') + expect(response.flash[:notice]).to eq(expected_message) + end + end +end diff --git a/spec/controllers/concerns/renders_commits_spec.rb b/spec/controllers/concerns/renders_commits_spec.rb new file mode 100644 index 00000000000..79350847383 --- /dev/null +++ b/spec/controllers/concerns/renders_commits_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe RendersCommits do + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + let_it_be(:user) { create(:user) } + + controller(ApplicationController) do + # `described_class` is not available in this context + include RendersCommits # rubocop:disable RSpec/DescribedClass + + def index + @merge_request = MergeRequest.find(params[:id]) + @commits = set_commits_for_rendering( + @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch), + commits_count: @merge_request.commits_count + ) + + render json: { html: view_to_html_string('projects/merge_requests/_commits') } + end + end + + before do + sign_in(user) + end + + def go + get :index, params: { id: merge_request.id } + end + + it 'sets instance variables for counts' do + stub_const("MergeRequestDiff::COMMITS_SAFE_SIZE", 10) + + go + + expect(assigns[:total_commit_count]).to eq(29) + expect(assigns[:hidden_commit_count]).to eq(19) + expect(assigns[:commits].size).to eq(10) + end + + context 'rendering commits' do + render_views + + it 'avoids N + 1' do + stub_const("MergeRequestDiff::COMMITS_SAFE_SIZE", 5) + + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + go + end.count + + stub_const("MergeRequestDiff::COMMITS_SAFE_SIZE", 15) + + expect do + go + end.not_to exceed_all_query_limit(control_count) + end + end +end diff --git a/spec/controllers/concerns/sourcegraph_gon_spec.rb b/spec/controllers/concerns/sourcegraph_gon_spec.rb new file mode 100644 index 00000000000..4fb7e37d148 --- /dev/null +++ b/spec/controllers/concerns/sourcegraph_gon_spec.rb @@ -0,0 +1,118 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SourcegraphGon do + let_it_be(:enabled_user) { create(:user, sourcegraph_enabled: true) } + let_it_be(:disabled_user) { create(:user, sourcegraph_enabled: false) } + let_it_be(:public_project) { create(:project, :public) } + let_it_be(:internal_project) { create(:project, :internal) } + + let(:sourcegraph_url) { 'http://sourcegraph.gitlab.com' } + let(:feature_enabled) { true } + let(:sourcegraph_enabled) { true } + let(:sourcegraph_public_only) { false } + let(:format) { :html } + let(:user) { enabled_user } + let(:project) { internal_project } + + controller(ApplicationController) do + include SourcegraphGon # rubocop:disable RSpec/DescribedClass + + def index + head :ok + end + end + + before do + Feature.get(:sourcegraph).enable(feature_enabled) + + stub_application_setting(sourcegraph_url: sourcegraph_url, sourcegraph_enabled: sourcegraph_enabled, sourcegraph_public_only: sourcegraph_public_only) + + allow(controller).to receive(:project).and_return(project) + + Gon.clear + + sign_in user if user + end + + after do + Feature.get(:sourcegraph).disable + end + + subject do + get :index, format: format + + Gon.sourcegraph + end + + shared_examples 'enabled' do + it { is_expected.to eq({ url: sourcegraph_url }) } + end + + shared_examples 'disabled' do + it { is_expected.to be_nil } + end + + context 'with feature enabled, application enabled, and user enabled' do + it_behaves_like 'enabled' + end + + context 'with feature enabled for specific project' do + let(:feature_enabled) { project } + + it_behaves_like 'enabled' + end + + context 'with feature enabled for different project' do + let(:feature_enabled) { create(:project) } + + it_behaves_like 'disabled' + end + + context 'with feature disabled' do + let(:feature_enabled) { false } + + it_behaves_like 'disabled' + end + + context 'with admin settings disabled' do + let(:sourcegraph_enabled) { false } + + it_behaves_like 'disabled' + end + + context 'with public only' do + let(:sourcegraph_public_only) { true } + + context 'with internal project' do + let(:project) { internal_project } + + it_behaves_like 'disabled' + end + + context 'with public project' do + let(:project) { public_project } + + it_behaves_like 'enabled' + end + end + + context 'with user disabled' do + let(:user) { disabled_user } + + it_behaves_like 'disabled' + end + + context 'with no user' do + let(:user) { nil } + + it_behaves_like 'disabled' + end + + context 'with non-html format' do + let(:format) { :json } + + it_behaves_like 'disabled' + end +end diff --git a/spec/controllers/google_api/authorizations_controller_spec.rb b/spec/controllers/google_api/authorizations_controller_spec.rb index 940bf9c6828..4d200140f16 100644 --- a/spec/controllers/google_api/authorizations_controller_spec.rb +++ b/spec/controllers/google_api/authorizations_controller_spec.rb @@ -13,8 +13,9 @@ describe GoogleApi::AuthorizationsController do before do sign_in(user) - allow_any_instance_of(GoogleApi::CloudPlatform::Client) - .to receive(:get_token).and_return([token, expires_at]) + allow_next_instance_of(GoogleApi::CloudPlatform::Client) do |instance| + allow(instance).to receive(:get_token).and_return([token, expires_at]) + end end shared_examples_for 'access denied' do diff --git a/spec/controllers/groups/clusters_controller_spec.rb b/spec/controllers/groups/clusters_controller_spec.rb index 51a6dcca640..d027405703b 100644 --- a/spec/controllers/groups/clusters_controller_spec.rb +++ b/spec/controllers/groups/clusters_controller_spec.rb @@ -85,7 +85,7 @@ describe Groups::ClustersController do end describe 'GET new' do - def go(provider: 'gke') + def go(provider: 'gcp') get :new, params: { group_id: group, provider: provider } end @@ -372,6 +372,150 @@ describe Groups::ClustersController do end end + describe 'POST #create_aws' do + let(:params) do + { + cluster: { + name: 'new-cluster', + provider_aws_attributes: { + key_name: 'key', + role_arn: 'arn:role', + region: 'region', + vpc_id: 'vpc', + instance_type: 'instance type', + num_nodes: 3, + security_group_id: 'security group', + subnet_ids: %w(subnet1 subnet2) + } + } + } + end + + def post_create_aws + post :create_aws, params: params.merge(group_id: group) + end + + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { post_create_aws }.to change { Clusters::Cluster.count } + .and change { Clusters::Providers::Aws.count } + + cluster = group.clusters.first + + expect(response.status).to eq(201) + expect(response.location).to eq(group_cluster_path(group, cluster)) + expect(cluster).to be_aws + expect(cluster).to be_kubernetes + end + + context 'params are invalid' do + let(:params) do + { + cluster: { name: '' } + } + end + + it 'does not create a cluster' do + expect { post_create_aws }.not_to change { Clusters::Cluster.count } + + expect(response.status).to eq(422) + expect(response.content_type).to eq('application/json') + expect(response.body).to include('is invalid') + end + end + + describe 'security' do + before do + allow(WaitForClusterCreationWorker).to receive(:perform_in) + end + + it { expect { post_create_aws }.to be_allowed_for(:admin) } + it { expect { post_create_aws }.to be_allowed_for(:owner).of(group) } + it { expect { post_create_aws }.to be_allowed_for(:maintainer).of(group) } + it { expect { post_create_aws }.to be_denied_for(:developer).of(group) } + it { expect { post_create_aws }.to be_denied_for(:reporter).of(group) } + it { expect { post_create_aws }.to be_denied_for(:guest).of(group) } + it { expect { post_create_aws }.to be_denied_for(:user) } + it { expect { post_create_aws }.to be_denied_for(:external) } + end + end + + describe 'POST authorize AWS role for EKS cluster' do + let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } + let(:role_external_id) { '12345' } + + let(:params) do + { + cluster: { + role_arn: role_arn, + role_external_id: role_external_id + } + } + end + + def go + post :authorize_aws_role, params: params.merge(group_id: group) + end + + it 'creates an Aws::Role record' do + expect { go }.to change { Aws::Role.count } + + expect(response.status).to eq 201 + + role = Aws::Role.last + expect(role.user).to eq user + expect(role.role_arn).to eq role_arn + expect(role.role_external_id).to eq role_external_id + end + + context 'role cannot be created' do + let(:role_arn) { 'invalid-role' } + + it 'does not create a record' do + expect { go }.not_to change { Aws::Role.count } + + expect(response.status).to eq 422 + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(group) } + it { expect { go }.to be_allowed_for(:maintainer).of(group) } + it { expect { go }.to be_denied_for(:developer).of(group) } + it { expect { go }.to be_denied_for(:reporter).of(group) } + it { expect { go }.to be_denied_for(:guest).of(group) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + end + + describe 'DELETE revoke AWS role for EKS cluster' do + let!(:role) { create(:aws_role, user: user) } + + def go + delete :revoke_aws_role, params: { group_id: group } + end + + it 'deletes the Aws::Role record' do + expect { go }.to change { Aws::Role.count } + + expect(response.status).to eq 204 + expect(user.reload_aws_role).to be_nil + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(group) } + it { expect { go }.to be_allowed_for(:maintainer).of(group) } + it { expect { go }.to be_denied_for(:developer).of(group) } + it { expect { go }.to be_denied_for(:reporter).of(group) } + it { expect { go }.to be_denied_for(:guest).of(group) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + end + describe 'GET cluster_status' do let(:cluster) { create(:cluster, :providing_by_gcp, cluster_type: :group_type, groups: [group]) } diff --git a/spec/controllers/groups/group_links_controller_spec.rb b/spec/controllers/groups/group_links_controller_spec.rb new file mode 100644 index 00000000000..8f04822fee6 --- /dev/null +++ b/spec/controllers/groups/group_links_controller_spec.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Groups::GroupLinksController do + let(:shared_with_group) { create(:group, :private) } + let(:shared_group) { create(:group, :private) } + let(:user) { create(:user) } + + before do + sign_in(user) + end + + describe '#create' do + let(:shared_with_group_id) { shared_with_group.id } + + subject do + post(:create, + params: { group_id: shared_group, + shared_with_group_id: shared_with_group_id, + shared_group_access: GroupGroupLink.default_access }) + end + + context 'when user has correct access to both groups' do + let(:group_member) { create(:user) } + + before do + shared_with_group.add_developer(user) + shared_group.add_owner(user) + + shared_with_group.add_developer(group_member) + end + + it 'links group with selected group' do + expect { subject }.to change { shared_with_group.shared_groups.include?(shared_group) }.from(false).to(true) + end + + it 'redirects to group links page' do + subject + + expect(response).to(redirect_to(group_group_members_path(shared_group))) + end + + it 'allows access for group member' do + expect { subject }.to change { group_member.can?(:read_group, shared_group) }.from(false).to(true) + end + + context 'when shared with group id is not present' do + let(:shared_with_group_id) { nil } + + it 'redirects to group links page' do + subject + + expect(response).to(redirect_to(group_group_members_path(shared_group))) + expect(flash[:alert]).to eq('Please select a group.') + end + end + + context 'when link is not persisted in the database' do + before do + allow(::Groups::GroupLinks::CreateService).to( + receive_message_chain(:new, :execute) + .and_return({ status: :error, + http_status: 409, + message: 'error' })) + end + + it 'redirects to group links page' do + subject + + expect(response).to(redirect_to(group_group_members_path(shared_group))) + expect(flash[:alert]).to eq('error') + end + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(share_group_with_group: false) + end + + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + end + + context 'when user does not have access to the group' do + before do + shared_group.add_owner(user) + end + + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when user does not have admin access to the shared group' do + before do + shared_with_group.add_developer(user) + shared_group.add_developer(user) + end + + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + end +end diff --git a/spec/controllers/groups/milestones_controller_spec.rb b/spec/controllers/groups/milestones_controller_spec.rb index e0a3605d50a..4f4f9e5143b 100644 --- a/spec/controllers/groups/milestones_controller_spec.rb +++ b/spec/controllers/groups/milestones_controller_spec.rb @@ -314,6 +314,24 @@ describe Groups::MilestonesController do expect(controller).to set_flash[:notice].to(group_moved_message(redirect_route, group)) end + context 'with an AJAX request' do + it 'redirects to the canonical path but does not set flash message' do + get :merge_requests, params: { group_id: redirect_route.path, id: title }, xhr: true + + expect(response).to redirect_to(merge_requests_group_milestone_path(group.to_param, title)) + expect(controller).not_to set_flash[:notice] + end + end + + context 'with JSON format' do + it 'redirects to the canonical path but does not set flash message' do + get :merge_requests, params: { group_id: redirect_route.path, id: title }, format: :json + + expect(response).to redirect_to(merge_requests_group_milestone_path(group.to_param, title, format: :json)) + expect(controller).not_to set_flash[:notice] + end + end + context 'when the old group path is a substring of the scheme or host' do let(:redirect_route) { group.redirect_routes.create(path: 'http') } diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index 3c39a6468e5..2ed2b319298 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -45,7 +45,7 @@ describe GroupsController do it { is_expected.to render_template('groups/show') } - it 'assigns events for all the projects in the group' do + it 'assigns events for all the projects in the group', :sidekiq_might_not_need_inline do subject expect(assigns(:events)).to contain_exactly(event) end @@ -125,7 +125,7 @@ describe GroupsController do end context 'as json' do - it 'includes events from all projects in group and subgroups' do + it 'includes events from all projects in group and subgroups', :sidekiq_might_not_need_inline do 2.times do project = create(:project, group: group) create(:event, project: project) @@ -255,7 +255,7 @@ describe GroupsController do end end - describe 'GET #issues' do + describe 'GET #issues', :sidekiq_might_not_need_inline do let(:issue_1) { create(:issue, project: project, title: 'foo') } let(:issue_2) { create(:issue, project: project, title: 'bar') } @@ -304,7 +304,7 @@ describe GroupsController do end end - describe 'GET #merge_requests' do + describe 'GET #merge_requests', :sidekiq_might_not_need_inline do let(:merge_request_1) { create(:merge_request, source_project: project) } let(:merge_request_2) { create(:merge_request, :simple, source_project: project) } diff --git a/spec/controllers/health_controller_spec.rb b/spec/controllers/health_controller_spec.rb deleted file mode 100644 index 8a2291bccd7..00000000000 --- a/spec/controllers/health_controller_spec.rb +++ /dev/null @@ -1,134 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe HealthController do - include StubENV - - let(:token) { Gitlab::CurrentSettings.health_check_access_token } - let(:whitelisted_ip) { '127.0.0.1' } - let(:not_whitelisted_ip) { '127.0.0.2' } - - before do - allow(Settings.monitoring).to receive(:ip_whitelist).and_return([whitelisted_ip]) - stub_storage_settings({}) # Hide the broken storage - stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') - end - - describe '#readiness' do - shared_context 'endpoint responding with readiness data' do - let(:request_params) { {} } - - subject { get :readiness, params: request_params } - - it 'responds with readiness checks data' do - subject - - expect(json_response['db_check']).to contain_exactly({ 'status' => 'ok' }) - expect(json_response['cache_check']).to contain_exactly({ 'status' => 'ok' }) - expect(json_response['queues_check']).to contain_exactly({ 'status' => 'ok' }) - expect(json_response['shared_state_check']).to contain_exactly({ 'status' => 'ok' }) - expect(json_response['gitaly_check']).to contain_exactly( - { 'status' => 'ok', 'labels' => { 'shard' => 'default' } }) - end - - it 'responds with readiness checks data when a failure happens' do - allow(Gitlab::HealthChecks::Redis::RedisCheck).to receive(:readiness).and_return( - Gitlab::HealthChecks::Result.new('redis_check', false, "check error")) - - subject - - expect(json_response['cache_check']).to contain_exactly({ 'status' => 'ok' }) - expect(json_response['redis_check']).to contain_exactly( - { 'status' => 'failed', 'message' => 'check error' }) - - expect(response.status).to eq(503) - expect(response.headers['X-GitLab-Custom-Error']).to eq(1) - end - end - - context 'accessed from whitelisted ip' do - before do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip) - end - - it_behaves_like 'endpoint responding with readiness data' - end - - context 'accessed from not whitelisted ip' do - before do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip) - end - - it 'responds with resource not found' do - get :readiness - - expect(response.status).to eq(404) - end - - context 'accessed with valid token' do - context 'token passed in request header' do - before do - request.headers['TOKEN'] = token - end - - it_behaves_like 'endpoint responding with readiness data' - end - end - - context 'token passed as URL param' do - it_behaves_like 'endpoint responding with readiness data' do - let(:request_params) { { token: token } } - end - end - end - end - - describe '#liveness' do - shared_context 'endpoint responding with liveness data' do - subject { get :liveness } - - it 'responds with liveness checks data' do - subject - - expect(json_response).to eq('status' => 'ok') - end - end - - context 'accessed from whitelisted ip' do - before do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(whitelisted_ip) - end - - it_behaves_like 'endpoint responding with liveness data' - end - - context 'accessed from not whitelisted ip' do - before do - allow(Gitlab::RequestContext).to receive(:client_ip).and_return(not_whitelisted_ip) - end - - it 'responds with resource not found' do - get :liveness - - expect(response.status).to eq(404) - end - - context 'accessed with valid token' do - context 'token passed in request header' do - before do - request.headers['TOKEN'] = token - end - - it_behaves_like 'endpoint responding with liveness data' - end - - context 'token passed as URL param' do - it_behaves_like 'endpoint responding with liveness data' do - subject { get :liveness, params: { token: token } } - end - end - end - end - end -end diff --git a/spec/controllers/import/gitlab_controller_spec.rb b/spec/controllers/import/gitlab_controller_spec.rb index e465eca6c71..6a3713a1212 100644 --- a/spec/controllers/import/gitlab_controller_spec.rb +++ b/spec/controllers/import/gitlab_controller_spec.rb @@ -20,8 +20,9 @@ describe Import::GitlabController do describe "GET callback" do it "updates access token" do - allow_any_instance_of(Gitlab::GitlabImport::Client) - .to receive(:get_token).and_return(token) + allow_next_instance_of(Gitlab::GitlabImport::Client) do |instance| + allow(instance).to receive(:get_token).and_return(token) + end stub_omniauth_provider('gitlab') get :callback diff --git a/spec/controllers/import/phabricator_controller_spec.rb b/spec/controllers/import/phabricator_controller_spec.rb index 85085a8e996..a127e3cda3a 100644 --- a/spec/controllers/import/phabricator_controller_spec.rb +++ b/spec/controllers/import/phabricator_controller_spec.rb @@ -52,7 +52,7 @@ describe Import::PhabricatorController do namespace_id: current_user.namespace_id } end - it 'creates a project to import' do + it 'creates a project to import', :sidekiq_might_not_need_inline do expect_next_instance_of(Gitlab::PhabricatorImport::Importer) do |importer| expect(importer).to receive(:execute) end diff --git a/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb b/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb index 6d588c8f915..ceab9754617 100644 --- a/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/ldap/omniauth_callbacks_controller_spec.rb @@ -11,6 +11,14 @@ describe Ldap::OmniauthCallbacksController do expect(request.env['warden']).to be_authenticated end + context 'with sign in prevented' do + let(:ldap_settings) { ldap_setting_defaults.merge(prevent_ldap_sign_in: true) } + + it 'does not allow sign in' do + expect { post provider }.to raise_error(ActionController::UrlGenerationError) + end + end + it 'respects remember me checkbox' do expect do post provider, params: { remember_me: '1' } diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index 7fb3578cd0a..1d378b9b9dc 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -23,7 +23,9 @@ describe MetricsController do allow(Prometheus::Client.configuration).to receive(:multiprocess_files_dir).and_return(metrics_multiproc_dir) allow(Gitlab::Metrics).to receive(:prometheus_metrics_enabled?).and_return(true) allow(Settings.monitoring).to receive(:ip_whitelist).and_return([whitelisted_ip, whitelisted_ip_range]) - allow_any_instance_of(MetricsService).to receive(:metrics_text).and_return("prometheus_counter 1") + allow_next_instance_of(MetricsService) do |instance| + allow(instance).to receive(:metrics_text).and_return("prometheus_counter 1") + end end describe '#index' do diff --git a/spec/controllers/projects/blame_controller_spec.rb b/spec/controllers/projects/blame_controller_spec.rb index f901fd45604..dd7c0f45dc2 100644 --- a/spec/controllers/projects/blame_controller_spec.rb +++ b/spec/controllers/projects/blame_controller_spec.rb @@ -25,14 +25,25 @@ describe Projects::BlameController do }) end - context "valid file" do + context "valid branch, valid file" do let(:id) { 'master/files/ruby/popen.rb' } + it { is_expected.to respond_with(:success) } end - context "invalid file" do - let(:id) { 'master/files/ruby/missing_file.rb'} - it { expect(response).to have_gitlab_http_status(404) } + context "valid branch, invalid file" do + let(:id) { 'master/files/ruby/invalid-path.rb' } + + it 'redirects' do + expect(subject) + .to redirect_to("/#{project.full_path}/tree/master") + end + end + + context "invalid branch, valid file" do + let(:id) { 'invalid-branch/files/ruby/missing_file.rb'} + + it { is_expected.to respond_with(:not_found) } end end end diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 17964c78e8d..78599935910 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -24,26 +24,34 @@ describe Projects::BlobController do context "valid branch, valid file" do let(:id) { 'master/README.md' } + it { is_expected.to respond_with(:success) } end context "valid branch, invalid file" do let(:id) { 'master/invalid-path.rb' } - it { is_expected.to respond_with(:not_found) } + + it 'redirects' do + expect(subject) + .to redirect_to("/#{project.full_path}/tree/master") + end end context "invalid branch, valid file" do let(:id) { 'invalid-branch/README.md' } + it { is_expected.to respond_with(:not_found) } end context "binary file" do let(:id) { 'binary-encoding/encoding/binary-1.bin' } + it { is_expected.to respond_with(:success) } end context "Markdown file" do let(:id) { 'master/README.md' } + it { is_expected.to respond_with(:success) } end end @@ -104,6 +112,7 @@ describe Projects::BlobController do context 'redirect to tree' do let(:id) { 'markdown/doc' } + it 'redirects' do expect(subject) .to redirect_to("/#{project.full_path}/tree/markdown/doc") @@ -311,7 +320,7 @@ describe Projects::BlobController do default_params[:project_id] = forked_project end - it 'redirects to blob' do + it 'redirects to blob', :sidekiq_might_not_need_inline do put :update, params: default_params expect(response).to redirect_to(project_blob_path(forked_project, 'master/CHANGELOG')) @@ -319,7 +328,7 @@ describe Projects::BlobController do end context 'when editing on the original repository' do - it "redirects to forked project new merge request" do + it "redirects to forked project new merge request", :sidekiq_might_not_need_inline do default_params[:branch_name] = "fork-test-1" default_params[:create_merge_request] = 1 diff --git a/spec/controllers/projects/clusters_controller_spec.rb b/spec/controllers/projects/clusters_controller_spec.rb index e1f6d571d27..5a0512a042e 100644 --- a/spec/controllers/projects/clusters_controller_spec.rb +++ b/spec/controllers/projects/clusters_controller_spec.rb @@ -79,7 +79,7 @@ describe Projects::ClustersController do end describe 'GET new' do - def go(provider: 'gke') + def go(provider: 'gcp') get :new, params: { namespace_id: project.namespace, project_id: project, @@ -373,6 +373,150 @@ describe Projects::ClustersController do end end + describe 'POST #create_aws' do + let(:params) do + { + cluster: { + name: 'new-cluster', + provider_aws_attributes: { + key_name: 'key', + role_arn: 'arn:role', + region: 'region', + vpc_id: 'vpc', + instance_type: 'instance type', + num_nodes: 3, + security_group_id: 'security group', + subnet_ids: %w(subnet1 subnet2) + } + } + } + end + + def post_create_aws + post :create_aws, params: params.merge(namespace_id: project.namespace, project_id: project) + end + + it 'creates a new cluster' do + expect(ClusterProvisionWorker).to receive(:perform_async) + expect { post_create_aws }.to change { Clusters::Cluster.count } + .and change { Clusters::Providers::Aws.count } + + cluster = project.clusters.first + + expect(response.status).to eq(201) + expect(response.location).to eq(project_cluster_path(project, cluster)) + expect(cluster).to be_aws + expect(cluster).to be_kubernetes + end + + context 'params are invalid' do + let(:params) do + { + cluster: { name: '' } + } + end + + it 'does not create a cluster' do + expect { post_create_aws }.not_to change { Clusters::Cluster.count } + + expect(response.status).to eq(422) + expect(response.content_type).to eq('application/json') + expect(response.body).to include('is invalid') + end + end + + describe 'security' do + before do + allow(WaitForClusterCreationWorker).to receive(:perform_in) + end + + it { expect { post_create_aws }.to be_allowed_for(:admin) } + it { expect { post_create_aws }.to be_allowed_for(:owner).of(project) } + it { expect { post_create_aws }.to be_allowed_for(:maintainer).of(project) } + it { expect { post_create_aws }.to be_denied_for(:developer).of(project) } + it { expect { post_create_aws }.to be_denied_for(:reporter).of(project) } + it { expect { post_create_aws }.to be_denied_for(:guest).of(project) } + it { expect { post_create_aws }.to be_denied_for(:user) } + it { expect { post_create_aws }.to be_denied_for(:external) } + end + end + + describe 'POST authorize AWS role for EKS cluster' do + let(:role_arn) { 'arn:aws:iam::123456789012:role/role-name' } + let(:role_external_id) { '12345' } + + let(:params) do + { + cluster: { + role_arn: role_arn, + role_external_id: role_external_id + } + } + end + + def go + post :authorize_aws_role, params: params.merge(namespace_id: project.namespace, project_id: project) + end + + it 'creates an Aws::Role record' do + expect { go }.to change { Aws::Role.count } + + expect(response.status).to eq 201 + + role = Aws::Role.last + expect(role.user).to eq user + expect(role.role_arn).to eq role_arn + expect(role.role_external_id).to eq role_external_id + end + + context 'role cannot be created' do + let(:role_arn) { 'invalid-role' } + + it 'does not create a record' do + expect { go }.not_to change { Aws::Role.count } + + expect(response.status).to eq 422 + end + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:maintainer).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + end + + describe 'DELETE revoke AWS role for EKS cluster' do + let!(:role) { create(:aws_role, user: user) } + + def go + delete :revoke_aws_role, params: { namespace_id: project.namespace, project_id: project } + end + + it 'deletes the Aws::Role record' do + expect { go }.to change { Aws::Role.count } + + expect(response.status).to eq 204 + expect(user.reload_aws_role).to be_nil + end + + describe 'security' do + it { expect { go }.to be_allowed_for(:admin) } + it { expect { go }.to be_allowed_for(:owner).of(project) } + it { expect { go }.to be_allowed_for(:maintainer).of(project) } + it { expect { go }.to be_denied_for(:developer).of(project) } + it { expect { go }.to be_denied_for(:reporter).of(project) } + it { expect { go }.to be_denied_for(:guest).of(project) } + it { expect { go }.to be_denied_for(:user) } + it { expect { go }.to be_denied_for(:external) } + end + end + describe 'GET cluster_status' do let(:cluster) { create(:cluster, :providing_by_gcp, projects: [project]) } diff --git a/spec/controllers/projects/discussions_controller_spec.rb b/spec/controllers/projects/discussions_controller_spec.rb index 6ed822bbb10..d59f76c1b32 100644 --- a/spec/controllers/projects/discussions_controller_spec.rb +++ b/spec/controllers/projects/discussions_controller_spec.rb @@ -104,7 +104,9 @@ describe Projects::DiscussionsController do end it "sends notifications if all discussions are resolved" do - expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(merge_request) + expect_next_instance_of(MergeRequests::ResolvedDiscussionNotificationService) do |instance| + expect(instance).to receive(:execute).with(merge_request) + end post :resolve, params: request_params end @@ -122,8 +124,10 @@ describe Projects::DiscussionsController do end it "renders discussion with serializer" do - expect_any_instance_of(DiscussionSerializer).to receive(:represent) - .with(instance_of(Discussion), { context: instance_of(described_class), render_truncated_diff_lines: true }) + expect_next_instance_of(DiscussionSerializer) do |instance| + expect(instance).to receive(:represent) + .with(instance_of(Discussion), { context: instance_of(described_class), render_truncated_diff_lines: true }) + end post :resolve, params: request_params end @@ -193,8 +197,10 @@ describe Projects::DiscussionsController do end it "renders discussion with serializer" do - expect_any_instance_of(DiscussionSerializer).to receive(:represent) - .with(instance_of(Discussion), { context: instance_of(described_class), render_truncated_diff_lines: true }) + expect_next_instance_of(DiscussionSerializer) do |instance| + expect(instance).to receive(:represent) + .with(instance_of(Discussion), { context: instance_of(described_class), render_truncated_diff_lines: true }) + end delete :unresolve, params: request_params end diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 3fe5ff5feee..7bb956201fd 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -330,11 +330,11 @@ describe Projects::EnvironmentsController do expect(response).to redirect_to(environment_metrics_path(environment)) end - it 'redirects to empty page if no environment exists' do + it 'redirects to empty metrics page if no environment exists' do get :metrics_redirect, params: { namespace_id: project.namespace, project_id: project } expect(response).to be_ok - expect(response).to render_template 'empty' + expect(response).to render_template 'empty_metrics' end end diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb index 31868f5f717..8155d6ddafe 100644 --- a/spec/controllers/projects/error_tracking_controller_spec.rb +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -46,17 +46,6 @@ describe Projects::ErrorTrackingController do end describe 'format json' do - shared_examples 'no data' do - it 'returns no data' do - get :index, params: project_params(format: :json) - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to match_response_schema('error_tracking/index') - expect(json_response['external_url']).to be_nil - expect(json_response['errors']).to eq([]) - end - end - let(:list_issues_service) { spy(:list_issues_service) } let(:external_url) { 'http://example.com' } @@ -66,6 +55,19 @@ describe Projects::ErrorTrackingController do .and_return(list_issues_service) end + context 'no data' do + before do + expect(list_issues_service).to receive(:execute) + .and_return(status: :error, http_status: :no_content) + end + + it 'returns no data' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + context 'service result is successful' do before do expect(list_issues_service).to receive(:execute) @@ -232,8 +234,186 @@ describe Projects::ErrorTrackingController do end end + describe 'GET #issue_details' do + let_it_be(:issue_id) { 1234 } + + let(:issue_details_service) { spy(:issue_details_service) } + + let(:permitted_params) do + ActionController::Parameters.new( + { issue_id: issue_id.to_s } + ).permit! + end + + before do + expect(ErrorTracking::IssueDetailsService) + .to receive(:new).with(project, user, permitted_params) + .and_return(issue_details_service) + end + + describe 'format json' do + context 'no data' do + before do + expect(issue_details_service).to receive(:execute) + .and_return(status: :error, http_status: :no_content) + end + + it 'returns no data' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'service result is successful' do + before do + expect(issue_details_service).to receive(:execute) + .and_return(status: :success, issue: error) + end + + let(:error) { build(:detailed_error_tracking_error) } + + it 'returns an error' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/issue_detailed') + expect(json_response['error']).to eq(error.as_json) + end + end + + context 'service result is erroneous' do + let(:error_message) { 'error message' } + + context 'without http_status' do + before do + expect(issue_details_service).to receive(:execute) + .and_return(status: :error, message: error_message) + end + + it 'returns 400 with message' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(error_message) + end + end + + context 'with explicit http_status' do + let(:http_status) { :no_content } + + before do + expect(issue_details_service).to receive(:execute).and_return( + status: :error, + message: error_message, + http_status: http_status + ) + end + + it 'returns http_status with message' do + get :details, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to eq(error_message) + end + end + end + end + end + + describe 'GET #stack_trace' do + let_it_be(:issue_id) { 1234 } + + let(:issue_stack_trace_service) { spy(:issue_stack_trace_service) } + + let(:permitted_params) do + ActionController::Parameters.new( + { issue_id: issue_id.to_s } + ).permit! + end + + before do + expect(ErrorTracking::IssueLatestEventService) + .to receive(:new).with(project, user, permitted_params) + .and_return(issue_stack_trace_service) + end + + describe 'format json' do + context 'awaiting data' do + before do + expect(issue_stack_trace_service).to receive(:execute) + .and_return(status: :error, http_status: :no_content) + end + + it 'returns no data' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'service result is successful' do + before do + expect(issue_stack_trace_service).to receive(:execute) + .and_return(status: :success, latest_event: error_event) + end + + let(:error_event) { build(:error_tracking_error_event) } + + it 'returns an error' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/issue_stack_trace') + expect(json_response['error']).to eq(error_event.as_json) + end + end + + context 'service result is erroneous' do + let(:error_message) { 'error message' } + + context 'without http_status' do + before do + expect(issue_stack_trace_service).to receive(:execute) + .and_return(status: :error, message: error_message) + end + + it 'returns 400 with message' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(error_message) + end + end + + context 'with explicit http_status' do + let(:http_status) { :no_content } + + before do + expect(issue_stack_trace_service).to receive(:execute).and_return( + status: :error, + message: error_message, + http_status: http_status + ) + end + + it 'returns http_status with message' do + get :stack_trace, params: issue_params(issue_id: issue_id, format: :json) + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to eq(error_message) + end + end + end + end + end + private + def issue_params(opts = {}) + project_params.reverse_merge(opts) + end + def project_params(opts = {}) opts.reverse_merge(namespace_id: project.namespace, project_id: project) end diff --git a/spec/controllers/projects/grafana_api_controller_spec.rb b/spec/controllers/projects/grafana_api_controller_spec.rb index 352a364295b..0ef96514961 100644 --- a/spec/controllers/projects/grafana_api_controller_spec.rb +++ b/spec/controllers/projects/grafana_api_controller_spec.rb @@ -94,4 +94,75 @@ describe Projects::GrafanaApiController do end end end + + describe 'GET #metrics_dashboard' do + let(:service_result) { { status: :success, dashboard: '{}' } } + let(:params) do + { + format: :json, + embedded: true, + grafana_url: 'https://grafana.example.com', + namespace_id: project.namespace.full_path, + project_id: project.name + } + end + + before do + allow(Gitlab::Metrics::Dashboard::Finder) + .to receive(:find) + .and_return(service_result) + end + + context 'when the result is still processing' do + let(:service_result) { nil } + + it 'returns 204 no content' do + get :metrics_dashboard, params: params + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'when the result was successful' do + it 'returns the dashboard response' do + get :metrics_dashboard, params: params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({ + 'dashboard' => '{}', + 'status' => 'success' + }) + end + end + + context 'when an error has occurred' do + shared_examples_for 'error response' do |http_status| + it "returns #{http_status}" do + get :metrics_dashboard, params: params + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['status']).to eq('error') + expect(json_response['message']).to eq('error message') + end + end + + context 'with an error accessing grafana' do + let(:service_result) do + { + http_status: :service_unavailable, + status: :error, + message: 'error message' + } + end + + it_behaves_like 'error response', :service_unavailable + end + + context 'with a processing error' do + let(:service_result) { { status: :error, message: 'error message' } } + + it_behaves_like 'error response', :bad_request + end + end + end end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index d36336a9f67..8770a5ee303 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1252,7 +1252,7 @@ describe Projects::IssuesController do stub_feature_flags(create_confidential_merge_request: true) end - it 'creates a new merge request' do + it 'creates a new merge request', :sidekiq_might_not_need_inline do expect { create_merge_request }.to change(target_project.merge_requests, :count).by(1) end end diff --git a/spec/controllers/projects/jobs_controller_spec.rb b/spec/controllers/projects/jobs_controller_spec.rb index 90ccb884927..349d73f13ca 100644 --- a/spec/controllers/projects/jobs_controller_spec.rb +++ b/spec/controllers/projects/jobs_controller_spec.rb @@ -154,7 +154,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do .and_return(merge_request) end - it 'does not serialize builds in exposed stages' do + it 'does not serialize builds in exposed stages', :sidekiq_might_not_need_inline do get_show_json json_response.dig('pipeline', 'details', 'stages').tap do |stages| @@ -183,7 +183,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do context 'job is cancelable' do let(:job) { create(:ci_build, :running, pipeline: pipeline) } - it 'cancel_path is present with correct redirect' do + it 'cancel_path is present with correct redirect', :sidekiq_might_not_need_inline do expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('job/job_details') expect(json_response['cancel_path']).to include(CGI.escape(json_response['build_path'])) @@ -193,7 +193,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do context 'with web terminal' do let(:job) { create(:ci_build, :running, :with_runner_session, pipeline: pipeline) } - it 'exposes the terminal path' do + it 'exposes the terminal path', :sidekiq_might_not_need_inline do expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('job/job_details') expect(json_response['terminal_path']).to match(%r{/terminal}) @@ -268,7 +268,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do project.add_maintainer(user) # Need to be a maintianer to view cluster.path end - it 'exposes the deployment information' do + it 'exposes the deployment information', :sidekiq_might_not_need_inline do get_show_json expect(response).to have_gitlab_http_status(:ok) @@ -292,7 +292,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do sign_in(user) end - it 'user can edit runner' do + it 'user can edit runner', :sidekiq_might_not_need_inline do get_show_json expect(response).to have_gitlab_http_status(:ok) @@ -312,7 +312,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do sign_in(user) end - it 'user can not edit runner' do + it 'user can not edit runner', :sidekiq_might_not_need_inline do get_show_json expect(response).to have_gitlab_http_status(:ok) @@ -331,7 +331,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do sign_in(user) end - it 'user can not edit runner' do + it 'user can not edit runner', :sidekiq_might_not_need_inline do get_show_json expect(response).to have_gitlab_http_status(:ok) @@ -412,7 +412,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do context 'when job has trace' do let(:job) { create(:ci_build, :running, :trace_live, pipeline: pipeline) } - it "has_trace is true" do + it "has_trace is true", :sidekiq_might_not_need_inline do get_show_json expect(response).to match_response_schema('job/job_details') @@ -458,7 +458,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do create(:ci_pipeline_variable, pipeline: pipeline, key: :TRIGGER_KEY_1, value: 'TRIGGER_VALUE_1') end - context 'user is a maintainer' do + context 'user is a maintainer', :sidekiq_might_not_need_inline do before do project.add_maintainer(user) diff --git a/spec/controllers/projects/labels_controller_spec.rb b/spec/controllers/projects/labels_controller_spec.rb index ff089df37f7..aee017b211a 100644 --- a/spec/controllers/projects/labels_controller_spec.rb +++ b/spec/controllers/projects/labels_controller_spec.rb @@ -204,6 +204,24 @@ describe Projects::LabelsController do expect(response).to redirect_to(project_labels_path(project)) expect(controller).to set_flash[:notice].to(project_moved_message(redirect_route, project)) end + + context 'with an AJAX request' do + it 'redirects to the canonical path but does not set flash message' do + get :index, params: { namespace_id: project.namespace, project_id: project.to_param + 'old' }, xhr: true + + expect(response).to redirect_to(project_labels_path(project)) + expect(controller).not_to set_flash[:notice] + end + end + + context 'with JSON format' do + it 'redirects to the canonical path but does not set flash message' do + get :index, params: { namespace_id: project.namespace, project_id: project.to_param + 'old' }, format: :json + + expect(response).to redirect_to(project_labels_path(project, format: :json)) + expect(controller).not_to set_flash[:notice] + end + end end end end diff --git a/spec/controllers/projects/mattermosts_controller_spec.rb b/spec/controllers/projects/mattermosts_controller_spec.rb index 45125385d9e..64440ed585d 100644 --- a/spec/controllers/projects/mattermosts_controller_spec.rb +++ b/spec/controllers/projects/mattermosts_controller_spec.rb @@ -13,8 +13,9 @@ describe Projects::MattermostsController do describe 'GET #new' do before do - allow_any_instance_of(MattermostSlashCommandsService) - .to receive(:list_teams).and_return([]) + allow_next_instance_of(MattermostSlashCommandsService) do |instance| + allow(instance).to receive(:list_teams).and_return([]) + end end it 'accepts the request' do @@ -42,7 +43,9 @@ describe Projects::MattermostsController do context 'no request can be made to mattermost' do it 'shows the error' do - allow_any_instance_of(MattermostSlashCommandsService).to receive(:configure).and_return([false, "error message"]) + allow_next_instance_of(MattermostSlashCommandsService) do |instance| + allow(instance).to receive(:configure).and_return([false, "error message"]) + end expect(subject).to redirect_to(new_project_mattermost_url(project)) end @@ -50,7 +53,9 @@ describe Projects::MattermostsController do context 'the request is succesull' do before do - allow_any_instance_of(Mattermost::Command).to receive(:create).and_return('token') + allow_next_instance_of(Mattermost::Command) do |instance| + allow(instance).to receive(:create).and_return('token') + end end it 'redirects to the new page' do diff --git a/spec/controllers/projects/merge_requests/creations_controller_spec.rb b/spec/controllers/projects/merge_requests/creations_controller_spec.rb index ce977f26ec6..1bbb80f9904 100644 --- a/spec/controllers/projects/merge_requests/creations_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/creations_controller_spec.rb @@ -85,7 +85,9 @@ describe Projects::MergeRequests::CreationsController do describe 'GET diffs' do context 'when merge request cannot be created' do it 'does not assign diffs var' do - allow_any_instance_of(MergeRequest).to receive(:can_be_created).and_return(false) + allow_next_instance_of(MergeRequest) do |instance| + allow(instance).to receive(:can_be_created).and_return(false) + end get :diffs, params: get_diff_params.merge(format: 'json') diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 5c02e8d6461..06d9af33189 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -34,6 +34,16 @@ describe Projects::MergeRequests::DiffsController do it 'saves the preferred diff view in a cookie' do expect(response.cookies['diff_view']).to eq('parallel') end + + it 'only renders the required view', :aggregate_failures do + diff_files_without_deletions = json_response['diff_files'].reject { |f| f['deleted_file'] } + have_no_inline_diff_lines = satisfy('have no inline diff lines') do |diff_file| + !diff_file.has_key?('highlighted_diff_lines') + end + + expect(diff_files_without_deletions).to all(have_key('parallel_diff_lines')) + expect(diff_files_without_deletions).to all(have_no_inline_diff_lines) + end end context 'when the user cannot view the merge request' do @@ -76,7 +86,9 @@ describe Projects::MergeRequests::DiffsController do end it 'serializes merge request diff collection' do - expect_any_instance_of(DiffsSerializer).to receive(:represent).with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), an_instance_of(Hash)) + expect_next_instance_of(DiffsSerializer) do |instance| + expect(instance).to receive(:represent).with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), an_instance_of(Hash)) + end go end @@ -88,7 +100,9 @@ describe Projects::MergeRequests::DiffsController do end it 'serializes merge request diff collection' do - expect_any_instance_of(DiffsSerializer).to receive(:represent).with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), an_instance_of(Hash)) + expect_next_instance_of(DiffsSerializer) do |instance| + expect(instance).to receive(:represent).with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), an_instance_of(Hash)) + end go end @@ -259,7 +273,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_response["diff_files"].map { |file| file['new_path'] } + paths = json_response['diff_files'].map { |file| file['new_path'] } expect(paths).to include(existing_path) end @@ -344,6 +358,7 @@ describe Projects::MergeRequests::DiffsController do let(:expected_options) do { merge_request: merge_request, + diff_view: :inline, pagination_data: { current_page: 1, next_page: nil, @@ -367,6 +382,7 @@ describe Projects::MergeRequests::DiffsController do let(:expected_options) do { merge_request: merge_request, + diff_view: :inline, pagination_data: { current_page: 2, next_page: 3, diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index ea702792557..9f7fde2f0da 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe Projects::MergeRequestsController do include ProjectForksHelper + include Gitlab::Routing let(:project) { create(:project, :repository) } let(:user) { project.owner } @@ -206,7 +207,7 @@ describe Projects::MergeRequestsController do it 'redirects to last_page if page number is larger than number of pages' do get_merge_requests(last_page + 1) - expect(response).to redirect_to(namespace_project_merge_requests_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope])) + expect(response).to redirect_to(project_merge_requests_path(project, page: last_page, state: controller.params[:state], scope: controller.params[:scope])) end it 'redirects to specified page' do @@ -227,7 +228,7 @@ describe Projects::MergeRequestsController do host: external_host } - expect(response).to redirect_to(namespace_project_merge_requests_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope])) + expect(response).to redirect_to(project_merge_requests_path(project, page: last_page, state: controller.params[:state], scope: controller.params[:scope])) end end @@ -404,7 +405,7 @@ describe Projects::MergeRequestsController do end it 'starts the merge immediately with permitted params' do - expect(MergeWorker).to receive(:perform_async).with(merge_request.id, anything, { 'squash' => false }) + expect(MergeWorker).to receive(:perform_async).with(merge_request.id, anything, { 'sha' => merge_request.diff_head_sha }) merge_with_sha end @@ -430,10 +431,15 @@ describe Projects::MergeRequestsController do context 'when a squash commit message is passed' do let(:message) { 'My custom squash commit message' } - it 'passes the same message to SquashService' do - params = { squash: '1', squash_commit_message: message } + it 'passes the same message to SquashService', :sidekiq_might_not_need_inline do + params = { squash: '1', + squash_commit_message: message, + sha: merge_request.diff_head_sha } + expected_squash_params = { squash_commit_message: message, + sha: merge_request.diff_head_sha, + merge_request: merge_request } - expect_next_instance_of(MergeRequests::SquashService, project, user, params.merge(merge_request: merge_request)) do |squash_service| + expect_next_instance_of(MergeRequests::SquashService, project, user, expected_squash_params) do |squash_service| expect(squash_service).to receive(:execute).and_return({ status: :success, squash_sha: SecureRandom.hex(20) @@ -723,7 +729,7 @@ describe Projects::MergeRequestsController do context 'with private builds' do context 'for the target project member' do - it 'does not respond with serialized pipelines' do + it 'does not respond with serialized pipelines', :sidekiq_might_not_need_inline do expect(json_response['pipelines']).to be_empty expect(json_response['count']['all']).to eq(0) expect(response).to include_pagination_headers @@ -733,7 +739,7 @@ describe Projects::MergeRequestsController do context 'for the source project member' do let(:user) { fork_user } - it 'responds with serialized pipelines' do + it 'responds with serialized pipelines', :sidekiq_might_not_need_inline do expect(json_response['pipelines']).to be_present expect(json_response['count']['all']).to eq(1) expect(response).to include_pagination_headers @@ -749,7 +755,7 @@ describe Projects::MergeRequestsController do end context 'for the target project member' do - it 'does not respond with serialized pipelines' do + it 'does not respond with serialized pipelines', :sidekiq_might_not_need_inline do expect(json_response['pipelines']).to be_present expect(json_response['count']['all']).to eq(1) expect(response).to include_pagination_headers @@ -759,7 +765,7 @@ describe Projects::MergeRequestsController do context 'for the source project member' do let(:user) { fork_user } - it 'responds with serialized pipelines' do + it 'responds with serialized pipelines', :sidekiq_might_not_need_inline do expect(json_response['pipelines']).to be_present expect(json_response['count']['all']).to eq(1) expect(response).to include_pagination_headers @@ -770,6 +776,172 @@ describe Projects::MergeRequestsController do end end + describe 'GET exposed_artifacts' do + let(:merge_request) do + create(:merge_request, + :with_merge_request_pipeline, + target_project: project, + source_project: project) + end + + let(:pipeline) do + create(:ci_pipeline, + :success, + project: merge_request.source_project, + ref: merge_request.source_branch, + sha: merge_request.diff_head_sha) + end + + let!(:job) { create(:ci_build, pipeline: pipeline, options: job_options) } + let!(:job_metadata) { create(:ci_job_artifact, :metadata, job: job) } + + before do + allow_any_instance_of(MergeRequest) + .to receive(:find_exposed_artifacts) + .and_return(report) + + allow_any_instance_of(MergeRequest) + .to receive(:actual_head_pipeline) + .and_return(pipeline) + end + + subject do + get :exposed_artifacts, params: { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid + }, + format: :json + end + + describe 'permissions on a public project with private CI/CD' do + let(:project) { create :project, :repository, :public, :builds_private } + let(:report) { { status: :parsed, data: [] } } + let(:job_options) { {} } + + context 'while signed out' do + before do + sign_out(user) + end + + it 'responds with a 404' do + subject + + expect(response).to have_gitlab_http_status(404) + expect(response.body).to be_blank + end + end + + context 'while signed in as an unrelated user' do + before do + sign_in(create(:user)) + end + + it 'responds with a 404' do + subject + + expect(response).to have_gitlab_http_status(404) + expect(response.body).to be_blank + end + end + end + + context 'when pipeline has jobs with exposed artifacts' do + let(:job_options) do + { + artifacts: { + paths: ['ci_artifacts.txt'], + expose_as: 'Exposed artifact' + } + } + end + + context 'when fetching exposed artifacts is in progress' do + let(:report) { { status: :parsing } } + + it 'sends polling interval' do + expect(Gitlab::PollingInterval).to receive(:set_header) + + subject + end + + it 'returns 204 HTTP status' do + subject + + expect(response).to have_gitlab_http_status(:no_content) + end + end + + context 'when fetching exposed artifacts is completed' do + let(:data) do + Ci::GenerateExposedArtifactsReportService.new(project, user) + .execute(nil, pipeline) + end + + let(:report) { { status: :parsed, data: data } } + + it 'returns exposed artifacts' do + subject + + expect(response).to have_gitlab_http_status(200) + expect(json_response['status']).to eq('parsed') + expect(json_response['data']).to eq([{ + 'job_name' => 'test', + 'job_path' => project_job_path(project, job), + 'url' => file_project_job_artifacts_path(project, job, 'ci_artifacts.txt'), + 'text' => 'Exposed artifact' + }]) + end + end + + context 'when feature flag :ci_expose_arbitrary_artifacts_in_mr is disabled' do + let(:job_options) do + { + artifacts: { + paths: ['ci_artifacts.txt'], + expose_as: 'Exposed artifact' + } + } + end + let(:report) { double } + + before do + stub_feature_flags(ci_expose_arbitrary_artifacts_in_mr: false) + end + + it 'does not send polling interval' do + expect(Gitlab::PollingInterval).not_to receive(:set_header) + + subject + end + + it 'returns 204 HTTP status' do + subject + + expect(response).to have_gitlab_http_status(:no_content) + end + end + end + + context 'when pipeline does not have jobs with exposed artifacts' do + let(:report) { double } + let(:job_options) do + { + artifacts: { + paths: ['ci_artifacts.txt'] + } + } + end + + it 'returns no content' do + subject + + expect(response).to have_gitlab_http_status(204) + expect(response.body).to be_empty + end + end + end + describe 'GET test_reports' do let(:merge_request) do create(:merge_request, @@ -879,23 +1051,6 @@ describe Projects::MergeRequestsController do expect(json_response).to eq({ 'status_reason' => 'Failed to parse test reports' }) end end - - context 'when something went wrong on our system' do - let(:comparison_status) { {} } - - it 'does not send polling interval' do - expect(Gitlab::PollingInterval).not_to receive(:set_header) - - subject - end - - it 'returns 500 HTTP status' do - subject - - expect(response).to have_gitlab_http_status(:internal_server_error) - expect(json_response).to eq({ 'status_reason' => 'Unknown error' }) - end - end end describe 'POST remove_wip' do @@ -1019,13 +1174,13 @@ describe Projects::MergeRequestsController do create(:merge_request, source_project: forked, target_project: project, target_branch: 'master', head_pipeline: pipeline) end - it 'links to the environment on that project' do + it 'links to the environment on that project', :sidekiq_might_not_need_inline do get_ci_environments_status expect(json_response.first['url']).to match /#{forked.full_path}/ end - context "when environment_target is 'merge_commit'" do + context "when environment_target is 'merge_commit'", :sidekiq_might_not_need_inline do it 'returns nothing' do get_ci_environments_status(environment_target: 'merge_commit') @@ -1056,13 +1211,13 @@ describe Projects::MergeRequestsController do # we're trying to reduce the overall number of queries for this method. # set a hard limit for now. https://gitlab.com/gitlab-org/gitlab-foss/issues/52287 - it 'keeps queries in check' do + it 'keeps queries in check', :sidekiq_might_not_need_inline do control_count = ActiveRecord::QueryRecorder.new { get_ci_environments_status }.count expect(control_count).to be <= 137 end - it 'has no N+1 SQL issues for environments', :request_store, retry: 0 do + it 'has no N+1 SQL issues for environments', :request_store, :sidekiq_might_not_need_inline, retry: 0 do # First run to insert test data from lets, which does take up some 30 queries get_ci_environments_status @@ -1225,6 +1380,33 @@ describe Projects::MergeRequestsController do end end + context 'with SELECT FOR UPDATE lock' do + before do + stub_feature_flags(merge_request_rebase_nowait_lock: false) + end + + it 'executes rebase' do + allow_any_instance_of(MergeRequest).to receive(:with_lock).with(true).and_call_original + expect(RebaseWorker).to receive(:perform_async) + + post_rebase + + expect(response.status).to eq(200) + end + end + + context 'with NOWAIT lock' do + it 'returns a 409' do + allow_any_instance_of(MergeRequest).to receive(:with_lock).with('FOR UPDATE NOWAIT').and_raise(ActiveRecord::LockWaitTimeout) + expect(RebaseWorker).not_to receive(:perform_async) + + post_rebase + + expect(response.status).to eq(409) + expect(json_response['merge_error']).to eq(MergeRequest::REBASE_LOCK_MESSAGE) + end + end + context 'with a forked project' do let(:forked_project) { fork_project(project, fork_owner, repository: true) } let(:fork_owner) { create(:user) } @@ -1253,7 +1435,7 @@ describe Projects::MergeRequestsController do sign_in(fork_owner) end - it 'returns 200' do + it 'returns 200', :sidekiq_might_not_need_inline do expect_rebase_worker_for(fork_owner) post_rebase diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb index fb3dd75460a..e14686970a1 100644 --- a/spec/controllers/projects/mirrors_controller_spec.rb +++ b/spec/controllers/projects/mirrors_controller_spec.rb @@ -51,10 +51,6 @@ describe Projects::MirrorsController do sign_in(project.owner) end - around do |example| - Sidekiq::Testing.fake! { example.run } - end - context 'With valid URL for a push' do let(:remote_mirror_attributes) do { "0" => { "enabled" => "0", url: 'https://updated.example.com' } } diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 3ab191c0032..e576a3d2d40 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -518,7 +518,7 @@ describe Projects::NotesController do project.id && Project.maximum(:id).succ end - it 'returns a 404' do + it 'returns a 404', :sidekiq_might_not_need_inline do create! expect(response).to have_gitlab_http_status(404) end @@ -527,13 +527,13 @@ describe Projects::NotesController do context 'when the user has no access to the fork' do let(:fork_visibility) { Gitlab::VisibilityLevel::PRIVATE } - it 'returns a 404' do + it 'returns a 404', :sidekiq_might_not_need_inline do create! expect(response).to have_gitlab_http_status(404) end end - context 'when the user has access to the fork' do + context 'when the user has access to the fork', :sidekiq_might_not_need_inline do let!(:discussion) { forked_project.notes.find_discussion(existing_comment.discussion_id) } let(:fork_visibility) { Gitlab::VisibilityLevel::PUBLIC } @@ -785,7 +785,9 @@ describe Projects::NotesController do end it "sends notifications if all discussions are resolved" do - expect_any_instance_of(MergeRequests::ResolvedDiscussionNotificationService).to receive(:execute).with(merge_request) + expect_next_instance_of(MergeRequests::ResolvedDiscussionNotificationService) do |instance| + expect(instance).to receive(:execute).with(merge_request) + end post :resolve, params: request_params end diff --git a/spec/controllers/projects/pages_domains_controller_spec.rb b/spec/controllers/projects/pages_domains_controller_spec.rb index 032f4f1418f..3987bebb124 100644 --- a/spec/controllers/projects/pages_domains_controller_spec.rb +++ b/spec/controllers/projects/pages_domains_controller_spec.rb @@ -32,10 +32,10 @@ describe Projects::PagesDomainsController do get(:show, params: request_params.merge(id: pages_domain.domain)) end - it "displays the 'show' page" do + it "redirects to the 'edit' page" do make_request - expect(response).to have_gitlab_http_status(200) - expect(response).to render_template('show') + + expect(response).to redirect_to(edit_project_pages_domain_path(project, pages_domain.domain)) end context 'when user is developer' do @@ -69,7 +69,7 @@ describe Projects::PagesDomainsController do created_domain = PagesDomain.reorder(:id).last expect(created_domain).to be_present - expect(response).to redirect_to(project_pages_domain_path(project, created_domain)) + expect(response).to redirect_to(edit_project_pages_domain_path(project, created_domain)) end end @@ -160,7 +160,7 @@ describe Projects::PagesDomainsController do post :verify, params: params - expect(response).to redirect_to project_pages_domain_path(project, pages_domain) + expect(response).to redirect_to edit_project_pages_domain_path(project, pages_domain) expect(flash[:notice]).to eq('Successfully verified domain ownership') end @@ -169,7 +169,7 @@ describe Projects::PagesDomainsController do post :verify, params: params - expect(response).to redirect_to project_pages_domain_path(project, pages_domain) + expect(response).to redirect_to edit_project_pages_domain_path(project, pages_domain) expect(flash[:alert]).to eq('Failed to verify domain ownership') end @@ -190,6 +190,56 @@ describe Projects::PagesDomainsController do end end + describe 'DELETE #clean_certificate' do + subject do + delete(:clean_certificate, params: request_params.merge(id: pages_domain.domain)) + end + + it 'redirects to edit page' do + subject + + expect(response).to redirect_to(edit_project_pages_domain_path(project, pages_domain)) + end + + it 'removes certificate' do + expect do + subject + end.to change { pages_domain.reload.certificate }.to(nil) + .and change { pages_domain.reload.key }.to(nil) + end + + it 'sets certificate source to user_provided' do + pages_domain.update!(certificate_source: :gitlab_provided) + + expect do + subject + end.to change { pages_domain.reload.certificate_source }.from("gitlab_provided").to("user_provided") + end + + context 'when pages_https_only is set' do + before do + project.update!(pages_https_only: true) + stub_pages_setting(external_https: '127.0.0.1') + end + + it 'does not remove certificate' do + subject + + pages_domain.reload + expect(pages_domain.certificate).to be_present + expect(pages_domain.key).to be_present + end + + it 'redirects to edit page with a flash message' do + subject + + expect(flash[:alert]).to include('Certificate') + expect(flash[:alert]).to include('Key') + expect(response).to redirect_to(edit_project_pages_domain_path(project, pages_domain)) + end + end + end + context 'pages disabled' do before do allow(Gitlab.config.pages).to receive(:enabled).and_return(false) diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index e3ad36f8d24..3c7f69f0e6e 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -93,7 +93,7 @@ describe Projects::PipelinesController do end context 'when performing gitaly calls', :request_store do - it 'limits the Gitaly requests' do + it 'limits the Gitaly requests', :sidekiq_might_not_need_inline do # Isolate from test preparation (Repository#exists? is also cached in RequestStore) RequestStore.end! RequestStore.clear! @@ -149,7 +149,7 @@ describe Projects::PipelinesController do end describe 'GET show.json' do - let(:pipeline) { create(:ci_pipeline_with_one_job, project: project) } + let(:pipeline) { create(:ci_pipeline, project: project) } it 'returns the pipeline' do get_pipeline_json @@ -571,7 +571,7 @@ describe Projects::PipelinesController do format: :json end - it 'cancels a pipeline without returning any content' do + it 'cancels a pipeline without returning any content', :sidekiq_might_not_need_inline do expect(response).to have_gitlab_http_status(:no_content) expect(pipeline.reload).to be_canceled end diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb index 2f473d395ad..072df1f5060 100644 --- a/spec/controllers/projects/project_members_controller_spec.rb +++ b/spec/controllers/projects/project_members_controller_spec.rb @@ -45,7 +45,9 @@ describe Projects::ProjectMembersController do end it 'adds user to members' do - expect_any_instance_of(Members::CreateService).to receive(:execute).and_return(status: :success) + expect_next_instance_of(Members::CreateService) do |instance| + expect(instance).to receive(:execute).and_return(status: :success) + end post :create, params: { namespace_id: project.namespace, @@ -59,7 +61,9 @@ describe Projects::ProjectMembersController do end it 'adds no user to members' do - expect_any_instance_of(Members::CreateService).to receive(:execute).and_return(status: :failure, message: 'Message') + expect_next_instance_of(Members::CreateService) do |instance| + expect(instance).to receive(:execute).and_return(status: :failure, message: 'Message') + end post :create, params: { namespace_id: project.namespace, diff --git a/spec/controllers/projects/prometheus/metrics_controller_spec.rb b/spec/controllers/projects/prometheus/metrics_controller_spec.rb index 17f9483be98..afdb8bbc983 100644 --- a/spec/controllers/projects/prometheus/metrics_controller_spec.rb +++ b/spec/controllers/projects/prometheus/metrics_controller_spec.rb @@ -85,7 +85,9 @@ describe Projects::Prometheus::MetricsController do end it 'calls prometheus adapter service' do - expect_any_instance_of(::Prometheus::AdapterService).to receive(:prometheus_adapter) + expect_next_instance_of(::Prometheus::AdapterService) do |instance| + expect(instance).to receive(:prometheus_adapter) + end subject.__send__(:prometheus_adapter) end diff --git a/spec/controllers/projects/releases_controller_spec.rb b/spec/controllers/projects/releases_controller_spec.rb index 5b9d21d3d5b..562119d967f 100644 --- a/spec/controllers/projects/releases_controller_spec.rb +++ b/spec/controllers/projects/releases_controller_spec.rb @@ -3,10 +3,36 @@ require 'spec_helper' describe Projects::ReleasesController do - let!(:project) { create(:project, :repository, :public) } - let!(:user) { create(:user) } + let!(:project) { create(:project, :repository, :public) } + let!(:private_project) { create(:project, :repository, :private) } + let(:user) { developer } + let(:developer) { create(:user) } + let(:reporter) { create(:user) } + let!(:release_1) { create(:release, project: project, released_at: Time.zone.parse('2018-10-18')) } + let!(:release_2) { create(:release, project: project, released_at: Time.zone.parse('2019-10-19')) } - describe 'GET #index' do + before do + project.add_developer(developer) + project.add_reporter(reporter) + end + + shared_examples_for 'successful request' do + it 'renders a 200' do + subject + + expect(response).to have_gitlab_http_status(:success) + end + end + + shared_examples_for 'not found' do + it 'renders 404' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + shared_examples 'common access controls' do it 'renders a 200' do get_index @@ -14,36 +40,135 @@ describe Projects::ReleasesController do end context 'when the project is private' do - let!(:project) { create(:project, :repository, :private) } + let(:project) { private_project } + + before do + sign_in(user) + end + + context 'when user is a developer' do + let(:user) { developer } - it 'renders a 302' do - get_index + it 'renders a 200 for a logged in developer' do + sign_in(user) - expect(response.status).to eq(302) + get_index + + expect(response.status).to eq(200) + end end - it 'renders a 200 for a logged in developer' do - project.add_developer(user) - sign_in(user) + context 'when user is an external user' do + let(:user) { create(:user) } - get_index + it 'renders a 404 when logged in but not in the project' do + sign_in(user) - expect(response.status).to eq(200) + get_index + + expect(response.status).to eq(404) + end end + end + end - it 'renders a 404 when logged in but not in the project' do - sign_in(user) + describe 'GET #index' do + before do + get_index + end - get_index + context 'as html' do + let(:format) { :html } - expect(response.status).to eq(404) + it 'returns a text/html content_type' do + expect(response.content_type).to eq 'text/html' end + + it_behaves_like 'common access controls' + + context 'when the project is private and the user is not logged in' do + let(:project) { private_project } + + it 'returns a redirect' do + expect(response).to have_gitlab_http_status(:redirect) + end + end + end + + context 'as json' do + let(:format) { :json } + + it 'returns an application/json content_type' do + expect(response.content_type).to eq 'application/json' + end + + it "returns the project's releases as JSON, ordered by released_at" do + expect(response.body).to eq([release_2, release_1].to_json) + end + + it_behaves_like 'common access controls' + + context 'when the project is private and the user is not logged in' do + let(:project) { private_project } + + it 'returns a redirect' do + expect(response).to have_gitlab_http_status(:redirect) + end + end + end + end + + describe 'GET #edit' do + subject do + get :edit, params: { namespace_id: project.namespace, project_id: project, tag: tag } + end + + before do + sign_in(user) + end + + let!(:release) { create(:release, project: project) } + let(:tag) { CGI.escape(release.tag) } + + it_behaves_like 'successful request' + + context 'when tag name contains slash' do + let!(:release) { create(:release, project: project, tag: 'awesome/v1.0') } + let(:tag) { CGI.escape(release.tag) } + + it_behaves_like 'successful request' + + it 'is accesible at a URL encoded path' do + expect(edit_project_release_path(project, release)) + .to eq("/#{project.namespace.path}/#{project.name}/-/releases/awesome%252Fv1.0/edit") + end + end + + context 'when feature flag `release_edit_page` is disabled' do + before do + stub_feature_flags(release_edit_page: false) + end + + it_behaves_like 'not found' + end + + context 'when release does not exist' do + let!(:release) { } + let(:tag) { 'non-existent-tag' } + + it_behaves_like 'not found' + end + + context 'when user is a reporter' do + let(:user) { reporter } + + it_behaves_like 'not found' end end private def get_index - get :index, params: { namespace_id: project.namespace, project_id: project } + get :index, params: { namespace_id: project.namespace, project_id: project, format: format } end end diff --git a/spec/controllers/projects/serverless/functions_controller_spec.rb b/spec/controllers/projects/serverless/functions_controller_spec.rb index eccc8e1d5de..73fb0fad646 100644 --- a/spec/controllers/projects/serverless/functions_controller_spec.rb +++ b/spec/controllers/projects/serverless/functions_controller_spec.rb @@ -13,6 +13,10 @@ describe Projects::Serverless::FunctionsController do let(:environment) { create(:environment, project: project) } let!(:deployment) { create(:deployment, :success, environment: environment, cluster: cluster) } let(:knative_services_finder) { environment.knative_services_finder } + let(:function_description) { 'A serverless function' } + let(:knative_stub_options) do + { namespace: namespace.namespace, name: cluster.project.name, description: function_description } + end let(:namespace) do create(:cluster_kubernetes_namespace, @@ -114,40 +118,33 @@ describe Projects::Serverless::FunctionsController do expect(response).to have_gitlab_http_status(200) expect(json_response).to include( - "name" => project.name, - "url" => "http://#{project.name}.#{namespace.namespace}.example.com", - "podcount" => 1 + 'name' => project.name, + 'url' => "http://#{project.name}.#{namespace.namespace}.example.com", + 'description' => function_description, + 'podcount' => 1 ) end end - context 'on Knative 0.5' do + context 'on Knative 0.5.0' do + before do + prepare_knative_stubs(knative_05_service(knative_stub_options)) + end + + include_examples 'GET #show with valid data' + end + + context 'on Knative 0.6.0' do before do - stub_kubeclient_service_pods - stub_reactive_cache(knative_services_finder, - { - services: kube_knative_services_body( - legacy_knative: true, - namespace: namespace.namespace, - name: cluster.project.name - )["items"], - pods: kube_knative_pods_body(cluster.project.name, namespace.namespace)["items"] - }, - *knative_services_finder.cache_args) + prepare_knative_stubs(knative_06_service(knative_stub_options)) end include_examples 'GET #show with valid data' end - context 'on Knative 0.6 or 0.7' do + context 'on Knative 0.7.0' do before do - stub_kubeclient_service_pods - stub_reactive_cache(knative_services_finder, - { - services: kube_knative_services_body(namespace: namespace.namespace, name: cluster.project.name)["items"], - pods: kube_knative_pods_body(cluster.project.name, namespace.namespace)["items"] - }, - *knative_services_finder.cache_args) + prepare_knative_stubs(knative_07_service(knative_stub_options)) end include_examples 'GET #show with valid data' @@ -172,11 +169,12 @@ describe Projects::Serverless::FunctionsController do expect(response).to have_gitlab_http_status(200) expect(json_response).to match({ - "knative_installed" => "checking", - "functions" => [ + 'knative_installed' => 'checking', + 'functions' => [ a_hash_including( - "name" => project.name, - "url" => "http://#{project.name}.#{namespace.namespace}.example.com" + 'name' => project.name, + 'url' => "http://#{project.name}.#{namespace.namespace}.example.com", + 'description' => function_description ) ] }) @@ -189,36 +187,38 @@ describe Projects::Serverless::FunctionsController do end end - context 'on Knative 0.5' do + context 'on Knative 0.5.0' do before do - stub_kubeclient_service_pods - stub_reactive_cache(knative_services_finder, - { - services: kube_knative_services_body( - legacy_knative: true, - namespace: namespace.namespace, - name: cluster.project.name - )["items"], - pods: kube_knative_pods_body(cluster.project.name, namespace.namespace)["items"] - }, - *knative_services_finder.cache_args) + prepare_knative_stubs(knative_05_service(knative_stub_options)) end include_examples 'GET #index with data' end - context 'on Knative 0.6 or 0.7' do + context 'on Knative 0.6.0' do before do - stub_kubeclient_service_pods - stub_reactive_cache(knative_services_finder, - { - services: kube_knative_services_body(namespace: namespace.namespace, name: cluster.project.name)["items"], - pods: kube_knative_pods_body(cluster.project.name, namespace.namespace)["items"] - }, - *knative_services_finder.cache_args) + prepare_knative_stubs(knative_06_service(knative_stub_options)) end include_examples 'GET #index with data' end + + context 'on Knative 0.7.0' do + before do + prepare_knative_stubs(knative_07_service(knative_stub_options)) + end + + include_examples 'GET #index with data' + end + end + + def prepare_knative_stubs(knative_service) + stub_kubeclient_service_pods + stub_reactive_cache(knative_services_finder, + { + services: [knative_service], + pods: kube_knative_pods_body(cluster.project.name, namespace.namespace)["items"] + }, + *knative_services_finder.cache_args) end end diff --git a/spec/controllers/projects/settings/ci_cd_controller_spec.rb b/spec/controllers/projects/settings/ci_cd_controller_spec.rb index c67e7f7dadd..98f8826397f 100644 --- a/spec/controllers/projects/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/projects/settings/ci_cd_controller_spec.rb @@ -125,7 +125,9 @@ describe Projects::Settings::CiCdController do context 'when run_auto_devops_pipeline is true' do before do - expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(true) + expect_next_instance_of(Projects::UpdateService) do |instance| + expect(instance).to receive(:run_auto_devops_pipeline?).and_return(true) + end end context 'when the project repository is empty' do @@ -159,7 +161,9 @@ describe Projects::Settings::CiCdController do context 'when run_auto_devops_pipeline is not true' do before do - expect_any_instance_of(Projects::UpdateService).to receive(:run_auto_devops_pipeline?).and_return(false) + expect_next_instance_of(Projects::UpdateService) do |instance| + expect(instance).to receive(:run_auto_devops_pipeline?).and_return(false) + end end it 'does not queue a CreatePipelineWorker' do diff --git a/spec/controllers/projects/settings/operations_controller_spec.rb b/spec/controllers/projects/settings/operations_controller_spec.rb index 0b34656e9e2..667a6336952 100644 --- a/spec/controllers/projects/settings/operations_controller_spec.rb +++ b/spec/controllers/projects/settings/operations_controller_spec.rb @@ -186,7 +186,8 @@ describe Projects::Settings::OperationsController do { grafana_integration_attributes: { grafana_url: 'https://grafana.gitlab.com', - token: 'eyJrIjoicDRlRTREdjhhOEZ5WjZPWXUzazJOSW0zZHJUejVOd3IiLCJuIjoiVGVzdCBLZXkiLCJpZCI6MX0=' + token: 'eyJrIjoicDRlRTREdjhhOEZ5WjZPWXUzazJOSW0zZHJUejVOd3IiLCJuIjoiVGVzdCBLZXkiLCJpZCI6MX0=', + enabled: 'true' } } end diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index 042a5542786..d372a94db56 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -92,7 +92,9 @@ describe Projects::SnippetsController do context 'when the snippet is spam' do before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive(:spam?).and_return(true) + end end context 'when the snippet is private' do @@ -170,7 +172,9 @@ describe Projects::SnippetsController do context 'when the snippet is spam' do before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive(:spam?).and_return(true) + end end context 'when the snippet is private' do @@ -278,7 +282,9 @@ describe Projects::SnippetsController do let(:snippet) { create(:project_snippet, :private, project: project, author: user) } before do - allow_any_instance_of(AkismetService).to receive_messages(submit_spam: true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive_messages(submit_spam: true) + end stub_application_setting(akismet_enabled: true) end diff --git a/spec/controllers/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb index 7f7cabe3b0c..c0c11db5dd6 100644 --- a/spec/controllers/projects/tree_controller_spec.rb +++ b/spec/controllers/projects/tree_controller_spec.rb @@ -30,46 +30,61 @@ describe Projects::TreeController do context "valid branch, no path" do let(:id) { 'master' } + it { is_expected.to respond_with(:success) } end context "valid branch, valid path" do let(:id) { 'master/encoding/' } + it { is_expected.to respond_with(:success) } end context "valid branch, invalid path" do let(:id) { 'master/invalid-path/' } - it { is_expected.to respond_with(:not_found) } + + it 'redirects' do + expect(subject) + .to redirect_to("/#{project.full_path}/tree/master") + end end context "invalid branch, valid path" do let(:id) { 'invalid-branch/encoding/' } + it { is_expected.to respond_with(:not_found) } end context "valid empty branch, invalid path" do let(:id) { 'empty-branch/invalid-path/' } - it { is_expected.to respond_with(:not_found) } + + it 'redirects' do + expect(subject) + .to redirect_to("/#{project.full_path}/tree/empty-branch") + end end context "valid empty branch" do let(:id) { 'empty-branch' } + it { is_expected.to respond_with(:success) } end context "invalid SHA commit ID" do let(:id) { 'ff39438/.gitignore' } + it { is_expected.to respond_with(:not_found) } end context "valid SHA commit ID" do let(:id) { '6d39438' } + it { is_expected.to respond_with(:success) } end context "valid SHA commit ID with path" do let(:id) { '6d39438/.gitignore' } + it { expect(response).to have_gitlab_http_status(302) } end end @@ -108,6 +123,7 @@ describe Projects::TreeController do context 'redirect to blob' do let(:id) { 'master/README.md' } + it 'redirects' do redirect_url = "/#{project.full_path}/blob/master/README.md" expect(subject) diff --git a/spec/controllers/projects/usage_ping_controller_spec.rb b/spec/controllers/projects/usage_ping_controller_spec.rb new file mode 100644 index 00000000000..a9abbff160d --- /dev/null +++ b/spec/controllers/projects/usage_ping_controller_spec.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::UsagePingController do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + + describe 'POST #web_ide_clientside_preview' do + subject { post :web_ide_clientside_preview, params: { namespace_id: project.namespace, project_id: project } } + + before do + sign_in(user) if user + end + + context 'when web ide clientside preview is enabled' do + before do + stub_application_setting(web_ide_clientside_preview_enabled: true) + end + + context 'when the user is not authenticated' do + let(:user) { nil } + + it 'returns 302' do + subject + + expect(response).to have_gitlab_http_status(302) + end + end + + context 'when the user does not have access to the project' do + it 'returns 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the user has access to the project' do + let(:user) { project.owner } + + it 'increments the counter' do + expect do + subject + end.to change { Gitlab::UsageDataCounters::WebIdeCounter.total_previews_count }.by(1) + end + end + end + + context 'when web ide clientside preview is not enabled' do + let(:user) { project.owner } + + before do + stub_application_setting(web_ide_clientside_preview_enabled: false) + end + + it 'returns 404' do + subject + + expect(response).to have_gitlab_http_status(404) + end + end + end +end diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index e0df9556eb8..ff0259cd40d 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -653,7 +653,7 @@ describe ProjectsController do describe "#destroy" do let(:admin) { create(:admin) } - it "redirects to the dashboard" do + it "redirects to the dashboard", :sidekiq_might_not_need_inline do controller.instance_variable_set(:@project, project) sign_in(admin) @@ -674,7 +674,7 @@ describe ProjectsController do target_project: project) end - it "closes all related merge requests" do + it "closes all related merge requests", :sidekiq_might_not_need_inline do project.merge_requests << merge_request sign_in(admin) @@ -927,6 +927,30 @@ describe ProjectsController do expect(json_response['body']).to match(/\!#{merge_request.iid} \(closed\)/) end end + + context 'when path parameter is provided' do + let(:project_with_repo) { create(:project, :repository) } + let(:preview_markdown_params) do + { + namespace_id: project_with_repo.namespace, + id: project_with_repo, + text: "\n", + path: 'files/images/README.md' + } + end + + before do + project_with_repo.add_maintainer(user) + end + + it 'renders JSON body with image links expanded' do + expanded_path = "/#{project_with_repo.full_path}/raw/master/files/images/logo-white.png" + + post :preview_markdown, params: preview_markdown_params + + expect(json_response['body']).to include(expanded_path) + end + end end describe '#ensure_canonical_path' do diff --git a/spec/controllers/registrations_controller_spec.rb b/spec/controllers/registrations_controller_spec.rb index ebeed94c274..c5cfdd32619 100644 --- a/spec/controllers/registrations_controller_spec.rb +++ b/spec/controllers/registrations_controller_spec.rb @@ -9,6 +9,51 @@ describe RegistrationsController do stub_feature_flags(invisible_captcha: false) end + describe '#new' do + subject { get :new } + + context 'with the experimental signup flow enabled and the user is part of the experimental group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: true) + end + + it 'tracks the event with the right parameters' do + expect(Gitlab::Tracking).to receive(:event).with( + 'Growth::Acquisition::Experiment::SignUpFlow', + 'start', + label: anything, + property: 'experimental_group' + ) + subject + end + + it 'renders new template and sets the resource variable' do + expect(subject).to render_template(:new) + expect(response).to have_gitlab_http_status(200) + expect(assigns(:resource)).to be_a(User) + end + end + + context 'with the experimental signup flow enabled and the user is part of the control group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: false) + end + + it 'does not track the event' do + expect(Gitlab::Tracking).not_to receive(:event) + subject + end + + it 'renders new template and sets the resource variable' do + subject + expect(response).to have_gitlab_http_status(302) + expect(response).to redirect_to(new_user_session_path(anchor: 'register-pane')) + end + end + end + describe '#create' do let(:base_user_params) { { name: 'new_user', username: 'new_username', email: 'new@user.com', password: 'Any_password' } } let(:user_params) { { user: base_user_params } } @@ -217,6 +262,37 @@ describe RegistrationsController do end end + describe 'tracking data' do + context 'with the experimental signup flow enabled and the user is part of the control group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: false) + end + + it 'tracks the event with the right parameters' do + expect(Gitlab::Tracking).to receive(:event).with( + 'Growth::Acquisition::Experiment::SignUpFlow', + 'end', + label: anything, + property: 'control_group' + ) + post :create, params: user_params + end + end + + context 'with the experimental signup flow enabled and the user is part of the experimental group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: true) + end + + it 'does not track the event' do + expect(Gitlab::Tracking).not_to receive(:event) + post :create, params: user_params + end + end + end + it "logs a 'User Created' message" do stub_feature_flags(registrations_recaptcha: false) @@ -304,4 +380,22 @@ describe RegistrationsController do end end end + + describe '#update_registration' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: true) + sign_in(create(:user)) + end + + it 'tracks the event with the right parameters' do + expect(Gitlab::Tracking).to receive(:event).with( + 'Growth::Acquisition::Experiment::SignUpFlow', + 'end', + label: anything, + property: 'experimental_group' + ) + patch :update_registration, params: { user: { name: 'New name', role: 'software_developer', setup_for_company: 'false' } } + end + end end diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 2108cf1c8ae..1e47df150b4 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -4,6 +4,7 @@ require 'spec_helper' describe SessionsController do include DeviseHelpers + include LdapHelpers describe '#new' do before do @@ -34,6 +35,63 @@ describe SessionsController do end end end + + context 'with LDAP enabled' do + before do + stub_ldap_setting(enabled: true) + end + + it 'assigns ldap_servers' do + get(:new) + + expect(assigns[:ldap_servers].first.to_h).to include('label' => 'ldap', 'provider_name' => 'ldapmain') + end + + context 'with sign_in disabled' do + before do + stub_ldap_setting(prevent_ldap_sign_in: true) + end + + it 'assigns no ldap_servers' do + get(:new) + + expect(assigns[:ldap_servers]).to eq [] + end + end + end + + describe 'tracking data' do + context 'when the user is part of the experimental group' do + before do + stub_experiment_for_user(signup_flow: true) + end + + it 'doesn\'t pass tracking parameters to the frontend' do + get(:new) + expect(Gon.tracking_data).to be_nil + end + end + + context 'with the experimental signup flow enabled and the user is part of the control group' do + before do + stub_experiment(signup_flow: true) + stub_experiment_for_user(signup_flow: false) + allow_any_instance_of(described_class).to receive(:experimentation_subject_id).and_return('uuid') + end + + it 'passes the right tracking parameters to the frontend' do + get(:new) + expect(Gon.tracking_data).to eq( + { + category: 'Growth::Acquisition::Experiment::SignUpFlow', + action: 'start', + label: 'uuid', + property: 'control_group' + } + ) + end + end + end end describe '#create' do diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index e892c736c69..054d448c28d 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -251,7 +251,9 @@ describe SnippetsController do context 'when the snippet is spam' do before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive(:spam?).and_return(true) + end end context 'when the snippet is private' do @@ -323,7 +325,9 @@ describe SnippetsController do context 'when the snippet is spam' do before do - allow_any_instance_of(AkismetService).to receive(:spam?).and_return(true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive(:spam?).and_return(true) + end end context 'when the snippet is private' do @@ -431,7 +435,9 @@ describe SnippetsController do let(:snippet) { create(:personal_snippet, :public, author: user) } before do - allow_any_instance_of(AkismetService).to receive_messages(submit_spam: true) + allow_next_instance_of(AkismetService) do |instance| + allow(instance).to receive_messages(submit_spam: true) + end stub_application_setting(akismet_enabled: true) end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index 5566df0c216..bbbb9691f53 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -174,7 +174,9 @@ describe UsersController do let(:user) { create(:user) } before do - allow_any_instance_of(User).to receive(:contributed_projects_ids).and_return([project.id]) + allow_next_instance_of(User) do |instance| + allow(instance).to receive(:contributed_projects_ids).and_return([project.id]) + end sign_in(user) project.add_developer(user) @@ -348,6 +350,48 @@ describe UsersController do end end + describe 'GET #suggests' do + context 'when user exists' do + it 'returns JSON indicating the user exists and a suggestion' do + get :suggests, params: { username: user.username } + + expected_json = { exists: true, suggests: ["#{user.username}1"] }.to_json + expect(response.body).to eq(expected_json) + end + + context 'when the casing is different' do + let(:user) { create(:user, username: 'CamelCaseUser') } + + it 'returns JSON indicating the user exists and a suggestion' do + get :suggests, params: { username: user.username.downcase } + + expected_json = { exists: true, suggests: ["#{user.username.downcase}1"] }.to_json + expect(response.body).to eq(expected_json) + end + end + end + + context 'when the user does not exist' do + it 'returns JSON indicating the user does not exist' do + get :suggests, params: { username: 'foo' } + + expected_json = { exists: false, suggests: [] }.to_json + expect(response.body).to eq(expected_json) + end + + context 'when a user changed their username' do + let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') } + + it 'returns JSON indicating a user by that username does not exist' do + get :suggests, params: { username: 'old-username' } + + expected_json = { exists: false, suggests: [] }.to_json + expect(response.body).to eq(expected_json) + end + end + end + end + describe '#ensure_canonical_path' do before do sign_in(user) |