Add Code Coverage reporting

We are actually planning to port as much as possible all the new GitLab CI features that were added to Drupal core and this is one of them.
I will close this issue and open the one to track the newer and improved version.

New issue #3389921: New templates for contrib: mirror as much as possible all the GitlabCI jobs from core →

The Drupal Core template currently does not have Code Coverage enabled, re-opening the issue.

I also don't want to see #3389921: New templates for contrib: mirror as much as possible all the GitlabCI jobs from core → possibly cause contrib to require large re-writes to the template to continue using code coverage (I tested with our current _PHPUNIT_CONCURRENT setting and it breaks.)

We do have a long standing feature request in core ✨ Allow run-tests.sh to generate coverage reports Needs work , maybe that is where this work actually needs to be done.

Regarding backward compatibility, we will most likely add it as a brand-new file, so people can choose to stay in the "old" version (by doing nothing) or the "new" version (by updating the reference to the file).

Note that the _PHPUNIT_CONCURRENT is still set as experimental. Keeping the default value for it shouldn't break things and it is explained that the options are slightly different if the value changes.

I'm pretty sure that @secretsayan has some insights/suggestions to share … 🧐🤓

PCOV is now installed in PHP8.3 images

We can evaluate what we want to do with this.

Even if we don’t enable coverage testing plugins we can safely add coverage: /^\s*Lines:\s*\d+.\d+\%/ to the global gitlab file as it will not cause test failures (it is used as a trigger to detect percentages when available).

Meanwhile I added xdebug as well but only to :dev 8.3 images - it's twice slower but more precise

@cmlara - so, adding coverage: '/^\s*Lines:\s*\d+.\d+\%/' would be all that's needed? I guess we'd add it to the `phpunit` job. No other changes for reporting or something else?

Example from the recipes initiative: #3442014: Add code coverage report →
https://git.drupalcode.org/project/distributions_recipes/-/commit/2f82a0...

coverage: '/^\s*Lines:\s*\d+.\d+\%/'
Is used to allow reporting of the percentage of code that is covered. Its the simplest level of coverage reporting. It can be used in a Badge and can be displayed on the MR page to give an overall indication of changes. This check is done by looking at the job log output making it always safe to have in the GitLab CI file.

This will usually also be paired with disabling colors to ensure the logs matches the REGEX.

  _PHPUNIT_EXTRA:
    value:  --colors=never

The asset file report is what generates the Line-By-Line highlighting (red/green) in an MR on if the code is covered. However IIRC if the asset file is missing GitLab will error out. This would either need to be optional or that we make sure we always enable code coverage before adding this code to the default template.

   reports:
      coverage_report:
        coverage_format: cobertura
        path: coverage.xml

✨ Allow run-tests.sh to generate coverage reports Needs work will need to figure this out for running in Parallel. I was going to say I had only found found scripts that can combine multiple PHPUnit runs into standard (directory based) coverage reports however I may have just found a script that can convert those to cobertura format which might help us solve the run-tests.sh problem.

Non-parallel runs are much easier

  _PHPUNIT_EXTRA:
    value: --coverage-cobertura $CI_PROJECT_DIR/coverage.xml --coverage-filter $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/ --coverage-text --colors=never

Can actually enable Code Coverage during PHPUnit execution (after the pcov module is enabled). Obviously if we submit this to production for all modules we would follow #10 and place this in the actual PHPUnit run line and not in $_PHPUNIT_EXTRA.

Though as I noted previously this requires that the module NOT be symlinked as PCOV does not work well with the module code being in the root of the $CI_PROJECT_DIR and symlinked from $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/, at least with the coverage filter. This may be possibly just configuration issues on what I designed to date, once I found a working setup (after dozens of diffrent styles) I stuck with it.

Using a PHPUnit.xml file is also another method to set these settings as shown by the example in #10 Nothing wrong with using a phpunit.xml, I only avoided it as it was one more file to write, and I was having issues making pcov work with the coverage restrictions. Some of those were very much likely errors on my part at the time.

