diff options
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/deployments_controller_spec.rb | 74 | ||||
-rw-r--r-- | spec/controllers/projects/merge_requests_controller_spec.rb | 16 | ||||
-rw-r--r-- | spec/models/deployment_metrics_spec.rb | 126 | ||||
-rw-r--r-- | spec/models/deployment_spec.rb | 147 | ||||
-rw-r--r-- | spec/models/environment_status_spec.rb | 3 | ||||
-rw-r--r-- | spec/serializers/environment_status_entity_spec.rb | 10 |
6 files changed, 177 insertions, 199 deletions
diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb index 95417936df4..b9ee69a617b 100644 --- a/spec/controllers/projects/deployments_controller_spec.rb +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -41,34 +41,26 @@ describe Projects::DeploymentsController do describe 'GET #metrics' do let(:deployment) { create(:deployment, :success, project: project, environment: environment) } - before do - allow(controller).to receive(:deployment).and_return(deployment) - end - context 'when metrics are disabled' do - before do - allow(deployment).to receive(:has_metrics?).and_return false - end - it 'responds with not found' do - get :metrics, params: deployment_params(id: deployment.id) + get :metrics, params: deployment_params(id: deployment.to_param) expect(response).to be_not_found end end context 'when metrics are enabled' do - before do - allow(deployment).to receive(:has_metrics?).and_return true - end - context 'when environment has no metrics' do before do - expect(deployment).to receive(:metrics).and_return(nil) + expect_next_instance_of(DeploymentMetrics) do |deployment_metrics| + allow(deployment_metrics).to receive(:has_metrics?).and_return(true) + + expect(deployment_metrics).to receive(:metrics).and_return(nil) + end end it 'returns a empty response 204 resposne' do - get :metrics, params: deployment_params(id: deployment.id) + get :metrics, params: deployment_params(id: deployment.to_param) expect(response).to have_gitlab_http_status(204) expect(response.body).to eq('') end @@ -84,11 +76,15 @@ describe Projects::DeploymentsController do end before do - expect(deployment).to receive(:metrics).and_return(empty_metrics) + expect_next_instance_of(DeploymentMetrics) do |deployment_metrics| + allow(deployment_metrics).to receive(:has_metrics?).and_return(true) + + expect(deployment_metrics).to receive(:metrics).and_return(empty_metrics) + end end it 'returns a metrics JSON document' do - get :metrics, params: deployment_params(id: deployment.id) + get :metrics, params: deployment_params(id: deployment.to_param) expect(response).to be_ok expect(json_response['success']).to be(true) @@ -96,54 +92,32 @@ describe Projects::DeploymentsController do expect(json_response['last_update']).to eq(42) end end - - context 'when metrics service does not implement deployment metrics' do - before do - allow(deployment).to receive(:metrics).and_raise(NotImplementedError) - end - - it 'responds with not found' do - get :metrics, params: deployment_params(id: deployment.id) - - expect(response).to be_not_found - end - end end end describe 'GET #additional_metrics' do let(:deployment) { create(:deployment, :success, project: project, environment: environment) } - before do - allow(controller).to receive(:deployment).and_return(deployment) - end - context 'when metrics are disabled' do - before do - allow(deployment).to receive(:has_metrics?).and_return false - end - it 'responds with not found' do - get :metrics, params: deployment_params(id: deployment.id) + get :metrics, params: deployment_params(id: deployment.to_param) expect(response).to be_not_found end end context 'when metrics are enabled' do - let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } - - before do - allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) - end - context 'when environment has no metrics' do before do - expect(deployment).to receive(:additional_metrics).and_return({}) + expect_next_instance_of(DeploymentMetrics) do |deployment_metrics| + allow(deployment_metrics).to receive(:has_metrics?).and_return(true) + + expect(deployment_metrics).to receive(:additional_metrics).and_return({}) + end end it 'returns a empty response 204 response' do - get :additional_metrics, params: deployment_params(id: deployment.id, format: :json) + get :additional_metrics, params: deployment_params(id: deployment.to_param, format: :json) expect(response).to have_gitlab_http_status(204) expect(response.body).to eq('') end @@ -159,11 +133,15 @@ describe Projects::DeploymentsController do end before do - expect(deployment).to receive(:additional_metrics).and_return(empty_metrics) + expect_next_instance_of(DeploymentMetrics) do |deployment_metrics| + allow(deployment_metrics).to receive(:has_metrics?).and_return(true) + + expect(deployment_metrics).to receive(:additional_metrics).and_return(empty_metrics) + end end it 'returns a metrics JSON document' do - get :additional_metrics, params: deployment_params(id: deployment.id, format: :json) + get :additional_metrics, params: deployment_params(id: deployment.to_param, format: :json) expect(response).to be_ok expect(json_response['success']).to be(true) diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 0eca663a683..9878f88a395 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -878,6 +878,22 @@ describe Projects::MergeRequestsController do expect(control_count).to be <= 137 end + it 'has no N+1 issues for environments', :request_store, retry: 0 do + # First run to insert test data from lets, which does take up some 30 queries + get_ci_environments_status + + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { get_ci_environments_status }.count + + environment2 = create(:environment, project: forked) + create(:deployment, :succeed, environment: environment2, sha: sha, ref: 'master', deployable: build) + + # TODO address the last 11 queries + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/63952 (5 queries) + # And https://gitlab.com/gitlab-org/gitlab-ce/issues/64105 (6 queries) + leeway = 11 + expect { get_ci_environments_status }.not_to exceed_all_query_limit(control_count + leeway) + end + def get_ci_environments_status(extra_params = {}) params = { namespace_id: merge_request.project.namespace.to_param, diff --git a/spec/models/deployment_metrics_spec.rb b/spec/models/deployment_metrics_spec.rb new file mode 100644 index 00000000000..0aadb1f3a5e --- /dev/null +++ b/spec/models/deployment_metrics_spec.rb @@ -0,0 +1,126 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe DeploymentMetrics do + describe '#has_metrics?' do + subject { described_class.new(deployment.project, deployment).has_metrics? } + + context 'when deployment is failed' do + let(:deployment) { create(:deployment, :failed) } + + it { is_expected.to be_falsy } + end + + context 'when deployment is success' do + let(:deployment) { create(:deployment, :success) } + + context 'without a monitoring service' do + it { is_expected.to be_falsy } + end + + context 'with a Prometheus Service' do + let(:prometheus_service) { instance_double(PrometheusService, can_query?: true) } + + before do + allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service + end + + it { is_expected.to be_truthy } + end + + context 'with a Prometheus Service that cannot query' do + let(:prometheus_service) { instance_double(PrometheusService, can_query?: false) } + + before do + allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service + end + + it { is_expected.to be_falsy } + end + + context 'with a cluster Prometheus' do + let(:deployment) { create(:deployment, :success, :on_cluster) } + let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: deployment.cluster) } + + before do + expect(deployment.cluster.application_prometheus).to receive(:can_query?).and_return(true) + end + + it { is_expected.to be_truthy } + end + + context 'fallback deployment platform' do + let(:cluster) { create(:cluster, :provided_by_user, environment_scope: '*', projects: [deployment.project]) } + let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } + + before do + expect(deployment.project).to receive(:deployment_platform).and_return(cluster.platform) + expect(cluster.application_prometheus).to receive(:can_query?).and_return(true) + end + + it { is_expected.to be_truthy } + end + end + end + + describe '#metrics' do + let(:deployment) { create(:deployment, :success) } + let(:prometheus_adapter) { instance_double(PrometheusService, can_query?: true) } + let(:deployment_metrics) { described_class.new(deployment.project, deployment) } + + subject { deployment_metrics.metrics } + + context 'metrics are disabled' do + it { is_expected.to eq({}) } + end + + context 'metrics are enabled' do + let(:simple_metrics) do + { + success: true, + metrics: {}, + last_update: 42 + } + end + + before do + allow(deployment_metrics).to receive(:prometheus_adapter).and_return(prometheus_adapter) + expect(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics) + end + + it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } + end + end + + describe '#additional_metrics' do + let(:project) { create(:project, :repository) } + let(:deployment) { create(:deployment, :succeed, project: project) } + let(:deployment_metrics) { described_class.new(deployment.project, deployment) } + + subject { deployment_metrics.additional_metrics } + + context 'metrics are disabled' do + it { is_expected.to eq({}) } + end + + context 'metrics are enabled' do + let(:simple_metrics) do + { + success: true, + metrics: {}, + last_update: 42 + } + end + + let(:prometheus_adapter) { instance_double('prometheus_adapter', can_query?: true) } + + before do + allow(deployment_metrics).to receive(:prometheus_adapter).and_return(prometheus_adapter) + expect(prometheus_adapter).to receive(:query).with(:additional_metrics_deployment, deployment).and_return(simple_metrics) + end + + it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } + end + end +end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index 713fb647708..d4e631f109b 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -295,125 +295,6 @@ describe Deployment do end end - describe '#has_metrics?' do - subject { deployment.has_metrics? } - - context 'when deployment is failed' do - let(:deployment) { create(:deployment, :failed) } - - it { is_expected.to be_falsy } - end - - context 'when deployment is success' do - let(:deployment) { create(:deployment, :success) } - - context 'without a monitoring service' do - it { is_expected.to be_falsy } - end - - context 'with a Prometheus Service' do - let(:prometheus_service) { double(:prometheus_service, can_query?: true) } - - before do - allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service - end - - it { is_expected.to be_truthy } - end - - context 'with a Prometheus Service that cannot query' do - let(:prometheus_service) { double(:prometheus_service, can_query?: false) } - - before do - allow(deployment.project).to receive(:find_or_initialize_service).with('prometheus').and_return prometheus_service - end - - it { is_expected.to be_falsy } - end - - context 'with a cluster Prometheus' do - let(:deployment) { create(:deployment, :success, :on_cluster) } - let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: deployment.cluster) } - - before do - expect(deployment.cluster.application_prometheus).to receive(:can_query?).and_return(true) - end - - it { is_expected.to be_truthy } - end - - context 'fallback deployment platform' do - let(:cluster) { create(:cluster, :provided_by_user, environment_scope: '*', projects: [deployment.project]) } - let!(:prometheus) { create(:clusters_applications_prometheus, :installed, cluster: cluster) } - - before do - expect(deployment.project).to receive(:deployment_platform).and_return(cluster.platform) - expect(cluster.application_prometheus).to receive(:can_query?).and_return(true) - end - - it { is_expected.to be_truthy } - end - end - end - - describe '#metrics' do - let(:deployment) { create(:deployment, :success) } - let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } - - subject { deployment.metrics } - - context 'metrics are disabled' do - it { is_expected.to eq({}) } - end - - context 'metrics are enabled' do - let(:simple_metrics) do - { - success: true, - metrics: {}, - last_update: 42 - } - end - - before do - allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) - allow(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics) - end - - it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } - end - end - - describe '#additional_metrics' do - let(:project) { create(:project, :repository) } - let(:deployment) { create(:deployment, :succeed, project: project) } - - subject { deployment.additional_metrics } - - context 'metrics are disabled' do - it { is_expected.to eq({}) } - end - - context 'metrics are enabled' do - let(:simple_metrics) do - { - success: true, - metrics: {}, - last_update: 42 - } - end - - let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } - - before do - allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) - allow(prometheus_adapter).to receive(:query).with(:additional_metrics_deployment, deployment).and_return(simple_metrics) - end - - it { is_expected.to eq(simple_metrics.merge({ deployment_time: deployment.created_at.to_i })) } - end - end - describe '#stop_action' do let(:build) { create(:ci_build) } @@ -441,32 +322,4 @@ describe Deployment do end end end - - describe '#deployment_platform_cluster' do - let(:deployment) { create(:deployment) } - let(:project) { deployment.project } - let(:environment) { deployment.environment } - - subject { deployment.deployment_platform_cluster } - - before do - expect(project).to receive(:deployment_platform) - .with(environment: environment.name).and_call_original - end - - context 'project has no deployment platform' do - before do - expect(project.clusters).to be_empty - end - - it { is_expected.to be_nil } - end - - context 'project has a deployment platform' do - let!(:cluster) { create(:cluster, projects: [project]) } - let!(:platform) { create(:cluster_platform_kubernetes, cluster: cluster) } - - it { is_expected.to eq cluster } - end - end end diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index c503c35305f..e2836420df9 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -11,11 +11,10 @@ describe EnvironmentStatus do let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) } let(:sha) { deployment.sha } - subject(:environment_status) { described_class.new(environment, merge_request, sha) } + subject(:environment_status) { described_class.new(project, environment, merge_request, sha) } it { is_expected.to delegate_method(:id).to(:environment) } it { is_expected.to delegate_method(:name).to(:environment) } - it { is_expected.to delegate_method(:project).to(:environment) } it { is_expected.to delegate_method(:deployed_at).to(:deployment) } it { is_expected.to delegate_method(:status).to(:deployment) } diff --git a/spec/serializers/environment_status_entity_spec.rb b/spec/serializers/environment_status_entity_spec.rb index 8a6a38fe5f8..f421432e8d6 100644 --- a/spec/serializers/environment_status_entity_spec.rb +++ b/spec/serializers/environment_status_entity_spec.rb @@ -9,7 +9,7 @@ describe EnvironmentStatusEntity do let(:project) { deployment.project } let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) } - let(:environment_status) { EnvironmentStatus.new(environment, merge_request, merge_request.diff_head_sha) } + let(:environment_status) { EnvironmentStatus.new(project, environment, merge_request, merge_request.diff_head_sha) } let(:entity) { described_class.new(environment_status, request: request) } subject { entity.as_json } @@ -55,8 +55,14 @@ describe EnvironmentStatusEntity do before do project.add_maintainer(user) allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) - allow(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics) allow(entity).to receive(:deployment).and_return(deployment) + + expect_next_instance_of(DeploymentMetrics) do |deployment_metrics| + allow(deployment_metrics).to receive(:prometheus_adapter).and_return(prometheus_adapter) + + allow(prometheus_adapter).to receive(:query) + .with(:deployment, deployment).and_return(simple_metrics) + end end context 'when deployment succeeded' do |