Convert some async TestCase code to coroutines #12375
Convert some async TestCase code to coroutines #12375adiroiban merged 11 commits intotwisted:trunkfrom
Conversation
CodSpeed Performance ReportMerging #12375 will degrade performances by 3.67%Comparing Summary
Benchmarks breakdown
|
|
please review |
adiroiban
left a comment
There was a problem hiding this comment.
Thanks for the cleanup and sorry for the delay in reviewing.
The changes looks good.
My only concern is about removign the public method.
|
With rejection of #12387 I don't think this PR has benefits, therefore I'm closing it. |
|
I wish we had some benchmarks here, because I wonder if this speed up Trial tests (which would be very much worthwhile!). |
|
Benchmarks have been added in #12393. On my machine I see improvement of around 2%, so probably it makes sense to land this PR still. |
65d4300 to
0ab3a19
Compare
0c17f26 to
f523f9e
Compare
bae8065 to
f523f9e
Compare
|
@adiroiban I have addressed all your comments. |
adiroiban
left a comment
There was a problem hiding this comment.
Looks good. Thanks for the refactoring.
Only a minor comment.
f523f9e to
5db18a1
Compare
670558b to
11d3e5b
Compare
This PR cleans up some asynchronous TestCase code. Previous code was relatively hard to follow due to it relying on callbacks based implementation, which forced the logic to be split across many small functions. The new code puts everything into regular code flow across several functions and uses standard
try...exceptandtry...finallyfor error handling.The code would be even simplier if
IReporter.addErrorsupported regular exceptions, but this is for a future.The PR has been split across many small commits so that it is possible to bisect the changes in the future if there are any bugs that may be caused by this PR. Also, review should be easier, as the changes in isolation are almost trivial.