The config 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
#44 config-testing-codingstandards-2381753-44.patch15.32 KBtadityar
#32 config-testing-codingstandards-2381753-32.patch15.32 KBtadityar
#27 config-testing-codingstandards-2381753-D8-26.patch15.36 KBtadityar
#26 config-testing-codingstandards-2381753-D8-26.patch15.36 KBtadityar
#24 interdiff_17-23.txt6.49 KBmile23
#23 config-testing-codingstandards-2381753-D8-23.patch15.32 KBtadityar
#15 config-testing-codingstandards-2381753-D8-15.patch15.43 KBtadityar
#13 config-testing-codingstandards-2381753-D8-13.patch15.43 KBtadityar
#17 config-testing-codingstandards-2381753-D8-17.patch15.38 KBtadityar
#10 config-testing-codingstandards-238175-9.patch15.38 KBtadityar
#7 config-testing-codingstandards-238175-7.patch15.43 KBtadityar
#5 config-testing-codingstandards-2381753-D8-1-5-interdiff.txt1.64 KBtibbsa
#5 config-testing-codingstandards-2381753-D8-5.patch15.38 KBtibbsa
#2 config-testing-codingstandards-2381753-D8-1.patch15.69 KBtibbsa

Comments

tibbsa’s picture

tibbsa commented
Assigned: Unassigned » tibbsa

Working on this.

tibbsa’s picture

tibbsa commented
Status: Active » Needs review
StatusFileSize
new config-testing-codingstandards-2381753-D8-1.patch15.69 KB

The remaining reference to $this->root_user in \Drupal\config\Tests\ConfigExportUITest requires changes to WebTestBase in the 'simpletest' module, and those changes will have consequences across multiple sub-issues: see comment #32 in the parent issue.

tibbsa’s picture

tibbsa commented
Assigned: tibbsa » Unassigned
cilefen’s picture

cilefen commented
Status: Needs review » Needs work

@tibbsa: nice work so far.

  • newSlogan and originalSlogan are undefined in ConfigExportImportUITest
  1. +++ b/core/modules/config/src/Tests/ConfigExportImportUITest.php
