Skip to content

Store exposed records as class/id pairs instead of the records - #77

Merged
ngan merged 1 commit into
mainfrom
np-expose-record-references
Aug 3, 2026
Merged

Store exposed records as class/id pairs instead of the records#77
ngan merged 1 commit into
mainfrom
np-expose-record-references

Conversation

@ngan

@ngan ngan commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Problem

Definition#expose stored the ActiveRecord objects it was handed:

@exposed[name] = record

Registry#@fixtures is never pruned and the Runner is memoized on the FixtureKit module, so every Definition lives for the whole process — and each one pinned its records, their @attributes, and any association caches the definition warmed while wiring things up. In a large suite that grows monotonically as more spec files execute.

The records were dead weight from the moment they were serialized: Repository re-fetches rows per test with find_by(id:), so only the class/id pairs matter.

Confirmed with a root-removal test — weak refs to the exposed records, clear the registry and @exposed, force GC: 2/2 collected. That chain was their only root.

Fix

Only Cache#save ever read exposed, and only to convert it into class/id pairs, 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 wasn't the inverse of anything on FileCacheFileCache#read converts {"User" => id} back to {User => id} because JSON.generate does the class-to-string half.

Impact

Measured with a synthetic suite of anonymous fixtures (ObjectSpace.memsize_of_all and GC.stat(:heap_live_slots), sampled before the diagnostic walk so the instrumentation doesn't inflate what it measures):

fixtures retained heap live objects
300, before 43.9 MB 753,181
300, after 26.5 MB 622,166

Per-fixture retained growth: ~101 KB → ~43 KB. Records retained: 0.

Scale note: the benchmark fixture is tiny (1 user, 3 projects, 6 tasks). A fixture exposing a company plus its employees and payrolls retains proportionally more, so the real-world win is larger.

Behavior change

A record's id is now read when expose is called rather than at the end of the definition. An unsaved record would therefore store a nil id and silently resolve to nil in tests, so this now raises:

FixtureKit.define do
  user = User.new(name: "Alice")
  expose(user: user)  # raises FixtureKit::UnpersistedRecordError
  user.save!
end

The check uses persisted?, so it also covers destroyed records, and it applies to each record inside an exposed collection.

One case can't be detected — a collection appended to after being exposed is captured as-is (empty). There's no way to tell that from a legitimately empty collection, so it's documented in docs/reference.md rather than guessed at.

Every fixture in this repo, and the documented idiom, call expose as the last statement of the definition, which avoids both cases.

Notes for review

  • definition.exposed now returns class/id pairs rather than records. Not documented as public API, but worth a grep in consuming apps for anything reading it after generation.
  • Three definition_spec examples exposed plain Strings; they use real records now, since expose reads .id. Side effect: exposing a non-record fails inside the definition block instead of deep in FileCache at save time.
  • The two serialize_exposed specs moved from file_cache_spec to definition_spec, plus new coverage for collections, STI, the unpersisted/destroyed guards, and that exposed holds no record references.
  • No version bump here — happy to add one if you'd like this in a release.

Testing

  • bundle exec rspec — 192 examples, 0 failures
  • FIXTURE_KIT_INTEGRATION_FRAMEWORK=minitest bundle exec rspec — 192 examples, 0 failures
  • Dummy app suites directly: minitest 21 runs / 67 assertions, RSpec 23 examples — all green, so the new guard doesn't fire on any real fixture

A follow-up PR will address a second, independent leak: the adapters build a throwaway ExampleGroup/TestCase subclass per fixture generation, which Rails' :active_record_fixtures load-hook registry retains forever.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PQjkiuuGX2t3TSZKpzqpPv

`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQjkiuuGX2t3TSZKpzqpPv
@ngan
ngan force-pushed the np-expose-record-references branch from 4c9c6ee to 546d629 Compare August 3, 2026 22:53
@ngan
ngan merged commit 3a0e041 into main Aug 3, 2026
18 checks passed
@ngan
ngan deleted the np-expose-record-references branch August 3, 2026 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant