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

Ensure generators aren't reused from failed state #12

Merged
merged 12 commits into from
Oct 23, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 7 additions & 18 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,16 @@ on:
- master
- main

jobs:
env:
COMPOSER_ROOT_VERSION: 0.1.x-dev

jobs:
tests:
runs-on: ubuntu-latest
strategy:
matrix:
php-version: ['7.2', '7.3', '7.4', '8.0', '8.1', '8.2']
php-version: ['7.4', '8.0', '8.1', '8.2', '8.3']
coverage: ['pcov']
include:
- php-version: '7.1'
coverage: xdebug

name: Test with PHP ${{ matrix.php-version }}

Expand Down Expand Up @@ -48,11 +47,6 @@ jobs:
composer-${{ matrix.php-version }}-
composer-

- name: Set package version for Composer
uses: ergebnis/composer-root-version-action@0.2.1
with:
branch: main

- name: Install dependencies
run: |
composer remove --no-update --dev --no-interaction --no-progress \
Expand All @@ -69,7 +63,7 @@ jobs:
runs-on: ubuntu-latest

env:
PHP_VERSION: '8.1'
PHP_VERSION: '8.2'

steps:
- name: Checkout code
Expand All @@ -92,11 +86,6 @@ jobs:
composer-${{ env.PHP_VERSION }}-
composer-

- name: Set package version for Composer
uses: ergebnis/composer-root-version-action@0.2.1
with:
branch: main

- name: Install dependencies
run: |
composer update --prefer-dist --no-interaction --no-progress
Expand All @@ -121,8 +110,8 @@ jobs:
runs-on: ubuntu-latest

env:
PHP_VERSION: '8.1'
PHP_CS_FIXER_VERSION: 'v3.12.0'
PHP_VERSION: '8.2'
PHP_CS_FIXER_VERSION: 'v3.35.1'

steps:
- name: Checkout
Expand Down
8 changes: 4 additions & 4 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@
}
],
"require": {
"php": ">=7.1"
"php": ">=7.4"
},
"require-dev": {
"ergebnis/composer-normalize": "^2.8",
"friendsofphp/php-cs-fixer": "^3.12",
"infection/infection": ">=0.18",
"friendsofphp/php-cs-fixer": "^3.35.1",
"infection/infection": ">=0.27.6",
"phan/phan": ">=2",
"php-coveralls/php-coveralls": "^2.0",
"phpstan/phpstan": ">=1.4.5",
"phpunit/phpunit": ">=7.4",
"phpunit/phpunit": ">=9.5 <10",
"vimeo/psalm": ">=2"
},
"autoload": {
Expand Down
39 changes: 20 additions & 19 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -1,23 +1,24 @@
<phpunit bootstrap="vendor/autoload.php"
<?xml version="1.0"?>
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
bootstrap="vendor/autoload.php"
executionOrder="random"
resolveDependencies="true"
forceCoversAnnotation="false"
colors="true">

<testsuites>
<testsuite name="Main">
<directory>tests/</directory>
</testsuite>
</testsuites>

<filter>
<whitelist>
<directory suffix=".php">src/</directory>
</whitelist>
</filter>

<logging>
<log type="coverage-clover" target="build/logs/clover.xml"/>
<log type="coverage-text" target="php://stdout"/>
</logging>
colors="true"
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
<coverage>
<include>
<directory suffix=".php">src/</directory>
</include>
<report>
<clover outputFile="build/logs/clover.xml"/>
<text outputFile="php://stdout"/>
</report>
</coverage>
<testsuites>
<testsuite name="Main">
<directory>tests/</directory>
</testsuite>
</testsuites>
<logging/>
</phpunit>
27 changes: 22 additions & 5 deletions src/Deferred.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
* Deferred: a wrapper object.
*
* @template T
* @template-implements Interfaces\Deferred<T>
*
* @psalm-suppress PropertyNotSetInConstructor
*
Expand All @@ -40,30 +41,46 @@ final class Deferred implements Interfaces\Deferred
*/
private $output;

/**
* @var ?\Throwable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In docblock tags it is \Throwable|null

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow Psalm understands ?Type in docblocks 🤔

Copy link
Contributor

@szepeviktor szepeviktor Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it does but if you look a docblocks in other projects, question mark is not used for null.
https://docs.phpdoc.org/guide/references/phpdoc/tags/return.html#return

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@szepeviktor szepeviktor Oct 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You haven't written ?int below.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I wasn't going to let it be a docblock forever 🙂

#16

*/
private $error;

/**
* @param iterable<T> $input
*/
public function __construct(iterable $input)
{
$this->input = $input;
$this->error = null;
}

/**
* @return T
*/
public function get()
{
if (null !== $this->error) {
throw $this->error;
}

if (null === $this->input) {
return $this->output;
}

foreach ($this->input as $output) {
$this->output = $output;
try {
foreach ($this->input as $output) {
$this->output = $output;

break;
}
break;
}
} catch (\Throwable $e) {
$this->error = $e;

$this->input = null;
throw $e;
} finally {
$this->input = null;
}

return $this->output;
}
Expand Down
1 change: 1 addition & 0 deletions src/Immediate.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
* Immediate: a wrapper object.
*
* @template T
* @template-implements Interfaces\Deferred<T>
*
* @internal
*/
Expand Down
28 changes: 28 additions & 0 deletions tests/DeferredTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,32 @@ private function yieldsNothing(bool $false = false): iterable
yield 1;
}
}

public function testThrowsSame(): void
{
$later = new Deferred($this->generatorThrows(true));
$e = null;

try {
$this->assertDeferredSame(1, $later);
$this->fail('Should have thrown');
} catch (\InvalidArgumentException $e) {
$this->assertSame(__CLASS__, $e->getMessage());
}

try {
$this->assertDeferredSame(1, $later);
} catch (\InvalidArgumentException $e2) {
$this->assertSame($e, $e2);
}
}

private function generatorThrows(bool $throw = false): iterable
{
if ($throw) {
throw new \InvalidArgumentException(__CLASS__);
}

yield 1;
}
}
2 changes: 1 addition & 1 deletion tests/Examples/Calculator.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ private function makeDependency(int $number)

for ($i = 1; $i <= $number; ++$i) {
\usleep(100);
$factorial = $factorial * $i;
$factorial *= $i;
}

yield $factorial;
Expand Down
2 changes: 1 addition & 1 deletion tests/LaterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public function testGet(): void
{
$later = later(
/** @return iterable<int> */
function (): iterable {
static function (): iterable {
yield 42;
}
);
Expand Down
Loading