Deprecation tests fail when all PHPUnit tests are run via PHPUnit

Thanks alex. I took a stab at this but either I'm doing something wrong or it is a bit tricky. Fixing one problem causes different failures.

hahahaha see... tricky.

"you only need to match on one expected deprecation for a pass."
Not sure about how the deprecation matching is implemented but are they handled through exceptions? If so the first one will stop execution and you can never have more then one match so matching any expected exception makes sense.

I spent some time banging on this and am basically cross eyed and delirious now so documenting what I've seen.

1) Obviously not exceptions. You're working with the error handler to fix things. derp on my part.
2) That error handling change is the "problem". Removing it "fixes" the tests.
3) "problem" and "fixes" because I don't think they ever tested what we thought. They tested the coverage weirdness because removing the covers annotations on a clean install causes the assertion to fail in both run-test.sh and phpunit
4) There _should_ be a deprecation to test in both of these tests. Neither work and it seems connected to the subprocess phpunit serialization magic which might not be working correctly in Drupal's test suite. Inspecting the serialized value reveals an empty array.

All of this to say, I think this patch might be fine the way it is but our deprecation testing doesn't seem to work 100%. Bonus observation, since unit tests run in their own process on testbot and their testing works, it is likely something kernel tests do specifically that is breaking things.

Thank god you knew where to look because I was going in circles trying to figure out how the interprocess communications wasn't working.

+++ b/core/modules/link/tests/src/Unit/Plugin/migrate/cckfield/LinkCckTest.php
@@ -11,6 +11,8 @@
+ * @arunInSeparateProcess
+ * @apreserveGlobalState disabled

Think this is the only thing keeping it from being RTBC

currently they're not running in a separate process though. there's an "a" in front.

:-D wfm. No real change from previous patch so going to assume it'll pass and preemptively RTBC.

Crediting @neclimdul for the time spent on #7

Committed as 891c39b and pushed to 8.5.x

Thanks folks

+++ b/core/tests/Drupal/Tests/Listeners/DeprecationListenerTrait.php
@@ -21,8 +21,20 @@
+      if (strpos($method, 'testLegacy') === 0
+        || strpos($method, 'provideLegacy') === 0
+        || strpos($method, 'getLegacy') === 0
+        || strpos(get_class($test), '\Legacy')
+        || in_array('legacy', $util_test_class::getGroups(get_class($test), $method), TRUE)) {
+        // This is a legacy test don't skip deprecations.

Shouldn't we have a CR for these? Apparently I can write test methods like testLegacySomethingBlah() and it won't skip deprecations?

Also: we have nothing that actually uses this?

Ahhh! A comment indicating that that's there just to match upstream logic would've helped make that clear: wouldn't have asked then.

Hi all : Is there a ticket for following the upstream changes and adding them later within phpunit?

Edit : I made the above comment before learning about symfony/phpunit-bridge. Please ignore