Merge branch 'speed-up-group-policy-spec' into 'master'

Speed up group policy spec

See merge request gitlab-org/gitlab!17872

doc/development/testing_guide/best_practices.md

@@ -202,15 +202,30 @@ so we need to set some guidelines for their use going forward:
   order is required, otherwise `let` will suffice. Remember that `let` is lazy and won't
   be evaluated until it is referenced.
 
-### `let_it_be` variables
-
-In some cases there is no need to recreate the same object for tests
-again for each example. For example, a project is needed to test issues
-on the same project, one project will do for the entire file. This can
-be achieved by using
-[`let_it_be`](https://test-prof.evilmartians.io/#/let_it_be) variables
+### Common test setup
+
+In some cases, there is no need to recreate the same object for tests
+again for each example. For example, a project and a guest of that project
+is needed to test issues on the same project, one project and user will do for the entire file.
+This can be achieved by using
+[`let_it_be`](https://test-prof.evilmartians.io/#/let_it_be) variables and the
+[`before_all`](https://test-prof.evilmartians.io/#/before_all) hook
 from the [`test-prof` gem](https://rubygems.org/gems/test-prof).
 
+```
+let_it_be(:project) { create(:project) }
+let_it_be(:user) { create(:user) }
+
+before_all do
+  project.add_guest(user)
+end
+```
+
+This will result in only one `Project`, `User`, and `ProjectMember` created for this context.
+
+`let_it_be` and `before_all` are also available within nested contexts. Cleanup after the context
+is handled automatically using a transaction rollback.
+
 Note that if you modify an object defined inside a `let_it_be` block,
 then you will need to reload the object as needed, or specify the `reload`
 option to reload for every example.

ee/spec/policies/group_policy_spec.rb

@@ -80,8 +80,7 @@ describe GroupPolicy do
 
     context 'with sso enforcement enabled' do
       let(:current_user) { guest }
-      let(:group) { create(:group, :private) }
-      let!(:saml_provider) { create(:saml_provider, group: group, enforced_sso: true) }
+      let_it_be(:saml_provider) { create(:saml_provider, group: group, enforced_sso: true) }
 
       context 'when the session has been set globally' do
         around do |example|
@@ -90,10 +89,6 @@ describe GroupPolicy do
           end
         end
 
-        before do
-          group.root_ancestor.reload
-        end
-
         it 'prevents access without a SAML session' do
           is_expected.not_to be_allowed(:read_group)
         end
@@ -115,7 +110,6 @@ describe GroupPolicy do
 
   context 'with ip restriction' do
     let(:current_user) { developer }
-    let(:group) { create(:group, :public) }
 
     before do
       allow(Gitlab::IpAddressState).to receive(:current).and_return('192.168.0.2')

spec/policies/group_policy_spec.rb

@@ -3,7 +3,7 @@ require 'spec_helper'
 describe GroupPolicy do
   include_context 'GroupPolicy context'
 
-  context 'with no user' do
+  context 'public group with no user' do
     let(:group) { create(:group, :public) }
     let(:current_user) { nil }
 
@@ -33,7 +33,6 @@ describe GroupPolicy do
 
   context 'with foreign user and public project' do
     let(:project) { create(:project, :public) }
-    let(:user) { create(:user) }
     let(:current_user) { create(:user) }
 
     before do
@@ -105,8 +104,8 @@ describe GroupPolicy do
     let(:current_user) { maintainer }
 
     context 'with subgroup_creation level set to maintainer' do
-      let(:group) do
-        create(:group, :private, subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS)
+      before_all do
+        group.update(subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS)
       end
 
       it 'allows every maintainer permission plus creating subgroups' do
@@ -164,11 +163,11 @@ describe GroupPolicy do
   end
 
   describe 'private nested group use the highest access level from the group and inherited permissions' do
-    let(:nested_group) do
+    let_it_be(:nested_group) do
       create(:group, :private, :owner_subgroup_creation_only, parent: group)
     end
 
-    before do
+    before_all do
       nested_group.add_guest(guest)
       nested_group.add_guest(reporter)
       nested_group.add_guest(developer)
@@ -268,6 +267,10 @@ describe GroupPolicy do
       context 'when the group share_with_group_lock is enabled' do
         let(:group) { create(:group, share_with_group_lock: true, parent: parent) }
 
+        before do
+          group.add_owner(owner)
+        end
+
         context 'when the parent group share_with_group_lock is enabled' do
           context 'when the group has a grandparent' do
             let(:parent) { create(:group, share_with_group_lock: true, parent: grandparent) }
@@ -353,7 +356,9 @@ describe GroupPolicy do
 
   context "create_projects" do
     context 'when group has no project creation level set' do
-      let(:group) { create(:group, project_creation_level: nil) }
+      before_all do
+        group.update(project_creation_level: nil)
+      end
 
       context 'reporter' do
         let(:current_user) { reporter }
@@ -381,7 +386,9 @@ describe GroupPolicy do
     end
 
     context 'when group has project creation level set to no one' do
-      let(:group) { create(:group, project_creation_level: ::Gitlab::Access::NO_ONE_PROJECT_ACCESS) }
+      before_all do
+        group.update(project_creation_level: ::Gitlab::Access::NO_ONE_PROJECT_ACCESS)
+      end
 
       context 'reporter' do
         let(:current_user) { reporter }
@@ -409,7 +416,9 @@ describe GroupPolicy do
     end
 
     context 'when group has project creation level set to maintainer only' do
-      let(:group) { create(:group, project_creation_level: ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS) }
+      before_all do
+        group.update(project_creation_level: ::Gitlab::Access::MAINTAINER_PROJECT_ACCESS)
+      end
 
       context 'reporter' do
         let(:current_user) { reporter }
@@ -437,7 +446,9 @@ describe GroupPolicy do
     end
 
     context 'when group has project creation level set to developers + maintainer' do
-      let(:group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) }
+      before_all do
+        group.update(project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS)
+      end
 
       context 'reporter' do
         let(:current_user) { reporter }
@@ -467,10 +478,8 @@ describe GroupPolicy do
 
   context "create_subgroup" do
     context 'when group has subgroup creation level set to owner' do
-      let(:group) do
-        create(
-          :group,
-          subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS)
+      before_all do
+        group.update(subgroup_creation_level: ::Gitlab::Access::OWNER_SUBGROUP_ACCESS)
       end
 
       context 'reporter' do
@@ -499,10 +508,8 @@ describe GroupPolicy do
     end
 
     context 'when group has subgroup creation level set to maintainer' do
-      let(:group) do
-        create(
-          :group,
-          subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS)
+      before_all do
+        group.update(subgroup_creation_level: ::Gitlab::Access::MAINTAINER_SUBGROUP_ACCESS)
       end
 
       context 'reporter' do

spec/spec_helper.rb

@@ -10,7 +10,7 @@ require 'rspec/rails'
 require 'shoulda/matchers'
 require 'rspec/retry'
 require 'rspec-parameterized'
-require "test_prof/recipes/rspec/let_it_be"
+require 'test_prof/recipes/rspec/let_it_be'
 
 rspec_profiling_is_configured =
   ENV['RSPEC_PROFILING_POSTGRES_URL'].present? ||

spec/support/shared_contexts/policies/group_policy_shared_context.rb

 # frozen_string_literal: true
 
 RSpec.shared_context 'GroupPolicy context' do
-  let(:guest) { create(:user) }
-  let(:reporter) { create(:user) }
-  let(:developer) { create(:user) }
-  let(:maintainer) { create(:user) }
-  let(:owner) { create(:user) }
-  let(:admin) { create(:admin) }
-  let(:group) { create(:group, :private, :owner_subgroup_creation_only) }
+  let_it_be(:guest) { create(:user) }
+  let_it_be(:reporter) { create(:user) }
+  let_it_be(:developer) { create(:user) }
+  let_it_be(:maintainer) { create(:user) }
+  let_it_be(:owner) { create(:user) }
+  let_it_be(:admin) { create(:admin) }
+  let_it_be(:group, refind: true) { create(:group, :private, :owner_subgroup_creation_only) }
 
   let(:guest_permissions) do
     %i[
@@ -37,7 +37,7 @@ RSpec.shared_context 'GroupPolicy context' do
     ].compact
   end
 
-  before do
+  before_all do
     group.add_guest(guest)
     group.add_reporter(reporter)
     group.add_developer(developer)