Skip to content

Fix parallel testing (issue #48547) - #48560

Merged
eileencodes merged 1 commit into
rails:mainfrom
eileencodes:fix-parallel-testing-48547
Jul 5, 2023
Merged

Fix parallel testing (issue #48547)#48560
eileencodes merged 1 commit into
rails:mainfrom
eileencodes:fix-parallel-testing-48547

Conversation

@eileencodes

Copy link
Copy Markdown
Member

Fixes 2 bugs in parallel testing.

The first bug was related to changes made in #45450 which meant that we were no longer replacing the connection in parallel testing because the config object is equal (we simply merge a new db name but the object id of the config stays the same). This bug only manifested in mysql and sqlite3 interestingly. It would fail on the internal metadata tables because they were missing in the schema version check.

To fix this I introduced a clobber: true kwarg onto the connection handler that allowsus to bypass the functionality that won't make a new connection if the config is the same. This is an easy way to fall back to the old behavior from before this change.

After implementing this fix I was still seeing failures in the mysql demo app I made due to the fact that purge was not re-establishing the connection to a config that had a database defined. Neither sqlite3 or postgresql were missing this.

I added a test for mysql2 so we don't have regressions in the future. I think this was missed because sqlite3 only demonstrates the bug if it was never successful on that worker and postgresql was fine.

Fixes #48547

cc/ @matthutchinson

@matthutchinson

Copy link
Copy Markdown

Thanks @eileencodes !

I've verified this fix works on my example app (sqlite3)

@eileencodes
eileencodes force-pushed the fix-parallel-testing-48547 branch 3 times, most recently from dd88153 to 4b3f27d Compare June 23, 2023 12:53
Fixes 2 bugs in parallel testing.

The first bug was related to changes made in rails#45450 which meant that we were
no longer replacing the connection in parallel testing because the
config object is equal (we simply merge a new db name but the object id
of the config stays the same). This bug only manifested in mysql and
sqlite3 interestingly. It would fail on the internal metadata tables
because they were missing in the schema version check.

To fix this I introduced a `clobber: true` kwarg onto the connection
handler that allows us to bypass the functionality that won't make a new
connection if the config is the same. This is an easy way to fall back
to the old behavior from before this change. I only added `clobber`
to the `reconstruct_from_schema` call because we need to actually
replace the connection for these. It's not safe to add everywhere since
we don't always want to replace the connection.

After implementing this fix I was still seeing failures in the mysql
demo app I made due to the fact that `purge` was not re-establishing the
connection to a config that had a database defined. Neither sqlite3 or
postgresql were missing this.

I added a test for mysql2 so we don't have regressions in the future. I
think this was missed because sqlite3 only demonstrates the bug if it
was never successful on that worker and postgresql was fine.

Fixes rails#48547
@eileencodes
eileencodes force-pushed the fix-parallel-testing-48547 branch from 4b3f27d to 5cf5a2a Compare July 5, 2023 14:51
@eileencodes
eileencodes merged commit 190a438 into rails:main Jul 5, 2023
@eileencodes
eileencodes deleted the fix-parallel-testing-48547 branch July 5, 2023 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

parallelize test runner broken in 7.1.0.alpha

2 participants