Merge pull request #31 from cardmagic/agent/test-helper-isolation

fix: clear every actor-owned table in reset_actors!

Author: cardmagic

CHANGELOG.md

@@ -1,5 +1,19 @@
 # Changelog
 
+## Unreleased
+
+- Delete every actor-owned row in `SolidObjects::TestHelper#reset_actors!`. It
+  deleted actor instances and processes and left the other seven tables to the
+  database cascade. That cascade is not enforced everywhere: SQLite has to be
+  asked for foreign keys, MySQL has to be on InnoDB, and a host application may
+  have stripped the constraints out of the copied migration. Where it does not
+  fire, messages, ready and claimed mailbox rows, reminders, effects,
+  broadcasts, and dead letters all survived into the next test with an
+  `instance_id` pointing at nothing, so a test reading any of them saw another
+  test's rows and failed depending on order. Reported as reminders leaking,
+  which is where it surfaces first because reminders outlive the message that
+  created them.
+
 ## 0.10.2 - 2026-08-10
 
 - Load the mailbox when the gem is required. `SolidObjects::Mailbox` was

docs/development.md

@@ -66,7 +66,13 @@ Pass `roles: [:actors]` when a test intentionally wants to leave outboxes or
 reminders pending.
 
 `SolidObjects::TestHelper.reset_actors!` is also available for explicit suite
-boundaries.
+boundaries. It deletes every actor-owned row itself rather than deleting actor
+instances and letting the database cascade remove the rest: SQLite has to be
+asked for foreign keys, MySQL has to be on InnoDB, and a host application may
+have stripped the constraints out of the copied migration. Where the cascade
+does not fire, a row that survives a reset carries an `instance_id` pointing at
+nothing, and the next test that reads reminders or dead letters sees another
+test's data.
 
 ## Inline RBS
 

lib/solid_objects/test_helper.rb

@@ -12,12 +12,35 @@ def included(test_case)
         test_case.teardown { reset_actors! }
       end
 
+      # Deleting instances alone left every other actor-owned row to the
+      # database cascade. That cascade is not enforced everywhere: SQLite has
+      # to be asked for foreign keys, MySQL has to be on InnoDB, and a host
+      # application may have stripped the constraints out of the copied
+      # migration. Where it does not fire, rows survive into the next test with
+      # an instance_id pointing at nothing, and a test that reads them sees
+      # another test's data. Deleting each table costs nothing and does not
+      # depend on referential integrity.
       # @rbs () -> void
       def reset_actors!
         SolidObjects.reset_caller_process!
-        Instance.delete_all
+        actor_owned_models.each(&:delete_all)
         Process.delete_all
       end
+
+      # Children first, so the order is safe whether or not the cascade fires.
+      # @rbs () -> Array[Class]
+      def actor_owned_models
+        [
+          DeadLetter,
+          ClaimedMessage,
+          ReadyMessage,
+          Broadcast,
+          Effect,
+          Reminder,
+          Message,
+          Instance
+        ]
+      end
     end
 
     # @rbs () -> void

sig/generated/lib/solid_objects/test_helper.rbs

@@ -5,9 +5,21 @@ module SolidObjects
     # @rbs (Class) -> void
     def self.included: (Class) -> void
 
+    # Deleting instances alone left every other actor-owned row to the
+    # database cascade. That cascade is not enforced everywhere: SQLite has
+    # to be asked for foreign keys, MySQL has to be on InnoDB, and a host
+    # application may have stripped the constraints out of the copied
+    # migration. Where it does not fire, rows survive into the next test with
+    # an instance_id pointing at nothing, and a test that reads them sees
+    # another test's data. Deleting each table costs nothing and does not
+    # depend on referential integrity.
     # @rbs () -> void
     def self.reset_actors!: () -> void
 
+    # Children first, so the order is safe whether or not the cascade fires.
+    # @rbs () -> Array[Class]
+    def self.actor_owned_models: () -> Array[Class]
+
     # @rbs () -> void
     def reset_actors!: () -> void
 

test/integration/public_test_helper_test.rb

@@ -48,6 +48,37 @@ class ActorTestCase < ActiveSupport::TestCase
     assert_empty SolidObjects::Process.all
   end
 
