Browse Source

Maintenance: Add Rubocop cop to ensure time zones in RSpec are set correctly

Mantas 6 days ago
parent
commit
16e54816af

+ 63 - 0
.dev/rubocop/cop/zammad/ensures_time_zone_at_spec_top_level.rb

@@ -0,0 +1,63 @@
+# Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
+
+module RuboCop
+  module Cop
+    module Zammad
+      # This spec ensures that time zones in system specs are set at the top level only.
+      # The problem is if time zone changes in the middle of the spec, browser does not pick it up.
+      # This leads to confusing issues when whole file vs specific portion are executed.
+      #
+      # Non-system tests are ignored. Timezone in pure RSPec tests can be set at any moment.
+      #
+      # @example
+      #   # bad
+      #   RSpec.describe 'Test', type: :system do
+      #     it 'is fine', time_zone: 'Europe/London' do
+      #       example
+      #     end
+      #   end
+      #
+      #   # good
+      #   RSpec.describe 'Test', type: :system, time_zone: 'Europe/London' do
+      #     it 'is fine' do
+      #       example
+      #     end
+      #   end
+      class EnsuresTimeZoneAtSpecTopLevel < Base
+        def_node_matcher :is_rspec_block_with_time_zone?, <<-PATTERN
+          $(send _ {:describe :context :it :shared_examples} (_ ...) (hash <(pair (sym :time_zone) (str...))...>))
+        PATTERN
+
+        def_node_matcher :is_system_type?, <<-PATTERN
+          $(block (send (const _ :RSpec) :describe (_ ...)  (hash <(pair (sym :type) (sym :system))...>)...)...)
+        PATTERN
+
+        def_node_matcher :is_rspec_top_level_block?, <<-PATTERN
+          $(send (const _ :RSpec) :describe...)
+        PATTERN
+
+        MSG = 'RSpec system tests (aka Capybara) should set custom time zones at top level only'.freeze
+
+        def on_send(node)
+          return if !is_rspec_block_with_time_zone?(node)
+          return if is_rspec_top_level_block?(node)
+          return if !in_system_spec?(node)
+
+          add_offense(node)
+        end
+
+        def in_system_spec?(node)
+          top_parent = node.parent
+          loop do
+            break if !top_parent
+            return true if is_system_type?(top_parent)
+
+            top_parent = top_parent.parent
+          end
+
+          false
+        end
+      end
+    end
+  end
+end

+ 5 - 0
.dev/rubocop/default.yml

@@ -492,3 +492,8 @@ Zammad/ExistsDbStrategy:
     - "spec/**/*.rb"
   Exclude:
     - "spec/support/capybara/*.rb"
+
+Zammad/EnsuresTimeZoneAtSpecTopLevel:
+  Enabled: true
+  Include:
+    - 'spec/system/**/*.rb'

+ 1 - 0
.dev/rubocop/rubocop_zammad.rb

@@ -21,3 +21,4 @@ require_relative 'cop/zammad/timezone_default'
 require_relative 'cop/zammad/to_forbid_over_not_to_permit'
 require_relative 'cop/zammad/trigger_from_commit_hooks'
 require_relative 'cop/zammad/update_copyright'
+require_relative 'cop/zammad/ensures_time_zone_at_spec_top_level'

+ 41 - 0
spec/rubocop/cop/zammad/ensures_time_zone_at_spec_top_level_spec.rb

@@ -0,0 +1,41 @@
+# Copyright (C) 2012-2025 Zammad Foundation, https://zammad-foundation.org/
+
+require 'rails_helper'
+require_relative '../../../../.dev/rubocop/cop/zammad/ensures_time_zone_at_spec_top_level'
+
+RSpec.describe RuboCop::Cop::Zammad::EnsuresTimeZoneAtSpecTopLevel, :aggregate_failures, type: :rubocop do
+  context 'when type is system' do
+    it 'allows time zone at top level' do
+      expect_no_offenses(<<~RUBY)
+        RSpec.describe 'Test', type: :system, time_zone: 'Europe/London' do
+          it 'is fine' do
+            example
+          end
+        end
+      RUBY
+    end
+
+    it 'rejects time zone at lower level' do
+      expect_offense(<<~RUBY)
+        RSpec.describe 'Test', type: :system do
+          it 'is fine', time_zone: 'Europe/London' do
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RSpec system tests (aka Capybara) should set custom time zones at top level only
+            example
+          end
+        end
+      RUBY
+    end
+  end
+
+  context 'when type is not system' do
+    it 'allows time zone at lower level if it is not a system/Capybara test' do
+      expect_no_offenses(<<~RUBY)
+        RSpec.describe 'Test' do
+          it 'is fine', time_zone: 'Europe/London' do
+            example
+          end
+        end
+      RUBY
+    end
+  end
+end

