Remove implicit dependency on `gitlab_sign_in` assigning `@user`rs-login_as

We shouldn't be using instance variables in specs to begin with, and
depending on this implicit behavior of `gitlab_sign_in` would have made
it more difficult to change to `sign_in` where possible.

...we've also gone ahead and changed to `sign_in` where possible.

6 files changed, 72 insertions, 66 deletions

diff --git a/spec/features/commits_spec.rb b/spec/features/commits_spec.rb
index ab2d85371bf..0373f649ee8 100644
--- a/spec/features/commits_spec.rb
+++ b/spec/features/commits_spec.rb
@@ -4,10 +4,11 @@ describe 'Commits' do
   include CiStatusHelper
 
   let(:project) { create(:project, :repository) }
+  let(:user) { create(:user) }
 
   describe 'CI' do
     before do
-      gitlab_sign_in :user
+      sign_in(user)
       stub_ci_pipeline_to_return_yaml_file
     end
 
@@ -27,7 +28,7 @@ describe 'Commits' do
       let!(:status) { create(:generic_commit_status, pipeline: pipeline) }
 
       before do
-        project.team << [@user, :reporter]
+        project.team << [user, :reporter]
       end
 
       describe 'Commit builds' do
@@ -52,7 +53,7 @@ describe 'Commits' do
 
       context 'when logged as developer' do
         before do
-          project.team << [@user, :developer]
+          project.team << [user, :developer]
         end
 
         describe 'Project commits' do
@@ -146,7 +147,7 @@ describe 'Commits' do
 
       context "when logged as reporter" do
         before do
-          project.team << [@user, :reporter]
+          project.team << [user, :reporter]
           build.update_attributes(artifacts_file: artifacts_file)
           visit ci_status_path(pipeline)
         end
@@ -187,11 +188,10 @@ describe 'Commits' do
 
   context 'viewing commits for a branch' do
     let(:branch_name) { 'master' }
-    let(:user) { create(:user) }
 
     before do
       project.team << [user, :master]
-      gitlab_sign_in(user)
+      sign_in(user)
       visit namespace_project_commits_path(project.namespace, project, branch_name)
     end
 
diff --git a/spec/features/gitlab_flavored_markdown_spec.rb b/spec/features/gitlab_flavored_markdown_spec.rb
index 7758f00da7d..1da88d3b1d2 100644
--- a/spec/features/gitlab_flavored_markdown_spec.rb
+++ b/spec/features/gitlab_flavored_markdown_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe "GitLab Flavored Markdown", feature: true do
+  let(:user) { create(:user) }
   let(:project) { create(:empty_project) }
   let(:issue) { create(:issue, project: project) }
   let(:fred) do
@@ -10,8 +11,8 @@ describe "GitLab Flavored Markdown", feature: true do
   end
 
   before do
-    gitlab_sign_in(:user)
-    project.add_developer(@user)
+    sign_in(user)
+    project.add_developer(user)
   end
 
   describe "for commits" do
@@ -51,12 +52,12 @@ describe "GitLab Flavored Markdown", feature: true do
   describe "for issues", feature: true, js: true do
     before do
       @other_issue = create(:issue,
-                            author: @user,
-                            assignees: [@user],
+                            author: user,
+                            assignees: [user],
                             project: project)
       @issue = create(:issue,
-                      author: @user,
-                      assignees: [@user],
+                      author: user,
+                      assignees: [user],
                       project: project,
                       title: "fix #{@other_issue.to_reference}",
                       description: "ask #{fred.to_reference} for details")
diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb
index ea637018617..f47b89fd718 100644
--- a/spec/features/issues_spec.rb
+++ b/spec/features/issues_spec.rb
@@ -5,20 +5,21 @@ describe 'Issues', feature: true do
   include IssueHelpers
   include SortingHelper
 
+  let(:user) { create(:user) }
   let(:project) { create(:empty_project, :public) }
 
   before do
