Problem/Motivation

We have introduced ParagraphTestBase and only used it on few tests.

Many have repetitive setup and the test base is not used.

Proposed resolution

Use the ParagraphsTestBase.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

miro_dietiker created an issue. See original summary.

miro_dietiker’s picture

Likely candidates for the Para*TestBase
ParagraphsAccessTest
ParagraphsAdministrationTest
ParagraphsPreviewTest
ParagraphsTranslationTest

ModernMantra’s picture

Small patch that possibly fixes some classes. I had some test fails on local machine that seems is not caused by changes in patch, lets see test bot...

Ginovski’s picture

Status: Needs review » Needs work
+++ b/src/Tests/ParagraphsAccessTest.php
@@ -13,7 +13,7 @@ use Drupal\user\Entity\Role;
+class ParagraphsAccessTest extends ParagraphsTestBase {

1.Remove all enabled modules in AccessTest that are already enabled in the TestBase ('node', 'paragraphs', 'field', 'field_ui', 'block').
Same for AdministrationTest, PreviewTest and TranslationTest.
2. Correct the setup function in all the extending classes (remove $this->drupalPlaceBlock() when it is repeating with the TestBase)
3. Use function loginAsAdmin from the TestBase when possible in the extending classes (instead of $this->drupalLogin()).

miro_dietiker’s picture

Just switching to the Paragraphs base is not of value. It only makes sense to switch to it, if the test itself gets more simple. Using a base introduces general setup and helpers. Each of the use cases need to switch leveraging them.

ModernMantra’s picture

Considering previous two comments #4 and #5, providing new patch. No interdiff since previous trials have not been correct.

Status: Needs review » Needs work

The last submitted patch, 6: use_paragraphstestbase-2780363-6.patch, failed testing.

ModernMantra’s picture

Reverted some stuff for ParagraphsUninstall (seems that we did not need to use there ParagraphsTestBase) and some small fixes for failures of tests. I hope it will be green.

johnchque’s picture

Status: Needs review » Needs work
+++ b/src/Tests/ParagraphsAccessTest.php
@@ -37,31 +32,20 @@ class ParagraphsAccessTest extends WebTestBase {
   protected function setUp() {
     parent::setUp();
-    $this->drupalPlaceBlock('local_tasks_block');
-    $this->drupalPlaceBlock('page_title_block');
   }

I think this can be all removed. since we are calling the parent anyway.

Let's try to use the methods provided by base so the tests are shorter. :)

ModernMantra’s picture

Some more refactoring... This refactoring and adopting classes to use ParagraphsTestBase broke many tests and thus going too much into refactoring will cause almost 'reimplementation' of some tests (for instance ParagraphsAdministrationTest)

Status: Needs review » Needs work

The last submitted patch, 10: use_paragraphstestbase-2780363-10.patch, failed testing.

miro_dietiker’s picture

IMHO you should apply the transformation step by step by trying to avoid to create bigger breaks as you started at #3
So yeah, that's the purpose of using the base - tranforming lots of duplicated code into the methods / helpers provided. But that is more refactoring than reimplementation.

ModernMantra’s picture

A little bit more refactoring in ParagraphsAdministrationTest.

Status: Needs review » Needs work

The last submitted patch, 13: use_paragraphstestbase-2780363-13.patch, failed testing.

ModernMantra’s picture

Tests should be green now :)

miro_dietiker’s picture

Status: Needs review » Needs work
  1. +++ b/src/Tests/ParagraphsAdministrationTest.php
    @@ -38,39 +33,21 @@ class ParagraphsAdministrationTest extends WebTestBase {
    +    //$this->drupalCreateContentType(array('type' => 'article', 'name' => 'Article'));
    
    +++ b/src/Tests/ParagraphsAdministrationTest.php
    @@ -234,31 +191,20 @@ class ParagraphsAdministrationTest extends WebTestBase {
    +//    $this->clickLink(t('Manage display'));
    +//    $this->drupalPostForm(NULL, array('fields[field_paragraphs][type]' => 'entity_reference_revisions_entity_view'), t('Save'));
    +//    $this->clickLink(t('Manage form display'));
    +//    $this->drupalPostForm(NULL, array('fields[field_paragraphs][type]' => 'entity_reference_paragraphs'), t('Save'));
    

    What should happen with this commented code?

johnchque’s picture

+++ b/src/Tests/ParagraphsAdministrationTest.php
@@ -234,31 +191,20 @@ class ParagraphsAdministrationTest extends WebTestBase {
-    // Create an article with paragraphs field.
-    static::fieldUIAddNewField('admin/structure/types/manage/article', 'paragraphs', 'Paragraphs', 'entity_reference_revisions', array(
-      'settings[target_type]' => 'paragraph',
-      'cardinality' => '-1',
-    ), array(
-      'description' => 'Help text.',
-    ));
-    // Configure article fields.
-    $this->drupalGet('admin/structure/types/manage/article/fields');
-    $this->clickLink(t('Edit'), 1);
-    $this->drupalPostForm(NULL, NULL, t('Save settings'));
-    $this->clickLink(t('Manage display'));
-    $this->drupalPostForm(NULL, array('fields[field_paragraphs][type]' => 'entity_reference_revisions_entity_view'), t('Save'));
-    $this->clickLink(t('Manage form display'));
-    $this->drupalPostForm(NULL, array('fields[field_paragraphs][type]' => 'entity_reference_paragraphs'), t('Save'));
+//    $this->clickLink(t('Manage display'));
+//    $this->drupalPostForm(NULL, array('fields[field_paragraphs][type]' => 'entity_reference_revisions_entity_view'), t('Save'));
+//    $this->clickLink(t('Manage form display'));
+//    $this->drupalPostForm(NULL, array('fields[field_paragraphs][type]' => 'entity_reference_paragraphs'), t('Save'));

I think we can use addParagraphedContentType instead if this code.

ModernMantra’s picture

Referring to comment 17, it is already done, otherwise it would be everything 'red'. Removed some commented block of code and improved comment. Should be ok now :)

miro_dietiker’s picture

Status: Needs review » Fixed

Yay, committed. :-)

Status: Fixed » Closed (fixed)

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