Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Followup of Symfony TestContainer issue #212

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

Merged
ondrejmirtes merged 1 commit into phpstan:master from Pierstoval:27-test-container
Dec 5, 2021
Merged

Followup of Symfony TestContainer issue #212

ondrejmirtes merged 1 commit into phpstan:master from Pierstoval:27-test-container
Dec 5, 2021

Conversation

Copy link
Contributor

@Pierstoval Pierstoval commented Nov 29, 2021

Followup of #27 and #210

adrienfr, IonBazan, and ossinkine reacted with rocket emoji
Copy link
Member

Please fix the build failures.

Copy link
Contributor Author

Pierstoval commented Nov 30, 2021
edited
Loading

Done!

Copy link

yakobe commented Dec 5, 2021

@Pierstoval This works perfectly for me too 👍 !
@ondrejmirtes Would it be possible to get this in? Or is there a release schedule to wait for?
Thanks for all the hard work everyone 😊.

@ondrejmirtes ondrejmirtes merged commit a5aed92 into phpstan:master Dec 5, 2021
Copy link
Member

Thank you! Gonna release it immediately.

yakobe reacted with hooray emoji yakobe reacted with rocket emoji

@Pierstoval Pierstoval deleted the 27-test-container branch December 5, 2021 19:50
Copy link
Contributor

This seems to end in a strange error now when using a trait expecting a getContainer method with ContainerInterface as return type to exist:

I'm now getting:

 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Line src/Sulu/Bundle/TestBundle/Testing/ContainerTrait.php (in context of class Sulu\Bundle\TestBundle\Testing\KernelTestCase)
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 32 Method Sulu\Bundle\TestBundle\Testing\KernelTestCase::getContainer() should return Symfony\Bundle\FrameworkBundle\Test\TestContainer but returns Symfony\Component\DependencyInjection\ContainerInterface.
 ------ ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

I'm not understanding why here Symfony\Bundle\FrameworkBundle\Test\TestContainer is expected.

I think the Problem is that phpstan is not knowing that TestContainer is an instance of Symfony\Component\DependencyInjection\ContainerInterface now? But I could not find it how to achieve this.

janedbal reacted with thumbs up emoji

/**
* @return TestContainer
*/
abstract public function getContainer();
Copy link
Contributor

@alexander-schranz alexander-schranz Dec 7, 2021

Choose a reason for hiding this comment

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

Copy link
Contributor

I'm meeting the same issue as @alexander-schranz, why KernelTestCase::getContainer is marked to return TestContainer? There is no such thing guarranteed in the implementation, is it?

Copy link
Contributor Author

Pierstoval commented Dec 20, 2021
edited
Loading

Indeed there's no real guarantee based on types, but in practice, it's always the case when using the full-stack framework: https://github.com/symfony/symfony/blob/60ce5a3dfbd90fad60cd39fcb3d7bf7888a48659/src/Symfony/Bundle/FrameworkBundle/Resources/config/test.php#L48

Copy link
Contributor

Think Symfony\Bundle\FrameworkBundle\Test\TestContainer is correct and nobody would overwrite that. But think the main problem is that PHPStan does not now that Symfony\Bundle\FrameworkBundle\Test\TestContainer is implementing Symfony\Component\DependencyInjection\ContainerInterface. Because if it would I think the error would be gone.

The stubs implementation looks for me currently not allowing to achieve this, and also think that the stubs are not very future proof as when typehints change in different symfony versions. Maybe there is another workaround to tell that KernelTestCase return a TestContainer without adding stubs here, so PHPStan also knows which interfaces TestContainer implements and also would implement in the future. /cc @ondrejmirtes do you see other ways to achieve the same result?

Maybe a more general solution to provide access to test container xml and dev container xml and based what files are analyzed the correct services are returned so also other services which are available only in test environment work like expected:

 container_xml_path: var/cache/dev/srcDevDebugProjectContainer.xml
 test_container_xml_path: var/cache/test/srcTestDebugProjectContainer.xml
 test_file_regex: 'Test.php$'

Or we can manipulate the loading of srcDevDebugProjectContainer that test.service_container points to TestContainer.

Copy link
Contributor Author

test.service_container should already contain an instance of TestContainer.

About the XML part, I'm not sure about what you'd like to achieve. I personally specify the Test container in my container_xml_path and it doesn't detect the TestContainer either, unless using the stubs in phpstan 😕

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@alexander-schranz alexander-schranz alexander-schranz left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /