Merge branch 'backstage/gb/use-persisted-stages-to-improve-pipelines-table' into 'master'

Improve pipeline index action performance by using persisted stages

Closes #43132

See merge request gitlab-org/gitlab-ce!19063

app/controllers/projects/pipelines_controller.rb

@@ -23,8 +23,6 @@ class Projects::PipelinesController < Projects::ApplicationController
     @finished_count = limited_pipelines_count(project, 'finished')
     @pipelines_count = limited_pipelines_count(project)
 
-    Gitlab::Ci::Pipeline::Preloader.preload(@pipelines)
-
     respond_to do |format|
       format.html
       format.json do
@@ -34,7 +32,7 @@ class Projects::PipelinesController < Projects::ApplicationController
           pipelines: PipelineSerializer
             .new(project: @project, current_user: @current_user)
             .with_pagination(request, response)
-            .represent(@pipelines, disable_coverage: true),
+            .represent(@pipelines, disable_coverage: true, preload: true),
           count: {
             all: @pipelines_count,
             running: @running_count,

app/models/ci/group.rb

@@ -31,6 +31,14 @@ module Ci
       end
     end
 
+    def self.fabricate(stage)
+      stage.statuses.ordered.latest
+        .sort_by(&:sortable_name).group_by(&:group_name)
+        .map do |group_name, grouped_statuses|
+          self.new(stage, name: group_name, jobs: grouped_statuses)
+        end
+    end
+
     private
 
     def commit_statuses

app/models/ci/legacy_stage.rb

@@ -16,11 +16,7 @@ module Ci
     end
 
     def groups
-      @groups ||= statuses.ordered.latest
-        .sort_by(&:sortable_name).group_by(&:group_name)
-        .map do |group_name, grouped_statuses|
-          Ci::Group.new(self, name: group_name, jobs: grouped_statuses)
-        end
+      @groups ||= Ci::Group.fabricate(self)
     end
 
     def to_param

app/models/ci/pipeline.rb

@@ -18,7 +18,7 @@ module Ci
       s&.project&.pipelines&.maximum(:iid) || s&.project&.pipelines&.count
     end
 
-    has_many :stages
+    has_many :stages, -> { order(position: :asc) }, inverse_of: :pipeline
     has_many :statuses, class_name: 'CommitStatus', foreign_key: :commit_id, inverse_of: :pipeline
     has_many :builds, foreign_key: :commit_id, inverse_of: :pipeline
     has_many :trigger_requests, dependent: :destroy, foreign_key: :commit_id # rubocop:disable Cop/ActiveRecordDependent
@@ -254,6 +254,20 @@ module Ci
       stage unless stage.statuses_count.zero?
     end
 
+    ##
+    # TODO We do not completely switch to persisted stages because of
+    # race conditions with setting statuses gitlab-ce#23257.
+    #
+    def ordered_stages
+      return legacy_stages unless complete?
+
+      if Feature.enabled?('ci_pipeline_persisted_stages')
+        stages
+      else
+        legacy_stages
+      end
+    end
+
     def legacy_stages
       # TODO, this needs refactoring, see gitlab-ce#26481.
 
@@ -416,7 +430,7 @@ module Ci
 
     def number_of_warnings
       BatchLoader.for(id).batch(default_value: 0) do |pipeline_ids, loader|
-        Build.where(commit_id: pipeline_ids)
+        ::Ci::Build.where(commit_id: pipeline_ids)
           .latest
           .failed_but_allowed
           .group(:commit_id)
@@ -508,7 +522,8 @@ module Ci
 
     def update_status
       retry_optimistic_lock(self) do
-        case latest_builds_status
+        case latest_builds_status.to_s
+        when 'created' then nil
         when 'pending' then enqueue
         when 'running' then run
         when 'success' then succeed
@@ -516,6 +531,9 @@ module Ci
         when 'canceled' then cancel
         when 'skipped' then skip
         when 'manual' then block
+        else
+          raise HasStatus::UnknownStatusError,
+                "Unknown status `#{latest_builds_status}`"
         end
       end
     end

app/models/ci/stage.rb

@@ -68,16 +68,44 @@ module Ci
     def update_status
       retry_optimistic_lock(self) do
         case statuses.latest.status
+        when 'created' then nil
         when 'pending' then enqueue
         when 'running' then run
         when 'success' then succeed
         when 'failed' then drop
         when 'canceled' then cancel
         when 'manual' then block
-        when 'skipped' then skip
-        else skip
+        when 'skipped', nil then skip
+        else
+          raise HasStatus::UnknownStatusError,
+                "Unknown status `#{statuses.latest.status}`"
         end
       end
     end
+
+    def groups
+      @groups ||= Ci::Group.fabricate(self)
+    end
+
+    def has_warnings?
+      number_of_warnings.positive?
+    end
+
+    def number_of_warnings
+      BatchLoader.for(id).batch(default_value: 0) do |stage_ids, loader|
+        ::Ci::Build.where(stage_id: stage_ids)
+          .latest
+          .failed_but_allowed
+          .group(:stage_id)
+          .count
+          .each { |id, amount| loader.call(id, amount) }
+      end
+    end
+
+    def detailed_status(current_user)
+      Gitlab::Ci::Status::Stage::Factory
+        .new(self, current_user)
+        .fabricate!
+    end
   end
 end

app/models/concerns/has_status.rb

@@ -11,6 +11,8 @@ module HasStatus
   STATUSES_ENUM = { created: 0, pending: 1, running: 2, success: 3,
                     failed: 4, canceled: 5, skipped: 6, manual: 7 }.freeze
 
+  UnknownStatusError = Class.new(StandardError)
+
   class_methods do
     def status_sql
       scope_relevant = respond_to?(:exclude_ignored) ? exclude_ignored : all

app/models/project.rb

@@ -228,6 +228,7 @@ class Project < ActiveRecord::Base
 
   has_many :commit_statuses
   has_many :pipelines, class_name: 'Ci::Pipeline', inverse_of: :project
+  has_many :stages, class_name: 'Ci::Stage', inverse_of: :project
 
   # Ci::Build objects store data on the file system such as artifact files and
   # build traces. Currently there's no efficient way of removing this data in
@@ -1425,8 +1426,14 @@ class Project < ActiveRecord::Base
     Ci::Runner.from("(#{union.to_sql}) ci_runners")
   end
 
+  def active_runners
+    strong_memoize(:active_runners) do
+      all_runners.active
+    end
+  end
+
   def any_runners?(&block)
-    all_runners.active.any?(&block)
+    active_runners.any?(&block)
   end
 
   def valid_runners_token?(token)

app/serializers/pipeline_details_entity.rb

 class PipelineDetailsEntity < PipelineEntity
   expose :details do
-    expose :legacy_stages, as: :stages, using: StageEntity
+    expose :ordered_stages, as: :stages, using: StageEntity
     expose :artifacts, using: BuildArtifactEntity
     expose :manual_actions, using: BuildActionEntity
   end

app/serializers/pipeline_serializer.rb

 class PipelineSerializer < BaseSerializer
   include WithPagination
-
-  InvalidResourceError = Class.new(StandardError)
-
   entity PipelineDetailsEntity
 
   def represent(resource, opts = {})
     if resource.is_a?(ActiveRecord::Relation)
-
       resource = resource.preload([
+        :stages,
         :retryable_builds,
         :cancelable_statuses,
         :trigger_requests,
@@ -20,10 +17,14 @@ class PipelineSerializer < BaseSerializer
     end
 
     if paginated?
-      super(@paginator.paginate(resource), opts)
-    else
-      super(resource, opts)
+      resource = paginator.paginate(resource)
     end
+
+    if opts.delete(:preload)
+      resource = Gitlab::Ci::Pipeline::Preloader.preload!(resource)
+    end
+
+    super(resource, opts)
   end
 
   def represent_status(resource)
@@ -36,7 +37,7 @@ class PipelineSerializer < BaseSerializer
   def represent_stages(resource)
     return {} unless resource.present?
 
-    data = represent(resource, { only: [{ details: [:stages] }] })
+    data = represent(resource, { only: [{ details: [:stages] }], preload: true })
     data.dig(:details, :stages) || []
   end
 end

db/migrate/20180530135500_add_index_to_stages_position.rb

+class AddIndexToStagesPosition < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :ci_stages, [:pipeline_id, :position]
+  end
+
+  def down
+    remove_concurrent_index :ci_stages, [:pipeline_id, :position]
+  end
+end

db/schema.rb

@@ -520,6 +520,7 @@ ActiveRecord::Schema.define(version: 20180531220618) do
   end
 
   add_index "ci_stages", ["pipeline_id", "name"], name: "index_ci_stages_on_pipeline_id_and_name", unique: true, using: :btree
+  add_index "ci_stages", ["pipeline_id", "position"], name: "index_ci_stages_on_pipeline_id_and_position", using: :btree
   add_index "ci_stages", ["pipeline_id"], name: "index_ci_stages_on_pipeline_id", using: :btree
   add_index "ci_stages", ["project_id"], name: "index_ci_stages_on_project_id", using: :btree
 

lib/gitlab/ci/pipeline/preloader.rb

@@ -5,22 +5,46 @@ module Gitlab
     module Pipeline
       # Class for preloading data associated with pipelines such as commit
       # authors.
-      module Preloader
-        def self.preload(pipelines)
-          # This ensures that all the pipeline commits are eager loaded before we
-          # start using them.
+      class Preloader
+        def self.preload!(pipelines)
+          ##
+          # This preloads all commits at once, because `Ci::Pipeline#commit` is
+          # using a lazy batch loading, what results in only one batched Gitaly
+          # call.
+          #
           pipelines.each(&:commit)
 
           pipelines.each do |pipeline|
-            # This preloads the author of every commit. We're using "lazy_author"
+            self.new(pipeline).tap do |preloader|
+              preloader.preload_commit_authors
+              preloader.preload_pipeline_warnings
+              preloader.preload_stages_warnings
+            end
+          end
+        end
+
+        def initialize(pipeline)
+          @pipeline = pipeline
+        end
+
+        def preload_commit_authors
+          # This also preloads the author of every commit. We're using "lazy_author"
           # here since "author" immediately loads the data on the first call.
-            pipeline.commit.try(:lazy_author)
+          @pipeline.commit.try(:lazy_author)
+        end
 
+        def preload_pipeline_warnings
           # This preloads the number of warnings for every pipeline, ensuring
           # that Ci::Pipeline#has_warnings? doesn't execute any additional
           # queries.
-            pipeline.number_of_warnings
+          @pipeline.number_of_warnings
         end
+
+        def preload_stages_warnings
+          # This preloads the number of warnings for every stage, ensuring
+          # that Ci::Stage#has_warnings? doesn't execute any additional
+          # queries.
+          @pipeline.stages.each { |stage| stage.number_of_warnings }
         end
       end
     end

lib/gitlab/ci/status/stage/common.rb

@@ -8,7 +8,9 @@ module Gitlab
           end
 
           def details_path
-            project_pipeline_path(subject.project, subject.pipeline, anchor: subject.name)
+            project_pipeline_path(subject.pipeline.project,
+                                  subject.pipeline,
+                                  anchor: subject.name)
           end
 
           def has_action?

spec/controllers/projects/pipelines_controller_spec.rb

@@ -17,44 +17,103 @@ describe Projects::PipelinesController do
 
   describe 'GET index.json' do
     before do
-      %w(pending running created success).each_with_index do |status, index|
-        sha = project.commit("HEAD~#{index}")
-        create(:ci_empty_pipeline, status: status, project: project, sha: sha)
+      %w(pending running success failed canceled).each_with_index do |status, index|
+        create_pipeline(status, project.commit("HEAD~#{index}"))
       end
     end
 
-    subject do
-      get :index, namespace_id: project.namespace, project_id: project, format: :json
+    context 'when using persisted stages', :request_store do
+      before do
+        stub_feature_flags(ci_pipeline_persisted_stages: true)
       end
 
-    it 'returns JSON with serialized pipelines' do
-      subject
+      it 'returns serialized pipelines', :request_store do
+        queries = ActiveRecord::QueryRecorder.new do
+          get_pipelines_index_json
+        end
 
         expect(response).to have_gitlab_http_status(:ok)
         expect(response).to match_response_schema('pipeline')
 
         expect(json_response).to include('pipelines')
-      expect(json_response['pipelines'].count).to eq 4
-      expect(json_response['count']['all']).to eq '4'
+        expect(json_response['pipelines'].count).to eq 5
+        expect(json_response['count']['all']).to eq '5'
         expect(json_response['count']['running']).to eq '1'
         expect(json_response['count']['pending']).to eq '1'
-      expect(json_response['count']['finished']).to eq '1'
+        expect(json_response['count']['finished']).to eq '3'
+
+        json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages|
+          expect(stages.count).to eq 3
+        end
+
+        expect(queries.count).to be
+      end
+    end
+
+    context 'when using legacy stages', :request_store  do
+      before do
+        stub_feature_flags(ci_pipeline_persisted_stages: false)
+      end
+
+      it 'returns JSON with serialized pipelines', :request_store do
+        queries = ActiveRecord::QueryRecorder.new do
+          get_pipelines_index_json
+        end
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to match_response_schema('pipeline')
+
+        expect(json_response).to include('pipelines')
+        expect(json_response['pipelines'].count).to eq 5
+        expect(json_response['count']['all']).to eq '5'
+        expect(json_response['count']['running']).to eq '1'
+        expect(json_response['count']['pending']).to eq '1'
+        expect(json_response['count']['finished']).to eq '3'
+
+        json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages|
+          expect(stages.count).to eq 3
+        end
+
+        expect(queries.count).to be_within(3).of(30)
+      end
     end
 
     it 'does not include coverage data for the pipelines' do
-      subject
+      get_pipelines_index_json
 
       expect(json_response['pipelines'][0]).not_to include('coverage')
     end
 
     context 'when performing gitaly calls', :request_store do
       it 'limits the Gitaly requests' do
-        expect { subject }.to change { Gitlab::GitalyClient.get_request_count }.by(3)
+        expect { get_pipelines_index_json }
+          .to change { Gitlab::GitalyClient.get_request_count }.by(2)
+      end
     end
+
+    def get_pipelines_index_json
+      get :index, namespace_id: project.namespace,
+                  project_id: project,
+                  format: :json
+    end
+
+    def create_pipeline(status, sha)
+      pipeline = create(:ci_empty_pipeline, status: status,
+                                            project: project,
+                                            sha: sha)
+
+      create_build(pipeline, 'build', 1, 'build')
+      create_build(pipeline, 'test', 2, 'test')
+      create_build(pipeline, 'deploy', 3, 'deploy')
+    end
+
+    def create_build(pipeline, stage, stage_idx, name)
+      status = %w[created running pending success failed canceled].sample
+      create(:ci_build, pipeline: pipeline, stage: stage, stage_idx: stage_idx, name: name, status: status)
     end
   end
 
-  describe 'GET show JSON' do
+  describe 'GET show.json' do
     let(:pipeline) { create(:ci_pipeline_with_one_job, project: project) }
 
     it 'returns the pipeline' do
@@ -67,6 +126,14 @@ describe Projects::PipelinesController do
     end
 
     context 'when the pipeline has multiple stages and groups', :request_store do
+      let(:project) { create(:project, :repository) }
+
+      let(:pipeline) do
+        create(:ci_empty_pipeline, project: project,
+                                   user: user,
+                                   sha: project.commit.id)
+      end
+
       before do
         create_build('build', 0, 'build')
         create_build('test', 1, 'rspec 0')
@@ -74,11 +141,6 @@ describe Projects::PipelinesController do
         create_build('post deploy', 3, 'pages 0')
       end
 
-      let(:project) { create(:project, :repository) }
-      let(:pipeline) do
-        create(:ci_empty_pipeline, project: project, user: user, sha: project.commit.id)
-      end
-
       it 'does not perform N + 1 queries' do
         control_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count
 
@@ -90,6 +152,7 @@ describe Projects::PipelinesController do
         create_build('post deploy', 3, 'pages 2')
 
         new_count = ActiveRecord::QueryRecorder.new { get_pipeline_json }.count
+
         expect(new_count).to be_within(12).of(control_count)
       end
     end

spec/lib/gitlab/ci/pipeline/preloader_spec.rb

@@ -3,18 +3,47 @@
 require 'spec_helper'
 
 describe Gitlab::Ci::Pipeline::Preloader do
-  describe '.preload' do
-    it 'preloads the author of every pipeline commit' do
-      commit = double(:commit)
-      pipeline = double(:pipeline, commit: commit)
+  let(:stage) { double(:stage) }
+  let(:commit) { double(:commit) }
 
-      expect(commit)
-        .to receive(:lazy_author)
+  let(:pipeline) do
+    double(:pipeline, commit: commit, stages: [stage])
+  end
+
+  describe '.preload!' do
+    context 'when preloading multiple commits' do
+      let(:project) { create(:project, :repository) }
+
+      it 'preloads all commits once' do
+        expect(Commit).to receive(:decorate).once.and_call_original
+
+        pipelines = [build_pipeline(ref: 'HEAD'),
+                     build_pipeline(ref: 'HEAD~1')]
+
+        described_class.preload!(pipelines)
+      end
+
+      def build_pipeline(ref:)
+        build_stubbed(:ci_pipeline, project: project, sha: project.commit(ref).id)
+      end
+    end
+
+    it 'preloads commit authors and number of warnings' do
+      expect(commit).to receive(:lazy_author)
+      expect(pipeline).to receive(:number_of_warnings)
+      expect(stage).to receive(:number_of_warnings)
+
+      described_class.preload!([pipeline])
+    end
+
+    it 'returns original collection' do
+      allow(commit).to receive(:lazy_author)
+      allow(pipeline).to receive(:number_of_warnings)
+      allow(stage).to receive(:number_of_warnings)
 
-      expect(pipeline)
-        .to receive(:number_of_warnings)
+      pipelines = [pipeline, pipeline]
 
-      described_class.preload([pipeline])
+      expect(described_class.preload!(pipelines)).to eq pipelines
     end
   end
 end

spec/lib/gitlab/import_export/all_models.yml

@@ -257,6 +257,7 @@ project:
 - import_data
 - commit_statuses
 - pipelines
+- stages
 - builds
 - runner_projects
 - runners

spec/models/ci/group_spec.rb

@@ -41,4 +41,55 @@ describe Ci::Group do
       end
     end
   end
+
+  describe '.fabricate' do
+    let(:pipeline) { create(:ci_empty_pipeline) }
+    let(:stage) { create(:ci_stage_entity, pipeline: pipeline) }
+
+    before do
+      create_build(:ci_build, name: 'rspec 0 2')
+      create_build(:ci_build, name: 'rspec 0 1')
+      create_build(:ci_build, name: 'spinach 0 1')
+      create_build(:commit_status, name: 'aaaaa')
+    end
+
+    it 'returns an array of three groups' do
+      expect(stage.groups).to be_a Array
+      expect(stage.groups).to all(be_a described_class)
+      expect(stage.groups.size).to eq 3
+    end
+
+    it 'returns groups with correctly ordered statuses' do
+      expect(stage.groups.first.jobs.map(&:name))
+        .to eq ['aaaaa']
+      expect(stage.groups.second.jobs.map(&:name))
+        .to eq ['rspec 0 1', 'rspec 0 2']
+      expect(stage.groups.third.jobs.map(&:name))
+        .to eq ['spinach 0 1']
+    end
+
+    it 'returns groups with correct names' do
+      expect(stage.groups.map(&:name))
+        .to eq %w[aaaaa rspec spinach]
+    end
+
+    context 'when a name is nil on legacy pipelines' do
+      before do
+        pipeline.builds.first.update_attribute(:name, nil)
+      end
+
+      it 'returns an array of three groups' do
+        expect(stage.groups.map(&:name))
+          .to eq ['', 'aaaaa', 'rspec', 'spinach']
+      end
+    end
+
+    def create_build(type, status: 'success', **opts)
+      create(type, pipeline: pipeline,
+                   stage: stage.name,
+                   status: status,
+                   stage_id: stage.id,
+                   **opts)
+    end
+  end
 end

spec/models/ci/pipeline_spec.rb

@@ -537,6 +537,87 @@ describe Ci::Pipeline, :mailer do
         end
       end
     end
+
+    describe '#stages' do
+      before do
+        create(:ci_stage_entity, project: project,
+                                 pipeline: pipeline,
+                                 name: 'build')
+      end
+
+      it 'returns persisted stages' do
+        expect(pipeline.stages).not_to be_empty
+        expect(pipeline.stages).to all(be_persisted)
+      end
+    end
+
+    describe '#ordered_stages' do
+      before do
+        create(:ci_stage_entity, project: project,
+                                 pipeline: pipeline,
+                                 position: 4,
+                                 name: 'deploy')
+
+        create(:ci_build, project: project,
+                          pipeline: pipeline,
+                          stage: 'test',
+                          stage_idx: 3,
+                          name: 'test')
+
+        create(:ci_build, project: project,
+                          pipeline: pipeline,
+                          stage: 'build',
+                          stage_idx: 2,
+                          name: 'build')
+
+        create(:ci_stage_entity, project: project,
+                                 pipeline: pipeline,
+                                 position: 1,
+                                 name: 'sanity')
+
+        create(:ci_stage_entity, project: project,
+                                 pipeline: pipeline,
+                                 position: 5,
+                                 name: 'cleanup')
+      end
+
+      subject { pipeline.ordered_stages }
+
+      context 'when using legacy stages' do
+        before do
+          stub_feature_flags(ci_pipeline_persisted_stages: false)
+        end
+
+        it 'returns legacy stages in valid order' do
+          expect(subject.map(&:name)).to eq %w[build test]
+        end
+      end
+
+      context 'when using persisted stages' do
+        before do
+          stub_feature_flags(ci_pipeline_persisted_stages: true)
+        end
+
+        context 'when pipelines is not complete' do
+          it 'still returns legacy stages' do
+            expect(subject).to all(be_a Ci::LegacyStage)
+            expect(subject.map(&:name)).to eq %w[build test]
+          end
+        end
+
+        context 'when pipeline is complete' do
+          before do
+            pipeline.succeed!
+          end
+
+          it 'returns stages in valid order' do
+            expect(subject).to all(be_a Ci::Stage)
+            expect(subject.map(&:name))
+              .to eq %w[sanity build test deploy cleanup]
+          end
+        end
+      end
+    end
   end
 
   describe 'state machine' do
@@ -1181,6 +1262,43 @@ describe Ci::Pipeline, :mailer do
     end
   end
 
+  describe '#update_status' do
+    context 'when pipeline is empty' do
+      it 'updates does not change pipeline status' do
+        expect(pipeline.statuses.latest.status).to be_nil
+
+        expect { pipeline.update_status }
+          .to change { pipeline.reload.status }.to 'skipped'
+      end
+    end
+
+    context 'when updating status to pending' do
+      before do
+        allow(pipeline)
+          .to receive_message_chain(:statuses, :latest, :status)
+          .and_return(:running)
+      end
+
+      it 'updates pipeline status to running' do
+        expect { pipeline.update_status }
+          .to change { pipeline.reload.status }.to 'running'
+      end
+    end
+
+    context 'when statuses status was not recognized' do
+      before do
+        allow(pipeline)
+          .to receive(:latest_builds_status)
+          .and_return(:unknown)
+      end
+
+      it 'raises an exception' do
+        expect { pipeline.update_status }
+          .to raise_error(HasStatus::UnknownStatusError)
+      end
+    end
+  end
+
   describe '#detailed_status' do
     subject { pipeline.detailed_status(user) }
 

spec/models/ci/stage_spec.rb

@@ -65,7 +65,31 @@ describe Ci::Stage, :models do
       end
     end
 
-    context 'when stage is skipped' do
+    context 'when stage has only created builds' do
+      let(:stage) { create(:ci_stage_entity, status: :created) }
+
+      before do
+        create(:ci_build, :created, stage_id: stage.id)
+      end
+
+      it 'updates status to skipped' do
+        expect(stage.reload.status).to eq 'created'
+      end
+    end
+
+    context 'when stage is skipped because of skipped builds' do
+      before do
+        create(:ci_build, :skipped, stage_id: stage.id)
+      end
+
+      it 'updates status to skipped' do
+        expect { stage.update_status }
+          .to change { stage.reload.status }
+          .to 'skipped'
+      end
+    end
+
+    context 'when stage is skipped because is empty' do
       it 'updates status to skipped' do
         expect { stage.update_status }
           .to change { stage.reload.status }
@@ -86,9 +110,85 @@ describe Ci::Stage, :models do
         expect(stage.reload).to be_failed
       end
     end
+
+    context 'when statuses status was not recognized' do
+      before do
+        allow(stage)
+          .to receive_message_chain(:statuses, :latest, :status)
+          .and_return(:unknown)
+      end
+
+      it 'raises an exception' do
+        expect { stage.update_status }
+          .to raise_error(HasStatus::UnknownStatusError)
+      end
+    end
+  end
+
+  describe '#detailed_status' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:user) { create(:user) }
+    let(:stage) { create(:ci_stage_entity, status: :created) }
+    subject { stage.detailed_status(user) }
+
+    where(:statuses, :label) do
+      %w[created]         | :created
+      %w[success]         | :passed
+      %w[pending]         | :pending
+      %w[skipped]         | :skipped
+      %w[canceled]        | :canceled
+      %w[success failed]  | :failed
+      %w[running pending] | :running
+    end
+
+    with_them do
+      before do
+        statuses.each do |status|
+          create(:commit_status, project: stage.project,
+                                 pipeline: stage.pipeline,
+                                 stage_id: stage.id,
+                                 status: status)
+
+          stage.update_status
+        end
+      end
+
+      it 'has a correct label' do
+        expect(subject.label).to eq label.to_s
+      end
+    end
+
+    context 'when stage has warnings' do
+      before do
+        create(:ci_build, project: stage.project,
+                          pipeline: stage.pipeline,
+                          stage_id: stage.id,
+                          status: :failed,
+                          allow_failure: true)
+
+        stage.update_status
+      end
+
+      it 'is passed with warnings' do
+        expect(subject.label).to eq 'passed with warnings'
+      end
+    end
+  end
+
+  describe '#groups' do
+    before do
+      create(:ci_build, stage_id: stage.id, name: 'rspec 0 1')
+      create(:ci_build, stage_id: stage.id, name: 'rspec 0 2')
     end
 
-  describe '#index' do
+    it 'groups stage builds by name' do
+      expect(stage.groups).to be_one
+      expect(stage.groups.first.name).to eq 'rspec'
+    end
+  end
+
+  describe '#position' do
     context 'when stage has been imported and does not have position index set' do
       before do
         stage.update_column(:position, nil)
@@ -119,4 +219,42 @@ describe Ci::Stage, :models do
       end
     end
   end
+
+  context 'when stage has warnings' do
+    before do
+      create(:ci_build, :failed, :allowed_to_fail, stage_id: stage.id)
+    end
+
+    describe '#has_warnings?' do
+      it 'returns true' do
+        expect(stage).to have_warnings
+      end
+    end
+
+    describe '#number_of_warnings' do
+      it 'returns a lazy stage warnings counter' do
+        lazy_queries = ActiveRecord::QueryRecorder.new do
+          stage.number_of_warnings
+        end
+
+        synced_queries = ActiveRecord::QueryRecorder.new do
+          stage.number_of_warnings.to_i
+        end
+
+        expect(lazy_queries.count).to eq 0
+        expect(synced_queries.count).to eq 1
+
+        expect(stage.number_of_warnings.inspect).to include 'BatchLoader'
+        expect(stage.number_of_warnings).to eq 1
+      end
+    end
+  end
+
+  context 'when stage does not have warnings' do
+    describe '#has_warnings?' do
+      it 'returns false' do
+        expect(stage).not_to have_warnings
+      end
+    end
+  end
 end

spec/serializers/pipeline_serializer_spec.rb

@@ -8,6 +8,10 @@ describe PipelineSerializer do
     described_class.new(current_user: user)
   end
 
+  before do
+    stub_feature_flags(ci_pipeline_persisted_stages: true)
+  end
+
   subject { serializer.represent(resource) }
 
   describe '#represent' do
@@ -99,7 +103,8 @@ describe PipelineSerializer do
       end
     end
 
-    context 'number of queries' do
+    describe 'number of queries when preloaded' do
+      subject { serializer.represent(resource, preload: true) }
       let(:resource) { Ci::Pipeline.all }
 
       before do
@@ -107,7 +112,7 @@ describe PipelineSerializer do
         # gitaly calls in this block
         # Issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/37772
         Gitlab::GitalyClient.allow_n_plus_1_calls do
-          Ci::Pipeline::AVAILABLE_STATUSES.each do |status|
+          Ci::Pipeline::COMPLETED_STATUSES.each do |status|
             create_pipeline(status)
           end
         end
@@ -120,7 +125,7 @@ describe PipelineSerializer do
         it 'verifies number of queries', :request_store do
           recorded = ActiveRecord::QueryRecorder.new { subject }
 
-          expect(recorded.count).to be_within(1).of(44)
+          expect(recorded.count).to be_within(2).of(27)
           expect(recorded.cached_count).to eq(0)
         end
       end
@@ -139,7 +144,7 @@ describe PipelineSerializer do
           # pipeline. With the same ref this check is cached but if refs are
           # different then there is an extra query per ref
           # https://gitlab.com/gitlab-org/gitlab-ce/issues/46368
-          expect(recorded.count).to be_within(1).of(51)
+          expect(recorded.count).to be_within(2).of(30)
           expect(recorded.cached_count).to eq(0)
         end
       end