Problem/Motivation

Please refer the problem/motivation section of #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method

Proposed resolution

Write EntityResourceTestBase subclass for the Editor entity.

Remaining tasks

References

1. Follow-up of #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method
2. Subtask of #2824572: Write EntityResourceTestBase subclasses for every other entity type.

CommentFileSizeAuthor
#5 interdiff-2843782-3-5.txt871 bytesshadcn
#5 entityresource_provide-2843782-5.patch9.93 KBshadcn
#5 interdiff-2843782-3-5-fail.txt930 bytesshadcn
#5 entityresource_provide-2843782-5-fail.patch9.93 KBshadcn
#3 entityresource_provide-2843782-3.patch9.92 KBshadcn
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

naveenvalecha created an issue. See original summary.

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.

shadcn’s picture

shadcn CreditAttribution: shadcn at Chapter Three commented
Version: 8.3.x-dev » 8.4.x-dev
Assigned: Unassigned » shadcn
Status: Active » Needs review
FileSize
entityresource_provide-2843782-3.patch9.92 KB

OK let's test this one.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Needs work

Nit:

+++ b/core/modules/rest/tests/src/Functional/EntityResource/Editor/EditorResourceTestBase.php
@@ -0,0 +1,184 @@
+      'editor' => 'ckeditor',
+      'image_upload' => [

Let's use setEditor() and setImageUploadSettings().

shadcn’s picture

shadcn CreditAttribution: shadcn at Chapter Three commented
Status: Needs work » Needs review
FileSize
entityresource_provide-2843782-5-fail.patch9.93 KB
interdiff-2843782-3-5-fail.txt930 bytes
entityresource_provide-2843782-5.patch9.93 KB
interdiff-2843782-3-5.txt871 bytes

It seems like there a bug where you can't create an Editor entity without passing the editor on the Editor::create(); . See the fail patch.

shadcn’s picture

shadcn CreditAttribution: shadcn at Chapter Three commented

Related: #2155895: \Drupal\editor\Entity\Editor should use a LazyPluginCollection.

The last submitted patch, 5: entityresource_provide-2843782-5-fail.patch, failed testing.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Reviewed & tested by the community

Aha! Yeah, fixing that is certainly out of scope.

That means this is ready!

Just one gotcha I want to call out for the core committer that looks at this:

+++ b/core/modules/rest/tests/src/Functional/EntityResource/Editor/EditorResourceTestBase.php
@@ -0,0 +1,184 @@
+      'settings' => [
+        'toolbar' => [
+          'rows' => [
+            [
+              [
+                'name' => 'Formatting',
+                'items' => [
+                  'Bold',
+                  'Italic',
+                ],
+              ],
+              [
+                'name' => 'Links',
+                'items' => [
+                  'DrupalLink',
+                  'DrupalUnlink',
+                ],
+              ],
+              [
+                'name' => 'Lists',
+                'items' => [
+                  'BulletedList',
+                  'NumberedList',
+                ],
+              ],
+              [
+                'name' => 'Media',
+                'items' => [
+                  'Blockquote',
+                  'DrupalImage',
+                ],
+              ],
+              [
+                'name' => 'Tools',
+                'items' => [
+                  'Source',
+                ],
+              ],
+            ],
+          ],
+        ],
+        'plugins' => [
+          'language' => [
+            'language_list' => 'un',
+          ],
+        ],
+      ],

This makes sense, these are the default settings, as specified by \Drupal\ckeditor\Plugin\Editor\CKEditor::getDefaultSettings().

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: entityresource_provide-2843782-5.patch, failed testing.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs work » Reviewed & tested by the community

DrupalCI infra fail.

  • catch committed af81a98 on 8.4.x 
    Issue #2843782 by arshadcn: EntityResource: Provide comprehensive test...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed af81a98 and pushed to 8.4.x. Thanks!

Test coverage only so moving 8.3.x for cherry-pick after the commit freeze.

  • alexpott committed 4de9495 on 8.3.x authored by catch 
    Issue #2843782 by arshadcn: EntityResource: Provide comprehensive test...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Patch (to be ported) » Fixed
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

Thanks!

Status: Fixed » Closed (fixed)

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