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

Release version 1.0.0 #42

Merged
merged 68 commits into from
Jun 21, 2021
Merged

Release version 1.0.0 #42

merged 68 commits into from
Jun 21, 2021

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Jun 21, 2021

No description provided.

jrfnl and others added 30 commits December 4, 2020 09:40
PHP 8.0 has been branched off two months ago, so `nightly` is now PHP 8.1 and in the mean time PHP 8.0 was released last week.

As of today, there is a PHP 8.0 image available on Travis.

This PR adds a new build against PHP 8.0 to the matrix and, as PHP 8.0 has been released, that build is not allowed to fail.
…failure

Travis: add build against PHP 8.0
... to prevent using a reserved keyword as a parameter name.

While this isn't forbidden, in PHP 8.0+ with named parameters this can lead to very confusing code, so better to use another name.
This library supports PHP >= 5.5, but the default `testVersion` as set in YoastCS is `5.6-`.
Overruling the `testVersion` from within a custom ruleset is currently not possible. This is a known issue in PHPCS itself and a fix is expected to be included with PHPCS 4.x.

In the mean time, as a work-around, the `testVersion` can be overruled from the command-line.
CI/QA: fix testVersion for PHPCompatibility
CS/QA: rename a function parameter
This commit:
* Adds a GH Actions workflow for the CI checks which were previously run on Travis in the `sniff` stage.
    While these aren't 100% CS (= code style) checks, for the badge and workflow display, `CS` still seemed the most descriptive name.
* Removes that part of the `.travis.yml` configuration.
* Adds a "Build Status" badge in the Readme to use the results from this particular GH Actions runs.

Notes:
Builds will run on all pushes and on pull requests and can be manually triggered.
This commit:
* Adds a GH Actions workflow for the CI checks which were previously run on Travis in the `test` stage.
* Removes the, now redundant, `.travis.yml` configuration.
* Updates the `.gitattributes` file.
* Updates the "Build Status" badge in the Readme to use the results from the GH `Test` Actions runs.

Notes:
    As there is one job which are "allowed to fail" (`experimental` = true), the build status _may_ unfortunately show as "failed" when that job would fail, even though all non-experimental jobs have succeeded.
         This is a known issue in GHA: https://github.com/actions/toolkit/issues/399
... with alternatives which can be used to replace that functionality.
…t-polyfill

README: add FAQ section covering functionality removed from PHPUnit
PHPUnit 6.4.2 added a fix to gracefully handle the case when an empty string was passed to `assertStringContainsString()` and variant assertions. Versions earlier than that gave a "mb_strpos(): Empty delimiter" error instead.

This change adds a fix to also handle that case, when using PHPUnit-Polyfills with versions of PHPUnit earlier than 6.4.2, and passing an empty needle string.

The use of asserting true is true stops the test from being marked as risky, which is what would happen if we just `return true;`.
There was only one PHP 5.5 specific syntax being used in this library - `::class` -.
This was the only blocker for allowing tests using the polyfills to run on PHP 5.4.

This blocker has now been removed, making the polyfills available on PHP 5.4 in combination with PHPUnit 4.8.x as well.
Allow for using the polyfills on PHP 5.4
- The backfill trait only applies for PHPUnit < 7.5.0
- The empty string handling was added in PHPUnit 6.4.2.
- Assertions with empty strings run with PHPUnit below 6.4.2 always need the fix, giving a skipped test.
- Assertions with empty strings run with PHPUnit 6.4.2 or above, don't need the fix, unless using a negative assertion, since an empty string will always exist in another string, hence the NotContains will always fail.
This aligns the visibility of the methods in the `XTestCase` class with the minimum acceptable visibility to allow the methods to work.
This also ensures that the visibility used in the class now matches that of the code sample in the README.

| Native method name     | Annotation     | Minimum visibility |
|------------------------|----------------|--------------------|
| `setUpBeforeClass()`   | `@beforeClass` | `public`           |
| `setUp()`              | `@before`      | `protected`        |
| `tearDown()`           | `@after`       | `protected`        |
| `tearDownAfterClass()` | `@afterClass`  | `public`           |

Fixes #10
XTestCase: set to lowest possible visibility
* Commit 057a83d added the `AssertNumericType` trait and a corresponding availability test to the `TestCaseTestTrait`, but the test name did not follow the naming convention used in the file.
* Commit b537298 added the `ExpectException` trait, but did not add a corresponding availability test to the `TestCaseTestTrait`.
* Commit 347c768 added the `AssertFileDirectory` trait, but did not add a corresponding availability test to the `TestCaseTestTrait`.

Fixed now.
…vailability-tests

TestCaseTestTrait: add missing tests
## Context

The main reason to add a test bootstrap file is to allow for defining some `class_alias`-es for PHPUnit native classes which are only used in the test suite.
This allows for some minor simplifications to the existing test code and for these same simplifications to be available for future test adjustments (which are needed for PHPUnit 10.0 support).

