Applied required changes to install and use PHP 5.6 for testing the Zammad PHP API client - until @jepf adds PHP 7 support (for tests).

.circleci/config.yml

@@ -11,7 +11,7 @@ jobs:
 
   install-mysql:
     docker:
-      - image: circleci/ruby:2.4.4
+      - image: circleci/ruby:2.5.5
       - image: circleci/mysql:5.7-ram
         command: --max_allowed_packet=64MB
         environment:
@@ -28,7 +28,7 @@ jobs:
 
   install-postgresql:
     docker:
-      - image: circleci/ruby:2.4.4
+      - image: circleci/ruby:2.5.5
       - image: circleci/postgres:11-ram
         environment:
           POSTGRES_DB: zammad_test

.gitlab-ci.yml

@@ -1,4 +1,4 @@
-image: registry.znuny.com/docker/zammad-ruby:2.4.4
+image: registry.znuny.com/docker/zammad-ruby:2.5.5
 
 # Global variables added to the ENV of each job
 variables:
@@ -61,7 +61,7 @@ variables:
 
 # Cache gems in between jobs and pipelines
 cache:
-  key: "ruby24"
+  key: "ruby25"
   paths:
     - vendor/ruby
 

.ruby-version

@@ -1 +1 @@
-2.4.4
+2.5.5

Gemfile

@@ -1,7 +1,7 @@
 source 'https://rubygems.org'
 
 # core - base
-ruby '2.4.4'
+ruby '2.5.5'
 gem 'rails', '5.1.7'
 
 # core - rails additions

Gemfile.lock

@@ -641,7 +641,7 @@ DEPENDENCIES
   zendesk_api
 
 RUBY VERSION
-   ruby 2.4.4p296
+   ruby 2.5.5p157
 
 BUNDLED WITH
    1.17.3

app/controllers/integration/ldap_controller.rb

@@ -10,19 +10,19 @@ class Integration::LdapController < ApplicationController
 
   def discover
     answer_with do
-      begin
-        ldap = ::Ldap.new(params)
 
-        {
+      ldap = ::Ldap.new(params)
-          attributes: ldap.preferences
+
-        }
+      {
-      rescue => e
+        attributes: ldap.preferences
-        # workaround for issue #1114
+      }
-        raise if !e.message.end_with?(', 48, Inappropriate Authentication')
+    rescue => e
+      # workaround for issue #1114
+      raise if !e.message.end_with?(', 48, Inappropriate Authentication')
+
+      # return empty result
+      {}
 
-        # return empty result
-        {}
-      end
     end
   end
 

app/controllers/reports_controller.rb

@@ -250,52 +250,52 @@ class ReportsController < ApplicationController
 
     row = 2
     result[:ticket_ids].each do |ticket_id|
-      begin
-        ticket = Ticket.lookup(id: ticket_id)
-        row += 1
-        worksheet.write_string(row, 0, ticket.number)
-        worksheet.write_string(row, 1, ticket.title)
-        worksheet.write_string(row, 2, ticket.state.name)
-        worksheet.write_string(row, 3, ticket.priority.name)
-        worksheet.write_string(row, 4, ticket.group.name)
-        worksheet.write_string(row, 5, ticket.owner.fullname)
-        worksheet.write_string(row, 6, ticket.customer.fullname)
-        worksheet.write_string(row, 7, ticket.try(:organization).try(:name))
-        worksheet.write_string(row, 8, ticket.create_article_type.name)
-        worksheet.write_string(row, 9, ticket.create_article_sender.name)
-        worksheet.write_string(row, 10, ticket.tag_list.join(','))
 
