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

Allow Drupal to wrap the Symfony test listener (5.1 backport) #38031

Conversation

alexpott
Copy link
Contributor

@alexpott alexpott commented Sep 2, 2020

Q A
Branch? 5.1
Bug fix? kinda
New feature? no
Deprecations? no
Tickets Fix #...
License MIT
Doc PR symfony/symfony-docs#...

This is a backport of #37708

@alexpott alexpott changed the title Allow Drupal to wrap the Symfony test listener Allow Drupal to wrap the Symfony test listener (5.1 backport) Sep 2, 2020
@nicolas-grekas nicolas-grekas added this to the 5.1 milestone Sep 2, 2020
$parsedMsg = unserialize($this->message);
restore_error_handler();
set_error_handler(function () {});
$parsedMsg = @unserialize($this->message);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the `@ is not required anymore since the error handler ignores everything

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's in master - that's why I added it back.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove it here and submit a PR to clean up master.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #38033 to address this in master.

This reverts commit ee8cc26.
@fabpot
Copy link
Member

fabpot commented Sep 2, 2020

Thank you @alexpott.

@fabpot fabpot merged commit a36c43b into symfony:5.1 Sep 2, 2020
@fabpot fabpot mentioned this pull request Sep 27, 2020
nicolas-grekas added a commit that referenced this pull request Oct 2, 2020
…e TestsListener (fancyweb)

This PR was merged into the 5.1 branch.

Discussion
----------

[PhpUnitBridge] Fix Deprecation file when it comes from the TestsListener

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

#38031 caused an unwanted regression: if the frame `class` matches `SymfonyTestsListenerFor` it used to always return, now it continues so the `originClass` and `originMethod` properties are set. Therefore, the deprecation is considered as coming from the test listener. It ends up in the "legacy" group and muted because the test listeners contains the word "Legacy" in their namespaces.

Example test:
```php
<?php

namespace App\Tests;

use PHPUnit\Framework\TestCase;
use Symfony\Component\EventDispatcher\DependencyInjection\ExtractingEventDispatcher;

final class FooTest extends TestCase
{
    public function test(): void
    {
        // ExtractingEventDispatcher is @internal
        new class extends ExtractingEventDispatcher {};

        $this->assertTrue(true);
    }
}
```

On 4.4:
```
Other deprecation notices (1)

  1x: The "Symfony\Component\EventDispatcher\DependencyInjection\ExtractingEventDispatcher" class is considered internal. It may change without further notice. You should not use it from "Symfony\Component\EventDispatcher\DependencyInjection\ExtractingEventDispatcher@anonymous".
```

On 5.1:
```
Legacy deprecation notices (1)
```

Commits
-------

1ba06a0 [PhpUnitBridge] Fix Deprecation file when it comes from the TestsListener
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants