From 546d62968e68ed83972f85bfdc88df185bfba3f7 Mon Sep 17 00:00:00 2001 From: Ngan Pham Date: Mon, 3 Aug 2026 15:45:42 -0700 Subject: [PATCH] Store exposed records as class/id pairs instead of the records `Definition#expose` held the ActiveRecord objects it was given, and the registry holds every fixture for the life of the process, so each fixture kept its whole object graph alive until the suite ended -- attributes and any warmed association caches included. In a large suite that grows monotonically as more spec files execute. Only `Cache#save` ever read `exposed`, and only to convert it into the class/id pairs the cache stores, so `expose` now does that conversion up front and never retains a record. `FileCache#serialize_exposed` moves to `Definition` as a private method; it never touched a file, and it was not the inverse of anything on `FileCache` -- `FileCache#read` converts `{"User" => id}` back to `{User => id}` because `JSON.generate` does the class-to-string half. Measured at 300 fixtures: retained heap 43.9 MB -> 26.5 MB, live objects 753,181 -> 622,166. Per-fixture retained growth drops from ~101 KB to ~43 KB. Because a record's id is now read when it is exposed rather than at the end of the definition, exposing an unsaved record would store a nil id and silently resolve to nil in tests. Exposing a record that is not persisted now raises FixtureKit::UnpersistedRecordError, which also covers records inside an exposed collection and destroyed records. The one case that cannot be detected is a collection appended to after being exposed; that is documented in docs/reference.md. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PQjkiuuGX2t3TSZKpzqpPv --- docs/reference.md | 31 ++++++++++ lib/fixture_kit.rb | 1 + lib/fixture_kit/cache.rb | 2 +- lib/fixture_kit/definition.rb | 21 ++++++- lib/fixture_kit/file_cache.rb | 10 --- spec/unit/definition_spec.rb | 104 ++++++++++++++++++++++++++++---- spec/unit/file_cache_spec.rb | 19 ------ spec/unit/fixture_cache_spec.rb | 23 +++++++ 8 files changed, 169 insertions(+), 42 deletions(-) diff --git a/docs/reference.md b/docs/reference.md index 2a9b6c1..d9eb4fd 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -78,6 +78,36 @@ end `Definition#expose(**records)`: - Exposed names become repository methods. - Duplicate exposed names raise `FixtureKit::DuplicateNameError`. +- Records are captured as class/id pairs at the moment `expose` is called, not + at the end of the definition. The record objects themselves are not retained. +- Exposing a record that is not persisted raises + `FixtureKit::UnpersistedRecordError`. This applies to records inside an + exposed collection as well, and to records that have been destroyed. + +Because capture happens when `expose` is called, expose a record only once it +has been saved: + +```ruby +FixtureKit.define do + user = User.new(name: "Alice") + expose(user: user) # raises FixtureKit::UnpersistedRecordError + user.save! +end +``` + +The same timing applies to collections, but an emptied or later-appended +collection cannot be detected — it is captured as-is: + +```ruby +FixtureKit.define do + projects = [] + expose(projects: projects) # captured as an empty collection + projects << Project.create!(...) # not reflected in `fixture.projects` +end +``` + +The conventional form -- `expose` as the last statement of the definition -- +avoids both cases. ## Fixture Inheritance (`extends`) @@ -337,6 +367,7 @@ Public error classes: - `FixtureKit::FixtureDefinitionNotFound` - `FixtureKit::RunnerAlreadyStartedError` - `FixtureKit::CircularFixtureInheritance` +- `FixtureKit::UnpersistedRecordError` ## Requirements diff --git a/lib/fixture_kit.rb b/lib/fixture_kit.rb index 7caf094..a718648 100644 --- a/lib/fixture_kit.rb +++ b/lib/fixture_kit.rb @@ -9,6 +9,7 @@ class CacheMissingError < Error; end class FixtureDefinitionNotFound < Error; end class RunnerAlreadyStartedError < Error; end class CircularFixtureInheritance < Error; end + class UnpersistedRecordError < Error; end autoload :VERSION, File.expand_path("fixture_kit/version", __dir__) autoload :Configuration, File.expand_path("fixture_kit/configuration", __dir__) diff --git a/lib/fixture_kit/cache.rb b/lib/fixture_kit/cache.rb index c31efba..34ba0c9 100644 --- a/lib/fixture_kit/cache.rb +++ b/lib/fixture_kit/cache.rb @@ -53,7 +53,7 @@ def save FixtureKit.runner.adapter.execute do |context| @content = MemoryCache.new( data: evaluate(FixtureKit.runner.coders, context), - exposed: file_cache.serialize_exposed(fixture.definition.exposed) + exposed: fixture.definition.exposed ) end diff --git a/lib/fixture_kit/definition.rb b/lib/fixture_kit/definition.rb index 4939892..e646551 100644 --- a/lib/fixture_kit/definition.rb +++ b/lib/fixture_kit/definition.rb @@ -25,12 +25,31 @@ def expose(**records) raise FixtureKit::DuplicateNameError, "Name #{name} already exposed" end - @exposed[name] = record + @exposed[name] = serialize(name, record) end end private + def serialize(name, record) + if record.is_a?(Array) + record.map { |item| reference(name, item) } + else + reference(name, record) + end + end + + def reference(name, record) + unless record.persisted? + raise FixtureKit::UnpersistedRecordError, + "cannot expose #{name.inspect}: the #{record.class} is not persisted. " \ + "Exposed records are captured as class/id pairs when `expose` is called, " \ + "so save the record before exposing it." + end + + { record.class => record.id } + end + def mixin(parent) definition = self diff --git a/lib/fixture_kit/file_cache.rb b/lib/fixture_kit/file_cache.rb index 51da248..e9f7fe5 100644 --- a/lib/fixture_kit/file_cache.rb +++ b/lib/fixture_kit/file_cache.rb @@ -48,16 +48,6 @@ def write(data) File.write(path, JSON.pretty_generate(content)) end - def serialize_exposed(exposed) - exposed.each_with_object({}) do |(name, record), hash| - if record.is_a?(Array) - hash[name] = record.map { |record| { record.class => record.id } } - else - hash[name] = { record.class => record.id } - end - end - end - private def coder_for(class_name) diff --git a/spec/unit/definition_spec.rb b/spec/unit/definition_spec.rb index a2eccbc..58534d0 100644 --- a/spec/unit/definition_spec.rb +++ b/spec/unit/definition_spec.rb @@ -12,41 +12,43 @@ end describe "#evaluate" do + let(:alice) { User.create!(name: "Alice", email: "alice-definition@example.com") } + it "captures exposed records from the definition block" do + record = alice definition = described_class.new do - expose(alice: "alice") + expose(alice: record) end definition.evaluate(Object.new) - expect(definition.exposed).to eq({ alice: "alice" }) + expect(definition.exposed).to eq({ alice: { User => alice.id } }) end it "supports helper methods from execution context" do + record = alice helper_context = Class.new do - def fixture_name - "alice" - end + define_method(:fixture_record) { record } end.new definition = described_class.new do - expose(alice: fixture_name) + expose(alice: fixture_record) end definition.evaluate(helper_context) - expect(definition.exposed).to eq({ alice: "alice" }) + expect(definition.exposed).to eq({ alice: { User => alice.id } }) end it "supports parent helper in execution context" do - parent_repository = Struct.new(:owner).new("alice") + parent_repository = Struct.new(:owner).new(alice) definition = described_class.new do expose(owner: parent.owner) end definition.evaluate(Object.new, parent: parent_repository) - expect(definition.exposed).to eq({ owner: "alice" }) + expect(definition.exposed).to eq({ owner: { User => alice.id } }) end end @@ -60,14 +62,94 @@ def fixture_name describe "#expose" do it "raises when the same name is exposed twice" do + alice = User.create!(name: "Alice", email: "alice-duplicate@example.com") definition = described_class.new do - expose(alice: "alice") - expose(alice: "duplicate") + expose(alice: alice) + expose(alice: alice) end expect do definition.evaluate(Object.new) end.to raise_error(FixtureKit::DuplicateNameError, "Name alice already exposed") end + + it "stores a record as a class/id pair" do + alice = User.create!(name: "Alice", email: "alice-pair@example.com") + definition = described_class.new { expose(alice: alice) } + + definition.evaluate(Object.new) + + expect(definition.exposed).to eq({ alice: { User => alice.id } }) + end + + it "stores a collection as an array of class/id pairs" do + alice = User.create!(name: "Alice", email: "alice-collection@example.com") + bob = User.create!(name: "Bob", email: "bob-collection@example.com") + definition = described_class.new { expose(users: [alice, bob]) } + + definition.evaluate(Object.new) + + expect(definition.exposed).to eq({ users: [{ User => alice.id }, { User => bob.id }] }) + end + + it "stores the record's own class for single-table-inheritance records" do + sedan = Car.create!(name: "Sedan") + definition = described_class.new { expose(sedan: sedan) } + + definition.evaluate(Object.new) + + expect(definition.exposed).to eq({ sedan: { Car => sedan.id } }) + end + + it "raises when the record is not persisted" do + unsaved = User.new(name: "Alice", email: "alice-unsaved@example.com") + definition = described_class.new { expose(alice: unsaved) } + + expect { definition.evaluate(Object.new) }.to raise_error( + FixtureKit::UnpersistedRecordError, + /cannot expose :alice: the User is not persisted/ + ) + end + + it "raises when a record inside a collection is not persisted" do + saved = User.create!(name: "Alice", email: "alice-mixed@example.com") + unsaved = User.new(name: "Bob", email: "bob-mixed@example.com") + definition = described_class.new { expose(users: [saved, unsaved]) } + + expect { definition.evaluate(Object.new) }.to raise_error( + FixtureKit::UnpersistedRecordError, + /cannot expose :users/ + ) + end + + it "raises when the record has been destroyed" do + destroyed = User.create!(name: "Alice", email: "alice-destroyed@example.com") + destroyed.destroy! + definition = described_class.new { expose(alice: destroyed) } + + expect { definition.evaluate(Object.new) } + .to raise_error(FixtureKit::UnpersistedRecordError) + end + + it "allows an empty collection" do + definition = described_class.new { expose(users: []) } + + definition.evaluate(Object.new) + + expect(definition.exposed).to eq({ users: [] }) + end + + # The registry holds every fixture for the life of the process, so a + # definition that held its records would keep their whole object graph + # alive until the suite ends. + it "does not hold a reference to the exposed record" do + alice = User.create!(name: "Alice", email: "alice-noref@example.com") + definition = described_class.new { expose(alice: alice) } + + definition.evaluate(Object.new) + + expect(definition.exposed.values.flatten).to all(be_a(Hash)) + expect(definition.exposed.values.flatten.flat_map(&:values)).to all(be_a(Integer)) + end end end diff --git a/spec/unit/file_cache_spec.rb b/spec/unit/file_cache_spec.rb index e3415e8..77d4c2a 100644 --- a/spec/unit/file_cache_spec.rb +++ b/spec/unit/file_cache_spec.rb @@ -84,23 +84,4 @@ expect(File.exist?(nested_path)).to be(true) end end - - describe "#serialize_exposed" do - it "converts ActiveRecord instances to class/id pairs" do - user = User.create!(name: "Alice", email: "alice-file-cache@example.com") - - result = file_cache.serialize_exposed({ alice: user }) - - expect(result).to eq({ alice: { User => user.id } }) - end - - it "converts arrays of ActiveRecord instances" do - alice = User.create!(name: "Alice", email: "alice-array@example.com") - bob = User.create!(name: "Bob", email: "bob-array@example.com") - - result = file_cache.serialize_exposed({ users: [alice, bob] }) - - expect(result).to eq({ users: [{ User => alice.id }, { User => bob.id }] }) - end - end end diff --git a/spec/unit/fixture_cache_spec.rb b/spec/unit/fixture_cache_spec.rb index 8d4a1bf..0926397 100644 --- a/spec/unit/fixture_cache_spec.rb +++ b/spec/unit/fixture_cache_spec.rb @@ -226,6 +226,29 @@ def identifier_for(identifier) expect(User.count).to eq(0) end + # The registry holds every fixture for the life of the process, so a + # definition that kept its exposed records would keep their whole object + # graph alive until the suite ends. + it "caches exposed records as class/id pairs without retaining the records" do + fixture_definition = FixtureKit::Definition.new do + alice = User.create!(name: "Alice", email: "alice-release@example.com") + expose(alice: alice) + end + fixture_double = instance_double( + FixtureKit::Fixture, + identifier: fixture_name, + definition: fixture_definition, + parent: nil + ) + fixture_cache = described_class.new(fixture_double) + + fixture_cache.save + + expect(fixture_definition.exposed.fetch(:alice).keys).to eq([User]) + expect(fixture_definition.exposed.fetch(:alice).values.first).to be_a(Integer) + expect(fixture_cache.load.alice.name).to eq("Alice") + end + it "includes parent fixture model records when saving inherited fixtures" do parent_cache_data = FixtureKit::MemoryCache.new( data: { FixtureKit::ActiveRecordCoder => { User => nil } },