Merge remote-tracking branch 'ce/master'

CHANGELOG

@@ -9,6 +9,7 @@ v 8.1.0 (unreleased)
   - Speed up load times of issue detail pages by roughly 1.5x
   - Make diff file view easier to use on mobile screens (Stan Hu)
   - Improved performance of finding users by username or Email address
+  - Fix bug where merge request comments created by API would not trigger notifications (Stan Hu)
   - Add support for creating directories from Files page (Stan Hu)
   - Allow removing of project without confirmation when JavaScript is disabled (Stan Hu)
   - Support filtering by "Any" milestone or issue and fix "No Milestone" and "No Label" filters (Stan Hu)
@@ -25,6 +26,7 @@ v 8.1.0 (unreleased)
   - Add first and last to pagination (Zeger-Jan van de Weg)
   - Added Commit Status API
   - Added Builds View
+  - Added when to .gitlab-ci.yml
   - Show CI status on commit page
   - Added CI_BUILD_TAG, _STAGE, _NAME and _TRIGGERED to CI builds
   - Show CI status on Your projects page and Starred projects page

Gemfile

@@ -96,7 +96,7 @@ gem "seed-fu", '~> 2.3.5'
 gem 'html-pipeline', '~> 1.11.0'
 gem 'task_list',     '~> 1.0.2', require: 'task_list/railtie'
 gem 'github-markup', '~> 1.3.1'
-gem 'redcarpet',     '~> 3.3.2'
+gem 'redcarpet',     '~> 3.3.3'
 gem 'RedCloth',      '~> 4.2.9'
 gem 'rdoc',          '~>3.6'
 gem 'org-ruby',      '~> 0.9.12'

Gemfile.lock

@@ -542,7 +542,7 @@ GEM
       trollop
     rdoc (3.12.2)
       json (~> 1.4)
-    redcarpet (3.3.2)
+    redcarpet (3.3.3)
     redis (3.2.1)
     redis-actionpack (4.0.0)
       actionpack (~> 4)
@@ -887,7 +887,7 @@ DEPENDENCIES
   rails (= 4.1.12)
   raphael-rails (~> 2.1.2)
   rdoc (~> 3.6)
-  redcarpet (~> 3.3.2)
+  redcarpet (~> 3.3.3)
   redis-rails (~> 4.0.0)
   request_store (~> 1.2.0)
   rerun (~> 0.10.0)

app/assets/stylesheets/framework/typography.scss

@@ -101,9 +101,9 @@
 
   pre {
     margin: 12px 0 12px 0 !important;
-    background-color: #f8fafc !important;
+    background-color: #f8fafc;
     font-size: 13px !important;
-    color: #5b6169 !important;
+    color: #5b6169;
     line-height: 1.6em !important;
     @include border-radius(2px);
   }

app/assets/stylesheets/highlight/dark.scss

 /* https://github.com/MozMorris/tomorrow-pygments */
