Comments

klausi created an issue. See original summary.

klausi’s picture

Patch.

dawehner’s picture

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

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

Status: Needs work » Needs review

Patch apply manually and tests success in local. Relaunch tests

GoZ’s picture

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

wrong tag for seville

alexpott’s picture

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

Status: Needs work » Needs review
FileSize
7.78 KB
900 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.

Jo Fitzgerald’s picture

Status: Needs work » Needs review

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

klausi’s picture

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.

Jo Fitzgerald’s picture

Status: Needs work » Needs review
FileSize
7.11 KB
988 bytes

Moved @trigger_error().

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

klausi’s picture

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

Looks good!

dawehner’s picture

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

alexpott’s picture

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.