-
-
Notifications
You must be signed in to change notification settings - Fork 81
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
Testing #40
Comments
I played with the code a little, and it seems to be that the critical bit is being able to inject a connection from the test code to the pool – ensuring that the test uses the one true test connection should be just a matter of setting the pool size to 1, right? And getting that same connection after the code under test has run is just a matter of The injection can be done at least in two ways – either I'd prefer seeing a general injection method, since it would unify the r2d2 ecosystem. |
The types would differ either way since the pool is parameterized over the manager type, right? |
That's right, I didn't think it through when I was writing that issue. Check out the PR however, it solves the problem without changing any types. |
I'd be a bit worried that the approach in the PR is fragile to future changes - there aren't really any strong guarantees that the connection won't be replaced. There is a more robust setup here, but it involves some Postgres Deep Magicks. Specifically, you can have multiple connections in the same transaction context! You can use a connection customizer to set up the pool's connections in the right way, and use a separate connection to do the transaction management. It'd look something like this: let conn = Connection::new(url).unwrap();
conn.execute("BEGIN ISOLATION LEVEL SERIALIZABLE", &[]).unwrap();
let mut rows = conn.query("SELECT pg_export_snapshot()", &[]).unwrap();
let id: String = rows.get(0).get(0);
struct SharedTransactionCustomizer(String);
impl CustomizeConnection for SharedTransactionCustomizer {
fn on_acquire(&self, conn: &mut Connection) -> Result<(), Error> {
conn.batch_execute(format!("BEGIN ISOLATION LEVEL SERIALIZABLE; SET TRANSACTION SNAPSHOT '{}';", self.0))
}
} Though you should note that any transaction-based approach like this will fall over if any of your application logic ever wants to start one of its own transactions. |
Oh, thanks for the pointer! And yeah, I get your worries about the non-robustness of the change. I fear that the discoverability of that solution of yours is low. It would be nice to have an easily accessible and documented way in the library itself to encourage testing – for example, I had to really push myself to write tests because it wasn't obvious for me how to do that in the first place. Do you have any ideas how to improve the situation? Do you think the idea behind my PR could be incorporated if the implementation was more robust? |
I'd be hesitant to recommend a transaction-based setup in general(whether it be the single-connection pool or the shared snapshot approach) because of the issue with application-level transactions. If your test infrastructure forbids those, you're in a pretty bad place. Ideally your tests would be designed in a way where they can be run in a "dirty" database in parallel, or you could manually serialize them with a mutex or something like that. Hopefully sometime soon we'll be able to create custom test harnesses so we could make one that supports before test/after test functions and sequential execution to more robustly support these kinds of integration tests. |
This was solved by your suggestion, thanks. I'm going to close this. |
If someone comes across this, also check out: diesel-rs/diesel#2733 |
It would be very nice if r2d2 would provide some mocking abilities to be able to test that database-accessing code will perform the things expected of it.
I'd like to be able to have an integration-style test where I init the connection, start a transaction, set some state for the DB, call the tests, and inspect the state of the DB afterwards, and finally, cancel the transaction. However, as my application (that uses Diesel + r2d2 + Rocket) uses r2d2 pool, achieving this isn't easy, since r2d2 manages all the connections, and having a transaction persist from before to after the test code doesn't work.
In the end, I ended up wrapping the r2d2 pool into a newtype, and having the implementation of the API of the newtype switched for a mock with
#[cfg(test)]
. Lately I'm embracing even more problems since I'm splitting my code into a library and a binary crate, and#[cfg(test)]
applies only for the bottom crate being compiled.If r2d2 had a "generic test connection manager" where instead of a pool, there would be only one prepared connection that is passed to it, writing tests like this would be a lot easier while keeping the same type interface. What do you think?
The text was updated successfully, but these errors were encountered: