From b6151cb0daf665dafad6d4eae4714ee69ec9199c Mon Sep 17 00:00:00 2001
From: Martin Gruner <mg@zammad.com>
Date: Tue, 1 Mar 2022 11:26:51 +0100
Subject: [PATCH] Maintenance: Switch to Selenium 4.

---
 .gitlab/ci/base.yml                            | 14 ++++++--------
 .gitlab/ci/browser-integration/otrs_chrome.yml |  2 +-
 spec/support/capybara/custom_extensions.rb     | 10 ++++++++++
 spec/support/capybara/selenium_driver.rb       | 13 +++++++++++++
 spec/system/system/translations_spec.rb        | 17 ++++-------------
 5 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/.gitlab/ci/base.yml b/.gitlab/ci/base.yml
index f0acf6326..79a2e6479 100644
--- a/.gitlab/ci/base.yml
+++ b/.gitlab/ci/base.yml
@@ -22,17 +22,15 @@
   variables:
     REMOTE_URL: "http://selenium-firefox:4444/wd/hub"
     BROWSER: "firefox"
-    # Settings for Selenium 3:
-    NODE_MAX_INSTANCES: "5"
-    NODE_MAX_SESSION: "5"
+    SE_NODE_MAX_SESSIONS: "5"
+    SE_NODE_OVERRIDE_MAX_SESSIONS: "true"
 
 .variables_selenium_chrome: &variables_selenium_chrome
   variables:
     REMOTE_URL: "http://selenium-chrome:4444/wd/hub"
     BROWSER: chrome
-    # Settings for Selenium 3:
-    NODE_MAX_INSTANCES: "5"
-    NODE_MAX_SESSION: "5"
+    SE_NODE_MAX_SESSIONS: "5"
+    SE_NODE_OVERRIDE_MAX_SESSIONS: "true"
 
 .variables_app_restart_cmd: &variables_app_restart_cmd
   variables:
@@ -86,11 +84,11 @@
   alias: elasticsearch
 
 .docker_selenium_chrome: &docker_selenium_chrome
-  name: $CI_REGISTRY/docker/zammad-selenium-chrome:stable
+  name: $CI_REGISTRY/docker/zammad-selenium-chrome:4
   alias: selenium-chrome
 
 .docker_selenium_firefox: &docker_selenium_firefox
-  name: $CI_REGISTRY/docker/zammad-selenium-firefox:stable
+  name: $CI_REGISTRY/docker/zammad-selenium-firefox:4
   alias: selenium-firefox
 
 .docker_imap: &docker_imap
diff --git a/.gitlab/ci/browser-integration/otrs_chrome.yml b/.gitlab/ci/browser-integration/otrs_chrome.yml
index 79ae8ed9a..2bd3f95e8 100644
--- a/.gitlab/ci/browser-integration/otrs_chrome.yml
+++ b/.gitlab/ci/browser-integration/otrs_chrome.yml
@@ -9,7 +9,7 @@ otrs_chrome:
   services:
     - name: $CI_REGISTRY/docker/zammad-postgresql:stable
       alias: postgresql
-    - name: $CI_REGISTRY/docker/zammad-selenium-chrome:stable
+    - name: $CI_REGISTRY/docker/zammad-selenium-chrome:4
       alias: selenium-chrome
     - name: $CI_REGISTRY/docker/zammad-ci-otrsimport-db:otrs6
       alias: zammad-ci-otrsimport-db
diff --git a/spec/support/capybara/custom_extensions.rb b/spec/support/capybara/custom_extensions.rb
index b8a65287a..05e82855c 100644
--- a/spec/support/capybara/custom_extensions.rb
+++ b/spec/support/capybara/custom_extensions.rb
@@ -152,6 +152,16 @@ module CapybaraCustomExtensions
   def page(...)
     ZammadCapybaraSessionDelegator.new(element: super, context: self)
   end
+
+  # Work around an issue with failures in Capybara if different specs of the same rspec run
+  #   use using_session with the same name, e.g. :second_browser. Solve by prepending the test name.
+  def using_session(name_or_session, &block)
+    if !(name_or_session.is_a? Capybara::Session)
+      # self.class.name refers to the RSpec test case.
+      name_or_session = (self.class.name + "::#{name_or_session}").to_sym
+    end
+    Capybara.using_session(name_or_session, &block)
+  end
 end
 
 RSpec.configure do |config|
diff --git a/spec/support/capybara/selenium_driver.rb b/spec/support/capybara/selenium_driver.rb
index 3ce1a1ffe..2f6091cf7 100644
--- a/spec/support/capybara/selenium_driver.rb
+++ b/spec/support/capybara/selenium_driver.rb
@@ -60,3 +60,16 @@ Capybara.register_driver(:zammad_firefox) do |app|
     driver.browser.file_detector = nil if ENV['REMOTE_URL'].present?
   end
 end
+
+class Capybara::Selenium::Driver
+  alias original_quit quit
+
+  def quit
+    original_quit
+  rescue Selenium::WebDriver::Error::ServerError
+    # Work around a possible capybara/Selenium bug. driver.quit() fails because there is already no session any more;
+    #   not sure why that happens.
+  ensure
+    @browser = nil
+  end
+end
diff --git a/spec/system/system/translations_spec.rb b/spec/system/system/translations_spec.rb
index 2b8241065..ba723366a 100644
--- a/spec/system/system/translations_spec.rb
+++ b/spec/system/system/translations_spec.rb
@@ -64,22 +64,13 @@ RSpec.describe 'System > Translations', type: :system do
         visit '/#system/translation'
 
         def toggle_inline_translations
-          if Gem::Platform.local.os.eql? 'darwin'
-            page.send_keys [:control, :alt, 't']
-          else
-            page.send_keys [:control, :shift, 't']
-          end
+          page.send_keys [*hot_keys, 't']
         end
 
         toggle_inline_translations
 
         span = find '.sidebar span.translation[title="Overviews"]'
-        # Move cursor to the end of the string.
-        if Gem::Platform.local.os.eql? 'darwin'
-          span.send_keys %i[command right], '_modified', :tab
-        else
-          span.send_keys %i[control right], '_modified', :tab
-        end
+        span.native.send_keys 'translation_modified', :tab
 
         # Leave the span to be able to turn off inline translations again
         visit '/#dashboard'
@@ -87,8 +78,8 @@ RSpec.describe 'System > Translations', type: :system do
 
         visit '/#system/translation'
         expect(page).to have_no_css('.sidebar span.translation[title="Overviews"]')
-        expect(find('a[href="#manage/overviews"]')).to have_text("#{overviews_translated}_modified")
-        expect(find('.content.active input.js-Item[data-source="Overviews"]').value).to eq("#{overviews_translated}_modified")
+        expect(find('a[href="#manage/overviews"]')).to have_text('translation_modified').and(have_text(overviews_translated))
+        expect(find('.content.active input.js-Item[data-source="Overviews"]').value).to have_text('translation_modified').and(have_text(overviews_translated))
       end
     end