Problem/Motivation

Follow-up of #3485069: [CI] Spin off Drupal Components tests in a job of their own, add test coverage metrics.

Components tests coverage metrics differ slightly by PHP version, and apparently the last PHP version job completing the components unit test is winning the box. Not a big deal, but this can cause some confusion when looking at test results and seeing that metrics are changing even if components code/tests are not part of the MR.

Example from HEAD on Apr 28, 2025:

PHP 8.3 and 8.4

  Classes: 31.62% (37/117)   
  Methods: 46.88% (315/672)  
  Lines:   36.59% (2782/7604)

PHP 8.5

  Classes: 30.77% (36/117)   
  Methods: 46.43% (312/672)  
  Lines:   36.53% (2778/7604)

Proposed resolution

Solution 1 from the list below.

Possible solutions:

  1. Produce code coverage report only in the job running the default PHP version; no code coverage for the other PHP versions.
  2. Produce a different set of metrics for each PHP version.
  3. Investigate why the differences and try to remove them (might be impossible if there are code paths dependent on PHP version checking)

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#26 3521541-nr-bot_kwkku7rn.txt91 bytesneeds-review-queue-bot

Issue fork drupal-3521541

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mondrake created an issue. See original summary.

mondrake opened merge request !11963

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review

MR implements option 1 from the issue summary. IMHO option 2 is overkill and 3 is complicated and risk regressions easily.

catch’s picture

catch
he/him
Primary language English
commented

Yeah only for the default PHP version sounds good to me too.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Assuming this will have to be tweaked when https://www.drupal.org/project/drupal/issues/3515706 lands?

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review

Why so?

smustgrave’s picture

smustgrave commented

Didn't mean to change status but if 8.4 is a default won't this change need to apply to 8.4

mondrake’s picture

mondrake
Location 🇮🇹
commented

Ah! No it’s already for 8.4, check the MR - 8.3 and 8.5 are getting the “no-coverage” job settings. Anyway this is about components unit tests which are getting rather independent as we speak.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Thanks for clarifying!

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Reviewed & tested by the community » Needs review

Sorry, I found opportunities to remove duplication

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Clean up seems to be good

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Reviewed & tested by the community » Needs work
mondrake’s picture

mondrake
Location 🇮🇹
commented
Related issues: +#3418267: Support PHPUnit 11 in Drupal 11

Adjusted post-#3418267: Support PHPUnit 11 in Drupal 11.

Letting PHPUnit's own deprecations fail the job execution on PHPUnit 8.5 as the purpose of that job is precisely to shout out when language deprecation or changes occur in this stage of PHP development, so I thought it is a good spot where to let PHPUnit shout as well.

With the PHPUNIT_FAIL_ON_PHPUNIT_DEPRECATION job variable it's a piece of cake to silence them if we do not manage to solve them before PHP 8.5 is released.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Still seems worth adding.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Reviewed & tested by the community » Postponed
Related issues: +#3525031: [CI] Run PHPStan job on PHP 8.4

#3525031: [CI] Run PHPStan job on PHP 8.4 needs to go in first as it fixes a nasty bug that would impact here.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: +PHPUnit 11
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [CI] Components tests coverage metrics differ by PHP version » [PP-3] [CI] Components tests coverage metrics differ by PHP version
Issue summary: View changes
Related issues: +#3515706: [CI] Switch the default test environment to PHP 8.4 and MySQL 8.4 , +#3446693: Convert test annotations to attributes in Drupal/Test/Component
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [PP-3] [CI] Components tests coverage metrics differ by PHP version » [PP-2] [CI] Components tests coverage metrics differ by PHP version
Priority: Minor » Normal
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [PP-2] [CI] Components tests coverage metrics differ by PHP version » [PP-3] [CI] Components tests coverage metrics differ by PHP version
Issue summary: View changes
Related issues: +#3534156: Complete test annotations to attributes conversion for Drupal/Test/Component
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [PP-3] [CI] Components tests coverage metrics differ by PHP version » [PP-2] [CI] Components tests coverage metrics differ by PHP version
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [PP-2] [CI] Components tests coverage metrics differ by PHP version » [PP-1] [CI] Components tests coverage metrics differ by PHP version
Issue summary: View changes
mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: [PP-1] [CI] Components tests coverage metrics differ by PHP version » [CI] Components tests coverage metrics differ by PHP version
Issue summary: View changes
Status: Postponed » Needs review
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3521541-nr-bot_kwkku7rn.txt91 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Additional changes look good to me

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Needs a rebase. I've looked at this one a couple of times, but as with all pipeline changes, find it hard to meaningfully review it / predict whether it will work etc. Since it's only the pipeline changes it should be low risk to find out though.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Reviewed & tested by the community

Rebased.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

We should also make the coverage version run on performance and the others not.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Reviewed & tested by the community

Just local naming changes, back to RTBC.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Addressed reviews.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 11.x-dev » 11.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed c5beab93a87 to 11.x and 929fbcab120 to 11.3.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • alexpott committed 929fbcab on 11.3.x 
    ci: #3521541 [CI] Components tests coverage metrics differ by PHP...

  • alexpott committed c5beab93 on 11.x 
    ci: #3521541 [CI] Components tests coverage metrics differ by PHP...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Fixed » Needs work

We broke this in #3515704: [CI] Move unit tests to a 'unit tests' stage as component tests are again excluded from the performance tests. Let's re-open this one and fix it here...

alexpott opened merge request !13916

alexpott closed merge request !11963

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Reviewed & tested by the community

  • catch committed c040355d on 11.x 
    fix: #3521541 [CI] Components tests coverage metrics differ by PHP...

  • catch committed d2cd53a6 on 11.3.x 
    fix: #3521541 [CI] Components tests coverage metrics differ by PHP...

catch closed merge request !13916

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

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

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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