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

Allow manual transaction management #47

Merged
merged 2 commits into from Jul 29, 2022
Merged

Allow manual transaction management #47

merged 2 commits into from Jul 29, 2022

Conversation

simolus3
Copy link
Contributor

The connection FSM expected a _TransactionProxy when receiving transaction messages, but it's actually safe to just expect a general execution context implementation.

Closes #43.

Copy link
Owner

@isoos isoos left a comment

Choose a reason for hiding this comment

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

This looks good to me.

Could you also please bump the version and add a changelog entry (in a similar format as the previous one, attributing the PR)?

@isoos isoos merged commit 82a142a into isoos:master Jul 29, 2022
@isoos
Copy link
Owner

isoos commented Jul 29, 2022

published as 2.4.6, thank you!

await conn.execute('ROLLBACK');

await expectLater(conn.query('SELECT * FROM t'), completion(isEmpty));
});
Copy link

@marcotas marcotas Jul 29, 2022

Choose a reason for hiding this comment

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

@simolus3 and @isoos is it possible to run queries inside the transaction? Something like this:

test('some tests in a transaction', () {
  await conn.execute('begin');
  await conn.execute('insert somethings');
  var results = await conn.query('select somethings');
  expect(results.length, 2);
  await conn.execute('rollback');
});

The reason why is because I'm running each tests inside transactions to increase test performance significantly. Laravel framework does this in their tests and it works perfectly. I've being using this approach for years and it is a very good feature during testing. This is a real example of how I can run the tests on Tunder framework (for dart):

main() {
  group('Some tests', () {
    useDatabaseTransactions();
    
    test('this test will execute inside a transction so you can follow the AAA approach', () async {
      // Arrange
      ContactFactory().createMany(10);
      
      // Act
      var contacts = Query<Contact>().paginate(5);
      
      // Assert
      expect(contacts.data.length, 5);
      expect(contacts.meta.perPage, 5);
    });
  });
}

We have many benefits of this approach:

  1. Faster queries because they're executed in memory and not really persisted in the hard drive or SSD. So no heavy IO operations.
  2. Cleaner tests because we don't need to cleanup the database after each test to make them isolated and less queries per test.
  3. Each test runs completely isolated from contexts because after each test the transaction is rolled back.

If this is not possible right now, how hard would be to implement something like this?

I'm really really looking forward to work with this. If this is possible I'll be definitely migrating it to this package. And thank you so much for all of you guys maintaining this package.

Copy link
Owner

Choose a reason for hiding this comment

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

In other projects we are using testWithX methods, e.g. testWithDbTransactions, where the helper method sets up the test, prepends it with DB connection initialization and cleanup afterwards. It is nice as the test's inner method is a callback, so you can do anything before/after easily.

For this specific case, I'm not familiar what the contactfactory does, but it looks like as a global variable, which will usually cause conflicts with tests that may be running concurrently.

Choose a reason for hiding this comment

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

Great! Thank you for sharing that. It seems it's a pattern in Dart community. I'm reading the tests to understand the behaviour of the package. Thank you for your help @isoos 👍🏻

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

Successfully merging this pull request may close these issues.

Exception when using transaction manually
3 participants