diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 01db92b09c927843b983db579450df708a9d922b..b807ca906eb8aa341b8acf74ab47926106df537a 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/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. diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index d697840601f234932a67c91e726ab7d265c51de4..02bcc716beeffb9f9441060c27003b08ba722e13 100644 --- a/spec/policies/group_policy_spec.rb +++ b/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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d6c326764d1f22d3189a146e558177c47e135354..2142a43177a301ea6777f9288031af5f910fdcf4 100644 --- a/spec/spec_helper.rb +++ b/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? || diff --git a/spec/support/shared_contexts/policies/group_policy_shared_context.rb b/spec/support/shared_contexts/policies/group_policy_shared_context.rb index 2f3a1b911e4bd019df2a5ba17c658bc81dce6d62..c503197a7734e89eb97feaae44b65cd6eaf49621 100644 --- a/spec/support/shared_contexts/policies/group_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/group_policy_shared_context.rb @@ -1,13 +1,13 @@ # 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)