-        worksheet.write_date_time(row, 11, time_in_localtime_for_excel(ticket.created_at, params[:timezone]), format_time)
+      ticket = Ticket.lookup(id: ticket_id)
-        worksheet.write_date_time(row, 12, time_in_localtime_for_excel(ticket.updated_at, params[:timezone]), format_time)
+      row += 1
-        worksheet.write_date_time(row, 13, time_in_localtime_for_excel(ticket.close_at, params[:timezone]), format_time) if ticket.close_at.present?
+      worksheet.write_string(row, 0, ticket.number)
+      worksheet.write_string(row, 1, ticket.title)
+      worksheet.write_string(row, 2, ticket.state.name)
+      worksheet.write_string(row, 3, ticket.priority.name)
+      worksheet.write_string(row, 4, ticket.group.name)
+      worksheet.write_string(row, 5, ticket.owner.fullname)
+      worksheet.write_string(row, 6, ticket.customer.fullname)
+      worksheet.write_string(row, 7, ticket.try(:organization).try(:name))
+      worksheet.write_string(row, 8, ticket.create_article_type.name)
+      worksheet.write_string(row, 9, ticket.create_article_sender.name)
+      worksheet.write_string(row, 10, ticket.tag_list.join(','))
 
-        # Object Manager attributes
+      worksheet.write_date_time(row, 11, time_in_localtime_for_excel(ticket.created_at, params[:timezone]), format_time)
-        column = 14
+      worksheet.write_date_time(row, 12, time_in_localtime_for_excel(ticket.updated_at, params[:timezone]), format_time)
-        # We already queried ObjectManager::Attributes, so we just use objects
+      worksheet.write_date_time(row, 13, time_in_localtime_for_excel(ticket.close_at, params[:timezone]), format_time) if ticket.close_at.present?
-        objects.each do |object|
+
-          key = object[:name]
+      # Object Manager attributes
-          case object[:data_type]
+      column = 14
-          when 'boolean', 'select'
+      # We already queried ObjectManager::Attributes, so we just use objects
-            value = ticket.send(key.to_sym)
+      objects.each do |object|
-            if object[:data_option] && object[:data_option]['options'] && object[:data_option]['options'][ticket.send(key.to_sym)]
+        key = object[:name]
-              value = object[:data_option]['options'][ticket.send(key.to_sym)]
+        case object[:data_type]
-            end
+        when 'boolean', 'select'
-            worksheet.write_string(row, column, value)
+          value = ticket.send(key.to_sym)
-          when 'datetime'
+          if object[:data_option] && object[:data_option]['options'] && object[:data_option]['options'][ticket.send(key.to_sym)]
-            worksheet.write_date_time(row, column, time_in_localtime_for_excel(ticket.send(key.to_sym), params[:timezone]), format_time) if ticket.send(key.to_sym).present?
+            value = object[:data_option]['options'][ticket.send(key.to_sym)]
-          when 'date'
-            worksheet.write_date_time(row, column, ticket.send(key.to_sym).to_s, format_date) if ticket.send(key.to_sym).present?
-          when 'integer'
-            worksheet.write_number(row, column, ticket.send(key.to_sym))
-          else
-            # for text, integer and tree select
-            worksheet.write_string(row, column, ticket.send(key.to_sym).to_s)
           end
-          column += 1
+          worksheet.write_string(row, column, value)
+        when 'datetime'
+          worksheet.write_date_time(row, column, time_in_localtime_for_excel(ticket.send(key.to_sym), params[:timezone]), format_time) if ticket.send(key.to_sym).present?
+        when 'date'
+          worksheet.write_date_time(row, column, ticket.send(key.to_sym).to_s, format_date) if ticket.send(key.to_sym).present?
+        when 'integer'
+          worksheet.write_number(row, column, ticket.send(key.to_sym))
+        else
+          # for text, integer and tree select
+          worksheet.write_string(row, column, ticket.send(key.to_sym).to_s)
         end
-      rescue => e
+        column += 1
-        Rails.logger.error "SKIP: #{e.message}"
       end
+    rescue => e
+      Rails.logger.error "SKIP: #{e.message}"
+
     end
 
     row += 2

app/models/channel.rb

