Merge branch 'zj-rspec-set' into 'master'

Use rspec-set to speed up examples

See merge request !10551

6 files changed, 58 insertions, 34 deletions

diff --git a/Gemfile b/Gemfile
index 58af6c51b77..decbcfe6375 100644
--- a/Gemfile
+++ b/Gemfile
@@ -293,6 +293,7 @@ group :development, :test do
   gem 'spinach-rails', '~> 0.2.1'
   gem 'spinach-rerun-reporter', '~> 0.0.2'
   gem 'rspec_profiling', '~> 0.0.5'
+  gem 'rspec-set', '~> 0.1.3'
 
   # Prevent occasions where minitest is not bundled in packaged versions of ruby (see #3826)
   gem 'minitest', '~> 5.7.0'
diff --git a/Gemfile.lock b/Gemfile.lock
index 83c9bb57c3b..fb11f590110 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -659,6 +659,7 @@ GEM
       rspec-support (~> 3.5.0)
     rspec-retry (0.4.5)
       rspec-core
+    rspec-set (0.1.3)
     rspec-support (3.5.0)
     rspec_profiling (0.0.5)
       activerecord
@@ -989,6 +990,7 @@ DEPENDENCIES
   rqrcode-rails3 (~> 0.1.7)
   rspec-rails (~> 3.5.0)
   rspec-retry (~> 0.4.5)
+  rspec-set (~> 0.1.3)
   rspec_profiling (~> 0.0.5)
   rubocop (~> 0.47.1)
   rubocop-rspec (~> 1.15.0)
diff --git a/doc/development/testing.md b/doc/development/testing.md
index 2c7154f1dea..9b0b9808827 100644
--- a/doc/development/testing.md
+++ b/doc/development/testing.md
@@ -202,6 +202,7 @@ Please consult the [dedicated "Frontend testing" guide](./fe_guide/testing.md).
 - Try to follow the [Four-Phase Test][four-phase-test] pattern, using newlines
   to separate phases.
 - Try to use `Gitlab.config.gitlab.host` rather than hard coding `'localhost'`
+- On `before` and `after` hooks, prefer it scoped to `:context` over `:all`
 
 [four-phase-test]: https://robots.thoughtbot.com/four-phase-test
 
@@ -225,6 +226,20 @@ so we need to set some guidelines for their use going forward:
 
 [lets-not]: https://robots.thoughtbot.com/lets-not
 
+#### `set` 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
+`set` in the same way you would use `let`.
+
+`rspec-set` only works on ActiveRecord objects, and before new examples it
+reloads or recreates the model, _only_ if needed. That is, when you changed
+properties or destroyed the object.
+
+There is one gotcha; you can't reference a model defined in a `let` block in a
+`set` block.
+
 ### Time-sensitive tests
 
 [Timecop](https://github.com/travisjeffery/timecop) is available in our
diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb
index 231fd85c464..a1ae1d746af 100644
--- a/spec/finders/issues_finder_spec.rb
+++ b/spec/finders/issues_finder_spec.rb
@@ -1,24 +1,24 @@
 require 'spec_helper'
 
 describe IssuesFinder do
-  let(:user) { create(:user) }
-  let(:user2) { create(:user) }
-  let(:project1) { create(:empty_project) }
-  let(:project2) { create(:empty_project) }
-  let(:milestone) { create(:milestone, project: project1) }
-  let(:label) { create(:label, project: project2) }
-  let(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone, title: 'gitlab') }
-  let(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'gitlab') }
-  let(:issue3) { create(:issue, author: user2, assignee: user2, project: project2, title: 'tanuki', description: 'tanuki') }
+  set(:user) { create(:user) }
+  set(:user2) { create(:user) }
+  set(:project1) { create(:empty_project) }
+  set(:project2) { create(:empty_project) }
+  set(:milestone) { create(:milestone, project: project1) }
+  set(:label) { create(:label, project: project2) }
+  set(:issue1) { create(:issue, author: user, assignee: user, project: project1, milestone: milestone, title: 'gitlab') }
+  set(:issue2) { create(:issue, author: user, assignee: user, project: project2, description: 'gitlab') }
+  set(:issue3) { create(:issue, author: user2, assignee: user2, project: project2, title: 'tanuki', description: 'tanuki') }
 
   describe '#execute' do
-    let(:closed_issue) { create(:issue, author: user2, assignee: user2, project: project2, state: 'closed') }
-    let!(:label_link) { create(:label_link, label: label, target: issue2) }
+    set(:closed_issue) { create(:issue, author: user2, assignee: user2, project: project2, state: 'closed') }
+    set(:label_link) { create(:label_link, label: label, target: issue2) }
     let(:search_user) { user }
     let(:params) { {} }
     let(:issues) { IssuesFinder.new(search_user, params.reverse_merge(scope: scope, state: 'opened')).execute }
 
-    before do
+    before(:context) do
       project1.team << [user, :master]
       project2.team << [user, :developer]
       project2.team << [user2, :developer]
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 784fd1ff885..3ca13111acb 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -3,14 +3,17 @@ require 'spec_helper'
 describe API::Issues do
   include EmailHelpers
 
