[FrameworkBundle][BC Break ?] Accessing static property $client in classes extending WebTestCase

[gnutix]

Symfony version(s) affected: 4.3.0-BETA2

Description
In symfony/framework-bundle@58e572e, protected static $client; was added to \Symfony\Bundle\FrameworkBundle\Test\WebTestCase.

I've upgraded my project to Symfony 4.3.0-BETA2 and had to fix my test suite and that of already multiple different libraries because it used $this->client to access the client (which is now triggering errors like Accessing static property Craue\FormFlowBundle\Tests\RevalidatePreviousStepsFlowTest::$client as non static, when it used to "work" before).

How to reproduce
Clone https://github.com/craue/CraueFormFlowBundle on branch "autoconfiguration" and run composer install && vendor/bin/phpunit.

Possible Solution
Either revert the addition, or write documentation as I think it's a BC break of sorts.

[raress96]

I also have this same problem with the full version 4.3.0.
What is more annoying is that I get this error from tests class from another custom bundle I made, but when running the tests from the main application.

[gmponos]

I am not sure if the WebTestCase should be under the BC promise

https://symfony.com/doc/current/contributing/code/bc.html

Theoritically it is not.. never the less I believe that this specific class it is extended by a LOT a LOT people and it makes their update path a little bit difficult. For me I have to update a gazillion of Tests classes in order to update to 4.3

I can not see the reason why the client was added there...

[raress96]

Or if the client is really required there, the name can be changed to testClient or something else to not conflict with other clients. This seems like a really big BC break, since there are bundles which use that class, and if you use them then the hole app will crash and you can not fix it.

[gnutix]

@fabpot This might require your attention as Symfony 4.3.0 has just been released with this?

[gmponos]

Since the Tests are not under BC then it could be easily reverted I guess to ease the upgrade process.

[gnutix]

@gmponos The doc states :

[...] classes located in the various *\Tests\ namespaces are an exception to this rule. They are meant for internal use only and should not be accessed by your own code.

But \Symfony\Bundle\FrameworkBundle\Test\WebTestCase is not under a *\Tests\* namespace (which would contain tests for Symfony code), but under a Test namespace (which contains utilities for testing). Therefore, I think it should be covered by the BC promise.

[xabbuh]

It is covered by the BC promise. But it also states that we may add new properties to a class. We rather need to discuss if we should use another name in this particular case to mitigate the impact.

[gmponos]

Ah... a small s that I missed there...

[Simperfit]

I already updated, but IMHO this should be treated as a BC Break and either be reverted or a new property with another name should be added to not break the existing. We can add new property but this is way to impacting to not be treated as a BC.

[nicolas-grekas]

See #31880

[curry684]

I had similar issues, rewrote half my test suite to accomodate for the 4.3.0 changes and rename $client in my own test cases, then upgraded to 4.3.1 just to see my CI phpStan section break again on invalid method calls on Response... #31904 should fix that last remnant.