Improved CI. Fix yaml_errors boolean evaluation.

2 files changed, 40 insertions, 57 deletions

diff --git a/app/finders/pipelines_finder.rb b/app/finders/pipelines_finder.rb
index 03183efadda..93f1ab9598f 100644
--- a/app/finders/pipelines_finder.rb
+++ b/app/finders/pipelines_finder.rb
@@ -89,7 +89,7 @@ class PipelinesFinder
 
   def by_yaml_errors(items)
     flg = Gitlab::Utils.to_boolean(params[:yaml_errors])
-    if flg.present?
+    if !flg.nil?
       if flg
         items.where("yaml_errors IS NOT NULL")
       else
diff --git a/spec/finders/pipelines_finder_spec.rb b/spec/finders/pipelines_finder_spec.rb
index 42c5868fa91..a1c89ba03db 100644
--- a/spec/finders/pipelines_finder_spec.rb
+++ b/spec/finders/pipelines_finder_spec.rb
@@ -1,28 +1,22 @@
 require 'spec_helper'
 
 describe PipelinesFinder do
-  let(:user) { create(:user) }
-  let(:project) { create(:project, :repository) }
-
-  let!(:tag_pipeline)         { create(:ci_pipeline, project: project, user: user, created_at: 10.minutes.ago, ref: 'v1.0.0') }
-  let!(:created_pipeline)     { create(:ci_pipeline, project: project, user: user, created_at:  9.minutes.ago, status: 'created') }
-  let!(:pending_pipeline)     { create(:ci_pipeline, project: project, user: user, created_at:  8.minutes.ago, status: 'pending') }
-  let!(:running_pipeline)     { create(:ci_pipeline, project: project, user: user, created_at:  7.minutes.ago, status: 'running') }
-  let!(:success_pipeline)     { create(:ci_pipeline, project: project, user: user, created_at:  6.minutes.ago, status: 'success') }
-  let!(:failed_pipeline)      { create(:ci_pipeline, project: project, user: user, created_at:  5.minutes.ago, status: 'failed') }
-  let!(:canceled_pipeline)    { create(:ci_pipeline, project: project, user: user, created_at:  2.minutes.ago, status: 'canceled') }
-  let!(:skipped_pipeline)     { create(:ci_pipeline, project: project, user: user, created_at:  1.minute.ago,  status: 'skipped') }
-  let!(:yaml_errors_pipeline) { create(:ci_pipeline, project: project, user: user, yaml_errors: 'Syntax error') }
-  let(:dummy_pipelines) do
-    [tag_pipeline,
-    created_pipeline,
-    pending_pipeline,
-    running_pipeline,
-    success_pipeline,
-    failed_pipeline,
-    canceled_pipeline,
-    skipped_pipeline,
-    yaml_errors_pipeline]
+  let!(:user1) { create(:user) }
+  let!(:user2) { create(:user) }
+  let!(:project) { create(:project, :repository) }
+
+  let!(:dummy_pipelines) do
+    {
+      tag:          create(:ci_pipeline, project: project, user: user1, created_at:  9.minutes.ago, ref: 'v1.0.0'),
+      created:      create(:ci_pipeline, project: project, user: user1, created_at:  8.minutes.ago, status: 'created'),
+      pending:      create(:ci_pipeline, project: project, user: user1, created_at:  7.minutes.ago, status: 'pending'),
+      running:      create(:ci_pipeline, project: project, user: user1, created_at:  6.minutes.ago, status: 'running'),
+      success:      create(:ci_pipeline, project: project, user: user1, created_at:  5.minutes.ago, status: 'success'),
+      failed:       create(:ci_pipeline, project: project, user: user2, created_at:  4.minutes.ago, status: 'failed'),
+      canceled:     create(:ci_pipeline, project: project, user: user2, created_at:  3.minutes.ago, status: 'canceled'),
+      skipped:      create(:ci_pipeline, project: project, user: user2, created_at:  2.minutes.ago, status: 'skipped'),
+      yaml_errors:  create(:ci_pipeline, project: project, user: user2, created_at:  1.minute.ago,  yaml_errors: 'Syntax error'),
+    }
   end
 
   subject { described_class.new(project, params).execute }
@@ -33,11 +27,11 @@ describe PipelinesFinder do
 
       it 'selects all pipelines' do
         expect(subject.count).to be dummy_pipelines.count
-        expect(subject).to match_array(dummy_pipelines)
+        expect(subject).to match_array dummy_pipelines.values
       end
 
       it 'orders in descending order on ID' do
-        expect(subject.map(&:id)).to eq dummy_pipelines.map(&:id).sort.reverse
+        expect(subject.map(&:id)).to eq dummy_pipelines.values.map(&:id).sort.reverse
       end
     end
 
@@ -71,7 +65,7 @@ describe PipelinesFinder do
 
         it 'excludes tags' do
           expect(subject.count).to be 1
-          expect(subject).not_to include tag_pipeline
+          expect(subject).not_to include dummy_pipelines[:tag]
           expect(subject.map(&:ref)).to include('master')
         end
       end
