Merge branch 'acunskis-fix-pat-creation' into 'master'

E2E: Do not assign nil value to token attribute when creating PAT

See merge request gitlab-org/gitlab!71170

qa/qa/resource/personal_access_token.rb

@@ -8,7 +8,7 @@ module QA
       attr_accessor :name
 
       # The user for which the personal access token is to be created
-      # This *could* be different than the api_client.user or the api_user provided by the QA::Resource::ApiFabricator module
+      # This *could* be different than the api_client.user or the api_user provided by the QA::Resource::ApiFabricator
       attr_writer :user
 
       attribute :token
@@ -17,7 +17,9 @@ module QA
       # If Runtime::Env.admin_personal_access_token is provided, fabricate via the API,
       # else, fabricate via the browser.
       def fabricate_via_api!
-        @token = QA::Resource::PersonalAccessTokenCache.get_token_for_username(user.username)
+        QA::Resource::PersonalAccessTokenCache.get_token_for_username(user.username).tap do |cached_token|
+          @token = cached_token if cached_token
+        end
         return if @token
 
         resource = if Runtime::Env.admin_personal_access_token && !@user.nil?
@@ -28,7 +30,7 @@ module QA
                      fabricate!
                    end
 
-        QA::Resource::PersonalAccessTokenCache.set_token_for_username(user.username, self.token)
+        QA::Resource::PersonalAccessTokenCache.set_token_for_username(user.username, token)
         resource
       end
 

qa/qa/runtime/api/client.rb

@@ -16,17 +16,21 @@ module QA
           enable_ip_limits if ip_limits
         end
 
+        # Personal access token
+        #
+        # It is possible to set the environment variable GITLAB_QA_ACCESS_TOKEN
+        # to use a specific access token rather than create one from the UI
+        # unless a specific user has been passed
+        #
+        # @return [String]
         def personal_access_token
-          @personal_access_token ||= begin
-            # you can set the environment variable GITLAB_QA_ACCESS_TOKEN
-            # to use a specific access token rather than create one from the UI
-            # unless a specific user has been passed
-            @user.nil? ? Runtime::Env.personal_access_token ||= create_personal_access_token : create_personal_access_token
-          end
+          @personal_access_token ||= if user.nil?
+                                       Runtime::Env.personal_access_token ||= create_personal_access_token
+                                     else
+                                       create_personal_access_token
+                                     end
 
-          if @user&.admin?
-            Runtime::Env.admin_personal_access_token = @personal_access_token
-          end
+          Runtime::Env.admin_personal_access_token = @personal_access_token if user&.admin? # rubocop:disable Cop/UserAdmin
 
           @personal_access_token
         end
@@ -82,27 +86,38 @@ module QA
           Page::Main::Menu.perform(&:sign_out)
         end
 
+        # Create PAT
+        #
+        # Use api if admin personal access token is present and skip any UI actions otherwise perform creation via UI
+        #
+        # @return [String]
         def create_personal_access_token
-          signed_in_initially = Page::Main::Menu.perform(&:signed_in?)
-
-          Page::Main::Menu.perform(&:sign_out) if @is_new_session && signed_in_initially
-
-          token = Resource::PersonalAccessToken.fabricate! do |pat|
-            pat.user = user
-          end.token
-
-          # If this is a new session, that tests that follow could fail if they
-          # try to sign in without starting a new session.
-          # Also, if the browser wasn't already signed in, leaving it
-          # signed in could cause tests to fail when they try to sign
-          # in again. For example, that would happen if a test has a
-          # before(:context) block that fabricates via the API, and
-          # it's the first test to run so it creates an access token
-          #
-          # Sign out so the tests can successfully sign in
-          Page::Main::Menu.perform(&:sign_out) if @is_new_session || !signed_in_initially
-
-          token
+          if Runtime::Env.admin_personal_access_token
+            Resource::PersonalAccessToken.fabricate_via_api! do |pat|
+              pat.user = user
+            end.token
+          else
+            signed_in_initially = Page::Main::Menu.perform(&:signed_in?)
+
+            Page::Main::Menu.perform(&:sign_out) if @is_new_session && signed_in_initially
+
+            token = Resource::PersonalAccessToken.fabricate! do |pat|
+              pat.user = user
+            end.token
+
+            # If this is a new session, that tests that follow could fail if they
+            # try to sign in without starting a new session.
+            # Also, if the browser wasn't already signed in, leaving it
+            # signed in could cause tests to fail when they try to sign
+            # in again. For example, that would happen if a test has a
+            # before(:context) block that fabricates via the API, and
+            # it's the first test to run so it creates an access token
+            #
+            # Sign out so the tests can successfully sign in
+            Page::Main::Menu.perform(&:sign_out) if @is_new_session || !signed_in_initially
+
+            token
+          end
         end
       end
     end