## Autoloading of the necessary files / Phar compatibility

The XML configuration allows for only one bootstrap file, so adding a `bootstrap` file now means that the Composer `vendor/autoload.php` file has to be loaded from within the `tests/bootstrap.php` file.

This presents a problem when running the tests via a PHPUnit Phar file as the loading of the Composer autoload will block the tests from running if the PHP version used to run the Phar file is not the same PHP version as the one used to run `composer install`.
In that case, the test run will exit before it starts with an error along the lines of:
```
Fatal error: Composer detected issues in your platform: Your Composer dependencies require a PHP version ">= 7.3.0". You are running 7.0.33. in ./vendor/composer/platform_check.php on line 24
```

To get round this conundrum, the bootstrap file will load the Composer autoload file conditionally: only when the tests are not run via a Phar.

When the tests are run via a Phar, the Polyfill autoload file will be hard-required and a custom autoloader function will take care of loading test helper files which are not automatically loaded by PHPUnit, like the Fixture classes and the `Yoast\PHPUnitPolyfills\Tests\TestCases\TestCaseTestTrait`.

## Version checks in the `phpunitpolyfills-autoload.php` file

As the `PHPUnit_Runner_Version` will be aliased to `PHPUnit\Runner\Version` in the test bootstrap file, it will - for the purposes of our tests - always exist.

In PHPUnit itself, the `PHPUnit_Runner_Version` class existed prior to PHPUnit 6.0.0, the `PHPUnit\Runner\Version` class as of PHPUnit 6.0.0. They have no overlap in tagged releases.

The Polyfill autoload file relies (relied) on only one of the class names existing, while with the change to the bootstrap file, this is no longer correct as in the context of PHPUnit 4.x and 5.x both class names will exist, while in the context of PHPUnit >= 6.0, only the namespaced name will exist.

Either way, this means that we have to (and safely can) reverse the condition used in the `phpunitpolyfills-autoload.php` file from checking whether the PHPUnit 6.0+ class name does not exists to checking whether the PHPUnit < 6.0 class name _does_ exist.

## Other

Includes removing work-arounds for the different class names in individual test classes.,
This build is - for now - allowed to fail while support for PHPUnit 10.0 is being added to the repo.
…nit-10

CI/QA: add experimental build against PHPUnit 10.0 (unreleased)
Selectively skip some tests from the `AssertFileDirectoryTest` class.

These tests test that a method which was available between PHPUnit 5.6.0 - 9.x is polyfilled correctly for PHPUnit < 5.6.0.

As these methods have been removed in PHPUnit 10.0.0 in favour of assertions with better names, these tests will fail on PHPUnit 10.0.0.

This is not an issue, as the methods with the new names have also been polyfilled and those tests _do_ pass on PHPUnit 10.0.0.

So if people want their tests to be able to run on PHPUnit 10.0.0, they should use the new names of the methods and all will be good.

