fix #1251 Add explicit defer parameter to VirtualTimeScheduler.advanceTime_ methods#2012
fix #1251 Add explicit defer parameter to VirtualTimeScheduler.advanceTime_ methods#2012yarosla wants to merge 1 commit into
defer parameter to VirtualTimeScheduler.advanceTime_ methods#2012Conversation
…dvanceTime_` methods
|
@yarosla Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
|
@yarosla Thank you for signing the Contributor License Agreement! |
simonbasle
changed the title
defer parameter to VirtualTimeScheduler.advanceTime_ methodsdefer parameter to VirtualTimeScheduler.advanceTime_ methods
simonbasle
left a comment
simonbasle
left a comment
There was a problem hiding this comment.
ok I don't think this approach works, because it puts the empty check outside the drain() loop, which can be reached by other path than advanceTime. there can also be a situation where 2 parallel calls to advanceTime are made, with different options (although that should be way more rare). All in all, putting this parameter at the method level makes things more brittle and harder to reason about.
Given that, another possible approach would be to create the VirtualTimeScheduler with a final defer option. In the drain(), each !queue.isEmpty() check would also be checking if defer == false
You also went against my requirement that the defer mode had a default of true.
|
Your requirements were:
I will look into this closer and make changes next weekend. |
|
@yarosla ah indeed, my bad. yeah please explore the alternative solution of setting this once and for all at VTSheduler creation when you have time 👍 |
|
Created new pull request #2017 |
By default, VTS created manually don't defer the advancing of time anymore, even if task queue is empty. Those created implicitly by StepVerifier still do however, to maintain the behavior in the case where deferring advanceTime is most likely to be necessary. See-Also: #2012
|
forgot to close that one, since #2017 has superseded it |
3 participants
No description provided.