Nodejs transaction redesign feature branch 4 all call types #4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Still an issue with system tests. commit still needs tweaks to work with async.
Remove the no-op, get commit tests in place.
Mocks and additional debugging hooks to introspect what is going on.
Make sure commit behaves the same way as before.
…t-tests' of https://github.com/danieljbruce/nodejs-datastore into nodejs-transaction-redesign-feature-branch-3-introducing-async-commit # Conflicts: # test/transaction.ts
Make the promise simpler. Change the tests to exclude functions with promisify.
The mutex and state should be moved down to the derived class. We are going to override get/runQuery/runAggregateQuery there.
Add the hook to call run before calling commit in existing tests.
commitAsync should be resolved and then() function should be called as it was not being called before
Use the private modifier instead to hide data
withBeginTransaction will be used with all calls that begin transactions and then intend to use the mutex for locking when the begin transaction call is made.
Add #beginWithCallback so that all the read calls can be made with one line of code.
The generic parameter should be used for types with run.
More specific types are better and it make code easier to read
runQuery should make the call to begin the transaction first if that hasn’t already happened yet.
Fixes for run aggregation query. Still getting undefined results.
Make sure that runAggregateQuery return results make it back to the user.
runAggregateQuery should not be excluded by promisify. Otherwise it will not return a promise, but we want it to return a promise.
Change the test to use deep strict equal since the objects being compared will not be reference equal.
Eliminate some unused code
…t-tests' of https://github.com/danieljbruce/nodejs-datastore into nodejs-transaction-redesign-feature-branch-4-all-call-types # Conflicts: # test/transaction.ts
Add modifications to runQuery to use some function to return values
…t-tests' of https://github.com/danieljbruce/nodejs-datastore into nodejs-transaction-redesign-feature-branch-4-all-call-types
Don’t start commit with a promise or the promise will run early.
get should use the same pattern as runQuery and runAggregationQuery to use a resolver for the mutex business logic
#beginWithCallback is no longer used so remove it
This function is no longer used
#someFunction should be named differently
A deepStrictEqual check is all that is needed in this test that currently reuses a transaction.
If we add a delay to the tests then the mutex has the opportunity to unlock before running the tests.
We want to make sure that the calls get delivered and received in order
Put the testing tool in an object so that we can explore more orderings and possibilities.
Allow expected order to be passed into the tester. This will make the object reusable.
Need a check to be sure that the transaction is not in progress if making another beginTransaction call.
…mit' of https://github.com/danieljbruce/nodejs-datastore into nodejs-transaction-redesign-feature-branch-2a-adding-commit-and-tests # Conflicts: # test/transaction.ts
Remove the NOT_TRANSACTION and default to NOT_STARTED
Remove only and let all unit tests run
Use an enum to track transaction state.
Excludes should contain the right functions in the right order.
Regroup functionality to simplify the run function
…mmit-and-tests' of https://github.com/danieljbruce/nodejs-datastore into nodejs-transaction-redesign-feature-branch-4-all-call-types # Conflicts: # src/transaction.ts # test/transaction.ts
Explain purpose of testing objects
The merge added a bug because the pound sign wasn’t used and it should have been.
Remove constructor parameter that is not used. Add comments to functions so that the tests are more readable.
Functions in transaction.ts need JSdoc comments.
Add JSdoc comment
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