This is already [documented as such in the README](https://github.com/Yoast/PHPUnit-Polyfills#use-with-phpunit--570).

This affects the following methods:

| Old name                     | New name                        |
|------------------------------|---------------------------------|
| `assertNotIsReadable()`      | `assertIsNotReadable()`         |
| `assertNotIsWritable()`      | `assertIsNotWritable()`         |
| `assertDirectoryNotExists()` | `assertDirectoryDoesNotExist()` |
…me-tests

AssertFileDirectoryTest: allow for running the tests on PHPUnit 10.0.0
The `$name` parameter for the `TestCase::__construct()` method has become mandatory and should contain the name of the test method to run.

Ref: sebastianbergmann/phpunit@705874f

---

This PHPUnit 10.0.0 change will generally speaking not affect "normal" users as it is rare for them to declare `__construct()` methods in test classes or to instantiate test classes directly, as this is the responsibility of PHPUnit.

In most cases, only test code testing for cross-version PHPUnit compatibility while using PHPUnit to run the tests will be affected.

Even so, fixing this is straight-forward, as the `$name` parameter was already an optional parameter since way back when, so passing the parameter in allows the tests to run (and pass) cross-version.
…tests

TestListenerTest: fix PHPUnit 10.0.0 compatibility
... by always passing a TestResult object to the `run()` method.

The `$testResult` parameter was previously optional, but will become required as of PHPUnit 10.0.0.
Also, the `run()` method used to return an instance of `TestResult`, but will now return `void`.

Ref: sebastianbergmann/phpunit@77e6015

---

Along the same lines as 25, this PHPUnit 10.0.0 change will generally speaking not affect "normal" users.

In most cases, only test code testing for cross-version PHPUnit compatibility while using PHPUnit to run the tests will be affected.

Even so, fixing this is straight-forward, as the `$result` parameter was already an optional parameter since way back when, so instantiating the `TestResult` and passing the parameter in allows the tests to run cross-version.
jrfnl and others added 26 commits June 11, 2021 04:09
Add version agnostic links to the PHPUnit native documentation for the assertions and exception methods polyfilled as well as miscellanous other links to the documentation.
README: add links to the PHPUnit documentation
PHPUnit 9.0.0 introduced the new `Assert::equalToCanonicalizing()`, `Assert::equalToIgnoringCase()` and `Assert::equalToWithDelta()` methods.

These methods are typically used to verify parameters passed to mocked objects.

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.
* An availability tests for the functionality polyfilled.

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.
... to test against the highest and a "low" PHPUnit version for each supported PHP version.
For optimal compatibility with cross-version PHP, `TypeError`s should be caught before `Exception`s.
…y-improvement

ResourceHelper: minor PHP cross-version compatibility fix
Add an extra test to document the behaviour of the `shouldClosedResourceAssertionBeSkipped()` method when a non-resource is passed.
AssertClosedResource/ResourceHelper: add extra test
With the matrix now expanded, linting would now be done multiple times per PHP version, which isn't really necessary.
Documentation: minor improvement to inline docs
Some of the existing code already references these classes and while not (currently) done in a PHP cross-version problematic way, it is still better to make sure these classes exist for optimal PHP cross-version compatibility.

While not strictly correct/PSR4-compliant, I've elected to put these classes in the `Exceptions` subfolder, even though these are non-namespaced classes and not strictly part of the _PHPUnit_ polyfills.

Note: the autoloader will only be called if these classes do not exist in PHP itself (PHP < 7.0), so the way it is set up now, these files will never get loaded on PHP >= 7.0 (which would otherwise cause a "Class already declared" error).

Includes:
* Explicitly excluding the PHPCompatibility notices about these classes in the PHPCS ruleset.
    _Note: due to the use of the PHPCompatibilityWP ruleset in YoastCS, these notices weren't showing anyway as WP backfills these classes. All the same, making it explicit in the ruleset documents the polyfills as now included in this repo._
* Adjustments to the Composer scripts to run the PHP linting and the GH Actions workflow which runs it.
    On PHP 7.x, these polyfills would cause a `Cannot declare class Error, because the name is already in use in ...` error otherwise.
Add polyfills for the PHP 7.0 `TypeError` and `Error` classes
When adding the `TypeError`/`Error` polyfill (#36), I suddenly realized that the use of the `PHPCompatibilityWP` ruleset by the `Yoast` standard was causing certain issues not to show up, while - for this repo - they **_should_** be shown.

By setting the `severity` of all the sniffs in the `PHPCompatibilility` ruleset back to `5`, the `exclude`s from the `PHPCompatibilityWP` ruleset are effectively "undone" and we are back to using `PHPCompatibility` proper.
PHPCS: use PHPCompatibility proper, not PHPCompatibilityWP
…ethod

PHPUnit 9.4.0 introduced the new `Assert::assertObjectEquals()` method.

This commit:
* Adds two traits with the same name.
    One to polyfill the method when not available in PHPUnit.
    The other to allow for `use`-ing the trait in PHPUnit versions in which the method is already natively available.
* Adds an `InvalidComparisonMethodException` exception class.
    _PHPUnit natively throws a range of different exceptions._
    _The polyfill included in this library throws one exception type - the `InvalidComparisonMethodException` - with a range of different messages._
* Logic to the custom autoloader which will load the correct trait depending on the PHPUnit version used.
* Adds tests.

As the polyfill contains logic to match the PHPUnit native implementation as closely as possible, while still being PHP and PHPUnit cross-version compatible, extensive unit tests have been added to ensure the behaviour of the polyfill matches that of the original function, with the exception of the _return type verification_.

As return types were not available in PHP prior to PHP 7.0, the return type verification as done in the PHPUnit native implementation, has been replaced by a verification that the _returned value_ is of the required type.
This provides the same safeguard as the PHPUnit native implementation, but in a PHP cross-version compatible manner.

Note: the method uses `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 for the fixtures used in the tests.

Refs:
 * sebastianbergmann/phpunit#4467
 * sebastianbergmann/phpunit#4707
 * sebastianbergmann/phpunit@1dba8c3
 * sebastianbergmann/phpunit@6099c5e
…equals

AssertObjectEquals trait: polyfill the Assert::assertObjectEquals() method
…provement

AssertObjectEquals: improve "not boolean return value" error message
The path building doesn't really need to use `DIRECTORY_SEPARATOR` as Windows will handle paths with forward slashes correctly anyhow.
…r-sep-constant

Autoloader: minor simplification
Changelog for the release of version 1.0.0
@jrfnl jrfnl added this to the 1.0.0 milestone Jun 21, 2021
@jrfnl jrfnl merged commit 5d257d5 into main Jun 21, 2021
@jrfnl jrfnl removed the yoastcs/qa label Oct 3, 2021
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.

3 participants