From a375d80eb072d62962af9b6f2decf9782cd7ee1f Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Tue, 21 Mar 2017 22:21:13 +0900 Subject: [PATCH] Use detailed_status effectively. Remove unnecesarry context(nest). Add new context in merge_requests_controller_spec.rb and fix a bug. Correct description of spec. --- .../projects/merge_requests_controller.rb | 1 + .../projects/builds_controller_spec.rb | 34 ++++++++--------- .../merge_requests_controller_spec.rb | 37 ++++++++++++------- .../projects/pipelines_controller_spec.rb | 32 +++++++--------- spec/serializers/build_serializer_spec.rb | 6 +-- spec/serializers/pipeline_serializer_spec.rb | 6 +-- 6 files changed, 58 insertions(+), 58 deletions(-) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 165035285f4..6a116f99f13 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -474,6 +474,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end def pipeline_status + render json: {} and return unless @merge_request.head_pipeline.present? render json: PipelineSerializer .new(project: @project, user: @current_user) .represent_status(@merge_request.head_pipeline) diff --git a/spec/controllers/projects/builds_controller_spec.rb b/spec/controllers/projects/builds_controller_spec.rb index 2234b2e6131..683667129e5 100644 --- a/spec/controllers/projects/builds_controller_spec.rb +++ b/spec/controllers/projects/builds_controller_spec.rb @@ -11,27 +11,23 @@ describe Projects::BuildsController do end describe 'GET status.json' do - context 'when accessing status' do - let(:status) do - Gitlab::Ci::Status::Success.new(double('object'), double('user')) - end + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, pipeline: pipeline) } + let(:status) { build.detailed_status(double('user')) } - before do - pipeline = create(:ci_pipeline, project: project) - build = create(:ci_build, pipeline: pipeline, status: :success) - get :status, namespace_id: project.namespace, - project_id: project, - id: build.id, - format: :json - end + before do + get :status, namespace_id: project.namespace, + project_id: project, + id: build.id, + format: :json + end - it 'return a correct pipeline status' do - expect(response).to have_http_status(:ok) - expect(json_response['text']).to eq status.text - expect(json_response['label']).to eq status.label - expect(json_response['icon']).to eq status.icon - expect(json_response['favicon']).to eq status.favicon - end + it 'return a detailed build status in json' do + expect(response).to have_http_status(:ok) + expect(json_response['text']).to eq status.text + expect(json_response['label']).to eq status.label + expect(json_response['icon']).to eq status.icon + expect(json_response['favicon']).to eq status.favicon end end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 9fa509c339e..72f41f7209a 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -1180,23 +1180,18 @@ describe Projects::MergeRequestsController do end describe 'GET pipeline_status.json' do - context 'when accessing pipeline_status' do - let(:status) do - Gitlab::Ci::Status::Success.new(double('object'), double('user')) - end - - before do + context 'when head_pipeline exists' do + let!(:pipeline) do create(:ci_pipeline, project: merge_request.source_project, ref: merge_request.source_branch, - sha: merge_request.diff_head_sha, - status: :success) - get :pipeline_status, namespace_id: project.namespace, - project_id: project, - id: merge_request.iid, - format: :json + sha: merge_request.diff_head_sha) end - it 'return a correct pipeline status' do + let(:status) { pipeline.detailed_status(double('user')) } + + before { get_pipeline_status } + + it 'return a detailed head_pipeline status in json' do expect(response).to have_http_status(:ok) expect(json_response['text']).to eq status.text expect(json_response['label']).to eq status.label @@ -1204,5 +1199,21 @@ describe Projects::MergeRequestsController do expect(json_response['favicon']).to eq status.favicon end end + + context 'when head_pipeline does not exist' do + before { get_pipeline_status } + + it 'return empty' do + expect(response).to have_http_status(:ok) + expect(json_response).to be_empty + end + end + + def get_pipeline_status + get :pipeline_status, namespace_id: project.namespace, + project_id: project, + id: merge_request.iid, + format: :json + end end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 9ce8f8e8da1..d8f9bfd0d37 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -71,26 +71,22 @@ describe Projects::PipelinesController do end describe 'GET status.json' do - context 'when accessing status' do - let(:status) do - Gitlab::Ci::Status::Success.new(double('object'), double('user')) - end + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:status) { pipeline.detailed_status(double('user')) } - before do - pipeline = create(:ci_pipeline, project: project, status: :success) - get :status, namespace_id: project.namespace, - project_id: project, - id: pipeline.id, - format: :json - end + before do + get :status, namespace_id: project.namespace, + project_id: project, + id: pipeline.id, + format: :json + end - it 'return a correct pipeline status' do - expect(response).to have_http_status(:ok) - expect(json_response['text']).to eq status.text - expect(json_response['label']).to eq status.label - expect(json_response['icon']).to eq status.icon - expect(json_response['favicon']).to eq status.favicon - end + it 'return a detailed pipeline status in json' do + expect(response).to have_http_status(:ok) + expect(json_response['text']).to eq status.text + expect(json_response['label']).to eq status.label + expect(json_response['icon']).to eq status.icon + expect(json_response['favicon']).to eq status.favicon end end end diff --git a/spec/serializers/build_serializer_spec.rb b/spec/serializers/build_serializer_spec.rb index 5a55d9d7dc1..3cc791bca50 100644 --- a/spec/serializers/build_serializer_spec.rb +++ b/spec/serializers/build_serializer_spec.rb @@ -29,10 +29,8 @@ describe BuildSerializer do describe '#represent_status' do context 'when represents only status' do - let(:status) do - Gitlab::Ci::Status::Success.new(double('object'), double('user')) - end - let(:resource) { create(:ci_build, status: :success) } + let(:resource) { create(:ci_build) } + let(:status) { resource.detailed_status(double('user')) } subject { serializer.represent_status(resource) } diff --git a/spec/serializers/pipeline_serializer_spec.rb b/spec/serializers/pipeline_serializer_spec.rb index 261f2152c22..8642b803844 100644 --- a/spec/serializers/pipeline_serializer_spec.rb +++ b/spec/serializers/pipeline_serializer_spec.rb @@ -97,10 +97,8 @@ describe PipelineSerializer do describe '#represent_status' do context 'when represents only status' do - let(:status) do - Gitlab::Ci::Status::Success.new(double('object'), double('user')) - end - let(:resource) { create(:ci_pipeline, status: :success) } + let(:resource) { create(:ci_pipeline) } + let(:status) { resource.detailed_status(double('user')) } subject { serializer.represent_status(resource) }