Merge branch 'feature/runner-config-untagged-jobs' into 'master'

Add config for CI Runner that prevents it from picking untagged jobs

Closes #3456

See merge request !4039

22 files changed, 314 insertions, 98 deletions

diff --git a/CHANGELOG b/CHANGELOG
index f07a4f65fd3..4c810ab21f2 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -6,6 +6,7 @@ v 8.8.0 (unreleased)
   - Assign labels and milestone to target project when moving issue. !3934 (Long Nguyen)
   - Use a case-insensitive comparison in sanitizing URI schemes
   - Toggle sign-up confirmation emails in application settings
+  - Make it possible to prevent tagged runner from picking untagged jobs
   - Project#open_branches has been cleaned up and no longer loads entire records into memory.
   - Escape HTML in commit titles in system note messages
   - Fix creation of Ci::Commit object which can lead to pending, failed in some scenarios
diff --git a/app/controllers/admin/runners_controller.rb b/app/controllers/admin/runners_controller.rb
index 8b8a7320072..7345c91f67d 100644
--- a/app/controllers/admin/runners_controller.rb
+++ b/app/controllers/admin/runners_controller.rb
@@ -9,23 +9,18 @@ class Admin::RunnersController < Admin::ApplicationController
   end
 
   def show
-    @builds = @runner.builds.order('id DESC').first(30)
-    @projects =
-      if params[:search].present?
-        ::Project.search(params[:search])
-      else
-        Project.all
-      end
-    @projects = @projects.where.not(id: @runner.projects.select(:id)) if @runner.projects.any?
-    @projects = @projects.page(params[:page]).per(30)
+    assign_builds_and_projects
   end
 
   def update
-    @runner.update_attributes(runner_params)
-
-    respond_to do |format|
-      format.js
-      format.html { redirect_to admin_runner_path(@runner) }
+    if @runner.update_attributes(runner_params)
+      respond_to do |format|
+        format.js
+        format.html { redirect_to admin_runner_path(@runner) }
+      end
+    else
+      assign_builds_and_projects
+      render 'show'
     end
   end
 
@@ -60,4 +55,16 @@ class Admin::RunnersController < Admin::ApplicationController
   def runner_params
     params.require(:runner).permit(Ci::Runner::FORM_EDITABLE)
   end
+
+  def assign_builds_and_projects
+    @builds = runner.builds.order('id DESC').first(30)
+    @projects =
+      if params[:search].present?
+        ::Project.search(params[:search])
+      else
+        Project.all
+      end
+    @projects = @projects.where.not(id: runner.projects.select(:id)) if runner.projects.any?
+    @projects = @projects.page(params[:page]).per(30)
+  end
 end
diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb
index 3a9d67aff64..0b4fa572501 100644
--- a/app/controllers/projects/runners_controller.rb
+++ b/app/controllers/projects/runners_controller.rb
@@ -20,7 +20,7 @@ class Projects::RunnersController < Projects::ApplicationController
     if @runner.update_attributes(runner_params)
       redirect_to runner_path(@runner), notice: 'Runner was successfully updated.'
     else
-      redirect_to runner_path(@runner), alert: 'Runner was not updated.'
+      render 'edit'
     end
   end
 
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 50190101f2b..ff7dd44c526 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -291,9 +291,15 @@ module Ci
     end
 
     def can_be_served?(runner)
+      return false unless has_tags? || runner.run_untagged?
+
       (tag_list - runner.tag_list).empty?
     end
 
+    def has_tags?
+      tag_list.any?
+    end
+
     def any_runners_online?
       project.any_runners? { |runner| runner.active? && runner.online? && can_be_served?(runner) }
     end
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 819064f99bb..6829dc91cb9 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -4,7 +4,7 @@ module Ci
 
     LAST_CONTACT_TIME = 5.minutes.ago
     AVAILABLE_SCOPES = %w[specific shared active paused online]
-    FORM_EDITABLE = %i[description tag_list active]
+    FORM_EDITABLE = %i[description tag_list active run_untagged]
 
     has_many :builds, class_name: 'Ci::Build'
     has_many :runner_projects, dependent: :destroy, class_name: 'Ci::RunnerProject'
@@ -26,6 +26,8 @@ module Ci
         .where("ci_runner_projects.gl_project_id = :project_id OR ci_runners.is_shared = true", project_id: project_id)
     end
 