@@ -188,29 +188,29 @@ stream all accounts
 
         # start threads for each channel
         @@channel_stream[channel_id][:thread] = Thread.new do
-          begin
+
-            logger.info "Started stream channel for '#{channel.id}' (#{channel.area})..."
+          logger.info "Started stream channel for '#{channel.id}' (#{channel.area})..."
-            channel.status_in = 'ok'
+          channel.status_in = 'ok'
-            channel.last_log_in = ''
+          channel.last_log_in = ''
-            channel.save!
+          channel.save!
-            @@channel_stream_started_till_at[channel_id] = Time.zone.now
+          @@channel_stream_started_till_at[channel_id] = Time.zone.now
-            @@channel_stream[channel_id] ||= {}
+          @@channel_stream[channel_id] ||= {}
-            @@channel_stream[channel_id][:stream_instance] = channel.stream_instance
+          @@channel_stream[channel_id][:stream_instance] = channel.stream_instance
-            @@channel_stream[channel_id][:stream_instance].stream
+          @@channel_stream[channel_id][:stream_instance].stream
-            @@channel_stream[channel_id][:stream_instance].disconnect
+          @@channel_stream[channel_id][:stream_instance].disconnect
-            @@channel_stream.delete(channel_id)
+          @@channel_stream.delete(channel_id)
-            @@channel_stream_started_till_at[channel_id] = Time.zone.now
+          @@channel_stream_started_till_at[channel_id] = Time.zone.now
-            logger.info " ...stopped stream thread for '#{channel.id}'"
+          logger.info " ...stopped stream thread for '#{channel.id}'"
-          rescue => e
+        rescue => e
-            error = "Can't use stream for channel (#{channel.id}): #{e.inspect}"
+          error = "Can't use stream for channel (#{channel.id}): #{e.inspect}"
-            logger.error error
+          logger.error error
-            logger.error e
+          logger.error e
-            channel.status_in = 'error'
+          channel.status_in = 'error'
-            channel.last_log_in = error
+          channel.last_log_in = error
-            channel.save!
+          channel.save!
-            @@channel_stream.delete(channel_id)
+          @@channel_stream.delete(channel_id)
-            @@channel_stream_started_till_at[channel_id] = Time.zone.now
+          @@channel_stream_started_till_at[channel_id] = Time.zone.now
-          end
+
         end
       end
 

app/models/channel/driver/imap.rb

@@ -127,11 +127,11 @@ example
 
     message_ids = nil
     timeout(6.minutes) do
-      begin
+
-        message_ids = @imap.sort(['DATE'], filter, 'US-ASCII')
+      message_ids = @imap.sort(['DATE'], filter, 'US-ASCII')
-      rescue
+    rescue
-        message_ids = @imap.search(filter)
+      message_ids = @imap.search(filter)
-      end
+
     end
 
     # check mode only

app/models/channel/email_parser.rb

