Testing about r2d2 HOT 9 CLOSED

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 getting that one connection from a cloned pool.

The injection can be done at least in two ways – either Pool could support injecting connections to the pool, or then every 3rd party ManageConnection implementation could provide a way to do that ad hoc inside the connect method.

I'd prefer seeing a general injection method, since it would unify the r2d2 ecosystem.

from r2d2.

Another thing besides the #41 would be "snatching" a connection from the pool, which would allow reusing the same test connection over multiple setups and teardowns of r2d2. @sfackler I'm interested to know what do you think about this stuff?

from r2d2.

The types would differ either way since the pool is parameterized over the manager type, right?

from r2d2.

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.

from r2d2.

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.

from r2d2.

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?

from r2d2.

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.

from r2d2.

This was solved by your suggestion, thanks. I'm going to close this.

from r2d2.

If someone comes across this, also check out: diesel-rs/diesel#2733
Might not be the best solution, and has problems with reusing connections, but it works for me.
I don't know if r2d2 has something better for testing by now. If so, let me know.

from r2d2.