The taxonomy 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
#6 clean_up_taxonomy-2396717-6.patch37.32 KBhussainweb
#1 clean_up_taxonomy-2396717-1.patch38.66 KBhussainweb

Comments

hussainweb’s picture

hussainweb
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new clean_up_taxonomy-2396717-1.patch38.66 KB
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs review » Needs work

Good work on plowing through these issues.

  1. +++ b/core/modules/taxonomy/src/Tests/RssTest.php
@@ -23,42 +23,55 @@ class RssTest extends TaxonomyTestBase {
-      'settings' => array(
-        'allowed_values' => array(
-          array(
+      'settings' => [
+        'allowed_values' => [
+          [

    Changing the array declarations is really out of scope, but not that big a deal. 

  2. +++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyTestBase.php
@@ -22,26 +22,33 @@
+  protected $vocabulary;

+++ b/core/modules/taxonomy/src/Tests/VocabularyUiTest.php
@@ -15,10 +15,16 @@
 class VocabularyUiTest extends TaxonomyTestBase {
 
+  /**
+   * The vocabulary used for creating terms.
+   *
+   * @var \Drupal\taxonomy\VocabularyInterface
+   */
+  protected $vocabulary;

    You added $vocabulary to both TaxonomyTestBase and VocabularyUiTest, which is its subclass. I'd leave the one in TaxonomyTestBase and remove the one in VocabularyUiTest.

hussainweb’s picture

hussainweb
Primary language English
commented
Status: Needs work » Needs review

Responding to comments in #2:

1. I know it is out of scope but I am touching all that I can. It looks concise and much better. Of course, there is no difference in functionality.

2. VocabularyUiTest extends \Drupal\taxonomy\Tests\TaxonomyTestBase. The other $vocabulary definition is actually in \Drupal\taxonomy\Tests\Views\TaxonomyTestBase. They are different classes. This means we need to define $vocabulary in VocabularyUiTest as well.

I am setting it back to needs review as there is nothing to do here. :)

mile23’s picture

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

OK, so checking with phpcs I find that the error of having under_score property names is fixed to camelCase. The $variables question is answered, and yes, you're right, they're different. The out-of-scope change isn't such a big deal, so RTBC the patch in #1.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/taxonomy/src/Tests/RssTest.php
@@ -23,42 +23,55 @@ class RssTest extends TaxonomyTestBase {
-  public static $modules = array('node', 'field_ui', 'views');
+  public static $modules = ['node', 'field_ui', 'views'];

+++ b/core/modules/taxonomy/src/Tests/TaxonomyTermIndentationTest.php
@@ -19,12 +19,18 @@ class TaxonomyTermIndentationTest extends TaxonomyTestBase {
-  public static $modules = array('taxonomy');
+  public static $modules = ['taxonomy'];

+++ b/core/modules/taxonomy/src/Tests/TermTranslationUITest.php
@@ -29,7 +29,7 @@ class TermTranslationUITest extends ContentTranslationUITest {
-  public static $modules = array('language', 'content_translation', 'taxonomy');
+  public static $modules = ['language', 'content_translation', 'taxonomy'];

+++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyFieldFilterTest.php
@@ -23,21 +23,28 @@ class TaxonomyFieldFilterTest extends ViewTestBase {
-  public static $modules = array('language', 'taxonomy', 'taxonomy_test_views', 'text', 'views', 'node');
+  public static $modules = ['language', 'taxonomy', 'taxonomy_test_views', 'text', 'views', 'node'];
...
-  public static $testViews = array('test_field_filters');
+  public static $testViews = ['test_field_filters'];

+++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyTermViewTest.php
@@ -26,7 +26,21 @@ class TaxonomyTermViewTest extends TaxonomyTestBase {
-  public static $modules = array('taxonomy', 'views');
+  public static $modules = ['taxonomy', 'views'];

+++ b/core/modules/taxonomy/src/Tests/Views/TaxonomyTestBase.php
@@ -22,26 +22,33 @@
-  public static $modules = array('taxonomy', 'taxonomy_test_views');
+  public static $modules = ['taxonomy', 'taxonomy_test_views'];

These are out-of-scope changes.

hussainweb’s picture

hussainweb
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new clean_up_taxonomy-2396717-6.patch37.32 KB

There is no interdiff as this was also a reroll. I removed all the changes with the new array syntax except where the lines are very different.

mile23’s picture

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

Thanks for sticking with it, @hussainweb. :-)

Patch in #6 fixes the camelCase errors, and also doesn't have out of scope array definition changes.

alexpott’s picture

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

Committed a658710 and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed a658710 on 8.0.x 
    Issue #2396717 by hussainweb: Clean-up taxonomy module test members -...

Status: Fixed » Closed (fixed)

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