-pre.code.highlight.dark,
 .code.dark {
 
-  background-color: #1d1f21;
-  color: #c5c8c6;
+  background-color: #1d1f21 !important;
+  color: #c5c8c6 !important;
 
-  pre.code,
+  pre.highlight,
   .line-numbers,
   .line-numbers a {
     background-color: #1d1f21 !important;
@@ -23,8 +22,8 @@ pre.code.highlight.dark,
 
   // Search result highlight
   span.highlight_word {
-    background: #ffe792;
-    color: #000000;
+    background-color: #ffe792 !important;
+    color: #000000 !important;
   }
 
   .hll { background-color: #373b41 }

app/assets/stylesheets/highlight/monokai.scss

 /* https://github.com/richleland/pygments-css/blob/master/monokai.css */
-pre.code.monokai,
 .code.monokai {
 
-  background: #272822;
-  color: #f8f8f2;
+  background-color: #272822 !important;
+  color: #f8f8f2 !important;
 
   pre.highlight,
   .line-numbers,
   .line-numbers a {
-    background:#272822 !important;
-    color:#f8f8f2 !important;
+    background-color :#272822 !important;
+    color: #f8f8f2 !important;
   }
 
   pre.code {
@@ -23,8 +22,8 @@ pre.code.monokai,
 
   // Search result highlight
   span.highlight_word {
-    background: #ffe792;
-    color: #000000;
+    background-color: #ffe792 !important;
+    color: #000000 !important;
   }
 
   .hll { background-color: #49483e }

app/assets/stylesheets/highlight/solarized_dark.scss

 /* https://gist.github.com/qguv/7936275 */
-pre.code.highlight.solarized-dark,
 .code.solarized-dark {
 
-  background-color: #002b36;
-  color: #93a1a1;
+  background-color: #002b36 !important;
+  color: #93a1a1 !important;
 
-  pre.code,
+  pre.highlight,
   .line-numbers,
   .line-numbers a {
     background-color: #002b36 !important;
@@ -23,7 +22,7 @@ pre.code.highlight.solarized-dark,
 
   // Search result highlight
   span.highlight_word {
-    background: #094554;
+    background-color: #094554 !important;
   }
 
   /* Solarized Dark

app/assets/stylesheets/highlight/solarized_light.scss

 /* https://gist.github.com/qguv/7936275 */
-pre.code.highlight.solarized-light,
 .code.solarized-light {
 
-  background-color: #fdf6e3;
-  color: #586e75;
+  background-color: #fdf6e3 !important;
+  color: #586e75 !important;
 
-  pre.code,
+  pre.highlight,
   .line-numbers,
   .line-numbers a {
     background-color: #fdf6e3 !important;
@@ -23,7 +22,7 @@ pre.code.highlight.solarized-light,
 
   // Search result highlight
   span.highlight_word {
-    background: #eee8d5;
+    background-color: #eee8d5 !important;
   }
 
   /* Solarized Light

app/assets/stylesheets/highlight/white.scss

 /* https://github.com/aahan/pygments-github-style */
-pre.code.highlight.white,
 .code.white {
-  background-color: #f8fafc;
-  font-size: 13px;
-  color: #5b6169;
-  line-height: 1.6em;
 
+  background-color: #f8fafc !important;
+  color: #5b6169 !important;
+
+  pre.highlight,
   .line-numbers,
   .line-numbers a {
     background-color: $background-color !important;
     color: $gl-gray !important;
   }
 
-  pre.highlight {
-    background-color: #fff !important;
-    color: #333 !important;
-  }
-
   pre.code {
     border-left: 1px solid $border-color;
+    background-color: #fff !important;
+    color: #333 !important;
   }
 
   // highlight line via anchor
@@ -28,7 +24,7 @@ pre.code.highlight.white,
 
   // Search result highlight
   span.highlight_word {
-    background: #fafe3d;
+    background-color: #fafe3d !important;
   }
 
   .hll { background-color: #f8f8f8 }

app/controllers/projects/builds_controller.rb

@@ -8,8 +8,7 @@ class Projects::BuildsController < Projects::ApplicationController
 
   def index
     @scope = params[:scope]
-    @all_builds = project.ci_builds.order('created_at DESC').page(params[:page]).per(30)
-
+    @all_builds = project.ci_builds
     @builds =
       case @scope
       when 'all'
@@ -19,6 +18,7 @@ class Projects::BuildsController < Projects::ApplicationController
       else
         @all_builds.running_or_pending
       end
+    @builds = @builds.order('created_at DESC').page(params[:page]).per(30)
   end
 
   def cancel_all

app/helpers/runners_helper.rb

@@ -15,7 +15,7 @@ module RunnersHelper
   end
 
   def runner_link(runner)
-    display_name = truncate(runner.display_name, length: 20)
+    display_name = truncate(runner.display_name, length: 15)
     id = "\##{runner.id}"
 
     if current_user && current_user.admin

app/models/ci/build.rb

@@ -93,10 +93,7 @@ module Ci
           Ci::WebHookService.new.build_end(build)
         end
 
-        if build.commit.should_create_next_builds?(build)
-          build.commit.create_next_builds(build.ref, build.tag, build.user, build.trigger_request)
-        end
-
+        build.commit.create_next_builds(build)
         project.execute_services(build)
 
         if project.coverage_enabled?

app/models/ci/commit.rb

@@ -91,19 +91,28 @@ module Ci
     def create_builds(ref, tag, user, trigger_request = nil)
       return unless config_processor
       config_processor.stages.any? do |stage|
-        CreateBuildsService.new.execute(self, stage, ref, tag, user, trigger_request).present?
+        CreateBuildsService.new.execute(self, stage, ref, tag, user, trigger_request, 'success').present?
       end
     end
 
-    def create_next_builds(ref, tag, user, trigger_request)
+    def create_next_builds(build)
       return unless config_processor
 
-      stages = builds.where(ref: ref, tag: tag, trigger_request: trigger_request).group_by(&:stage)
+      # don't create other builds if this one is retried
+      latest_builds = builds.similar(build).latest
+      return unless latest_builds.exists?(build.id)
 
-      config_processor.stages.any? do |stage|
-        unless stages.include?(stage)
-          CreateBuildsService.new.execute(self, stage, ref, tag, user, trigger_request).present?
-        end
+      # get list of stages after this build
+      next_stages = config_processor.stages.drop_while { |stage| stage != build.stage }
+      next_stages.delete(build.stage)
+
+      # get status for all prior builds
+      prior_builds = latest_builds.reject { |other_build| next_stages.include?(other_build.stage) }
+      status = Ci::Status.get_status(prior_builds)
+
+      # create builds for next stages based
+      next_stages.any? do |stage|
+        CreateBuildsService.new.execute(self, stage, build.ref, build.tag, build.user, build.trigger_request, status).present?
       end
     end
 
@@ -132,24 +141,7 @@ module Ci
         return 'failed'
       end
 
-      @status ||= begin
-        latest = latest_statuses
-        latest.reject! { |status| status.try(&:allow_failure?) }
-
-        if latest.none?
-          'skipped'
-        elsif latest.all?(&:success?)
-          'success'
-        elsif latest.all?(&:pending?)
-          'pending'
-        elsif latest.any?(&:running?) || latest.any?(&:pending?)
-          'running'
-        elsif latest.all?(&:canceled?)
-          'canceled'
-        else
-          'failed'
-        end
-      end
+      @status ||= Ci::Status.get_status(latest_statuses)
     end
 
     def pending?
@@ -219,16 +211,6 @@ module Ci
       update!(committed_at: DateTime.now)
     end
 
-    def should_create_next_builds?(build)
-      # don't create other builds if this one is retried
-      other_builds = builds.similar(build).latest
-      return false unless other_builds.include?(build)
-
-      other_builds.all? do |build|
-        build.success? || build.ignored?
-      end
-    end
-
     private
 
     def save_yaml_error(error)

app/models/commit_status.rb

@@ -28,7 +28,7 @@ class CommitStatus < ActiveRecord::Base
     end
 
     event :drop do
-      transition running: :failed
+      transition [:pending, :running] => :failed
     end
 
     event :success do

app/services/ci/create_builds_service.rb

 module Ci
   class CreateBuildsService
-    def execute(commit, stage, ref, tag, user, trigger_request)
+    def execute(commit, stage, ref, tag, user, trigger_request, status)
       builds_attrs = commit.config_processor.builds_for_stage_and_ref(stage, ref, tag)
 
+      # check when to create next build
+      builds_attrs = builds_attrs.select do |build_attrs|
+        case build_attrs[:when]
+        when 'on_success'
+          status == 'success'
+        when 'on_failure'
+          status == 'failed'
+        when 'always'
+          %w(success failed).include?(status)
+        end
+      end
+
       builds_attrs.map do |build_attrs|
         # don't create the same build twice
         unless commit.builds.find_by(ref: ref, tag: tag, trigger_request: trigger_request, name: build_attrs[:name])

app/views/projects/builds/_build.html.haml

@@ -9,6 +9,9 @@
     - else
       %strong Build ##{build.id}
 
+    - if build.show_warning?
+      %i.fa.fa-warning.text-warning
+
   %td
     = link_to build.short_sha, namespace_project_commit_path(@project.namespace, @project, build.sha)
 

app/views/projects/builds/index.html.haml

@@ -12,17 +12,17 @@
     %li{class: ('active' if @scope.nil?)}
       = link_to project_builds_path(@project) do
         Running
-        %span.badge.js-running-count= @all_builds.running_or_pending.size
+        %span.badge.js-running-count= @all_builds.running_or_pending.count(:id)
 
     %li{class: ('active' if @scope == 'finished')}
       = link_to project_builds_path(@project, scope: :finished) do
         Finished
-        %span.badge.js-running-count= @all_builds.finished.size
+        %span.badge.js-running-count= @all_builds.finished.count(:id)
 
     %li{class: ('active' if @scope == 'all')}
       = link_to project_builds_path(@project, scope: :all) do
         All
-        %span.badge.js-totalbuilds-count= @all_builds.size
+        %span.badge.js-totalbuilds-count= @all_builds.count(:id)
 
 .gray-content-block
   List of #{@scope || 'running'} builds from this project

app/views/projects/builds/show.html.haml

@@ -44,16 +44,14 @@
       .bs-callout.bs-callout-warning
         %p
           - if no_runners_for_project?(@build.project)
-            This build is stuck, because the project doesn't have runners assigned.
+            This build is stuck, because the project doesn't have any runners online assigned to it.
           - elsif @build.tags.any?
-            This build is stuck.
-            %br
-            This build is stuck, because you don't have any active runners online with these tags assigned to the project:
+            This build is stuck, because you don't have any active runners online with any of these tags assigned to them:
             - @build.tags.each do |tag|
               %span.label.label-primary
                 = tag
           - else
-            This build is stuck, because you don't have any active runners online that can run this build.
+            This build is stuck, because you don't have any active runners that can run this build.
 
           %br
           Go to

app/views/projects/merge_requests/_show.html.haml

@@ -36,6 +36,7 @@
     - if @merge_request.open? && @merge_request.can_be_merged?
       .light.append-bottom-20
         You can also accept this merge request manually using the
+        = succeed '.' do
           = link_to "command line", "#modal_merge_info", class: "how_to_merge_link vlink", title: "How To Merge", "data-toggle" => "modal"
 
   - if @commits.present?

doc/ci/yaml/README.md

@@ -140,6 +140,7 @@ job_name:
 | except        | optional | Defines a list of git refs for which build is not created |
 | tags          | optional | Defines a list of tags which are used to select runner |
 | allow_failure | optional | Allow build to fail. Failed build doesn't contribute to commit status |
+| when          | optional | Define when to run build. Can be `on_success`, `on_failure` or `always` |
 
 ### script
 `script` is a shell script which is executed by runner. The shell script is prepended with `before_script`.
@@ -196,6 +197,55 @@ job:
 
 The above specification will make sure that `job` is built by a runner that have `ruby` AND `postgres` tags defined.
 
+### when
+`when` is used to implement jobs that are run in case of failure or despite the failure.
+
+`when` can be set to one of the following values:
+
+1. `on_success` - execute build only when all builds from prior stages succeeded. This is the default.
+1. `on_failure` - execute build only when at least one build from prior stages failed.
+1. `always` - execute build despite the status of builds from prior stages.
+
+```
+stages:
+- build
+- cleanup_build
+- test
+- deploy
+- cleanup
+
+build:
+  stage: build
+  script:
+  - make build
+
+cleanup_build:
+  stage: cleanup_build
+  script:
+  - cleanup build when failed
+  when: on_failure
+
+test:
+  stage: test
+  script:
+  - make test
+
+deploy:
+  stage: deploy
+  script:
+  - make deploy
+
+cleanup:
+  stage: cleanup
+  script:
+  - cleanup after builds
+  when: always
+```
+
+The above script will:
+1. Execute `cleanup_build` only when the `build` failed,
+2. Always execute `cleanup` as the last step in pipeline.
+
 ## Validate the .gitlab-ci.yml
 Each instance of GitLab CI has an embedded debug tool called Lint.
 You can find the link to the Lint in the project's settings page or use short url `/lint`.

doc/install/installation.md

@@ -211,9 +211,9 @@ We recommend using a PostgreSQL database. For MySQL check [MySQL setup guide](da
 ### Clone the Source
 
     # Clone GitLab repository
-    sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 8-0-stable-ee gitlab
+    sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 8-1-stable-ee gitlab
 
-**Note:** You can change `8-0-stable-ee` to `master` if you want the *bleeding edge* version, but never install master on a production server!
+**Note:** You can change `8-1-stable-ee` to `master` if you want the *bleeding edge* version, but never install master on a production server!
 
 ### Configure It
 

doc/update/8.0-to-8.1.md

+# From 8.0 to 8.1
+
+**NOTE:** GitLab 8.0 introduced several significant changes related to
+installation and configuration which *are not duplicated here*. Be sure you're
+already running a working version of 8.0 before proceeding with this guide.
+
+### 0. Double-check your Git version
+
+**This notice applies only to /usr/local/bin/git**
+
+If you compiled Git from source on your GitLab server then please double-check
+that you are using a version that protects against CVE-2014-9390. For six
+months after this vulnerability became known the GitLab installation guide
+still contained instructions that would install the outdated, 'vulnerable' Git
+version 2.1.2.
+
+Run the following command to get your current Git version:
+
+```sh
+/usr/local/bin/git --version
+```
+
+If you see 'No such file or directory' then you did not install Git according
+to the outdated instructions from the GitLab installation guide and you can go
+to the next step 'Stop server' below.
+
+If you see a version string then it should be v1.8.5.6, v1.9.5, v2.0.5, v2.1.4,
+v2.2.1 or newer. You can use the [instructions in the GitLab source
+installation
+guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/installation.md#1-packages-dependencies)
+to install a newer version of Git.
+
+### 1. Stop server
+
+    sudo service gitlab stop
+
+### 2. Backup
+
+```bash
+cd /home/git/gitlab
+sudo -u git -H bundle exec rake gitlab:backup:create RAILS_ENV=production
+```
+
+### 3. Get latest code
+
+```bash
+sudo -u git -H git fetch --all
+sudo -u git -H git checkout -- db/schema.rb # local changes will be restored automatically
+```
+
+For GitLab Community Edition:
+
+```bash
+sudo -u git -H git checkout 8-1-stable
+```
+
+OR
+
+For GitLab Enterprise Edition:
+
+```bash
+sudo -u git -H git checkout 8-1-stable-ee
+```
+
+### 4. Update gitlab-shell
+
+```bash
+cd /home/git/gitlab-shell
+sudo -u git -H git fetch
+sudo -u git -H git checkout v2.6.5
+```
+
+### 5. Install libs, migrations, etc.
+
+```bash
+cd /home/git/gitlab
+
+# MySQL installations (note: the line below states '--without postgres')
+sudo -u git -H bundle install --without postgres development test --deployment
+
+# PostgreSQL installations (note: the line below states '--without mysql')
+sudo -u git -H bundle install --without mysql development test --deployment
+
+# Run database migrations
+sudo -u git -H bundle exec rake db:migrate RAILS_ENV=production
+
+# Clean up assets and cache
+sudo -u git -H bundle exec rake assets:clean assets:precompile cache:clear RAILS_ENV=production
+
+# Update init.d script
+sudo cp lib/support/init.d/gitlab /etc/init.d/gitlab
+```
+
+### 6. Update configuration files
+
+#### New configuration options for `gitlab.yml`
+
+There are new configuration options available for [`gitlab.yml`](config/gitlab.yml.example). View them with the command below and apply them manually to your current `gitlab.yml`:
+
+```sh
+git diff origin/8-0-stable:config/gitlab.yml.example origin/8-1-stable:config/gitlab.yml.example
+```
+
+### 7. Start application
+
+    sudo service gitlab start
+    sudo service nginx restart
+
+### 8. Check application status
+
+Check if GitLab and its environment are configured correctly:
+
+    sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production
+
+To make sure you didn't miss anything run a more thorough check:
+
+    sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production
+
+If all items are green, then congratulations, the upgrade is complete!
+
+## Things went south? Revert to previous version (8.0)
+
+### 1. Revert the code to the previous version
+
+Follow the [upgrade guide from 7.14 to 8.0](7.14-to-8.0.md), except for the database migration
+(The backup is already migrated to the previous version)
+
+### 2. Restore from the backup
+
+```bash
+cd /home/git/gitlab
+sudo -u git -H bundle exec rake gitlab:backup:restore RAILS_ENV=production
+```
+
+If you have more than one backup `*.tar` file(s) please add `BACKUP=timestamp_of_backup` to the command above.
+
+## Troubleshooting
+
+### "You appear to have cloned an empty repository."
+
+See the [7.14 to 8.0 update guide](7.14-to-8.0.md#troubleshooting).

lib/api/merge_requests.rb

@@ -249,8 +249,16 @@ module API
         required_attributes! [:note]
 
         merge_request = user_project.merge_requests.find(params[:merge_request_id])
-        note = merge_request.notes.new(note: params[:note], project_id: user_project.id)
-        note.author = current_user
+
+        authorize! :create_note, merge_request
+
+        opts = {
+          note: params[:note],
+          noteable_type: 'MergeRequest',
+          noteable_id: merge_request.id
+        }
+
+        note = ::Notes::CreateService.new(user_project, current_user, opts).execute
 
         if note.save
           present note, with: Entities::MRNote

lib/ci/gitlab_ci_yaml_processor.rb

@@ -5,7 +5,7 @@ module Ci
     DEFAULT_STAGES = %w(build test deploy)
     DEFAULT_STAGE = 'test'
     ALLOWED_YAML_KEYS = [:before_script, :image, :services, :types, :stages, :variables]
-    ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services, :allow_failure, :type, :stage]
+    ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services, :allow_failure, :type, :stage, :when]
 
     attr_reader :before_script, :image, :services, :variables
 
@@ -93,6 +93,7 @@ module Ci
         only: job[:only],
         except: job[:except],
         allow_failure: job[:allow_failure] || false,
+        when: job[:when] || 'on_success',
         options: {
           image: job[:image] || @image,
           services: job[:services] || @services
@@ -184,6 +185,10 @@ module Ci
       if job[:allow_failure] && !job[:allow_failure].in?([true, false])
         raise ValidationError, "#{name}: allow_failure parameter should be an boolean"
       end
+
+      if job[:when] && !job[:when].in?(%w(on_success on_failure always))
+        raise ValidationError, "#{name}: when parameter should be on_success, on_failure or always"
+      end
     end
 
     private

lib/ci/status.rb

+module Ci
+  class Status
+    def self.get_status(statuses)
+      statuses.reject! { |status| status.try(&:allow_failure?) }
+
+      if statuses.none?
+        'skipped'
+      elsif statuses.all?(&:success?)
+        'success'
+      elsif statuses.all?(&:pending?)
+        'pending'
+      elsif statuses.any?(&:running?) || statuses.any?(&:pending?)
+        'running'
+      elsif statuses.all?(&:canceled?)
+        'canceled'
+      else
+        'failed'
+      end
+    end
+  end
+end

spec/lib/ci/gitlab_ci_yaml_processor_spec.rb

@@ -24,7 +24,8 @@ module Ci
           commands: "pwd\nrspec",
           tag_list: [],
           options: {},
-          allow_failure: false
+          allow_failure: false,
+          when: "on_success"
         })
       end
 
@@ -125,7 +126,8 @@ module Ci
             image: "ruby:2.1",
             services: ["mysql"]
           },
-          allow_failure: false
+          allow_failure: false,
+          when: "on_success"
         })
       end
 
@@ -152,7 +154,8 @@ module Ci
             image: "ruby:2.5",
             services: ["postgresql"]
           },
-          allow_failure: false
+          allow_failure: false,
+          when: "on_success"
         })
       end
     end
@@ -174,6 +177,21 @@ module Ci
       end
     end
 
+    describe "When" do
+      %w(on_success on_failure always).each do |when_state|
+        it "returns #{when_state} when defined" do
+          config = YAML.dump({
+                               rspec: { script: "rspec", when: when_state }
+                             })
+
+          config_processor = GitlabCiYamlProcessor.new(config)
+          builds = config_processor.builds_for_stage_and_ref("test", "master")
+          expect(builds.size).to eq(1)
+          expect(builds.first[:when]).to eq(when_state)
+        end
+      end
+    end
+
     describe "Error handling" do
       it "indicates that object is invalid" do
         expect{GitlabCiYamlProcessor.new("invalid_yaml\n!ccdvlf%612334@@@@")}.to raise_error(GitlabCiYamlProcessor::ValidationError)
@@ -311,6 +329,13 @@ module Ci
           GitlabCiYamlProcessor.new(config)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-valued strings")
       end
+
+      it "returns errors if job when is not on_success, on_failure or always" do
+        config = YAML.dump({ rspec: { script: "test", when: 1 } })
+        expect do
+          GitlabCiYamlProcessor.new(config)
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: when parameter should be on_success, on_failure or always")
+      end
     end
   end
 end

spec/requests/api/merge_requests_spec.rb

@@ -379,9 +379,14 @@ describe API::API, api: true  do
 
   describe "POST /projects/:id/merge_request/:merge_request_id/comments" do
     it "should return comment" do
+      original_count = merge_request.notes.size
+
       post api("/projects/#{project.id}/merge_request/#{merge_request.id}/comments", user), note: "My comment"
       expect(response.status).to eq(201)
       expect(json_response['note']).to eq('My comment')
+      expect(json_response['author']['name']).to eq(user.name)
+      expect(json_response['author']['username']).to eq(user.username)
+      expect(merge_request.notes.size).to eq(original_count + 1)
     end
 
     it "should return 400 if note is missing" do