browser_output is empty when testing with Drupal 11

In PHPUnit 9 (which is used in Drupal 10) we have the --printer option but this is removed in PHPUnit 10 (whihc is used in Drupal 11)

Change record https://www.drupal.org/node/3453468 →

📌 Prepare for PHPUnit 10 Fixed is where the --printer option was conditionally added.
Maybe somehow the phpunit.xml config is not being loaded? It should be done by default without an explicit arghument?

I think we need to add a config file -c argument. If a project has it's own then that may already be loaded by default (not sure, investigation required). But most projects do not have their own phpunit.xml so we should use -c ./core

Maybe somehow the phpunit.xml config is not being loaded?

It's certainly not loaded.

We'd need to test this with _PHPUNIT_CONCURRENT set to 0 and 1.

When it's 0, we do this

        PHPUNIT_OPTIONS=''
        if [ "$PHPUNIT_VERSION" == "9" ]; then
          PHPUNIT_OPTIONS="--printer=\Drupal\Tests\Listeners\HtmlOutputPrinter"
        fi

So I suggest checking after that that _PHPUNIT_EXTRA does not have -c , and if it doesn't, we can add to PHPUNIT_OPTIONS -c $CI_PROJECT_DIR/$_WEB_ROOT/core

I already pushed an initial commit, but you are right that we need not double up the -c if it has already been added into _PHPUNIT_EXTRA.

Also I think that if a project has a phpunit.xml then this is used without the need for -c but I will verify that.
If that is the case, then an alternative to setting the config option might be to curl the core file if the project has none (like we do for other configs)

Agree on the previous. We need to test what happens if the project has one. If it doesn't, we don't need to curl it because the file is already in there (inside the core files) so we can just reference it or even copy to the module's folder if needed, but referencing the core one might be enough.

This is not a problem with _PHPUNIT_CONCURRENT=1 and run-tests.sh, as we do get the output artifacts and the html files are included - see this test from today
https://git.drupalcode.org/project/scheduler/-/jobs/3057168/artifacts/br...

But I confirm that it is a problem when _PHPUNIT_CONCURRENT=0, that is, when running the native phpunit - the browser_output folder is empty
https://git.drupalcode.org/project/scheduler/-/jobs/3059092/artifacts/br...

Using the MR272 we get html files in browser_output
https://git.drupalcode.org/project/scheduler/-/jobs/3059909/artifacts/br...

So that shows the basic idea is OK. Now we need to check what happens if the project has their own phpunit.xml file.

Is there at least one contributed project that ships with its own phpinit.xml file?

I can create one temporarily in the Scheduler MR, unless you know of a contrib module that has one? I do not expect it is common, but there may be.

I copied the core phpunit.xml to Scheduler's root directory, and I'm confiddent that it does get used by default without any -c </cocde> parameter. Not only because the log shows <code>Configuration: /builds/project/scheduler/phpunit.xml but also I set parameter name="verbose" value="false" and this has prevented the log listing every output .html file.

Here is the job log and the simpletest/browser_output artifacts.

I've added a check to make sure -c is not already specified in _PHPUNIT_EXTRA. It is coded out in a long-hand way for now, while testing. There should be an opposite of =~ meaning "does not match with" but I do not know that operator. It is not !~ or !=~ . Maybe there is another simple way to negate the condition?

This is ready for review. Here are my test pipelines:

1. The project has a phpunit.xml file and the varianble _phpunit_extra contains -c

https://git.drupalcode.org/project/scheduler/-/jobs/3065841#L383

2. Project has phpunit.xml, but no config specified in _phpunit_extra

The main phpunit jobs uses the projects own phpunit.xml because we see Configuration: /builds/project/scheduler/phpunit.xml
https://git.drupalcode.org/project/scheduler/-/jobs/3065978

The 'test-only' job has actually reverted (removed) the project's phpunit.xml [it should not do this, but that error is for another issue]. We see that as there is now no phpunit.xml the new change adds -c /builds/project/scheduler/web/core to the call, as required.

3. project does not have phpunit.xml

both jobs add -c /builds/project/scheduler/web/core
https://git.drupalcode.org/project/scheduler/-/jobs/3066184#L383
https://git.drupalcode.org/project/scheduler/-/jobs/3066185#L418

Question: Do we also need to check for a project's own phpunit.xml.dist file? I don't know whether some projects have that file instead? I think probably not, as the .dist is the file distributed ready for customisation by a 3rd-party, and that is unlikely for tetsing a contrib module.

Re modules having phpunit.xml.dist, there are a few: https://git.drupalcode.org/search?group_id=2&nav_source=navbar&scope=blo...

But I agree that it might just be a distributed, rather than one use for testing.

Based on your thorough tests at #15 and after the code review, I'm setting this to RTBC.

Thanks, yes 390 projects have a phpunit.xml.dist file. I'd like to check what happens when a project has one, but not a phpunit.xml and see if that .dist file gets automatically detected and used. If it does, then we should not add -c core in those cases, as it would change the existing behaviors. Will be easy to verify.

That was worth checking. If the project does have a phpunit.xml.dist then it is automatically used as the config. We see Configuration: /builds/project/scheduler/phpunit.xml.dist in the log
https://git.drupalcode.org/project/scheduler/-/jobs/3069422#L396

So this also needs to be checked for, to prevent the core config being added.

Thanks so much for checking!

Test with projects own phpunit.xml.dist - this MR does not add the -c argument.
https://git.drupalcode.org/project/scheduler/-/jobs/3069853

Test with neither phpunit.xml nor phpunit.xml.dist, works as requred, the -c argument is added.
https://git.drupalcode.org/project/scheduler/-/jobs/3069920

I will look at the question about not needing the PHPUNIT 9 option. But if I don't get that answer before you want to merge, then go ahead. It can be done in a follow-up.

Here's a test with a custom (copied) script section for PHPunit Previous Major with the 'if phphunit_version=9' removed, so there is no --printer=\Drupal\Tests\Listeners\HtmlOutputPrinter added. It appears to work, and we do get .html files saved in the browser_output folder. We do not get the confirmation line Configuration: /builds/project... etc probably because that output was not in PHPUnit 9. But it must be working as we get the saved artifacts:
https://git.drupalcode.org/project/scheduler/-/jobs/3070086

Therefore I will remove those two conditional blocks (tomorrow).

probably because that output was not in PHPUnit 9

In PHPUnit 9 that line is printed only when --verbose option is provided.

Great findings and tests! It's great, if we don't need the option, we can clean up the code a bit. I'm glad that "core" ported the necessary changes to the configuration file.

Setting to NW just based on that last change.

Made the changes and re-tested. The 'previous major' has .html output and we can see there is no --printer argument in the "executing" log line
https://git.drupalcode.org/project/scheduler/-/jobs/3079151#L389

Ready for review.

Thanks for including that change in the MR and the additional testing.
Downstream pipelines: https://git.drupalcode.org/issue/gitlab_templates-3480767/-/pipelines/31...

The code looks good and the additional "print" statements are useful, so let's leave them there too.
RTBC.

By the way, when you run the downstream pipelines the decoupled_pages always ends with an amber warning, but I have fixed that phpstan error in 📌 Fix PHPstan warning \Drupal calls should be avoided in classes Active . If that was committed we'd have three green downstream pipelines.

I'll review the "decoupled_pages" one. I didn't know it was there. If it's a quick one I might be ok merging it myself.

For now, I've merged this one. Thanks!

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