CLDC-4115: Fix flaky tests (#3149)

* CLDC-4115: Fix flaky collection_time_helper test

if the max date was returned test would fail

use date objects instead

* CLDC-4115: Ensure page is loaded before running accessibility tests

unclear whether this will make a difference, will push now and run the pipeline a couple times

* CLDC-4115: Search for post merge scheme by name

rather than relying on it being first

* Revert "CLDC-4115: Ensure page is loaded before running accessibility tests"

This reverts commit ff1ef0a466a1ca4f70e44f6d584c666c975184e1.

doesnt seem to work

* CLDC-4115: Ensure question uses default label logic

* CLDC-4115: Ensure form is loaded correctly before testing filters

* CLDC-4115: Ensure form is loaded correctly before testing locations

* CLDC-4115: Force page_load timeout to 10 seconds

* fixup! CLDC-4115: Search for post merge scheme by name

* fixup! CLDC-4115: Ensure form is loaded correctly before testing filters

remove 23/24 tests

make tests no longer year specific

* CLDC-4115: Remove order of csv requirement from count duplicates rake test

app/helpers/collection_time_helper.rb

@@ -54,6 +54,10 @@ module CollectionTimeHelper
     current_collection_start_year - 1
   end
 
+  def previous_collection_end_year
+    current_collection_end_year - 1
+  end
+
   def previous_collection_start_date
     current_collection_start_date - 1.year
   end
@@ -62,6 +66,10 @@ module CollectionTimeHelper
     current_collection_start_year - 2
   end
 
+  def archived_collection_end_year
+    current_collection_end_year - 2
+  end
+
   def previous_collection_new_logs_end_date
     FormHandler.instance.lettings_form_for_start_year(previous_collection_start_year).new_logs_end_date
   end

spec/features/accessibility_spec.rb

@@ -21,6 +21,22 @@ RSpec.describe "Accessibility", js: true do
   end
 
   before do
+    # see https://github.com/dequelabs/axe-core-gems/issues/386#issuecomment-2135927504 for why this is needed
+    # axe-core will normally use 1 second page load timeout.
+    # see https://github.com/dequelabs/axe-core-gems/blob/87632f5e8e947214f10e35b57ea5e2c327f20612/packages/axe-core-api/lib/axe/api/run.rb#L36
+    # this causes timeout errors for slower pages
+    # so we override the page load timeout to 10 seconds for feature specs
+    # this is for the a11y tests, we can only scope to feature specs
+    page.driver.browser.manage.timeouts.instance_eval do
+      def page_load=(*)
+        # no-op don't want axe-core changing this
+      end
+
+      def page_load
+        10 # seconds
+      end
+    end
+
     allow(Storage::S3Service).to receive(:new).and_return(storage_service)
     allow(storage_service).to receive(:configuration).and_return(OpenStruct.new(bucket_name: "core-test-collection-resources"))
     allow(user).to receive(:need_two_factor_authentication?).and_return(false)

spec/helpers/check_answers_helper_spec.rb

@@ -41,7 +41,8 @@ RSpec.describe CheckAnswersHelper do
 
   describe "#get_answer_label" do
     context "when unanswered and bulk upload" do
-      let(:question) { log.form.questions.reject { |q| log.optional_fields.include?(q.id) }.sample }
+      # make sure to not include questions that override the answer label
+      let(:question) { log.form.questions.reject { |q| log.optional_fields.include?(q.id) || q.answer_label(log, current_user).present? }.sample }
       let(:bulk_upload) { create(:bulk_upload) }
       let(:log) { create(:sales_log, creation_method: "bulk upload", bulk_upload:) }
 

spec/helpers/collection_time_helper_spec.rb

@@ -152,7 +152,7 @@ RSpec.describe CollectionTimeHelper do
       it "returns a different date within the collection year" do
         result = generate_different_date_within_collection_year(date)
         expect(result).not_to eq(date)
-        expect(result).to be_between(Time.zone.local(2024, 4, 1), Time.zone.local(2025, 3, 31)).inclusive
+        expect(result).to be_between(Date.new(2024, 4, 1), Date.new(2025, 3, 31)).inclusive
       end
     end
 
@@ -162,7 +162,7 @@ RSpec.describe CollectionTimeHelper do
       it "returns a different date within the collection year" do
         result = generate_different_date_within_collection_year(date)
         expect(result).not_to eq(date)
-        expect(result).to be_between(Time.zone.local(2024, 4, 1), Time.zone.local(2025, 3, 31)).inclusive
+        expect(result).to be_between(Date.new(2024, 4, 1), Date.new(2025, 3, 31)).inclusive
       end
     end
 
@@ -170,7 +170,7 @@ RSpec.describe CollectionTimeHelper do
       it "ignores the override and returns a different date within the collection year" do
         result = generate_different_date_within_collection_year(date, start_date_override: Time.zone.local(2023, 12, 31))
         expect(result).not_to eq(date)
-        expect(result).to be_between(Time.zone.local(2024, 4, 1), Time.zone.local(2025, 3, 31)).inclusive
+        expect(result).to be_between(Date.new(2024, 4, 1), Date.new(2025, 3, 31)).inclusive
       end
     end
 
@@ -178,7 +178,7 @@ RSpec.describe CollectionTimeHelper do
       it "ignores the override and returns a different date within the collection year" do
         result = generate_different_date_within_collection_year(date, end_date_override: Time.zone.local(2025, 12, 1))
         expect(result).not_to eq(date)
-        expect(result).to be_between(Time.zone.local(2024, 4, 1), Time.zone.local(2025, 3, 31)).inclusive
+        expect(result).to be_between(Date.new(2024, 4, 1), Date.new(2025, 3, 31)).inclusive
       end
     end
 
@@ -186,7 +186,7 @@ RSpec.describe CollectionTimeHelper do
       it "returns a different date within the overridden range" do
         result = generate_different_date_within_collection_year(date, start_date_override: Time.zone.local(2024, 8, 1), end_date_override: Time.zone.local(2024, 9, 1))
         expect(result).not_to eq(date)
-        expect(result).to be_between(Time.zone.local(2024, 8, 1), Time.zone.local(2024, 9, 1)).inclusive
+        expect(result).to be_between(Date.new(2024, 8, 1), Date.new(2024, 9, 1)).inclusive
       end
     end
 
@@ -194,7 +194,7 @@ RSpec.describe CollectionTimeHelper do
       it "ignores the start_date_override and returns a different date within the collection year" do
         result = generate_different_date_within_collection_year(date, start_date_override: Time.zone.local(2023, 12, 31), end_date_override: Time.zone.local(2024, 5, 1))
         expect(result).not_to eq(date)
-        expect(result).to be_between(Time.zone.local(2024, 4, 1), Time.zone.local(2024, 5, 1)).inclusive
+        expect(result).to be_between(Date.new(2024, 4, 1), Date.new(2024, 5, 1)).inclusive
       end
     end
 
@@ -202,7 +202,7 @@ RSpec.describe CollectionTimeHelper do
       it "ignores the end_date_override and returns a different date within the collection year" do
         result = generate_different_date_within_collection_year(date, start_date_override: Time.zone.local(2025, 3, 1), end_date_override: Time.zone.local(2025, 12, 1))
         expect(result).not_to eq(date)
-        expect(result).to be_between(Time.zone.local(2025, 3, 1), Time.zone.local(2025, 3, 31)).inclusive
+        expect(result).to be_between(Date.new(2025, 3, 1), Date.new(2025, 3, 31)).inclusive
       end
     end
 
@@ -210,7 +210,7 @@ RSpec.describe CollectionTimeHelper do
       it "ignores both overrides and returns a different date within the collection year" do
         result = generate_different_date_within_collection_year(date, start_date_override: Time.zone.local(2023, 12, 31), end_date_override: Time.zone.local(2025, 12, 1))
         expect(result).not_to eq(date)
-        expect(result).to be_between(Time.zone.local(2024, 4, 1), Time.zone.local(2025, 3, 31)).inclusive
+        expect(result).to be_between(Date.new(2024, 4, 1), Date.new(2025, 3, 31)).inclusive
       end
     end
   end

spec/helpers/filters_helper_spec.rb

@@ -478,136 +478,100 @@ RSpec.describe FiltersHelper do
   end
 
   describe "#collection_year_options" do
-    context "with 23/24 as the current collection year" do
+    context "and in crossover period" do
       before do
-        allow(Time).to receive(:now).and_return(Time.zone.local(2023, 5, 1))
+        allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(true)
       end
 
-      context "and in crossover period" do
-        before do
-          allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(true)
-        end
-
-        it "has the correct options" do
-          expect(collection_year_options).to eq(
-            {
-              "2023" => "2023 to 2024", "2022" => "2022 to 2023", "2021" => "2021 to 2022"
-            },
-          )
-        end
-      end
-
-      context "and not in crossover period" do
-        before do
-          allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(false)
-        end
-
-        it "has the correct options" do
-          expect(collection_year_options).to eq(
-            {
-              "2023" => "2023 to 2024", "2022" => "2022 to 2023"
-            },
-          )
-        end
+      it "has the correct options" do
+        expect(collection_year_options).to eq(
+          {
+            current_collection_start_year.to_s => "#{current_collection_start_year} to #{current_collection_end_year}",
+            previous_collection_start_year.to_s => "#{previous_collection_start_year} to #{previous_collection_end_year}",
+            archived_collection_start_year.to_s => "#{archived_collection_start_year} to #{archived_collection_end_year}",
+          },
+        )
       end
     end
 
-    context "with 24/25 as the current collection year" do
+    context "and not in crossover period" do
       before do
-        allow(Time).to receive(:now).and_return(Time.zone.local(2024, 5, 1))
+        allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(false)
       end
 
-      context "and in crossover period" do
-        before do
-          allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(true)
-        end
-
-        it "has the correct options" do
-          expect(collection_year_options).to eq(
-            {
-              "2024" => "2024 to 2025", "2023" => "2023 to 2024", "2022" => "2022 to 2023"
-            },
-          )
-        end
+      it "has the correct options" do
+        expect(collection_year_options).to eq(
+          {
+            current_collection_start_year.to_s => "#{current_collection_start_year} to #{current_collection_end_year}",
+            previous_collection_start_year.to_s => "#{previous_collection_start_year} to #{previous_collection_end_year}",
+          },
+        )
       end
 
-      context "and not in crossover period" do
+      context "with future form use turned on" do
         before do
-          allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(false)
+          allow(FeatureToggle).to receive(:allow_future_form_use?).and_return(true)
         end
 
-        it "has the correct options" do
+        it "includes next year in the options" do
           expect(collection_year_options).to eq(
             {
-              "2024" => "2024 to 2025", "2023" => "2023 to 2024"
+              next_collection_start_year.to_s => "#{next_collection_start_year} to #{next_collection_end_year}",
+              current_collection_start_year.to_s => "#{current_collection_start_year} to #{current_collection_end_year}",
+              previous_collection_start_year.to_s => "#{previous_collection_start_year} to #{previous_collection_end_year}",
             },
           )
         end
-
-        context "with future form use turned on" do
-          before do
-            allow(FeatureToggle).to receive(:allow_future_form_use?).and_return(true)
-          end
-
-          it "includes next year in the options" do
-            expect(collection_year_options).to eq(
-              {
-                "2025" => "2025 to 2026", "2024" => "2024 to 2025", "2023" => "2023 to 2024"
-              },
-            )
-          end
-        end
       end
     end
   end
 
   describe "#collection_year_radio_options" do
-    context "with 23/24 as the current collection year" do
+    context "and in crossover period" do
       before do
-        allow(Time).to receive(:now).and_return(Time.zone.local(2023, 5, 1))
-      end
-
-      context "and in crossover period" do
-        before do
-          allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(true)
-        end
-
-        it "has the correct options" do
-          expect(collection_year_radio_options).to eq(
-            {
-              "2023" => { label: "2023 to 2024" }, "2022" => { label: "2022 to 2023" }, "2021" => { label: "2021 to 2022" }
-            },
-          )
-        end
+        allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(true)
       end
 
-      context "and not in crossover period" do
-        before do
-          allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(false)
-        end
-
-        it "has the correct options" do
-          expect(collection_year_radio_options).to eq(
-            {
-              "2023" => { label: "2023 to 2024" }, "2022" => { label: "2022 to 2023" }
-            },
-          )
-        end
+      it "has the correct options" do
+        expect(collection_year_radio_options).to eq(
+          {
+            current_collection_start_year.to_s => { label: "#{current_collection_start_year} to #{current_collection_end_year}" },
+            previous_collection_start_year.to_s => { label: "#{previous_collection_start_year} to #{previous_collection_end_year}" },
+            archived_collection_start_year.to_s => { label: "#{archived_collection_start_year} to #{archived_collection_end_year}" },
+          },
+        )
       end
     end
 
-    context "with 24/25 as the current collection year" do
+    context "and not in crossover period" do
       before do
-        allow(Time).to receive(:now).and_return(Time.zone.local(2024, 5, 1))
+        allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(false)
       end
 
       it "has the correct options" do
         expect(collection_year_radio_options).to eq(
           {
-            "2024" => { label: "2024 to 2025" }, "2023" => { label: "2023 to 2024" }, "2022" => { label: "2022 to 2023" }
+            current_collection_start_year.to_s => { label: "#{current_collection_start_year} to #{current_collection_end_year}" },
+            previous_collection_start_year.to_s => { label: "#{previous_collection_start_year} to #{previous_collection_end_year}" },
           },
         )
       end
+
+      context "with future form use turned on" do
+        before do
+          allow(FeatureToggle).to receive(:allow_future_form_use?).and_return(true)
+        end
+
+        it "includes next year in the options" do
+          expect(collection_year_radio_options).to eq(
+            {
+              next_collection_start_year.to_s => { label: "#{next_collection_start_year} to #{next_collection_end_year}" },
+              current_collection_start_year.to_s => { label: "#{current_collection_start_year} to #{current_collection_end_year}" },
+              previous_collection_start_year.to_s => { label: "#{previous_collection_start_year} to #{previous_collection_end_year}" },
+            },
+          )
+        end
+      end
     end
   end
 

spec/helpers/locations_helper_spec.rb

@@ -59,6 +59,7 @@ RSpec.describe LocationsHelper do
     before do
       allow(Time).to receive(:now).and_return(today)
       allow(FormHandler.instance).to receive(:lettings_in_crossover_period?).and_return(true)
+      Singleton.__init__(FormHandler)
     end
 
     it "returns one active period without to date" do

spec/lib/tasks/count_duplicates_spec.rb

@@ -101,7 +101,12 @@ RSpec.describe "count_duplicates" do
         end
 
         it "creates a csv with correct duplicate numbers" do
-          expect(storage_service).to receive(:write_file).with(/location-duplicates-.*\.csv/, "\uFEFFOrganisation id,Duplicate sets within individual schemes,Duplicate locations within individual schemes,All duplicate sets,All duplicates\n#{organisation.id},3,6,4,9\n#{organisation2.id},2,7,2,7\n")
+          expect(storage_service).to receive(:write_file).with(/location-duplicates-.*\.csv/, satisfy do |s|
+            s.start_with?("\uFEFFOrganisation id,Duplicate sets within individual schemes,Duplicate locations within individual schemes,All duplicate sets,All duplicates") &&
+              s.include?("#{organisation.id},3,6,4,9") &&
+              s.include?("#{organisation2.id},2,7,2,7") &&
+              s.count("\n") == 3
+          end)
           expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}")
           task.invoke
         end
@@ -149,7 +154,12 @@ RSpec.describe "count_duplicates" do
         end
 
         it "creates a csv with correct duplicate numbers" do
-          expect(storage_service).to receive(:write_file).with(/scheme-duplicates-.*\.csv/, "\uFEFFOrganisation id,Number of duplicate sets,Total duplicate schemes\n#{organisation.id},2,5\n#{organisation2.id},1,5\n")
+          expect(storage_service).to receive(:write_file).with(/scheme-duplicates-.*\.csv/, satisfy do |s|
+            s.start_with?("\uFEFFOrganisation id,Number of duplicate sets,Total duplicate schemes") &&
+              s.include?("#{organisation.id},2,5") &&
+              s.include?("#{organisation2.id},1,5") &&
+              s.count("\n") == 3
+          end)
           expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}")
           task.invoke
         end
@@ -211,7 +221,12 @@ RSpec.describe "count_duplicates" do
         end
 
         it "creates a csv with correct duplicate numbers" do
-          expect(storage_service).to receive(:write_file).with(/location-duplicates-.*\.csv/, "\uFEFFOrganisation id,Duplicate sets within individual schemes,Duplicate locations within individual schemes,All duplicate sets,All duplicates\n#{organisation.id},3,6,4,9\n#{organisation2.id},2,7,2,7\n")
+          expect(storage_service).to receive(:write_file).with(/location-duplicates-.*\.csv/, satisfy do |s|
+            s.start_with?("\uFEFFOrganisation id,Duplicate sets within individual schemes,Duplicate locations within individual schemes,All duplicate sets,All duplicates")
+            s.include?("#{organisation.id},3,6,4,9") &&
+            s.include?("#{organisation2.id},2,7,2,7") &&
+            s.count("\n") == 3
+          end)
           expect(Rails.logger).to receive(:info).with("Download URL: #{test_url}")
           task.invoke
         end

spec/services/merge/merge_organisations_service_spec.rb

@@ -546,9 +546,12 @@ RSpec.describe Merge::MergeOrganisationsService do
             merging_organisation.reload
             expect(absorbing_organisation.owned_lettings_logs.count).to eq(2)
             expect(absorbing_organisation.managed_lettings_logs.count).to eq(1)
-            expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).scheme).to eq(absorbing_organisation.owned_schemes.first)
-            expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).location).to eq(absorbing_organisation.owned_schemes.first.locations.first)
-            expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(absorbing_organisation.owned_schemes.first)
+
+            absorbed_active_scheme = absorbing_organisation.owned_schemes.find_by(service_name: scheme.service_name)
+            absorbed_active_location = absorbed_active_scheme.locations.find_by(postcode: location.postcode)
+            expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).scheme).to eq(absorbed_active_scheme)
+            expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log.id).location).to eq(absorbed_active_location)
+            expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).scheme).to eq(absorbed_active_scheme)
             expect(absorbing_organisation.owned_lettings_logs.find(owned_lettings_log_no_location.id).location).to eq(nil)
           end
 
@@ -814,12 +817,12 @@ RSpec.describe Merge::MergeOrganisationsService do
               it "moves active schemes and locations to absorbing organisation" do
                 expect(absorbing_organisation.owned_schemes.count).to eq(2)
 