+    validate :tag_constraints
+
     acts_as_taggable
 
     # Searches for runners matching the given query.
@@ -96,5 +98,18 @@ module Ci
     def short_sha
       token[0...8] if token
     end
+
+    def has_tags?
+      tag_list.any?
+    end
+
+    private
+
+    def tag_constraints
+      unless has_tags? || run_untagged?
+        errors.add(:tags_list,
+          'can not be empty when runner is not allowed to pick untagged jobs')
+      end
+    end
   end
 end
diff --git a/app/views/admin/runners/show.html.haml b/app/views/admin/runners/show.html.haml
index 4dfb3ed05bb..c3784bf7192 100644
--- a/app/views/admin/runners/show.html.haml
+++ b/app/views/admin/runners/show.html.haml
@@ -9,8 +9,6 @@
         %span.runner-state.runner-state-specific
           Specific
 
-
-
 - if @runner.shared?
   .bs-callout.bs-callout-success
     %h4 This runner will process builds from ALL UNASSIGNED projects
diff --git a/app/views/projects/runners/_form.html.haml b/app/views/projects/runners/_form.html.haml
index 2d6c964ae94..d62f5c8f131 100644
--- a/app/views/projects/runners/_form.html.haml
+++ b/app/views/projects/runners/_form.html.haml
@@ -1,4 +1,5 @@
 = form_for runner, url: runner_form_url, html: { class: 'form-horizontal' } do |f|
+  = form_errors(runner)
   .form-group
     = label :active, "Active", class: 'control-label'
     .col-sm-10
@@ -6,6 +7,12 @@
         = f.check_box :active
         %span.light Paused runners don't accept new builds
   .form-group
+    = label :run_untagged, 'Run untagged jobs', class: 'control-label'
+    .col-sm-10
+      .checkbox
+        = f.check_box :run_untagged
+        %span.light Indicates whether this runner can pick jobs without tags
+  .form-group
     = label_tag :token, class: 'control-label' do
       Token
     .col-sm-10
diff --git a/app/views/projects/runners/_runner.html.haml b/app/views/projects/runners/_runner.html.haml
index 47ec420189d..96e2aac451f 100644
--- a/app/views/projects/runners/_runner.html.haml
+++ b/app/views/projects/runners/_runner.html.haml
@@ -5,7 +5,7 @@
       - if @runners.include?(runner)
         = link_to runner.short_sha, runner_path(runner)
         %small
-          =link_to edit_namespace_project_runner_path(@project.namespace, @project, runner) do
+          = link_to edit_namespace_project_runner_path(@project.namespace, @project, runner) do
             %i.fa.fa-edit.btn
       - else
         = runner.short_sha
diff --git a/app/views/projects/runners/edit.html.haml b/app/views/projects/runners/edit.html.haml
index 771947d7908..95706888655 100644
--- a/app/views/projects/runners/edit.html.haml
+++ b/app/views/projects/runners/edit.html.haml
@@ -1,5 +1,6 @@
 - page_title "Edit", "#{@runner.description} ##{@runner.id}", "Runners"
 
 %h4 Runner ##{@runner.id}
+
 %hr
  = render 'form', runner: @runner, runner_form_url: runner_path(@runner)
diff --git a/app/views/projects/runners/show.html.haml b/app/views/projects/runners/show.html.haml
index 5bf4c09ca25..f24e1b9144e 100644
--- a/app/views/projects/runners/show.html.haml
+++ b/app/views/projects/runners/show.html.haml
@@ -17,50 +17,39 @@
         %th Property Name
         %th Value
     %tr
-      %td
-        Tags
+      %td Active
+      %td= @runner.active? ? 'Yes' : 'No'
+    %tr
+      %td Can run untagged jobs
+      %td= @runner.run_untagged? ? 'Yes' : 'No'
+    %tr
+      %td Tags
       %td
         - @runner.tag_list.each do |tag|
           %span.label.label-primary
             = tag
     %tr
-      %td
-        Name
-      %td
-        = @runner.name
+      %td Name
+      %td= @runner.name
     %tr
-      %td
-        Version
-      %td
-        = @runner.version
+      %td Version
+      %td= @runner.version
     %tr
-      %td
-        Revision
-      %td
-        = @runner.revision
+      %td Revision
+      %td= @runner.revision
     %tr
