See #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)

Postponed on

Needs partial conversion to JavaScriptTestBase: #2809481: Convert AJAX part of \Drupal\editor\Tests\EditorAdminTest to JavascriptTestBase

Tests do be converted:

\Drupal\editor\Tests\EditorLoadingTest
\Drupal\editor\Tests\EditorSecurityTest
\Drupal\editor\Tests\EditorUploadImageScaleTest
\Drupal\editor\Tests\QuickEditIntegrationLoadingTest
CommentFileSizeAuthor
#21 interdiff-17-21.txt2.49 KBTess Bakker
#21 2870443-21.patch22.49 KBTess Bakker
#17 interdiff-13-17.txt14.91 KBTess Bakker
#17 2870443-17.patch22.23 KBTess Bakker
#13 2870443-13.patch10.59 KBLendude
#11 2870443-11.patch10.63 KBLendude
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

michielnugter created an issue. See original summary.

michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Parent issue: #2870441: Convert web tests to browser tests for content_translation module » #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)
michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Issue summary: View changes
Status: Active » Postponed
michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Related issues: +#2809481: Convert AJAX part of \Drupal\editor\Tests\EditorAdminTest to JavascriptTestBase
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Issue summary: View changes
Status: Postponed » Active
michielnugter’s picture

michielnugter CreditAttribution: michielnugter as a volunteer and at Synetic commented
Component: phpunit » editor.module
Issue tags: +phpunit initiative

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.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea as a volunteer and at Webikon commented
Status: Active » Postponed
Related issues: +#2795041: BrowserTestBase: Add drupalPostWithFormat

This is blocked on #2795041: BrowserTestBase: Add drupalPostWithFormat because of EditorSecurityTest test.

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.

nlisgo’s picture

nlisgo CreditAttribution: nlisgo commented
Status: Postponed » Active

Since #2795041: BrowserTestBase: Add drupalPostWithFormat is closed (won't fix) I am switching this back to active.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Status: Active » Needs review
FileSize
2870443-11.patch10.63 KB

first roll, just moving stuff around and fixing node element stuff.

This will fail on the ajaxy stuff.

Status: Needs review » Needs work

The last submitted patch, 11: 2870443-11.patch, failed testing. View results

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
FileSize
2870443-13.patch10.59 KB

and now rolled against 8.6.x

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Status: Needs work » Needs review

Status: Needs review » Needs work

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

Tess Bakker’s picture

Tess Bakker
they/them
Primary language Dutch
CreditAttribution: Tess Bakker at LimoenGroen commented
Assigned: Unassigned » Tess Bakker
Tess Bakker’s picture

Tess Bakker
they/them
Primary language Dutch
CreditAttribution: Tess Bakker at LimoenGroen commented
Status: Needs work » Needs review
FileSize
2870443-17.patch22.23 KB
interdiff-13-17.txt14.91 KB

New patch with changes to work with BrowserTestBase.

Done:
* Fixed all the "ajax/post" calls
* replaced t() with FormattableMarkup() when needed
* few other minor changes

Tess Bakker’s picture

Tess Bakker
they/them
Primary language Dutch
CreditAttribution: Tess Bakker at LimoenGroen commented
Assigned: Tess Bakker » Unassigned
Tess Bakker’s picture

Tess Bakker
they/them
Primary language Dutch
CreditAttribution: Tess Bakker at LimoenGroen commented
Issue tags: +DevDaysLisbon
Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Status: Needs review » Needs work

@Tessa Bakker this is looking really good already, just some nits:

  1. +++ b/core/modules/editor/tests/src/Functional/QuickEditIntegrationLoadingTest.php
@@ -1,18 +1,21 @@
+use Drupal\FunctionalJavascriptTests\JavascriptTestBase;

    unused use 

  2. +++ b/core/modules/editor/tests/src/Functional/QuickEditIntegrationLoadingTest.php
@@ -1,18 +1,21 @@
+use GuzzleHttp\Exception\ClientException;

    unused use 

  3. +++ b/core/modules/editor/tests/src/Functional/QuickEditIntegrationLoadingTest.php
@@ -108,13 +124,30 @@ public function testUserWithPermission() {
+  protected function getCookies() {

    this needs a docblock 

  4. +++ b/core/modules/editor/tests/src/Functional/EditorSecurityTest.php
@@ -389,13 +390,18 @@ public function testSwitchingSecurity() {
+    $domain = parse_url($this->getUrl(), PHP_URL_HOST);
+    $session_id = $this->getSession()->getCookie($this->getSessionName());
+    $cookies = CookieJar::fromArray([$this->getSessionName() => $session_id], $domain);

+++ b/core/modules/editor/tests/src/Functional/QuickEditIntegrationLoadingTest.php
@@ -108,13 +124,30 @@ public function testUserWithPermission() {
+      'cookies' => $this->getCookies(),

    two tests with two different ways of setting the needed cookies, can we do this in one consistent way?

Tess Bakker’s picture

Tess Bakker
they/them
Primary language Dutch
CreditAttribution: Tess Bakker at LimoenGroen commented
Status: Needs work » Needs review
FileSize
2870443-21.patch22.49 KB
interdiff-17-21.txt2.49 KB

Thanks for the review!

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

This looks great!

  1. +++ b/core/modules/editor/tests/src/Functional/EditorAdminTest.php
@@ -64,8 +65,8 @@ public function testNoEditorAvailable() {
-    $this->assertTrue(((string) $options[0]) === 'None', 'Option 1 in the Text Editor select is "None".');
...
+    $this->assertTrue(($options[0]->getText()) === 'None', 'Option 1 in the Text Editor select is "None".');

    This is already so much nicer! 

  2. +++ b/core/modules/editor/tests/src/Functional/EditorSecurityTest.php
@@ -433,7 +448,21 @@ public function testEditorXssFilterOverride() {
+  /**
+   * Get session cookies from current session.
+   *
+   * @return \GuzzleHttp\Cookie\CookieJar
+   *   A cookie jar with the current session.
+   */
+  protected function getCookies() {
+    $domain = parse_url($this->getUrl(), PHP_URL_HOST);
+    $session_id = $this->getSession()->getCookie($this->getSessionName());
+    $cookies = CookieJar::fromArray([$this->getSessionName() => $session_id], $domain);
+
+    return $cookies;
   }

    Should we create a follow up to make this process easier for people? Maybe it would it be worth to moving this to BrowserTestBase?

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude as a volunteer and at Dx Experts commented
Status: Needs review » Reviewed & tested by the community

Yeah this looks great.

Totally agree on the follow up, #2983504: Add a way to easily set the cookies in a request done using the Guzzle client

alexpott’s picture

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

Crediting reviewers and @michielnugter for the original issue creation / management.

Committed 0b8ffb4 and pushed to 8.6.x. Thanks!

  • alexpott committed 0b8ffb4 on 8.6.x 
    Issue #2870443 by Tessa Bakker, Lendude, michielnugter, dawehner:...

Status: Fixed » Closed (fixed)

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