Skip to content

Commit

Permalink
Fixed random fails of RetryAcquireTraitTest (yiisoft#17024)
Browse files Browse the repository at this point in the history
Property `RetryAcquireTrait::$retryDelay` sets the minimum delay, but not the exact delay. In fact, the delay may be longer. Because of this, sometimes less than 20 blocking attempts occur. Checking the exact number of locks sometimes leads to the following errors:
```
$ vendor/bin/phpunit --filter=testRetryAcquire
PHPUnit 4.8.34 by Sebastian Bergmann and contributors.

F

You should really fix these slow tests (>500ms)...
 1. 1050ms to run yiiunit\framework\mutex\RetryAcquireTraitTest:testRetryAcquire

Time: 4.08 seconds, Memory: 44.00MB

There was 1 failure:

1) yiiunit\framework\mutex\RetryAcquireTraitTest::testRetryAcquire
Failed asserting that 19 is identical to 20.

/yii2/tests/framework/mutex/RetryAcquireTraitTest.php:36
/yii2/vendor/phpunit/phpunit/phpunit:52

FAILURES!
Tests: 1, Assertions: 3, Failures: 1.
```

I reworked the test so that it checks the duration of the delays, but not the exact number of blocking attempts.
  • Loading branch information
rugabarbo authored and samdark committed Jan 8, 2019
1 parent 108155a commit e5ecea6
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 3 deletions.
12 changes: 11 additions & 1 deletion tests/framework/mutex/RetryAcquireTraitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,17 @@ public function testRetryAcquire()
$this->assertTrue($mutexOne->acquire($mutexName));
$this->assertFalse($mutexTwo->acquire($mutexName, 1));

$this->assertSame(20, $mutexTwo->attemptsCounter);
$this->assertGreaterThanOrEqual(1, count($mutexTwo->attemptsTime));
$this->assertLessThanOrEqual(20, count($mutexTwo->attemptsTime));

foreach ($mutexTwo->attemptsTime as $i => $attemptTime) {
if ($i === 0) {
continue;
}

$intervalMilliseconds = ($mutexTwo->attemptsTime[$i] - $mutexTwo->attemptsTime[$i-1]) * 1000;
$this->assertGreaterThanOrEqual($mutexTwo->retryDelay, $intervalMilliseconds);
}
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/framework/mutex/mocks/DumbMutex.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class DumbMutex extends Mutex
{
use RetryAcquireTrait;

public $attemptsCounter = 0;
public $attemptsTime = [];
public static $locked = false;

/**
Expand All @@ -28,7 +28,7 @@ class DumbMutex extends Mutex
protected function acquireLock($name, $timeout = 0)
{
return $this->retryAcquire($timeout, function () {
$this->attemptsCounter++;
$this->attemptsTime[] = \microtime(true);
if (!static::$locked) {
static::$locked = true;

Expand Down

0 comments on commit e5ecea6

Please sign in to comment.