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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PhpUnitBridge] Url encoded deprecations helper config #29211

Merged

Conversation

Projects
None yet
7 participants
@greg0ire
Copy link
Contributor

commented Nov 13, 2018

Q A
Branch? master
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets #28048
License MIT
Doc PR symfony/symfony-docs#10701

First stab at implementing a new way of configuring the deprecation error handler. Includes a refactoring to keep things manageable.

Rework of #24867, blocked by #29718

TODO:

  • make the code 5.5 compatible 馃槩
  • add more tests
  • deprecate modes (using echo :P)
  • test this on real life projects and add some screenshots
  • docs PR
  • handle strict
  • adapt existing CI config

Quiet configuration

quiet

Default configuration

verbose

@greg0ire greg0ire force-pushed the greg0ire:url_encoded_deprecations_helper_config branch 2 times, most recently from 308d9b0 to 586053a Nov 13, 2018

@nicolas-grekas nicolas-grekas added this to the next milestone Nov 14, 2018

@greg0ire greg0ire force-pushed the greg0ire:url_encoded_deprecations_helper_config branch 6 times, most recently from d010f02 to 4c7adb1 Nov 19, 2018

@ostrolucky

This comment has been minimized.

Copy link
Contributor

commented Nov 24, 2018

make the code 5.5 compatible cry

Why when you are targetting master?

@greg0ire

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2018

IIRC that's because recent versions of the bridge will be used to test old versions of other Symfony components:

"php": ">=5.3.3 EVEN ON LATEST SYMFONY VERSIONS TO ALLOW USING",
"php": "THIS BRIDGE WHEN TESTING LOWEST SYMFONY VERSIONS.",
"php": ">=5.3.3"

greg0ire added a commit to greg0ire/symfony-docs that referenced this pull request Nov 24, 2018

@greg0ire

This comment has been minimized.

Copy link
Contributor Author

commented Nov 24, 2018

Wait @nicolas-grekas , is it really 5.3, not 5.5? Did you maybe tell me we could bump to 5.5?

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Nov 24, 2018

5.5 will be good soon because 2.8 will be out of maintenance in 1 week.
We'll have to bump that 5.3 in Symfony 4.3

@greg0ire greg0ire force-pushed the greg0ire:url_encoded_deprecations_helper_config branch 6 times, most recently from eea83c2 to 9c62a77 Nov 24, 2018

@greg0ire

This comment has been minimized.

Copy link
Contributor Author

commented Nov 25, 2018

Here is how the "Improve stack trace display" commit changes things:

Before

#0 [internal function]: Symfony\Bridge\PhpUnit\DeprecationErrorHandler::Symfony\Bridge\PhpUnit\{closure}(16384, 'The Sonata\\Core...', '/home/greg/dev/...', 17, Array)
#1 /home/greg/dev/SonataAdminBundle/vendor/sonata-project/core-bundle/src/Model/Adapter/AdapterInterface.php(17): trigger_error('The Sonata\\Core...', 16384)
#2 /home/greg/dev/SonataAdminBundle/vendor/composer/ClassLoader.php(444): include('/home/greg/dev/...')
#3 /home/greg/dev/SonataAdminBundle/vendor/composer/ClassLoader.php(322): Composer\Autoload\includeFile('/home/greg/dev/...')
#4 [internal function]: Composer\Autoload\ClassLoader->loadClass('Sonata\\CoreBund...')
#5 /home/greg/dev/SonataAdminBundle/tests/Admin/AdminTest.php(1817): spl_autoload_call('Sonata\\CoreBund...')
#6 [internal function]: Sonata\AdminBundle\Tests\Admin\AdminTest->provideGetSubject()
#7 phar:///home/greg/bin/phpunit/phpunit/Util/Test.php(856): ReflectionMethod->invoke(Object(Sonata\AdminBundle\Tests\Admin\AdminTest))
#8 phar:///home/greg/bin/phpunit/phpunit/Util/Test.php(410): PHPUnit\Util\Test::getDataFromDataProviderAnnotation('/**\n     * @dat...', 'Sonata\\AdminBun...', 'testGetSubjectF...')
#9 phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(157): PHPUnit\Util\Test::getProvidedData('Sonata\\AdminBun...', 'testGetSubjectF...')
#10 phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(906): PHPUnit\Framework\TestSuite::createTest(Object(ReflectionClass), 'testGetSubjectF...')
#11 phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(397): PHPUnit\Framework\TestSuite->addTestMethod(Object(ReflectionClass), Object(ReflectionMethod))
#12 phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(511): PHPUnit\Framework\TestSuite->__construct(Object(ReflectionClass))
#13 phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(592): PHPUnit\Framework\TestSuite->addTestSuite(Object(ReflectionClass))
#14 phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(618): PHPUnit\Framework\TestSuite->addTestFile('/home/greg/dev/...')
#15 phar:///home/greg/bin/phpunit/phpunit/Util/Configuration.php(1139): PHPUnit\Framework\TestSuite->addTestFiles(Array)
#16 phar:///home/greg/bin/phpunit/phpunit/Util/Configuration.php(988): PHPUnit\Util\Configuration->getTestSuite(Object(DOMElement), Array)
#17 phar:///home/greg/bin/phpunit/phpunit/TextUI/Command.php(889): PHPUnit\Util\Configuration->getTestSuiteConfiguration('')
#18 phar:///home/greg/bin/phpunit/phpunit/TextUI/Command.php(170): PHPUnit\TextUI\Command->handleArguments(Array)
#19 phar:///home/greg/bin/phpunit/phpunit/TextUI/Command.php(159): PHPUnit\TextUI\Command->run(Array, true)
#20 /home/greg/bin/phpunit(595): PHPUnit\TextUI\Command::main()
#21 {main}

