Skip to content

Conversation

@drbyte
Copy link
Contributor

@drbyte drbyte commented Nov 28, 2020

Update test suite to phpunit 9.5 syntax
Refactored to use polyfill for older PHP versions via yoast/phpunit-polyfills

Note: this includes 2 important differences from usual phpunit test suites:

  • instead of extending PHPUnit\Framework\TestCase we extend Yoast\PHPUnitPolyfills\TestCases\TestCase
  • instead of handling fixtures via setUp() and tearDown() we use set_up() and tear_down() respectively

Comment regarding formatting: I chose to use the FQDN in the extends syntax of the class declaration instead of using use so that it is more quickly apparent that we're doing something slightly different than usual phpunit syntax, particularly in regards to the set_up() / tear_down() methods that appear immediately following the extends line.


IMO this closes #1002

Update test suite to phpunit 9.5 syntax
Refactored to use polyfill for older PHP versions via `yoast/phpunit-polyfills`

Note: this includes 2 important differences from usual phpunit test suites:
- instead of extending `PHPUnit\Framework\TestCase` we extend `Yoast\PHPUnitPolyfills\TestCases\TestCase`
- instead of handling fixtures via `setUp()` and `tearDown()` we use `set_up()` and `tear_down()` respectively

Comment regarding formatting: I chose to use the FQDN in the `extends` syntax of the class declaration instead of using `use` so that it is more quickly apparent that we're doing something slightly different than usual phpunit syntax, particularly in regards to the set_up() / tear_down() methods that appear immediately following the `extends` line.
@drbyte
Copy link
Contributor Author

drbyte commented Nov 29, 2020

@mattstauffer I have no strong opinions whether to use this yoast package vs the symfony package in # 1009.

I'll note two factors I discovered while exploring both options:

  1. The caveat with the symfony package is you can NOT run phpunit. You must run simple-phpunit instead. That tripped me up a number of times when working locally. This is my primary frustration with it and I wish it could be avoided.

  2. The symfony package uses a tiny fraction more RAM to do its tests, and in the background it re-runs composer to clone the phpunit version it is trying to support for a given PHP version. This can make its setup take a tiny bit longer.
    Both are negligible.

@soilSpoon
Copy link
Contributor

@drbyte @mattstauffer It has the same drawbacks as above, but symfony project is more mature than yoast.

I'm not sure if maturity or good performance suits the valet project better.

Please good choice

@mattstauffer
Copy link
Collaborator

Hm.. I very much like what @drbyte wrote about the values of the Yoast package. However, 42 million installs vs 4000 installs is a big difference.

That said, I think the PHPUnit Bridge package does more than just the polyfills, and I'm inclined to pull in the smallest surface area we can for this.

I'm leaning toward pulling in Yoast and taking the risk on it, knowing we have Symfony (and #1009) as a backup.

@mattstauffer mattstauffer merged commit acb55d3 into laravel:master Nov 30, 2020
@drbyte drbyte deleted the phpunit-polyfill branch November 30, 2020 04:04
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.

Update supported php versions

3 participants