DatabaseCleaner / database_cleaner-active_record

Strategies for cleaning databases using ActiveRecord. Can be used to ensure a clean state for testing.
MIT License
64 stars 63 forks source link

Fix an issue with same db name for different adapters #63

Closed iMacTia closed 1 year ago

iMacTia commented 3 years ago

Appreciate this might be a weird setup, but I have the same database name being served by 2 adapters (2 different DB instances). Here is my database.yml:

development:
  postgresql:
    adapter: postgresql
    encoding: unicode
    database: db_benchmarks
    host: localhost
    user: postgres
    pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %>
    migrations_paths: db/migrations
  clickhouse:
    adapter: clickhouse
    database: db_benchmarks
    migrations_paths: db/migrations

(Please note I've already tested this patch locally and I can confirm ClickHouse cleaning with truncation works as expected.) But when I did the following:

require 'database_cleaner/active_record'
DatabaseCleaner[:active_record, db: :clickhouse].strategy = :truncation
DatabaseCleaner[:active_record, db: :clickhouse].clean

I noticed that the PostgreSQL database was being cleaned instead! A bit of digging showed that this is an issue with the connection selector:

cleaner = DatabaseCleaner[:active_record, db: :clickhouse]
cleaner.strategy = :truncation
pp cleaner.strategy.connection_class.connection_pool.db_config
# => {:adapter=>"postgresql", :encoding=>"unicode", :database=>"db_benchmarks", :host=>"localhost", :user=>"postgres", :pool=>5, :migrations_paths=>"db/pg_migrations"}
# postgres connection is being selected instead of ClickHouse since the DB name is the same

This PR attempt to fix that by adding the adapter as a filter as well

iMacTia commented 3 years ago

I need to work out why tests are failing, will hopefully give it a go over the weekend

etagwerker commented 1 year ago

@iMacTia Did you get a chance to review the failing tests? There is a new CI setup, so you might want to update your branch with the latest from main

iMacTia commented 1 year ago

My branch diverted too much so I'd need to make changes to make this work, but since I don't need this anymore it's not really something that I can spend time on right now 😄 Happy to close since no one else seem to need this anyway