-
Notifications
You must be signed in to change notification settings - Fork 51
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
test: Rework transaction test framework capabilities #1603
test: Rework transaction test framework capabilities #1603
Conversation
19b689b
to
a1b7c0b
Compare
a1b7c0b
to
1f9ecd7
Compare
action TransactionRequest2, | ||
) []datastore.Txn { | ||
if action.TransactionID >= len(txns) { | ||
txnsPointer *[]datastore.Txn, |
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.
note: The ptr param is quite ugly, but I think it is less ugly than returning txns from every test action handler. At somepoint we should probably just have a test-context object that hold txn, nodes, collections, etc, but not now.
Codecov ReportPatch coverage has no change and project coverage change:
@@ Coverage Diff @@
## develop #1603 +/- ##
===========================================
+ Coverage 73.59% 73.62% +0.03%
===========================================
Files 188 188
Lines 19608 19608
===========================================
+ Hits 14429 14435 +6
+ Misses 4112 4108 -4
+ Partials 1067 1065 -2
Flags with carried forward coverage won't be shown. Click here to find out more. see 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Note: A couple of these were broken out of 'grouped' test. This is because I have no desire to convert all of the others within the same test function, it is a much lower priority to do so, and I have quite a strong dislike of such tests as it makes debugging them much harder than they would be if there was only one test in each test func.
1f9ecd7
to
d908b67
Compare
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.
LGTM
) ## Relevant issue(s) Resolves sourcenetwork#1602 ## Description Instead of a dedicated txn action (like we currently have), we can instead follow a similar pattern to node ids where any action can specify a transaction id. I would like to use this on other actions, most immediately for Lens testing, although it will likely be very useful elsewhere too.
Relevant issue(s)
Resolves #1602
Description
Instead of a dedicated txn action (like we currently have), we can instead follow a similar pattern to node ids where any action can specify a transaction id. I would like to use this on other actions, most immediately for Lens testing, although it will likely be very useful elsewhere too.