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

Keep track of transaction information #35

Merged
merged 9 commits into from
Nov 21, 2022

Conversation

Fryuni
Copy link
Contributor

@Fryuni Fryuni commented Nov 21, 2022

Closes #34

This PR adds support for testing the relation of queries and transactions.

Edit: Transaction stack is managed independently of Knee internal state since it is not exposed.


First iteration:

Knex automatically generates a new unique ID for each transaction, passed on the __knexTxId of the connection.

Each nested transaction receives a separate ID, with no relation to its parent. So this PR changes the inline connection object returned by the acquireConnection to a new MockConnection class that keeps track of the transaction stack to properly support nested transactions. Documented here

Sadly, Knex does not revert this ID once a transaction is over as, so far, this is not necessary for any of their driver implementations. As a workaround, we pass the connection down to the tracker, allowing it to walk up the transaction stack when a transaction is completed. Documented here

Also, the transaction ID generated by Knex is globally unique within a process, depending on a global state within one of its dependencies. This makes the ID impractical for tests since they are no predictable. To handle this, the transaction ID from Knex is replaced with a sequential numeric ID that is restarted when resetting the tracker. Documented here

@Fryuni
Copy link
Contributor Author

Fryuni commented Nov 21, 2022

Since we have a reference to the connection, we could also not rely on Knex's transaction ID at all.

Your call @felixmosh. I can sent the change if you want.

@Fryuni
Copy link
Contributor Author

Fryuni commented Nov 21, 2022

This also solves #32, but only when running inside transactions.

We could have a transaction with ID null representing all the queries that are executed outside of any transactions.

@Fryuni
Copy link
Contributor Author

Fryuni commented Nov 21, 2022

The new test should keep track of interleaving transactions is also an example of something that could not be tracked before.

If any of the trx1/trx2 got mixed up, there is no way to detect that till now.

src/Tracker.ts Outdated Show resolved Hide resolved
@felixmosh
Copy link
Owner

@Fryuni I've really this like this approach, and the implementation!
Thank you 🙏🏼

src/MockConnection.ts Outdated Show resolved Hide resolved
src/Tracker.ts Outdated Show resolved Hide resolved
src/Tracker.ts Outdated Show resolved Hide resolved
src/Tracker.ts Outdated Show resolved Hide resolved
@felixmosh felixmosh merged commit 30f4779 into felixmosh:master Nov 21, 2022
@felixmosh
Copy link
Owner

Again, thank you for this awesome contribution 🙏🏼

It released at v1.9.0

@Fryuni Fryuni deleted the transaction-information branch November 21, 2022 18:20
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.

Add support for testing transaction commands
2 participants