From 460553281164b9ea84368abd5d8f457e59cc691d Mon Sep 17 00:00:00 2001
From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com>
Date: Mon, 2 Mar 2026 13:48:53 +0000
Subject: [PATCH 1/2] Stop swallowing errors from tasks

We have had lots of intermittent test failures on these tasks. These tasks are all swallowing their errors which makes it difficult to debug.

We could make the tests output the rails log to standard out or make the log available after but this will make the output hard to match up to the individual test.

Generally we shouldn't swallow errors like this as it makes it difficult to debug and easy for tests to pass for the wrong reasons.

The transaction will work as before as any error raised in a transaction will call a rollback.
---
 lib/tasks/for_education.rake | 12 ------------
 lib/tasks/test_seeds.rake    |  7 -------
 2 files changed, 19 deletions(-)

diff --git a/lib/tasks/for_education.rake b/lib/tasks/for_education.rake
index cc88b75c5..b148eda76 100644
--- a/lib/tasks/for_education.rake
+++ b/lib/tasks/for_education.rake
@@ -41,9 +41,6 @@ namespace :for_education do
       School.find(school_id).destroy
 
       Rails.logger.info 'Done...'
-    rescue StandardError => e
-      Rails.logger.error "Failed: #{e.message}"
-      raise ActiveRecord::Rollback
     end
   end
 
@@ -60,9 +57,6 @@ namespace :for_education do
       create_school(creator_id, TEST_SCHOOL)
 
       Rails.logger.info 'Done...'
-    rescue StandardError => e
-      Rails.logger.error "Failed: #{e.message}"
-      raise ActiveRecord::Rollback
     end
   end
 
@@ -80,9 +74,6 @@ namespace :for_education do
       school = create_school(creator_id, TEST_SCHOOL)
       verify_school(school)
       Rails.logger.info 'Done...'
-    rescue StandardError => e
-      Rails.logger.error "Failed: #{e.message}"
-      raise ActiveRecord::Rollback
     end
   end
 
@@ -110,9 +101,6 @@ namespace :for_education do
         create_project(creator_id, school, lesson)
       end
       Rails.logger.info 'Done...'
-    rescue StandardError => e
-      Rails.logger.error "Failed: #{e.message}"
-      raise ActiveRecord::Rollback
     end
   end
 end
diff --git a/lib/tasks/test_seeds.rake b/lib/tasks/test_seeds.rake
index a4acc95c2..1ea9cac9b 100644
--- a/lib/tasks/test_seeds.rake
+++ b/lib/tasks/test_seeds.rake
@@ -39,10 +39,6 @@ namespace :test_seeds do
       School.where(id: school_ids).destroy_all
 
       Rails.logger.info 'Done...'
-    rescue StandardError => e
-      pp "Failed: #{e.message}"
-      Rails.logger.error "Failed: #{e.message}"
-      raise ActiveRecord::Rollback
     end
   end
 
@@ -74,9 +70,6 @@ namespace :test_seeds do
         end
       end
       Rails.logger.info 'Done...'
-    rescue StandardError => e
-      Rails.logger.error "Failed: #{e.message}"
-      raise ActiveRecord::Rollback
     end
   end
 end

From b3150886a9468e69570097a2dc9a00929215a86d Mon Sep 17 00:00:00 2001
From: Chris Zetter <253059100+zetter-rpf@users.noreply.github.com>
Date: Mon, 2 Mar 2026 13:58:58 +0000
Subject: [PATCH 2/2] Only destroy the school if it exists

The previous commit caused of the for education tests to fail as it was trying to destroy a school that didn't exist.

This might have been the cause of the intermittent failures as previously if the school didn't exist (but other data did) it would rollback and restore the other data.
---
 lib/tasks/for_education.rake | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/tasks/for_education.rake b/lib/tasks/for_education.rake
index b148eda76..e9a672db8 100644
--- a/lib/tasks/for_education.rake
+++ b/lib/tasks/for_education.rake
@@ -38,7 +38,7 @@ namespace :for_education do
       SchoolClass.where(id: [school_class_ids]).destroy_all
 
       # Destroy the school
-      School.find(school_id).destroy
+      School.where(id: school_id).destroy_all
 
       Rails.logger.info 'Done...'
     end