+  # The helper used to delete instances and let the database cascade remove
+  # everything else. Where the cascade does not fire, rows survive into the
+  # next test pointing at an instance that no longer exists, and a test that
+  # reads them sees another test's data.
+  test "reset actors clears actor-owned rows without the database cascade" do
+    skip unless database_family == :sqlite
+
+    instance = create_actor_owned_rows
+    without_foreign_keys do
+      SolidObjects::TestHelper.reset_actors!
+    end
+
+    remaining = SolidObjects::TestHelper.actor_owned_models.reject { |model| model.count.zero? }
+    assert_empty remaining.map(&:table_name),
+      "these tables survived a reset that could not rely on the cascade"
+    refute_nil instance
+  end
+
+  # A table added later is only covered if the helper is told about it, and the
+  # cascade would hide the omission on every database that enforces it.
+  test "every actor-owned table is in the reset list" do
+    owned = SolidObjects::Record.connection.tables
+      .grep(/\Asolid_objects_/)
+      .reject { |table| table == "solid_objects_test_domain_records" }
+      .sort
+    listed = SolidObjects::TestHelper.actor_owned_models.map(&:table_name)
+
+    assert_equal owned, (listed + [ SolidObjects::Process.table_name ]).sort,
+      "a Solid Objects table is missing from reset_actors!"
+  end
+
   test "drain actor messages processes queued work deterministically" do
     test_case = ActorTestCase.new("unused")
     message_reference = HelperActor.ref("async").async(:increment)
@@ -90,4 +121,90 @@ class ActorTestCase < ActiveSupport::TestCase
 
     assert_includes error.message, "unknown"
   end
+
+  private
+
+  # One row in every actor-owned table, so an omission from the reset list
+  # shows up as a surviving table rather than as a passing test.
+  def create_actor_owned_rows
+    now = Time.current
+    instance = SolidObjects::Instance.create!(
+      actor_type: "reset-probe",
+      actor_id: "one",
+      state: {},
+      state_version: 1
+    )
+    message = SolidObjects::Message.create!(
+      instance:,
+      actor_type: instance.actor_type,
+      actor_id: instance.actor_id,
+      message_name: "noop",
+      message_kind: "async",
+      arguments: {},
+      sequence: 1,
+      max_attempts: 1,
+      request_id: SecureRandom.uuid,
+      enqueued_at: now,
+      available_at: now
+    )
+    SolidObjects::ReadyMessage.create!(message:, instance:, sequence: 1, available_at: now)
+    SolidObjects::ClaimedMessage.create!(
+      message:,
+      instance:,
+      activation_generation: 1,
+      claimed_at: now
+    )
+    SolidObjects::Reminder.create!(
+      instance:,
+      actor_type: instance.actor_type,
+      actor_id: instance.actor_id,
+      name: "probe",
+      message_name: "noop",
+      arguments: {},
+      next_run_at: now,
+      status: "scheduled"
+    )
+    SolidObjects::Effect.create!(
+      instance:,
+      message:,
+      effect_id: SecureRandom.uuid,
+      name: "probe",
+      arguments: {},
+      max_attempts: 1,
+      available_at: now
+    )
+    SolidObjects::Broadcast.create!(
+      instance:,
+      message:,
+      broadcast_id: SecureRandom.uuid,
+      observable_name: "probe",
+      value: {},
+      state_version: 1,
+      activation_generation: 1,
+      available_at: now
+    )
+    SolidObjects::DeadLetter.create!(
+      instance:,
+      message:,
+      actor_type: instance.actor_type,
+      actor_id: instance.actor_id,
+      message_name: "noop",
+      arguments: {},
+      attempts: 1,
+      exception_class: "RuntimeError",
+      exception_message: "probe",
+      backtrace: [],
+      first_failed_at: now,
+      last_failed_at: now
+    )
+    instance
+  end
+
+  def without_foreign_keys
+    connection = SolidObjects::Record.connection
+    connection.execute("PRAGMA foreign_keys = OFF")
+    yield
+  ensure
+    connection.execute("PRAGMA foreign_keys = ON")
+  end
 end