PHPUnit 10 behaves differently when invoked outside web root

FWIW if you run tests via PhpStorm you're not affected by this as it use absolute file paths.

I don't understand what has changed here, the chdir() was added a while back in 🐛 InfoParser returns an empty array if passed a non-existing file Fixed so why is PHPUnit 9 behaviour different?

Guessing PHPUnit 10 handles bootstrap differently, maybe it loads the file earlier now.

FWIW the docs for bootstrap say:

For common use cases, this script should not do more than register an autoloader so that PHP can find the tested units of code.

If I get a chance I'll try track down the change in PHPUnit.

Missing build tests - but they might mess with the working directory even more.

Re: #5

In PHPUnit 9 I put a breakpoint in \PHPUnit\TextUI\Command::handleArguments. Stepping throw I can see that $this->arguments['test'] = realpath($arguments->argument()); converts the relative path to absolute much before $this->handleBootstrap() is called in the same method.

In PHPUnit 10 I put a breakpoint in core/tests/bootstrap.php and in \PHPUnit\TextUI\Configuration\TestSuiteBuilder::build. The bootstrap file is loaded first, invoked from \PHPUnit\TextUI\Application::run, then TestSuiteBuilder::build calls realpath($cliArgument) which fails.

Re: #7

Build tests are currently passing and seem to have their own logic for how to find Drupal. They also weren't touched in 🐛 InfoParser returns an empty array if passed a non-existing file Fixed and don't have an existing $root property the others have. I'd be inclined to leave them out.

I was able to reproduce the problem running core/contrib tests with Drupal 11.x and PHPUnit 10.

I've tested this MR with PHPUnit 10 and can confirm that it resolves the issue with the webroot. Tests execute as expected, and pass. Marking this as RTBC!

I was able to install and test the following contrib modules against the patch, using core 11.0.x-dev:

• Migrate Plus
• Migrate Tools
• dynamic_entity_reference
• svg_image_field
• Advanced email validation

All install and test results are available on the MR comments. No more testing to be done from my end.

I'm having similar problems just running tests from the core directory with ../vendor/bin/phpunit tests/whatever have to use core/tests/whatever instead which breaks terminal autocomplete. This is working well so I think we should go ahead here. If phpunit hadn't intentionally deprecated so many things we've been relying on in phpunit 10 would have suggested a bit more investigation and an upstream bug report, but not sure what that would get us here.

Committed/pushed to 11.x and cherry-picked to 11.0.x, thanks!

Nope this broke HEAD, might be new tests that need an update. https://git.drupalcode.org/project/drupal/-/pipelines/179231

I see three failures.

1. Core test job is failing on test RecipeQuickStartTest @ docroot/web/core/tests/Drupal/Tests/Core/Recipe/RecipeQuickStartTest.php.

Curious that locally I see it getting skipped instead of attempted and failed.

2. Build job is failing with

    Drupal\BuildTests\Command\GenerateThemeTest::testContribStarterkitDevSnapshotWithGitNotInstalled
    Fake git used by process.
    Failed asserting that 0 matches expected 127.
    
    /builds/project/drupal/core/tests/Drupal/BuildTests/Command/GenerateThemeTest.php:291

3. As for nightwatch it is reporting it is failing to install the database during one of the tests:

Failed to connect to database. The database engine reports the following message: <em class="placeholder">SQLSTATE[HY000]: General error: 5 database is locked</em>.<ul><li>Does the database file exist?</li><li>Does web server have permission to write to the database file?</li>Does the web server have permission to write to the directory the database file should be created in?

The command is

│       Command failed: php ./scripts/test-site.php install  --install-profile "nightwatch_testing"  --base-url http://localhost/subdirectory --db-url sqlite://localhost//builds/project/drupal/sites/default/files/db.sqlite?module=sqlite --json      

Under the [Tests/Toolbar Api Test] Test Suite

Running the core unit test with--debugshows PHPUnit decides to bounce on the test while doing the setup

- Drupal\Tests\Core\Recipe\RecipeQuickStartTest::setUp
Test Prepared (Drupal\Tests\Core\Recipe\RecipeQuickStartTest::testQuickStartRecipeCommand)
Test Skipped (Drupal\Tests\Core\Recipe\RecipeQuickStartTest::testQuickStartRecipeCommand)
After Test Method Called (Drupal\Tests\Core\Recipe\RecipeQuickStartTest::tearDown)
After Test Method Finished:

I'm none the wiser as to the exact root cause so I'll be debugging it. I am on the same version of PHPUnit reported on the build log:

PHPUnit Started (PHPUnit 10.5.20 using PHP 8.3.0 (cli) on Linux)

The nightwatch one is a random fail, SQLite doesn't work perfectly under the sorts of loads we put it under in the test suite, "database is locked" is a symptom of this and the test will likely pass on the next run.

@longwave, thanks for the context.

For the SQLite unit test, I am observing that it is path related.

Changing

    $install_command = [
      $this->php,
      'core/scripts/drupal',

to web/core/scripts/drupal does allow the install step to resume (as opposed to causing it to skip right then and there).

Once the install proceeds Drupal site is installed but fails in configuration with error Error: Class "SimpleXMLElement" not found in . That last one might or might not be environmentally related. Fun XD

First attempt ever at submitting a patch to core. I don't have a fork to which issue MRs from. Please take it easy on me :)

@Alexander Allen thanks for investigating this and creating the MR.

In future, instead of creating a new MR, please rebase the existing MR and commit to that, so we can see what has changed. I've generated an interdiff below for committers to see.

I'm not sure we need the additional assertion but I can see how it helps. But you're creating a new local variable but then not using it in $install_command. Also don't need {} around $script.

$ interdiff 7952.diff 8153.diff 
only in patch2:
unchanged:
--- a/core/tests/Drupal/Tests/Core/Recipe/RecipeQuickStartTest.php
+++ b/core/tests/Drupal/Tests/Core/Recipe/RecipeQuickStartTest.php
@@ -92,14 +92,17 @@ public function testQuickStartRecipeCommand(): void {
     // Install a site using the standard recipe to ensure the one time login
     // link generation works.
 
+    $script = $this->root . '/core/scripts/drupal';
     $install_command = [
       $this->php,
-      'core/scripts/drupal',
+      $this->root . '/core/scripts/drupal',
       'quick-start',
       'core/recipes/standard',
       "--site-name='Test site {$this->testDb->getDatabasePrefix()}'",
       '--suppress-login',
     ];
+    $this->assertFileExists($script, "Install script is found in {$script}");
+
     $process = new Process($install_command, NULL, ['DRUPAL_DEV_SITE_PATH' => $this->testDb->getTestSitePath()]);
     $process->setTimeout(500);
     $process->start();
@@ -123,7 +126,7 @@ public function testQuickStartRecipeCommand(): void {
 
     // Generate a cookie so we can make a request against the installed site.
     define('DRUPAL_TEST_IN_CHILD_SITE', FALSE);
-    chmod($this->testDb->getTestSitePath(), 0755);
+    chmod($this->root . '/' . $this->testDb->getTestSitePath(), 0755);
     $cookieJar = CookieJar::fromArray([
       'SIMPLETEST_USER_AGENT' => drupal_generate_test_ua($this->testDb->getDatabasePrefix()),
     ], '127.0.0.1');

I noticed the unused variable, and caught it when reviewing the MR. Then I overwrote the fix when changing the MR from 11.0.x to 11.x, didn't notice it that time. Thanks for pointing it out.

Wasn't sure about pushing to existing MR branch, thanks for the guidance.

There are two MRs here both on 11.x and with different diff stats. Which one should be reviewed? The issue summary should have details on the difference.

mstrelan → changed the visibility of the branch 3445847-phpunit-10-chdir-retry to hidden.

mstrelan → changed the visibility of the branch 3445847-phpunit-10-chdir-retry to active.

mstrelan → changed the visibility of the branch 3445847-phpunit-10-chdir to hidden.

@quietone I've hidden the original branch that was previously committed. The remaining MR is for review. As mentioned in an earlier comment we should have continued on with the one MR, but Alexander Allen was not aware of this. Please review MR 8153.

Removing issue summary tag as there is only 1 MR up now.

I reviewed the changes between the two MRs; basically, there was an additional test base class added for recipe quickstart things between the original MR and the commit. Everything else is the same.

Given the obvious necessity and that this was already committed once aside from the above, I'm also comfortable committing it despite RTBCing it myself.

Saving credits.

Committed to 11.x and cherry-picked to 11.0.x. Thanks everyone!

Noticed that writing test.bbb to the filesystem was causing issues here so created a follow up 📌 FileSaveUploadTest should not write to the app root Needs review

xjm → closed merge request !7952

xjm → closed merge request !8153

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