Undeprecate \Drupal\FunctionalTests\AssertLegacyTrait and \Drupal\KernelTests\AssertLegacyTrait in Drupal 8

This makes perfect sense to me, the move from simpletest to PHPUnit should be easy. Taking \Drupal\FunctionalTests\AssertLegacyTrait out of D9 would make the hurdle much higher for modules still on Simpletest as they would also need to update a huge amount of methods in the test at the same time.

Should we undeprecate or just change the deprecation to Drupal 10 now?

I'm trying to understand the logic behind this.

As a side effect we greatly reduce the noise from static analysis of deprecations in code.

This doesn't seem to be a good logic to me. This just shows that we have to improve our analysis tools.

There is no learning curve involved here. We can have one change record with a table in it to show 'From' to 'To' methods. It will just become find and replace which is already shared in #3027952-2: [Plan] Remove the usage of deprecated methods in tests.

We have added \Drupal\FunctionalTests\AssertLegacyTrait almost two and half year ago. D9 is still more than a year away. I honestly don't get the reason why this is not enough time for people to adapt to the new standards.

Personally, I'm worried that if on day 0 of D9 we'll go in with deprecated stuff then by the time we'll create a D10 branch we'll be getting rid of a lot more of dead code.

The whole idea behind \Drupal\FunctionalTests\AssertLegacyTrait was to make the transition from Simpletest to PHPUnit based tests easier, I think AssertLegacyTrait has value until that transition is fully done. The first moment we know that this transition is done, is going to be when modules run on a system without Simpletest (hopefully D9).

Imagine the horror show the conversions would have been if we needed to update all these methods at the same time we did the conversions. If we take this out before D9, that is exactly what we would be asking from contrib that hasn't moved over yet.

I think the burden of maintaining this in D9 is lower then the relief it provides for modules having to do the conversions.

I think the burden of maintaining this in D9 is lower then the relief it provides for modules having to do the conversions.

That is my assessment as well.

Should we undeprecate or just change the deprecation to Drupal 10 now?

Ideally we can change it to deprecate until Drupal 10 and have our tools be able to deal with ignoring those deprecations or report them differently.

That seems to be a good tradeoff. The benefits from moving to phpunit (better assertions etc.) come for free, whether you use the legacy trait or not. The different assertions in phpunit doesn't do. Given that it seems fine to not deprecate them.

+++ b/core/tests/Drupal/FunctionalTests/AssertLegacyTrait.php
@@ -1,5 +1,5 @@
-
+// @codingStandardsIgnoreFile

Is it possible to ignore just one rule in this file?

+1

As @alexpott said, we can't change how PHPStorm and other tools displays these and a lot of our tests currently look like a mess with basically every other line striked through. That really makes working with these tests harder because you don't know how much you should clean up while making some small changes.

Doing them method-by-method with some automated tools then in 9.x sounds like a good plan. We have tons of more important things to do still until 9.0. That said, a downside is that it will be easy to use the wrong method when writing new tests when they are not marked as deprecated.

The "Deprecated Scheduled for removal" stuff reads odd but I can see you want to keep the greppability. I think this sounds like a fine compromise. Newly added usage of the "deprecated" methods is IMHO not that bad since it will appear as soon as we actually re-deprecate.

The point about @deprecation tooling in places like phpstorm shows us we need to be careful when we start deprecate for Drupal 10.

Unrelated random fail. Sent for retest.

OK this seems like a decent balance between premature deprecation and not telling anyone that it's going to go.

Committed/pushed to 8.7.x, thanks!