Add latest changes from gitlab-org/gitlab@master

app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue

@@ -115,7 +115,10 @@ export default {
               data-qa-selector="merge_request_pipeline_info_content"
             >
               {{ pipeline.details.name }}
-              <gl-link :href="pipeline.path" class="pipeline-id font-weight-normal pipeline-number"
+              <gl-link
+                :href="pipeline.path"
+                class="pipeline-id font-weight-normal pipeline-number"
+                data-qa-selector="pipeline_link"
                 >#{{ pipeline.id }}</gl-link
               >
               {{ pipeline.details.status.label }}

app/services/git/base_hooks_service.rb

@@ -15,8 +15,6 @@ module Git
       # Not a hook, but it needs access to the list of changed commits
       enqueue_invalidate_cache
 
-      update_remote_mirrors
-
       success
     end
 
@@ -121,13 +119,6 @@ module Git
       {}
     end
 
-    def update_remote_mirrors
-      return unless project.has_remote_mirror?
-
-      project.mark_stuck_remote_mirrors_as_failed!
-      project.update_remote_mirrors
-    end
-
     def log_pipeline_errors(exception)
       data = {
         class: self.class.name,

app/services/git/branch_push_service.rb

@@ -57,13 +57,6 @@ module Git
       Ci::StopEnvironmentsService.new(project, current_user).execute(branch_name)
     end
 
-    def update_remote_mirrors
-      return unless project.has_remote_mirror?
-
-      project.mark_stuck_remote_mirrors_as_failed!
-      project.update_remote_mirrors
-    end
-
     def execute_related_hooks
       BranchHooksService.new(project, current_user, params).execute
     end

app/services/system_note_service.rb

@@ -16,20 +16,9 @@ module SystemNoteService
   # existing_commits - Array of Commits added in a previous push
   # oldrev           - Optional String SHA of a previous Commit
   #
-  # See new_commit_summary and existing_commit_summary.
-  #
   # Returns the created Note object
   def add_commits(noteable, project, author, new_commits, existing_commits = [], oldrev = nil)
-    total_count  = new_commits.length + existing_commits.length
-    commits_text = "#{total_count} commit".pluralize(total_count)
-
-    text_parts = ["added #{commits_text}"]
-    text_parts << commits_list(noteable, new_commits, existing_commits, oldrev)
-    text_parts << "[Compare with previous version](#{diff_comparison_path(noteable, project, oldrev)})"
-
-    body = text_parts.join("\n\n")
-
-    create_note(NoteSummary.new(noteable, project, author, body, action: 'commit', commit_count: total_count))
+    ::SystemNotes::CommitService.new(noteable: noteable, project: project, author: author).add_commits(new_commits, existing_commits, oldrev)
   end
 
   # Called when a commit was tagged
@@ -41,10 +30,7 @@ module SystemNoteService
   #
   # Returns the created Note object
   def tag_commit(noteable, project, author, tag_name)
-    link = url_helpers.project_tag_path(project, id: tag_name)
-    body = "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})"
-
-    create_note(NoteSummary.new(noteable, project, author, body, action: 'tag'))
+    ::SystemNotes::CommitService.new(noteable: noteable, project: project, author: author).tag_commit(tag_name)
   end
 
   # Called when the assignee of a Noteable is changed or removed
@@ -497,17 +483,6 @@ module SystemNoteService
     notes_for_mentioner(mentioner, noteable, notes).exists?
   end
 
-  # Build an Array of lines detailing each commit added in a merge request
-  #
-  # new_commits - Array of new Commit objects
-  #
-  # Returns an Array of Strings
-  def new_commit_summary(new_commits)
-    new_commits.collect do |commit|
-      content_tag('li', "#{commit.short_id} - #{commit.title}")
-    end
-  end
-
   # Called when the status of a Task has changed
   #
   # noteable  - Noteable object.
@@ -637,71 +612,10 @@ module SystemNoteService
     "#{cross_reference_note_prefix}#{gfm_reference}"
   end
 
-  # Builds a list of existing and new commits according to existing_commits and
-  # new_commits methods.
-  # Returns a String wrapped in `ul` and `li` tags.
-  def commits_list(noteable, new_commits, existing_commits, oldrev)
-    existing_commit_summary = existing_commit_summary(noteable, existing_commits, oldrev)
-    new_commit_summary = new_commit_summary(new_commits).join
-
-    content_tag('ul', "#{existing_commit_summary}#{new_commit_summary}".html_safe)
-  end
-
-  # Build a single line summarizing existing commits being added in a merge
-  # request
-  #
-  # noteable         - MergeRequest object
-  # existing_commits - Array of existing Commit objects
-  # oldrev           - Optional String SHA of a previous Commit
-  #
-  # Examples:
-  #
-  #   "* ea0f8418...2f4426b7 - 24 commits from branch `master`"
-  #
-  #   "* ea0f8418..4188f0ea - 15 commits from branch `fork:master`"
-  #
-  #   "* ea0f8418 - 1 commit from branch `feature`"
-  #
-  # Returns a newline-terminated String
-  def existing_commit_summary(noteable, existing_commits, oldrev = nil)
-    return '' if existing_commits.empty?
-
-    count = existing_commits.size
-
-    commit_ids = if count == 1
-                   existing_commits.first.short_id
-                 else
-                   if oldrev && !Gitlab::Git.blank_ref?(oldrev)
-                     "#{Commit.truncate_sha(oldrev)}...#{existing_commits.last.short_id}"
-                   else
-                     "#{existing_commits.first.short_id}..#{existing_commits.last.short_id}"
-                   end
-                 end
-
-    commits_text = "#{count} commit".pluralize(count)
-
-    branch = noteable.target_branch
-    branch = "#{noteable.target_project_namespace}:#{branch}" if noteable.for_fork?
-
-    branch_name = content_tag('code', branch)
-    content_tag('li', "#{commit_ids} - #{commits_text} from branch #{branch_name}".html_safe)
-  end
-
   def url_helpers
     @url_helpers ||= Gitlab::Routing.url_helpers
   end
 
-  def diff_comparison_path(merge_request, project, oldrev)
-    diff_id = merge_request.merge_request_diff.id
-
-    url_helpers.diffs_project_merge_request_path(
-      project,
-      merge_request,
-      diff_id: diff_id,
-      start_sha: oldrev
-    )
-  end
-
   def content_tag(*args)
     ActionController::Base.helpers.content_tag(*args)
   end

app/services/system_notes/base_service.rb

+# frozen_string_literal: true
+
+module SystemNotes
+  class BaseService
+    attr_reader :noteable, :project, :author
+
+    def initialize(noteable: nil, author: nil, project: nil)
+      @noteable = noteable
+      @project = project
+      @author = author
+    end
+
+    protected
+
+    def create_note(note_summary)
+      note = Note.create(note_summary.note.merge(system: true))
+      note.system_note_metadata = SystemNoteMetadata.new(note_summary.metadata) if note_summary.metadata?
+
+      note
+    end
+
+    def content_tag(*args)
+      ActionController::Base.helpers.content_tag(*args)
+    end
+
+    def url_helpers
+      @url_helpers ||= Gitlab::Routing.url_helpers
+    end
+  end
+end

app/services/system_notes/commit_service.rb

+# frozen_string_literal: true
+
+module SystemNotes
+  class CommitService < ::SystemNotes::BaseService
+    # Called when commits are added to a Merge Request
+    #
+    # new_commits      - Array of Commits added since last push
+    # existing_commits - Array of Commits added in a previous push
+    # oldrev           - Optional String SHA of a previous Commit
+    #
+    # See new_commit_summary and existing_commit_summary.
+    #
+    # Returns the created Note object
+    def add_commits(new_commits, existing_commits = [], oldrev = nil)
+      total_count  = new_commits.length + existing_commits.length
+      commits_text = "#{total_count} commit".pluralize(total_count)
+
+      text_parts = ["added #{commits_text}"]
+      text_parts << commits_list(noteable, new_commits, existing_commits, oldrev)
+      text_parts << "[Compare with previous version](#{diff_comparison_path(noteable, project, oldrev)})"
+
+      body = text_parts.join("\n\n")
+
+      create_note(NoteSummary.new(noteable, project, author, body, action: 'commit', commit_count: total_count))
+    end
+
+    # Called when a commit was tagged
+    #
+    # tag_name  - The created tag name
+    #
+    # Returns the created Note object
+    def tag_commit(tag_name)
+      link = url_helpers.project_tag_path(project, id: tag_name)
+      body = "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})"
+
+      create_note(NoteSummary.new(noteable, project, author, body, action: 'tag'))
+    end
+
+    # Build an Array of lines detailing each commit added in a merge request
+    #
+    # new_commits - Array of new Commit objects
+    #
+    # Returns an Array of Strings
+    def new_commit_summary(new_commits)
+      new_commits.collect do |commit|
+        content_tag('li', "#{commit.short_id} - #{commit.title}")
+      end
+    end
+
+    private
+
+    # Builds a list of existing and new commits according to existing_commits and
+    # new_commits methods.
+    # Returns a String wrapped in `ul` and `li` tags.
+    def commits_list(noteable, new_commits, existing_commits, oldrev)
+      existing_commit_summary = existing_commit_summary(noteable, existing_commits, oldrev)
+      new_commit_summary = new_commit_summary(new_commits).join
+
+      content_tag('ul', "#{existing_commit_summary}#{new_commit_summary}".html_safe)
+    end
+
+    # Build a single line summarizing existing commits being added in a merge
+    # request
+    #
+    # existing_commits - Array of existing Commit objects
+    # oldrev           - Optional String SHA of a previous Commit
+    #
+    # Examples:
+    #
+    #   "* ea0f8418...2f4426b7 - 24 commits from branch `master`"
+    #
+    #   "* ea0f8418..4188f0ea - 15 commits from branch `fork:master`"
+    #
+    #   "* ea0f8418 - 1 commit from branch `feature`"
+    #
+    # Returns a newline-terminated String
+    def existing_commit_summary(noteable, existing_commits, oldrev = nil)
+      return '' if existing_commits.empty?
+
+      count = existing_commits.size
+
+      commit_ids = if count == 1
+                     existing_commits.first.short_id
+                   else
+                     if oldrev && !Gitlab::Git.blank_ref?(oldrev)
+                       "#{Commit.truncate_sha(oldrev)}...#{existing_commits.last.short_id}"
+                     else
+                       "#{existing_commits.first.short_id}..#{existing_commits.last.short_id}"
+                     end
+                   end
+
+      commits_text = "#{count} commit".pluralize(count)
+
+      branch = noteable.target_branch
+      branch = "#{noteable.target_project_namespace}:#{branch}" if noteable.for_fork?
+
+      branch_name = content_tag('code', branch)
+      content_tag('li', "#{commit_ids} - #{commits_text} from branch #{branch_name}".html_safe)
+    end
+
+    def diff_comparison_path(merge_request, project, oldrev)
+      diff_id = merge_request.merge_request_diff.id
+
+      url_helpers.diffs_project_merge_request_path(
+        project,
+        merge_request,
+        diff_id: diff_id,
+        start_sha: oldrev
+      )
+    end
+  end
+end

app/workers/post_receive.rb

@@ -70,6 +70,7 @@ class PostReceive
       refs << ref
     end
 
+    update_remote_mirrors(post_received)
     after_project_changes_hooks(post_received, user, refs.to_a, changes)
   end
 
@@ -93,6 +94,16 @@ class PostReceive
     )
   end
 
+  def update_remote_mirrors(post_received)
+    return unless post_received.includes_branches? || post_received.includes_tags?
+
+    project = post_received.project
+    return unless project.has_remote_mirror?
+
+    project.mark_stuck_remote_mirrors_as_failed!
+    project.update_remote_mirrors
+  end
+
   def after_project_changes_hooks(post_received, user, refs, changes)
     hook_data = Gitlab::DataBuilder::Repository.update(post_received.project, user, changes, refs)
     SystemHooksService.new.execute_hooks(hook_data, :repository_update_hooks)

changelogs/unreleased/bvl-only-one-pushmirror-per-push.yml

+---
+title: Only schedule updating push-mirrors once per push
+merge_request: 17902
+author:
+type: performance

changelogs/unreleased/sh-enable-google-api-retries.yml

+---
+title: Enable Google API retries for uploads
+merge_request: 18040
+author:
+type: fixed

config/initializers/google_api_client.rb

@@ -10,6 +10,12 @@
 #
 
 require 'google/apis/container_v1beta1'
+require 'google/apis/options'
+
+# As stated in https://github.com/googleapis/google-api-ruby-client#errors--retries,
+# enabling retries is strongly encouraged but disabled by default. Large uploads
+# that may hit timeouts will mainly benefit from this.
+Google::Apis::RequestOptions.default.retries = 3 if Gitlab::Utils.to_boolean(ENV.fetch('ENABLE_GOOGLE_API_RETRIES', true))
 
 Google::Apis::ContainerV1beta1::AddonsConfig::Representation.tap do |representation|
   representation.hash :cloud_run_config, as: 'cloudRunConfig'

doc/administration/high_availability/monitoring_node.md

@@ -34,6 +34,9 @@ Omnibus:
    prometheus['listen_address'] = '0.0.0.0:9090'
    prometheus['monitor_kubernetes'] = false
 
+   # Enable Login form
+   grafana['disable_login_form'] = false
+
    # Enable Grafana
    grafana['enable'] = true
    grafana['admin_password'] = 'toomanysecrets'
@@ -63,6 +66,7 @@ Omnibus:
    sidekiq['enable'] = false
    unicorn['enable'] = false
    node_exporter['enable'] = false
+   gitlab_exporter['enable'] = false
    ```
 
 1. Run `sudo gitlab-ctl reconfigure` to compile the configuration.

doc/user/permissions.md

@@ -48,7 +48,7 @@ The following table depicts the various user permission levels in a project.
 | View Insights charts **(ULTIMATE)**               | ✓       | ✓          | ✓           | ✓        | ✓      |
 | View approved/blacklisted licenses **(ULTIMATE)** | ✓       | ✓          | ✓           | ✓        | ✓      |
 | View License Compliance reports **(ULTIMATE)**    | ✓ (*1*) | ✓          | ✓           | ✓        | ✓      |
-| View Security reports **(ULTIMATE)**              | ✓ (*1*) | ✓          | ✓           | ✓        | ✓      |
+| View Security reports **(ULTIMATE)**              | ✓ (*3*) | ✓          | ✓           | ✓        | ✓      |
 | View Dependency list **(ULTIMATE)**               | ✓ (*1*) | ✓          | ✓           | ✓        | ✓      |
 | View licenses in Dependency list **(ULTIMATE)**   | ✓ (*1*) | ✓          | ✓           | ✓        | ✓      |
 | View [Design Management](project/issues/design_management.md) pages **(PREMIUM)** | ✓ (*1*) | ✓          | ✓           | ✓        | ✓      |

lib/gitlab/import_export/fast_hash_serializer.rb

@@ -26,6 +26,51 @@ module Gitlab
     class FastHashSerializer
       attr_reader :subject, :tree
 
+      # Usage of this class results in delayed
+      # serialization of relation. The serialization
+      # will be triggered when the `JSON.generate`
+      # is exected.
+      #
+      # This class uses memory-optimised, lazily
+      # initialised, fast to recycle relation
+      # serialization.
+      #
+      # The `JSON.generate` does use `#to_json`,
+      # that returns raw JSON content that is written
+      # directly to file.
+      class JSONBatchRelation
+        include Gitlab::Utils::StrongMemoize
+
+        def initialize(relation, options, preloads)
+          @relation = relation
+          @options = options
+          @preloads = preloads
+        end
+
+        def raw_json
+          strong_memoize(:raw_json) do
+            result = +''
+
+            batch = @relation
+            batch = batch.preload(@preloads) if @preloads
+            batch.each do |item|
+              result.concat(",") unless result.empty?
+              result.concat(item.to_json(@options))
+            end
+
+            result
+          end
+        end
+
+        def to_json(options = {})
+          raw_json
+        end
+
+        def as_json(*)
+          raise NotImplementedError
+        end
+      end
+
       BATCH_SIZE = 100
 
       def initialize(subject, tree, batch_size: BATCH_SIZE)
@@ -34,8 +79,11 @@ module Gitlab
         @tree = tree
       end
 
-      # Serializes the subject into a Hash for the given option tree
-      # (e.g. Project#as_json)
+      # With the usage of `JSONBatchRelation`, it returns partially
+      # serialized hash which is not easily accessible.
+      # It means you can only manipulate and replace top-level objects.
+      # All future mutations of the hash (such as `fix_project_tree`)
+      # should be aware of that.
       def execute
         simple_serialize.merge(serialize_includes)
       end
@@ -85,13 +133,20 @@ module Gitlab
           return record.as_json(options)
         end
 
-        # has-many relation
         data = []
 
-        record.in_batches(of: @batch_size) do |batch| # rubocop:disable Cop/InBatches
-          batch = batch.preload(preloads[key]) if preloads&.key?(key)
-          data += batch.as_json(options)
+        # rubocop:disable Cop/InBatches
+        # If we put `rubocop:disable` inline after `do |batch|`,
+        # `Cop/LineBreakAroundConditionalBlock` will fail
+        record.in_batches(of: @batch_size) do |batch|
+          if Feature.enabled?(:export_fast_serialize_with_raw_json, default_enabled: true)
+            data.append(JSONBatchRelation.new(batch, options, preloads[key]).tap(&:raw_json))
+          else
+            batch = batch.preload(preloads[key]) if preloads&.key?(key)
+            data += batch.as_json(options)
+          end
         end
+        # rubocop:enable Cop/InBatches
 
         data
       end

lib/gitlab/import_export/project_tree_saver.rb

@@ -20,7 +20,8 @@ module Gitlab
 
         project_tree = serialize_project_tree
         fix_project_tree(project_tree)
-        File.write(full_path, project_tree.to_json)
+        project_tree_json = JSON.generate(project_tree)
+        File.write(full_path, project_tree_json)
 
         true
       rescue => e
@@ -30,6 +31,8 @@ module Gitlab
 
       private
 
+      # Aware that the resulting hash needs to be pure-hash and
+      # does not include any AR objects anymore, only objects that run `.to_json`
       def fix_project_tree(project_tree)
         if @params[:description].present?
           project_tree['description'] = @params[:description]

qa/qa.rb

@@ -357,6 +357,7 @@ module QA
     # Classes describing components that are used by several pages.
     #
     module Component
+      autoload :CiBadgeLink, 'qa/page/component/ci_badge_link'
       autoload :ClonePanel, 'qa/page/component/clone_panel'
       autoload :LazyLoader, 'qa/page/component/lazy_loader'
       autoload :LegacyClonePanel, 'qa/page/component/legacy_clone_panel'

qa/qa/page/base.rb

@@ -95,7 +95,7 @@ module QA
 
       # replace with (..., page = self.class)
       def click_element(name, page = nil, text: nil)
-        find_element(name, text: nil).click
+        find_element(name, text: text).click
         page.validate_elements_present! if page
       end
 

qa/qa/page/component/ci_badge_link.rb

+# frozen_string_literal: true
+
+module QA
+  module Page
+    module Component
+      module CiBadgeLink
+        COMPLETED_STATUSES = %w[passed failed canceled blocked skipped manual].freeze # excludes created, pending, running
+        PASSED_STATUS = 'passed'.freeze
+
+        def self.included(base)
+          base.view 'app/assets/javascripts/vue_shared/components/ci_badge_link.vue' do
+            element :status_badge
+          end
+        end
+
+        def status_badge
+          find_element(:status_badge).text
+        end
+
+        def successful?(timeout: 60)
+          raise "Timed out waiting for the status to be a valid completed state" unless completed?(timeout: timeout)
+
+          status_badge == PASSED_STATUS
+        end
+
+        private
+
+        def completed?(timeout: 60)
+          wait(reload: false, max: timeout) do
+            COMPLETED_STATUSES.include?(status_badge)
+          end
+        end
+      end
+    end
+  end
+end

qa/qa/page/merge_request/show.rb

@@ -14,6 +14,7 @@ module QA
 
         view 'app/assets/javascripts/vue_merge_request_widget/components/mr_widget_pipeline.vue' do
           element :merge_request_pipeline_info_content
+          element :pipeline_link
         end
 
         view 'app/assets/javascripts/vue_merge_request_widget/components/states/ready_to_merge.vue' do
@@ -59,6 +60,18 @@ module QA
           element :edit_button
         end
 
+        def click_discussions_tab
+          click_element :notes_tab
+        end
+
+        def click_diffs_tab
+          click_element :diffs_tab
+        end
+
+        def click_pipeline_link
+          click_element :pipeline_link
+        end
+
         def fast_forward_possible?
           has_no_text?('Fast-forward merge is not possible')
         end
@@ -156,14 +169,6 @@ module QA
           click_element :squash_checkbox
         end
 
-        def click_discussions_tab
-          click_element :notes_tab
-        end
-
-        def click_diffs_tab
-          click_element :diffs_tab
-        end
-
         def add_comment_to_diff(text)
           wait(interval: 5) do
             has_text?("No newline at end of file")

qa/qa/page/project/job/show.rb

@@ -3,17 +3,12 @@
 module QA::Page
   module Project::Job
     class Show < QA::Page::Base
-      COMPLETED_STATUSES = %w[passed failed canceled blocked skipped manual].freeze # excludes created, pending, running
-      PASSED_STATUS = 'passed'.freeze
+      include Component::CiBadgeLink
 
       view 'app/assets/javascripts/jobs/components/job_log.vue' do
         element :build_trace
       end
 
-      view 'app/assets/javascripts/vue_shared/components/ci_badge_link.vue' do
-        element :status_badge
-      end
-
       view 'app/assets/javascripts/jobs/components/stages_dropdown.vue' do
         element :pipeline_path
       end
@@ -45,16 +40,6 @@ module QA::Page
           has_element?(:build_trace, wait: 1)
         end
       end
-
-      def completed?(timeout: 60)
-        wait(reload: false, max: timeout) do
-          COMPLETED_STATUSES.include?(status_badge)
-        end
-      end
-
-      def status_badge
-        find_element(:status_badge).text
-      end
     end
   end
 end

qa/qa/page/project/pipeline/show.rb

@@ -3,6 +3,8 @@
 module QA::Page
   module Project::Pipeline
     class Show < QA::Page::Base
+      include Component::CiBadgeLink
+
       view 'app/assets/javascripts/vue_shared/components/header_ci_component.vue' do
         element :pipeline_header, /header class.*ci-header-container.*/ # rubocop:disable QA/ElementWithPattern
       end
@@ -38,6 +40,14 @@ module QA::Page
         end
       end
 
+      def has_job?(job_name)
+        has_element?(:job_link, text: job_name)
+      end
+
+      def has_no_job?(job_name)
+        has_no_element?(:job_link, text: job_name)
+      end
+
       def has_tag?(tag_name)
         within_element(:pipeline_badges) do
           has_selector?('.badge', text: tag_name)
@@ -45,7 +55,11 @@ module QA::Page
       end
 
       def click_job(job_name)
-        find_element(:job_link, text: job_name).click
+        click_element(:job_link, text: job_name)
+      end
+
+      def click_linked_job(project_name)
+        click_element(:linked_pipeline_button, text: /#{project_name}/)
       end
 
       def click_on_first_job

qa/qa/resource/project.rb

@@ -88,6 +88,10 @@ module QA
         "#{api_get_path}/members"
       end
 
+      def api_runners_path
+        "#{api_get_path}/runners"
+      end
+
       def api_post_path
         '/projects'
       end
@@ -108,6 +112,11 @@ module QA
         post_body
       end
 
+      def runners
+        response = get Runtime::API::Request.new(api_client, api_runners_path).url
+        parse_body(response)
+      end
+
       def share_with_group(invitee, access_level = Resource::Members::AccessLevel::DEVELOPER)
         post Runtime::API::Request.new(api_client, "/projects/#{id}/share").url, { group_id: invitee.id, group_access: access_level }
       end

qa/qa/resource/runner.rb

@@ -6,8 +6,9 @@ module QA
   module Resource
     class Runner < Base
       attr_writer :name, :tags, :image
-      attr_accessor :config
+      attr_accessor :config, :token
 
+      attribute :id
       attribute :project do
         Project.fabricate_via_api! do |resource|
           resource.name = 'project-with-ci-cd'
@@ -30,7 +31,7 @@ module QA
       def fabricate_via_api!
         Service::Runner.new(name).tap do |runner|
           runner.pull
-          runner.token = project.runners_token
+          runner.token = @token ||= project.runners_token
           runner.address = Runtime::Scenario.gitlab_address
           runner.tags = tags
           runner.image = image
@@ -40,6 +41,18 @@ module QA
         end
       end
 
+      def remove_via_api!
+        @id = project.runners.find { |runner| runner[:description] == name }[:id]
+
+        super
+
+        Service::Runner.new(name).remove!
+      end
+
+      def api_delete_path
+        "/runners/#{id}"
+      end
+
       def api_get_path
       end
 

qa/qa/resource/sandbox.rb

@@ -10,6 +10,7 @@ module QA
       attr_accessor :path
 
       attribute :id
+      attribute :runners_token
 
       def initialize
         @path = Runtime::Namespace.sandbox_name

spec/lib/gitlab/import_export/fast_hash_serializer_spec.rb

 require 'spec_helper'
 
 describe Gitlab::ImportExport::FastHashSerializer do
-  subject { described_class.new(project, tree).execute }
+  # FastHashSerializer#execute generates the hash which is not easily accessible
+  # and includes `JSONBatchRelation` items which are serialized at this point.
+  # Wrapping the result into JSON generating/parsing is for making
+  # the testing more convenient. Doing this, we can check that
+  # all items are properly serialized while traversing the simple hash.
+  subject { JSON.parse(JSON.generate(described_class.new(project, tree).execute)) }
 
   let!(:project) { setup_project }
   let(:user) { create(:user) }

spec/services/git/base_hooks_service_spec.rb

@@ -85,78 +85,4 @@ describe Git::BaseHooksService do
       end
     end
   end
-
-  describe 'with remote mirrors' do
-    class TestService < described_class
-      def commits
-        []
-      end
-    end
-
-    let(:project) { create(:project, :repository, :remote_mirror) }
-
-    subject { TestService.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) }
-
-    before do
-      expect(subject).to receive(:execute_project_hooks)
-    end
-
-    context 'when remote mirror feature is enabled' do
-      it 'fails stuck remote mirrors' do
-        allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors)
-        expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!)
-
-        subject.execute
-      end
-
-      it 'updates remote mirrors' do
-        expect(project).to receive(:update_remote_mirrors)
-
-        subject.execute
-      end
-    end
-
-    context 'when remote mirror feature is disabled' do
-      before do
-        stub_application_setting(mirror_available: false)
-      end
-
-      context 'with remote mirrors global setting overridden' do
-        before do
-          project.remote_mirror_available_overridden = true
-        end
-
-        it 'fails stuck remote mirrors' do
-          allow(project).to receive(:update_remote_mirrors).and_return(project.remote_mirrors)
-          expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!)
-
-          subject.execute
-        end
-
-        it 'updates remote mirrors' do
-          expect(project).to receive(:update_remote_mirrors)
-
-          subject.execute
-        end
-      end
-
-      context 'without remote mirrors global setting overridden' do
-        before do
-          project.remote_mirror_available_overridden = false
-        end
-
-        it 'does not fails stuck remote mirrors' do
-          expect(project).not_to receive(:mark_stuck_remote_mirrors_as_failed!)
-
-          subject.execute
-        end
-
-        it 'does not updates remote mirrors' do
-          expect(project).not_to receive(:update_remote_mirrors)
-
-          subject.execute
-        end
-      end
-    end
-  end
 end

spec/services/git/branch_hooks_service_spec.rb

@@ -19,12 +19,6 @@ describe Git::BranchHooksService do
     described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
   end
 
-  it 'update remote mirrors' do
-    expect(service).to receive(:update_remote_mirrors).and_call_original
-
-    service.execute
-  end
-
   describe "Git Push Data" do
     subject(:push_data) { service.send(:push_data) }
 

spec/services/git/tag_hooks_service_spec.rb

@@ -18,12 +18,6 @@ describe Git::TagHooksService, :service do
     described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref)
   end
 
-  it 'update remote mirrors' do
-    expect(service).to receive(:update_remote_mirrors).and_call_original
-
-    service.execute
-  end
-
   describe 'System hooks' do
     it 'Executes system hooks' do
       push_data = service.send(:push_data)

spec/services/system_note_service_spec.rb

@@ -14,127 +14,29 @@ describe SystemNoteService do
   let(:noteable) { create(:issue, project: project) }
   let(:issue)    { noteable }
 
-  shared_examples_for 'a note with overridable created_at' do
-    let(:noteable) { create(:issue, project: project, system_note_timestamp: Time.at(42)) }
-
-    it 'the note has the correct time' do
-      expect(subject.created_at).to eq Time.at(42)
-    end
-  end
-
-  shared_examples_for 'a system note' do
-    let(:expected_noteable) { noteable }
-    let(:commit_count)      { nil }
-
-    it 'has the correct attributes', :aggregate_failures do
-      expect(subject).to be_valid
-      expect(subject).to be_system
-
-      expect(subject.noteable).to eq expected_noteable
-      expect(subject.project).to eq project
-      expect(subject.author).to eq author
-
-      expect(subject.system_note_metadata.action).to eq(action)
-      expect(subject.system_note_metadata.commit_count).to eq(commit_count)
-    end
-  end
-
   describe '.add_commits' do
-    subject { described_class.add_commits(noteable, project, author, new_commits, old_commits, oldrev) }
-
-    let(:noteable)    { create(:merge_request, source_project: project, target_project: project) }
-    let(:new_commits) { noteable.commits }
-    let(:old_commits) { [] }
-    let(:oldrev)      { nil }
-
-    it_behaves_like 'a system note' do
-      let(:commit_count) { new_commits.size }
-      let(:action)       { 'commit' }
-    end
+    let(:new_commits) { double }
+    let(:old_commits) { double }
+    let(:oldrev)      { double }
 
-    describe 'note body' do
-      let(:note_lines) { subject.note.split("\n").reject(&:blank?) }
-
-      describe 'comparison diff link line' do
-        it 'adds the comparison text' do
-          expect(note_lines[2]).to match "[Compare with previous version]"
-        end
+    it 'calls CommitService' do
+      expect_next_instance_of(::SystemNotes::CommitService) do |service|
+        expect(service).to receive(:add_commits).with(new_commits, old_commits, oldrev)
       end
 
-      context 'without existing commits' do
-        it 'adds a message header' do
-          expect(note_lines[0]).to eq "added #{new_commits.size} commits"
-        end
-
-        it 'adds a message for each commit' do
-          decoded_note_content = HTMLEntities.new.decode(subject.note)
-
-          new_commits.each do |commit|
-            expect(decoded_note_content).to include("<li>#{commit.short_id} - #{commit.title}</li>")
-          end
-        end
-      end
-
-      describe 'summary line for existing commits' do
-        let(:summary_line) { note_lines[1] }
-
-        context 'with one existing commit' do
-          let(:old_commits) { [noteable.commits.last] }
-
-          it 'includes the existing commit' do
-            expect(summary_line).to start_with("<ul><li>#{old_commits.first.short_id} - 1 commit from branch <code>feature</code>")
-          end
-        end
-
-        context 'with multiple existing commits' do
-          let(:old_commits) { noteable.commits[3..-1] }
-
-          context 'with oldrev' do
-            let(:oldrev) { noteable.commits[2].id }
-
-            it 'includes a commit range and count' do
-              expect(summary_line)
-                .to start_with("<ul><li>#{Commit.truncate_sha(oldrev)}...#{old_commits.last.short_id} - 26 commits from branch <code>feature</code>")
-            end
-          end
-
-          context 'without oldrev' do
-            it 'includes a commit range and count' do
-              expect(summary_line)
-                .to start_with("<ul><li>#{old_commits[0].short_id}..#{old_commits[-1].short_id} - 26 commits from branch <code>feature</code>")
-            end
-          end
-
-          context 'on a fork' do
-            before do
-              expect(noteable).to receive(:for_fork?).and_return(true)
-            end
-
-            it 'includes the project namespace' do
-              expect(summary_line).to include("<code>#{noteable.target_project_namespace}:feature</code>")
-            end
-          end
-        end
-      end
+      described_class.add_commits(noteable, project, author, new_commits, old_commits, oldrev)
     end
   end
 
   describe '.tag_commit' do
