Fix Drupal\FunctionalTests\AssertLegacyTrait inconsistent deprecation messages

+++ b/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php
@@ -696,13 +696,13 @@ protected function assertPattern($pattern) {
-   * Deprecated Scheduled for removal in Drupal 10.0.0.
-   *   Use $this->assertSession()->responseNotMatches() instead.
+   * @deprecated in drupal:8.4.0 and is removed from drupal:10.0.0. Use
+   *   $this->assertSession()->responseNotMatches() instead.

These were explicitly not using @deprecated so our tooling (which currently cannot distinguish between deprecations for Drupal 9 and 10) will not lump them together. Unfortunately the best possible way now is to open an issue against Drupal 9.1 for these parts, see https://groups.drupal.org/node/535670 The trigger_error() sections should still be fixed here (also the "Deprecated ..." text without the @ should IMHO still be updated to include the source deprecation version, so its easier to fix in Drupal 9.1).

Looks like the issue should just change title - Properly deprecate Drupal\FunctionalTests\AssertLegacyTrait methods?!

@andypost: if we are properly deprecating then it would go to 9.1 as well as per https://groups.drupal.org/node/535670 but looks like there is some cleanup to be done here first in the existing codebase? Should we remove the trigger_error()s too as per that policy?

So either we need to do this, or remove all the deprecations in this issue and open a 9.1.x only issue to properly deprecate for Drupal 10 again. The left-around trigger_error()s are somewhat concerning for me but I am not a release manager.

I like the patch in #8 here, this is all test code and not a new deprecation by any means, undeprecating would be mis-informing people and potentially lead to more usages.

@catch: do you think keeping the incosistency of "Deprecated" (vs. @deprecated) while still calling trigger_error() is the way to go? People trying to use this will practically not pass their tests with unsuppressed deprecations even though the deprecation is for Drupal 10. That was the goal of https://groups.drupal.org/node/535670 AFAIK to avoid "introducing" Drupal 10 deprecations in Drupal 9.

Ohhh I kind of skipped past that. I would probably switch to @deprecated too, but would be good to get a second opinion from @xjm.

For me https://groups.drupal.org/node/535670 is really about not introducing actual new deprecations in Drupal 9, I don't think it means we should go backwards when informing people about already-existing technical debt they've had the potential to clean up for two+ years already.

In that case @mondrake' patch from #3 should be fine. Re-uploading for clarity. I did not contribute to that patch :)

Meh, sorry. Not sure what went wrong there. Re-downloaded and re-uploading.

RTBC++ to #14 let's improve filters on deprecations to filter out 10.x ones

@andypost: I looked at the remaining @deprecated and none of them seem to be for Drupal 10 yet, so this would be a first. That said, at least the counting we can update to filter them for the graph.

Just had a somewhat unrelated discussion with @andypost and realized this issue is only partially covering the deprecated APIs. If you

$ grep -r "Drupal 10" core

core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php: * Deprecated Scheduled for removal in Drupal 10.0.0. Use the methods on
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php: *   will be removed in Drupal 10.0.0. Adding the @ back should re-enable coding
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0.
core/tests/Drupal/KernelTests/AssertLegacyTrait.php: * Deprecated Scheduled for removal in Drupal 10.0.0. Use PHPUnit's native
core/tests/Drupal/KernelTests/AssertLegacyTrait.php: *   will be removed in Drupal 10.0.0.
core/tests/Drupal/KernelTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0. Use self::assertTrue()
core/tests/Drupal/KernelTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0. Use self::assertEquals()
core/tests/Drupal/KernelTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0. Use
core/tests/Drupal/KernelTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0. Use self::assertSame()
core/tests/Drupal/KernelTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0. Use
core/tests/Drupal/KernelTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0. Use self::assertEquals()
core/tests/Drupal/KernelTests/AssertLegacyTrait.php:   * Deprecated Scheduled for removal in Drupal 10.0.0. Use self::assertTrue()
core/modules/rest/tests/src/Functional/BcTimestampNormalizerUnixTestTrait.php:@trigger_error(__NAMESPACE__ . '\BcTimestampNormalizerUnixTestTrait is deprecated in Drupal 9.0.0 and will be removed before Drupal 10.0.0. Instead of BcTimestampNormalizerUnixTestTrait::formatExpectedTimestampItemValues(123456789), use (new \DateTime())->setTimestamp(123456789)->setTimezone(new \DateTimeZone("UTC"))->format(\DateTime::RFC3339), see https://www.drupal.org/node/2859657.', E_USER_DEPRECATED);

So this marks some of those not using a @deprecated to use an @deprecated in FunctionalTests/AssertLegacyTrait in the name of consistency with their trigger_error()s but not all of them would then get @deprecated instead of "Deprecated".

The file has this towards the top which contradicts @catch's #11:

* @todo https://www.drupal.org/project/drupal/issues/3031580 Note that
 *   deprecations in this file do not use the @ symbol in Drupal 8 because this
 *   will be removed in Drupal 10.0.0. Adding the @ back should re-enable coding
 *   stanadrds checks.

If we are to add back @deprecated in Drupal 8 then we should do it for all the instances in the file (and likely the other trait that has the same). If we are not, then we should not convert to @deprecated while we fix the format.

Marking a beta requirement because this is cleaning up deprecation messages for code that would otherwise need to be removed at least if we would be following the letter of the strings in trigger_error().

+++ b/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php
@@ -696,13 +696,13 @@ protected function assertPattern($pattern) {
-    @trigger_error('assertNoPattern() is deprecated and scheduled for removal in Drupal 9.0.0. Use $this->assertSession()->responseNotMatches($pattern) instead. See https://www.drupal.org/node/2864262.', E_USER_DEPRECATED);
+    @trigger_error('AssertLegacyTrait::assertNoPattern() is deprecated in drupal:8.4.0 and is removed from drupal:10.0.0. Use $this->assertSession()->responseHeaderNotContains() instead. See https://www.drupal.org/node/2864262', E_USER_DEPRECATED);

Copy/paste error I think, responseNotMatches and responseHeaderNotContains are not the same.

Re #11 and the @todo comment, I thought the point of using improper deprecation markers here was to avoid overwhelming contrib/custom developers with too many deprecations. While they are getting their code ready for Drupal 9, they don't actually need to think about these deprecations for Drupal 10 yet.

Therefore I think the correct solution to this is commit #8 plus #17/19 now and then consider committing #3 to either 9.0.x or 9.1.x?

Working on this now. Requires a lot of git archeology to figure out version numbers.

Ok reviewed every single deprecation in Drupal\FunctionalTests\AssertLegacyTrait in this pass and updated their deprecation text based on their respective versions. Here is a list of which method was added where and which minor version:

8.2.0 #2735045: Convert StandardTest to BrowserTestBase by introducing a AssertLegacyTrait and new assertions on the WebAssert was the original issue.

assertElementPresent
assertElementNotPresent
assertText
assertNoText
assertResponse
assertFieldByName
assertRaw
assertTitle
assertLink

8.2.0 #2759879: Additional assertions for WebAssert and AssertLegacyTrait, part 2

assertUniqueText
assertNoUniqueText
assertNoFieldByName
assertOptionSelected
assertFieldChecked
assertNoFieldChecked
assertPattern

8.2.0 #2759879: Additional assertions for WebAssert and AssertLegacyTrait, part 2

assertFieldById
assertOption
assertNoOption
assertUrl

8.2.0 #2750941: Additional BC assertions from WebTestBase to BrowserTestBase

assertField
assertNoField
assertNoRaw
assertNoLink
assertLinkByHref
assertNoLinkByHref
assertNoFieldById
assertUrl
assertEscaped
assertCacheTag
assertSession
buildXPathQuery

8.4.0 #2862470: Add assertOptionByText() to AssertLegacyTrait for better browser test compatibility

assertOptionByText

8.3.0 #2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests

assertFieldByXPath
assertNoFieldByXPath
assertFieldsByValue

8.2.0 #2735199: Convert web tests to browser tests for help module

assertNoEscaped

8.4.0 #2864257: Convert web tests AssertNoPattern to Browser Test

assertNoPattern

8.3.0 #2795611: Provide a assertHeader legacy assertion

assertHeader

8.4.0 #2795111: WTB to BTB, add getRawContent() in BTB

getRawContent

8.4.0 #2795085: Add assertNoCacheTag to assertLegacyTrait

assertNoCacheTag

8.5.0 #2868019: AssertLegacyTrait field assertions not compatible with Simpletest assertions

constructFieldXPath

8.5.0 #2907485: Add getAllOptions() to AssertLegacyTrait

getAllOptions

This will make it trivial to turn these into actual deprecation messages in Drupal 9 with a search and replace for "Deprecated in" to "@deprecated in".

Forgot to mention that #22 also resolved #19 from @longwave :) All of the items from #17 that are in scope for this issue (in functional tests rather than kernel tests) are also resolved to the best of my knowledge. So this should be possible to commit to 8.9.x and 9.0.x. Then #3 is not a valid patch to commit to 9.x anymore because it only turns some of the "Deprecated in" to "@deprecated in". So I opened #3114617: Properly deprecate methods in Drupal\FunctionalTests\AssertLegacyTrait and Drupal\KernelTests\AssertLegacyTrait, while still keeping deprecations silenced for that.

I verified what the @todo at the top of the file was and it was in fact for the closed issue that unmarked the deprecation. That is not a @todo anymore. The @todo is to make it a @deprecated again. So I changed that link to #3114617: Properly deprecate methods in Drupal\FunctionalTests\AssertLegacyTrait and Drupal\KernelTests\AssertLegacyTrait, while still keeping deprecations silenced now.

This should be IMHO complete for this issue and then we can move into #3114617: Properly deprecate methods in Drupal\FunctionalTests\AssertLegacyTrait and Drupal\KernelTests\AssertLegacyTrait, while still keeping deprecations silenced for Drupal 9.0 or 9.1 (that one NOT backported to 8.9.x).

+++ b/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php
@@ -12,13 +12,13 @@
  *   stanadrds checks.

Typo in "standards" that we could fix while we are here?

Fixing the two fails from #23 which were both due to fixes in the deprecation message not fixed in the test. (There was a third fail in #22 that is a random media fail).

Also fixing #25.

Opened #3114636: Properly deprecate Drupal\KernelTests\AssertLegacyTrait methods for the sibling file in KernelTests. That does not have any @deprecated or trigger_error() so not a beta requirement, but would be nice to fix alongside this nonetheless for consistency.

Looking good.

Committed and pushed 5e39512612 to 9.0.x and 9df1ea9b76 to 8.9.x and b4f0fc65e6 to 8.8.x. Thanks!

Backported to 8.8.x as this is a test-only and mostly docs change.

I think #23 resolves the release manager concerns that resulted in the tag in #8.

If we need to turn to active deprecations as debated above with @catch, we can still backport #3114617: Properly deprecate methods in Drupal\FunctionalTests\AssertLegacyTrait and Drupal\KernelTests\AssertLegacyTrait, while still keeping deprecations silenced when done :) This is a good interim step that can keep us moving.