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

CommentFileSizeAuthor
#17 2862494-17.patch5.22 KBklausi
#16 interdiff-12-16.txt988 bytesjofitz
#16 convert_web_tests_to-2862494-16.patch7.11 KBjofitz
#12 interdiff-2862494-3-12.txt900 bytesgoz
#12 convert_web_tests_to-2862494-12.patch7.78 KBgoz
#3 basic_auth-2862494-3.patch4.89 KBklausi

Comments

klausi created an issue. See original summary.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Parent issue: » #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)
Related issues: -#2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase)
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Active » Needs review
StatusFileSize
new basic_auth-2862494-3.patch4.89 KB

Patch.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

This is as minimal as it can be.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: basic_auth-2862494-3.patch, failed testing.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Reviewed & tested by the community

Random test failure because disk full on testbot machine.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 3: basic_auth-2862494-3.patch, failed testing.

goz’s picture

goz commented
Status: Needs work » Needs review

Patch apply manually and tests success in local. Relaunch tests

goz’s picture

goz commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +drupaldevdays
goz’s picture

goz commented
Issue tags: -drupaldevdays +DevDaysSeville

wrong tag for seville

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/basic_auth/src/Tests/BasicAuthTestTrait.php
@@ -4,6 +4,9 @@
+ *
+ * @deprecated Scheduled for removal in Drupal 9.0.0.
+ *   Use \Drupal\Tests\basic_auth\Traits\BasicAuthTestTrait instead.

We should follow the deprecation policy and add an @trigger_error and a change notice. See https://www.drupal.org/node/2856615

goz’s picture

goz commented
Status: Needs work » Needs review
StatusFileSize
new convert_web_tests_to-2862494-12.patch7.78 KB
new interdiff-2862494-3-12.txt900 bytes

Change record has been created https://www.drupal.org/node/2862800.

Status: Needs review » Needs work

The last submitted patch, 12: convert_web_tests_to-2862494-12.patch, failed testing.

jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Needs review

Latest test is now passing so setting back to Needs Review.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

Thanks. Can you create the patch with "git diff --find-copies-harder" or "git diff -M5%" so that we see that the new trait is just a copy with minor modifications?

+++ b/core/modules/basic_auth/src/Tests/BasicAuthTestTrait.php
@@ -4,7 +4,13 @@
+@trigger_error(__FILE__ . ' is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. Use \Drupal\Tests\basic_auth\Traits\BasicAuthTestTrait instead. See https://www.drupal.org/node/2862800.', E_USER_DEPRECATED);

https://www.drupal.org/node/2856615#how-class says "Add @trigger_error('...', E_USER_DEPRECATED) under the namespace declaration". So this is in the wrong place right now, should be further up.

jofitz’s picture

jofitz
he/him
commented
Status: Needs work » Needs review
StatusFileSize
new convert_web_tests_to-2862494-16.patch7.11 KB
new interdiff-12-16.txt988 bytes

Moved @trigger_error().

Created patch with -M5% (although it makes no difference with this patch).

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new 2862494-17.patch5.22 KB

Hm, my git diff detects the copy correctly. Uploading that patch, not other changes.

Looks good!

dawehner’s picture

dawehner
Primary language German
commented

So we still keep the @deprecated tag around on top of the trigger_error() bit?

alexpott’s picture

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

@dawehner yep that we cover both runtime and IDE.

Committed and pushed f1bfa50 to 8.4.x and 59f0a25 to 8.3.x. Thanks!

diff --git a/core/modules/basic_auth/src/Tests/BasicAuthTestTrait.php b/core/modules/basic_auth/src/Tests/BasicAuthTestTrait.php
index 78056c9..0e0aceb 100644
--- a/core/modules/basic_auth/src/Tests/BasicAuthTestTrait.php
+++ b/core/modules/basic_auth/src/Tests/BasicAuthTestTrait.php
@@ -2,12 +2,12 @@
 
 namespace Drupal\basic_auth\Tests;
 
-@trigger_error(__FILE__ . ' is deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0. Use \Drupal\Tests\basic_auth\Traits\BasicAuthTestTrait instead. See https://www.drupal.org/node/2862800.', E_USER_DEPRECATED);
+@trigger_error(__FILE__ . ' is deprecated in Drupal 8.3.0 and will be removed before Drupal 9.0.0. Use \Drupal\Tests\basic_auth\Traits\BasicAuthTestTrait instead. See https://www.drupal.org/node/2862800.', E_USER_DEPRECATED);
 
 /**
  * Provides common functionality for Basic Authentication test classes.
  *
- * @deprecated in Drupal 8.4.0 and will be removed before Drupal 9.0.0.
+ * @deprecated in Drupal 8.3.0 and will be removed before Drupal 9.0.0.
  *   Use \Drupal\Tests\basic_auth\Traits\BasicAuthTestTrait instead.
  *
  * @see https://www.drupal.org/node/2862800

Since we can commit this to 8.3.x change this on commit.

  • alexpott committed f1bfa50 on 8.4.x 
    Issue #2862494 by klausi, GoZ, Jo Fitzgerald: Convert web tests to...

  • alexpott committed 59f0a25 on 8.3.x 
    Issue #2862494 by klausi, GoZ, Jo Fitzgerald: Convert web tests to...

Status: Fixed » Closed (fixed)

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