-    gitlab_sign_in :user
+    sign_in(user)
     user2 = create(:user)
 
-    project.team << [[@user, user2], :developer]
+    project.team << [[user, user2], :developer]
   end
 
   describe 'Edit issue' do
     let!(:issue) do
       create(:issue,
-             author: @user,
-             assignees: [@user],
+             author: user,
+             assignees: [user],
              project: project)
     end
 
@@ -35,15 +36,15 @@ describe 'Issues', feature: true do
   describe 'Editing issue assignee' do
     let!(:issue) do
       create(:issue,
-             author: @user,
-             assignees: [@user],
+             author: user,
+             assignees: [user],
              project: project)
     end
 
     it 'allows user to select unassigned', js: true do
       visit edit_namespace_project_issue_path(project.namespace, project, issue)
 
-      expect(page).to have_content "Assignee #{@user.name}"
+      expect(page).to have_content "Assignee #{user.name}"
 
       first('.js-user-search').click
       click_link 'Unassigned'
@@ -86,7 +87,7 @@ describe 'Issues', feature: true do
     end
 
     context 'on edit form' do
-      let(:issue) { create(:issue, author: @user, project: project, due_date: Date.today.at_beginning_of_month.to_s) }
+      let(:issue) { create(:issue, author: user, project: project, due_date: Date.today.at_beginning_of_month.to_s) }
 
       before do
         visit edit_namespace_project_issue_path(project.namespace, project, issue)
@@ -131,10 +132,10 @@ describe 'Issues', feature: true do
 
   describe 'Issue info' do
     it 'excludes award_emoji from comment count' do
-      issue = create(:issue, author: @user, assignees: [@user], project: project, title: 'foobar')
+      issue = create(:issue, author: user, assignees: [user], project: project, title: 'foobar')
       create(:award_emoji, awardable: issue)
 
-      visit namespace_project_issues_path(project.namespace, project, assignee_id: @user.id)
+      visit namespace_project_issues_path(project.namespace, project, assignee_id: user.id)
 
       expect(page).to have_content 'foobar'
       expect(page.all('.no-comments').first.text).to eq "0"
@@ -145,8 +146,8 @@ describe 'Issues', feature: true do
     before do
       %w(foobar barbaz gitlab).each do |title|
         create(:issue,
-               author: @user,
-               assignees: [@user],
+               author: user,
+               assignees: [user],
                project: project,
                title: title)
       end
@@ -168,7 +169,7 @@ describe 'Issues', feature: true do
     end
 
     it 'allows filtering by a specified assignee' do
-      visit namespace_project_issues_path(project.namespace, project, assignee_id: @user.id)
+      visit namespace_project_issues_path(project.namespace, project, assignee_id: user.id)
 
       expect(page).not_to have_content 'foobar'
       expect(page).to have_content 'barbaz'
@@ -366,13 +367,13 @@ describe 'Issues', feature: true do
   end
 
   describe 'when I want to reset my incoming email token' do
-    let(:project1) { create(:empty_project, namespace: @user.namespace) }
+    let(:project1) { create(:empty_project, namespace: user.namespace) }
     let!(:issue) { create(:issue, project: project1) }
 
     before do
       stub_incoming_email_setting(enabled: true, address: "p+%{key}@gl.ab")
-      project1.team << [@user, :master]
-      visit namespace_project_issues_path(@user.namespace, project1)
+      project1.team << [user, :master]
+      visit namespace_project_issues_path(user.namespace, project1)
     end
 
     it 'changes incoming email address token', js: true do
@@ -383,7 +384,7 @@ describe 'Issues', feature: true do
       wait_for_requests
 
       expect(page).to have_no_field('issue_email', with: previous_token)
