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