8000 weak_vendors mode and isolated test · Issue #25684 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

weak_vendors mode and isolated test #25684

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.

Sign up for GitHub

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

Jump to bottom
Closed
alexpott opened this issue Jan 4, 2018 · 3 comments
Closed

weak_vendors mode and isolated test #25684

alexpott opened this issue Jan 4, 2018 · 3 comments
Labels
Bug PhpUnitBridge Status: Needs Review

Comments

@alexpott
Copy link
Contributor
alexpott commented Jan 4, 2018 •
edited by nicolas-grekas
Loading
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 3.4.0

If you set SYMFONY_DEPRECATIONS_HELPER to weak_vendors and then have a test uses the @runTestsInSeparateProcesses annotation and does not have an @group legacy even though deprecations are triggered from non-vendor code, they are ignored. This is because the with-process-isolation-silenced-error is actually triggered from \Symfony\Bridge\PhpUnit\Legacy\SymfonyTestsListenerTrait::endTest() which is vendor code for most projects (eg. Drupal). For a real world example see: https://www.drupal.org/project/drupal/issues/2934336
@greg0ire
Copy link
Contributor
greg0ire commented Jan 4, 2018

I think that will imply changing how this method:

symfony/src/Symfony/Bridge/PhpUnit/DeprecationErrorHandler.php

Lines 64 to 86 in 96e53f8

$inVendors = function ($path) {
/** @var string[] absolute paths to vendor directories */
static $vendors;
if (null === $vendors) {
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')) {
$vendors[] = $v;
}
}
}
}
$path = realpath($path) ?: $path;
foreach ($vendors as $vendor) {
if (0 === strpos($path, $vendor) && false !== strpbrk(substr($path, strlen($vendor), 1), '/'.DIRECTORY_SEPARATOR)) {
return true;
}
}
return false;
};

Right now it accepts only one file, maybe it should accept the whole trace, and then we should go one step up in the trace if the file happens to be the trait.

@greg0ire
Copy link
Contributor
greg0ire commented Jan 4, 2018

The offending piece of code, for reference:

symfony/src/Symfony/Bridge/PhpUnit/Legacy/SymfonyTestsListenerTrait.php

Lines 261 to 273 in 96e53f8

if ($this->runsInSeparateProcess) {
$deprecations = file_get_contents($this->runsInSeparateProcess);
unlink($this->runsInSeparateProcess);
putenv('SYMFONY_DEPRECATIONS_SERIALIZE');
foreach ($deprecations ? unserialize($deprecations) : array() as $deprecation) {
if ($deprecation[0]) {
trigger_error(serialize(array('deprecation' => $deprecation[1], 'class' => $className, 'method' => $test->getName(false))), E_USER_DEPRECATED);
} else {
@trigger_error(serialize(array('deprecation' => $deprecation[1], 'class' => $className, 'method' => $test->getName(false))), E_USER_DEPRECATED);
}
}
$this->runsInSeparateProcess = false;
}

@alexpott alexpott mentioned this issue Jan 4, 2018
Closed
nicolas-grekas added a commit that referenced this issue Jan 4, 2018
@nicolas-grekas
bug #25685 Use triggering file to determine weak vendors if when the …
228453b 
…test is run in a separate process (alexpott)

This PR was squashed before being merged into the 3.4 branch (closes #25685).

Discussion
----------

Use triggering file to determine weak vendors if when the test is run in a separate process

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | I think so
| Fixed tickets | #25684
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!--highly recommended for new features-->

<!--
- Bug fixes must be submitted against the lowest 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 the master branch.
- Replace this comment by a description of what your PR is solving.
-->

Commits
-------

3830577 Use triggering file to determine weak vendors if when the test is run in a separate process
@greg0ire
Copy link
Contributor

@alexpott I believe this can be closed, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug PhpUnitBridge Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@nicolas-grekas @greg0ire @alexpott @xabbuh @carsonbot
0