Skip to content

New AssertClosedResource trait: polyfill the Assert::assertIs[Not]ClosedResource() methods#27

Merged
jrfnl merged 2 commits into
developfrom
feature/4-add-polyfill-assertisclosedresource
Jun 9, 2021
Merged

New AssertClosedResource trait: polyfill the Assert::assertIs[Not]ClosedResource() methods#27
jrfnl merged 2 commits into
developfrom
feature/4-add-polyfill-assertisclosedresource

Conversation

@jrfnl

@jrfnl jrfnl commented Jun 9, 2021

Copy link
Copy Markdown
Collaborator

AssertClosedResource trait: polyfill the Assert::assertIs[Not]ClosedResource() methods

PHPUnit 9.3.0 introduced the new Assert::assertIsClosedResource() and Assert::assertIsNotClosedResource() methods.

This commit:

  • Adds two traits with the same name.
    One to polyfill the methods when not available in PHPUnit.
    The other to allow for use-ing the trait in PHPUnit versions in which the methods are already natively available.
  • Logic to the custom autoloader which will load the correct trait depending on the PHPUnit version used.
  • A ResourceHelper helper class containing the actual logic to determine whether something is a resource and whether that resource is open or closed.

As the PHP native functionality used in PHPUnit itself was only added to PHP in PHP 7.2.0 and the polyfills are available to older PHP versions, this polyfill needs actual logic and doesn't just fall through to an underlying PHPUnit native method.

With that in mind, an availability test is not sufficient to test this.
Additionally, resources in various extensions may behave and identify differently, so this commit also adds extensive tests with a variety of functionality using resources in PHP.

Note: the methods use static:: to call the PHPUnit native functionality. This allows for existing method overloads in a child class of the PHPUnit native TestCase to be respected.

Includes:

  • Adding information on the new polyfill to the README.
  • Adding the new polyfill to the existing TestCases classes.
  • Adding a few select exceptions to the PHPCS ruleset.

AssertClosedResource: add shouldClosedResourceAssertionBeSkipped() helper function

While creating tests for this polyfill, I discovered that for resources created via the libxml extension, the PHP native functions showed some curious behaviour.

This behaviour basically makes the "is closed resource" determination unreliable for a select group of PHP versions in combination with resources created via the libxml extension.

To prevent tests failing on this, calls to the assertIs[Not]ClosedResource() method should - selectively - be wrapped in a condition checking the output of the AssertClosedResource::shouldClosedResourceAssertionBeSkipped() method, like so:

if ( $this->shouldClosedResourceAssertionBeSkipped( $actual ) === false ) {
    $this->assertIsClosedResource( $actual );
}

As the PHPUnit native implementation of the assertions is also afflicted by the underlying bugs in PHP itself, this helper method is also available when the "empty" version of the AssertClosedResource trait is loaded and can therefore safely be used PHPUnit cross-version.

As things stand, the libxml inconsistency is the only one I have discovered so far.
However, there are quite a few extensions producing resources which are currently not covered in the tests as that would require databases or ssh/ftp/mail credentials to be added to the tests.

Users are encouraged to report other inconsistencies found and the ResourceHelper::isResourceStateReliable() method can be expanded with additional inconsistencies when found in the future.

Includes tests for this trait in combination with the libxml extension.
Also adds calls to a few of the other tests to ensure that the shouldClosedResourceAssertionBeSkipped() correctly returns false in all non-affected situations.

Fixes #4

jrfnl added 2 commits June 9, 2021 01:43
…esource() methods

PHPUnit 9.3.0 introduced the new `Assert::assertIsClosedResource()` and `Assert::assertIsNotClosedResource()` methods.

This commit:
* Adds two traits with the same name.
    One to polyfill the methods when not available in PHPUnit.
    The other to allow for `use`-ing the trait in PHPUnit versions in which the methods are already natively available.
* Logic to the custom autoloader which will load the correct trait depending on the PHPUnit version used.
* A `ResourceHelper` helper class containing the actual logic to determine whether something is a resource and whether that resource is open or closed.

As the PHP native functionality used in PHPUnit itself was only added to PHP in PHP 7.2.0 and the polyfills are available to older PHP versions, this polyfill needs actual logic and doesn't just fall through to an underlying PHPUnit native method.

With that in mind, an availability test is not sufficient to test this.
Additionally, resources in various extensions may behave and identify differently, so this commit also adds extensive tests with a variety of functionality using resources in PHP.

Note: the methods use `static::` to call the PHPUnit native functionality. This allows for existing method overloads in a child class of the PHPUnit native `TestCase` to be respected.

Includes:
* Adding information on the new polyfill to the README.
* Adding the new polyfill to the existing `TestCases` classes.
* Adding a few select exceptions  to the PHPCS ruleset.
…helper function

While creating tests for this polyfill, I discovered that for resources created via the `libxml` extension, the PHP native functions showed some curious behaviour.

This behaviour basically makes the "is closed resource" determination unreliable for a select group of PHP versions in combination with resources created via the `libxml` extension.

To prevent tests failing on this, calls to the `assertIs[Not]ClosedResource()` method should - selectively - be wrapped in a condition checking the output of the `AssertClosedResource::shouldClosedResourceAssertionBeSkipped()` method, like so:
```php
if ( $this->shouldClosedResourceAssertionBeSkipped( $actual ) === false ) {
    $this->assertIsClosedResource( $actual );
}
```

As the PHPUnit native implementation of the assertions is also afflicted by the underlying bugs in PHP itself, this helper method is also available when the "empty" version of the `AssertClosedResource` trait is loaded and can therefore safely be used PHPUnit cross-version.

As things stand, the `libxml` inconsistency is the only one I have discovered so far.
However, there are quite a few extensions producing resources which are currently not covered in the tests as that would require databases or ssh/ftp/mail credentials to be added to the tests.

Users are encouraged to report other inconsistencies found and the `ResourceHelper::isResourceStateReliable()` method can be expanded with additional inconsistencies when found in the future.

Includes tests for this trait in combination with the `libxml` extension.
Also adds calls to a few of the other tests to ensure that the `shouldClosedResourceAssertionBeSkipped()` correctly returns `false` in all non-affected situations.
@jrfnl jrfnl added this to the 0.x Next Release milestone Jun 9, 2021
@jrfnl jrfnl merged commit 5d92ed7 into develop Jun 9, 2021
@jrfnl jrfnl deleted the feature/4-add-polyfill-assertisclosedresource branch June 9, 2021 00:17
@jrfnl

jrfnl commented Jun 15, 2021

Copy link
Copy Markdown
Collaborator Author

For the record - comparison of resource determination functions as used in the ResourceHelper class and source of the libxml incompatible PHP versions list: https://3v4l.org/kBRrE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FR] Add polyfill for new assertIs[Not]ClosedResource()

1 participant