8000 Marked the ResolveParameterPlaceHoldersPassTest as legacy by saro0h · Pull Request #13935 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Marked the ResolveParameterPlaceHoldersPassTest as legacy #13935

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

Closed
wants to merge 1 commit into from

Conversation

saro0h
Copy link
Contributor
@saro0h saro0h commented Mar 16, 2015
Q A
Bug fix? kinda
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets ~
License MIT
Doc PR ~

Since the last merge of 2.6 in 2.7, the tests are not ok. (a003380...17ad6fd)

@saro0h
Copy link
Contributor Author
saro0h commented Mar 16, 2015

Ping @fabpot @nicolas-grekas

(There is one test failing =>

  1. Symfony\Component\Process\Tests\SimpleProcessTest::testIdleTimeoutNotExceededWhenOutputIsSent

Failed asserting that false is true.)

@nicolas-grekas
Copy link
Member

I think we need to split this in two test classes, one testing parameter resolution with the legacy interface, and one testing with the new interface.
Please also add the @group legacy annotation to any legacy test or class.

@saro0h saro0h force-pushed the fix-legacy branch 7 times, most recently from fcb855e to 0a3f60e Compare March 16, 2015 12:19
@saro0h
Copy link
Contributor Author
saro0h commented Mar 16, 2015

ping @nicolas-grekas : it's done. Is this what you meant ?


8000 namespace Symfony\Component\DependencyInjection\Tests\Compiler;

class LegacyResolveParameterPlaceHoldersPassTest extends ResolveParameterPlaceHoldersPassTest
Copy link
Member

Choose a reason for hiding this comment

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

This setup is wrong: you are running all the ResolveParameterPlaceHoldersPassTest twice now: once for ResolveParameterPlaceHoldersPassTest and once for LegacyResolveParameterPlaceHoldersPassTest. LegacyResolveParameterPlaceHoldersPassTest should run only the legacy tests

@saro0h saro0h force-pushed the fix-legacy branch 4 times, most recently from 74c589e to 52d7909 Compare March 16, 2015 15:36
@saro0h
Copy link
Contributor Author
saro0h commented Mar 16, 2015

This PR must be reopen on 2.6 => #13941

@saro0h saro0h closed this Mar 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0