This from cilefen

https://www.drupal.org/node/2205673#comment-12077914

git grep -e '@deprecated.*before Drupal 8'
core/modules/system/src/Tests/Entity/EntityUnitTestBase.php: * @deprecated in Drupal 8.1.x, will be removed before Drupal 8.2.x. Use
core/tests/Drupal/FunctionalJavascriptTests/JavascriptTestBase.php: * @deprecated in Drupal 8.1.x, will be removed before Drupal 8.3.x. Use
core/tests/Drupal/FunctionalJavascriptTests/JavascriptTestBase.php: * @deprecated in Drupal 8.1.x, will be removed before Drupal 8.3.x. Use
core/tests/Drupal/KernelTests/KernelTestBase.php: * @deprecated in Drupal 8.0.x, will be removed before Drupal 8.2.0.

My conclusion we have stuff to delete in 8.4.x

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

martin107 created an issue. See original summary.

martin107’s picture

Status: Active » Needs review
FileSize
10.49 KB

the patch

andypost’s picture

Status: Needs review » Reviewed & tested by the community

That's strange that code removed begore 9.x but as IS said

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

We should update these deprecation notices. We agreed that we wouldn't do this. We have the chance of breaking tests if we do this. And that's wrong.

Mile23’s picture

Issue tags: +@deprecated

So mark them for removal before 8.5.x or before 9.0.0? Guessing 9.0.0. :-)

Also, they need @trigger_error() as per https://www.drupal.org/core/deprecation

gaurav.kapoor’s picture

Status: Needs work » Needs review
FileSize
2.5 KB

Updated deprecation notices accordingly.Should @trigger_error be added to every method or only before the class declaration.

Mile23’s picture

Status: Needs review » Needs work

Based on https://www.drupal.org/core/deprecation, we can figure out that EntityUnitTestBase should @trigger_error() right after the namespace declaration.

The methods of JavascriptTestBase should get @trigger_error() at the end of execution in the deprecated methods.

And Drupal\KernelTests\KernelTestBase::__get() already triggers errors and throws exceptions for its special cases, because it's trying to tell you not to access those properties. So don't add another one.

We might well see some failed tests based on these changes.

c.nish2k3’s picture

Updated patch with comments from #7

c.nish2k3’s picture

Status: Needs work » Needs review
dawehner’s picture

+++ b/core/modules/system/src/Tests/Entity/EntityUnitTestBase.php
@@ -2,6 +2,10 @@
+@trigger_error(__FILE__ . ' is deprecated in Drupal 8.1.0 and will be removed ¶
+before Drupal 9.0.0. Use \Drupal\KernelTests\Core\Entity\EntityKernelTestBase ¶
+instead.', E_USER_DEPRECATED);

+++ b/core/tests/Drupal/FunctionalJavascriptTests/JavascriptTestBase.php
@@ -71,11 +71,13 @@ protected function tearDown() {
+    @trigger_error('The ' . __METHOD__ . ' method is deprecated since version ¶
+    8.1.0 and will be removed in 9.0.0. Use \Behat\Mink\Element\NodeElement::isVisible() instead.', E_USER_DEPRECATED);

@@ -86,11 +88,13 @@ protected function assertElementVisible($css_selector, $message = '') {
+    @trigger_error('The ' . __METHOD__ . ' method is deprecated since version ¶
+    8.1.0 and will be removed in 9.0.0. Use \Behat\Mink\Element\NodeElement::isVisible() instead.', E_USER_DEPRECATED);

We don't need newline for PHP code. Do you mind moving it into one line?

c.nish2k3’s picture

Updated patch. Moved them to one line.

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.

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.

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

The problem from #10 was fixed in #11.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I checked for change records for these changes - they don't exist. Since they are so old I don't think adding them now is that useful however have deprecation notices that correctly state that these things will be removed in Drupal 9 is.

Committed and pushed 2defaa4e44 to 8.6.x and c5df1de53b to 8.5.x. Thanks!

Since this is tests only backported to 8.5.x

Gave review credit to @dawehner and @Mile23

  • alexpott committed 2defaa4 on 8.6.x
    Issue #2877030 by c.nish2k3, martin107, gaurav.kapoor, Mile23, dawehner...

  • alexpott committed c5df1de on 8.5.x
    Issue #2877030 by c.nish2k3, martin107, gaurav.kapoor, Mile23, dawehner...

Status: Fixed » Closed (fixed)

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