Merge branch 'backstage/gb/optimize-some-specs-with-rspec-set' into 'master'

Optimize some tests with RSpec Set

See merge request !14047

8 files changed, 59 insertions, 41 deletions

diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb
index d4da30b1641..f49a61062c1 100644
--- a/spec/models/abuse_report_spec.rb
+++ b/spec/models/abuse_report_spec.rb
@@ -1,8 +1,9 @@
 require 'rails_helper'
 
-RSpec.describe AbuseReport do
-  subject     { create(:abuse_report) }
-  let(:user)  { create(:admin) }
+describe AbuseReport do
+  set(:report) { create(:abuse_report) }
+  set(:user) { create(:admin) }
+  subject { report }
 
   it { expect(subject).to be_valid }
 
diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb
index b5d5d58697b..49f44525b29 100644
--- a/spec/models/appearance_spec.rb
+++ b/spec/models/appearance_spec.rb
@@ -1,6 +1,6 @@
 require 'rails_helper'
 
-RSpec.describe Appearance do
+describe Appearance do
   subject { build(:appearance) }
 
   it { is_expected.to be_valid }
diff --git a/spec/models/chat_name_spec.rb b/spec/models/chat_name_spec.rb
index 8581bcbb08b..e89e534d914 100644
--- a/spec/models/chat_name_spec.rb
+++ b/spec/models/chat_name_spec.rb
@@ -1,7 +1,8 @@
 require 'spec_helper'
 
 describe ChatName do
-  subject { create(:chat_name) }
+  set(:chat_name) { create(:chat_name) }
+  subject { chat_name }
 
   it { is_expected.to belong_to(:service) }
   it { is_expected.to belong_to(:user) }
diff --git a/spec/models/chat_team_spec.rb b/spec/models/chat_team_spec.rb
index e0e5f73e6fe..70a9a206faa 100644
--- a/spec/models/chat_team_spec.rb
+++ b/spec/models/chat_team_spec.rb
@@ -1,7 +1,8 @@
 require 'spec_helper'
 
 describe ChatTeam do
-  subject { create(:chat_team) }
+  set(:chat_team) { create(:chat_team) }
+  subject { chat_team }
 
   # Associations
   it { is_expected.to belong_to(:namespace) }
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index c2c9f1c12d1..451968c7342 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1,18 +1,19 @@
 require 'spec_helper'
 
 describe Ci::Build do
-  let(:user) { create(:user) }
-  let(:project) { create(:project, :repository) }
-  let(:build) { create(:ci_build, pipeline: pipeline) }
-  let(:test_trace) { 'This is a test' }
+  set(:user) { create(:user) }
+  set(:group) { create(:group, :access_requestable) }
+  set(:project) { create(:project, :repository, group: group) }
 
-  let(:pipeline) do
+  set(:pipeline) do
     create(:ci_pipeline, project: project,
                          sha: project.commit.id,
                          ref: project.default_branch,
                          status: 'success')
   end
 
+  let(:build) { create(:ci_build, pipeline: pipeline) }
+
   it { is_expected.to belong_to(:runner) }
   it { is_expected.to belong_to(:trigger_request) }
   it { is_expected.to belong_to(:erased_by) }
@@ -282,7 +283,7 @@ describe Ci::Build do
       let(:project_regex) { '\(\d+\.\d+\) covered' }
 
       before do
-        project.build_coverage_regex = project_regex
+        project.update_column(:build_coverage_regex, project_regex)
       end
 
       context 'and coverage_regex attribute is not set' do
@@ -1096,9 +1097,6 @@ describe Ci::Build do
   end
 
   describe '#repo_url' do
-    let(:build) { create(:ci_build) }
-    let(:project) { build.project }
-
     subject { build.repo_url }
 
     it { is_expected.to be_a(String) }
@@ -1199,6 +1197,8 @@ describe Ci::Build do
       end
 
       context 'use from gitlab-ci.yml' do
+        let(:pipeline) { create(:ci_pipeline) }
+
         before do
           stub_ci_pipeline_yaml_file(config)
         end
@@ -1442,11 +1442,7 @@ describe Ci::Build do
         { key: 'SECRET_KEY', value: 'secret_value', public: false }
       end
 
-      let(:group) { create(:group, :access_requestable) }
-
       before do
-        build.project.update(group: group)
-
         create(:ci_group_variable,
                secret_variable.slice(:key, :value).merge(group: group))
       end
@@ -1459,11 +1455,7 @@ describe Ci::Build do
         { key: 'PROTECTED_KEY', value: 'protected_value', public: false }
       end
 
-      let(:group) { create(:group, :access_requestable) }
-
       before do
-        build.project.update(group: group)
-
         create(:ci_group_variable,
                :protected,
                protected_variable.slice(:key, :value).merge(group: group))
@@ -1486,6 +1478,10 @@ describe Ci::Build do
       end
 
       context 'when the ref is not protected' do
+        before do
+          build.update_column(:ref, 'some/feature')
+        end
+
         it { is_expected.not_to include(protected_variable) }
       end
     end
