[PP-upstream] Rename test classes not ending with Test to end with Test

Still needs a test to check if test ends with Test

Bhanu951 → closed merge request !4332

The ending with test feels like something we could add to phpstan - any non-abstract subclass of whatever the root test base class is. I think we should open a follow-up for that. These specific tests aren't going to be renamed again once this goes in and it'll stop regressions that would cause those tests to fail.

Opened https://github.com/mglaman/phpstan-drupal/issues/608 and tried to implement it with https://github.com/mglaman/phpstan-drupal/pull/609

As to be expected, that unveiled more incorrectly named test classes.

Also, we need guidance on *TestBase classes, should they always be abstract?

For giggles, tried that in a new, separate MR, so to not mess up the one from @Bhanu951.
Seems if we make the offending *TestBase classes abstract, nothing breaks.

The base classes should always be abstract yes, not sure whether it's documented but it's what should happen.

One more related issue #3268490: Rename and organise Test Traits → which we need to work on.

We have a test to check file names here → we can utilise it for resolving this issue.

Seems this issue can resolve few more old issues.

Thanks @catch for the confirmation on the "base classes should always be abstract".

@Bhanu951: Not sure if we want to solve all the mentioned issues in one go and or in this one issue.

Let's try the get the proposed rule in PHPStan first and take it from there. Extending it later to deal with class names and Traits seems more manageable.

Postponing this issue on:

- Getting the proposed rule PR into mglaman/phpstan-drupal
- A release of mglaman/phpstan-drupal with that rule in it.

If that's done we can up the version of mglaman/phpstan-drupal in the root composer.json to that release and fix the found problems both in this issue.

I'm not sure we should postpone getting 'ghost' tests to run on the phpstan rule, since every day they don't run is an additional chance we could commit something that will make them fail, so would be fine with reversing the order.

Fair point, although it looks like all of the tests are running with our without the "Test" prefix:

From QA run https://dispatcher.drupalci.org/job/drupal8_core_regression_tests/86948/...

00:04:35.008 Drupal\Tests\Core\Theme\CoreThemesAutoloadedForTests           1 passes                                      
00:09:13.038 Drupal\Tests\taxonomy\Kernel\Migrate\d6\MigrateTermNodeCompl   1 passes                                      
00:16:53.327 Drupal\Tests\migrate_drupal_ui\Functional\d7\Upgrade7TestWit   1 passes                                      
00:17:18.780 Drupal\Tests\migrate_drupal_ui\Functional\d6\Upgrade6TestWit   1 passes     
00:17:38.772 Drupal\FunctionalTests\Asset\AssetOptimizationTestUmami        1 passes                                       
00:17:39.009 Drupal\Tests\block_content\Functional\Update\BlockContentRem   1 passes             
00:21:08.832 Drupal\FunctionalTests\Installer\InstallerExistingConfigProf   1 passes           
00:21:09.731 Drupal\FunctionalTests\Installer\InstallerExistingConfigSync   1 passes                                                                                                                        
00:28:16.826 Drupal\Tests\help\Functional\Update\HelpTopicsMerge            1 passes                                      
00:28:16.941 Drupal\Tests\help\Functional\Update\HelpTopicsUninstall        1 passes      
00:21:35.225 Drupal\Tests\jsonapi\Functional\RestJsonApiUnsupported         1 passes                                                                      
00:21:35.592 Drupal\Tests\jsonapi\Functional\RestExportJsonApiUnsupported   1 passes                                      
00:23:26.689 Drupal\Tests\node\Functional\NodeAccessCacheabilityWithNodeG   1 passes         
00:28:34.944 Drupal\Tests\system\Functional\Update\UpdateDescriptionConfi   1 passes                                                                   

Spokje → closed merge request !4914