-  let(:user)        { create(:user) }
+  set(:user) { create(:user) }
+  set(:project) do
+    create(:empty_project, :public, creator_id: user.id, namespace: user.namespace)
+  end
+
   let(:user2)       { create(:user) }
   let(:non_member)  { create(:user) }
-  let(:guest)       { create(:user) }
-  let(:author)      { create(:author) }
-  let(:assignee)    { create(:assignee) }
+  set(:guest)       { create(:user) }
+  set(:author)      { create(:author) }
+  set(:assignee)    { create(:assignee) }
   let(:admin)       { create(:user, :admin) }
-  let!(:project)    { create(:empty_project, :public, creator_id: user.id, namespace: user.namespace ) }
   let(:issue_title)       { 'foo' }
   let(:issue_description) { 'closed' }
   let!(:closed_issue) do
@@ -43,19 +46,19 @@ describe API::Issues do
            title: issue_title,
            description: issue_description
   end
-  let!(:label) do
+  set(:label) do
     create(:label, title: 'label', color: '#FFAABB', project: project)
   end
   let!(:label_link) { create(:label_link, label: label, target: issue) }
-  let!(:milestone) { create(:milestone, title: '1.0.0', project: project) }
-  let!(:empty_milestone) do
+  set(:milestone) { create(:milestone, title: '1.0.0', project: project) }
+  set(:empty_milestone) do
     create(:milestone, title: '2.0.0', project: project)
   end
   let!(:note) { create(:note_on_issue, author: user, project: project, noteable: issue) }
 
   let(:no_milestone_title) { URI.escape(Milestone::None.title) }
 
-  before do
+  before(:all) do
     project.team << [user, :reporter]
     project.team << [guest, :guest]
   end
@@ -70,6 +73,8 @@ describe API::Issues do
     end
 
     context "when authenticated" do
+      let(:first_issue) { json_response.first }
+
       it "returns an array of issues" do
         get api("/issues", user)
 
@@ -79,46 +84,46 @@ describe API::Issues do
       end
 
       it 'returns an array of closed issues' do
-        get api('/issues?state=closed', user)
+        get api('/issues', user), state: :closed
 
         expect_paginated_array_response(size: 1)
-        expect(json_response.first['id']).to eq(closed_issue.id)
+        expect(first_issue['id']).to eq(closed_issue.id)
       end
 
       it 'returns an array of opened issues' do
-        get api('/issues?state=opened', user)
+        get api('/issues', user), state: :opened
 
         expect_paginated_array_response(size: 1)
-        expect(json_response.first['id']).to eq(issue.id)
+        expect(first_issue['id']).to eq(issue.id)
       end
 
       it 'returns an array of all issues' do
-        get api('/issues?state=all', user)
+        get api('/issues', user), state: :all
 
         expect_paginated_array_response(size: 2)
-        expect(json_response.first['id']).to eq(issue.id)
+        expect(first_issue['id']).to eq(issue.id)
         expect(json_response.second['id']).to eq(closed_issue.id)
       end
 
       it 'returns issues matching given search string for title' do
-        get api("/issues?search=#{issue.title}", user)
+        get api("/issues", user), search: issue.title
 
         expect_paginated_array_response(size: 1)
         expect(json_response.first['id']).to eq(issue.id)
       end
 
       it 'returns issues matching given search string for description' do
-        get api("/issues?search=#{issue.description}", user)
+        get api("/issues", user), search: issue.description
 
         expect_paginated_array_response(size: 1)
-        expect(json_response.first['id']).to eq(issue.id)
+        expect(first_issue['id']).to eq(issue.id)
       end
 
       it 'returns an array of labeled issues' do
-        get api("/issues?labels=#{label.title}", user)
+        get api("/issues", user), labels: label.title
 
         expect_paginated_array_response(size: 1)
-        expect(json_response.first['labels']).to eq([label.title])
+        expect(first_issue['labels']).to eq([label.title])
       end
 
       it 'returns an array of labeled issues when all labels matches' do
@@ -135,13 +140,13 @@ describe API::Issues do
       end
 
       it 'returns an empty array if no issue matches labels' do
-        get api('/issues?labels=foo,bar', user)
+        get api('/issues', user), labels: 'foo,bar'
 
         expect_paginated_array_response(size: 0)
       end
 
       it 'returns an array of labeled issues matching given state' do
-        get api("/issues?labels=#{label.title}&state=opened", user)
+        get api("/issues", user), labels: label.title, state: :opened
 
         expect_paginated_array_response(size: 1)
         expect(json_response.first['labels']).to eq([label.title])
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 904e049d767..c4bff1647b5 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -26,6 +26,7 @@ describe API::MergeRequests do
     context "when unauthenticated" do
       it "returns authentication error" do
         get api("/projects/#{project.id}/merge_requests")
+
         expect(response).to have_http_status(401)
       end
     end