Lint our factories creation in addition to their build33139-lint-our-factories

Signed-off-by: Rémy Coutable <remy@rymai.me>

15 files changed, 93 insertions, 47 deletions

diff --git a/app/models/commit.rb b/app/models/commit.rb
index dbc0a22829e..0a16af688bd 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -14,7 +14,7 @@ class Commit
   participant :committer
   participant :notes_with_associations
 
-  attr_accessor :project
+  attr_accessor :project, :author
 
   DIFF_SAFE_LINES = Gitlab::Git::DiffCollection::DEFAULT_LIMITS[:max_lines]
 
diff --git a/spec/controllers/profiles/keys_controller_spec.rb b/spec/controllers/profiles/keys_controller_spec.rb
index 61e4fae46fb..363ed410bc0 100644
--- a/spec/controllers/profiles/keys_controller_spec.rb
+++ b/spec/controllers/profiles/keys_controller_spec.rb
@@ -49,7 +49,7 @@ describe Profiles::KeysController do
         expect(response.body).to eq(user.all_ssh_keys.join("\n"))
 
         expect(response.body).to include(key.key.sub(' dummy@gitlab.com', ''))
-        expect(response.body).to include(another_key.key)
+        expect(response.body).to include(another_key.key.sub(' dummy@gitlab.com', ''))
 
         expect(response.body).not_to include(deploy_key.key)
       end
diff --git a/spec/factories/ci/pipelines.rb b/spec/factories/ci/pipelines.rb
index 361c5b9a49e..c601661d122 100644
--- a/spec/factories/ci/pipelines.rb
+++ b/spec/factories/ci/pipelines.rb
@@ -8,14 +8,14 @@ FactoryGirl.define do
 
     factory :ci_pipeline_without_jobs do
       after(:build) do |pipeline|
-        allow(pipeline).to receive(:ci_yaml_file) { YAML.dump({}) }
+        pipeline.instance_variable_set(:@ci_yaml_file, YAML.dump({}))
       end
     end
 
     factory :ci_pipeline_with_one_job do
       after(:build) do |pipeline|
         allow(pipeline).to receive(:ci_yaml_file) do
-          YAML.dump({ rspec: { script: "ls" } })
+          pipeline.instance_variable_set(:@ci_yaml_file, YAML.dump({ rspec: { script: "ls" } }))
         end
       end
     end
@@ -33,17 +33,14 @@ FactoryGirl.define do
       transient { config nil }
 
       after(:build) do |pipeline, evaluator|
-        allow(pipeline).to receive(:ci_yaml_file) do
-          if evaluator.config
-            YAML.dump(evaluator.config)
-          else
-            File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml'))
-          end
-        end
+        if evaluator.config
+          pipeline.instance_variable_set(:@ci_yaml_file, YAML.dump(evaluator.config))
 
