From b9d43e27b294e0ac18421e4e0540ba2c5fd80d15 Mon Sep 17 00:00:00 2001
From: drew cimino <dcimino@gitlab.com>
Date: Tue, 25 Jun 2019 17:05:18 -0400
Subject: [PATCH 1/3] Added specs for Ci::Pipeline::Seed::Build

- #included? when only: and except: both match
---
 .../lib/gitlab/ci/pipeline/seed/build_spec.rb | 57 ++++++++++++++++---
 1 file changed, 49 insertions(+), 8 deletions(-)

diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
index 7991e2f48b5..633000ec8fe 100644
--- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
@@ -3,14 +3,9 @@ require 'spec_helper'
 describe Gitlab::Ci::Pipeline::Seed::Build do
   let(:project) { create(:project, :repository) }
   let(:pipeline) { create(:ci_empty_pipeline, project: project) }
+  let(:attributes) { { name: 'rspec', ref: 'master' } }
 
-  let(:attributes) do
-    { name: 'rspec', ref: 'master' }
-  end
-
-  subject do
-    described_class.new(pipeline, attributes)
-  end
+  subject { described_class.new(pipeline, attributes) }
 
   describe '#attributes' do
     it 'returns hash attributes of a build' do
@@ -76,7 +71,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
     end
   end
 
-  describe 'applying only/except policies' do
+  describe 'applying job inclusion policies' do
     context 'when no branch policy is specified' do
       let(:attributes) { { name: 'rspec' } }
 
@@ -95,6 +90,12 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
         it { is_expected.to be_included }
       end
+
+      context 'with both only and except policies' do
+        let(:attributes) { { name: 'rspec', only: { refs: %w(deploy) }, except: { refs: %(deploy) } } }
+
+        it { is_expected.not_to be_included }
+      end
     end
 
     context 'when branch regexp policy does not match' do
@@ -109,6 +110,12 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
         it { is_expected.to be_included }
       end
+
+      context 'with both only and except policies' do
+        let(:attributes) { { name: 'rspec', only: { refs: %w(/^deploy$/) }, except: { refs: %w(/^deploy$/) } } }
+
+        it { is_expected.not_to be_included }
+      end
     end
 
     context 'when branch policy matches' do
@@ -123,6 +130,12 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
         it { is_expected.not_to be_included }
       end
+
+      context 'when using both only and except policies' do
+        let(:attributes) { { name: 'rspec', only: { refs: %w(deploy master) }, except: { refs: %w(deploy master) } } }
+
+        it { is_expected.not_to be_included }
+      end
     end
 
     context 'when keyword policy matches' do
@@ -137,6 +150,12 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
         it { is_expected.not_to be_included }
       end
+
+      context 'when using both only and except policies' do
+        let(:attributes) { { name: 'rspec', only: { refs: %w(branches) }, except: { refs: %(branches) } } }
+
+        it { is_expected.not_to be_included }
+      end
     end
 
     context 'when keyword policy does not match' do
@@ -151,6 +170,12 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
         it { is_expected.to be_included }
       end
+
+      context 'when using both only and except policies' do
+        let(:attributes) { { name: 'rspec', only: { refs: %(tags) }, except: { refs: %(tags) } } }
+
+        it { is_expected.not_to be_included }
+      end
     end
 
     context 'with source-keyword policy' do
@@ -239,6 +264,14 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
         it { is_expected.not_to be_included }
       end
+
+      context 'when using both only and except policies' do
+        let(:attributes) do
+          { name: 'rspec', only: { refs: ["branches@#{pipeline.project_full_path}"] }, except: { refs: ["branches@#{pipeline.project_full_path}"] } }
+        end
+
+        it { is_expected.not_to be_included }
+      end
     end
 
     context 'when repository path does not matches' do
@@ -257,6 +290,14 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
         it { is_expected.to be_included }
       end
+
+      context 'when using both only and except policies' do
+        let(:attributes) do
+          { name: 'rspec', only: { refs: ['branches@fork'] }, except: { refs: ['branches@fork'] } }
+        end
+
+        it { is_expected.not_to be_included }
+      end
     end
   end
 end

From 24941101483421f2661c040be0fb7612df40d375 Mon Sep 17 00:00:00 2001
From: drew cimino <dcimino@gitlab.com>
Date: Tue, 25 Jun 2019 18:46:28 -0400
Subject: [PATCH 2/3] Polish for Ci::Pipeline::Seed::Build specs

---
 .../lib/gitlab/ci/pipeline/seed/build_spec.rb | 205 ++++++++++++------
 1 file changed, 143 insertions(+), 62 deletions(-)

diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
index 633000ec8fe..e0bff638233 100644
--- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe Gitlab::Ci::Pipeline::Seed::Build do
@@ -5,23 +7,24 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
   let(:pipeline) { create(:ci_empty_pipeline, project: project) }
   let(:attributes) { { name: 'rspec', ref: 'master' } }
 
-  subject { described_class.new(pipeline, attributes) }
+  let(:seed_build) { described_class.new(pipeline, attributes) }
 
   describe '#attributes' do
-    it 'returns hash attributes of a build' do
-      expect(subject.attributes).to be_a Hash
-      expect(subject.attributes)
-        .to include(:name, :project, :ref)
-    end
+    subject { seed_build.attributes }
+
+    it { is_expected.to be_a(Hash) }
+    it { is_expected.to include(:name, :project, :ref) }
   end
 
   describe '#bridge?' do
+    subject { seed_build.bridge? }
+
     context 'when job is a bridge' do
       let(:attributes) do
         { name: 'rspec', ref: 'master', options: { trigger: 'my/project' } }
       end
 
-      it { is_expected.to be_bridge }
+      it { is_expected.to be_truthy }
     end
 
     context 'when trigger definition is empty' do
@@ -29,20 +32,20 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
         { name: 'rspec', ref: 'master', options: { trigger: '' } }
       end
 
-      it { is_expected.not_to be_bridge }
+      it { is_expected.to be_falsey }
     end
 
     context 'when job is not a bridge' do
-      it { is_expected.not_to be_bridge }
+      it { is_expected.to be_falsey }
     end
   end
 
   describe '#to_resource' do
+    subject { seed_build.to_resource }
+
     context 'when job is not a bridge' do
-      it 'returns a valid build resource' do
-        expect(subject.to_resource).to be_a(::Ci::Build)
-        expect(subject.to_resource).to be_valid
-      end
+      it { is_expected.to be_a(::Ci::Build) }
+      it { is_expected.to be_valid }
     end
 
     context 'when job is a bridge' do
@@ -50,49 +53,57 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
         { name: 'rspec', ref: 'master', options: { trigger: 'my/project' } }
       end
 
-      it 'returns a valid bridge resource' do
-        expect(subject.to_resource).to be_a(::Ci::Bridge)
-        expect(subject.to_resource).to be_valid
-      end
+      it { is_expected.to be_a(::Ci::Bridge) }
+      it { is_expected.to be_valid }
     end
 
     it 'memoizes a resource object' do
-      build = subject.to_resource
-
-      expect(build.object_id).to eq subject.to_resource.object_id
+      expect(subject.object_id).to eq seed_build.to_resource.object_id
     end
 
     it 'can not be persisted without explicit assignment' do
-      build = subject.to_resource
-
       pipeline.save!
 
-      expect(build).not_to be_persisted
+      expect(subject).not_to be_persisted
     end
   end
 
   describe 'applying job inclusion policies' do
+    subject { seed_build }
+
     context 'when no branch policy is specified' do
-      let(:attributes) { { name: 'rspec' } }
+      let(:attributes) do
+        { name: 'rspec' }
+      end
 
       it { is_expected.to be_included }
     end
 
     context 'when branch policy does not match' do
       context 'when using only' do
-        let(:attributes) { { name: 'rspec', only: { refs: ['deploy'] } } }
+        let(:attributes) do
+          { name: 'rspec', only: { refs: ['deploy'] } }
+        end
 
         it { is_expected.not_to be_included }
       end
 
       context 'when using except' do
-        let(:attributes) { { name: 'rspec', except: { refs: ['deploy'] } } }
+        let(:attributes) do
+          { name: 'rspec', except: { refs: ['deploy'] } }
+        end
 
         it { is_expected.to be_included }
       end
 
       context 'with both only and except policies' do
-        let(:attributes) { { name: 'rspec', only: { refs: %w(deploy) }, except: { refs: %(deploy) } } }
+        let(:attributes) do
+          {
+            name: 'rspec',
+            only: { refs: %w[deploy] },
+            except: { refs: %w[deploy] }
+          }
+        end
 
         it { is_expected.not_to be_included }
       end
@@ -100,19 +111,29 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
     context 'when branch regexp policy does not match' do
       context 'when using only' do
-        let(:attributes) { { name: 'rspec', only: { refs: ['/^deploy$/'] } } }
+        let(:attributes) do
+          { name: 'rspec', only: { refs: %w[/^deploy$/] } }
+        end
 
         it { is_expected.not_to be_included }
       end
 
       context 'when using except' do
-        let(:attributes) { { name: 'rspec', except: { refs: ['/^deploy$/'] } } }
+        let(:attributes) do
+          { name: 'rspec', except: { refs: %w[/^deploy$/] } }
+        end
 
         it { is_expected.to be_included }
       end
 
       context 'with both only and except policies' do
