Skip to content

Use lease_connection in in_paginated_batches spec to support permanent_connection_checkout = :disallowed#8958

Merged
tagliala merged 2 commits intobugfix/8881-permanent-connection-cherrypickfrom
copilot/sub-pr-8883
Mar 6, 2026
Merged

Use lease_connection in in_paginated_batches spec to support permanent_connection_checkout = :disallowed#8958
tagliala merged 2 commits intobugfix/8881-permanent-connection-cherrypickfrom
copilot/sub-pr-8883

Conversation

Copy link
Contributor

Copilot AI commented Mar 6, 2026 •
edited
Loading

spec/support/rails_template.rb sets config.active_record.permanent_connection_checkout = :disallowed across all environments, including test. The in_paginated_batches spec was calling ActiveRecord::Base.connection.query_cache_enabled directly, which raises under that setting.

Changes

  • spec/unit/resource_controller/data_access_spec.rb: Replace ActiveRecord::Base.connection with ActiveRecord::Base.lease_connection — the Rails 7.2+ API compatible with permanent_connection_checkout = :disallowed.
# Before
expect(ActiveRecord::Base.connection.query_cache_enabled).to be_falsy

# After
expect(ActiveRecord::Base.lease_connection.query_cache_enabled).to be_falsy

rails_template.rb is unchanged — the permanent_connection_checkout = :disallowed config intentionally applies to all environments.

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Copilot AI mentioned this pull request Mar 6, 2026
Merged
@tagliala
Fix spec to use lease_connection instead of connection
efca6c3 
Replace ActiveRecord::Base.connection with ActiveRecord::Base.lease_connection
in the in_paginated_batches spec to be compatible with
permanent_connection_checkout = :disallowed set in test environment.

Co-authored-by: tagliala <[email protected]>
Copilot AI changed the title [WIP] Fix legacy connection support in test environment Use lease_connection in in_paginated_batches spec to support permanent_connection_checkout = :disallowed Mar 6, 2026
tagliala
tagliala reviewed Mar 6, 2026
View reviewed changes
Copy link
Contributor

@tagliala tagliala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot ... all good, except that it was not raising and the test did pass

Why?

Copy link
Contributor Author

Copilot AI commented Mar 6, 2026

@copilot ... all good, except that it was not raising and the test did pass

Why?

Because config.use_transactional_fixtures = true (spec/rails_helper.rb:18) wraps every test in a database transaction, a connection is already leased on the test thread before the test body runs.

permanent_connection_checkout = :disallowed only raises when .connection would cause a new permanent checkout. When a connection is already leased for the thread (as it is throughout the transactional fixture), .connection simply returns that existing connection without triggering the restriction.

The change to lease_connection is still the correct modern API for Rails 7.2+, but you're right that the old code was never actually raising in this context.

@codecov
Copy link

codecov bot commented Mar 6, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.08%. Comparing base (e6e6273) to head (efca6c3).
⚠️ Report is 1 commits behind head on bugfix/8881-permanent-connection-cherrypick.

Additional details and impacted files 
@@                             Coverage Diff                              @@
##           bugfix/8881-permanent-connection-cherrypick    #8958   +/-   ##
============================================================================
  Coverage                                        99.08%   99.08%           
============================================================================
  Files                                              139      139           
  Lines                                             4046     4046           
============================================================================
  Hits                                              4009     4009           
  Misses                                              37       37

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tagliala tagliala marked this pull request as ready for review March 6, 2026 10:27
@tagliala tagliala merged commit d867eb8 into bugfix/8881-permanent-connection-cherrypick Mar 6, 2026
27 checks passed
@tagliala tagliala deleted the copilot/sub-pr-8883 branch March 6, 2026 10:27
tagliala added a commit that referenced this pull request Mar 6, 2026
@tagliala
Use lease_connection in in_paginated_batches spec to support `per…
bcb07d7 
…manent_connection_checkout = :disallowed` (#8958)

* Initial plan

* Fix spec to use lease_connection instead of connection

Replace ActiveRecord::Base.connection with ActiveRecord::Base.lease_connection
in the in_paginated_batches spec to be compatible with
permanent_connection_checkout = :disallowed set in test environment.

Co-authored-by: tagliala <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: tagliala <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tagliala tagliala tagliala left review comments

Assignees

@tagliala tagliala

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tagliala