Problem/Motivation

I would like to refactor the UserData class cos currently the get method looks like something the cat dragged in. Compare to the tidy MongoDB version which can be applied to the SQL version as well just the query is different, the result rows are not. But there are no tests. Woe.

Proposed resolution

Add tests.

Remaining tasks

Add tests.

User interface changes

API changes

CommentFileSizeAuthor
#11 add-tests-2373573-11.diff2.8 KBavpaderno
#7 interdiff-2373573-1-7.txt591 bytesyogeshmpawar
#7 userdata_needs_testing-2373573-7.patch2.74 KByogeshmpawar
#1 2373573-user-data-test-1.patch2.8 KBvijaycs85

Comments

vijaycs85’s picture

vijaycs85
Location London, UK
commented
StatusFileSize
new 2373573-user-data-test-1.patch2.8 KB

Here is the incomplete test blocked by #2373613: FakeStatement doesn't behaves as same as Statement

chx’s picture

chx commented
Assigned: Unassigned » chx

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Version: 8.3.x-dev » 8.4.x-dev
Status: Active » Needs work 
+/**
+ * @file
+ * Contains \Drupal\user\Tests\UserDataTest.
+ */

@file is not required anymore for those files.

+/**
+ * Tests the user entity class.
+ *
+ * @group user
+ * @see \Drupal\user\UserData
+ */
+class UserDataTest extends UnitTestCase {

It is testing the user.data service, not the user entity class.

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location Pune(MH), INDIA 🇮🇳
commented
Status: Needs work » Needs review
StatusFileSize
new userdata_needs_testing-2373573-7.patch2.74 KB
new interdiff-2373573-1-7.txt591 bytes

Changes done as per comment # & also added an interdiff.

Status: Needs review » Needs work

The last submitted patch, 7: userdata_needs_testing-2373573-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review

The failure doesn't show any error in the patch, but rather a problem with the testing system. I am going to force a re-test.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Needs work

PHP_CodeSniffer is suggesting to replace array() with []; there are few places where that is needed, in the patch.

This is the patch against the patch it reports. 

--- modules/user/src/Tests/UserDataTest.php
+++ PHP_CodeSniffer
@@ -20,12 +20,12 @@
    * @see \Drupal\user\UserData::get()
    */
   public function testGetMethod() {
-    $database_contents = array('users_data' => $this->getDummyUserData());
+    $database_contents = ['users_data' => $this->getDummyUserData()];
     $connection = $this->getDatabase($database_contents);
     $user_data = new UserData($connection);
 
     // 1. Get all data of given module.
-    $expected = array();
+    $expected = [];
     $actual = $user_data->get('foo_module');
     $this->assertSame($expected, $actual);
 
@@ -53,7 +53,7 @@
    * @return \Drupal\Core\Database\Driver\fake\FakeConnection
    *   The database connection.
    */
-  protected function getDatabase(array $database_contents, $connection_options = array(), $prefix = '') {
+  protected function getDatabase(array $database_contents, $connection_options = [], $prefix = '') {
     return new FakeConnection($database_contents, $connection_options, $prefix);
   }
 
@@ -63,36 +63,36 @@
    * @return array
    */
   protected function getDummyUserData() {
-    return array(
-      array(
+    return [
+      [
         'module' => 'foo_module',
         'uid' => 1,
         'name' => 'key1',
         'value' => 'value1',
         'serialized' => FALSE
-      ),
-      array(
+      ],
+      [
         'module' => 'foo_module',
         'uid' => 2,
         'name' => 'key2',
         'value' => 'value2',
         'serialized' => FALSE
-      ),
-      array(
+      ],
+      [
         'module' => 'foo_module',
         'uid' => 3,
         'name' => 'key3',
         'value' => 'value3',
         'serialized' => FALSE
-      ),
-      array(
+      ],
+      [
         'module' => 'bar_module',
         'uid' => 1,
         'name' => 'key1',
         'value' => 'value1',
         'serialized' => FALSE
-      ),
-    );
+      ],
+    ];
   }
 
 }

I apologize for changing the status twice. I first didn't notice the file attached to the automatic comment.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new add-tests-2373573-11.diff2.8 KB

I also added the comma after the last array item, as per Drupal coding standards, the section about arrays.

Status: Needs review » Needs work

The last submitted patch, 11: add-tests-2373573-11.diff, failed testing. View results

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented

I take the error is saying that core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php doesn't exist anymore.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: chx » Unassigned

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.

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

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bradjones1’s picture

bradjones1
Primary language English
commented
Related issues: +#3343634: Add "json" as core data type to Schema and Database API

Not sure this helps at all but in the course of creating an example implementation for #3343634: Add "json" as core data type to Schema and Database API, I added some test coverage for user data to ensure I was hitting (most) of this implementation. The polymorphism is strong, especially in the delete() method, but it might be something to help move this along.

https://git.drupalcode.org/project/drupal/-/blob/168d5353dcb8fc2b8decfe6...

https://git.drupalcode.org/project/drupal/-/blob/168d5353dcb8fc2b8decfe6...

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.