-        let(:attributes) { { name: 'rspec', only: { refs: %w(/^deploy$/) }, except: { refs: %w(/^deploy$/) } } }
+        let(:attributes) do
+          {
+            name: 'rspec',
+            only: { refs: %w[/^deploy$/] },
+            except: { refs: %w[/^deploy$/] }
+          }
+        end
 
         it { is_expected.not_to be_included }
       end
@@ -120,19 +141,29 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
     context 'when branch policy matches' do
       context 'when using only' do
-        let(:attributes) { { name: 'rspec', only: { refs: %w[deploy master] } } }
+        let(:attributes) do
+          { name: 'rspec', only: { refs: %w[deploy master] } }
+        end
 
         it { is_expected.to be_included }
       end
 
       context 'when using except' do
-        let(:attributes) { { name: 'rspec', except: { refs: %w[deploy master] } } }
+        let(:attributes) do
+          { name: 'rspec', except: { refs: %w[deploy master] } }
+        end
 
         it { is_expected.not_to be_included }
       end
 
       context 'when using both only and except policies' do
-        let(:attributes) { { name: 'rspec', only: { refs: %w(deploy master) }, except: { refs: %w(deploy master) } } }
+        let(:attributes) do
+          {
+            name: 'rspec',
+            only: { refs: %w[deploy master] },
+            except: { refs: %w[deploy master] }
+          }
+        end
 
         it { is_expected.not_to be_included }
       end
@@ -140,19 +171,29 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
     context 'when keyword policy matches' do
       context 'when using only' do
-        let(:attributes) { { name: 'rspec', only: { refs: ['branches'] } } }
+        let(:attributes) do
+          { name: 'rspec', only: { refs: %w[branches] } }
+        end
 
         it { is_expected.to be_included }
       end
 
       context 'when using except' do
-        let(:attributes) { { name: 'rspec', except: { refs: ['branches'] } } }
+        let(:attributes) do
+          { name: 'rspec', except: { refs: %w[branches] } }
+        end
 
         it { is_expected.not_to be_included }
       end
 
       context 'when using both only and except policies' do
-        let(:attributes) { { name: 'rspec', only: { refs: %w(branches) }, except: { refs: %(branches) } } }
+        let(:attributes) do
+          {
+            name: 'rspec',
+            only: { refs: %w[branches] },
+            except: { refs: %w[branches] }
+          }
+        end
 
         it { is_expected.not_to be_included }
       end
@@ -160,19 +201,29 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
     context 'when keyword policy does not match' do
       context 'when using only' do
-        let(:attributes) { { name: 'rspec', only: { refs: ['tags'] } } }
+        let(:attributes) do
+          { name: 'rspec', only: { refs: %w[tags] } }
+        end
 
         it { is_expected.not_to be_included }
       end
 
       context 'when using except' do
-        let(:attributes) { { name: 'rspec', except: { refs: ['tags'] } } }
+        let(:attributes) do
+          { name: 'rspec', except: { refs: %w[tags] } }
+        end
 
         it { is_expected.to be_included }
       end
 
       context 'when using both only and except policies' do
-        let(:attributes) { { name: 'rspec', only: { refs: %(tags) }, except: { refs: %(tags) } } }
+        let(:attributes) do
+          {
+            name: 'rspec',
+            only: { refs: %w[tags] },
+            except: { refs: %w[tags] }
+          }
+        end
 
         it { is_expected.not_to be_included }
       end
@@ -181,35 +232,47 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
     context 'with source-keyword policy' do
       using RSpec::Parameterized
 
-      let(:pipeline) { build(:ci_empty_pipeline, ref: 'deploy', tag: false, source: source) }
+      let(:pipeline) do
+        create(:ci_empty_pipeline, ref: 'deploy', tag: false, source: source)
+      end
 
       context 'matches' do
         where(:keyword, :source) do
           [
-            %w(pushes push),
-            %w(web web),
-            %w(triggers trigger),
-            %w(schedules schedule),
-            %w(api api),
-            %w(external external)
+            %w[pushes push],
+            %w[web web],
+            %w[triggers trigger],
+            %w[schedules schedule],
+            %w[api api],
+            %w[external external]
           ]
         end
 
         with_them do
           context 'using an only policy' do
-            let(:attributes) { { name: 'rspec', only: { refs: [keyword] } } }
+            let(:attributes) do
+              { name: 'rspec', only: { refs: [keyword] } }
+            end
 
             it { is_expected.to be_included }
           end
 
           context 'using an except policy' do
-            let(:attributes) { { name: 'rspec', except: { refs: [keyword] } } }
+            let(:attributes) do
+              { name: 'rspec', except: { refs: [keyword] } }
+            end
 
             it { is_expected.not_to be_included }
           end
 
           context 'using both only and except policies' do
