Open dmolesUC opened 3 years ago
@dmolesUC Looking at this, I completely agree with your conclusion. While the safeguard container and running logic should be in database_cleaner-core
, the specific safeguards themselves should be supplied by the adapter gems. We just need to figure out how to do this in a backwards-compatible way. Want to take a stab at this?
I debated filing this against database_cleaner-active_record, but after digging in a bit I think it would need a more comprehensive fix.
I have a Rails app running in a Docker stack, where under test, the database is configured the old-fashioned way in
database.yml
rather than withENV['DATABASE_URL']
-- or rather, both are supported, but if DATABASE_URL isn't set, it falls back to a hard-coded value.I wanted a safeguard that would work both with the Docker stack in CI, or for a developer setting up a test database locally or on some other database server, so I assumed I had to set
url_allowlist
. My first attempt was this lambda:This blows up, though, because the URL that gets passed here is
ENV['DATABASE_URL']
, andURI.parse(nil)
fails.My next concern was that
DatabaseCleaner
would truncate the database atENV['DATABASE_URL']
(if it exists) rather than the one ActiveRecord is actually using in tests. Thankfullydatabase_cleaner-active_record
is smarter than that, and it's using the ActiveRecord connection.But it seems like if that's how it actually works,
Safeguard
should somehow hook into that, rather than relying onENV['DATABASE_URL']
. Maybe instead of running once before all cleaners, it should be run per-cleaner, and the cleaner implementations should be responsible for providing the URL they're actually using?