Fix FixtureSupport#run_in_transaction? #2495
Conversation
| include FixtureSupport | ||
| self.use_transactional_tests = true | ||
|
|
||
| uses_transaction "doesn't run in transaction" |
There was a problem hiding this comment.
Aha. So uses_transaction is a way to turn use_transactional_tests for select examples?
I missed this feature so much.
There was a problem hiding this comment.
It's not documented. But yes, based on the code and the current behavior it turns that off for the given tests.
There was a problem hiding this comment.
If its not documented I'm hesitant to support it, if its public api fine but private api I'm a bit reluctant about
There was a problem hiding this comment.
Public https://api.rubyonrails.org/classes/ActiveRecord/TestFixtures/ClassMethods.html#method-i-uses_transaction, but undocumented. Originally introduced here.
Frankly, a couple of times I needed it desperately to test code that needs to have control of the outermost transaction or has a conditional statement with a check if the transaction is an outermost or a nested.
With DatabaseCleaner it's possible to turn off per-example transaction for select examples. With use_transactional_fixtures it's not, and it's a major inconvenience.
I was under the spell that it was, and it took me a while to realize that it didn't work like that on my last project. Our documentation lacks this moment.
st0012
force-pushed
the
fix-uses_transaction
branch
from
cb415f3 to
11db9fc
Compare
st0012
force-pushed
the
fix-uses_transaction
branch
from
11db9fc to
7b4f40b
Compare
🤷♂️ Re-running. |
| uses_transaction "doesn't run in transaction" | ||
|
|
||
| it "doesn't run in transaction" do | ||
| expect(ActiveRecord::Base.connection.transaction_open?).to eq(false) |
There was a problem hiding this comment.
I'm having a big WTF moment at this, so uses_transaction turns off transactional tests?
There was a problem hiding this comment.
Yes. No doubt uses_transaction is an ambiguous and confusing naming.
|
@pirj I'll leave any decisions on this up to you, my feedback is non blocking |
`ActiveRecord::TestFixture`'s `uses_transaction` is designed to be used
like this:
```ruby
uses_transaction :the_test_method_name
```
And in RSpec, the method name would be the example's name.
```ruby
uses_transaction "does someting"
it "does someting" {}
```
But in the current implementation, it's passing the example object
instead of its name, which would always fail the name comparison in
https://github.com/rails/rails/blob/adc0146a07ccc72405aec78ccb65aac3502a4300/activerecord/lib/active_record/test_fixtures.rb#L94-L97
So this commit fixes the issue by passing the example's name instead of
the example object.
pirj
force-pushed
the
fix-uses_transaction
branch
from
c038413 to
9ce49af
Compare
|
Thank you for the contribution and for revealing the mystery behind |
|
@pirj I've been using rspec-rails for years and it's finally my turn to make a small contribution 🙂 |
Fix FixtureSupport#run_in_transaction?
ActiveRecord::TestFixture'suses_transactionis designed to be used like this:And in RSpec, the method name would be the example's name.
But in the current implementation, it's passing the example object instead of its name, which would always fail the name comparison in https://github.com/rails/rails/blob/main/activerecord/lib/active_record/test_fixtures.rb#L94-L97
So this PR fixes the issue by passing the example's name instead of the example object.