-                expect(absorbing_organisation.owned_schemes.first.locations.map(&:postcode)).to match_array([location, deactivated_location, location_without_startdate, location_with_past_startdate, location_with_future_startdate].map(&:postcode))
-                expect(absorbing_organisation.owned_schemes.first.locations.find_by(postcode: location_without_startdate.postcode).startdate).to eq(Time.zone.yesterday)
-                expect(absorbing_organisation.owned_schemes.first.locations.find_by(postcode: location_with_past_startdate.postcode).startdate).to eq(Time.zone.yesterday)
-                expect(absorbing_organisation.owned_schemes.first.locations.find_by(postcode: location_with_future_startdate.postcode).startdate.to_date).to eq(Time.zone.today + 2.months)
                 absorbed_active_scheme = absorbing_organisation.owned_schemes.find_by(service_name: scheme.service_name)
                 absorbed_active_location = absorbed_active_scheme.locations.find_by(postcode: location.postcode)
+                expect(absorbed_active_scheme.locations.map(&:postcode)).to match_array([location, deactivated_location, location_without_startdate, location_with_past_startdate, location_with_future_startdate].map(&:postcode))
+                expect(absorbed_active_scheme.locations.find_by(postcode: location_without_startdate.postcode).startdate).to eq(Time.zone.yesterday)
+                expect(absorbed_active_scheme.locations.find_by(postcode: location_with_past_startdate.postcode).startdate).to eq(Time.zone.yesterday)
+                expect(absorbed_active_scheme.locations.find_by(postcode: location_with_future_startdate.postcode).startdate.to_date).to eq(Time.zone.today + 2.months)
                 expect(absorbed_active_scheme.service_name).to eq(scheme.service_name)
                 expect(absorbed_active_scheme.old_id).to be_nil
                 expect(absorbed_active_scheme.old_visible_id).to be_nil