Commit 8c9a4ed3 authored by Lin Jen-Shin's avatar Lin Jen-Shin

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.
parent f35336a1
...@@ -229,7 +229,7 @@ module API ...@@ -229,7 +229,7 @@ module API
end end
def render_api_error!(message, status) def render_api_error!(message, status)
error!({ 'message' => message }, status) error!({ 'message' => message }, status, header)
end end
def handle_api_exception(exception) def handle_api_exception(exception)
......
...@@ -17,7 +17,7 @@ module Ci ...@@ -17,7 +17,7 @@ module Ci
update_runner_info update_runner_info
if current_runner.is_runner_queue_value_latest?(params[:last_update]) 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! return build_not_found!
end end
...@@ -33,7 +33,7 @@ module Ci ...@@ -33,7 +33,7 @@ module Ci
else else
Gitlab::Metrics.add_event(:build_not_found) Gitlab::Metrics.add_event(:build_not_found)
headers 'X-GitLab-Last-Update', new_update header 'X-GitLab-Last-Update', new_update
build_not_found! build_not_found!
end end
......
...@@ -1180,4 +1180,14 @@ describe Ci::Build, models: true do ...@@ -1180,4 +1180,14 @@ describe Ci::Build, models: true do
it { is_expected.to eq('review/master') } it { is_expected.to eq('review/master') }
end end
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 end
...@@ -263,6 +263,39 @@ describe Ci::Runner, models: true do ...@@ -263,6 +263,39 @@ describe Ci::Runner, models: true do
end end
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 describe '.assignable_for' do
let(:runner) { create(:ci_runner) } let(:runner) { create(:ci_runner) }
let(:project) { create(:project) } let(:project) { create(:project) }
......
...@@ -5,6 +5,7 @@ describe Ci::API::Builds do ...@@ -5,6 +5,7 @@ describe Ci::API::Builds do
let(:runner) { FactoryGirl.create(:ci_runner, tag_list: ["mysql", "ruby"]) } let(:runner) { FactoryGirl.create(:ci_runner, tag_list: ["mysql", "ruby"]) }
let(:project) { FactoryGirl.create(:empty_project) } let(:project) { FactoryGirl.create(:empty_project) }
let(:last_update) { nil }
describe "Builds API for runners" do describe "Builds API for runners" do
let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') } let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') }
...@@ -24,7 +25,31 @@ describe Ci::API::Builds do ...@@ -24,7 +25,31 @@ describe Ci::API::Builds do
shared_examples 'no builds available' do shared_examples 'no builds available' do
context 'when runner sends version in User-Agent' do context 'when runner sends version in User-Agent' do
context 'for stable version' 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 end
context 'for beta version' do context 'for beta version' do
...@@ -44,6 +69,7 @@ describe Ci::API::Builds do ...@@ -44,6 +69,7 @@ describe Ci::API::Builds do
register_builds info: { platform: :darwin } register_builds info: { platform: :darwin }
expect(response).to have_http_status(201) 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(json_response['sha']).to eq(build.sha)
expect(runner.reload.platform).to eq("darwin") expect(runner.reload.platform).to eq("darwin")
expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] }) expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] })
...@@ -219,7 +245,9 @@ describe Ci::API::Builds do ...@@ -219,7 +245,9 @@ describe Ci::API::Builds do
end end
def register_builds(token = runner.token, **params) 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
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment