Skip to content

Fix ArgumentError from TestFixtures setup_shared_connection_pool#379

Merged
mnovelo merged 7 commits intomainfrom
man/test-fixtures-compat
Apr 11, 2026
Merged

Fix ArgumentError from TestFixtures setup_shared_connection_pool#379
mnovelo merged 7 commits intomainfrom
man/test-fixtures-compat

Conversation

@mnovelo
Copy link
Copy Markdown
Collaborator

@mnovelo mnovelo commented Apr 11, 2026

Summary

  • Fix ArgumentError: pool_config for :reading role and :apartment_...:reading shard was nil caused by Rails' setup_shared_connection_pool iterating apartment's role-specific tenant shards
  • Auto-wire cleanup via Railtie on_load(:active_record_fixtures) hook, matching activerecord-tenanted's precedent for fixture integration
  • Guard ivar prevents re-entry from the !connection.active_record notification subscriber mid-example

Problem

Rails' ActiveRecord::TestFixtures#setup_shared_connection_pool assumes every shard has a :writing pool_config. Apartment v4 registers tenant pools under role-specific custom shard keys (e.g., shard :apartment_acme:reading under role :reading). When fixture setup encounters this shard, writing_pool_config resolves to nil, and set_pool_config(role, shard, nil) raises ArgumentError.

Surfaces when a before(:all) block (or prior test) triggers the elevator under connected_to(role: :reading), creating a tenant pool, and a subsequent example's fixture setup discovers and iterates it.

Changes

File What
lib/apartment/test_fixtures.rb New Apartment::TestFixtures module: overrides setup_shared_connection_pool to deregister apartment pools before Rails iterates them
lib/apartment/railtie.rb on_load(:active_record_fixtures) hook, guarded by Rails.env.test? and config.test_fixture_cleanup
lib/apartment/config.rb test_fixture_cleanup attribute (default true) — escape hatch to disable
spec/unit/test_fixtures_spec.rb 6 specs: regression test, cleanup, pool manager clear, re-entry guard, teardown reset
docs/designs/v4-test-fixtures-compatibility.md Design spec
docs/plans/apartment-v4/test-fixtures-compatibility.md Implementation plan

Test plan

  • bundle exec appraisal rails-8.1-sqlite3 rspec spec/unit/test_fixtures_spec.rb — 6 examples, 0 failures
  • bundle exec appraisal rails-8.0-sqlite3 rspec spec/unit/test_fixtures_spec.rb — 6 examples, 0 failures
  • bundle exec appraisal rails-8.1-sqlite3 rspec spec/unit/ — 688 examples, 0 failures
  • bundle exec rubocop on all changed files — no offenses

🤖 Generated with Claude Code

mnovelo and others added 7 commits April 11, 2026 10:01
… setup

Rails' setup_shared_connection_pool assumes every registered shard has a
:writing pool_config; Apartment's role-keyed tenant shards violate this,
raising ArgumentError. This module strips tenant pools from the
ConnectionHandler and PoolManager before super runs, with a re-entrant
guard to prevent double-cleanup from AR's notification subscriber.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…_pools!

- Add boolean validation for test_fixture_cleanup in Config#validate!
  (matches pattern of all other boolean config attributes)
- Extract public Apartment.reset_tenant_pools! method encapsulating the
  three-step cleanup (deregister + clear + reset)
- Update TestFixtures to call the public API instead of send(:deregister_all_tenant_pools)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The previous review-fix commit (8fabf00) accidentally reverted the
Railtie wiring added in 35eda8c. The hook was lost because the
working tree had a stale railtie.rb when the review fixes were staged.

Without this hook, test_fixture_cleanup config is validated but never
read, and downstream apps don't get the automatic cleanup.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Restore config_spec default assertion for test_fixture_cleanup
  (lost in external file modification)
- Add comment explaining re-entry test is a unit-level approximation
  of the !connection.active_record subscriber scenario
- Improve YARD doc on reset_tenant_pools! for discoverability

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@mnovelo mnovelo merged commit 7b8071f into main Apr 11, 2026
43 checks passed
@mnovelo mnovelo deleted the man/test-fixtures-compat branch April 11, 2026 15:08
mnovelo added a commit that referenced this pull request Apr 11, 2026
PR #379 guarded cleanup but still called super on re-entry.
When the !connection.active_record subscriber fires mid-test
(after the elevator creates a tenant pool), super iterates
shard_names and crashes on the apartment shard that has no
:writing pool_config.

Fix: return early (skip both cleanup AND super) when the guard
is set. Apartment pools should not participate in Rails'
writing/reading pool config swap; the subscriber still pins and
leases the connection independently.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mnovelo added a commit that referenced this pull request Apr 11, 2026
* Fix TestFixtures subscriber re-entry crash (Trigger B)

PR #379 guarded cleanup but still called super on re-entry.
When the !connection.active_record subscriber fires mid-test
(after the elevator creates a tenant pool), super iterates
shard_names and crashes on the apartment shard that has no
:writing pool_config.

Fix: return early (skip both cleanup AND super) when the guard
is set. Apartment pools should not participate in Rails'
writing/reading pool config swap; the subscriber still pins and
leases the connection independently.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Clarify guard comment: first call vs re-entry behavior

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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