xdebug being enabled causes tests to fail without clear indication that it is the problem

My input:

1. 👎 because this makes the test behave differently from real world scenarios.
2. 🤔 maybe because it's easy to miss the fact that a test is marked as risky.
3. 👍 this is very much preferable. In the error message, we should inform the user of how to bypass this temporarily, which should only happen when debugging a test. This must NOT involve an environment variable, because of the risks already explained in the issue summary. Valid work-arounds are:
   • "add early return to \Drupal\package_manager\Validator\XdebugValidator::checkForXdebug"
   • "comment out the package_manager.validator.xdebug and automatic_updates.validator.xdebug services"

IOW: I disagree strongly with #2, because it modifies application state in functional aka integration tests, which is precisely where we're testing how the entire application works.

P.S.: why do we have both \Drupal\automatic_updates\Validator\XdebugValidator and \Drupal\automatic_updates\Validator\XdebugValidator? Either we should add the missing docs for that here, alternatively we need a follow-up issue for that.

What you wrote in #5 is documentation that's missing from \Drupal\package_manager\Validator\XdebugValidator.

This blocked @kunal.sachdev for hours 😬

This just cost us 30 minutes of a pairing session to figure out the root cause for! 😭 (Issue: #3312619: Ensure all validation results use translatable strings except when created from throwables.)

This very much impedes productivity. This needs to be solved ASAP.

Before starting to work on this, according to the proposed solution if we fail the test if xdebug is on, won't it be a problem for debugging?

Discussed this with @wim on a call today so, we are not facing this problem anymore, we get clear message that the error in the test is caused by XDebug being on. So, this issue can be closed as outdated, @omakr.podey and @kunal.sachdev agrees with this!

To expand on #11: I marked this critical because while mentoring @yash.rode, @omkar.podey and @kunal.sachdev in early November — when I just got started on AU — they ran into problems with debugging of tests all the time.

Since then, I've helped them get their environments in order, and they now all use xdebug off and xdebug on commands. For the past ~6 weeks they have not run into problems around this anymore. So … closing 😊

Just discussed in call.

Rescoping this to instead of doing this as a precondition prior to running tests:

1. making \Drupal\automatic_updates\Validator\XdebugValidator aware of whether it runs in a test or not, and NOT complain about xdebug if we're in a test
2. getting rid of all the $this->disableValidators[] = 'package_manager.validator.xdebug'; cases

This implements #14.2

\Drupal\Tests\automatic_updates\Kernel\StatusCheck\StatusCheckerTest::testGetResults() is an example of a kernel test that will fail with xdebug on, \Drupal\Tests\automatic_updates\Functional\StatusCheckTest::testStatusChecksOnStatusReport is an example of a functional test.

Let's find out what other tests will fail due to #15, by pretending xdebug is always enabled… 🤓

+++ b/package_manager/src/Validator/XdebugValidator.php
@@ -40,7 +40,7 @@ class XdebugValidator implements EventSubscriberInterface {
   protected function checkForXdebug(): ?array {
-    if (function_exists('xdebug_break')) {
+    if (TRUE || function_exists('xdebug_break')) {

This makes XdebugValidator correctly detect when it's running in a test, and then it won't do anything.

Adjust the two XdebugValidatorTests.

Making XdebugValidator not fail in build tests was especially tricky :O That actually already didn't work correctly in HEAD either, because #15 touched nothing on the build test side…

… but I never ever want to have to revisit this, so I spent an hour or so trying to find a way that this could reliably be detected. I think I just managed to find a way! 2🤓

Yay! That worked 🥳

Time to stop simulating Xdebug being on at all times now!

Only a couple of minor points which, honestly, don't block commit in my opinion. But, feel free to implement if you agree.

1. +++ b/package_manager/src/Validator/XdebugValidator.php
   @@ -40,6 +41,19 @@ class XdebugValidator implements EventSubscriberInterface {
   +    // Xdebug is allowed to be enabled while running tests, for debugging
   +    // purposes. It's just not allowed to be enabled while using Package Manager
   +    // in a real environment.
   +    // @see \Drupal\Core\CoreServiceProvider::registerTest()
   +    $in_functional_test = drupal_valid_test_ua();
   +    // @see \Drupal\Core\DependencyInjection\DependencySerializationTrait::__wakeup()
   +    $in_kernel_test = isset($GLOBALS['__PHPUNIT_BOOTSTRAP']);
   +    // @see \Drupal\BuildTests\Framework\BuildTestBase::setUp()
   +    $in_build_test = str_contains(__FILE__, DrupalFilesystem::getOsTemporaryDirectory() . '/build_workspace_');
   +    if (($in_functional_test || $in_kernel_test || $in_build_test) && !function_exists('xdebug_break_TESTED')) {
   +      return NULL;
   +    }
   

   I wonder if this should be moved to a private static helper function, so as to keep a cleaner separation between production code and test code.

2. +++ b/package_manager/tests/src/Kernel/XdebugValidatorTest.php
   @@ -21,6 +21,8 @@ class XdebugValidatorTest extends PackageManagerKernelTestBase {
   +      eval('function xdebug_break_TESTED() {}');
   

   We should probably add a @see the relevant part of XdebugValidator, since this otherwise looks kinda inexplicable.

3. +++ b/tests/src/Functional/AutomaticUpdatesFunctionalTestBase.php
   @@ -44,8 +44,6 @@ abstract class AutomaticUpdatesFunctionalTestBase extends BrowserTestBase {
   -    // Always allow tests to run with Xdebug on.
   -    'package_manager.validator.xdebug',
   

   I love that we can finally get away from disabling the validator in tests!

#24:

1. +1! Done.
2. +1! Done.
3. 😊

Self-RTBC'ing per #24.