[PhpUnitBridge] deprecate the testLegacy test name prefix #21140
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
xabbuh
commented
Jan 2, 2017
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | no |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | yes |
| Tests pass? | yes |
| Fixed tickets | #18755 |
| License | MIT |
| Doc PR |
| @@ -197,6 +197,11 @@ public function endTest(\PHPUnit_Framework_Test $test, $time) | |||
| DnsMock::withMockedHosts(array()); | |||
| } | |||
| } | |||
|
|
|||
| if ($test instanceof \PHPUnit_Framework_TestCase && 0 === strpos($test->getName(), 'testLegacy') && empty($test->getTestResultObject()->warningCount())) { | |||
There was a problem hiding this comment.
when the annotation is in place, we should not trigger the warning. What about the other cases?
From DeprecationErrorHandler:
} elseif (0 === strpos($method, 'testLegacy')
|| 0 === strpos($method, 'provideLegacy')
|| 0 === strpos($method, 'getLegacy')
|| strpos($class, '\Legacy')There was a problem hiding this comment.
Adding the warning based on class/method names does work. I am looking into how to do that (if possible) based on the data transformer.
|
Status: Needs work |
OskarStark
reviewed
Jan 3, 2017
| @@ -170,6 +172,13 @@ public function startTest(\PHPUnit_Framework_Test $test) | |||
| } | |||
| } | |||
|
|
|||
| public function addWarning(\PHPUnit_Framework_Test $test, \PHPUnit_Framework_Warning $e, $time) | |||
| { | |||
| if ($test instanceof \PHPUnit_Framework_TestCase) { | |||
There was a problem hiding this comment.
why do we need this check here, we already typehint the parameter
There was a problem hiding this comment.
The type hint is for \PHPUnit_Framework_Test, but not \PHPUnit_Framework_TestCase.
|
The Status: Needs review |
|
After talking to @nicolas-grekas the warnings will now only be triggered if the |
693cf86 to
2448fbc
Compare
| if (in_array('time-sensitive', $groups, true)) { | ||
| ClockMock::withClockMock(false); | ||
| } | ||
| if (in_array('dns-sensitive', $groups, true)) { | ||
| DnsMock::withMockedHosts(array()); | ||
| } | ||
| } | ||
|
|
||
| if ($test instanceof \PHPUnit_Framework_TestCase && 0 === strpos($test->getName(), 'testLegacy') && !isset($this->testsWithWarnings[$test->getName()]) && !in_array('legacy', $groups, true)) { | ||
| $test->getTestResultObject()->addWarning($test, new \PHPUnit_Framework_Warning('Using the testLegacy prefix to mark tests as legacy is deprecated since version 3.3 and will be removed in 4.0. Use the "@group legacy" notation instead to add the test to the legacy group.'), $time); |
There was a problem hiding this comment.
quotes around "testLegacy"
There was a problem hiding this comment.
quotes added in both places
| } | ||
|
|
||
| if ($test instanceof \PHPUnit_Framework_TestCase && strpos($className, '\Legacy') && !isset($this->testsWithWarnings[$test->getName()]) && !in_array('legacy', $classGroups, true)) { | ||
| $test->getTestResultObject()->addWarning($test, new \PHPUnit_Framework_Warning('Using the Legacy prefix to mark all tests of a class as legacy is deprecated since version 3.3 and will be removed in 4.0. Use the "@group legacy" notation instead to add the test to the legacy group.'), $time); |
|
Thank you @xabbuh. |
nicolas-grekas
added a commit
that referenced
this pull request
Jan 6, 2017
…fix (xabbuh) This PR was merged into the 3.3-dev branch. Discussion ---------- [PhpUnitBridge] deprecate the testLegacy test name prefix | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #18755 | License | MIT | Doc PR | Commits ------- 21b72dd deprecate the Legacy/testLegacy test name prefix
javiereguiluz
added a commit
to symfony/symfony-docs
that referenced
this pull request
Apr 24, 2017
…, javiereguiluz) This PR was merged into the master branch. Discussion ---------- Do not document deprecated legacy test mechanisms see symfony/symfony#21140 Commits ------- c2c680a Fixed a minor typo 6f546aa do not document deprecated legacy test mechanisms
Merged
xabbuh
added a commit
to xabbuh/symfony
that referenced
this pull request
May 21, 2017
fabpot
added a commit
that referenced
this pull request
May 21, 2017
This PR was merged into the 3.3 branch. Discussion ---------- [PhpUnitBridge] add changelog entries for #21140 | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #21140 | License | MIT | Doc PR | Commits ------- c83c5bc [PhpUnitBridge] add changelog entries for #21140
nicolas-grekas
added a commit
that referenced
this pull request
May 22, 2017
* 3.3: [Serializer] Remove a useless legacy annotation Fixed extra semi-colon fix docblock position [DependencyInjection] remove unused variable [PhpUnitBridge] add changelog entries for #21140 [DI] Remove dead service_container checks
nicolas-grekas
added a commit
that referenced
this pull request
May 22, 2017
* 3.4: [Serializer] Remove a useless legacy annotation Fixed extra semi-colon fix docblock position [DependencyInjection] remove unused variable [PhpUnitBridge] add changelog entries for #21140 [DI] Remove dead service_container checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.