Improve commit builds API endpoint RESTful behavior

1. Return 404 if commit is not found (RESTful resource not found)
2. Return an empty array if pipeline is not found (resource present, no
   associated builds found)
3. Return an empty array if pipeline found but no builds there (resource
   present, no associated builds)

2 files changed, 51 insertions, 16 deletions

diff --git a/lib/api/builds.rb b/lib/api/builds.rb
index 33aa86999f5..b7987461fd1 100644
--- a/lib/api/builds.rb
+++ b/lib/api/builds.rb
@@ -33,8 +33,10 @@ module API
       get ':id/repository/commits/:sha/builds' do
         authorize_read_builds!
 
+        return not_found! unless user_project.commit(params[:sha])
+
         pipelines = user_project.pipelines.where(sha: params[:sha])
-        return not_found! if pipelines.empty?
+        return [] if pipelines.empty?
 
         builds = user_project.builds.where(pipeline: pipelines).order('id DESC')
         builds = filter_builds(builds, params[:scope])
diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb
index e9948b0afa2..f5b39c3d698 100644
--- a/spec/requests/api/builds_spec.rb
+++ b/spec/requests/api/builds_spec.rb
@@ -63,27 +63,60 @@ describe API::API, api: true  do
   end
 
   describe 'GET /projects/:id/repository/commits/:sha/builds' do
-    before do
-      create(:ci_pipeline, project: project, sha: project.commit.id)
-      create(:ci_build, pipeline: pipeline)
-      create(:ci_build)
+    context 'when commit does not exist in repository' do
+      before do
+        get api("/projects/#{project.id}/repository/commits/1a271fd1/builds", api_user)
+      end
 
-      get api("/projects/#{project.id}/repository/commits/#{project.commit.id}/builds", api_user)
+      it 'responds with 404' do
+        expect(response).to have_http_status(404)
+      end
     end
 
-    context 'authorized user' do
-      it 'should return project builds for specific commit' do
-        expect(response).to have_http_status(200)
-        expect(json_response).to be_an Array
-        expect(json_response.size).to eq 2
+    context 'when commit exists in repository' do
+      context 'when user is authorized' do
+        context 'when pipeline has builds' do
+          before do
+            create(:ci_pipeline, project: project, sha: project.commit.id)
+            create(:ci_build, pipeline: pipeline)
+            create(:ci_build)
+
+            get api("/projects/#{project.id}/repository/commits/#{project.commit.id}/builds", api_user)
+          end
+
+          it 'should return project builds for specific commit' do
+            expect(response).to have_http_status(200)
+            expect(json_response).to be_an Array
+            expect(json_response.size).to eq 2
+          end
+        end
+
+        context 'when pipeline has no builds' do
+          before do
+            branch_head = project.commit('feature').id
+            get api("/projects/#{project.id}/repository/commits/#{branch_head}/builds", api_user)
+          end
+
+          it 'returns an empty array' do
+            expect(response).to have_http_status(200)
+            expect(json_response).to be_an Array
+            expect(json_response).to be_empty
+          end
+        end
       end
-    end
 
-    context 'unauthorized user' do
-      let(:api_user) { nil }
+      context 'when user is not authorized' do
+        before do
+          create(:ci_pipeline, project: project, sha: project.commit.id)
+          create(:ci_build, pipeline: pipeline)
 
-      it 'should not return project builds' do
-        expect(response).to have_http_status(401)
+          get api("/projects/#{project.id}/repository/commits/#{project.commit.id}/builds", nil)
+        end
+
+        it 'should not return project builds' do
+          expect(response).to have_http_status(401)
+          expect(json_response.except('message')).to be_empty
+        end
       end
     end
   end