After:

#0 [internal function] Symfony\Bridge\PhpUnit\DeprecationErrorHandler::Symfony\Bridge\PhpUnit\{closure}(16384, 'The Sonata\Core鈥', '/home/greg/dev/鈥', 17, Array)
#1 [sonata-project/core-bundle] /home/greg/dev/SonataAdminBundle/vendor/sonata-project/core-bundle/src/Model/Adapter/AdapterInterface.php(17): ::trigger_error('The Sonata\Core鈥', 16384)
#2 [composer] /home/greg/dev/SonataAdminBundle/vendor/composer/ClassLoader.php(444): ::include('/home/greg/dev/鈥')
#3 [composer] /home/greg/dev/SonataAdminBundle/vendor/composer/ClassLoader.php(322): ::Composer\Autoload\includeFile('/home/greg/dev/鈥')
#4 [internal function] Composer\Autoload\ClassLoader::loadClass('Sonata\CoreBund鈥')
#5 [source code] /home/greg/dev/SonataAdminBundle/tests/Admin/AdminTest.php(1817): ::spl_autoload_call('Sonata\CoreBund鈥')
#6 [internal function] Sonata\AdminBundle\Tests\Admin\AdminTest::provideGetSubject()
#7 [unknown] phar:///home/greg/bin/phpunit/phpunit/Util/Test.php(856): ReflectionMethod::invoke(Object(Sonata\AdminBundle\Tests\Admin\AdminTest))
#8 [unknown] phar:///home/greg/bin/phpunit/phpunit/Util/Test.php(410): PHPUnit\Util\Test::getDataFromDataProviderAnnotation('/**\n     * @dat鈥', 'Sonata\AdminBun鈥', 'testGetSubjectF鈥')
#9 [unknown] phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(157): PHPUnit\Util\Test::getProvidedData('Sonata\AdminBun鈥', 'testGetSubjectF鈥')
#10 [unknown] phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(906): PHPUnit\Framework\TestSuite::createTest(Object(ReflectionClass), 'testGetSubjectF鈥')
#11 [unknown] phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(397): PHPUnit\Framework\TestSuite::addTestMethod(Object(ReflectionClass), Object(ReflectionMethod))
#12 [unknown] phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(511): PHPUnit\Framework\TestSuite::__construct(Object(ReflectionClass))
#13 [unknown] phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(592): PHPUnit\Framework\TestSuite::addTestSuite(Object(ReflectionClass))
#14 [unknown] phar:///home/greg/bin/phpunit/phpunit/Framework/TestSuite.php(618): PHPUnit\Framework\TestSuite::addTestFile('/home/greg/dev/鈥')
#15 [unknown] phar:///home/greg/bin/phpunit/phpunit/Util/Configuration.php(1139): PHPUnit\Framework\TestSuite::addTestFiles(Array)
#16 [unknown] phar:///home/greg/bin/phpunit/phpunit/Util/Configuration.php(988): PHPUnit\Util\Configuration::getTestSuite(Object(DOMElement), Array)
#17 [unknown] phar:///home/greg/bin/phpunit/phpunit/TextUI/Command.php(889): PHPUnit\Util\Configuration::getTestSuiteConfiguration('鈥')
#18 [unknown] phar:///home/greg/bin/phpunit/phpunit/TextUI/Command.php(170): PHPUnit\TextUI\Command::handleArguments(Array)
#19 [unknown] phar:///home/greg/bin/phpunit/phpunit/TextUI/Command.php(159): PHPUnit\TextUI\Command::run(Array, true)
#20 [source code] /home/greg/bin/phpunit(595): PHPUnit\TextUI\Command::main()

The package name is now included, which helps me. Tell me if you do not like it, and I will remove it just before this is merged.

@greg0ire greg0ire force-pushed the greg0ire:url_encoded_deprecations_helper_config branch from 9c62a77 to 14fd0e1 Nov 25, 2018

llaakkkk added a commit to llaakkkk/symfony-docs that referenced this pull request Apr 6, 2019

llaakkkk added a commit to llaakkkk/symfony-docs that referenced this pull request Apr 6, 2019

