Fix failing tests

[kelunik]

Test results when running locally with PHP 7.1.3RC1 and uv and event extensions.

PHPUnit 6.0.8 by Sebastian Bergmann and contributors.

Error:         No code coverage driver is available

...............................................................  63 / 624 ( 10%)
..........................................................SSSSS 126 / 624 ( 20%)
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS 189 / 624 ( 30%)
SSSSSSSSSSSSSSSSSSSSSSS..........S............................. 252 / 624 ( 40%)
.....S.....................SSS...............................S. 315 / 624 ( 50%)
.................................S.....................SSS..... 378 / 624 ( 60%)
..........................S.................................FS. 441 / 624 ( 70%)
....................SSS........................................ 504 / 624 ( 80%)
................................F.............................. 567 / 624 ( 90%)
.........................................................       624 / 624 (100%)

Time: 11.83 seconds, Memory: 37.01MB

There were 2 failures:

1) Amp\Test\Loop\UvDriverTest::testExecutionOrderGuarantees
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'01 02 03 04 05 05 05 05 05 05 05 05 10 11 12 13 13 13 13 20 21 21 21 21 30 40 41 '
+'01 02 03 04 05 05 05 05 05 10 11 12 13 05 20 21 21 21 21 30 05 05 40 13 13 13 41 '

2) Amp\Test\ProducerTest::testEmitReactBackPressure
Failed asserting that 299.91698265075684 is greater than 300.

/home/kelunik/GitHub/amphp/amp/test/ProducerTest.php:140

FAILURES!
Tests: 8814, Assertions: 1275, Failures: 2, Skipped: 106.

[kelunik]

Also, all signal tests are skipped for me right now.

[kelunik]

Signal tests have been fixed, all timer tests have a grace period of 1 millisecond now, but the UvDriverTest::testExecutionOrderGuarantees is still broken.

[KorvinSzanto]

So the issue with this is delaying from what I can tell. I've written a shorter test that's a little easier to follow and displays the issue more clearly:

    public function testExecutionOrder()
    {
        // Our expected output
        $this->expectOutputString('defer writable reenabled delay ');

        // Set up a closure to output
        $output = function(Driver $loop, string $output) {
            $called = false;
            // Return a function that tracks whether it has been called
            return function($id) use ($loop, $output, &$called) {
                if ($called) {
                    $this->fail('Watcher callback called too often');
                }

                $called = true;
                $loop->cancel($id);
                echo $output . ' ';
            };
        };

        // Begin a loop for testing
        $this->start(function (Driver $loop) use (&$ticks, $output) {
            // Lets start with the reenabled watcher
            $reenabled = $loop->onWritable(STDIN, $output($loop, 'reenabled'));
            $loop->disable($reenabled);

            // Lets defer something now
            $loop->defer($output($loop, 'defer'));

            // Move on to delay
            $loop->delay(0, $output($loop, 'delay'));

            // Writable
            $loop->onWritable(STDIN, $output($loop, 'writable'));

            // Reenable
            $loop->enable($reenabled);
        });

    }

Adding this to the DriverTest class will result in a failure for the UV driver looking like this:

1) Amp\Test\Loop\UvDriverTest::testExecutionOrder
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'defer writable reenabled delay '
+'defer delay writable reenabled '

Which tells me that UV's timer functionality is perhaps acting unexpectedly with low millisecond delays. I threw the whole of the handling code for delay in a $this->defer block, but that didn't really do much beyond move the delay to output after writable.

Is there a good spot where I could talk to someone who maybe knows more about this than I do?

[kelunik]

Actually, we do not guarantee timer and IO watcher order according to

amp/lib/Loop/Driver.php

Line 55 in 95b3f62

* 3. Execute all due timer, pending signal and actionable stream callbacks, each only once per tick.

. I think we should just remove that test.

[KorvinSzanto]

Well, perhaps we should redo the test in a way that allows for that and makes it clear that that may happen. Let me see if I can come up with anything today and I'll submit a PR.

[kelunik]

The test totally needs a redo, it's horrible. Thanks for looking into that. 👍