From 8c9a4ed373f4b517aeae669e64023dc52c8d704a Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 4 Jan 2017 17:46:56 +0800
Subject: [PATCH] WIP: Add tests and make sure that headers are set
* We realized that headers were not set whenever we give 204
because `render_api_error!` doesn't preserve the headers.
* We also realized that `update_runner_info` would be called in
POST /builds/register every time therefore runner is updated
every time, ticking the queue, making this last_update didn't
work very well, and the test would be failing due to that.
---
lib/api/helpers.rb | 2 +-
lib/ci/api/builds.rb | 4 ++--
spec/models/build_spec.rb | 10 +++++++++
spec/models/ci/runner_spec.rb | 33 +++++++++++++++++++++++++++++
spec/requests/ci/api/builds_spec.rb | 32 ++++++++++++++++++++++++++--
5 files changed, 76 insertions(+), 5 deletions(-)
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 746849ef4c0..3324001c468 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -229,7 +229,7 @@ module API
end
def render_api_error!(message, status)
- error!({ 'message' => message }, status)
+ error!({ 'message' => message }, status, header)
end
def handle_api_exception(exception)
diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb
index 8264210c460..de3e224bcee 100644
--- a/lib/ci/api/builds.rb
+++ b/lib/ci/api/builds.rb
@@ -17,7 +17,7 @@ module Ci
update_runner_info
if current_runner.is_runner_queue_value_latest?(params[:last_update])
- headers 'X-GitLab-Last-Update', params[:last_update]
+ header 'X-GitLab-Last-Update', params[:last_update]
return build_not_found!
end
@@ -33,7 +33,7 @@ module Ci
else
Gitlab::Metrics.add_event(:build_not_found)
- headers 'X-GitLab-Last-Update', new_update
+ header 'X-GitLab-Last-Update', new_update
build_not_found!
end
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index d4970e38f7c..e902d9ef73d 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -1180,4 +1180,14 @@ describe Ci::Build, models: true do
it { is_expected.to eq('review/master') }
end
end
+
+ describe 'State transition: any => [:pending]' do
+ let(:build) { create(:ci_build, :created) }
+
+ it 'queues BuildQueueWorker' do
+ expect(BuildQueueWorker).to receive(:perform_async).with(build.id)
+
+ build.enqueue
+ end
+ end
end
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index ef65eb99328..06eaa6d04d9 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -263,6 +263,39 @@ describe Ci::Runner, models: true do
end
end
+ describe '#tick_runner_queue' do
+ let(:runner) { create(:ci_runner) }
+
+ it 'returns a new last_update value' do
+ expect(runner.tick_runner_queue).not_to be_empty
+ end
+ end
+
+ describe '#ensure_runner_queue_value' do
+ let(:runner) { create(:ci_runner) }
+
+ it 'sets a new last_update value when it is called the first time' do
+ last_update = runner.ensure_runner_queue_value
+
+ expect_value_in_redis(last_update)
+ end
+
+ it 'does not change if it is not expired and called again' do
+ last_update = runner.ensure_runner_queue_value
+
+ expect(runner.ensure_runner_queue_value).to eq(last_update)
+ expect_value_in_redis(last_update)
+ end
+
+ def expect_value_in_redis(last_update)
+ Gitlab::Redis.with do |redis|
+ runner_queue_key = runner.send(:runner_queue_key)
+
+ expect(redis.get(runner_queue_key)).to eq(last_update)
+ end
+ end
+ end
+
describe '.assignable_for' do
let(:runner) { create(:ci_runner) }
let(:project) { create(:project) }
diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index 80652129928..8ccae0d5bff 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -5,6 +5,7 @@ describe Ci::API::Builds do
let(:runner) { FactoryGirl.create(:ci_runner, tag_list: ["mysql", "ruby"]) }
let(:project) { FactoryGirl.create(:empty_project) }
+ let(:last_update) { nil }
describe "Builds API for runners" do
let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') }
@@ -24,7 +25,31 @@ describe Ci::API::Builds do
shared_examples 'no builds available' do
context 'when runner sends version in User-Agent' do
context 'for stable version' do
- it { expect(response).to have_http_status(204) }
+ it 'gives 204 and set X-GitLab-Last-Update' do
+ expect(response).to have_http_status(204)
+ expect(response.header).to have_key('X-GitLab-Last-Update')
+ end
+ end
+
+ context 'when last_update is up-to-date' do
+ let(:last_update) { runner.ensure_runner_queue_value }
+
+ it 'gives 204 and set the same X-GitLab-Last-Update' do
+ expect(response).to have_http_status(204)
+ expect(response.header['X-GitLab-Last-Update'])
+ .to eq(last_update)
+ end
+ end
+
+ context 'when last_update is outdated' do
+ let(:last_update) { runner.ensure_runner_queue_value }
+ let!(:new_update) { runner.tick_runner_queue }
+
+ it 'gives 204 and set a new X-GitLab-Last-Update' do
+ expect(response).to have_http_status(204)
+ expect(response.header['X-GitLab-Last-Update'])
+ .to eq(new_update)
+ end
end
context 'for beta version' do
@@ -44,6 +69,7 @@ describe Ci::API::Builds do
register_builds info: { platform: :darwin }
expect(response).to have_http_status(201)
+ expect(response.headers).not_to have_key('X-GitLab-Last-Update')
expect(json_response['sha']).to eq(build.sha)
expect(runner.reload.platform).to eq("darwin")
expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] })
@@ -219,7 +245,9 @@ describe Ci::API::Builds do
end
def register_builds(token = runner.token, **params)
- post ci_api("/builds/register"), params.merge(token: token), { 'User-Agent' => user_agent }
+ new_params = params.merge(token: token, last_update: last_update)
+
+ post ci_api("/builds/register"), new_params, { 'User-Agent' => user_agent }
end
end
--
GitLab