Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[PhpUnitBridge] Fix some errors when using serialized deprecations#33820
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
greg0ire commentedOct 14, 2019
I just renamed test methods that were tied to methods that no longer exists ( |
l-vo commentedOct 14, 2019
@greg0ire Excuse me, I'm currently running out of time and I have to get back into the context. But I'm going to look at this as soon as I can 😊 |
l-vo commentedOct 20, 2019
I reviewed and tested the changes, it looks good to me. Although there is a problem, the tests don't pass. The changes on the type "self" determination makes the type "self" difficult to test. |
| publicfunctionproviderGetTypeDetectsSelf():array | ||
| { | ||
| return [ | ||
| 'not_from_vendors_file' => [Deprecation::TYPE_SELF,'','MyClass1',''], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Don't pass anymore. A way to test it:
foreach (get_declared_classes()as$class) {if ('C' ===$class[0] &&0 ===strpos($class,'ComposerAutoloaderInit')) {$r =new \ReflectionClass($class);$v =\dirname(\dirname($r->getFileName()));if (file_exists($v.'/composer/installed.json')) {$loader =require$v.'/autoload.php';$reflection =new \ReflectionClass($loader);$prop =$reflection->getProperty('prefixDirsPsr4');$prop->setAccessible(true);$currentValue =$prop->getValue($loader);$currentValue['Symfony\\Bridge\\PhpUnit\\'] = [realpath(__DIR__.'/../..')];$prop->setValue($loader,$currentValue); } } }return ['not_from_vendors_file' => [Deprecation::TYPE_SELF,'','MyClass1',__FILE__],//...];
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Cannot say I understand but I applied this change 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It's just change composer loader psr4 paths which are used to determine "self" type :)
| '', | ||
| ], | ||
| 'serialized_trace_with_not_from_vendors_triggering_file' => [ | ||
| Deprecation::TYPE_SELF, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Hum it will depend from where tests are launched. I think we can remove this data provided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
removed
| 'not_from_vendors_file' => [Deprecation::TYPE_SELF,'','MyClass1',''], | ||
| 'nonexistent_file' => [Deprecation::TYPE_UNDETERMINED,'','MyClass1','dummy_vendor_path'], | ||
| 'serialized_trace_without_triggering_file' => [ | ||
| Deprecation::TYPE_SELF, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
it will depend from where tests are launched. I think we can remove this data provided.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
removed
greg0ire commentedOct 24, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@l-vo I applied your suggestions, please review again :) |
l-vo left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM :)
nicolas-grekas commentedFeb 4, 2020
Thank you@greg0ire. |
…ecations (l-vo)This PR was submitted for the 4.3 branch but it was squashed and merged into the 4.4 branch instead.Discussion----------[PhpUnitBridge] Fix some errors when using serialized deprecations| Q | A| ------------- | ---| Branch? | 4.3| Bug fix? | yes| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tickets | n/a| License | MIT| Doc PR | n/aThis PR attempts to fix conflicts that arose in#31478Creating as a draft for now as I think having separate test methods no longer make sense (`isSelf()` and `isIndirect()` have been replaced with `getType()`).@l-vo please review and confirm I did not loose anything valuable from your original contribution.Commits-------056d598 [PhpUnitBridge] Fix some errors when using serialized deprecations
I failed to apply perfectly this comment:symfony#33820 (comment)It should fix one failing test in the bridge.
I failed to apply this comment perfectly:symfony#33820 (comment)It should fix one failing test in the bridge.
This PR was merged into the 4.4 branch.Discussion----------[PHPunit bridge] Provide current file as file pathI failed to apply perfectly this comment:#33820 (comment)It should fix one failing test in the bridge.| Q | A| ------------- | ---| Branch? |4.4| Bug fix? | not for the end user| New feature? | no| Deprecations? | no| Tickets | n/a| License | MIT| Doc PR | n/a<!--Replace this notice by a short README for your feature/bugfix. This will help peopleunderstand your PR and can be used as a start for the documentation.Additionally (seehttps://symfony.com/roadmap): - Always add tests and ensure they pass. - Never break backward compatibility (seehttps://symfony.com/bc). - Bug fixes must be submitted against the lowest maintained branch where they apply (lowest branches are regularly merged to upper ones so they get the fixes too.) - Features and deprecations must be submitted against branch master.-->Commits-------d5302cb Provide current file as file path
This PR attempts to fix conflicts that arose in#31478
Creating as a draft for now as I think having separate test methods no longer make sense (
isSelf()andisIndirect()have been replaced withgetType()).@l-vo please review and confirm I did not loose anything valuable from your original contribution.