The menu_ui module uses test class members with underscored names. Some examples are big_user, web_user and admin_user, but there could be others. According to our coding conventions, these should be renamed to bigUser, webUser and adminUser. In addition, some properties are undefined but should be.

See the parent issue #1811638: [meta] Clean-up Test members - ensure property definition and use of camelCase naming convention.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task, because this is a coding standards change.
Issue priority Not critical because coding standard changes are not critical.
Unfrozen changes Unfrozen because it only changes automated tests.
Disruption There is no disruption expected from this sort of change.
CommentFileSizeAuthor
#7 issue-2396699-7.patch5.79 KBSivaji_Ganesh_Jojodae
#4 clean_up_menu_ui_module-2396699-4.patch6.12 KBhussainweb
#1 clean_up_menu_ui_module-2396699-1.patch6.05 KBhussainweb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Status: Active » Needs review
FileSize
clean_up_menu_ui_module-2396699-1.patch6.05 KB

Removed a few property definitions which were not used out of a single method and renamed others.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs review » Reviewed & tested by the community

phpcs says no camel case errors in test classes. The testbot run is 4 days old but I think it's recent enough.

Rawk.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: clean_up_menu_ui_module-2396699-1.patch, failed testing.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Status: Needs work » Needs review
FileSize
clean_up_menu_ui_module-2396699-4.patch6.12 KB

Straight, simple reroll. :)

No changes to code, just surrounding lines. I daresay it is good for RTBC as soon as it passes but let's see what others say.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Issue tags: +Needs reroll

Needs reroll, and while you're there...

+++ b/core/modules/menu_ui/src/Tests/MenuNodeTest.php
@@ -22,24 +22,23 @@ class MenuNodeTest extends WebTestBase {
-  public static $modules = array('menu_ui', 'test_page_test', 'node', 'block');
+  public static $modules = ['menu_ui', 'test_page_test', 'node', 'block'];

Out of scope....

Otherwise great. Thanks, @hussainweb!

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs review » Needs work
Sivaji_Ganesh_Jojodae’s picture

Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
issue-2396699-7.patch5.79 KB

Re-rolled patch #4 with proper usage of array() mentioned in #5.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs review » Reviewed & tested by the community

No underscore property names in the test classes, and no array re-declarations.

Thanks sivaji@knackforge.com and hussainweb!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed e988258 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed e988258 on 8.0.x 
    Issue #2396699 by hussainweb, sivaji@knackforge.com: Clean-up menu_ui...

Status: Fixed » Closed (fixed)

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