From c191c1103b37903f2293c2a662cdc616228b9eb7 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 16 Mar 2017 14:45:05 +0100 Subject: [PATCH] Send only defined dependencies In APIv1 we've been sending all jobs from previous stages and a `dependencies` list with names of jobs that user want to download artifacts from. This was selected on Runners side. In APIv1 we've planned to send only jobs that were defined (if any; and all previous jobs by default). However I've missed the fact that it was Runner who selected jobs, not GitLab. And now current version of APIV4 sends all jobs everytime. This commit fixes this. If user will define `dependencies` in his job, then GitLab will send only selected jobs. --- app/models/ci/build.rb | 15 +++++++++++++ lib/api/entities.rb | 2 +- spec/requests/api/runner_spec.rb | 37 ++++++++++++++++++++++++++++---- 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 53fc0d87823..90a195dc048 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -539,6 +539,21 @@ module Ci Gitlab::Ci::Build::Credentials::Factory.new(self).create! end + def dependencies + depended_jobs = depends_on_builds + + return depended_jobs unless options[:dependencies] && !options[:dependencies].empty? + + selected = [] + depended_jobs.each do |job| + options[:dependencies].each do |job_name| + selected << job if job.name == job_name + end + end + + selected + end + private def update_artifacts_size diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 29c640ddb19..5954aea8041 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -796,7 +796,7 @@ module API expose :artifacts, using: Artifacts expose :cache, using: Cache expose :credentials, using: Credentials - expose :depends_on_builds, as: :dependencies, using: Dependency + expose :dependencies, using: Dependency end end end diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb index 76461aabd9a..d4d6ac51c03 100644 --- a/spec/requests/api/runner_spec.rb +++ b/spec/requests/api/runner_spec.rb @@ -417,10 +417,39 @@ describe API::Runner do end context 'when project and pipeline have multiple jobs' do - let!(:job) { create(:ci_build_tag, pipeline: pipeline, token: 'job-token', name: 'spinach', stage: 'test', stage_idx: 0) } - let!(:test_job) { create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'deploy', stage: 'deploy', stage_idx: 1) } + let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + let!(:job2) { create(:ci_build_tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) } + let!(:test_job) { create(:ci_build, pipeline: pipeline, name: 'deploy', stage: 'deploy', stage_idx: 1) } - before { job.success } + before do + job.success + job2.success + end + + it 'returns dependent jobs' do + request_job + + expect(response).to have_http_status(201) + expect(json_response['id']).to eq(test_job.id) + expect(json_response['dependencies'].count).to eq(2) + expect(json_response['dependencies']).to include({ 'id' => job.id, 'name' => job.name, 'token' => job.token }, + { 'id' => job2.id, 'name' => job2.name, 'token' => job2.token }) + end + end + + context 'when explicit dependencies are defined' do + let!(:job) { create(:ci_build_tag, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } + let!(:job2) { create(:ci_build_tag, pipeline: pipeline, name: 'rubocop', stage: 'test', stage_idx: 0) } + let!(:test_job) do + create(:ci_build, pipeline: pipeline, token: 'test-job-token', name: 'deploy', + stage: 'deploy', stage_idx: 1, + options: { dependencies: [job2.name] }) + end + + before do + job.success + job2.success + end it 'returns dependent jobs' do request_job @@ -428,7 +457,7 @@ describe API::Runner do expect(response).to have_http_status(201) expect(json_response['id']).to eq(test_job.id) expect(json_response['dependencies'].count).to eq(1) - expect(json_response['dependencies'][0]).to include('id' => job.id, 'name' => 'spinach', 'token' => job.token) + expect(json_response['dependencies'][0]).to include('id' => job2.id, 'name' => job2.name, 'token' => job2.token) end end