-    let(:noteable) do
-      project.commit
-    end
-    let(:tag_name) { 'v1.2.3' }
-
-    subject { described_class.tag_commit(noteable, project, author, tag_name) }
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'tag' }
-    end
+    let(:tag_name) { double }
 
-    it 'sets the note text' do
-      link = "/#{project.full_path}/-/tags/#{tag_name}"
+    it 'calls CommitService' do
+      expect_next_instance_of(::SystemNotes::CommitService) do |service|
+        expect(service).to receive(:tag_commit).with(tag_name)
+      end
 
-      expect(subject.note).to eq "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})"
+      described_class.tag_commit(noteable, project, author, tag_name)
     end
   end
 
@@ -804,15 +706,6 @@ describe SystemNoteService do
     end
   end
 
-  describe '.new_commit_summary' do
-    it 'escapes HTML titles' do
-      commit = double(title: '<pre>This is a test</pre>', short_id: '12345678')
-      escaped = '&lt;pre&gt;This is a test&lt;/pre&gt;'
-
-      expect(described_class.new_commit_summary([commit])).to all(match(/- #{escaped}/))
-    end
-  end
-
   describe 'Jira integration' do
     include JiraServiceHelper
 

spec/services/system_notes/base_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe SystemNotes::BaseService do
+  let(:noteable) { double }
+  let(:project) { double }
+  let(:author) { double }
+
+  let(:base_service) { described_class.new(noteable: noteable, project: project, author: author) }
+
+  describe '#noteable' do
+    subject { base_service.noteable }
+
+    it { is_expected.to eq(noteable) }
+
+    it 'returns nil if no arguments are given' do
+      instance = described_class.new
+      expect(instance.noteable).to be_nil
+    end
+  end
+
+  describe '#project' do
+    subject { base_service.project }
+
+    it { is_expected.to eq(project) }
+
+    it 'returns nil if no arguments are given' do
+      instance = described_class.new
+      expect(instance.project).to be_nil
+    end
+  end
+
+  describe '#author' do
+    subject { base_service.author }
+
+    it { is_expected.to eq(author) }
+
+    it 'returns nil if no arguments are given' do
+      instance = described_class.new
+      expect(instance.author).to be_nil
+    end
+  end
+end

spec/services/system_notes/commit_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe SystemNotes::CommitService do
+  set(:group)    { create(:group) }
+  set(:project)  { create(:project, :repository, group: group) }
+  set(:author)   { create(:user) }
+
+  let(:commit_service) { described_class.new(noteable: noteable, project: project, author: author) }
+
+  describe '#add_commits' do
+    subject { commit_service.add_commits(new_commits, old_commits, oldrev) }
+
+    let(:noteable)    { create(:merge_request, source_project: project, target_project: project) }
+    let(:new_commits) { noteable.commits }
+    let(:old_commits) { [] }
+    let(:oldrev)      { nil }
+
+    it_behaves_like 'a system note' do
+      let(:commit_count) { new_commits.size }
+      let(:action)       { 'commit' }
+    end
+
+    describe 'note body' do
+      let(:note_lines) { subject.note.split("\n").reject(&:blank?) }
+
+      describe 'comparison diff link line' do
+        it 'adds the comparison text' do
+          expect(note_lines[2]).to match "[Compare with previous version]"
+        end
+      end
+
+      context 'without existing commits' do
+        it 'adds a message header' do
+          expect(note_lines[0]).to eq "added #{new_commits.size} commits"
+        end
+
+        it 'adds a message for each commit' do
+          decoded_note_content = HTMLEntities.new.decode(subject.note)
+
+          new_commits.each do |commit|
+            expect(decoded_note_content).to include("<li>#{commit.short_id} - #{commit.title}</li>")
+          end
+        end
+      end
+
+      describe 'summary line for existing commits' do
+        let(:summary_line) { note_lines[1] }
+
+        context 'with one existing commit' do
+          let(:old_commits) { [noteable.commits.last] }
+
+          it 'includes the existing commit' do
+            expect(summary_line).to start_with("<ul><li>#{old_commits.first.short_id} - 1 commit from branch <code>feature</code>")
+          end
+        end
+
+        context 'with multiple existing commits' do
+          let(:old_commits) { noteable.commits[3..-1] }
+
+          context 'with oldrev' do
+            let(:oldrev) { noteable.commits[2].id }
+
+            it 'includes a commit range and count' do
+              expect(summary_line)
+                .to start_with("<ul><li>#{Commit.truncate_sha(oldrev)}...#{old_commits.last.short_id} - 26 commits from branch <code>feature</code>")
+            end
+          end
+
+          context 'without oldrev' do
+            it 'includes a commit range and count' do
+              expect(summary_line)
+                .to start_with("<ul><li>#{old_commits[0].short_id}..#{old_commits[-1].short_id} - 26 commits from branch <code>feature</code>")
+            end
+          end
+
+          context 'on a fork' do
+            before do
+              expect(noteable).to receive(:for_fork?).and_return(true)
+            end
+
+            it 'includes the project namespace' do
+              expect(summary_line).to include("<code>#{noteable.target_project_namespace}:feature</code>")
+            end
+          end
+        end
+      end
+    end
+  end
+
+  describe '#tag_commit' do
+    let(:noteable) { project.commit }
+    let(:tag_name) { 'v1.2.3' }
+
+    subject { commit_service.tag_commit(tag_name) }
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'tag' }
+    end
+
+    it 'sets the note text' do
+      link = "/#{project.full_path}/-/tags/#{tag_name}"
+
+      expect(subject.note).to eq "tagged commit #{noteable.sha} to [`#{tag_name}`](#{link})"
+    end
+  end
+
+  describe '#new_commit_summary' do
+    it 'escapes HTML titles' do
+      commit = double(title: '<pre>This is a test</pre>', short_id: '12345678')
+      escaped = '&lt;pre&gt;This is a test&lt;/pre&gt;'
+
+      expect(described_class.new.new_commit_summary([commit])).to all(match(/- #{escaped}/))
+    end
+  end
+end

spec/support/shared_examples/common_system_notes_examples.rb

@@ -27,3 +27,28 @@ shared_examples 'WIP notes creation' do |wip_action|
     expect(Note.second.note).to match('changed title')
   end
 end
+
+shared_examples_for 'a note with overridable created_at' do
+  let(:noteable) { create(:issue, project: project, system_note_timestamp: Time.at(42)) }
+
+  it 'the note has the correct time' do
+    expect(subject.created_at).to eq Time.at(42)
+  end
+end
+
+shared_examples_for 'a system note' do
+  let(:expected_noteable) { noteable }
+  let(:commit_count)      { nil }
+
+  it 'has the correct attributes', :aggregate_failures do
+    expect(subject).to be_valid
+    expect(subject).to be_system
+
+    expect(subject.noteable).to eq expected_noteable
+    expect(subject.project).to eq project
+    expect(subject.author).to eq author
+
+    expect(subject.system_note_metadata.action).to eq(action)
+    expect(subject.system_note_metadata.commit_count).to eq(commit_count)
+  end
+end

spec/workers/post_receive_spec.rb

@@ -61,6 +61,16 @@ describe PostReceive do
       end
     end
 
+    shared_examples 'not updating remote mirrors' do
+      it 'does not schedule an update' do
+        expect(project).not_to receive(:has_remote_mirror?)
+        expect(project).not_to receive(:mark_stuck_remote_mirrors_as_failed!)
+        expect(project).not_to receive(:update_remote_mirrors)
+
+        perform
+      end
+    end
+
     context 'empty changes' do
       it "does not call any PushService but runs after project hooks" do
         expect(Git::BranchPushService).not_to receive(:new)
@@ -69,6 +79,8 @@ describe PostReceive do
 
         perform(changes: "")
       end
+
+      it_behaves_like 'not updating remote mirrors'
     end
 
     context 'unidentified user' do
@@ -88,6 +100,16 @@ describe PostReceive do
         allow(Gitlab::GlRepository).to receive(:parse).and_return([project, Gitlab::GlRepository::PROJECT])
       end
 
+      shared_examples 'updating remote mirrors' do
+        it 'schedules an update if the project had mirrors' do
+          expect(project).to receive(:has_remote_mirror?).and_return(true)
+          expect(project).to receive(:mark_stuck_remote_mirrors_as_failed!)
+          expect(project).to receive(:update_remote_mirrors)
+
+          perform
+        end
+      end
+
       context "branches" do
         let(:changes) do
           <<~EOF
@@ -126,6 +148,8 @@ describe PostReceive do
           perform
         end
 
+        it_behaves_like 'updating remote mirrors'
+
         context 'with a default branch' do
           let(:changes) do
             <<~EOF
@@ -191,6 +215,8 @@ describe PostReceive do
 
           perform
         end
+
+        it_behaves_like 'updating remote mirrors'
       end
 
       context "merge-requests" do
@@ -202,6 +228,8 @@ describe PostReceive do
 
           perform
         end
+
+        it_behaves_like 'not updating remote mirrors'
       end
 
       context "gitlab-ci.yml" do