Problem/Motivation

Note that the class names must end with the word 'Test'.

See: PHPUnit file structure, namespace, and required metadata

Class TestSettingSummariesContentType does not follow the test class naming convention

Proposed resolution

Rename core/modules/node/tests/src/FunctionalJavascript/TestSettingSummariesContentType.php to core/modules/node/tests/src/FunctionalJavascript/SettingSummariesContentTypeTest.php

And rename the class TestSettingSummariesContentType to SettingSummariesContentTypeTest

CommentFileSizeAuthor
#2 rename-3079856-2.patch820 bytesjungle

Comments

jungle created an issue. See original summary.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Assigned: jungle » Unassigned
Status: Active » Needs review
StatusFileSize
new rename-3079856-2.patch820 bytes

Submit the patch

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

dww’s picture

dww
we/he/they
commented

Re-queued for a more recent test run. I'd expect the # of passed assertions to go up relative to the branch values. But these test results are so stale, I can't tell. ;)

dww’s picture

dww
we/he/they
commented
Issue tags: +Needs issue summary update

Hrm, weird:

https://www.drupal.org/pift-ci-job/1623474

"28,260 pass" for both the patch and the latest from the branch. So it seems like this test class is already being automatically discovered and run. At least by run-tests.sh and the d.o testbot. Seems worth doing to match conventions, but I don't think the summary is entirely accurate.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Title: Rename TestSettingSummariesContentType.php to SettingSummariesContentTypeTest.php in the node module » Rename TestSettingSummariesContentType to SettingSummariesContentTypeTest by convention
Issue summary: View changes
Issue tags: -Needs issue summary update

Thanks for reviewing @dww! Nice catch!

dww’s picture

dww
we/he/they
commented
Priority: Normal » Minor
Status: Needs review » Reviewed & tested by the community

@jungle Re: #6: Cool, thanks for the updates.

I don't see a reason not to do this. Tentatively setting RTBC. Let's see what the core committers say. :)

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Related issues: +#3079860: Move node/tests/src/Functional/AssertButtonsTrait.php into node/tests/src/Traits

Cheers! BTW, a similar one I filed, but it seems #3079860 could be closed as I just checked they made a BC layer against 8.9.x

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Closed (duplicate)
Related issues: +#2296635: Evaluate performance impact of limiting test discovery to *Test.php filename suffix

Test discovery doesn't rely on the class ending in Test - if you search the current qa logs for "TestSettingSummaries" you'll find this test is running. However there is an issue to make this true... see #2296635: Evaluate performance impact of limiting test discovery to *Test.php filename suffix - in fact this issue is a straight up duplicate of that issue and there are other classes to fix.