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

Made tests be able to run on all supported PHP versions, and run successfully #353

Merged
merged 1 commit into from
Aug 9, 2023

Conversation

boenrobot
Copy link

@boenrobot boenrobot commented Jun 12, 2023

Made tests be able to run on all supported PHP versions, and run successfully.

As part of accomplishing this, the following was done:

  • Added "yoast/phpunit-polyfills" as a dev dependency.
  • Modified fixtures to use the polyfilled methods (as they are cross version compatible).
  • Added a custom test runner (that is compatible across PHPUnit versions).
  • Modified PHPUnit version to be "^7|^8|^9", since each is supported by different PHP versions.
  • Modified the rector version to "^0.12.19", as that is the last one still supporting PHP 7.1.
  • Changed expectNotice() and expectError() to expecting the equivalent PHPUnit exception, to avoid deprecation notice and support PHPUnit 7, where this feature is absent.
  • Removed the requires annotation in Zend_Filter_Encrypt_OpensslTest
  • Explicitly added "RC4" as the value for the algo to appease supported PHP versions. Why RC4? Because that is the default, and has been the default, and ZF has had no option to override it. Such option could be introduced in a separate PR maybe.
  • Fixed a type related warning in OpenSSL related tests, by casting array keys to strings before the remaining checks that assume a string.
  • Added an openssl.conf file that enables the legacy provider, in addition to the default, in order to enable the tests to pass in newer OpenSSL versions that even older PHP versions are now being compiled against.

Also provided higher test coverage overall. To accomplish that part:

  • Added "symfony/polyfill-ctype" as a runtime dependency, as it can be disabled (while the test tooling does not require it), but is required by many components.
  • Made PHPUnit run twice - once with the bare minimum of extensions needed to get it going, and a second time with all extensions that can be enabled for the given PHP version. Zend_Barcode_AllTests in particular benefits of this, as it has both tests that work only if GD is disabled, as well as tests that run only if GD is enabled.
  • Moved the PHP extensions from the env into a matrix, and adjusted the set of extensions to be one that the PHP version would support.
  • Adjusted the JSON encoder and decoder to explicitly use "UTF-16BE" instead of just "UTF-16", as the mbstring polyfill uses iconv, which in turn aliases "UTF-16" to "UTF-16LE", while the true mbstring extension aliases "UTF-16" to "UTF-16BE". The explicit notation works with both.
  • Adjusted the testDisablingTryCommonMagicFilesIgnoresCommonLocations() test to call setMagicFile(false) when the environment is not good, i.e. when fileinfo is not present.
  • Added some requires annotations where PDO was assumed to be present.

@boenrobot boenrobot mentioned this pull request Jun 13, 2023
…essfully.

As part of accomplishing this, the following was done:

- Added "yoast/phpunit-polyfills" as a dev dependency.
- Modified fixtures to use the polyfilled methods (as they are cross version compatible).
- Added a custom test runner (that is compatible across PHPUnit versions).
- Modified PHPUnit version to be "^7|^8|^9", since each is supported by different PHP versions.
- Modified the rector version to "^0.12.19", as that is the last one still supporting PHP 7.1.
- Changed expectNotice() and expectError() to expecting the equivalent PHPUnit exception, to avoid deprecation notice and support PHPUnit 7, where this feature is absent.
- Removed the requires annotation in Zend_Filter_Encrypt_OpensslTest
- Explicitly added "RC4" as the value for the algo to appease supported PHP versions. Why RC4? Because that is the default, and has been the default, and ZF has had no option to override it. Such option could be introduced in a separate PR maybe.
- Fixed a type related warning in OpenSSL related tests, by casting array keys to strings before the remaining checks that assume a string.
- Added an openssl.conf file that enables the legacy provider, in addition to the default, in order to enable the tests to pass in newer OpenSSL versions that even older PHP versions are now being compiled against.

Also provided higher test coverage overall. To accomplish that part:

- Added "symfony/polyfill-ctype" as a runtime dependency, as it can be disabled (while the test tooling does not require it), but is required by many components.
- Made PHPUnit run twice - once with the bare minimum of extensions needed to get it going, and a second time with all extensions that can be enabled for the given PHP version. Zend_Barcode_AllTests in particular benefits of this, as it has both tests that work only if GD is disabled, as well as tests that run only if GD is enabled.
- Moved the PHP extensions from the env into a matrix, and adjusted the set of extensions to be one that the PHP version would support.
- Adjusted the JSON encoder and decoder to explicitly use "UTF-16BE" instead of just "UTF-16", as the mbstring polyfill uses iconv, which in turn aliases "UTF-16" to "UTF-16LE", while the true mbstring extension aliases "UTF-16" to "UTF-16BE". The explicit notation works with both.
- Adjusted the testDisablingTryCommonMagicFilesIgnoresCommonLocations() test to call setMagicFile(false) when the environment is not good, i.e. when fileinfo is not present.
- Added some requires annotations where PDO was assumed to be present.
@boenrobot
Copy link
Author

Side note: If #342 lands, it might also be possible to add "apcu" to the list of extensions, and TESTS_ZEND_CACHE_APC_ENABLED: true env, and have the APCu cache adapter tests pass.

@develart-projects develart-projects added the enhancement New feature or request label Aug 9, 2023
@develart-projects develart-projects merged commit 41a26bf into Shardj:master Aug 9, 2023
@develart-projects
Copy link
Collaborator

@boenrobot now it's merged to master. Please create PR for the apcu, if you like to have that covered as well, thanks

@develart-projects develart-projects modified the milestones: 1.3.0, 1.23.0 Aug 9, 2023
@boenrobot boenrobot mentioned this pull request Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants