Merge pull request #551: activate Acceptance/Extra tests, fix bugs

roxblnfk · web-flow · commit fc67190d6176 · 2025-01-14T03:38:16.000+04:00
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
@@ -18,6 +18,7 @@
 >
     <testsuites>
         <testsuite name="Acceptance">
+            <directory suffix="Test.php">tests/Acceptance/Extra</directory>
             <directory suffix="Test.php">tests/Acceptance/Harness</directory>
         </testsuite>
         <testsuite name="Arch">
diff --git a/src/Internal/Declaration/Dispatcher/Dispatcher.php b/src/Internal/Declaration/Dispatcher/Dispatcher.php
@@ -120,7 +120,10 @@ private function createExecutorFromFunction(\ReflectionFunction $fun): \Closure
                     throw new \BadMethodCallException($message, $type, $error);
                 });
 
-                return $closure->call($ctx, ...$arguments);
+
+                return $fun->isStatic()
+                    ? $closure->bindTo(null, $ctx::class)?->__invoke(...$arguments) ?? $closure(...$arguments)
+                    : $closure->call($ctx, ...$arguments);
             } finally {
                 \restore_error_handler();
             }
diff --git a/src/Internal/Workflow/Process/DeferredGenerator.php b/src/Internal/Workflow/Process/DeferredGenerator.php
@@ -89,7 +89,7 @@ public function send(mixed $value): mixed
      */
     public function getReturn(): mixed
     {
-        // $this->start();
+        $this->finished or throw new \LogicException('Cannot get return value of a generator that was not finished.');
         try {
             return $this->generator->getReturn();
         } catch (\Throwable $e) {
@@ -149,7 +149,8 @@ public function valid(): bool
     {
         $this->start();
         try {
-            return $this->generator->valid();
+            $result = $this->generator->valid() or $this->finished = true;
+            return $result;
         } catch (\Throwable $e) {
             $this->handleException($e);
         }
@@ -218,9 +219,9 @@ private function handleException(\Throwable $e): never
     {
         $this->finished and throw $e;
         $this->finished = true;
-        foreach ($this->catchers as $catch) {
+        foreach ($this->catchers as $catcher) {
             try {
-                $catch($e);
+                $catcher($e);
             } catch (\Throwable) {
                 // Do nothing.
             }
diff --git a/src/Internal/Workflow/Process/Scope.php b/src/Internal/Workflow/Process/Scope.php
@@ -186,7 +186,8 @@ public function startSignal(callable $handler, ValuesInterface $values, string $
      */
     public function attach(\Generator $generator): self
     {
-        $this->coroutine = DeferredGenerator::fromGenerator($generator);
+        $this->coroutine = DeferredGenerator::fromGenerator($generator)
+            ->catch($this->onException(...));
 
         $this->next();
         return $this;
@@ -386,13 +387,13 @@ protected function next(): void
         begin:
         $this->context->resolveConditions();
 
-        if (!$this->coroutine->valid()) {
-            try {
+        try {
+            if (!$this->coroutine->valid()) {
                 $this->onResult($this->coroutine->getReturn());
-            } catch (\Throwable) {
-                $this->onResult(null);
+                return;
             }
-
+        } catch (\Throwable) {
+            $this->onResult(null);
             return;
         }
 
@@ -418,11 +419,7 @@ protected function next(): void
                 break;
 
             case $current instanceof \Generator:
-                try {
-                    $this->nextPromise($this->createScope(false)->attach($current));
-                } catch (\Throwable $e) {
-                    $this->coroutine->throw($e);
-                }
+                $this->nextPromise($this->createScope(false)->attach($current));
                 break;
 
             default:
diff --git a/src/Workflow.php b/src/Workflow.php
@@ -18,6 +18,7 @@
 use Temporal\Client\WorkflowStubInterface;
 use Temporal\DataConverter\Type;
 use Temporal\DataConverter\ValuesInterface;
+use Temporal\Exception\Failure\CanceledFailure;
 use Temporal\Exception\OutOfContextException;
 use Temporal\Internal\Support\Facade;
 use Temporal\Internal\Workflow\ScopeContext;
@@ -958,6 +959,9 @@ public static function uuid7(?\DateTimeInterface $dateTime = null): PromiseInter
      * Run a function when the mutex is released.
      * The mutex is locked for the duration of the function.
      *
+     * Note that calling the method creates a non-detached asynchronous context {@see Workflow::async()}.
+     * Closing the context using the `cancel()` method will reject the returned promise with a {@see CanceledFailure}.
+     *
      * @template T
      * @param Mutex $mutex Mutex name or instance.
      * @param callable(): T $callable Function to run.
diff --git a/tests/Acceptance/Extra/Schedule/ScheduleClientTest.php b/tests/Acceptance/Extra/Schedule/ScheduleClientTest.php
@@ -40,6 +40,17 @@ public function listSchedulesWithQuery(
             );
         }
 
+        // Wait for schedules to be created
+        $deadline = \microtime(true) + 5;
+        check:
+        $paginator = $client->listSchedules(
+            pageSize: 10,
+            query: 'bar = 4242'
+        );
+        if (\count($paginator->getPageItems()) < 6 && \microtime(true) < $deadline) {
+            goto check;
+        }
+
         try {
             $paginator = $client->listSchedules(
                 pageSize: 5,
diff --git a/tests/Acceptance/Extra/Update/UntypedStubTest.php b/tests/Acceptance/Extra/Update/UntypedStubTest.php
@@ -9,7 +9,7 @@
 use Temporal\Client\WorkflowClientInterface;
 use Temporal\Client\WorkflowStubInterface;
 use Temporal\Exception\Client\TimeoutException;
-use Temporal\Exception\Client\UntypedStubException;
+use Temporal\Exception\Client\WorkflowUpdateException;
 use Temporal\Internal\Support\DateInterval;
 use Temporal\Tests\Acceptance\App\Attribute\Stub;
 use Temporal\Tests\Acceptance\App\TestCase;
@@ -37,7 +37,11 @@ public function fetchResolvedResultAfterWorkflowCompleted(
 
         $this->assertSame(['key' => 'resolved'], (array)$result, 'Workflow result contains resolved value');
         $this->assertFalse($handle->hasResult());
-        $this->assertFalse($resolver->hasResult(), 'Resolver should not have result because of wait policy');
+
+        // Since Temporal CLI 1.2.0, the result is available immediately after the operation
+        $this->assertTrue($resolver->hasResult());
+        $this->assertSame('resolved', $resolver->getResult());
+
         // Fetch result
         $this->assertSame('resolved', $handle->getResult());
         $this->assertTrue($handle->hasResult());
@@ -88,7 +92,7 @@ public function handleUnknownUpdate(
         try {
             $stub->startUpdate('unknownUpdateMethod', '42');
             $this->fail('Should throw exception');
-        } catch (UntypedStubException $e) {
+        } catch (WorkflowUpdateException $e) {
             $this->assertStringContainsString(
                 'unknown update method unknownUpdateMethod',
                 $e->getPrevious()->getMessage(),
diff --git a/tests/Acceptance/Extra/Update/UpdateWithStartTest.php b/tests/Acceptance/Extra/Update/UpdateWithStartTest.php
@@ -60,7 +60,7 @@ public function failWithBadUpdateName(
             try {
                 $stub->getResult(timeout: 1);
                 $this->fail('Workflow must fail');
-            } catch (WorkflowFailedException $e) {
+            } catch (WorkflowFailedException) {
                 $this->assertTrue(true);
             }
         }
diff --git a/tests/Acceptance/Extra/Workflow/AllHandlersFinishedTest.php b/tests/Acceptance/Extra/Workflow/AllHandlersFinishedTest.php
@@ -33,7 +33,11 @@ public function updateHandlersWithOneCall(
 
         $this->assertSame(['key' => 'resolved'], (array) $result, 'Workflow result contains resolved value');
         $this->assertFalse($handle->hasResult());
-        $this->assertFalse($resolver->hasResult(), 'Resolver should not have result because of wait policy');
+
+        // Since Temporal CLI 1.2.0, the result is available immediately after the operation
+        $this->assertTrue($resolver->hasResult());
+        $this->assertSame('resolved', $resolver->getResult());
+
         // Fetch signal's result
         $this->assertSame('resolved', $handle->getResult());
         $this->assertTrue($handle->hasResult());
diff --git a/tests/Acceptance/Extra/Workflow/MutexRunLockedTest.php b/tests/Acceptance/Extra/Workflow/MutexRunLockedTest.php
@@ -8,6 +8,7 @@
 use React\Promise\PromiseInterface;
 use Temporal\Client\WorkflowStubInterface;
 use Temporal\DataConverter\Type;
+use Temporal\Exception\Failure\CanceledFailure;
 use Temporal\Tests\Acceptance\App\Attribute\Stub;
 use Temporal\Tests\Acceptance\App\TestCase;
 use Temporal\Workflow;
@@ -28,6 +29,7 @@ public function runLockedWithGeneratorAndAwait(
         $this->assertTrue($result[0], 'Mutex must be unlocked after runLocked is finished');
         $this->assertTrue($result[1], 'The function inside runLocked mist wait for signal');
         $this->assertTrue($result[2], 'Mutex must be locked during runLocked');
+        $this->assertNull($result[3], 'No exception must be thrown');
     }
 
     #[Test]
@@ -41,6 +43,7 @@ public function runLockedAndCancel(
 
         $this->assertTrue($result[0], 'Mutex must be unlocked after runLocked is cancelled');
         $this->assertNull($result[2], 'Mutex must be locked during runLocked');
+        $this->assertSame(CanceledFailure::class, $result[3], 'CanceledFailure must be thrown');
     }
 }
 
@@ -64,7 +67,12 @@ public function __construct()
     #[Workflow\ReturnType(Type::TYPE_ARRAY)]
     public function handle(): \Generator
     {
-        $result = yield $this->promise = Workflow::runLocked($this->mutex, $this->runLocked(...));
+        $exception = null;
+        try {
+            $result = yield $this->promise = Workflow::runLocked($this->mutex, $this->runLocked(...));
+        } catch (\Throwable $e) {
+            $exception = $e::class;
+        }
 
         $trailed = false;
         yield Workflow::await(
@@ -78,7 +86,7 @@ public function handle(): \Generator
         // that was created inside the first runLocked
         $trailed and throw new \Exception('The trailed runLocked must not be executed.');
 
-        return [$this->unlocked, $this->unblock, $result];
+        return [$this->unlocked, $this->unblock, $result, $exception];
     }
 
     #[Workflow\SignalMethod]
diff --git a/tests/Acceptance/Harness/ChildWorkflow/ThrowOnExecuteTest.php b/tests/Acceptance/Harness/ChildWorkflow/ThrowOnExecuteTest.php
@@ -19,7 +19,7 @@
 class ThrowOnExecuteTest extends TestCase
 {
     #[Test]
-    public static function check(#[Stub('Harness_ChildWorkflow_ThrowsOnExecute')]WorkflowStubInterface $stub): void
+    public static function throwExceptionOnInit(#[Stub('Harness_ChildWorkflow_ThrowsOnExecute')]WorkflowStubInterface $stub): void
     {
         try {
             $stub->getResult();
@@ -41,16 +41,42 @@ public static function check(#[Stub('Harness_ChildWorkflow_ThrowsOnExecute')]Wor
             self::assertSame(['foo' => 'bar'], $failure->getDetails()->getValue(0, 'array'));
         }
     }
+
+    #[Test]
+    public static function throwExceptionAfterInit(
+        #[Stub('Harness_ChildWorkflow_ThrowsOnExecute', args: [true])]
+        WorkflowStubInterface $stub,
+    ): void {
+        try {
+            $stub->getResult();
+            throw new \Exception('Expected exception');
+        } catch (WorkflowFailedException $e) {
+            self::assertSame('Harness_ChildWorkflow_ThrowsOnExecute', $e->getWorkflowType());
+
+            /** @var ChildWorkflowFailure $previous */
+            $previous = $e->getPrevious();
+            self::assertInstanceOf(ChildWorkflowFailure::class, $previous);
+            self::assertSame('Harness_ChildWorkflow_ThrowsOnExecute_ChildThrowOnInit', $previous->getWorkflowType());
+
+            /** @var ApplicationFailure $failure */
+            $failure = $previous->getPrevious();
+            self::assertInstanceOf(ApplicationFailure::class, $failure);
+            self::assertStringContainsString('Test message', $failure->getOriginalMessage());
+            self::assertSame('TestError', $failure->getType());
+            self::assertTrue($failure->isNonRetryable());
+            self::assertSame(['foo' => 'bar'], $failure->getDetails()->getValue(0, 'array'));
+        }
+    }
 }
 
 #[WorkflowInterface]
 class MainWorkflow
 {
     #[WorkflowMethod('Harness_ChildWorkflow_ThrowsOnExecute')]
-    public function run()
+    public function run(bool $onInit = false)
     {
         return yield Workflow::newChildWorkflowStub(
-            ChildWorkflow::class,
+            $onInit ? ChildWorkflowThrowOnInit::class : ChildWorkflow::class,
         )->run();
     }
 }
@@ -65,3 +91,14 @@ public function run()
         throw new ApplicationFailure('Test message', 'TestError', true, EncodedValues::fromValues([['foo' => 'bar']]));
     }
 }
+
+
+#[WorkflowInterface]
+class ChildWorkflowThrowOnInit
+{
+    #[WorkflowMethod('Harness_ChildWorkflow_ThrowsOnExecute_ChildThrowOnInit')]
+    public function run()
+    {
+        throw new ApplicationFailure('Test message', 'TestError', true, EncodedValues::fromValues([['foo' => 'bar']]));
+    }
+}
diff --git a/tests/Unit/Worker/AutowiringTestCase.php b/tests/Unit/Worker/AutowiringTestCase.php
@@ -38,12 +38,6 @@ public static function reflectionDataProvider(): array
         $instance = (new \ReflectionClass(static::class))->newInstanceWithoutConstructor();
 
         return [
-            // Closure
-            'closure' => [new \ReflectionFunction($instance->instanceMethod(...))],
-
-            // Static Closure
-            'static closure' => [new \ReflectionFunction(static fn() => global_function())],
-
             // Instance Method
             static::class . '->instanceMethod' => [new \ReflectionMethod($instance, 'instanceMethod')],
 
diff --git a/tests/Unit/Workflow/DeferredGeneratorTestCase.php b/tests/Unit/Workflow/DeferredGeneratorTestCase.php
@@ -81,7 +81,6 @@ public function testCompareReturn(): void
             [
                 'current', 'key', 'current', 'key', 'valid',
                 'next',
-                'getReturn',
             ],
         );
     }
@@ -137,17 +136,6 @@ public function testCompareEmptyThrowValid(): void
         );
     }
 
-    public function testCompareEmptyThrowGetReturn(): void
-    {
-        $this->compare(
-            fn() => (function () {
-                throw new \Exception('foo');
-                yield;
-            })(),
-            ['getReturn', 'getReturn'],
-        );
-    }
-
     public function testCompareEmptyThrowGetKey(): void
     {
         $this->compare(
@@ -184,7 +172,6 @@ public function testLazyNotGeneratorWithException(): void
         $lazy->current();
     }
 
-
     public function testLazyNotGeneratorWithException2(): void
     {
         $lazy = DeferredGenerator::fromHandler(fn() => throw new \Exception('foo'), EncodedValues::empty());
@@ -198,6 +185,35 @@ public function testLazyNotGeneratorWithException2(): void
         $this->assertNull($lazy->current());
     }
 
+    public function testLazyOnGeneratorHandler(): void
+    {
+        $lazy = DeferredGenerator::fromHandler(static function () {
+            throw new \LogicException('foo');
+            yield;
+        }, EncodedValues::empty());
+
+        try {
+            $lazy->current();
+            $this->fail('Exception was not thrown');
+        } catch (\LogicException) {
+            // ignore
+        }
+
+        $this->assertNull($lazy->current());
+    }
+
+    public function testGetResultFromNotStartedGenerator(): void
+    {
+        $closure = fn() => (function () {
+            yield 1;
+        });
+
+        $handler = DeferredGenerator::fromHandler($closure, EncodedValues::empty());
+
+        $this->expectException(\LogicException::class);
+        $handler->getReturn();
+    }
+
     /**
      * @param callable(): \Generator $generatorFactory
      * @param iterable<Action|int, array{Action, mixed}> $actions

Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@`
`18`	`18`	`>`
`19`	`19`	`<testsuites>`
`20`	`20`	`<testsuite name="Acceptance">`
	`21`	`+ <directory suffix="Test.php">tests/Acceptance/Extra</directory>`
`21`	`22`	`<directory suffix="Test.php">tests/Acceptance/Harness</directory>`
`22`	`23`	`</testsuite>`
`23`	`24`	`<testsuite name="Arch">`
Original file line number	Diff line number	Diff line change
`@@ -89,7 +89,7 @@ public function send(mixed $value): mixed`
`89`	`89`	`*/`
`90`	`90`	`public function getReturn(): mixed`
`91`	`91`	`{`
`92`		`- // $this->start();`
	`92`	`+ $this->finished or throw new \LogicException('Cannot get return value of a generator that was not finished.');`
`93`	`93`	`try {`
`94`	`94`	`return $this->generator->getReturn();`
`95`	`95`	`} catch (\Throwable $e) {`
`@@ -149,7 +149,8 @@ public function valid(): bool`
`149`	`149`	`{`
`150`	`150`	`$this->start();`
`151`	`151`	`try {`
`152`		`- return $this->generator->valid();`
	`152`	`+ $result = $this->generator->valid() or $this->finished = true;`
	`153`	`+ return $result;`
`153`	`154`	`} catch (\Throwable $e) {`
`154`	`155`	`$this->handleException($e);`
`155`	`156`	`}`
`@@ -218,9 +219,9 @@ private function handleException(\Throwable $e): never`
`218`	`219`	`{`
`219`	`220`	`$this->finished and throw $e;`
`220`	`221`	`$this->finished = true;`
`221`		`- foreach ($this->catchers as $catch) {`
	`222`	`+ foreach ($this->catchers as $catcher) {`
`222`	`223`	`try {`
`223`		`- $catch($e);`
	`224`	`+ $catcher($e);`
`224`	`225`	`} catch (\Throwable) {`
`225`	`226`	`// Do nothing.`
`226`	`227`	`}`
Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ public function failWithBadUpdateName(`
`60`	`60`	`try {`
`61`	`61`	`$stub->getResult(timeout: 1);`
`62`	`62`	`$this->fail('Workflow must fail');`
`63`		`- } catch (WorkflowFailedException $e) {`
	`63`	`+ } catch (WorkflowFailedException) {`
`64`	`64`	`$this->assertTrue(true);`
`65`	`65`	`}`
`66`	`66`	`}`