StreamSelectLoop: Use fresh time so Timers added during stream events are accurate

[andrewminerd]

No description provided.

[clue]

I understand this is a bit tricky, but can we add a test case for this? 👍

[andrewminerd]

Haha, had a feeling that was coming. :-p Yeah, a little tricky, but I'll see what I can do!

[WyriHaximus]

IIRC partial mocks could do the trick :)

[cebe]

for mocking time functions you can override them inside of a namespace, like it's done here: https://github.com/yiisoft/yii2/blob/ae1fbdd737720c614ca3ee7e2d94e0173e40c46d/tests/framework/caching/CacheTestCase.php#L3-L24

functions defined in the namespace have preference over functions in root namespace.

[andrewminerd]

I apologize for the lapse...

@cebe Thanks for the suggestion! However, this would require enabling process isolation during the PHPUnit run to prevent the time functions from being replaced in all tests (some of which seem to require 'real' time functions), so for now I haven't gone this route. Perhaps @clue could indicate whether this is something that would be acceptable.

@WyriHaximus Thanks as well! Unfortunately, as the current fix uses microtime directly, I could really only mock out half of the microtime calls, which, while master failed and my branch passed, felt a bit like cheating. Another option would be to create a method that just wraps microtime which could be used in both locations within the Timers class and be mocked out in the test.

For now, I've used a real sleep in the test. Obviously this isn't optimal, but felt like the least fragile method at present. If you'd prefer a different solution, let me know!

[cebe]

@cebe Thanks for the suggestion! However, this would require enabling process isolation during the PHPUnit run to prevent the time functions from being replaced in all tests (some of which seem to require 'real' time functions), so for now I haven't gone this route. Perhaps @clue could indicate whether this is something that would be acceptable.

This should not be a problem, as the mock function I linked is implemented in such a way that it returns the mock value if a property of the test is not null but the real time if the test property is null. This way you can enable mock by simply setting the value.

[clue]

The tests may not be perfect, but they LGTM until we can come up with anything better 👍 Can you amend your changes to conform to the existing code style (PSR-2)?

[Tatikoma]

Thanks for fix. I got this issue with predis-async library and this fix worked for me.

This should merged to master, it's very-very issue. Reproduce:
create predis-async client instance.
do nothing 10 seconds.
try to connect.

You 'll receive connection time-out immediately (and after, connection 'll be ok).