Skip to content
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

Provide a r2d2::CustomizeConnection for testing #4152

Closed
weiznich opened this issue Aug 9, 2024 · 3 comments · Fixed by #4186
Closed

Provide a r2d2::CustomizeConnection for testing #4152

weiznich opened this issue Aug 9, 2024 · 3 comments · Fixed by #4186

Comments

@weiznich
Copy link
Member

weiznich commented Aug 9, 2024

The r2d2 pooling crate provides hooks to customize connections on checkout from the pool. Diesel should provide a struct that implements CustomizeConnection by calling Connection::begin_test_transaction to simplify the pooling setup for integration tests. In addition we might want to extend the documentation in the diesel r2d2 module to include information about how to setup the pool for integration tests.

This involves the following steps:

  1. Add a new public TestCustomizer<C> type to the r2d2 module
  2. Implement CustomizeConnection for this type by calling Connection::begin_test_transaction in the on_acquire function
  3. Add some documentation to the new type
  4. Extend the module level documentation here to explain how to setup the pool for tests:
    • Set pool size to 1
    • Set the connection customizer
    • -> Run Tests in parallel as changes are not submitted to the database, but local to each test due to the "dangling" transaction.
  5. Add an entry to the Changelog.md file in the repository root
  6. Submit the PR
@kpagacz
Copy link
Contributor

kpagacz commented Aug 19, 2024

I would love to take on this issue. It would be my first in diesel, so I would be grateful for some guidance.

Here's where I am at:

  • I followed the contributor's guide and set up diesel locally. It passed all the tests on my machine, so I looked at the places where the changes should be made.
  • I understand that the new type is going to modify the instance of C: Connection while implementing the function on_acquire of CustomizeConnection
  • Do you want anything to happen to the connection when calling on_release or do you want it left as the default (which is no-op)

@weiznich
Copy link
Member Author

Thanks for working on this.

I understand that the new type is going to modify the instance of C: Connection while implementing the function on_acquire of CustomizeConnection

That's correct

Do you want anything to happen to the connection when calling on_release or do you want it left as the default (which is no-op)

It can use the default implementation.

And to answer the question from the Gitter channel:

And I have a question regarding the doc changes. I can see that some of the lines in this doc:

were commented seemingly in the anticipation of CustomizeConnection. My instinct is to uncomment and refactor the docs to use the newly added CustomizeConnection. Would that work for you guys?

I imagine to just extend the documentation at the linked location. That means:

  • Add a new heading "Testing with connection pools"
  • Write a few words about pool size (should be 1) and the new test connection customizer
  • Add a new example showing how to use the customizer.

@kpagacz
Copy link
Contributor

kpagacz commented Aug 19, 2024

Understood. Thanks a lot for the clarifications and the pointers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants