Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix timing sensitive flaky test #48663

Closed
wants to merge 1 commit into from
Closed

Fix timing sensitive flaky test #48663

wants to merge 1 commit into from

Conversation

KentarouTakeda
Copy link
Contributor

Summary

Fixed the issue where the Queue retry time test, when using the Redis Driver, would occasionally fail.

For example, #48662 (comment) also seems to have encountered this problem.

An earlier attempt was made to address this problem by retrying the test itself, as seen in commit e0d8e3d .
However, it was not a fundamental solution.

In the test, we specified a delay of 10 minutes and asserted that the actual delay was 600 seconds . However, due to the time elapsed during the test execution, the test may rarely reach 599 seconds and fail. I solved this problem by fixing the time using Carbon::setTestNow() at the beginning of the test.

Before the fix, the test would fail after about 30 to 100 attempts. After the fix, I tried it 10,000 times and it didn't fail.

Note

It actually asserts for more than 600 seconds. This is because redis-server calculates the retry time, so it cannot be controlled with setTestNow().

public function acquire()
{
$results = $this->redis->eval(
$this->luaScript(), 1, $this->name, microtime(true), time(), $this->decay, $this->maxLocks
);
$this->decaysAt = $results[1];
$this->remaining = max(0, $results[2]);
return (bool) $results[0];
}

@github-actions
Copy link

github-actions bot commented Oct 7, 2023

Thanks for submitting a PR!

In order to review and merge PRs most efficiently, we require that all PRs grant maintainer edit access before we review them. For information on how to do this, see the relevant GitHub documentation. Additionally, GitHub doesn't allow maintainer permissions from organization accounts. Please resubmit this PR from a personal GitHub account with maintainer permissions enabled.

@github-actions github-actions bot closed this Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant