From c7ae922892362a861748c7a3c6b1f9391b66ff4c Mon Sep 17 00:00:00 2001 From: Lucas Carlson Date: Mon, 10 Aug 2026 17:00:10 -0700 Subject: [PATCH] fix: clear every actor-owned table in reset_actors! The helper deleted 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, rows survive a reset with an instance_id pointing at nothing and the next test reads them as its own. Reported as reminders leaking, and reminders are where it surfaces first because they outlive the message that created them, but the same reset left messages, ready and claimed mailbox rows, effects, broadcasts, and dead letters behind too. Deleting each table explicitly costs nothing and removes the dependency on referential integrity. Order is children before parents so it is correct whether or not the cascade fires. The existing test asserted only that instances and processes were empty, which is the same blind spot one level up. It now populates every actor-owned table, resets with foreign keys switched off so the cascade cannot cover for an incomplete list, and separately asserts the list covers every Solid Objects table so a table added later cannot be omitted silently. --- CHANGELOG.md | 14 +++ docs/development.md | 8 +- lib/solid_objects/test_helper.rb | 25 +++- .../lib/solid_objects/test_helper.rbs | 12 ++ test/integration/public_test_helper_test.rb | 117 ++++++++++++++++++ 5 files changed, 174 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4507d11..02114d9 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/development.md b/docs/development.md index b137403..01aa965 100644 --- a/docs/development.md +++ b/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 diff --git a/lib/solid_objects/test_helper.rb b/lib/solid_objects/test_helper.rb index b98bd90..6652a9c 100644 --- a/lib/solid_objects/test_helper.rb +++ b/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 diff --git a/sig/generated/lib/solid_objects/test_helper.rbs b/sig/generated/lib/solid_objects/test_helper.rbs index b8a56fe..de8d9e1 100644 --- a/sig/generated/lib/solid_objects/test_helper.rbs +++ b/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 diff --git a/test/integration/public_test_helper_test.rb b/test/integration/public_test_helper_test.rb index 8aaa76b..6743380 100644 --- a/test/integration/public_test_helper_test.rb +++ b/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