From d9c296cdb31c9a3c709ae6e6d09916e140a9366c Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Fri, 16 Aug 2024 16:00:35 +1000
Subject: [PATCH] Stabilise flaky report specs

It looks like we have a new race condition that may only be a problem in
specs. If you trigger one report, it displays via websockets and then
you trigger the next report, there may still be some Javascript active
that displays the first report while the second one is loading. I'm not
sure if users would navigate that fast though.

To minimise the problem, I adjusted the polling to leave more room for
the default websockets response.
---
 app/views/admin/reports/_fallback_display.html.haml |  7 +++++--
 .../admin/reports/enterprise_fee_summaries_spec.rb  |  9 +++------
 .../admin/reports/orders_and_fulfillment_spec.rb    |  2 +-
 spec/system/admin/reports_spec.rb                   | 13 +++++++------
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/app/views/admin/reports/_fallback_display.html.haml b/app/views/admin/reports/_fallback_display.html.haml
index 1538794665..cfac68c742 100644
--- a/app/views/admin/reports/_fallback_display.html.haml
+++ b/app/views/admin/reports/_fallback_display.html.haml
@@ -28,7 +28,7 @@
             }
           });
         } else {
-          setTimeout(tryDownload, 1000);
+          setTimeout(tryDownload, 2000);
         }
       });
     }
@@ -39,6 +39,9 @@
       https://openfoodnetwork.org.au/admin/sidekiq/metrics/ReportJob?period=8h
       https://openfoodnetwork.org.uk/admin/sidekiq/metrics/ReportJob?period=8h
       https://coopcircuits.fr/admin/sidekiq/metrics/ReportJob?period=8h
+
+      But let's leave the timed response to websockets for now and just poll
+      as a backup mechanism.
     */
-    setTimeout(tryDownload, 250);
+    setTimeout(tryDownload, 3000);
   })();
diff --git a/spec/system/admin/reports/enterprise_fee_summaries_spec.rb b/spec/system/admin/reports/enterprise_fee_summaries_spec.rb
index 4d1b70fee7..13bffa16e1 100644
--- a/spec/system/admin/reports/enterprise_fee_summaries_spec.rb
+++ b/spec/system/admin/reports/enterprise_fee_summaries_spec.rb
@@ -93,8 +93,7 @@ RSpec.describe "enterprise fee summaries" do
 
         it "generates file with data for all enterprises" do
           select "CSV"
-          click_on "Go"
-          perform_enqueued_jobs(only: ReportJob)
+          run_report
           click_on "Download Report"
           expect(downloaded_filename).to include ".csv"
           expect(downloaded_content).to have_content(distributor.name)
@@ -118,8 +117,7 @@ RSpec.describe "enterprise fee summaries" do
 
         it "generates file with data for the enterprise" do
           select "CSV"
-          click_on "Go"
-          perform_enqueued_jobs(only: ReportJob)
+          run_report
           click_on "Download Report"
 
           expect(downloaded_filename).to include ".csv"
@@ -154,8 +152,7 @@ RSpec.describe "enterprise fee summaries" do
 
         find("#report_format").click
         select "CSV"
-        click_on "Go"
-        perform_enqueued_jobs(only: ReportJob)
+        run_report
         click_on "Download Report"
 
         expect(downloaded_filename).to include ".csv"
diff --git a/spec/system/admin/reports/orders_and_fulfillment_spec.rb b/spec/system/admin/reports/orders_and_fulfillment_spec.rb
index 1aa808e2fc..7c542ca608 100644
--- a/spec/system/admin/reports/orders_and_fulfillment_spec.rb
+++ b/spec/system/admin/reports/orders_and_fulfillment_spec.rb
@@ -256,12 +256,12 @@ RSpec.describe "Orders And Fulfillment" do
         describe "Totals" do
           before do
             click_link "Order Cycle Supplier Totals"
-            run_report
           end
 
           context "with the header row option not selected" do
             before do
               find("#display_header_row").set(false) # hides the header row
+              run_report
             end
 
             it "displays the report" do
diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb
index 62de258992..b2d86417e8 100644
--- a/spec/system/admin/reports_spec.rb
+++ b/spec/system/admin/reports_spec.rb
@@ -139,14 +139,15 @@ RSpec.describe '
       # Unlocking the breakpoint will continue execution of the controller.
       breakpoint.unlock
 
-      # We have to wait to be sure that the "loading" spinner won't appear
-      # within the next half second. The default wait time would wait for
-      # 10 seconds which slows down the spec.
-      using_wait_time 0.5 do
-        page.has_selector? ".loading"
-      end
+      # Now the controller response will show the loading spinner again and
+      # the fallback mechanism will render the report later.
+      expect(page).to have_selector ".loading"
+
+      # Wait for the fallback mechanism:
+      sleep 3
 
       expect(page).not_to have_selector ".loading"
+      expect(page).to have_content "First Name Last Name Billing Address Email"
     end
   end