llaakkkk added a commit to llaakkkk/symfony-docs that referenced this pull request Apr 7, 2019

@nicolas-grekas nicolas-grekas changed the title [PHPUnitBridge]Url encoded deprecations helper config [PhpUnitBridge] Url encoded deprecations helper config Apr 10, 2019

@nicolas-grekas nicolas-grekas force-pushed the greg0ire:url_encoded_deprecations_helper_config branch from bfa5f79 to 545b86f Apr 10, 2019

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

I'm finally reviewing this PR \o/
I rebase+squashed it, then added a commit. I think there is no need for deprecating any mode except "weak_vendor". This means the "regexp" key in the query string should be removed. Then I'll really insist to never use the term "vendor" anywhere except in the doc. Fingerpointing at vendors is wrong and doesn't encourage the correct attitude. There are only "self" (replaces "internal"), "direct" and "indirect" deprecations.

@greg0ire greg0ire force-pushed the greg0ire:url_encoded_deprecations_helper_config branch from ce021f8 to 181bb37 Apr 10, 2019

@greg0ire greg0ire force-pushed the greg0ire:url_encoded_deprecations_helper_config branch 8 times, most recently from 9251b1a to 1006b12 Apr 10, 2019

@greg0ire greg0ire force-pushed the greg0ire:url_encoded_deprecations_helper_config branch from 1006b12 to 1c73f9c Apr 11, 2019

@nicolas-grekas
Copy link
Member

left a comment

Nice, thanks! This will improve a lot the semantics and the flexibility!

@fabpot

This comment has been minimized.

Copy link
Member

commented Apr 12, 2019

Thank you @greg0ire.

@fabpot fabpot merged commit 1c73f9c into symfony:master Apr 12, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Apr 12, 2019

feature #29211 [PhpUnitBridge] Url encoded deprecations helper config鈥
鈥 (greg0ire)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[PhpUnitBridge] Url encoded deprecations helper config

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #28048
| License       | MIT
| Doc PR        | symfony/symfony-docs#10701

First stab at implementing a new way of configuring the deprecation error handler. Includes a refactoring to keep things manageable.

Rework of #24867, blocked by #29718

TODO:

- [x] make the code 5.5 compatible 馃槩
- [x] add more tests
- [x] deprecate modes (using echo :P)
- [x] test this on real life projects and add some screenshots
- [x] docs PR
- [x] handle `strict`
- [x] adapt existing CI config

# Quiet configuration

![quiet](https://user-images.githubusercontent.com/657779/49341318-fa78c900-f64b-11e8-9504-a8a9eac4baf8.png)

# Default configuration

![verbose](https://user-images.githubusercontent.com/657779/49341322-10868980-f64c-11e8-9d90-dc3f6a18c335.png)

Commits
-------

1c73f9c [PhpUnitBridge] Url encoded deprecations helper config
return false !== strpos($key, 'Count') && false === strpos($key, 'legacy');
}, ARRAY_FILTER_USE_KEY);
if (array_sum($deprecationCounts) > $this->thresholds['total']) {

This comment has been minimized.

Copy link
@stof

stof Apr 12, 2019

Member

this is broken if there is no such threshold

This comment has been minimized.

Copy link
@greg0ire

greg0ire Apr 12, 2019

Author Contributor

It is always initialized to 999999, see the ctor

return false;
}
foreach (['self', 'direct', 'indirect'] as $deprecationType) {
if ($deprecationCounts['remaining '.$deprecationType.'Count'] > $this->thresholds[$deprecationType]) {

This comment has been minimized.

Copy link
@stof

stof Apr 12, 2019

Member

this is broken if there is no such threshold

This comment has been minimized.

Copy link
@greg0ire

greg0ire Apr 12, 2019

Author Contributor

same as above

}
if (self::MODE_WEAK_VENDORS === $mode) {
echo sprintf('Setting SYMFONY_DEPRECATIONS_HELPER to "%s" is deprecated in favor of "max[self]=0"', $mode).PHP_EOL;
exit(1);

This comment has been minimized.

Copy link
@stof

stof Apr 12, 2019

Member

this is a BC break, as it will exit without running the tests. This should report a self deprecation, and return a configuration based on max[self]=0

This comment has been minimized.

Copy link
@greg0ire

greg0ire Apr 12, 2019

Author Contributor

This is master, and @nicolas-grekas asked me to change my previous code, that was as you described, for this.

@greg0ire greg0ire deleted the greg0ire:url_encoded_deprecations_helper_config branch Apr 12, 2019

nicolas-grekas added a commit that referenced this pull request Apr 12, 2019

bug #31094 [PhpUnitBridge] fixes (nicolas-grekas)
This PR was merged into the 4.3-dev branch.

Discussion
----------

[PhpUnitBridge] fixes

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Fixes @stof's review in #29211 + fixes PHP5.5 support (array consts are PHP5.6+)
/cc @greg0ire

Commits
-------

b11585e [PhpUnitBridge] fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.