@@ -81,7 +75,7 @@ describe PipelinesFinder do
 
         it 'excludes branches' do
           expect(subject.count).to be 1
-          expect(subject).to     include tag_pipeline
+          expect(subject).to include dummy_pipelines[:tag]
           expect(subject.map(&:ref)).not_to include('master')
         end
       end
@@ -142,15 +136,8 @@ describe PipelinesFinder do
         let(:params) { { ref: 'master' } }
 
         it 'selects all pipelines which belong to the ref' do
-          expect(subject.count).to be 8
-          expect(subject).to include created_pipeline
-          expect(subject).to include pending_pipeline
-          expect(subject).to include running_pipeline
-          expect(subject).to include success_pipeline
-          expect(subject).to include failed_pipeline
-          expect(subject).to include canceled_pipeline
-          expect(subject).to include skipped_pipeline
-          expect(subject).to include yaml_errors_pipeline
+          expected = dummy_pipelines.except(:tag)
+          expect(subject).to match_array expected.values
         end
       end
 
@@ -165,11 +152,11 @@ describe PipelinesFinder do
 
     context 'when a username is passed' do
       context 'when a username exists' do
-        let(:params) { { username: user.name } }
+        let(:params) { { username: user1.name } }
 
         it 'selects all pipelines which belong to the username' do
-          expect(subject).to include success_pipeline
-          expect(subject).to include failed_pipeline
+          expected = dummy_pipelines.except(:failed).except(:canceled).except(:skipped).except(:yaml_errors)
+          expect(subject).to match_array expected.values
         end
       end
 
@@ -186,22 +173,19 @@ describe PipelinesFinder do
       context 'when yaml_errors is true' do
         let(:params) { { yaml_errors: true } }
 
-        it 'selects only pipelines has yaml_errors' do
-          expect(subject).to include yaml_errors_pipeline
+        it 'selects only pipelines have yaml_errors' do
+          expect(subject.count).to be 1
+          expect(subject.first).to eq dummy_pipelines[:yaml_errors]
         end
       end
 
       context 'when yaml_errors is false' do
         let(:params) { { yaml_errors: false } }
 
-        it 'selects only pipelines does not have yaml_errors' do
-          expect(subject).to include created_pipeline
-          expect(subject).to include pending_pipeline
-          expect(subject).to include running_pipeline
-          expect(subject).to include success_pipeline
-          expect(subject).to include failed_pipeline
-          expect(subject).to include canceled_pipeline
-          expect(subject).to include skipped_pipeline
+        it 'selects only pipelines do not have yaml_errors' do
+          expected = dummy_pipelines.except(:yaml_errors)
+          expect(subject.count).to be expected.count
+          expect(subject).to match_array expected.values
         end
       end
 
@@ -209,8 +193,7 @@ describe PipelinesFinder do
         let(:params) { { yaml_errors: "UnexpectedValue" } }
 
         it 'selects all pipelines' do
-          expect(subject.count).to be dummy_pipelines.count
-          expect(subject).to match_array(dummy_pipelines)
+          expect(subject).to match_array dummy_pipelines.values
         end
       end
     end
@@ -220,8 +203,8 @@ describe PipelinesFinder do
         let(:params) { { order_by: 'created_at', sort: 'asc' } }
 
         it 'sorts by created_at asc' do
-          expect(subject.first).to eq(tag_pipeline)
-          expect(subject.last).to eq(yaml_errors_pipeline)
+          expect(subject.first).to eq dummy_pipelines[:tag]
+          expect(subject.last).to eq dummy_pipelines[:yaml_errors]
         end
       end
 
@@ -229,8 +212,8 @@ describe PipelinesFinder do
         let(:params) { { order_by: 'created_at', sort: 'desc' } }
 
         it 'sorts by created_at desc' do
-          expect(subject.first).to eq(yaml_errors_pipeline)
-          expect(subject.last).to eq(tag_pipeline)
+          expect(subject.first).to eq dummy_pipelines[:yaml_errors]
+          expect(subject.last).to eq dummy_pipelines[:tag]
         end
       end
 
@@ -238,7 +221,7 @@ describe PipelinesFinder do
         let(:params) { { order_by: 'abnormal_column', sort: 'desc' } }
 
         it 'sorts by default' do
-          expect(subject.map(&:id)).to eq dummy_pipelines.map(&:id).sort.reverse
+          expect(subject.map(&:id)).to eq dummy_pipelines.values.map(&:id).sort.reverse
         end
       end
 
@@ -246,7 +229,7 @@ describe PipelinesFinder do
         let(:params) { { order_by: 'created_at', sort: 'abnormal_sort' } }
 
         it 'sorts by default' do
-          expect(subject.map(&:id)).to eq dummy_pipelines.map(&:id).sort.reverse
+          expect(subject.map(&:id)).to eq dummy_pipelines.values.map(&:id).sort.reverse
         end
       end
     end