WIP: Add tests and make sure that headers are set21698-redis-runner-last-build

* 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.

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