-        # Populates pipeline with errors
-        #
-        pipeline.config_processor if evaluator.config
+          # Populates pipeline with errors
+          pipeline.config_processor if evaluator.config
+        else
+          pipeline.instance_variable_set(:@ci_yaml_file, File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')))
+        end
       end
 
       trait :invalid do
diff --git a/spec/factories/ci/stages.rb b/spec/factories/ci/stages.rb
index 7f557b25ccb..d37eabb3e8c 100644
--- a/spec/factories/ci/stages.rb
+++ b/spec/factories/ci/stages.rb
@@ -1,5 +1,7 @@
 FactoryGirl.define do
   factory :ci_stage, class: Ci::Stage do
+    skip_create
+
     transient do
       name 'test'
       status nil
diff --git a/spec/factories/ci/trigger_requests.rb b/spec/factories/ci/trigger_requests.rb
index b8d8fab0e0b..10e0ab4fd3c 100644
--- a/spec/factories/ci/trigger_requests.rb
+++ b/spec/factories/ci/trigger_requests.rb
@@ -1,8 +1,8 @@
 FactoryGirl.define do
   factory :ci_trigger_request, class: Ci::TriggerRequest do
-    factory :ci_trigger_request_with_variables do
-      trigger factory: :ci_trigger
+    trigger factory: :ci_trigger
 
+    factory :ci_trigger_request_with_variables do
       variables do
         {
           TRIGGER_KEY_1: 'TRIGGER_VALUE_1',
diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb
index 89e260cf65b..36b9645438a 100644
--- a/spec/factories/commits.rb
+++ b/spec/factories/commits.rb
@@ -4,19 +4,14 @@ FactoryGirl.define do
   factory :commit do
     git_commit RepoHelpers.sample_commit
     project factory: :empty_project
+    author { build(:author) }
 
     initialize_with do
       new(git_commit, project)
     end
 
-    after(:build) do |commit|
-      allow(commit).to receive(:author).and_return build(:author)
-    end
-
     trait :without_author do
-      after(:build) do |commit|
-        allow(commit).to receive(:author).and_return nil
-      end
+      author nil
     end
   end
 end
diff --git a/spec/factories/file_uploader.rb b/spec/factories/file_uploaders.rb
index bc74aeecc3b..d397dd705a5 100644
--- a/spec/factories/file_uploader.rb
+++ b/spec/factories/file_uploaders.rb
@@ -1,5 +1,7 @@
 FactoryGirl.define do
   factory :file_uploader do
+    skip_create
+
     project factory: :empty_project
     secret nil
 
diff --git a/spec/factories/keys.rb b/spec/factories/keys.rb
index 4e140102492..a13b6e3596e 100644
--- a/spec/factories/keys.rb
+++ b/spec/factories/keys.rb
@@ -1,27 +1,18 @@
+require_relative '../support/helpers/key_generator_helper'
+
 FactoryGirl.define do
   factory :key do
     title
-    key do
-      'ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= dummy@gitlab.com'
-    end
+    key { Spec::Support::Helpers::KeyGeneratorHelper.new(1024).generate + ' dummy@gitlab.com' }
 
-    factory :deploy_key, class: 'DeployKey' do
-      key do
-        'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDFf6RYK3qu/RKF/3ndJmL5xgMLp3O96x8lTay+QGZ0+9FnnAXMdUqBq/ZU6d/gyMB4IaW3nHzM1w049++yAB6UPCzMB8Uo27K5/jyZCtj7Vm9PFNjF/8am1kp46c/SeYicQgQaSBdzIW3UDEa1Ef68qroOlvpi9PYZ/tA7M0YP0K5PXX+E36zaIRnJVMPT3f2k+GnrxtjafZrwFdpOP/Fol5BQLBgcsyiU+LM1SuaCrzd8c9vyaTA1CxrkxaZh+buAi0PmdDtaDrHd42gqZkXCKavyvgM5o2CkQ5LJHCgzpXy05qNFzmThBSkb+XtoxbyagBiGbVZtSVow6Xa7qewz'
-      end
-    end
+    factory :deploy_key, class: 'DeployKey'
 
     factory :personal_key do
       user
     end
 
     factory :another_key do
-      key do
-        'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDmTillFzNTrrGgwaCKaSj+QCz81E6jBc/s9av0+3b1Hwfxgkqjl4nAK/OD2NjgyrONDTDfR8cRN4eAAy6nY8GLkOyYBDyuc5nTMqs5z3yVuTwf3koGm/YQQCmo91psZ2BgDFTor8SVEE5Mm1D1k3JDMhDFxzzrOtRYFPci9lskTJaBjpqWZ4E9rDTD2q/QZntCqbC3wE9uSemRQB5f8kik7vD/AD8VQXuzKladrZKkzkONCPWsXDspUitjM8HkQdOf0PsYn1CMUC1xKYbCxkg5TkEosIwGv6CoEArUrdu/4+10LVslq494mAvEItywzrluCLCnwELfW+h/m8UHoVhZ'
-      end
-
-      factory :another_deploy_key, class: 'DeployKey' do
-      end
+      factory :another_deploy_key, class: 'DeployKey'
     end
 
     factory :write_access_key, class: 'DeployKey' do
diff --git a/spec/factories/project_statistics.rb b/spec/factories/project_statistics.rb
index 72d43096216..6c2ed7c6581 100644
--- a/spec/factories/project_statistics.rb
+++ b/spec/factories/project_statistics.rb
@@ -1,6 +1,10 @@
 FactoryGirl.define do
   factory :project_statistics do
-    project { create :project }
-    namespace { project.namespace }
+    project
+
+    initialize_with do
+      # statistics are automatically created when a project is created
+      project&.statistics || new
+    end
   end
 end
diff --git a/spec/factories/project_wikis.rb b/spec/factories/project_wikis.rb
index a3403fd76ae..ae222d5e69a 100644
--- a/spec/factories/project_wikis.rb
+++ b/spec/factories/project_wikis.rb
@@ -1,5 +1,7 @@
 FactoryGirl.define do
   factory :project_wiki do
+    skip_create
+
     project factory: :empty_project
     user factory: :user
     initialize_with { new(project, user) }
diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index e8a9b688319..19a85e5a38f 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -1,3 +1,5 @@
+require_relative '../support/test_env'
+
 FactoryGirl.define do
   # Project without repository
   #
diff --git a/spec/factories/wiki_directories.rb b/spec/factories/wiki_directories.rb
index 3f3c864ac2b..3b4cfc380b8 100644
--- a/spec/factories/wiki_directories.rb
+++ b/spec/factories/wiki_directories.rb
@@ -1,5 +1,7 @@
 FactoryGirl.define do
   factory :wiki_directory do
+    skip_create
+
     slug '/path_up_to/dir'
     initialize_with { new(slug) }
   end
diff --git a/spec/factories_spec.rb b/spec/factories_spec.rb
index 786e1456f5f..09b3c0b0994 100644
--- a/spec/factories_spec.rb
+++ b/spec/factories_spec.rb
@@ -3,14 +3,20 @@ require 'spec_helper'
 describe 'factories' do
   FactoryGirl.factories.each do |factory|
     describe "#{factory.name} factory" do
-      let(:entity) { build(factory.name) }
+      it 'does not raise error when built' do
+        expect { build(factory.name) }.not_to raise_error
+      end
 
       it 'does not raise error when created' do
-        expect { entity }.not_to raise_error
+        expect { create(factory.name) }.not_to raise_error
       end
 
-      it 'is valid', if: factory.build_class < ActiveRecord::Base do
-        expect(entity).to be_valid
+      factory.definition.defined_traits.map(&:name).each do |trait_name|
+        describe "linting #{trait_name} trait" do
+          skip 'does not raise error when created' do
+            expect { create(factory.name, trait_name) }.not_to raise_error
+          end
+        end
       end
     end
   end
diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb
index 7c40cfd8253..f1e2a2cc518 100644
--- a/spec/models/key_spec.rb
+++ b/spec/models/key_spec.rb
@@ -66,14 +66,16 @@ describe Key, models: true do
     end
 
     it "does not accept the exact same key twice" do
-      create(:key, user: user)
-      expect(build(:key, user: user)).not_to be_valid
+      first_key = create(:key, user: user)
+
+      expect(build(:key, user: user, key: first_key.key)).not_to be_valid
     end
 
     it "does not accept a duplicate key with a different comment" do
-      create(:key, user: user)
-      duplicate = build(:key, user: user)
+      first_key = create(:key, user: user)
+      duplicate = build(:key, user: user, key: first_key.key)
       duplicate.key << ' extra comment'
+
       expect(duplicate).not_to be_valid
     end
   end
diff --git a/spec/support/helpers/key_generator_helper.rb b/spec/support/helpers/key_generator_helper.rb
new file mode 100644
index 00000000000..b1c289ffef7
--- /dev/null
+++ b/spec/support/helpers/key_generator_helper.rb
@@ -0,0 +1,41 @@
+module Spec
+  module Support
+    module Helpers
+      class KeyGeneratorHelper
+        # The components in a openssh .pub / known_host RSA public key.
+        RSA_COMPONENTS = ['ssh-rsa', :e, :n].freeze
+
+        attr_reader :size
+
+        def initialize(size = 2048)
+          @size = size
+        end
+
+        def generate
+          key = OpenSSL::PKey::RSA.generate(size)
+          components = RSA_COMPONENTS.map do |component|
+            key.respond_to?(component) ? encode_mpi(key.public_send(component)) : component
+          end
+
+          # Ruby tries to be helpful and adds new lines every 60 bytes :(
+          'ssh-rsa ' + [pack_pubkey_components(components)].pack('m').delete("\n")
+        end
+
+        private
+
+        # Encodes an openssh-mpi-encoded integer.
+        def encode_mpi(n)
+          chars, n = [], n.to_i
+          chars << (n & 0xff) && n >>= 8 while n != 0
+          chars << 0 if chars.empty? || chars.last >= 0x80
+          chars.reverse.pack('C*')
+        end
+
+        # Packs string components into an openssh-encoded pubkey.
+        def pack_pubkey_components(strings)
+          (strings.map { |s| [s.length].pack('N') }).zip(strings).flatten.join
+        end
+      end
+    end
+  end
+end