-
Notifications
You must be signed in to change notification settings - Fork 126
chore(specs): Fix DatabaseCleaner
for events
database
#3914
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vincent-pochet
approved these changes
Jul 7, 2025
`events` database cleanup currently does not work properly if the `transaction` is used. This is because `DatabaseCleaner` will only apply the strategy on the default database connection. Since `events` is using a separate connection, the connection will not be rolled back. Take the following spec: ```ruby require "rails_helper" RSpec.describe DatabaseCleaner do it "has zero events" do expect(Event.count).to eq(0) create(:event) expect(Event.count).to eq(1) end it "also has zero events" do # This should work if each example is wrapped in a transaction expect(Event.count).to eq(0) end end ``` This will fail with the following error: ```shell ⋊> lago exec api bundle exec rspec --color --format documentation spec/database_cleaner_spec.rb DatabaseCleaner has zero events also has zero events (FAILED - 1) Failures: 1) DatabaseCleaner also has zero events Failure/Error: expect(Event.count).to eq(0) expected: 0 got: 1 (compared using ==) # ./spec/database_cleaner_spec.rb:16:in 'block (2 levels) in <top (required)>' # ./spec/spec_helper.rb:160:in 'block (3 levels) in <top (required)>' # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/strategy.rb:30:in 'DatabaseCleaner::Strategy#cleaning' # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/cleaners.rb:34:in 'block (2 levels) in DatabaseCleaner::Cleaners#cleaning' # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/cleaners.rb:35:in 'DatabaseCleaner::Cleaners#cleaning' # ./spec/spec_helper.rb:159:in 'block (2 levels) in <top (required)>' # /usr/local/bundle/gems/webmock-3.23.1/lib/webmock/rspec.rb:39:in 'block (2 levels) in <main>' Finished in 0.58037 seconds (files took 2.43 seconds to load) 2 examples, 1 failure Failed examples: rspec ./spec/database_cleaner_spec.rb:14 # DatabaseCleaner also has zero events ``` The only reason why it works with `deletion` strategy is because `events` table are within the same database as the `default` connection and gets deleted/truncated when `DatabaseCleaner` cleans the `default` database. The fix is to use define a specific strategy for `events` database: ```ruby DatabaseCleaner[:active_record, db: EventsRecord].strategy = :transaction ``` Note that we have to use `EventsRecord` instead of `events` as `db`. The reason is that `DatabaseCleaner` tries to retrieve the first active record class whose connection is matching the database name. This will be `ApplicationRecord` since it uses the same database configuration as `events`. Then `DatabaseCleaner` will use the connection from that class to handle the cleanup. In this case, the cleanup will happen on the `default` database instead of the `events`. See https://github.com/DatabaseCleaner/database_cleaner-active_record/blob/912acbe1facddfe1b14f21107f6ddc9df290de53/lib/database_cleaner/active_record/base.rb#L82.
diegocharles
pushed a commit
that referenced
this pull request
Jul 11, 2025
## Context `events` table cleanup currently does not work properly if the `transaction` strategy is used. This is because `DatabaseCleaner` will only apply the strategy on the default database connection. Since `events` is using a separate connection, the connection will not be rolled back. Take the following spec: ```ruby require "rails_helper" RSpec.describe DatabaseCleaner do it "has zero events" do expect(Event.count).to eq(0) create(:event) expect(Event.count).to eq(1) end it "also has zero events" do # This should work if each example is wrapped in a transaction expect(Event.count).to eq(0) end end ``` This will fail with the following error: ```shell ⋊> lago exec api bundle exec rspec --color --format documentation spec/database_cleaner_spec.rb DatabaseCleaner has zero events also has zero events (FAILED - 1) Failures: 1) DatabaseCleaner also has zero events Failure/Error: expect(Event.count).to eq(0) expected: 0 got: 1 (compared using ==) # ./spec/database_cleaner_spec.rb:16:in 'block (2 levels) in <top (required)>' # ./spec/spec_helper.rb:160:in 'block (3 levels) in <top (required)>' # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/strategy.rb:30:in 'DatabaseCleaner::Strategy#cleaning' # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/cleaners.rb:34:in 'block (2 levels) in DatabaseCleaner::Cleaners#cleaning' # /usr/local/bundle/gems/database_cleaner-core-2.0.1/lib/database_cleaner/cleaners.rb:35:in 'DatabaseCleaner::Cleaners#cleaning' # ./spec/spec_helper.rb:159:in 'block (2 levels) in <top (required)>' # /usr/local/bundle/gems/webmock-3.23.1/lib/webmock/rspec.rb:39:in 'block (2 levels) in <main>' Finished in 0.58037 seconds (files took 2.43 seconds to load) 2 examples, 1 failure Failed examples: rspec ./spec/database_cleaner_spec.rb:14 # DatabaseCleaner also has zero events ``` The only reason it works with `deletion` strategy is because `events` table is within the same database as the `default` (`primary`) connection and gets deleted/truncated when `DatabaseCleaner` cleans the `default` database. ## Description **How to fix it ?** The fix is to define a specific strategy for `events` database: ```ruby DatabaseCleaner[:active_record, db: EventsRecord].strategy = :transaction ``` Note that we have to use `EventsRecord` instead of `events` as `db`. The reason is that `DatabaseCleaner` tries to retrieve the first active record class whose connection is matching the database name. This will be `ApplicationRecord` since its connection (`primary`/`default`) uses the same database name as the `events` connection. Then `DatabaseCleaner` will use the connection from that class to handle the cleanup. In this case, the cleanup (`transaction`) will happen on the `default` database instead of the `events`. See https://github.com/DatabaseCleaner/database_cleaner-active_record/blob/912acbe1facddfe1b14f21107f6ddc9df290de53/lib/database_cleaner/active_record/base.rb#L82. **Failing tests** Fixing the configuration caused a few tests to fail as they were relying on the creation of `events`. Those events used to be created without a transaction so the tests worked fine. Now we need to set `transaction: false` for those tests in order for the events to be saved to the database as the default strategy for `events` is now `transaction`. **Performance impact** In order to compare the impact of the deletion on the spec performance, I ran the following command on `main` and this branch: ```shell lago exec api bundle exec rspec spec/scenarios/fees/recurring_fees_spec.rb spec/scenarios/invoices/advance_charges_spec.rb spec /scenarios/invoices/invoices_spec.rb spec/services/billable_metrics/breakdown/unique_count_service_spec.rb spec/services/events/stores/postgres_store_spec.rb sp ec/services/fees/charge_service_spec.rb spec/services/invoices/calculate_fees_service_spec.rb spec/services/invoices/preview_service_spec.rb spec/scenarios/charge_models/prorated_graduated_spec.rb ``` The results I got: * On `main`: 49.94 seconds * On this branch: 55.92 seconds If the performance impact is/become unacceptable, an alternative could be to use the `null` strategy or use a top-level `after(:context) { DatabaseCleaner[:active_record, db: EventsRecord].clean_with(:deletion) }` for those tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
events
table cleanup currently does not work properly if thetransaction
strategy is used. This is becauseDatabaseCleaner
will only apply the strategy on the default database connection. Sinceevents
is using a separate connection, the connection will not be rolled back.Take the following spec:
This will fail with the following error:
The only reason it works with
deletion
strategy is becauseevents
table is within the same database as thedefault
(primary
) connection and gets deleted/truncated whenDatabaseCleaner
cleans thedefault
database.Description
How to fix it ?
The fix is to define a specific strategy for
events
database:Note that we have to use
EventsRecord
instead ofevents
asdb
. The reason is thatDatabaseCleaner
tries to retrieve the first active record class whose connection is matching the database name. This will beApplicationRecord
since its connection (primary
/default
) uses the same database name as theevents
connection. ThenDatabaseCleaner
will use the connection from that class to handle the cleanup. In this case, the cleanup (transaction
) will happen on thedefault
database instead of theevents
. See https://github.com/DatabaseCleaner/database_cleaner-active_record/blob/912acbe1facddfe1b14f21107f6ddc9df290de53/lib/database_cleaner/active_record/base.rb#L82.Failing tests
Fixing the configuration caused a few tests to fail as they were relying on the creation of
events
. Those events used to be created without a transaction so the tests worked fine. Now we need to settransaction: false
for those tests in order for the events to be saved to the database as the default strategy forevents
is nowtransaction
.Performance impact
In order to compare the impact of the deletion on the spec performance, I ran the following command on
main
and this branch:The results I got:
main
: 49.94 secondsIf the performance impact is/become unacceptable, an alternative could be to use the
null
strategy or use a top-levelafter(:context) { DatabaseCleaner[:active_record, db: EventsRecord].clean_with(:deletion) }
for those tests.