I'm going to need to look at #10 closer, one item I see different between is no use of echo -e '[pcov]\npcov.directory=.' > /usr/local/etc/php/conf.d/pcov.ini. In my experience without this PCOV would only show coverage for files in the src/ folder ignoring PHP code in the root folder, and in sub-modules IIRC. The phpunit.xml may be solving that issue.

I created an MR https://git.drupalcode.org/project/gitlab_templates/-/merge_requests/253 with the implementation of the code coverage report.
Here is an example of a successful pipeline job with it: https://git.drupalcode.org/project/test_helpers/-/jobs/2599892

It reports the code coverage results to Gitlab MR interface:

Merged result pipeline #269172 passed
Merged result pipeline passed for 2df47e4b
Test coverage 71.19% from 1 job

Could you please review the MR?

This looks interesting, thanks for starting it.

I can see in your MR107 that the project's repo also contains the new file phpunit-drupal-ci-module.xml which is being added in the gitlab templates MR into assets, and downloaded to the projects top-level folder if it does not exist. But the extra arguments supplied add this as a config file via export _PHPUNIT_COVERAGE_ARGS="-c $CI_PROJECT_DIR/$_PHPUNIT_CONFIG_FILE --coverage-text --colors=never". What happens if the project already has a phpunit config file, but it is named differently, such as the default phpunit.xml?

I will test the MR and see how it goes.

First run, I forgot to add new variable _PHPUNIT_COVERAGE: 1

Second run failed
https://git.drupalcode.org/project/scheduler/-/pipelines/269432
This project does have its own phpunit.xml which is the config used by default. I don't know if that is the cause of the error, it is probably something simple, but I'll let you investiagte.

FYI pcov added to PHP 8.4 image (with patch) https://git.drupalcode.org/project/drupalci_environments/-/commit/6982c0...

Second run failed
https://git.drupalcode.org/project/scheduler/-/pipelines/269432

@jonathan1055, thanks for the testing! It's failed because runs in the concurrent mode, which uses the web/core/scripts/run-tests.sh script. And this script, unfortunately, doesn't support the --coverage-text argument:

executing: sudo SYMFONY_DEPRECATIONS_HELPER='disabled' MINK_DRIVER_ARGS_WEBDRIVER='["chrome", {"browserName":"chrome","goog:chromeOptions":{"args":["--disable-dev-shm-usage","--disable-gpu","--headless"]}}, "http://localhost:9515"]' -u www-data php web/core/scripts/run-tests.sh --color --keep-results --concurrency 32 --repeat '1' --sqlite 'sites/default/files/.sqlite' --dburl mysql://drupaltestbot:drupaltestbotpw@database/mysql --url http://localhost/web --verbose --non-html  -c /builds/project/scheduler/phpunit-drupal-ci-module.xml --coverage-text --colors=never  scheduler_js
...
  ERROR: Unknown argument '--coverage-text'.

I will prepare a patch for Drupal Core to fix this, but for now - try to test with no-parallel mode using _PHPUNIT_CONCURRENT=0 - it runs phpunit directly.

OK if it only runs when _PHPUNIT_CONCURRENT=0 maybe it needs a conditional if _PHPUNIT_CONCURRENT=1 give a message and do not add the coverage options.

I re-ran with _PHPUNIT_CONCURRENT=0 and did get some coverage output. However, the log is huge, > 35,000 lines and it seems to be checking not only this project, but also the other module dependencies loaded for the test, and 3rd-party software (webdriver, twig, symfony, etc)
https://git.drupalcode.org/project/scheduler/-/jobs/2609518

However, the log is huge, > 35,000 lines and it seems to be checking not only this project, but also the other module dependencies loaded for the test, and 3rd-party software (webdriver, twig, symfony, etc)

Hope that fixed this in the last commit, please retry.

Failed earlier this time, with
curl: (3) URL using bad/illegal format or missing URL
https://git.drupalcode.org/project/scheduler/-/jobs/2610500

I think I will leave the testing until you've done some proper test runs yourself, to iron out these things :-)

@jonathan1055, with your project, I found a problem in the MR and fixed it, here is your green pipeline: https://git.drupalcode.org/issue/scheduler-3445052/-/jobs/2612741

It's better to have testing on different projects to catch more bugs, not only on my one project ;)

Seems the coverage reporting is not supported in the parallel (concurrent) mode at all, so I added throwing an error if both modes are enabled:

if [[ "$_PHPUNIT_CONCURRENT" == "1" ]]; then
  echo "Error: The coverage mode is not supported the parallel mode (_PHPUNIT_CONCURRENT=1). Set the _PHPUNIT_CONCURRENT=0 to enable coverage reporting." >&2
  exit 1
fi

Seems all the issues were fixed, so moving back to Needs review.

Thanks so much for working on this. In the provided phpunit.xml file, there is reference to a class that no longer exist in Drupal 11.
Setting to "Needs work" based on that.

Adding some testing cases to the description so we can cover some of the cases.

I also added reporting the per-line code coverage using Cobertura, cuz GitLab doesn't support JUnit - only Cobertura or JaCoCo, see https://docs.gitlab.com/ee/ci/testing/test_coverage_visualization/index....

And it works well - a job: https://git.drupalcode.org/project/test_helpers/-/jobs/2626173 and the screenshot:

Thanks for working on this, murz, but I have reservations about adding this functionality in the form proposed here, for several reasons:

1. It only works when _phpunit_concurrent=0 however our preferred/recommended/default is likely to change to run with concurrency, see #3447087: Switch to using run-tests.sh by default →
2. The coverage details and result are for the entire project codebase so when the testing is split into separate jobs using @group or parallel jobs the result is meaningless because each job only runs a subset of the tests so the actual coverage is much higher than what is reported
3. Additional runtime - for example the Scheduler javascript test from #22 took 7 mins 3 seconds, but the equivalent test using concurrent=0 without coverage only took 3 mins 13 secs. I know this is only a sample of one so more test runs would be needed to prove it, but at first glance is appears significant
4. There may be implications on CPU but I have not got that info yet
5. The coverage is run for every variant phpunit job but this is unnecessary, extra work

I would like to suggest that instead of adding this in to the existing phpunit job we create a new separate job for it, called ‘Test Coverage’ which would be manually triggered thus only run on demand, and only with "current" core, no variants, to save resources. Test coverage does not need to be run in every pipeline as it won’t be changing all the time. The new job would ignore the _phpunit_concurrent variable and always run the phpunit binary directly. It would ignore @group and always run all tests in the project so that a meaningful result was produced. The new job can extend phpunit in a similar way to 'test-only changes'

I also think it would be better to get the phpunit.xml file from the core directory in the job, and not store it in the GitLab templates repo, so that the correct version is used for whatever Core is being tested. I have not looked in detail to see exactly what changes are done to this file. Ideally it would be better to find another way entirely. Maybe get from core if it does not exist, then use sed to modify the file.

I am sure we'll find a good way to create the code coverage reports, as it will be a big benefit to developers and maintainers. I just think we need to take a step back from the current approach.

I like above approach to run ‘Test Coverage’ or 'Code Coverage' on demand.

Some notes after looking at this:

• Recently, a script was introduced to massage the core phpunit.xml.dist, see 🐛 #3480767 Causes break in behavior of tests configuration Active . We can probably extend the logic here.
• I also have reservations regarding performance as mentioned in #27. Plus one for making it an additional manual job.
• We should add documentation for the different settings, limitations regarding concurrency and the coverage configuration.

If you are fine with it, I can continue with some changes in the existing merge request.

I also like the idea of a new on-demand job. Adding that new feature will be easier than modifying this key part of the CI system.

@simonbaese - there was no additional work since September and the issue is not assigned so I'd say feel free to work on the existing MR, or create a new one as needed. I've rebased the "main" branch from the fork, because the other branch (linked to the existing MR) was giving a conflict. Feel free to create a new branch and MR and borrow as much as you need from the existing MR taking into account all the feedback above.