Skip to content

[PhpUnitBridge] Add the ability to expect a deprecation inside a test #35192

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

Conversation

fancyweb
Copy link
Contributor

@fancyweb fancyweb commented Jan 3, 2020

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

Replaces #25757

Proposed implementation uses a dedicated trait for a better DX. Using $this->expectDeprecation() feels natural.

Unfortunately it is not currently possible to test the cases that should produce errors or risky, so there are some things that are not testable here. I plan to propose another feature for the PhpUnitBridge to be able to test those kind of things. If it's accepted, we will then be able to strenghten the tests of this one.

@fancyweb
Copy link
Contributor Author

fancyweb commented Jan 3, 2020

fabbot failure must remain for 5.3 compat.

@fancyweb fancyweb force-pushed the phpunit-bridge-expect-deprecation-in-test branch from 3037bdc to c84e1c8 Compare January 3, 2020 11:36
@nicolas-grekas nicolas-grekas added this to the next milestone Jan 4, 2020
@fancyweb fancyweb force-pushed the phpunit-bridge-expect-deprecation-in-test branch 2 times, most recently from 19e3d1f to 4dd759e Compare January 21, 2020 09:37
@fancyweb fancyweb force-pushed the phpunit-bridge-expect-deprecation-in-test branch from 4dd759e to 295fdc2 Compare February 3, 2020 17:36
@nicolas-grekas nicolas-grekas force-pushed the phpunit-bridge-expect-deprecation-in-test branch from 295fdc2 to 532a477 Compare February 6, 2020 17:05
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM, just minor things.

@fancyweb fancyweb force-pushed the phpunit-bridge-expect-deprecation-in-test branch 3 times, most recently from 0a59633 to b953982 Compare February 6, 2020 19:27
@fancyweb fancyweb force-pushed the phpunit-bridge-expect-deprecation-in-test branch from b953982 to a5cb78a Compare February 10, 2020 08:39
@nicolas-grekas nicolas-grekas force-pushed the phpunit-bridge-expect-deprecation-in-test branch from a5cb78a to a3a9280 Compare February 11, 2020 10:39
@nicolas-grekas
Copy link
Member

Thank you @fancyweb.

nicolas-grekas added a commit that referenced this pull request Feb 11, 2020
…n inside a test (fancyweb)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[PhpUnitBridge] Add the ability to expect a deprecation inside a test

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

Replaces #25757

Proposed implementation uses a dedicated trait for a better DX. Using `$this->expectDeprecation()` feels natural.

Unfortunately it is not currently possible to test the cases that should produce errors or risky, so there are some things that are not testable here. I plan to propose another feature for the PhpUnitBridge to be able to test those kind of things. If it's accepted, we will then be able to strenghten the tests of this one.

Commits
-------

a3a9280 [PhpUnitBridge] Add the ability to expect a deprecation inside a test
@nicolas-grekas nicolas-grekas merged commit a3a9280 into symfony:master Feb 11, 2020
public function testMany()
{
$this->expectDeprecation('foo');
$this->expectDeprecation('bar');
Copy link
Member

Choose a reason for hiding this comment

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

To actually test that the feature is working, we would also need to write tests where expectDeprecation is called but not deprecation is triggered, to show that the test fail in such case.

Of course, this must not be part of the main Symfony testsuite run (here, you are writing tests as if they were testing trigger_error, not as if they were testing the bridge), but as phpt tests which can assert that running these tests report a failure.

@stof
Copy link
Member

stof commented Feb 11, 2020

expectDeprecation conflicts with a method introduced in the base TestCase class in PHPUnit 9. Is it OK or should we find another name ?

@nicolas-grekas
Copy link
Member

expectDeprecation conflicts with a method introduced in the base TestCase class in PHPUnit 9. Is it OK or should we find another name ?

good catch.

I think this is fine: when ppl use the trait, they want the behavior of the bridge.

@fancyweb fancyweb deleted the phpunit-bridge-expect-deprecation-in-test branch February 13, 2020 07:54
fabpot added a commit that referenced this pull request Mar 16, 2020
…ion (hkdobrev)

This PR was squashed before being merged into the 5.1-dev branch.

Discussion
----------

[PhpUnitBridge] Deprecate @expectedDeprecation annotation

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | yes<!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | N/A
| License       | MIT
| Doc PR        | N/A

Addresses https://github.com/orgs/symfony/projects/1#card-32934769 as a follow-up to #35192.

Deprecating `@expectedDeprecation` annotation on tests in favour of the `expectDeprecation()` method similar to other PHPUnit deprecations of annotations in favour of methods.

Commits
-------

36a57cc [PhpUnitBridge] Deprecate @expectedDeprecation annotation
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
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