From 8cca6c83a99100fcf3a0a3a56f10eebe3c9b7716 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 23 May 2018 10:37:01 +0200
Subject: [PATCH] DRY creating groups of common builds in a stage

---
 app/models/ci/group.rb        |  8 ++++++
 app/models/ci/legacy_stage.rb |  6 +----
 app/models/ci/stage.rb        |  6 +----
 spec/models/ci/group_spec.rb  | 51 +++++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/app/models/ci/group.rb b/app/models/ci/group.rb
index 87898b086c6..9c1046e8715 100644
--- a/app/models/ci/group.rb
+++ b/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
diff --git a/app/models/ci/legacy_stage.rb b/app/models/ci/legacy_stage.rb
index 9b536af672b..ce691875e42 100644
--- a/app/models/ci/legacy_stage.rb
+++ b/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
diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb
index faedb9c29fd..0044d1af106 100644
--- a/app/models/ci/stage.rb
+++ b/app/models/ci/stage.rb
@@ -81,11 +81,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 has_warnings?
diff --git a/spec/models/ci/group_spec.rb b/spec/models/ci/group_spec.rb
index 51123e73fe6..9052dd59369 100644
--- a/spec/models/ci/group_spec.rb
+++ b/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 Ci::Group)
+      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