Merge branch 'ml-qa-log-out-after-tests' into 'master'

Fail if a test leaves the browser logged in

See merge request gitlab-org/gitlab!18978

doc/development/testing_guide/end_to_end/best_practices.md

@@ -65,3 +65,23 @@ This library [saves the screenshots in the RSpec's `after` hook](https://github.
 
 Given this fact, we should limit the use of `before(:all)` to only those operations where a screenshot is not
 necessary in case of failure and QA logs would be enough for debugging.
+
+## Ensure tests do not leave the browser logged in
+
+All QA tests expect to be able to log in at the start of the test.
+
+That's not possible if a test leaves the browser logged in when it finishes. Normally this isn't a problem because [Capybara resets the session after each test](https://github.com/teamcapybara/capybara/blob/9ebc5033282d40c73b0286e60217515fd1bb0b5d/lib/capybara/rspec.rb#L18). But Capybara does that in an `after` block, so when a test logs in in an `after(:context)` block, the browser returns to a logged in state *after* Capybara had logged it out. And so the next test will fail.
+
+For an example see: <https://gitlab.com/gitlab-org/gitlab/issues/34736>
+
+Ideally, any actions peformed in an `after(:context)` (or [`before(:context)`](#limit-the-use-of-beforeall-hook)) block would be performed via the API. But if it's necessary to do so via the UI (e.g., if API functionality doesn't exist), make sure to log out at the end of the block.
+
+```ruby
+after(:all) do
+  login unless Page::Main::Menu.perform(&:signed_in?)
+
+  # Do something while logged in
+
+  Page::Main::Menu.perform(&:sign_out)
+end
+```

qa/qa/runtime/browser.rb

@@ -19,6 +19,12 @@ module QA
         self.class.configure!
       end
 
+      def self.blank_page?
+        ['', 'about:blank', 'data:,'].include?(Capybara.current_session.driver.browser.current_url)
+      rescue
+        true
+      end
+
       ##
       # Visit a page that belongs to a GitLab instance under given address.
       #

qa/qa/specs/features/ee/browser_ui/1_manage/project/project_templates_spec.rb

@@ -16,17 +16,14 @@ module QA
             }
         ]
 
-        Runtime::Browser.visit(:gitlab, Page::Main::Login)
-        Page::Main::Login.perform(&:sign_in_using_credentials)
-
         @template_container_group_name = "instance-template-container-group-#{SecureRandom.hex(8)}"
 
-        template_container_group = QA::Resource::Group.fabricate! do |group|
+        template_container_group = QA::Resource::Group.fabricate_via_api! do |group|
           group.path = @template_container_group_name
           group.description = 'Instance template container group'
         end
 
-        @template_project = Resource::Project.fabricate! do |project|
+        @template_project = Resource::Project.fabricate_via_api! do |project|
           project.name = 'template-project-1'
           project.group = template_container_group
         end
@@ -36,14 +33,13 @@ module QA
           push.files = @files
           push.commit_message = 'Add test files'
         end
+
+        Page::Main::Menu.perform(&:sign_out_if_signed_in)
       end
 
       context 'built-in' do
         before do
-          # Log out if already logged in
-          Page::Main::Menu.perform do |menu|
-            menu.sign_out if menu.has_personal_area?(wait: 0)
-          end
+          Page::Main::Menu.perform(&:sign_out_if_signed_in)
 
           Runtime::Browser.visit(:gitlab, Page::Main::Login)
           Page::Main::Login.perform(&:sign_in_using_admin_credentials)
@@ -76,10 +72,7 @@ module QA
       # Failure issue: https://gitlab.com/gitlab-org/quality/staging/issues/61
       context 'instance level', :quarantine do
         before do
-          # Log out if already logged in
-          Page::Main::Menu.perform do |menu|
-            menu.sign_out if menu.has_personal_area?(wait: 0)
-          end
+          Page::Main::Menu.perform(&:sign_out_if_signed_in)
 
           Runtime::Browser.visit(:gitlab, Page::Main::Login)
           Page::Main::Login.perform(&:sign_in_using_admin_credentials)
@@ -126,9 +119,7 @@ module QA
         before do
           # Log out if already logged in. This is necessary because
           # a previous test might have logged in as admin
-          Page::Main::Menu.perform do |menu|
-            menu.sign_out if menu.has_personal_area?(wait: 0)
-          end
+          Page::Main::Menu.perform(&:sign_out_if_signed_in)
 
           Runtime::Browser.visit(:gitlab, Page::Main::Login)
           Page::Main::Login.perform(&:sign_in_using_credentials)

qa/spec/spec_helper.rb

@@ -23,6 +23,24 @@ RSpec.configure do |config|
     QA::Runtime::Logger.debug("Starting test: #{example.full_description}") if QA::Runtime::Env.debug?
   end
 
+  config.after(:context) do
+    if !QA::Runtime::Browser.blank_page? && QA::Page::Main::Menu.perform(&:signed_in?)
+      QA::Page::Main::Menu.perform(&:sign_out)
+      raise(
+        <<~ERROR
+          The test left the browser signed in.
+
+          Usually, Capybara prevents this from happening but some things can
+          interfere. For example, if it has an `after(:context)` block that logs
+          in, the browser will stay logged in and this will cause the next test
+          to fail.
+
+          Please make sure the test does not leave the browser signed in.
+        ERROR
+      )
+    end
+  end
+
   config.expect_with :rspec do |expectations|
     expectations.include_chain_clauses_in_custom_matcher_descriptions = true
   end