Merge branch 'freeze_create_default_objects' into 'master'

Freeze objects when when calling create_default

See merge request gitlab-org/gitlab!59709

doc/development/testing_guide/best_practices.md

@@ -176,9 +176,9 @@ 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.
+To prevent accidental reliance between test examples, objects created
+with `create_default` are
+[frozen](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/support/factory_default.rb).
 
 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

spec/factories/namespaces.rb

@@ -5,22 +5,7 @@ FactoryBot.define do
     sequence(:name) { |n| "namespace#{n}" }
     path { name.downcase.gsub(/\s/, '_') }
 
-    # This is a workaround to avoid the user creating another namespace via
-    # User#ensure_namespace_correct. We should try to remove it and then
-    # we could remove this workaround
-    association :owner, factory: :user, strategy: :build
-    before(:create) do |namespace|
-      owner = namespace.owner
-
-      if owner
-        # We're changing the username here because we want to keep our path,
-        # and User#ensure_namespace_correct would change the path based on
-        # username, so we're forced to do this otherwise we'll need to change
-        # a lot of existing tests.
-        owner.username = namespace.path
-        owner.namespace = namespace
-      end
-    end
+    owner { association(:user, strategy: :build, namespace: instance, username: path) }
 
     trait :with_aggregation_schedule do
       after(:create) do |namespace|

spec/models/namespace_spec.rb

@@ -141,7 +141,7 @@ RSpec.describe Namespace do
       end
 
       it 'allows updating other attributes for existing record' do
-        namespace = build(:namespace, path: 'j')
+        namespace = build(:namespace, path: 'j', owner: create(:user))
         namespace.save(validate: false)
         namespace.reload
 

spec/models/project_services/slack_service_spec.rb

@@ -65,10 +65,16 @@ RSpec.describe SlackService do
       end
 
       context 'wiki_page notification' do
-        let_it_be(:wiki_page) { create(:wiki_page, wiki: project.wiki, message: 'user created page: Awesome wiki_page') }
+        let(:wiki_page) { create(:wiki_page, wiki: project.wiki, message: 'user created page: Awesome wiki_page') }
 
         let(:data) { Gitlab::DataBuilder::WikiPage.build(wiki_page, user, 'create') }
 
+        before do
+          # Skip this method that is not relevant to this test to prevent having
+          # to update project which is frozen
+          allow(project.wiki).to receive(:after_wiki_activity)
+        end
+
         it_behaves_like 'increases the usage data counter', 'i_ecosystem_slack_service_wiki_page_notification'
       end
 

spec/support/factory_default.rb

 # frozen_string_literal: true
 
+module Gitlab
+  module FreezeFactoryDefault
+    def set_factory_default(name, obj, preserve_traits: nil)
+      obj.freeze unless obj.frozen?
+
+      super
+    end
+  end
+end
+
+TestProf::FactoryDefault::DefaultSyntax.prepend Gitlab::FreezeFactoryDefault
+
 RSpec.configure do |config|
   config.after do |ex|
     TestProf::FactoryDefault.reset unless ex.metadata[:factory_default] == :keep