@@ -31,6 +31,27 @@ class ConfigExportImportUITest extends WebTestBase {
+   * @var \Drupal\node\Entity\NodeType

    This should be the interface, not the actual class. 

  2. +++ b/core/modules/config/src/Tests/ConfigExportImportUITest.php
@@ -31,6 +31,27 @@ class ConfigExportImportUITest extends WebTestBase {
+   * @var \Drupal\field\Entity\FieldStorageConfig

    This should be the interface, not the actual class. 

  3. +++ b/core/modules/config/src/Tests/ConfigFileContentTest.php
@@ -16,6 +16,7 @@
+

    There is no reason to change this file in this patch. 

  4. +++ b/core/modules/config/src/Tests/ConfigImportUITest.php
@@ -18,15 +18,28 @@
+   * Enable the Options and Text modules to ensure dependencies are handled
+   * correctly.

    You could remove this. 

  5. +++ b/core/modules/config/src/Tests/ConfigInstallTest.php
@@ -16,6 +16,10 @@
+  /**
+   * {@inheritdoc}
+   */

    Adding this is appropriate for the coding standards, but it is out-of-scope for this issue. I am not going to ask you to remove these, which was done in several files, but another reviewer may ask you to remove them.

tibbsa’s picture

tibbsa commented
Status: Needs work » Needs review
StatusFileSize
new config-testing-codingstandards-2381753-D8-5.patch15.38 KB
new config-testing-codingstandards-2381753-D8-1-5-interdiff.txt1.64 KB
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

Needs re-roll.

tadityar’s picture

tadityar commented
Assigned: Unassigned » tadityar
StatusFileSize
new config-testing-codingstandards-238175-7.patch15.43 KB

Re-roll of the patch.

tadityar’s picture

tadityar commented
Status: Needs work » Needs review

forgot to change to needs review

Status: Needs review » Needs work

The last submitted patch, 7: config-testing-codingstandards-238175-7.patch, failed testing.

tadityar’s picture

tadityar commented
StatusFileSize
new config-testing-codingstandards-238175-9.patch15.38 KB
tadityar’s picture

tadityar commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 10: config-testing-codingstandards-238175-9.patch, failed testing.

tadityar’s picture

tadityar commented
Status: Needs work » Needs review
StatusFileSize
new config-testing-codingstandards-2381753-D8-13.patch15.43 KB

re-roll try again

Status: Needs review » Needs work

The last submitted patch, 13: config-testing-codingstandards-2381753-D8-13.patch, failed testing.

tadityar’s picture

tadityar commented
Status: Needs work » Needs review
StatusFileSize
new config-testing-codingstandards-2381753-D8-15.patch15.43 KB

Status: Needs review » Needs work

The last submitted patch, 15: config-testing-codingstandards-2381753-D8-15.patch, failed testing.

tadityar’s picture

tadityar commented
Status: Needs work » Needs review
StatusFileSize
new config-testing-codingstandards-2381753-D8-17.patch15.38 KB

re-roll again. This time erase the DrupalTestBase

mile23’s picture

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

Good work, @tadityar. :-)

The patch in #17 refactors class-level properties from under_score to camelCase as the coding standards demand.

I know this because I applied the patch, ran my own coding standards review with netbeansdrupalcomposed, and poked through all the test classes to find camel case and underscore violations.

cilefen’s picture

cilefen commented 
  1. +++ b/core/modules/config/src/Tests/ConfigInstallTest.php
@@ -15,7 +15,13 @@
+

    Added space should not be here. 

  2. +++ b/core/modules/config/src/Tests/ConfigInstallTest.php
@@ -15,7 +15,13 @@
+

    Added space should not be here.

cilefen’s picture

cilefen commented 
  1. +++ b/core/modules/config/src/Tests/ConfigInstallTest.php
@@ -15,7 +15,13 @@
  * @group config
  * @see \Drupal\Core\Config\ConfigInstaller
  */
+
 class ConfigInstallTest extends KernelTestBase {

    here 

  2. +++ b/core/modules/config/src/Tests/ConfigInstallTest.php
@@ -15,7 +15,13 @@
+
+  /**
+   * {@inheritdoc}
+   */
+
   protected function setUp() {
     parent::setUp();

    and here

tadityar’s picture

tadityar commented

@cilefen, will work on that later this day. Thank you for pointing that out!

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/config/src/Tests/ConfigEntityNormalizeTest.php
@@ -11,6 +11,11 @@
+   * Modules to enable.

+++ b/core/modules/config/src/Tests/ConfigImportUITest.php
@@ -18,15 +18,25 @@
+   * Modules to enable.

+++ b/core/modules/config/src/Tests/ConfigImportUploadTest.php
@@ -16,13 +16,25 @@
+   * Modules to enable.

+++ b/core/modules/config/src/Tests/ConfigLanguageOverrideWebTest.php
@@ -18,8 +18,16 @@
+   * Modules to enable.

+++ b/core/modules/config/src/Tests/ConfigModuleOverridesTest.php
@@ -16,6 +16,11 @@
+   * Modules to enable.

+++ b/core/modules/config/src/Tests/ConfigOverridesPriorityTest.php
@@ -18,6 +18,11 @@
+   * Modules to enable.

This should be "install" not "enable".

tadityar’s picture

tadityar commented
Status: Needs work » Needs review
StatusFileSize
new config-testing-codingstandards-2381753-D8-23.patch15.32 KB
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
StatusFileSize
new interdiff_17-23.txt6.49 KB

Here's an interdiff.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work 
  1. +++ b/core/modules/config/src/Tests/ConfigLanguageOverrideWebTest.php
@@ -19,12 +19,13 @@
+  ¶

    Introduces a bunch of whitespace errors in addition to this one. 

  2. +++ b/core/modules/config/src/Tests/Storage/ConfigStorageTestBase.php
@@ -15,7 +15,7 @@
- * Therefore, storage tests use an uncommon test case class structure;
+ * Therefore, storage tests use a uncommon test case class structure;

    It should be 'an.' :-)

The rest is good. :-)

tadityar’s picture

tadityar commented
Status: Needs work » Needs review
StatusFileSize
new config-testing-codingstandards-2381753-D8-26.patch15.36 KB

Let's go!

tadityar’s picture

tadityar commented
StatusFileSize
new config-testing-codingstandards-2381753-D8-26.patch15.36 KB

re-upload for re-testing, the previous patch was stuck in queue and testing.

mile23’s picture

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

The patch in #27 accomplishes the goal of changing all under_score properties to camelCase, within the config module's SimpleTest namespace.

I know this because I used a combination of NetBeans and phpcs to look for the underscore coding standard error.

The patch also does a few things outside the scope of this issue, but they're all positive and never negative. :-)

So therefore: RTBC.

Thanks, @tadityar!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 27: config-testing-codingstandards-2381753-D8-26.patch, failed testing.

Status: Needs work » Needs review

areke queued 27: config-testing-codingstandards-2381753-D8-26.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 27: config-testing-codingstandards-2381753-D8-26.patch, failed testing.

tadityar’s picture

tadityar commented
Status: Needs work » Needs review
StatusFileSize
new config-testing-codingstandards-2381753-32.patch15.32 KB

re-rolled as the patch in #27 suddenly failed testing.

mile23’s picture

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

Looks good. Patch applies now. Re-upping the RTBC from #28.

basic queued 27: config-testing-codingstandards-2381753-D8-26.patch for re-testing.

The last submitted patch, 26: config-testing-codingstandards-2381753-D8-26.patch, failed testing.

The last submitted patch, 27: config-testing-codingstandards-2381753-D8-26.patch, failed testing.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

OK, #36 says the patch failed, but #32 is green. So let's ask the testbot *yet again.*

Mile23 queued 32: config-testing-codingstandards-2381753-32.patch for re-testing.

tibbsa’s picture

tibbsa commented

Note those failures are failures of an earlier patch (#26). I'm not sure why @basic re-queued #26 for testing.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

OK, well that answers that question. Still green, still RTBC.

tadityar’s picture

tadityar commented

@Mile23, ah.. so that's it. I was confused about why even though something failed testing the status is still RTBC..

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 32: config-testing-codingstandards-2381753-32.patch, failed testing.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

Patch doesn't apply, needs reroll: https://www.drupal.org/patch/reroll

tadityar’s picture

tadityar commented
Status: Needs work » Needs review
StatusFileSize
new config-testing-codingstandards-2381753-44.patch15.32 KB

Re-rolled

mile23’s picture

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

phpcs says no camel case errors in the test classes. Everything else looks good, and we're green on tests.

adci_contributor’s picture

adci_contributor commented
Issue tags: -Needs reroll
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Looks like Alex has been committing others of these, so joining the club. :P

Committed and pushed to 8.0.x. Thanks!

  • webchick committed cc14b66 on 8.0.x 
    Issue #2381753 by tadityar, tibbsa, Mile23, cilefen: Clean-up Config...

Status: Fixed » Closed (fixed)

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