-      %td
-        Platform
-      %td
-        = @runner.platform
+      %td Platform
+      %td= @runner.platform
     %tr
-      %td
-        Architecture
-      %td
-        = @runner.architecture
+      %td Architecture
+      %td= @runner.architecture
     %tr
-      %td
-        Description
-      %td
-        = @runner.description
+      %td Description
+      %td= @runner.description
     %tr
-      %td
-        Last contact
+      %td Last contact
       %td
         - if @runner.contacted_at
           #{time_ago_in_words(@runner.contacted_at)} ago
         - else
           Never
-
-
-        
diff --git a/db/migrate/20160504112519_add_run_untagged_to_ci_runner.rb b/db/migrate/20160504112519_add_run_untagged_to_ci_runner.rb
new file mode 100644
index 00000000000..84e5e4eabe2
--- /dev/null
+++ b/db/migrate/20160504112519_add_run_untagged_to_ci_runner.rb
@@ -0,0 +1,13 @@
+class AddRunUntaggedToCiRunner < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:ci_runners, :run_untagged, :boolean,
+                            default: true, allow_null: false)
+  end
+
+  def down
+    remove_column(:ci_runners, :run_untagged)
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index af4f4c609e7..2e154b98b34 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -269,6 +269,7 @@ ActiveRecord::Schema.define(version: 20160509201028) do
     t.string   "revision"
     t.string   "platform"
     t.string   "architecture"
+    t.boolean  "run_untagged", default: true,  null: false
   end
 
   add_index "ci_runners", ["description"], name: "index_ci_runners_on_description_trigram", using: :gin, opclasses: {"description"=>"gin_trgm_ops"}
diff --git a/doc/ci/runners/README.md b/doc/ci/runners/README.md
index a06650b3387..b42d7a62ebc 100644
--- a/doc/ci/runners/README.md
+++ b/doc/ci/runners/README.md
@@ -125,7 +125,13 @@ shared runners will only run the jobs they are equipped to run.
 For instance, at GitLab we have runners tagged with "rails" if they contain
 the appropriate dependencies to run Rails test suites.
 
-### Be Careful with Sensitive Information
+### Prevent runner with tags from picking jobs without tags
+
+You can configure a runner to prevent it from picking jobs with tags when
+the runnner does not have tags assigned. This setting is available on each
+runner in *Project Settings* > *Runners*.
+
+### Be careful with sensitive information
 
 If you can run a build on a runner, you can get access to any code it runs
 and get the token of the runner. With shared runners, this means that anyone
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index dbd03ea74fa..8298e3ad34c 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -408,6 +408,7 @@ module API
 
     class RunnerDetails < Runner
       expose :tag_list
+      expose :run_untagged
       expose :version, :revision, :platform, :architecture
       expose :contacted_at
       expose :token, if: lambda { |runner, options| options[:current_user].is_admin? || !runner.is_shared? }
diff --git a/lib/api/runners.rb b/lib/api/runners.rb
index 8ec91485b26..4faba9dc87b 100644
--- a/lib/api/runners.rb
+++ b/lib/api/runners.rb
@@ -49,7 +49,7 @@ module API
         runner = get_runner(params[:id])
         authenticate_update_runner!(runner)
 
-        attrs = attributes_for_keys [:description, :active, :tag_list]
+        attrs = attributes_for_keys [:description, :active, :tag_list, :run_untagged]
         if runner.update(attrs)
           present runner, with: Entities::RunnerDetails, current_user: current_user
         else
diff --git a/lib/ci/api/runners.rb b/lib/ci/api/runners.rb
index 192b1d18a51..0c41f22c7c5 100644
--- a/lib/ci/api/runners.rb
+++ b/lib/ci/api/runners.rb
@@ -28,20 +28,20 @@ module Ci
         post "register" do
           required_attributes! [:token]
 
+          attributes = { description: params[:description],
+                         tag_list: params[:tag_list] }
+
+          unless params[:run_untagged].nil?
+            attributes[:run_untagged] = params[:run_untagged]
+          end
+
           runner =
             if runner_registration_token_valid?
               # Create shared runner. Requires admin access
-              Ci::Runner.create(
-                description: params[:description],
-                tag_list: params[:tag_list],
-                is_shared: true
-              )
+              Ci::Runner.create(attributes.merge(is_shared: true))
             elsif project = Project.find_by(runners_token: params[:token])
               # Create a specific runner for project.