@@ -586,14 +586,14 @@ process unprocessable_mails (tmp/unprocessable_mail/*.eml) again
     end
 
     mail.parts.each do |part|
-      begin
-        new_attachments = get_attachments(part, attachments, mail).flatten.compact
-        attachments.push(*new_attachments)
-      rescue => e # Protect process to work with spam emails (see test/fixtures/mail15.box)
-        raise e if (fail_count ||= 0).positive?
 
-        (fail_count += 1) && retry
+      new_attachments = get_attachments(part, attachments, mail).flatten.compact
-      end
+      attachments.push(*new_attachments)
+    rescue => e # Protect process to work with spam emails (see test/fixtures/mail15.box)
+      raise e if (fail_count ||= 0).positive?
+
+      (fail_count += 1) && retry
+
     end
 
     attachments
@@ -608,15 +608,15 @@ process unprocessable_mails (tmp/unprocessable_mail/*.eml) again
     file.header.fields.each do |field|
 
       # full line, encode, ready for storage
-      begin
+
-        value = field.to_utf8
+      value = field.to_utf8
-        if value.blank?
+      if value.blank?
-          value = field.raw_value
+        value = field.raw_value
-        end
-        headers_store[field.name.to_s] = value
-      rescue => e
-        headers_store[field.name.to_s] = field.raw_value
       end
+      headers_store[field.name.to_s] = value
+    rescue => e
+      headers_store[field.name.to_s] = field.raw_value
+
     end
 
     # cleanup content id, <> will be added automatically later

app/models/channel/filter/database.rb

@@ -12,32 +12,32 @@ module Channel::Filter::Database
       all_matches_ok = true
       min_one_rule_exists = false
       filter[:match].each do |key, meta|
-        begin
-          next if meta.blank? || meta['value'].blank?
 
-          value = mail[ key.downcase.to_sym ]
+        next if meta.blank? || meta['value'].blank?
-          match_rule = meta['value']
+
-          min_one_rule_exists = true
+        value = mail[ key.downcase.to_sym ]
-          if meta[:operator] == 'contains not'
+        match_rule = meta['value']
-            if value.present? && Channel::Filter::Match::EmailRegex.match(value: value, match_rule: match_rule)
+        min_one_rule_exists = true
-              all_matches_ok = false
+        if meta[:operator] == 'contains not'
-              Rails.logger.info "  matching #{key.downcase}:'#{value}' on #{match_rule}, but shoud not"
+          if value.present? && Channel::Filter::Match::EmailRegex.match(value: value, match_rule: match_rule)
-            end
-          elsif meta[:operator] == 'contains'
-            if value.blank? || !Channel::Filter::Match::EmailRegex.match(value: value, match_rule: match_rule)
-              all_matches_ok = false
-              Rails.logger.info "  not matching #{key.downcase}:'#{value}' on #{match_rule}, but should"
-            end
-          else
             all_matches_ok = false
-            Rails.logger.info "  Invalid operator in match #{meta.inspect}"
+            Rails.logger.info "  matching #{key.downcase}:'#{value}' on #{match_rule}, but shoud not"
           end
-          break if !all_matches_ok
+        elsif meta[:operator] == 'contains'
-        rescue => e
+          if value.blank? || !Channel::Filter::Match::EmailRegex.match(value: value, match_rule: match_rule)
+            all_matches_ok = false
+            Rails.logger.info "  not matching #{key.downcase}:'#{value}' on #{match_rule}, but should"
+          end
+        else
           all_matches_ok = false
-          Rails.logger.error "can't use match rule #{match_rule} on #{value}"
+          Rails.logger.info "  Invalid operator in match #{meta.inspect}"
-          Rails.logger.error e.inspect
         end
+        break if !all_matches_ok
+      rescue => e
+        all_matches_ok = false
+        Rails.logger.error "can't use match rule #{match_rule} on #{value}"
+        Rails.logger.error e.inspect
+
       end
 
       next if !min_one_rule_exists

app/models/store/provider/file.rb

@@ -69,6 +69,7 @@ class Store::Provider::File
       local_location = locations[0, count].join('/')
       break if local_location.match?(%r{storage/fs/{0,4}$})
       break if Dir["#{local_location}/*"].present?
+      next if !Dir.exist?(local_location)
 
       FileUtils.rmdir(local_location)
     end

app/models/ticket.rb

@@ -444,22 +444,22 @@ get count of tickets and tickets which match on selector
     return [] if !query
 
     ActiveRecord::Base.transaction(requires_new: true) do
-      begin
-        if !current_user
-          ticket_count = Ticket.distinct.where(query, *bind_params).joins(tables).count
-          tickets = Ticket.distinct.where(query, *bind_params).joins(tables).limit(limit)
-          return [ticket_count, tickets]
-        end
-
-        access_condition = Ticket.access_condition(current_user, access)
-        ticket_count = Ticket.distinct.where(access_condition).where(query, *bind_params).joins(tables).count
-        tickets = Ticket.distinct.where(access_condition).where(query, *bind_params).joins(tables).limit(limit)
 
+      if !current_user
+        ticket_count = Ticket.distinct.where(query, *bind_params).joins(tables).count
+        tickets = Ticket.distinct.where(query, *bind_params).joins(tables).limit(limit)
         return [ticket_count, tickets]
-      rescue ActiveRecord::StatementInvalid => e
-        Rails.logger.error e
-        raise ActiveRecord::Rollback
       end
+
+      access_condition = Ticket.access_condition(current_user, access)
+      ticket_count = Ticket.distinct.where(access_condition).where(query, *bind_params).joins(tables).count
+      tickets = Ticket.distinct.where(access_condition).where(query, *bind_params).joins(tables).limit(limit)
+
+      return [ticket_count, tickets]
+    rescue ActiveRecord::StatementInvalid => e
+      Rails.logger.error e
+      raise ActiveRecord::Rollback
+
     end
     []
   end

db/migrate/20170516000001_trigger_recipient_update.rb

@@ -5,19 +5,19 @@ class TriggerRecipientUpdate < ActiveRecord::Migration[4.2]
     return if !Setting.find_by(name: 'system_init_done')
 
     ['auto reply (on new tickets)', 'auto reply (on follow up of tickets)'].each do |name|
-      begin
-        trigger = Trigger.find_by(name: name)
-        next if trigger.blank?
-        next if trigger.perform.blank?
-        next if trigger.perform['notification.email'].blank?
-        next if trigger.perform['notification.email']['recipient'].blank?
-        next if trigger.perform['notification.email']['recipient'] != 'ticket_customer'
 
-        trigger.perform['notification.email']['recipient'] = 'article_last_sender'
+      trigger = Trigger.find_by(name: name)
-        trigger.save!
+      next if trigger.blank?
-      rescue => e
+      next if trigger.perform.blank?
-        Rails.logger.error "Unable to update Trigger.find(#{trigger.id}) '#{trigger.inspect}': #{e.message}"
+      next if trigger.perform['notification.email'].blank?
-      end
+      next if trigger.perform['notification.email']['recipient'].blank?
+      next if trigger.perform['notification.email']['recipient'] != 'ticket_customer'
+
+      trigger.perform['notification.email']['recipient'] = 'article_last_sender'
+      trigger.save!
+    rescue => e
+      Rails.logger.error "Unable to update Trigger.find(#{trigger.id}) '#{trigger.inspect}': #{e.message}"
+
     end
 
   end

db/migrate/20170531144425_foreign_keys.rb

@@ -220,11 +220,11 @@ class ForeignKeys < ActiveRecord::Migration[4.2]
 
     foreign_keys.each do |foreign_key|
       ActiveRecord::Base.transaction do
-        begin
+
-          add_foreign_key(*foreign_key)
+        add_foreign_key(*foreign_key)
-        rescue => e
+      rescue => e
-          Rails.logger.error "Inconsistent data status detected while adding foreign key '#{foreign_key.inspect}': #{e.message}"
+        Rails.logger.error "Inconsistent data status detected while adding foreign key '#{foreign_key.inspect}': #{e.message}"
-        end
+
       end
     end
   end

db/migrate/20170822000001_agend_based_sender_issue_1351.rb

@@ -5,11 +5,11 @@ class AgendBasedSenderIssue1351 < ActiveRecord::Migration[4.2]
     return if !Setting.find_by(name: 'system_init_done')
 
     EmailAddress.all.each do |email_address|
-      begin
+
-        email_address.save!
+      email_address.save!
-      rescue => e
+    rescue => e
-        Rails.logger.error "Unable to update EmailAddress.find(#{email_address.id}) '#{email_address.inspect}': #{e.message}"
+      Rails.logger.error "Unable to update EmailAddress.find(#{email_address.id}) '#{email_address.inspect}': #{e.message}"
-      end
+
     end
   end
 end

db/migrate/20171023000001_fixed_store_upgrade_ror_45.rb

@@ -7,11 +7,11 @@ class FixedStoreUpgradeRor45 < ActiveRecord::Migration[5.0]
     Cache.clear
     [Macro, Taskbar, Calendar, Trigger, Channel, Job, PostmasterFilter, Report::Profile, Setting, Sla, Template].each do |class_name|
       class_name.all.each do |record|
-        begin
+
-          record.save!
+        record.save!
-        rescue => e
+      rescue => e
-          Rails.logger.error "Unable to save/update #{class_name}.find(#{record.id}): #{e.message}"
+        Rails.logger.error "Unable to save/update #{class_name}.find(#{record.id}): #{e.message}"
-        end
+
       end
     end
 

db/migrate/20180426134922_issue_1977_remove_invalid_user_foreign_keys.rb

@@ -24,13 +24,13 @@ class Issue1977RemoveInvalidUserForeignKeys < ActiveRecord::Migration[5.1]
     ]
 
     foreign_keys.each do |args|
-      begin
+
-        add_foreign_key(*args)
+      add_foreign_key(*args)
-      rescue ActiveRecord::StatementInvalid => e
+    rescue ActiveRecord::StatementInvalid => e
-        Rails.logger.info "Can't add foreign_keys '#{args.inspect}'"
+      Rails.logger.info "Can't add foreign_keys '#{args.inspect}'"
-        Rails.logger.info e
+      Rails.logger.info e
-        ActiveRecord::Base.connection.reconnect!
+      ActiveRecord::Base.connection.reconnect!
-      end
+
     end
   end
 end

lib/core_ext/string.rb

@@ -444,14 +444,14 @@ class String
     #map['word-en-de'] = "[^#{marker}].{1,250}\s(wrote|schrieb):"
 
     map.each_value do |regexp|
-      begin
+
-        string.sub!(/#{regexp}/) do |placeholder|
+      string.sub!(/#{regexp}/) do |placeholder|
-          placeholder = "#{marker}#{placeholder}"
+        placeholder = "#{marker}#{placeholder}"
-        end
-      rescue
-        # regexp was not possible because of some string encoding issue, use next
-        Rails.logger.debug { "Invalid string/charset combination with regexp #{regexp} in string" }
       end
+    rescue
+      # regexp was not possible because of some string encoding issue, use next
+      Rails.logger.debug { "Invalid string/charset combination with regexp #{regexp} in string" }
+
     end
 
     string
@@ -494,11 +494,11 @@ class String
 
     # try to find valid encodings of string
     viable_encodings.each do |enc|
-      begin
+
-        return encode!('utf-8', enc)
+      return encode!('utf-8', enc)
-      rescue EncodingError => e
+    rescue EncodingError => e
-        Rails.logger.error { e.inspect }
+      Rails.logger.error { e.inspect }
-      end
+
     end
 
     case options[:fallback]

lib/sequencer/unit/import/exchange/attribute_examples.rb

@@ -14,18 +14,17 @@ class Sequencer
 
                 ews_folder_ids.collect do |folder_id|
 
-                  begin
+                  ews_folder.find(folder_id).items.each do |resource|
-                    ews_folder.find(folder_id).items.each do |resource|
+                    attributes = ::Import::Exchange::ItemAttributes.extract(resource)
-                      attributes = ::Import::Exchange::ItemAttributes.extract(resource)
+                    extractor.extract(attributes)
-                      extractor.extract(attributes)
+                    break if extractor.enough
-                      break if extractor.enough
-                    end
-                  rescue NoMethodError => e
-                    raise if e.message !~ /Viewpoint::EWS::/
-
-                    logger.error e
-                    logger.error "Skipping folder_id '#{folder_id}' due to unsupported entries."
                   end
+                rescue NoMethodError => e
+                  raise if e.message !~ /Viewpoint::EWS::/
+
+                  logger.error e
+                  logger.error "Skipping folder_id '#{folder_id}' due to unsupported entries."
+
                 end
               end.examples
             end

lib/tasks/zammad/db/reset.rake

@@ -10,14 +10,13 @@ namespace :zammad do
       # there is no way in rake to achive that
       %w[db:drop:_unsafe db:create db:schema:load db:seed].each do |task|
 
-        begin
+        $stdout = StringIO.new if task == 'db:schema:load'.freeze
-          $stdout = StringIO.new if task == 'db:schema:load'.freeze
+
+        Rake::Task[task].reenable
+        Rake::Task[task].invoke
+      ensure
+        $stdout = STDOUT
 
-          Rake::Task[task].reenable
-          Rake::Task[task].invoke
-        ensure
-          $stdout = STDOUT
-        end
       end
 
       ActiveRecord::Base.connection.reconnect!

lib/telegram.rb

@@ -294,17 +294,17 @@ returns
     end
     if title == '-'
       %i[sticker photo document voice].each do |area|
-        begin
-          next if !params[:message]
-          next if !params[:message][area]
-          next if !params[:message][area][:emoji]
 
-          title = params[:message][area][:emoji]
+        next if !params[:message]
-          break
+        next if !params[:message][area]
-        rescue
+        next if !params[:message][area][:emoji]
-          # just go ahead
+
-          title
+        title = params[:message][area][:emoji]
-        end
+        break
+      rescue
+        # just go ahead
+        title
+
       end
     end
     if title.length > 60

spec/lib/notification_factory_spec.rb

@@ -25,14 +25,14 @@ RSpec.describe NotificationFactory do
         let(:template_path) { Rails.root.to_s + "/app/views/mailer/signup/#{rendered_locale}.html.erb.custom" }
 
         it 'uses that file' do
-          begin
-            File.write(template_path, "Subject\nBody\nbody\n")
 
-            expect(described_class.template_read(read_params))
+          File.write(template_path, "Subject\nBody\nbody\n")
-              .to eq({ subject: "Subject\n", body: "Body\nbody\n" })
+
-          ensure
+          expect(described_class.template_read(read_params))
-            File.delete(template_path)
+            .to eq({ subject: "Subject\n", body: "Body\nbody\n" })
-          end
+        ensure
+          File.delete(template_path)
+
         end
       end
 

spec/scripts/websocket_server_spec.rb

@@ -27,18 +27,18 @@ describe 'websocket-server', type: :script do
     end
 
     it 'starts up successfully' do
-      begin
-        system("RAILS_ENV=test #{ws_server} start -db #{ipv6_addr} -p #{port} >/dev/null 2>&1")
 
-        # Wait for daemon to start
+      system("RAILS_ENV=test #{ws_server} start -db #{ipv6_addr} -p #{port} >/dev/null 2>&1")
-        Timeout.timeout(20, Timeout::Error, 'WebSocket Server startup timed out') do
-          loop { break if File.size(output_log) + File.size(error_log) > 0 }
-        end
 
-        expect(File.read(error_log)).not_to include(error_msg)
+      # Wait for daemon to start
-      ensure
+      Timeout.timeout(20, Timeout::Error, 'WebSocket Server startup timed out') do
-        system("#{ws_server} stop >/dev/null 2>&1") if File.exist?(pidfile)
+        loop { break if File.size(output_log) + File.size(error_log) > 0 }
       end
+
+      expect(File.read(error_log)).not_to include(error_msg)
+    ensure
+      system("#{ws_server} stop >/dev/null 2>&1") if File.exist?(pidfile)
+
     end
   end
 end

spec/support/capybara/browser_test_helper.rb

@@ -38,10 +38,10 @@ module BrowserTestHelper
     #
     def until_exists
       self.until do
-        begin
+
-          yield
+        yield
-        rescue Capybara::ElementNotFound # rubocop:disable Lint/HandleExceptions
+      rescue Capybara::ElementNotFound # rubocop:disable Lint/HandleExceptions
-        end
+
       end
     rescue Selenium::WebDriver::Error::TimeOutError => e
       # cleanup backtrace

test/browser_test_helper.rb

@@ -105,14 +105,14 @@ class TestCase < ActiveSupport::TestCase
 
     # avoid "Cannot read property 'get_Current' of undefined" issues
     (1..5).each do |count|
-      begin
+
-        local_browser = browser_instance_remote
+      local_browser = browser_instance_remote
-        break
+      break
-      rescue => e
+    rescue => e
-        wait_until_ready = rand(5..13)
+      wait_until_ready = rand(5..13)
-        log('browser_instance', { rescure: true, count: count, sleep: wait_until_ready, exception: e })
+      log('browser_instance', { rescure: true, count: count, sleep: wait_until_ready, exception: e })
-        sleep wait_until_ready
+      sleep wait_until_ready
-      end
+
     end
 
     local_browser
@@ -1579,25 +1579,25 @@ wait untill text in selector disabppears
 
     99.times do
       #sleep 0.5
-      begin
-        if instance.find_elements(css: '#navigation .tasks .task:first-child')[0]
-          instance.action.move_to(instance.find_elements(css: '#navigation .tasks .task:first-child')[0]).release.perform
-          click_element = instance.find_elements(css: '#navigation .tasks .task:first-child .js-close')[0]
-          if click_element
-            click_element.click
 
-            # accept task close warning
+      if instance.find_elements(css: '#navigation .tasks .task:first-child')[0]
-            if instance.find_elements(css: '.modal button.js-submit')[0]
+        instance.action.move_to(instance.find_elements(css: '#navigation .tasks .task:first-child')[0]).release.perform
-              sleep 0.4
+        click_element = instance.find_elements(css: '#navigation .tasks .task:first-child .js-close')[0]
-              instance.find_elements(css: '.modal button.js-submit')[0].click
+        if click_element
-            end
+          click_element.click
+
+          # accept task close warning
+          if instance.find_elements(css: '.modal button.js-submit')[0]
+            sleep 0.4
+            instance.find_elements(css: '.modal button.js-submit')[0].click
           end
-        else
-          break
         end
-      rescue
+      else
-        # try again
+        break
       end
+    rescue
+      # try again
+
     end
     assert(true, 'all tasks closed')
   end
@@ -1790,13 +1790,13 @@ wait untill text in selector disabppears
 
     if data[:roles]
       99.times do
-        begin
-          element = instance.find_elements(css: '.modal .js-selected[data-name=role_ids] .js-option:not(.is-hidden)')[0]
-          break if !element
 
-          element.click
+        element = instance.find_elements(css: '.modal .js-selected[data-name=role_ids] .js-option:not(.is-hidden)')[0]
-          sleep 0.1
+        break if !element
-        end
+
+        element.click
+        sleep 0.1
+
       end
       data[:roles].each do |role|
         instance.execute_script("$(\".modal [data-name=role_ids] .js-pool .js-option:not(.is-hidden):contains('#{role}')\").first().click()")
@@ -1945,13 +1945,13 @@ wait untill text in selector disabppears
     end
     if data[:roles]
       99.times do
-        begin
-          element = instance.find_elements(css: '.modal .js-selected[data-name=role_ids] .js-option:not(.is-hidden)')[0]
-          break if !element
 
-          element.click
+        element = instance.find_elements(css: '.modal .js-selected[data-name=role_ids] .js-option:not(.is-hidden)')[0]
-          sleep 0.1
+        break if !element
-        end
+
+        element.click
+        sleep 0.1
+
       end
       data[:roles].each do |role|
         instance.execute_script("$(\".modal [data-name=role_ids] .js-pool .js-option:not(.is-hidden):contains('#{role}')\").first().click()")