Problem/Motivation

For now, the CKEditorIntegrationTest for the DrupalImageCaption plugin doesn't exists.
I would suggest a new method for testDrupalImageCaptionDialog into the CKEditorIntegrationTest which will check the basic behaviours of DrupalImageCaption CKEditor Plugin.

Proposed resolution

Add a new testDrupalImageCaptionDialog method.

Should checks for:

  • Caption's checkbox is visible when the filter_caption is enable
  • Caption's checkbox is not visible when the filter_caption is disable

Remaining tasks

Review & Improve

User interface changes

None

API changes

None

Data model changes

None

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

wengerk created an issue. See original summary.

wengerk’s picture

First draft for a patch, let's test it with testbot.

wengerk’s picture

The patch in #2 works but I use some features scheduled for removal in Drupal 9.0.0 e.g. assertElementNotPresent from trait Drupal\FunctionalTests\AssertLegacyTrait.

I switch to the Drupal\Tests\WebAssert methods as recommended in Drupal\FunctionalTests\AssertLegacyTrait.

wengerk’s picture

Status: Needs work » Needs review
Wim Leers’s picture

Title: Extend the CKEditorIntegrationTest for DrupalImageCaption » [PP-1] Extend the CKEditorIntegrationTest for DrupalImageCaption
Status: Needs review » Postponed

Let's wait for #2912399: Extend the CKEditorIntegrationTest for DrupalImage to land first, so we can adopt the same pattern as that one used.

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.

wengerk’s picture

Refactoring code following suggestion on #2912399.

Add a patch variante with same tests x110 to detect random fail.
Let's test them.

wengerk’s picture

Title: [PP-1] Extend the CKEditorIntegrationTest for DrupalImageCaption » Extend the CKEditorIntegrationTest for DrupalImageCaption
Status: Postponed » Needs work
Issue tags: +Needs reroll

Now that the parent isssu 2912399 is merged, let's work on this one.

wengerk’s picture

Here is the rerolled patch, let's try it with testbot !

PS: Added a patch variante with same tests x110 to detect random fail (as we did for the parent issue #2912399).

wengerk’s picture

Issue tags: -Needs reroll
wengerk’s picture

Status: Needs work » Needs review
Wim Leers’s picture

@wengerk++
@wengerk++
@wengerk++

WELL DONE!

Sadly, it seems like you found a random failure :( Retesting to see if we can reproduce it.

wengerk’s picture

I also relaunch twice, to be sure it pass. Let's see

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
2.99 KB
2.63 KB

Passed 3 times out of 4, with 110 test runs each. LGTM!

Time for a review.

  1. +++ b/core/modules/ckeditor/tests/src/FunctionalJavascript/CKEditorIntegrationTest.php
    @@ -36,12 +36,12 @@ protected function setUp() {
    +    $this->filtered_html_format->save();
    

    My main remark: $this->filtered_html_format is not actually an object property. Fixed that.

  2. +++ b/core/modules/ckeditor/tests/src/FunctionalJavascript/CKEditorIntegrationTest.php
    @@ -139,4 +139,37 @@ public function testDrupalImageDialog() {
    +    // Disable the filter caption.
    

    Nit: s/filter caption/caption filter/

  3. +++ b/core/modules/ckeditor/tests/src/FunctionalJavascript/CKEditorIntegrationTest.php
    @@ -139,4 +139,37 @@ public function testDrupalImageDialog() {
    +    // If the filter caption is disabled, the caption's checkbox
    +    // should not be present.
    

    Nit: 80 cols.

  4. +++ b/core/modules/ckeditor/tests/src/FunctionalJavascript/CKEditorIntegrationTest.php
    @@ -139,4 +139,37 @@ public function testDrupalImageDialog() {
    +    // If the filter caption is enable, the caption's checkbox
    

    Nit: s/enable/enabled/

Fixed all my feedback. All minor. So going to RTBC right away :)

Thanks again!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 7254e79e8e to 8.7.x and 9d30c1a795 to 8.6.x. Thanks!

  • alexpott committed 7254e79 on 8.7.x
    Issue #2916589 by wengerk, Wim Leers: Extend the CKEditorIntegrationTest...

  • alexpott committed 9d30c1a on 8.6.x
    Issue #2916589 by wengerk, Wim Leers: Extend the CKEditorIntegrationTest...

Status: Fixed » Closed (fixed)

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