-              project.runners.create(
-                description: params[:description],
-                tag_list: params[:tag_list]
-              )
+              project.runners.create(attributes)
             end
 
           return forbidden! unless runner
diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb
index 8edeb8d18af..49156130ad3 100644
--- a/spec/features/runners_spec.rb
+++ b/spec/features/runners_spec.rb
@@ -110,4 +110,37 @@ describe "Runners" do
       expect(page).to have_content(@specific_runner.platform)
     end
   end
+
+  feature 'configuring runners ability to picking untagged jobs' do
+    given(:project) { create(:empty_project) }
+    given(:runner) { create(:ci_runner) }
+
+    background do
+      project.team << [user, :master]
+      project.runners << runner
+    end
+
+    scenario 'user checks default configuration' do
+      visit namespace_project_runner_path(project.namespace, project, runner)
+
+      expect(page).to have_content 'Can run untagged jobs Yes'
+    end
+
+    context 'when runner has tags' do
+      before { runner.update_attribute(:tag_list, ['tag']) }
+
+      scenario 'user wants to prevent runner from running untagged job' do
+        visit runners_path(project)
+        page.within('.activated-specific-runners') do
+          first('small > a').click
+        end
+
+        uncheck 'runner_run_untagged'
+        click_button 'Save changes'
+
+        expect(page).to have_content 'Can run untagged jobs No'
+        expect(runner.reload.run_untagged?).to eq false
+      end
+    end
+  end
 end
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index b5d356aa066..abae3271a5c 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -259,11 +259,11 @@ describe Ci::Build, models: true do
   end
 
   describe '#can_be_served?' do
-    let(:runner) { FactoryGirl.create :ci_runner }
+    let(:runner) { create(:ci_runner) }
 
     before { build.project.runners << runner }
 
-    context 'runner without tags' do
+    context 'when runner does not have tags' do
       it 'can handle builds without tags' do
         expect(build.can_be_served?(runner)).to be_truthy
       end
@@ -274,25 +274,53 @@ describe Ci::Build, models: true do
       end
     end
 
-    context 'runner with tags' do
+    context 'when runner has tags' do
       before { runner.tag_list = ['bb', 'cc'] }
 
-      it 'can handle builds without tags' do
-        expect(build.can_be_served?(runner)).to be_truthy
+      shared_examples 'tagged build picker' do
+        it 'can handle build with matching tags' do
+          build.tag_list = ['bb']
+          expect(build.can_be_served?(runner)).to be_truthy
+        end
+
+        it 'cannot handle build without matching tags' do
+          build.tag_list = ['aa']
+          expect(build.can_be_served?(runner)).to be_falsey
+        end
       end
 
-      it 'can handle build with matching tags' do
-        build.tag_list = ['bb']
-        expect(build.can_be_served?(runner)).to be_truthy
+      context 'when runner can pick untagged jobs' do
+        it 'can handle builds without tags' do
+          expect(build.can_be_served?(runner)).to be_truthy
+        end
+
+        it_behaves_like 'tagged build picker'
       end
 
-      it 'cannot handle build with not matching tags' do
-        build.tag_list = ['aa']
-        expect(build.can_be_served?(runner)).to be_falsey
+      context 'when runner can not pick untagged jobs' do
+        before { runner.run_untagged = false }
+
+        it 'can not handle builds without tags' do
+          expect(build.can_be_served?(runner)).to be_falsey
+        end
+
+        it_behaves_like 'tagged build picker'
       end
     end
   end
 
+  describe '#has_tags?' do
+    context 'when build has tags' do
+      subject { create(:ci_build, tag_list: ['tag']) }
+      it { is_expected.to have_tags }
+    end
+
+    context 'when build does not have tags' do
+      subject { create(:ci_build, tag_list: []) }
+      it { is_expected.to_not have_tags }
+    end
+  end
+
   describe '#any_runners_online?' do
     subject { build.any_runners_online? }
 
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index eaa94228922..7c6e39419ed 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -1,6 +1,24 @@
 require 'spec_helper'
 
 describe Ci::Runner, models: true do
