Skip to content

Improve the Project factory to make `creator` defaults to `namespace.owner`

Rémy Coutable requested to merge rc/improve-projects-factory into master

EE MR: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2651

What does this MR do?

It improves the Project factory to make creator defaults to namespace.owner.

This also improves the create_templates transient attribute and use project.project_feature.update_columns instead of project.project_feature.update_attributes! since it's faster.

FWIW here are some stats I gathered locally:

### Before

- `bin/rspec spec/lib/gitlab/template`:
  - Duration: 9.57s
  - Records: `{:projects=>18, :users=>54, :members=>36}`
- `bin/rspec spec/models/project_spec.rb`:
  - Duration: 1m10s
  - Records: `{:projects=>525, :users=>1130, :members=>256}`

### After

- `bin/rspec spec/lib/gitlab/template`:
  - Duration: 7s
  - Records: `{:projects=>18, :users=>18, :members=>18}` (36 less users, 18 less members)
- `bin/rspec spec/models/project_spec.rb`:
  - Duration: 50.79s
  - Records: `{:projects=>525, :users=>877, :members=>256}` (253 less users)

For the record, here is the diff I've used to get the number of records created during a test suite run:

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 0ba6ed5631..5564804a57 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -70,11 +70,13 @@ RSpec.configure do |config|
   config.raise_errors_for_deprecations!
 
   config.before(:suite) do
+    $records_count = Hash.new { |h, k| h[k] = 0 }
     TestEnv.init
   end
 
   config.after(:suite) do
     TestEnv.cleanup
+    p $records_count
   end
 
   config.before(:example) do
diff --git a/spec/support/db_cleaner.rb b/spec/support/db_cleaner.rb
index 7f5769209b..bc5b641ebc 100644
--- a/spec/support/db_cleaner.rb
+++ b/spec/support/db_cleaner.rb
@@ -28,6 +28,9 @@ RSpec.configure do |config|
   end
 
   config.append_after(:each) do
+    $records_count[:projects] += Project.count
+    $records_count[:users] += User.count
+    $records_count[:members] += Member.count
     DatabaseCleaner.clean
   end
 end

Are there points in the code the reviewer needs to double check?

If the specs still pass, no.

Why was this MR needed?

Because factories are at the fundation of all our tests so if we can optimize them, it's a quick (and hopefully big) win.

/cc @rspeicher

Edited by Rémy Coutable

Merge request reports