-      new_token = project1.new_issue_address(@user.reload)
+      new_token = project1.new_issue_address(user.reload)
       expect(page).to have_field(
         'issue_email',
         with: new_token
@@ -392,7 +393,7 @@ describe 'Issues', feature: true do
   end
 
   describe 'update labels from issue#show', js: true do
-    let(:issue) { create(:issue, project: project, author: @user, assignees: [@user]) }
+    let(:issue) { create(:issue, project: project, author: user, assignees: [user]) }
     let!(:label) { create(:label, project: project) }
 
     before do
@@ -411,14 +412,14 @@ describe 'Issues', feature: true do
   end
 
   describe 'update assignee from issue#show' do
-    let(:issue) { create(:issue, project: project, author: @user, assignees: [@user]) }
+    let(:issue) { create(:issue, project: project, author: user, assignees: [user]) }
 
     context 'by authorized user' do
       it 'allows user to select unassigned', js: true do
         visit namespace_project_issue_path(project.namespace, project, issue)
 
         page.within('.assignee') do
-          expect(page).to have_content "#{@user.name}"
+          expect(page).to have_content "#{user.name}"
 
           click_link 'Edit'
           click_link 'Unassigned'
@@ -433,7 +434,7 @@ describe 'Issues', feature: true do
       end
 
       it 'allows user to select an assignee', js: true do
-        issue2 = create(:issue, project: project, author: @user)
+        issue2 = create(:issue, project: project, author: user)
         visit namespace_project_issue_path(project.namespace, project, issue2)
 
         page.within('.assignee') do
@@ -445,28 +446,28 @@ describe 'Issues', feature: true do
         end
 
         page.within '.dropdown-menu-user' do
-          click_link @user.name
+          click_link user.name
         end
 
         page.within('.assignee') do
-          expect(page).to have_content @user.name
+          expect(page).to have_content user.name
         end
       end
 
       it 'allows user to unselect themselves', js: true do
-        issue2 = create(:issue, project: project, author: @user)
+        issue2 = create(:issue, project: project, author: user)
         visit namespace_project_issue_path(project.namespace, project, issue2)
 
         page.within '.assignee' do
           click_link 'Edit'
-          click_link @user.name
+          click_link user.name
 
           page.within '.value .author' do
-            expect(page).to have_content @user.name
+            expect(page).to have_content user.name
           end
 
           click_link 'Edit'
-          click_link @user.name
+          click_link user.name
 
           page.within '.value .assign-yourself' do
             expect(page).to have_content "No assignee"
@@ -483,8 +484,8 @@ describe 'Issues', feature: true do
       end
 
       it 'shows assignee text', js: true do
-        gitlab_sign_out
-        gitlab_sign_in guest
+        sign_out(:user)
+        sign_in(guest)
 
         visit namespace_project_issue_path(project.namespace, project, issue)
         expect(page).to have_content issue.assignees.first.name
@@ -493,7 +494,7 @@ describe 'Issues', feature: true do
   end
 
   describe 'update milestone from issue#show' do
-    let!(:issue) { create(:issue, project: project, author: @user) }
+    let!(:issue) { create(:issue, project: project, author: user) }
     let!(:milestone) { create(:milestone, project: project) }
 
     context 'by authorized user' do
@@ -546,8 +547,8 @@ describe 'Issues', feature: true do
       end
 
       it 'shows milestone text', js: true do
-        gitlab_sign_out
-        gitlab_sign_in guest
+        sign_out(:user)
+        sign_in(guest)
 
         visit namespace_project_issue_path(project.namespace, project, issue)
         expect(page).to have_content milestone.title
@@ -560,7 +561,7 @@ describe 'Issues', feature: true do
 
     context 'by unauthenticated user' do
       before do
-        gitlab_sign_out
+        sign_out(:user)
       end
 
       it 'redirects to signin then back to new issue after signin' do
@@ -570,7 +571,9 @@ describe 'Issues', feature: true do
 
         expect(current_path).to eq new_user_session_path
 
-        gitlab_sign_in :user
+        # NOTE: This is specifically testing the redirect after login, so we
+        # need the full login flow
+        gitlab_sign_in(create(:user))
 
         expect(current_path).to eq new_namespace_project_issue_path(project.namespace, project)
       end
@@ -599,7 +602,7 @@ describe 'Issues', feature: true do
 
       before do
         project.repository.create_file(
-          @user,
+          user,
           '.gitlab/issue_templates/bug.md',
           'this is a test "bug" template',
           message: 'added issue template',
@@ -628,7 +631,7 @@ describe 'Issues', feature: true do
 
       it 'click the button to show modal for the new email' do
         page.within '#issue-email-modal' do
-          email = project.new_issue_address(@user)
+          email = project.new_issue_address(user)
 
           expect(page).to have_selector("input[value='#{email}']")
         end
@@ -636,7 +639,7 @@ describe 'Issues', feature: true do
     end
 
     context 'with existing issues' do
-      let!(:issue) { create(:issue, project: project, author: @user) }
+      let!(:issue) { create(:issue, project: project, author: user) }
 
       it_behaves_like 'show the email in the modal'
     end
@@ -648,7 +651,7 @@ describe 'Issues', feature: true do
 
   describe 'due date' do
     context 'update due on issue#show', js: true do
-      let(:issue) { create(:issue, project: project, author: @user, assignees: [@user]) }
+      let(:issue) { create(:issue, project: project, author: user, assignees: [user]) }
 
       before do
         visit namespace_project_issue_path(project.namespace, project, issue)
@@ -693,7 +696,7 @@ describe 'Issues', feature: true do
 
   describe 'title issue#show', js: true do
     it 'updates the title', js: true do
-      issue = create(:issue, author: @user, assignees: [@user], project: project, title: 'new title')
+      issue = create(:issue, author: user, assignees: [user], project: project, title: 'new title')
 
       visit namespace_project_issue_path(project.namespace, project, issue)
 
diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb
index 5c3d1b3f1ee..8694366de35 100644
--- a/spec/features/projects/branches_spec.rb
+++ b/spec/features/projects/branches_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe 'Branches', feature: true do
+  let(:user) { create(:user) }
   let(:project) { create(:project, :public) }
   let(:repository) { project.repository }
 
@@ -12,8 +13,8 @@ describe 'Branches', feature: true do
 
   context 'logged in as developer' do
     before do
-      gitlab_sign_in :user
-      project.team << [@user, :developer]
+      sign_in(user)
+      project.team << [user, :developer]
     end
 
     describe 'Initial branches page' do
@@ -27,7 +28,7 @@ describe 'Branches', feature: true do
       it 'avoids a N+1 query in branches index' do
         control_count = ActiveRecord::QueryRecorder.new { visit namespace_project_branches_path(project.namespace, project) }.count
 
-        %w(one two three four five).each { |ref| repository.add_branch(@user, ref, 'master') }
+        %w(one two three four five).each { |ref| repository.add_branch(user, ref, 'master') }
 
         expect { visit namespace_project_branches_path(project.namespace, project) }.not_to exceed_query_limit(control_count)
       end
@@ -64,14 +65,14 @@ describe 'Branches', feature: true do
 
     describe 'Delete protected branch' do
       before do
-        project.add_user(@user, :master)
+        project.add_user(user, :master)
         visit namespace_project_protected_branches_path(project.namespace, project)
         set_protected_branch_name('fix')
         click_on "Protect"
 
         within(".protected-branches-list") { expect(page).to have_content('fix') }
         expect(ProtectedBranch.count).to eq(1)
-        project.add_user(@user, :developer)
+        project.add_user(user, :developer)
       end
 
       it 'does not allow devleoper to removes protected branch', js: true do
@@ -87,8 +88,8 @@ describe 'Branches', feature: true do
 
   context 'logged in as master' do
     before do
-      gitlab_sign_in :user
-      project.team << [@user, :master]
+      sign_in(user)
+      project.team << [user, :master]
     end
 
     describe 'Delete protected branch' do
diff --git a/spec/features/projects/commit/cherry_pick_spec.rb b/spec/features/projects/commit/cherry_pick_spec.rb
index 1fb68a0f5e4..0d3fa72fbf5 100644
--- a/spec/features/projects/commit/cherry_pick_spec.rb
+++ b/spec/features/projects/commit/cherry_pick_spec.rb
@@ -1,14 +1,15 @@
 require 'spec_helper'
 
 describe 'Cherry-pick Commits' do
+  let(:user) { create(:user) }
   let(:group) { create(:group) }
   let(:project) { create(:project, namespace: group) }
   let(:master_pickable_commit)  { project.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') }
   let(:master_pickable_merge)  { project.commit('e56497bb5f03a90a51293fc6d516788730953899') }
 
   before do
-    gitlab_sign_in :user
-    project.team << [@user, :master]
+    sign_in(user)
+    project.team << [user, :master]
     visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id)
   end
 
diff --git a/spec/features/triggers_spec.rb b/spec/features/triggers_spec.rb
index bc3c9917a42..5af2c0e9035 100644
--- a/spec/features/triggers_spec.rb
+++ b/spec/features/triggers_spec.rb
@@ -7,7 +7,7 @@ feature 'Triggers', feature: true, js: true do
   let(:guest_user) { create(:user) }
 
   before do
-    gitlab_sign_in(user)
+    sign_in(user)
 
     @project = create(:empty_project)
     @project.team << [user, :master]
@@ -33,7 +33,7 @@ feature 'Triggers', feature: true, js: true do
       # See if "trigger creation successful" message displayed and description and owner are correct
       expect(page.find('.flash-notice')).to have_content 'Trigger was created successfully.'
       expect(page.find('.triggers-list')).to have_content 'trigger desc'
-      expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name
+      expect(page.find('.triggers-list .trigger-owner')).to have_content user.name
     end
   end
 
@@ -61,7 +61,7 @@ feature 'Triggers', feature: true, js: true do
       # See if "trigger updated successfully" message displayed and description and owner are correct
       expect(page.find('.flash-notice')).to have_content 'Trigger was successfully updated.'
       expect(page.find('.triggers-list')).to have_content new_trigger_title
-      expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name
+      expect(page.find('.triggers-list .trigger-owner')).to have_content user.name
     end
 
     scenario 'edit "legacy" trigger and save' do
@@ -98,7 +98,7 @@ feature 'Triggers', feature: true, js: true do
       page.accept_confirm do
         expect(page.find('.flash-notice')).to have_content 'Trigger was re-assigned.'
         expect(page.find('.triggers-list')).to have_content trigger_title
-        expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name
+        expect(page.find('.triggers-list .trigger-owner')).to have_content user.name
       end
     end
   end
@@ -157,7 +157,7 @@ feature 'Triggers', feature: true, js: true do
       expect(page.find('.triggers-list')).not_to have_selector('button.btn-clipboard')
 
       # See if trigger owner name doesn't match with current_user and trigger is non-editable
-      expect(page.find('.triggers-list .trigger-owner')).not_to have_content @user.name
+      expect(page.find('.triggers-list .trigger-owner')).not_to have_content user.name
       expect(page.find('.triggers-list')).not_to have_selector('a[title="Edit"]')
     end
 
@@ -170,7 +170,7 @@ feature 'Triggers', feature: true, js: true do
       expect(page.find('.triggers-list')).to have_selector('button.btn-clipboard')
 
       # See if trigger owner name matches with current_user and is editable
-      expect(page.find('.triggers-list .trigger-owner')).to have_content @user.name
+      expect(page.find('.triggers-list .trigger-owner')).to have_content user.name
       expect(page.find('.triggers-list')).to have_selector('a[title="Edit"]')
     end
   end