Async setUp & tearDown #13
Conversation
src/AsyncTestCase.php
Outdated
| $promise = call([$this, $this->realTestName], ...$args); | ||
| $promise->onResolve(function ($error, $value) use (&$invoked, &$exception, &$returnValue) { | ||
| $invoked = true; | ||
| $exception = $error; | ||
| $returnValue = $value; | ||
| yield $this->tearDownAsync(); |
There was a problem hiding this comment.
There shouldn't be a yield inside onResolve.
There was a problem hiding this comment.
Why not? It seems to be supported: https://github.com/amphp/amp/blob/c34b6791429c0c55d8b66df748a1c265d6b9e75a/lib/Internal/Placeholder.php#L45-L54
| @@ -146,4 +149,14 @@ abstract class AsyncTestCase extends PHPUnitTestCase | |||
|
|
|||
| return $mock; | |||
| } | |||
|
|
|||
| protected function setUpAsync(): Promise | |||
There was a problem hiding this comment.
Not sure whether we should use this name or something like beforeEach.
There was a problem hiding this comment.
I'm against, because those names (without Asnyc suffix) are used by PHPUnit and people will immediately understand what those are used for.
|
It's just a few lines of code, can you make something like this work? I'm
not sure how to setup this correctly.
…On Sun, Mar 29, 2020, 16:17 Niklas Keller ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/AsyncTestCase.php
<#13 (comment)>:
> $promise = call([$this, $this->realTestName], ...$args);
$promise->onResolve(function ($error, $value) use (&$invoked, &$exception, &$returnValue) {
$invoked = true;
$exception = $error;
$returnValue = $value;
+ yield $this->tearDownAsync();
There shouldn't be a yield inside onResolve.
------------------------------
In src/AsyncTestCase.php
<#13 (comment)>:
> @@ -146,4 +149,14 @@ abstract class AsyncTestCase extends PHPUnitTestCase
return $mock;
}
+
+ protected function setUpAsync(): Promise
Not sure whether we should use this name or something like beforeEach.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#13 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADAJPHRN6EC24P4Y3UJ6KDRJ6UGTANCNFSM4LWDL5MA>
.
|
|
@kelunik I changed the code a bit and added a test case. Is this good for you to merge? |
| class AsyncTestCaseWithSetUpAndTearDownTest extends AsyncTestCase | ||
| { | ||
| protected $setupCalled = false; | ||
| protected $tearDownCalled = false; |
There was a problem hiding this comment.
These should be reset in setup.
| @@ -55,7 +57,21 @@ abstract class AsyncTestCase extends PHPUnitTestCase | |||
| $start = \microtime(true); | |||
|
|
|||
| Loop::run(function () use (&$returnValue, &$exception, &$invoked, $args) { | |||
| $promise = call([$this, $this->realTestName], ...$args); | |||
| yield $this->setUpAsync(); | |||
There was a problem hiding this comment.
Setup could be moved inside the call, too.
| } catch (\Throwable $e) { | ||
| } | ||
|
|
||
| yield $this->tearDownAsync(); |
There was a problem hiding this comment.
Exceptions thrown from teardown should probably be ignored if the actual test already threw an exception, otherwise they're hidden here?
|
Had these comments in a pending state the whole time. |
No description provided.