@@ -1552,6 +1548,8 @@ describe Ci::Build do
     end
 
     context 'when yaml_variables are undefined' do
+      let(:pipeline) { create(:ci_pipeline, project: project) }
+
       before do
         build.yaml_variables = nil
       end
@@ -1645,7 +1643,10 @@ describe Ci::Build do
 
       before do
         build.environment = 'production'
-        allow(project).to receive(:deployment_variables).and_return([deployment_variable])
+
+        allow_any_instance_of(Project)
+          .to receive(:deployment_variables)
+          .and_return([deployment_variable])
       end
 
       it { is_expected.to include(deployment_variable) }
@@ -1669,14 +1670,19 @@ describe Ci::Build do
 
       before do
         allow(build).to receive(:predefined_variables) { [build_pre_var] }
-        allow(project).to receive(:predefined_variables) { [project_pre_var] }
-        allow(pipeline).to receive(:predefined_variables) { [pipeline_pre_var] }
         allow(build).to receive(:yaml_variables) { [build_yaml_var] }
 
-        allow(project).to receive(:secret_variables_for)
+        allow_any_instance_of(Project)
+          .to receive(:predefined_variables) { [project_pre_var] }
+
+        allow_any_instance_of(Project)
+          .to receive(:secret_variables_for)
           .with(ref: 'master', environment: nil) do
             [create(:ci_variable, key: 'secret', value: 'value')]
           end
+
+        allow_any_instance_of(Ci::Pipeline)
+          .to receive(:predefined_variables) { [pipeline_pre_var] }
       end
 
       it do
diff --git a/spec/models/lfs_objects_project_spec.rb b/spec/models/lfs_objects_project_spec.rb
index d24d4cf7695..0a3180f43e8 100644
--- a/spec/models/lfs_objects_project_spec.rb
+++ b/spec/models/lfs_objects_project_spec.rb
@@ -1,8 +1,11 @@
 require 'spec_helper'
 
 describe LfsObjectsProject do
-  subject { create(:lfs_objects_project, project: project) }
-  let(:project) { create(:project) }
+  set(:project) { create(:project) }
+
+  subject do
+    create(:lfs_objects_project, project: project)
+  end
 
   describe 'associations' do
     it { is_expected.to belong_to(:project) }
@@ -11,9 +14,13 @@ describe LfsObjectsProject do
 
   describe 'validation' do
     it { is_expected.to validate_presence_of(:lfs_object_id) }
-    it { is_expected.to validate_uniqueness_of(:lfs_object_id).scoped_to(:project_id).with_message("already exists in project") }
-
     it { is_expected.to validate_presence_of(:project_id) }
+
+    it 'validates object id' do
+      is_expected.to validate_uniqueness_of(:lfs_object_id)
+        .scoped_to(:project_id)
+        .with_message("already exists in project")
+    end
   end
 
   describe '#update_project_statistics' do
diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb
index 48d99841385..7e174903918 100644
--- a/spec/requests/api/services_spec.rb
+++ b/spec/requests/api/services_spec.rb
@@ -1,10 +1,13 @@
 require "spec_helper"
 
 describe API::Services do
-  let(:user) { create(:user) }
-  let(:admin) { create(:admin) }
-  let(:user2) { create(:user) }
-  let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) }
+  set(:user) { create(:user) }
+  set(:admin) { create(:admin) }
+  set(:user2) { create(:user) }
+
+  set(:project) do
+    create(:project, creator_id: user.id, namespace: user.namespace)
+  end
 
   Service.available_services_names.each do |service|
     describe "PUT /projects/:id/services/#{service.dasherize}" do
@@ -98,8 +101,6 @@ describe API::Services do
   end
 
   describe 'POST /projects/:id/services/:slug/trigger' do
-    let!(:project) { create(:project) }
-
     describe 'Mattermost Service' do
       let(:service_name) { 'mattermost_slash_commands' }
 
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index bbc3a8c79f5..fbb3213f42b 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -1,9 +1,10 @@
 require 'spec_helper'
 
 describe Ci::RetryBuildService do
-  let(:user) { create(:user) }
-  let(:project) { create(:project) }
-  let(:pipeline) { create(:ci_pipeline, project: project) }
+  set(:user) { create(:user) }
+  set(:project) { create(:project) }
+  set(:pipeline) { create(:ci_pipeline, project: project) }
+
   let(:build) { create(:ci_build, pipeline: pipeline) }
 
   let(:service) do
@@ -37,7 +38,7 @@ describe Ci::RetryBuildService do
              :queued, :coverage, :tags, :allowed_to_fail, :on_tag,
              :triggered, :trace, :teardown_environment,
              description: 'my-job', stage: 'test',  pipeline: pipeline,
-             auto_canceled_by: create(:ci_empty_pipeline)) do |build|
+             auto_canceled_by: create(:ci_empty_pipeline, project: project)) do |build|
                ##
                # TODO, workaround for FactoryGirl limitation when having both
                # stage (text) and stage_id (integer) columns in the table.