+ 2 - 2
spec/system/apps/desktop/form_helpers_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe 'Form helpers', app: :desktop_view, authenticated_as: :agent, db_strategy: :reset, type: :system do
+RSpec.describe 'Form helpers', app: :desktop_view, authenticated_as: :agent, db_strategy: :reset, time_zone: 'Europe/London', type: :system do
   let(:group)       { Group.find_by(name: 'Users') }
   let(:agent)       { create(:agent, groups: [group]) }
   let(:object_name) { 'Ticket' }
@@ -245,7 +245,7 @@ RSpec.describe 'Form helpers', app: :desktop_view, authenticated_as: :agent, db_
     end
   end
 
-  context 'with date and datetime fields', authenticated_as: :authenticate, time_zone: 'Europe/London' do
+  context 'with date and datetime fields', authenticated_as: :authenticate do
     let(:date)     { Date.parse('2022-09-07') }
     let(:datetime) { DateTime.parse('2023-09-07T12:00:00.000Z') }
 

+ 2 - 2
spec/system/apps/mobile/form_helpers_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe 'Form helpers', app: :mobile, authenticated_as: :agent, db_strategy: :reset, type: :system do
+RSpec.describe 'Form helpers', app: :mobile, authenticated_as: :agent, db_strategy: :reset, time_zone: 'Europe/London', type: :system do
   let(:group)       { Group.find_by(name: 'Users') }
   let(:agent)       { create(:agent, groups: [group]) }
   let(:object_name) { 'Ticket' }
@@ -253,7 +253,7 @@ RSpec.describe 'Form helpers', app: :mobile, authenticated_as: :agent, db_strate
     end
   end
 
-  context 'with date and datetime fields', authenticated_as: :authenticate, time_zone: 'Europe/London' do
+  context 'with date and datetime fields', authenticated_as: :authenticate do
     let(:date)     { Date.parse('2022-09-07') }
     let(:datetime) { DateTime.parse('2023-09-07T08:00:00.000Z') }
 

+ 2 - 2
spec/system/apps/mobile_old/tickets/ticket_update_spec.rb

@@ -3,7 +3,7 @@
 require 'rails_helper'
 require 'system/apps/mobile_old/examples/core_workflow_examples'
 
-RSpec.describe 'Mobile > Ticket > Update', app: :mobile, authenticated_as: :agent, type: :system do
+RSpec.describe 'Mobile > Ticket > Update', app: :mobile, authenticated_as: :agent, time_zone: 'Europe/London', type: :system do
   let(:group)    { Group.find_by(name: 'Users') }
   let(:group_2)  { create(:group, name: 'Group 2') }
   let(:owner)    { User.find_by(email: 'agent1@example.com') }
@@ -126,7 +126,7 @@ RSpec.describe 'Mobile > Ticket > Update', app: :mobile, authenticated_as: :agen
         expect(ticket.owner.id).to be(1)
       end
 
-      it 'changing ticket state to pending requires pending time', time_zone: 'Europe/London' do
+      it 'changing ticket state to pending requires pending time' do
         visit "/tickets/#{ticket.id}/information"
 
         wait_for_form_to_settle('form-ticket-edit')

+ 2 - 2
spec/system/manage/calendars_spec.rb

@@ -2,12 +2,12 @@
 
 require 'rails_helper'
 
-RSpec.describe 'Manage > Calendars', type: :system do
+RSpec.describe 'Manage > Calendars', time_zone: 'America/Sao_Paulo', type: :system do
 
   context 'Date' do
     let(:calendar_title) { "test calendar #{SecureRandom.uuid}" }
 
-    it 'show festivity dates correctly far away from UTC', time_zone: 'America/Sao_Paulo' do
+    it 'show festivity dates correctly far away from UTC' do
       visit '/#manage/calendars'
 
       click '.js-new'

+ 3 - 3
spec/system/ticket/create_spec.rb

@@ -5,7 +5,7 @@ require 'rails_helper'
 require 'system/examples/core_workflow_examples'
 require 'system/examples/text_modules_examples'
 
-RSpec.describe 'Ticket Create', type: :system do
+RSpec.describe 'Ticket Create', time_zone: 'Europe/London', type: :system do
 
   context 'when calling without session' do
     describe 'redirect to' do
@@ -229,7 +229,7 @@ RSpec.describe 'Ticket Create', type: :system do
     end
   end
 
-  describe 'object manager attributes default date', time_zone: 'Europe/London' do
+  describe 'object manager attributes default date' do
     before :all do # rubocop:disable RSpec/BeforeAfterAll
       screens = {
         'create_top' => {
@@ -1084,7 +1084,7 @@ RSpec.describe 'Ticket Create', type: :system do
     end
   end
 
-  describe 'Ticket templates are missing pending till option #4318', time_zone: 'Europe/London' do
+  describe 'Ticket templates are missing pending till option #4318' do
 
     shared_examples 'check datetime field' do