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

Tests configuration setup #459

Closed
jrfnl opened this issue Mar 20, 2021 · 4 comments · Fixed by #503
Closed

Tests configuration setup #459

jrfnl opened this issue Mar 20, 2021 · 4 comments · Fixed by #503

Comments

@jrfnl
Copy link
Member

jrfnl commented Mar 20, 2021

PR #457 contains a commit adding a test to the config.
This is the second time I've had to create a separate PR to add tests which were not automatically being run. Also see #366.

I'd like to propose removing the separate test suite configurations in favour of a generic setup which will automatically discover any new tests added to the tests directory.

The PHPUnit --filter CLI argument is powerful enough to handle selective running of the tests for most PHPUnit versions.

If needs be, for older PHPUnit versions, a @group annotation could be added to various tests to still allow them to be run in groups alike to the original testsuites.

@jrfnl
Copy link
Member Author

jrfnl commented Mar 20, 2021

Probably would be good to take this issue into account when setting up the namespaces for the tests for Requests 2.0.0. Also see #460.

@jrfnl jrfnl added this to the 2.0.0 milestone Mar 20, 2021
@jrfnl jrfnl assigned jrfnl and unassigned jrfnl Mar 21, 2021
@jrfnl
Copy link
Member Author

jrfnl commented Jun 12, 2021

To make this change, it would be best if all test classes are named as such.

Currently, the classes follow a pattern like: RequestsTest_[SubDir_]_ClassName.

For PHPUnit to be able to pick up on the tests automatically, the test classes should end on Test and for compatibility with PHPUnit 9.1.0+, the file names should match the class names, so, this ties in with the namespacing proposal in #460 and also PR #446 and should wait until the latter has been merged or a commit actioning this should be added to that PR.

Proposed naming conventions for the tests:

Namespace name ClassName FileName
Requests\Tests[\SubDir] OriginalClassNameTest OriginalClassNameTest.php

This is in line with PSR4 and will allow for adding a autoload-dev section to the composer.json file to set up the autoloading of the tests. The custom autoload function can then be removed from the test bootstrap file.

Note: for dev setups using a PHPUnit phar file, we may need to test if the above is desirable or if this will inhibit testing against multiple PHP and PHPUnit versions via the Phar too much.
If that's the case, I would favour leaving the custom autoloader in the test bootstrap file in place.

Action list:

  • Add namespace statements to all files in the tests directory
  • Rename the test classes and files
  • Adjust the phpunit.xml.dist file to only declare one testsuite looking for all files ending on Test.php
  • MAYBE Add autoload-dev to composer.json
  • MAYBE Remove custom autoloader from bootstrap file in favour of loading the Composer vendor/autoload.php file.
    Alternatively, the custom autoloader should be adjusted to handle the new PSR4 based classname to filename translation.

@schlessera
Copy link
Member

Note: for dev setups using a PHPUnit phar file, we may need to test if the above is desirable or if this will inhibit testing against multiple PHP and PHPUnit versions via the Phar too much.
If that's the case, I would favour leaving the custom autoloader in the test bootstrap file in place.

I'm not entirely sure what issue you expect to arise here, but I've never had any problems as long as I add the Composer autoloader itself to the PHPUnit configuration's bootstrap section.

@jrfnl
Copy link
Member Author

jrfnl commented Jun 18, 2021

@schlessera And that's exactly the "secret" - If the Composer autoload file is the bootstrap file (per the config or CLI args), there is no problem, however as soon as there is a custom bootstrap file which requires the Composer autoload file, you start running into signature mismatch problems if the composer install was run on a high version of PHP and you are running the tests against a PHPUnit phar on a low version of PHP.

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

Successfully merging a pull request may close this issue.

2 participants