+  describe 'validation' do
+    context 'when runner is not allowed to pick untagged jobs' do
+      context 'when runner does not have tags' do
+        it 'is not valid' do
+          runner = build(:ci_runner, tag_list: [], run_untagged: false)
+          expect(runner).to be_invalid
+        end
+      end
+
+      context 'when runner has tags' do
+        it 'is valid' do
+          runner = build(:ci_runner, tag_list: ['tag'], run_untagged: false)
+          expect(runner).to be_valid
+        end
+      end
+    end
+  end
+
   describe '#display_name' do
     it 'should return the description if it has a value' do
       runner = FactoryGirl.build(:ci_runner, description: 'Linux/Ruby-1.9.3-p448')
@@ -114,7 +132,19 @@ describe Ci::Runner, models: true do
     end
   end
 
-  describe '#search' do
+  describe '#has_tags?' do
+    context 'when runner has tags' do
+      subject { create(:ci_runner, tag_list: ['tag']) }
+      it { is_expected.to have_tags }
+    end
+
+    context 'when runner does not have tags' do
+      subject { create(:ci_runner, tag_list: []) }
+      it { is_expected.to_not have_tags }
+    end
+  end
+
+  describe '.search' do
     let(:runner) { create(:ci_runner, token: '123abc') }
 
     it 'returns runners with a matching token' do
diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb
index 3af61d4b335..73ae8ef631c 100644
--- a/spec/requests/api/runners_spec.rb
+++ b/spec/requests/api/runners_spec.rb
@@ -184,21 +184,24 @@ describe API::Runners, api: true  do
           description = shared_runner.description
           active = shared_runner.active
 
-          put api("/runners/#{shared_runner.id}", admin), description: "#{description}_updated", active: !active,
-                                                          tag_list: ['ruby2.1', 'pgsql', 'mysql']
+          update_runner(shared_runner.id, admin, description: "#{description}_updated",
+                                                 active: !active,
+                                                 tag_list: ['ruby2.1', 'pgsql', 'mysql'],
+                                                 run_untagged: 'false')
           shared_runner.reload
 
           expect(response.status).to eq(200)
           expect(shared_runner.description).to eq("#{description}_updated")
           expect(shared_runner.active).to eq(!active)
           expect(shared_runner.tag_list).to include('ruby2.1', 'pgsql', 'mysql')
+          expect(shared_runner.run_untagged?).to be false
         end
       end
 
       context 'when runner is not shared' do
         it 'should update runner' do
           description = specific_runner.description
-          put api("/runners/#{specific_runner.id}", admin), description: 'test'
+          update_runner(specific_runner.id, admin, description: 'test')
           specific_runner.reload
 
           expect(response.status).to eq(200)
@@ -208,10 +211,14 @@ describe API::Runners, api: true  do
       end
 
       it 'should return 404 if runner does not exists' do
-        put api('/runners/9999', admin), description: 'test'
+        update_runner(9999, admin, description: 'test')
 
         expect(response.status).to eq(404)
       end
+
+      def update_runner(id, user, args)
+        put api("/runners/#{id}", user), args
+      end
     end
 
     context 'authorized user' do
diff --git a/spec/requests/ci/api/builds_spec.rb b/spec/requests/ci/api/builds_spec.rb
index cae4656010f..7ebf8e41f3b 100644
--- a/spec/requests/ci/api/builds_spec.rb
+++ b/spec/requests/ci/api/builds_spec.rb
@@ -128,6 +128,38 @@ describe Ci::API::API do
           end
         end
       end
+
+      context 'when build has no tags' do
+        before do
+          commit = create(:ci_commit, project: project)
+          create(:ci_build, commit: commit, tags: [])
+        end
+
+        context 'when runner is allowed to pick untagged builds' do
+          before { runner.update_column(:run_untagged, true) }
+
+          it 'picks build' do
+            register_builds
+
+            expect(response).to have_http_status 201
+          end
+        end
+
+        context 'when runner is not allowed to pick untagged builds' do
+          before { runner.update_column(:run_untagged, false) }
+
+          it 'does not pick build' do
+            register_builds
+
+            expect(response).to have_http_status 404
+          end
+        end
+
+        def register_builds
+          post ci_api("/builds/register"), token: runner.token,
+                                           info: { platform: :darwin }
+        end
+      end
     end
 
     describe "PUT /builds/:id" do
