Merge branch 'ab-use-factory-prof' into 'master'

Use factory prof to find un-needed db access in tests

See merge request gitlab-org/gitlab!26532

doc/development/testing_guide/best_practices.md

@@ -52,6 +52,13 @@ bundle exec guard
 
 When using spring and guard together, use `SPRING=1 bundle exec guard` instead to make use of spring.
 
+Use [Factory Doctor](https://test-prof.evilmartians.io/#/factory_doctor.md) to find cases on un-necessary database manipulation, which can cause slow tests.
+
+```shell
+# run test for path
+FDOC=1 bin/rspec spec/[path]/[to]/[spec].rb
+```
+
 ### General guidelines
 
 - Use a single, top-level `describe ClassName` block.

spec/models/analytics/cycle_analytics/project_stage_spec.rb

@@ -8,7 +8,7 @@ describe Analytics::CycleAnalytics::ProjectStage do
   end
 
   it 'default stages must be valid' do
-    project = create(:project)
+    project = build(:project)
 
     Gitlab::Analytics::CycleAnalytics::DefaultStages.all.each do |params|
       stage = described_class.new(params.merge(project: project))
@@ -17,13 +17,13 @@ describe Analytics::CycleAnalytics::ProjectStage do
   end
 
   it_behaves_like 'cycle analytics stage' do
-    let(:parent) { create(:project) }
+    let(:parent) { build(:project) }
     let(:parent_name) { :project }
   end
 
   context 'relative positioning' do
     it_behaves_like 'a class that supports relative positioning' do
-      let(:project) { create(:project) }
+      let(:project) { build(:project) }
       let(:factory) { :cycle_analytics_project_stage }
       let(:default_params) { { project: project } }
     end

spec/models/clusters/cluster_spec.rb

@@ -573,17 +573,17 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
   end
 
   describe '#allow_user_defined_namespace?' do
-    let(:cluster) { create(:cluster, :provided_by_gcp) }
-
     subject { cluster.allow_user_defined_namespace? }
 
     context 'project type cluster' do
       context 'gitlab managed' do
+        let(:cluster) { build(:cluster, :provided_by_gcp) }
+
         it { is_expected.to be_truthy }
       end
 
       context 'not managed' do
-        let(:cluster) { create(:cluster, :provided_by_gcp, managed: false) }
+        let(:cluster) { build(:cluster, :provided_by_gcp, managed: false) }
 
         it { is_expected.to be_truthy }
       end
@@ -591,13 +591,13 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
 
     context 'group type cluster' do
       context 'gitlab managed' do
-        let(:cluster) { create(:cluster, :provided_by_gcp, :group) }
+        let(:cluster) { build(:cluster, :provided_by_gcp, :group) }
 
         it { is_expected.to be_falsey }
       end
 
       context 'not managed' do
-        let(:cluster) { create(:cluster, :provided_by_gcp, :group, managed: false) }
+        let(:cluster) { build(:cluster, :provided_by_gcp, :group, managed: false) }
 
         it { is_expected.to be_truthy }
       end
@@ -605,13 +605,13 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
 
     context 'instance type cluster' do
       context 'gitlab managed' do
-        let(:cluster) { create(:cluster, :provided_by_gcp, :instance) }
+        let(:cluster) { build(:cluster, :provided_by_gcp, :instance) }
 
         it { is_expected.to be_falsey }
       end
 
       context 'not managed' do
-        let(:cluster) { create(:cluster, :provided_by_gcp, :instance, managed: false) }
+        let(:cluster) { build(:cluster, :provided_by_gcp, :instance, managed: false) }
 
         it { is_expected.to be_truthy }
       end
@@ -649,18 +649,18 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
   end
 
   describe '#kube_ingress_domain' do
-    let(:cluster) { create(:cluster, :provided_by_gcp) }
+    let(:cluster) { build(:cluster, :provided_by_gcp) }
 
     subject { cluster.kube_ingress_domain }
 
     context 'with domain set in cluster' do
-      let(:cluster) { create(:cluster, :provided_by_gcp, :with_domain) }
+      let(:cluster) { build(:cluster, :provided_by_gcp, :with_domain) }
 
       it { is_expected.to eq(cluster.domain) }
     end
 
     context 'with no domain on cluster' do
-      let(:cluster) { create(:cluster, :project, :provided_by_gcp) }
+      let(:cluster) { build(:cluster, :project, :provided_by_gcp) }
       let(:project) { cluster.project }
 
       context 'with domain set at instance level' do
@@ -754,7 +754,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     end
 
     context 'with no domain' do
-      let(:cluster) { create(:cluster, :provided_by_gcp, :project) }
+      let(:cluster) { build(:cluster, :provided_by_gcp, :project) }
 
       it 'returns an empty array' do
         expect(subject.to_hash).to be_empty
@@ -782,7 +782,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     subject { cluster.status_name }
 
     context 'the cluster has a provider' do
-      let(:cluster) { create(:cluster, :provided_by_gcp) }
+      let(:cluster) { build(:cluster, :provided_by_gcp) }
       let(:provider_status) { :errored }
 
       before do
@@ -816,7 +816,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     end
 
     context 'there is a cached connection status' do
-      let(:cluster) { create(:cluster, :provided_by_user) }
+      let(:cluster) { build(:cluster, :provided_by_user) }
 
       before do
         allow(cluster).to receive(:connection_status).and_return(:connected)
@@ -826,7 +826,7 @@ describe Clusters::Cluster, :use_clean_rails_memory_store_caching do
     end
 
     context 'there is no connection status in the cache' do
-      let(:cluster) { create(:cluster, :provided_by_user) }
+      let(:cluster) { build(:cluster, :provided_by_user) }
 
       before do
         allow(cluster).to receive(:connection_status).and_return(nil)

spec/models/error_tracking/project_error_tracking_setting_spec.rb

@@ -8,7 +8,7 @@ describe ErrorTracking::ProjectErrorTrackingSetting do
 
   let_it_be(:project) { create(:project) }
 
-  subject(:setting) { create(:project_error_tracking_setting, project: project) }
+  subject(:setting) { build(:project_error_tracking_setting, project: project) }
 
   describe 'Associations' do
     it { is_expected.to belong_to(:project) }

spec/models/merge_request_spec.rb

@@ -276,6 +276,8 @@ describe MergeRequest do
   end
 
   describe 'respond to' do
+    subject { build(:merge_request) }
+
     it { is_expected.to respond_to(:unchecked?) }
     it { is_expected.to respond_to(:checking?) }
     it { is_expected.to respond_to(:can_be_merged?) }