The field_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
#20 clean_up_field_ui-2394417-20.patch22.96 KBhussainweb
#13 clean_up_field_ui-2394417-13.patch22.96 KBhussainweb
#13 interdiff-11-13.txt696 byteshussainweb
#11 issue-2394417-11.patch22.96 KBSivaji_Ganesh_Jojodae
#1 clean_up_field_ui-2394417-1.patch22.96 KBtibbsa
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Priority: Normal » Minor
Related issues: +#2382195: Clean-up simpletest module test members - ensure property definition and use of camelCase naming convention
FileSize
clean_up_field_ui-2394417-1.patch22.96 KB

There remains one instance of $this->root_user, but that will have to change as part of the Simpletest updates.

tibbsa’s picture

tibbsa CreditAttribution: tibbsa commented
Assigned: tibbsa » Unassigned
Mile23’s picture

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

Patch applies, phpcs says there aren't any test class camel case issues. Will re-run the testbot since the last run is a little stale. RTBC after that.

Mile23 queued 1: clean_up_field_ui-2394417-1.patch for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1: clean_up_field_ui-2394417-1.patch, failed testing.

Status: Needs work » Needs review

subhojit777 queued 1: clean_up_field_ui-2394417-1.patch for re-testing.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented

Patch still applies... Let's see if it passes the testbot... Again.

Mile23 queued 1: clean_up_field_ui-2394417-1.patch for re-testing.

Mile23’s picture

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

Still happy and green. Re-RTBC from #3.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Needs a reroll

Sivaji_Ganesh_Jojodae’s picture

Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
issue-2394417-11.patch22.96 KB

Patch #1 re-rolled.

Status: Needs review » Needs work

The last submitted patch, 11: issue-2394417-11.patch, failed testing.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Status: Needs work » Needs review
FileSize
interdiff-11-13.txt696 bytes
clean_up_field_ui-2394417-13.patch22.96 KB

Fixing the error.

Mile23’s picture

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

phpcs says no more underscore errors. Test passes, patch applies.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 13: clean_up_field_ui-2394417-13.patch, failed testing.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented

The test failure said: Filter by username returned the right amount. Other UserAdminTest.php 69 Drupal\user\Tests\UserAdminTest->testUserAdmin()
I'll retest, because I think this test is unrelated.

Status: Needs work » Needs review

Mile23 queued 13: clean_up_field_ui-2394417-13.patch for re-testing.

Mile23’s picture

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

Patch still looks good, all the way back from #3, through all the rerolls.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 13: clean_up_field_ui-2394417-13.patch, failed testing.

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Status: Needs work » Needs review
FileSize
clean_up_field_ui-2394417-20.patch22.96 KB

Straight reroll...

hussainweb’s picture

hussainweb
Primary language English
CreditAttribution: hussainweb commented
Status: Needs review » Reviewed & tested by the community

Marking it as RTBC as per #18 and #3 before that. It was a straight reroll, so another review is probably not needed.

alexpott’s picture

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

Committed c9e1f93 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed c9e1f93 on 8.0.x 
    Issue #2394417 by hussainweb, tibbsa, sivaji@knackforge.com: Clean-up...

Status: Fixed » Closed (fixed)

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