diff --git a/spec/requests/ci/api/runners_spec.rb b/spec/requests/ci/api/runners_spec.rb
index db8189ffb79..43596f07cb5 100644
--- a/spec/requests/ci/api/runners_spec.rb
+++ b/spec/requests/ci/api/runners_spec.rb
@@ -12,44 +12,85 @@ describe Ci::API::API do
   end
 
   describe "POST /runners/register" do
-    describe "should create a runner if token provided" do
+    context 'when runner token is provided' do
       before { post ci_api("/runners/register"), token: registration_token }
 
-      it { expect(response.status).to eq(201) }
+      it 'creates runner with default values' do
+        expect(response).to have_http_status 201
+        expect(Ci::Runner.first.run_untagged).to be true
+      end
     end
 
-    describe "should create a runner with description" do
-      before { post ci_api("/runners/register"), token: registration_token, description: "server.hostname" }
+    context 'when runner description is provided' do
+      before do
+        post ci_api("/runners/register"), token: registration_token,
+                                          description: "server.hostname"
+      end
 
-      it { expect(response.status).to eq(201) }
-      it { expect(Ci::Runner.first.description).to eq("server.hostname") }
+      it 'creates runner' do
+        expect(response).to have_http_status 201
+        expect(Ci::Runner.first.description).to eq("server.hostname")
+      end
     end
 
-    describe "should create a runner with tags" do
-      before { post ci_api("/runners/register"), token: registration_token, tag_list: "tag1, tag2" }
+    context 'when runner tags are provided' do
+      before do
+        post ci_api("/runners/register"), token: registration_token,
+                                          tag_list: "tag1, tag2"
+      end
 
-      it { expect(response.status).to eq(201) }
-      it { expect(Ci::Runner.first.tag_list.sort).to eq(["tag1", "tag2"]) }
+      it 'creates runner' do
+        expect(response).to have_http_status 201
+        expect(Ci::Runner.first.tag_list.sort).to eq(["tag1", "tag2"])
+      end
     end
 
-    describe "should create a runner if project token provided" do
+    context 'when option for running untagged jobs is provided' do
+      context 'when tags are provided' do
+        it 'creates runner' do
+          post ci_api("/runners/register"), token: registration_token,
+                                            run_untagged: false,
+                                            tag_list: ['tag']
+
+          expect(response).to have_http_status 201
+          expect(Ci::Runner.first.run_untagged).to be false
+        end
+      end
+
+      context 'when tags are not provided' do
+        it 'does not create runner' do
+          post ci_api("/runners/register"), token: registration_token,
+                                            run_untagged: false
+
+          expect(response).to have_http_status 404
+        end
+      end
+    end
+
+    context 'when project token is provided' do
       let(:project) { FactoryGirl.create(:empty_project) }
       before { post ci_api("/runners/register"), token: project.runners_token }
 
-      it { expect(response.status).to eq(201) }
-      it { expect(project.runners.size).to eq(1) }
+      it 'creates runner' do
+        expect(response).to have_http_status 201
+        expect(project.runners.size).to eq(1)
+      end
     end
 
-    it "should return 403 error if token is invalid" do
-      post ci_api("/runners/register"), token: 'invalid'
+    context 'when token is invalid' do
+      it 'returns 403 error' do
+        post ci_api("/runners/register"), token: 'invalid'
 
-      expect(response.status).to eq(403)
+        expect(response).to have_http_status 403
+      end
     end
 
-    it "should return 400 error if no token" do
-      post ci_api("/runners/register")
+    context 'when no token provided' do
+      it 'returns 400 error' do
+        post ci_api("/runners/register")
 
-      expect(response.status).to eq(400)
+        expect(response).to have_http_status 400
+      end
     end
 
     %w(name version revision platform architecture).each do |param|
@@ -60,7 +101,7 @@ describe Ci::API::API do
 
         it do
           post ci_api("/runners/register"), token: registration_token, info: { param => value }
-          expect(response.status).to eq(201)
+          expect(response).to have_http_status 201
           is_expected.to eq(value)
         end
       end
@@ -71,7 +112,7 @@ describe Ci::API::API do
     let!(:runner) { FactoryGirl.create(:ci_runner) }
     before { delete ci_api("/runners/delete"), token: runner.token }
 
-    it { expect(response.status).to eq(200) }
+    it { expect(response).to have_http_status 200 }
     it { expect(Ci::Runner.count).to eq(0) }
   end
 end