Skip to content

[FrameworkBundle] Add KernelTestCase::getContainer() #40366

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
merged 1 commit into from
Mar 12, 2021

Conversation

Nyholm
Copy link
Member

@Nyholm Nyholm commented Mar 4, 2021

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? yes
Tickets
License MIT
Doc PR symfony/symfony-docs#14731

There are at least 3 ways to get the container in a test class:

class FooTest extends WebTestCase
{
    public function testGetContainerA()
    {
        $kernel = self::bootKernel();
        $container = $kernel->getContainer();
    }

    public function testGetContainerB()
    {
        self::bootKernel();
        $container = self::$container;
    }

    public function testGetContainerC()
    {
        $client = self::createClient();
        $container = $client->getContainer();
    }
}

I suggest to add a fourth =)

Basically, in tests you should always use the test.service_container. It is hard to remove A and C, but I can deprecate C and add a helper function.

class FooTest extends WebTestCase
{
    public function testGetContainerTheOnlyWayYouShouldUse()
    {
        $container = $this->getContainer();
    }
}

This new way will also boot your kernel if it is not already booted.

@nicolas-grekas nicolas-grekas added this to the 5.x milestone Mar 9, 2021
Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

At first I thought: Oh no. But I think this makes sense: you'll get a friendly error (instead of a completely different container) and no more "you'll have to boot the kernel first".

Sorry for the complete nitpicking on anything that wasn't PHP code in this review 🙈

@Nyholm
Copy link
Member Author

Nyholm commented Mar 9, 2021

Thank you for the review. I've updated the PR according to your suggestions.

@fabpot
Copy link
Member

fabpot commented Mar 12, 2021

Thank you @Nyholm.

@fabpot fabpot merged commit 183b91c into symfony:5.x Mar 12, 2021
@Nyholm Nyholm deleted the test-container branch March 12, 2021 06:58
@Nyholm
Copy link
Member Author

Nyholm commented Mar 12, 2021

Thank you for merging.

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.

7 participants