Remove use of TestClass constructor

Neat find.

Not going to pretend to fully understand but the change of remvoing fake() makes sense said out loud. And all tests are passing (Not even a nightwatch failure)

Does PHPUnit 10 throw a specific exception/error/etc. when this kind of thing happens, so that we can ensure it won't creep back in? Or ideas on other ways to prevent future weirdness along these lines?

Its actually a PHP ArgumentCountError error because the require arguments for the constructor don't exist.

I've updated the IS with a specific examples of the error that this causes.

As far as preventing it that's a good question. In the short term, I'm not sure. But it looks like the plan in the parent issue is to support 2 versions and do upgrade testing like we did with previous PHPUnit upgrades. Once we get that PHPUnit 10 testing in place it would definitely start flagging weirdness like this. This issue was mostly part of getting obvious errors out of the way so the upgrade is a smaller easier to review task.

Its possible this could be caught in static analysis though and we _could_ reach out to mglaman about implementing a rule in phpstan-drupal to catch it. That might help contrib see the error before trying to upgrade to PHPUnit 10. It might also be overkill though because its technically code outside of Drupal. *shrug*

Think that sounds like a plan

Or is there a script we could run in gitlab? And make it part of the check temporarily?

The script would have to be pretty complicated because the ways this get triggered are complicated. Code could be directly interacting with TestClass like core/tests/Drupal/Tests/Core/Test/AssertContentTraitTest.php which would only require understanding use statements and namespaces or something more complicated like a child class in core/tests/Drupal/Tests/Component/Utility/VariableTest.php which would require understanding LSB, static class instantiation, and the entire php inheritance of every class. Both are pretty complicated and the latter is _very_ difficult. That's why I suggested if we wanted a test, a static analysis tool that understands the structure of php code like phpstan would be the place to put it..

This seems to me such an edge case that I do not think it makes sense to overengineer a preventive check. If it fails, it will fail in PHPUnit 10 and people will fix the edge case.

+1 on RTBC.

@mondrake My concern is that the tests might not fail on PHPUnit 10, and instead would silently behave in weird ways, which could lead to false positives or difficult-to-debug seemingly random failures or such.

However, given the feedback above I don't think that needs to be a blocking concern. Perhaps we can create a followup for static analysis or such regarding it? Tagging for such.

Followup added: 📌 Determine if we should add static analysis for weird uses of TestCase Active

Committed to 11.x, and cherry-picked to 10.2.x as a patch-safe test improvement. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.