Merge branch 'let_it_be_freeze' into 'master'

Freeze let_it_be objects in `factory_default: :keep` specs

See merge request gitlab-org/gitlab!54176

doc/development/testing_guide/best_practices.md

@@ -168,7 +168,7 @@ can be used:
 
 ```ruby
 RSpec.describe API::Search, factory_default: :keep do
-  let_it_be(:namespace) { create_default(:namespace) }
+  let_it_be(:namespace) { create_default(:namespace).freeze }
 ```
 
 Then every project we create uses this `namespace`, without us having to pass
@@ -176,13 +176,17 @@ it as `namespace: namespace`. In order to make it work along with `let_it_be`, `
 must be explicitly specified. That keeps the default factory for every example in a suite instead of
 recreating it for each example.
 
+Objects created inside a `factory_default: :keep`, and using
+`create_default` inside a `let_it_be` should be frozen to prevent accidental reliance
+between test examples.
+
 Maybe we don't need to create 208 different projects - we
 can create one and reuse it. In addition, we can see that only about 1/3 of the
 projects we create are ones we ask for (76/208). There is benefit in setting
 a default value for projects as well:
 
 ```ruby
-  let_it_be(:project) { create_default(:project) }
+  let_it_be(:project) { create_default(:project).freeze }
 ```
 
 In this case, the `total time` and `top-level time` numbers match more closely:

ee/spec/requests/api/search_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe API::Search, factory_default: :keep do
   let_it_be(:user) { create(:user) }
   let_it_be(:group) { create(:group) }
-  let_it_be(:namespace) { create_default(:namespace) }
+  let_it_be(:namespace) { create_default(:namespace).freeze }
 
   let(:project) { create(:project, :public, :repository, :wiki_repo, name: 'awesome project', group: group) }
 

spec/factories_spec.rb

@@ -29,9 +29,16 @@ RSpec.describe 'factories' do
   # and reuse them in other factories.
   #
   # However, for some factories we cannot use FactoryDefault because the
-  # associations must be unique and cannot be reused.
+  # associations must be unique and cannot be reused, or the factory default
+  # is being mutated.
   skip_factory_defaults = %i[
     fork_network_member
+    group_member
+    import_state
+    namespace
+    project_broken_repo
+    users_star_project
+    wiki_page
   ].to_set.freeze
 
   # Some factories and their corresponding models are based on
@@ -46,9 +53,9 @@ RSpec.describe 'factories' do
     .partition { |factory| skip_factory_defaults.include?(factory.name) }
 
   context 'with factory defaults', factory_default: :keep do
-    let_it_be(:namespace) { create_default(:namespace) }
-    let_it_be(:project) { create_default(:project, :repository) }
-    let_it_be(:user) { create_default(:user) }
+    let_it_be(:namespace) { create_default(:namespace).freeze }
+    let_it_be(:project) { create_default(:project, :repository).freeze }
+    let_it_be(:user) { create_default(:user).freeze }
 
     before do
       factories_based_on_view.each do |factory|

spec/lib/gitlab/ci/trace_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::Trace, :clean_gitlab_redis_shared_state, factory_default: :keep do
-  let_it_be(:project) { create_default(:project) }
+  let_it_be(:project) { create_default(:project).freeze }
   let_it_be_with_reload(:build) { create(:ci_build) }
   let(:trace) { described_class.new(build) }
 

spec/models/ci/pipeline_spec.rb

@@ -8,8 +8,8 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
   include Ci::SourcePipelineHelpers
 
   let_it_be(:user) { create(:user) }
-  let_it_be(:namespace) { create_default(:namespace) }
-  let_it_be(:project) { create_default(:project, :repository) }
+  let_it_be(:namespace) { create_default(:namespace).freeze }
+  let_it_be(:project) { create_default(:project, :repository).freeze }
 
   let(:pipeline) do
     create(:ci_empty_pipeline, status: :created, project: project)
@@ -3785,17 +3785,11 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do
   end
 
   describe '#default_branch?' do
-    let(:default_branch) { 'master'}
-
     subject { pipeline.default_branch? }
 
-    before do
-      allow(project).to receive(:default_branch).and_return(default_branch)
-    end
-
     context 'when pipeline ref is the default branch of the project' do
       let(:pipeline) do
-        build(:ci_empty_pipeline, status: :created, project: project, ref: default_branch)
+        build(:ci_empty_pipeline, status: :created, project: project, ref: project.default_branch)
       end
 
       it "returns true" do

spec/models/merge_request_spec.rb

@@ -9,8 +9,8 @@ RSpec.describe MergeRequest, factory_default: :keep do
 
   using RSpec::Parameterized::TableSyntax
 
-  let_it_be(:namespace) { create_default(:namespace) }
-  let_it_be(:project, refind: true) { create_default(:project, :repository) }
+  let_it_be(:namespace) { create_default(:namespace).freeze }
+  let_it_be(:project, refind: true) { create_default(:project, :repository).freeze }
 
   subject { create(:merge_request) }
 

spec/models/project_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Project, factory_default: :keep do
   include ExternalAuthorizationServiceHelpers
   using RSpec::Parameterized::TableSyntax
 
-  let_it_be(:namespace) { create_default(:namespace) }
+  let_it_be(:namespace) { create_default(:namespace).freeze }
 
   it_behaves_like 'having unique enum values'
 
@@ -1799,7 +1799,8 @@ RSpec.describe Project, factory_default: :keep do
   describe '#default_branch_protected?' do
     using RSpec::Parameterized::TableSyntax
 
-    let_it_be(:project) { create(:project) }
+    let_it_be(:namespace) { create(:namespace) }
+    let_it_be(:project) { create(:project, namespace: namespace) }
 
     subject { project.default_branch_protected? }
 
@@ -2802,7 +2803,8 @@ RSpec.describe Project, factory_default: :keep do
   end
 
   describe '#emails_disabled?' do
-    let(:project) { build(:project, emails_disabled: false) }
+    let_it_be(:namespace) { create(:namespace) }
+    let(:project) { build(:project, namespace: namespace, emails_disabled: false) }
 
     context 'emails disabled in group' do
       it 'returns true' do
@@ -2830,7 +2832,8 @@ RSpec.describe Project, factory_default: :keep do
   end
 
   describe '#lfs_enabled?' do
-    let(:project) { build(:project) }
+    let(:namespace) { create(:namespace) }
+    let(:project) { build(:project, namespace: namespace) }
 
     shared_examples 'project overrides group' do
       it 'returns true when enabled in project' do
@@ -4877,7 +4880,8 @@ RSpec.describe Project, factory_default: :keep do
     end
 
     context 'branch protection' do
-      let(:project) { create(:project, :repository) }
+      let_it_be(:namespace) { create(:namespace) }
+      let(:project) { create(:project, :repository, namespace: namespace) }
 
       before do
         create(:import_state, :started, project: project)

spec/workers/jira_connect/sync_project_worker_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe JiraConnect::SyncProjectWorker, factory_default: :keep do
   describe '#perform' do
-    let_it_be(:project) { create_default(:project) }
+    let_it_be(:project) { create_default(:project).freeze }
     let!(:mr_with_jira_title) { create(:merge_request, :unique_branches, title: 'TEST-123') }
     let!(:mr_with_jira_description) { create(:merge_request, :unique_branches, description: 'TEST-323') }
     let!(:mr_with_other_title) { create(:merge_request, :unique_branches) }