Tracking Issue for Transactional Tests

[ELD]

Summary

One thing that's always been discussed after adding database pooling support as a first-class structure in rocket_contrib has been supporting transactional database tests (or some other facility for easy testing that doesn't require writing user code to rollback database changes). We've had user reports (#697) where they're unable to write integration tests with transactions because the data is rolled back before they can make assertions.

This is going to serve as a tracking issue to allow for the discussion of implementation and the expectations of such a solution. I'll update this origin message as the conversation evolves to summarize the discussion.

Prior Art

Prior art that I've researched has primarily come from the Rails framework. ActiveRecord allows for transactional fixture loading and data manipulation. However, given its integration with testing frameworks like rspec, they're able to make data assertions before the data is rolled back.

Previous Attempt(s) and outcomes

I've made a previous attempt at implementing this based on @jebrosen's suggestion that we allow a connection-customizing callback be added to a database pool. This callback would initiative a transaction before passing the connection to the handler. It would then roll the transaction back when the connection was returned to the pool (on Drop) and the database would be restored to a pristine state.

The chief pitfall with this approach is that the transaction terminates at the end of the handler, not the end of the test and thus does not allow for assertions that require database interaction. Having a connection-customizing callback may be useful for other reasons, though, but it doesn't solve this particular problem.

Here's a link to that attempt

Next Steps

So, I propose that we start a discussion where we can clarify expectations of such a feature and how such a feature could work. This hopefully drives out finer grained items that can guide implementation.

[ELD]

To add to this, I created a rough sketch of a diagram (that may not quite be correct) about how long the transaction will need to live for in order to allow for transactional testing. I'd love feedback on the diagram.

          ┌───────────────┐                            
       ┌ ─│  Test Begin   │                            
          └───────────────┘                            
       │      ┌─────────────────────────────┐          
              │       DB Pool Created       │          
┌─────┐│      └─────────────────────────────┘          
│     │       ┌─────────────────────────────┐          
│ T   ││      │   Rocket Instance Created   │          
│ r L │       └─────────────────────────────┘          
│ a i ││      ┌────────────────────────────┐           
│ n f │       │       Client Request       │           
│ s e ││      └────────────────────────────┘           
│ a t │              ┌─────────────────────────┐       
│ c i ││             │     Handler Invoked     │       
│ t m │              └─────────────────────────┘       
│ i e ││                 ┌────────────────────────────┐
│ o   │                  │Connection Fetched From Pool│
│ n   ││                 └────────────────────────────┘
│     │       ┌────────────────────────────┐           
└─────┘│      │      Client Response       │           
              └────────────────────────────┘           
       │      ┌────────────────────────────┐           
              │      Test Assertions       │           
       │      └────────────────────────────┘           
          ┌───────────────┐                            
       └ ─│   Test End    │                            
          └───────────────┘                            

[jebrosen]

This depends a bit on how exactly a "transaction" is defined. If we do actually use database-level transactions, those are often scoped to the connection they originate in. In that world, "DB Pool Created" would have to be something like "single long-lived connection created" and "connection fetched from pool" would be "take a lock on the single connection".

Some databases offer different levels of Transaction Isolation that might get around this, but afaict Postgres for example (https://www.postgresql.org/docs/12/transaction-iso.html) does not support the "see non-committed reads from other transactions" isolation level that I believe would be required to use that method.

[ELD]

@jebrosen That's a good point. Depending on the abstraction we use to represent a "transaction," the implementation is going to differ. In the case of using true database transactions, we run into issues that we've discussed before, such as potentially deadlocking a test that requires more than one database connection. Given how easy it is to need more than one connection, it's likely this would be a widespread problem.

As we discussed earlier today on IRC, another option is to create temporary databases per Rocket instance that we trash at the end of a test run. This lets us run integration tests in parallel (unlike the todo example that uses a Mutex) and gives us some transactional/hygiene guarantees with the data.

[ducharmemp]

@ELD there is some prior art in doing what you've proposed (not sure if you've talked about this before) https://github.com/hgzimmerman/diesel_test_setup however this would mean that Rocket would need to be aware of the migration/db-up strategy that each webapp should use, unless using something like pg_dump would be an option to "clone" the database before each test.

[ELD]

@ducharmemp Sorry for taking so long to get back to you.

I drew on the way Rails does transactional tests/migrations for how to go about it with Rocket. The issue I ran into, though, is that the test harness has to be aware of the request lifecycle. Since DB pools live in Rocket's managed state, the DB connection gets dropped when the handler returns a response. I started going down that route on a branch, but hit that roadblock and abandoned that approach. You can see that here: ELD@f13a56b.

I then settled on something similar to what you linked and assembled a proof-of-concept but not much more. It stands up a database for each instantiation of Rocket and then tears it down at the end. It's not efficient an the proof-of-concept is really hacky, but it works. You can check out the source here: https://github.com/ELD/db-manager.

It's been a bit since I've spent time thinking about this, but it's still something I'd like. Having an easy way to execute hygienic database integration tests would be nice.

[vhakulinen]

I've done the "setup a new database for each test" approach for many of my projects with great success. It also makes fixture loading easy if you just make sure to apply your schema to the new database.

One note on the usability thing regarding that: I usually wont drop the database after the test ends, but rather drop it before creating it again. This way, if the test fails you can go and inspect the database afterwards.

By far the biggest annoyance with this approach in rust is that I haven't figure out how to get a unique name for the database (e.g. the test function name).

Edit: Just throwing this code snippet here as an really simple example for anyone interested:

pub struct TestDB {
    pub url: String,
}

impl TestDB {
    pub async fn create(dbname: String) -> Self {
        let default_url = std::env::var("TEST_DATABASE_URL_BASE").unwrap();
        let url = format!("{}{}", default_url, dbname);

        tokio::process::Command::new("psql")
            .args([
                default_url,
                "--command".to_string(),
                format!("DROP DATABASE {}", dbname).to_string(),
                "--command".to_string(),
                format!("CREATE DATABASE {}", dbname).to_string(),
            ])
            .output()
            .await
            .expect("Failed to create test database");

        Self { url }
    }
}

This is what I use in my tests to create a tests specific database.