-
Notifications
You must be signed in to change notification settings - Fork 69
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
Improve tests & result builder classes #193
Conversation
ali-ince
commented
Jul 3, 2017
- Added tests for Session#RunAsync, Session#BeginTransactionAsync and Session#Dispose after async calls,
- Added tests for Transaction#RunAsync,
- Improved ResultBuilder & ResultReaderBuilder with a new base class including common members from both classes,
- Added tests for ResultReaderBuilder & StatementResultReader,
- Added Session#BeginTransactionAsync(string bookmark) method.
1. Added tests for Session#RunAsync, Session#BeginTransactionAsync and Session#Dispose after async calls, 2. Added tests for Transaction#RunAsync, 3. Improved ResultBuilder & ResultReaderBuilder with a new base class including common members from both classes, 4. Added tests for ResultReaderBuilder & StatementResultReader, 5. Added Session#BeginTransactionAsync(string bookmark) method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few naming comments.
@@ -427,6 +427,13 @@ public Task<ITransactionAsync> BeginTransactionAsync() | |||
return TryExecuteAsync(async()=> await BeginTransactionWithoutLoggingAsync(_defaultMode)); | |||
} | |||
|
|||
public Task<ITransactionAsync> BeginTransactionAsync(string bookmark) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be no BeginTx with bookmark as it is deprecated method already
@@ -23,23 +23,20 @@ | |||
|
|||
namespace Neo4j.Driver.Internal.Result | |||
{ | |||
internal class ResultReaderBuilder : IMessageResponseCollector | |||
internal class ResultReaderBuilder : BuilderBase |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it easy to break the logic of IMessageResponseCollector
to an independent class such as MessageResponseCollector
and have an field in this class instead?
Suggesting to change in another PR if agreed.
@@ -85,7 +82,7 @@ public IStatementResultReader PreBuild() | |||
return _records.Count > 0 ? _records.Dequeue() : null; | |||
} | |||
|
|||
private void SetReceiveOneFunc(Func<Task> receiveOneAction) | |||
internal void SetReceiveOneAction(Func<Task> receiveOneAction) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About naming:
It is actually a function, so suggesting to name this to SetReceiveOneFunc