From adb8b33686559a9d9e28bc9f2eb71265e226fe0d Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 27 May 2026 13:59:53 +0200 Subject: [PATCH 1/5] Lazy-load parent cache content when saving a child fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a child fixture extends a parent, `Cache#evaluate` reads the parent's coder data with: fixture.parent.cache.content.data_for(coder.class) But `@content` is only populated by `Cache#load` (full mount) or `Cache#save`. If the parent was generated in a previous process (cache file on disk) but never mounted in the current one — which happens when the child cache is missing while the parent cache exists — the child goes straight to `save` and `parent.cache.content` is `nil`, raising: NoMethodError: undefined method 'data_for' for nil This is reproducible by clearing only the anonymous child cache (or modifying the child definition so its identifier changes) while preserving the parent cache. It also hits any setup where the test DB has been reset (e.g. `db:test:prepare`) and the parent cache survives, which is common when `FIXTURE_KIT_PRESERVE_CACHE=1` is used to support parallel workers. Extract the lazy read into a new `Cache#read_content` method so `evaluate` (and `load`) can populate `@content` on demand without triggering a full mount. Add a spec that reproduces the original crash: save a parent, call `clear_memory` to simulate a fresh process, then save a child that depends on the parent. Without the fix the spec raises `NoMethodError`. Refs https://github.com/yespark/yespark-rails/pull/21339 for the downstream workaround that motivated this upstream fix. --- lib/fixture_kit/cache.rb | 12 +++++++-- spec/unit/fixture_cache_spec.rb | 43 ++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/lib/fixture_kit/cache.rb b/lib/fixture_kit/cache.rb index c31efba..5601be4 100644 --- a/lib/fixture_kit/cache.rb +++ b/lib/fixture_kit/cache.rb @@ -40,7 +40,7 @@ def load raise FixtureKit::CacheMissingError, "Cache does not exist for fixture '#{fixture.identifier}'" end - @content ||= file_cache.read + read_content FixtureKit.runner.coders.each do |coder| coder.mount(content.data_for(coder.class)) @@ -49,6 +49,14 @@ def load Repository.new(content.exposed) end + # Lazily loads @content from the file cache. Used when content is needed in + # memory without a full mount (e.g. when a child fixture is being saved and + # needs the parent's coder data, while the parent itself was already cached + # to disk in a previous process and has not yet been mounted). + def read_content + @content ||= file_cache.read + end + def save FixtureKit.runner.adapter.execute do |context| @content = MemoryCache.new( @@ -68,7 +76,7 @@ def evaluate(coders, context, data = {}, &block) else coder, *remaining_coders = coders - parent_data = fixture.parent ? fixture.parent.cache.content.data_for(coder.class) : nil + parent_data = fixture.parent&.cache&.read_content&.data_for(coder.class) data[coder.class] = coder.generate(parent_data: parent_data) do evaluate(remaining_coders, context, data, &block) end diff --git a/spec/unit/fixture_cache_spec.rb b/spec/unit/fixture_cache_spec.rb index 8d4a1bf..5bd9b5b 100644 --- a/spec/unit/fixture_cache_spec.rb +++ b/spec/unit/fixture_cache_spec.rb @@ -231,7 +231,7 @@ def identifier_for(identifier) data: { FixtureKit::ActiveRecordCoder => { User => nil } }, exposed: {} ) - parent_cache = instance_double(FixtureKit::Cache, content: parent_cache_data) + parent_cache = instance_double(FixtureKit::Cache, read_content: parent_cache_data) parent_fixture = instance_double(FixtureKit::Fixture, cache: parent_cache) allow(parent_fixture).to receive(:mount) do User.create!(name: "Parent Owner", email: "parent-owner@example.com") @@ -256,6 +256,47 @@ def identifier_for(identifier) data = JSON.parse(File.read(child_cache.path)) expect(data["data"]["FixtureKit::ActiveRecordCoder"].keys).to include("User", "Project") end + + it "loads parent content from disk when child saves without parent in memory" do + # Reproduit le scénario "data_for for nil" : un fixture parent a déjà + # été persisté sur disque (process précédent), puis un fixture enfant + # doit être généré dans un nouveau process où parent.cache.content est nil. + parent_definition = FixtureKit::Definition.new do + User.create!(name: "Parent Owner", email: "parent-owner-cross-process@example.com") + end + parent_fixture = instance_double( + FixtureKit::Fixture, + identifier: "parent_fixture", + definition: parent_definition, + parent: nil + ) + parent_cache = described_class.new(parent_fixture) + parent_cache.save + parent_cache.clear_memory # simule un nouveau process : @content = nil mais le fichier existe + + allow(parent_fixture).to receive(:cache).and_return(parent_cache) + allow(parent_fixture).to receive(:mount) do + User.create!(name: "Parent Owner", email: "parent-owner-cross-process@example.com") + FixtureKit::Repository.new({}) + end + + child_definition = FixtureKit::Definition.new do + owner = User.find_by!(email: "parent-owner-cross-process@example.com") + Project.create!(name: "Cross-process project", owner: owner) + end + child_fixture = instance_double( + FixtureKit::Fixture, + identifier: "child_fixture", + definition: child_definition, + parent: parent_fixture + ) + child_cache = described_class.new(child_fixture) + + expect { child_cache.save }.not_to raise_error + + data = JSON.parse(File.read(child_cache.path)) + expect(data["data"]["FixtureKit::ActiveRecordCoder"].keys).to include("User", "Project") + end end describe "#clear_memory" do From 6cdf7282835083561f162805fc504dc55c17dccc Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 27 May 2026 14:17:39 +0200 Subject: [PATCH 2/5] Tighten parent-data lookup to a parent-nil check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The safe-nav chain `parent&.cache&.read_content&.data_for(...)` was over-defensive: `Cache` is never nil when `parent` exists, and `read_content` either returns a `MemoryCache` or raises (it cannot return nil). Reverting to the original explicit `parent ? ... : nil` preserves the pre-fix semantics — nil only when there's no parent; any other failure surfaces loudly. --- lib/fixture_kit/cache.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/fixture_kit/cache.rb b/lib/fixture_kit/cache.rb index 5601be4..025fb6f 100644 --- a/lib/fixture_kit/cache.rb +++ b/lib/fixture_kit/cache.rb @@ -76,7 +76,7 @@ def evaluate(coders, context, data = {}, &block) else coder, *remaining_coders = coders - parent_data = fixture.parent&.cache&.read_content&.data_for(coder.class) + parent_data = fixture.parent ? fixture.parent.cache.read_content.data_for(coder.class) : nil data[coder.class] = coder.generate(parent_data: parent_data) do evaluate(remaining_coders, context, data, &block) end From 97f4fdebce06f2f68bc6855fab8520521cfd163f Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 27 May 2026 14:32:36 +0200 Subject: [PATCH 3/5] Address review feedback: rename + strengthen specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename `read_content` to `ensure_content`. The `read_` prefix suggested a pure read, hiding the memoization side effect. The new name reflects the actual contract: ensure `@content` is populated, reading from disk only if necessary. Docstring also calls out that the method raises when the cache file is absent — callers must guarantee existence. - Strengthen the cross-process reproducer spec. The previous stub of `parent.mount` recreated `User` inside the child save block, so `ActiveRecordCoder` captured `User` from the event stream regardless of `parent_data`. A faux fix that returned `parent_data: nil` would still satisfy the assertion. The stub now returns an empty repository (User already lives in DB from `parent_cache.save`), so `User` only ends up in the child cache via `parent_data.keys` — making the assertion load-bearing. Also adds an explicit `expect(parent_cache.content).to be_nil` precondition right after `clear_memory`. - Add three focused specs for `#ensure_content`: returns memoized `@content` without touching disk, re-reads disk after `clear_memory`, and raises `Errno::ENOENT` when the cache file is absent. Formalizes the contract instead of inheriting it implicitly from `File.read`. --- lib/fixture_kit/cache.rb | 9 +++--- spec/unit/fixture_cache_spec.rb | 54 ++++++++++++++++++++++++++++++--- 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/lib/fixture_kit/cache.rb b/lib/fixture_kit/cache.rb index 025fb6f..fb85c77 100644 --- a/lib/fixture_kit/cache.rb +++ b/lib/fixture_kit/cache.rb @@ -40,7 +40,7 @@ def load raise FixtureKit::CacheMissingError, "Cache does not exist for fixture '#{fixture.identifier}'" end - read_content + ensure_content FixtureKit.runner.coders.each do |coder| coder.mount(content.data_for(coder.class)) @@ -52,8 +52,9 @@ def load # Lazily loads @content from the file cache. Used when content is needed in # memory without a full mount (e.g. when a child fixture is being saved and # needs the parent's coder data, while the parent itself was already cached - # to disk in a previous process and has not yet been mounted). - def read_content + # to disk in a previous process and has not yet been mounted). Raises if the + # cache file is absent or unreadable — callers must guarantee existence. + def ensure_content @content ||= file_cache.read end @@ -76,7 +77,7 @@ def evaluate(coders, context, data = {}, &block) else coder, *remaining_coders = coders - parent_data = fixture.parent ? fixture.parent.cache.read_content.data_for(coder.class) : nil + parent_data = fixture.parent ? fixture.parent.cache.ensure_content.data_for(coder.class) : nil data[coder.class] = coder.generate(parent_data: parent_data) do evaluate(remaining_coders, context, data, &block) end diff --git a/spec/unit/fixture_cache_spec.rb b/spec/unit/fixture_cache_spec.rb index 5bd9b5b..84840cd 100644 --- a/spec/unit/fixture_cache_spec.rb +++ b/spec/unit/fixture_cache_spec.rb @@ -231,7 +231,7 @@ def identifier_for(identifier) data: { FixtureKit::ActiveRecordCoder => { User => nil } }, exposed: {} ) - parent_cache = instance_double(FixtureKit::Cache, read_content: parent_cache_data) + parent_cache = instance_double(FixtureKit::Cache, ensure_content: parent_cache_data) parent_fixture = instance_double(FixtureKit::Fixture, cache: parent_cache) allow(parent_fixture).to receive(:mount) do User.create!(name: "Parent Owner", email: "parent-owner@example.com") @@ -274,11 +274,16 @@ def identifier_for(identifier) parent_cache.save parent_cache.clear_memory # simule un nouveau process : @content = nil mais le fichier existe + # Précondition : confirme qu'on est bien dans l'état "cache sur disque, + # rien en mémoire" — c'est ce que reproduit le bug d'origine. + expect(parent_cache.content).to be_nil + allow(parent_fixture).to receive(:cache).and_return(parent_cache) - allow(parent_fixture).to receive(:mount) do - User.create!(name: "Parent Owner", email: "parent-owner-cross-process@example.com") - FixtureKit::Repository.new({}) - end + # Le stub de mount NE recrée PAS User (la save parente l'a laissé en DB). + # Du coup, User n'apparaît dans le child cache QUE via parent_data.keys : + # un faux fix qui renverrait parent_data: nil ferait disparaître "User" + # de l'assertion ci-dessous. + allow(parent_fixture).to receive(:mount).and_return(FixtureKit::Repository.new({})) child_definition = FixtureKit::Definition.new do owner = User.find_by!(email: "parent-owner-cross-process@example.com") @@ -354,6 +359,45 @@ def identifier_for(identifier) end end + describe "#ensure_content" do + it "returns the memoized @content without touching disk" do + in_memory = FixtureKit::MemoryCache.new(data: {}, exposed: {}) + cache.instance_variable_set(:@content, in_memory) + + # Pas de fichier sur disque — si ensure_content lit le disque on aurait + # Errno::ENOENT. Le fait qu'il retourne sans erreur prouve qu'il sert + # bien la version mémoïsée. + expect(File.exist?(cache.path)).to be(false) + expect(cache.ensure_content).to be(in_memory) + end + + it "re-reads from disk after clear_memory" do + fixture_definition = FixtureKit::Definition.new do + alice = User.create!(name: "Alice", email: "alice-ensure@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 + fixture_cache.clear_memory + + reloaded = fixture_cache.ensure_content + + expect(reloaded).to be_a(FixtureKit::MemoryCache) + expect(reloaded.data_for(FixtureKit::ActiveRecordCoder)).to have_key(User) + end + + it "raises when the cache file is absent" do + expect(File.exist?(cache.path)).to be(false) + expect { cache.ensure_content }.to raise_error(Errno::ENOENT) + end + end + describe "#load" do it "documents that connection execute_batch is currently private" do connection = User.connection From 7f1bd23083d60f6e5a035df8ec11fe00d0ec3582 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 27 May 2026 14:34:02 +0200 Subject: [PATCH 4/5] Translate spec comments to English The maintainer reads English, so French inline comments in the cache spec are noise. Translates the cross-process reproducer comments and the new #ensure_content memoization comment. No behavior change. --- spec/unit/fixture_cache_spec.rb | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/spec/unit/fixture_cache_spec.rb b/spec/unit/fixture_cache_spec.rb index 84840cd..ca73b62 100644 --- a/spec/unit/fixture_cache_spec.rb +++ b/spec/unit/fixture_cache_spec.rb @@ -258,9 +258,10 @@ def identifier_for(identifier) end it "loads parent content from disk when child saves without parent in memory" do - # Reproduit le scénario "data_for for nil" : un fixture parent a déjà - # été persisté sur disque (process précédent), puis un fixture enfant - # doit être généré dans un nouveau process où parent.cache.content est nil. + # Reproduces the "data_for for nil" scenario: a parent fixture has + # already been persisted to disk (previous process), then a child + # fixture must be generated in a new process where + # parent.cache.content is nil. parent_definition = FixtureKit::Definition.new do User.create!(name: "Parent Owner", email: "parent-owner-cross-process@example.com") end @@ -272,17 +273,17 @@ def identifier_for(identifier) ) parent_cache = described_class.new(parent_fixture) parent_cache.save - parent_cache.clear_memory # simule un nouveau process : @content = nil mais le fichier existe + parent_cache.clear_memory # simulates a fresh process: @content = nil but the file exists - # Précondition : confirme qu'on est bien dans l'état "cache sur disque, - # rien en mémoire" — c'est ce que reproduit le bug d'origine. + # Precondition: confirms we are really in the "cache on disk, nothing + # in memory" state — this is what triggered the original bug. expect(parent_cache.content).to be_nil allow(parent_fixture).to receive(:cache).and_return(parent_cache) - # Le stub de mount NE recrée PAS User (la save parente l'a laissé en DB). - # Du coup, User n'apparaît dans le child cache QUE via parent_data.keys : - # un faux fix qui renverrait parent_data: nil ferait disparaître "User" - # de l'assertion ci-dessous. + # The mount stub does NOT recreate User (parent_cache.save already left + # it in the DB). As a result, User can only appear in the child cache + # via parent_data.keys — a faux fix that returned parent_data: nil + # would make "User" disappear from the assertion below. allow(parent_fixture).to receive(:mount).and_return(FixtureKit::Repository.new({})) child_definition = FixtureKit::Definition.new do @@ -364,9 +365,9 @@ def identifier_for(identifier) in_memory = FixtureKit::MemoryCache.new(data: {}, exposed: {}) cache.instance_variable_set(:@content, in_memory) - # Pas de fichier sur disque — si ensure_content lit le disque on aurait - # Errno::ENOENT. Le fait qu'il retourne sans erreur prouve qu'il sert - # bien la version mémoïsée. + # No file on disk — if ensure_content read from disk we would get + # Errno::ENOENT. The fact that it returns without error proves it is + # serving the memoized version. expect(File.exist?(cache.path)).to be(false) expect(cache.ensure_content).to be(in_memory) end From b01ae5f1af2d8befcd3bbf61090b9dbfb50ca3c3 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Wed, 3 Jun 2026 15:07:49 +0200 Subject: [PATCH 5/5] Collapse ensure_content into a lazy #content reader Per review feedback, fold the separate `ensure_content` method into `content` itself, making `content` lazily read from disk and memoize: def content @content ||= file_cache.read end This shrinks the public surface to a single accessor and restores the original, natural call site in `evaluate`: fixture.parent.cache.content.data_for(coder.class) The only place that relied on `content` returning nil without touching disk is `exists?`, which now reads the `@content` ivar directly so it never triggers a disk read. Every other reader is already guarded by `exists?` (load) or sets `@content` first (save), so making `content` raise on a missing/corrupt file keeps failures loud exactly where the old code surfaced them. Specs that asserted the in-memory state via `content` (the four `content == nil` checks after `clear_memory`, plus the cross-process precondition) now assert `@content` directly, since `content` would lazily re-read from disk. The `#ensure_content` examples move under `#content` unchanged in intent. No behavior change for callers: load/save/evaluate semantics are identical; only the method name and the exists? internals change. --- lib/fixture_kit/cache.rb | 28 ++++++++++++++-------------- spec/unit/fixture_cache_spec.rb | 27 ++++++++++++++------------- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/lib/fixture_kit/cache.rb b/lib/fixture_kit/cache.rb index fb85c77..fd57d21 100644 --- a/lib/fixture_kit/cache.rb +++ b/lib/fixture_kit/cache.rb @@ -6,7 +6,7 @@ class Cache include ConfigurationHelper - attr_reader :fixture, :content + attr_reader :fixture def initialize(fixture) @fixture = fixture @@ -28,7 +28,18 @@ def identifier end def exists? - content || file_cache.exists? + @content || file_cache.exists? + end + + # The cache content, lazily read from disk and memoized. Populated by #load + # (mount), #save, or — when neither has run in this process — by reading the + # file cache on first access. This is what lets a child fixture pick up its + # parent's coder data when the parent was cached to disk in a previous + # process and has not yet been mounted. Raises if the file is absent or + # unreadable, so callers that have not populated content must guarantee the + # file exists (e.g. behind #exists?). + def content + @content ||= file_cache.read end def clear_memory @@ -40,8 +51,6 @@ def load raise FixtureKit::CacheMissingError, "Cache does not exist for fixture '#{fixture.identifier}'" end - ensure_content - FixtureKit.runner.coders.each do |coder| coder.mount(content.data_for(coder.class)) end @@ -49,15 +58,6 @@ def load Repository.new(content.exposed) end - # Lazily loads @content from the file cache. Used when content is needed in - # memory without a full mount (e.g. when a child fixture is being saved and - # needs the parent's coder data, while the parent itself was already cached - # to disk in a previous process and has not yet been mounted). Raises if the - # cache file is absent or unreadable — callers must guarantee existence. - def ensure_content - @content ||= file_cache.read - end - def save FixtureKit.runner.adapter.execute do |context| @content = MemoryCache.new( @@ -77,7 +77,7 @@ def evaluate(coders, context, data = {}, &block) else coder, *remaining_coders = coders - parent_data = fixture.parent ? fixture.parent.cache.ensure_content.data_for(coder.class) : nil + parent_data = fixture.parent ? fixture.parent.cache.content.data_for(coder.class) : nil data[coder.class] = coder.generate(parent_data: parent_data) do evaluate(remaining_coders, context, data, &block) end diff --git a/spec/unit/fixture_cache_spec.rb b/spec/unit/fixture_cache_spec.rb index ca73b62..91a0baa 100644 --- a/spec/unit/fixture_cache_spec.rb +++ b/spec/unit/fixture_cache_spec.rb @@ -231,7 +231,7 @@ def identifier_for(identifier) data: { FixtureKit::ActiveRecordCoder => { User => nil } }, exposed: {} ) - parent_cache = instance_double(FixtureKit::Cache, ensure_content: parent_cache_data) + parent_cache = instance_double(FixtureKit::Cache, content: parent_cache_data) parent_fixture = instance_double(FixtureKit::Fixture, cache: parent_cache) allow(parent_fixture).to receive(:mount) do User.create!(name: "Parent Owner", email: "parent-owner@example.com") @@ -260,8 +260,8 @@ def identifier_for(identifier) it "loads parent content from disk when child saves without parent in memory" do # Reproduces the "data_for for nil" scenario: a parent fixture has # already been persisted to disk (previous process), then a child - # fixture must be generated in a new process where - # parent.cache.content is nil. + # fixture must be generated in a new process where the parent's + # content has not been loaded into memory. parent_definition = FixtureKit::Definition.new do User.create!(name: "Parent Owner", email: "parent-owner-cross-process@example.com") end @@ -276,8 +276,9 @@ def identifier_for(identifier) parent_cache.clear_memory # simulates a fresh process: @content = nil but the file exists # Precondition: confirms we are really in the "cache on disk, nothing - # in memory" state — this is what triggered the original bug. - expect(parent_cache.content).to be_nil + # in memory" state — this is what triggered the original bug. We assert + # @content directly because #content now lazily re-reads from disk. + expect(parent_cache.instance_variable_get(:@content)).to be_nil allow(parent_fixture).to receive(:cache).and_return(parent_cache) # The mount stub does NOT recreate User (parent_cache.save already left @@ -311,7 +312,7 @@ def identifier_for(identifier) cache.clear_memory - expect(cache.content).to be_nil + expect(cache.instance_variable_get(:@content)).to be_nil end it "still reports exists? as true when file cache is present" do @@ -330,7 +331,7 @@ def identifier_for(identifier) fixture_cache.clear_memory - expect(fixture_cache.content).to be_nil + expect(fixture_cache.instance_variable_get(:@content)).to be_nil expect(fixture_cache.exists?).to be(true) end @@ -349,7 +350,7 @@ def identifier_for(identifier) fixture_cache.save fixture_cache.clear_memory - expect(fixture_cache.content).to be_nil + expect(fixture_cache.instance_variable_get(:@content)).to be_nil User.delete_all repository = fixture_cache.load @@ -360,16 +361,16 @@ def identifier_for(identifier) end end - describe "#ensure_content" do + describe "#content" do it "returns the memoized @content without touching disk" do in_memory = FixtureKit::MemoryCache.new(data: {}, exposed: {}) cache.instance_variable_set(:@content, in_memory) - # No file on disk — if ensure_content read from disk we would get + # No file on disk — if #content read from disk we would get # Errno::ENOENT. The fact that it returns without error proves it is # serving the memoized version. expect(File.exist?(cache.path)).to be(false) - expect(cache.ensure_content).to be(in_memory) + expect(cache.content).to be(in_memory) end it "re-reads from disk after clear_memory" do @@ -387,7 +388,7 @@ def identifier_for(identifier) fixture_cache.save fixture_cache.clear_memory - reloaded = fixture_cache.ensure_content + reloaded = fixture_cache.content expect(reloaded).to be_a(FixtureKit::MemoryCache) expect(reloaded.data_for(FixtureKit::ActiveRecordCoder)).to have_key(User) @@ -395,7 +396,7 @@ def identifier_for(identifier) it "raises when the cache file is absent" do expect(File.exist?(cache.path)).to be(false) - expect { cache.ensure_content }.to raise_error(Errno::ENOENT) + expect { cache.content }.to raise_error(Errno::ENOENT) end end