-            let(:attributes) { { name: 'rspec', only: { refs: [keyword] }, except: { refs: [keyword] } } }
+            let(:attributes) do
+              {
+                name: 'rspec',
+                only: { refs: [keyword] },
+                except: { refs: [keyword] }
+              }
+            end
 
             it { is_expected.not_to be_included }
           end
@@ -218,29 +281,39 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
       context 'non-matches' do
         where(:keyword, :source) do
-          %w(web trigger schedule api external).map { |source| ['pushes', source] } +
-          %w(push trigger schedule api external).map { |source| ['web', source] } +
-          %w(push web schedule api external).map { |source| ['triggers', source] } +
-          %w(push web trigger api external).map { |source| ['schedules', source] } +
-          %w(push web trigger schedule external).map { |source| ['api', source] } +
-          %w(push web trigger schedule api).map { |source| ['external', source] }
+          %w[web trigger schedule api external].map  { |source| ['pushes', source] } +
+          %w[push trigger schedule api external].map { |source| ['web', source] } +
+          %w[push web schedule api external].map { |source| ['triggers', source] } +
+          %w[push web trigger api external].map { |source| ['schedules', source] } +
+          %w[push web trigger schedule external].map { |source| ['api', source] } +
+          %w[push web trigger schedule api].map { |source| ['external', source] }
         end
 
         with_them do
           context 'using an only policy' do
-            let(:attributes) { { name: 'rspec', only: { refs: [keyword] } } }
+            let(:attributes) do
+              { name: 'rspec', only: { refs: [keyword] } }
+            end
 
             it { is_expected.not_to be_included }
           end
 
           context 'using an except policy' do
-            let(:attributes) { { name: 'rspec', except: { refs: [keyword] } } }
+            let(:attributes) do
+              { name: 'rspec', except: { refs: [keyword] } }
+            end
 
             it { is_expected.to be_included }
           end
 
           context 'using both only and except policies' do
-            let(:attributes) { { name: 'rspec', only: { refs: [keyword] }, except: { refs: [keyword] } } }
+            let(:attributes) do
+              {
+                name: 'rspec',
+                only: { refs: [keyword] },
+                except: { refs: [keyword] }
+              }
+            end
 
             it { is_expected.not_to be_included }
           end
@@ -267,7 +340,11 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
       context 'when using both only and except policies' do
         let(:attributes) do
-          { name: 'rspec', only: { refs: ["branches@#{pipeline.project_full_path}"] }, except: { refs: ["branches@#{pipeline.project_full_path}"] } }
+          {
+            name: 'rspec',
+            only: { refs: ["branches@#{pipeline.project_full_path}"] },
+            except: { refs: ["branches@#{pipeline.project_full_path}"] }
+          }
         end
 
         it { is_expected.not_to be_included }
@@ -277,7 +354,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
     context 'when repository path does not matches' do
       context 'when using only' do
         let(:attributes) do
-          { name: 'rspec', only: { refs: ['branches@fork'] } }
+          { name: 'rspec', only: { refs: %w[branches@fork] } }
         end
 
         it { is_expected.not_to be_included }
@@ -285,7 +362,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
       context 'when using except' do
         let(:attributes) do
-          { name: 'rspec', except: { refs: ['branches@fork'] } }
+          { name: 'rspec', except: { refs: %w[branches@fork] } }
         end
 
         it { is_expected.to be_included }
@@ -293,7 +370,11 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
 
       context 'when using both only and except policies' do
         let(:attributes) do
-          { name: 'rspec', only: { refs: ['branches@fork'] }, except: { refs: ['branches@fork'] } }
+          {
+            name: 'rspec',
+            only: { refs: %w[branches@fork] },
+            except: { refs: %w[branches@fork] }
+          }
         end
 
         it { is_expected.not_to be_included }

From d589a4e8e1e54eb3810cc2c29649f34a632fd3e5 Mon Sep 17 00:00:00 2001
From: drew cimino <dcimino@gitlab.com>
Date: Tue, 16 Jul 2019 11:43:08 -0400
Subject: [PATCH 3/3] Build instead of create

---
 spec/lib/gitlab/ci/pipeline/seed/build_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
index e0bff638233..46ea0d7554b 100644
--- a/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/seed/build_spec.rb
@@ -233,7 +233,7 @@ describe Gitlab::Ci::Pipeline::Seed::Build do
       using RSpec::Parameterized
 
       let(:pipeline) do
-        create(:ci_empty_pipeline, ref: 'deploy', tag: false, source: source)
+        build(:ci_empty_pipeline, ref: 'deploy', tag: false, source: source)
       end
 
       context 'matches' do