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

Add ability to set expected deprecation message inside a test #25757

Closed
wants to merge 7 commits into from

Conversation

alexpott
Copy link
Contributor

@alexpott alexpott commented Jan 10, 2018

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

It could be useful to have the ability to dynamic set expected deprecations within tests. In https://www.drupal.org/project/drupal/issues/2858482 we are deprecating a lot of dynamically generated routes and these routes have tests that can't set an expected deprecation message because each test case has something like a provider which results in different deprecation messages.

@wimleers
Copy link

Also, it nicely mirrors \PHPUnit_Framework_TestCase::setExpectedException(), just like @expectedDeprecation mirrors @expectedException.

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Jan 10, 2018
$this->error = new $AssertionFailedError('Only tests with the `@group legacy` annotation can have `@expectedDeprecation`.');
}

$test->getTestResultObject()->beStrictAboutTestsThatDoNotTestAnything(false);
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've moved this to end tests but because we reset the value using $this->reportUselessTests this code does not seem to have the intended affect and we still to add $this->addToAssertionCount(1); to the test to make it not risky.

@nicolas-grekas
Copy link
Member

Note this is a new feature, so should target 4.1/master.

/**
* Sets an expected deprecation message.
*
* @param $msg
Copy link
Member

Choose a reason for hiding this comment

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

one line, missing type

@alexpott alexpott changed the base branch from 3.4 to master January 11, 2018 12:01
@alexpott
Copy link
Contributor Author

The current solution with have problems in isolated tests.

@wimleers
Copy link

wimleers commented Jan 17, 2018

@alexpott: Couldn't you port the changes you made in https://www.drupal.org/project/drupal/issues/2936802 over to this PR to address that?

4.1.0
-----

* support for adding expected deprecations via `SymfonyTestsListenerTrait::expectDeprecation()`
Copy link
Member

Choose a reason for hiding this comment

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

What about adding a new class to make this a bit more friendly,
eg SymfonyTestHelper::expectDeprecation() at the root of the bridge's namespace?

Copy link
Contributor

Choose a reason for hiding this comment

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

Calling anything on a trait definitely smells.
Keeping it in sync with $this->expectDeprecation() called inside tests would be the best imho.

@Majkl578
Copy link
Contributor

Majkl578 commented Apr 8, 2018

ping?

@nicolas-grekas nicolas-grekas modified the milestones: 3.4, next Mar 31, 2019
@chalasr chalasr added this to chalasr's in Help Wanted Aug 4, 2019
@vudaltsov
Copy link
Contributor

@alexpott , do you mind me finishing this?

@alexpott
Copy link
Contributor Author

alexpott commented Aug 5, 2019

@vudaltsov not at all! That would be fantastic.

@nicolas-grekas
Copy link
Member

Continued in #35192, thanks @alexpott

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 modified the milestones: next, 5.1 May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Help Wanted
chalasr's
Development

Successfully merging this pull request may close these issues.

None yet

8 participants