Problem/Motivation

Issue has been noticed on #2765437: _user_mail_notify() always sends emails even if $notify is FALSE#43 after adding the @covers annotation to the tests.

In that case tests with @covers always fail due function_exists() on Drupal\Tests\Listeners\DrupalStandardsListener::checkValidCoversForTest() not finding the _user_mail_notify() global function provided by the user.module component.

Clearly the component has not been loaded when @covers annotation is processed.

Proposed resolution

I see two possible ways:
1) Documentation change: functional tests - even if testing a single unit function - don't need @cover tag. This should be documented and occurrences on @covers for functional tests should be removed (I spotted at least one on Drupal\KernelTests\KernelTestBase\DrupalSetMessageTest, where annotation is wrongly added to the class docblock).
2) Bug report/fix: @covers should be allowed on functional tests for single unit functions too, so DrupalStandardsListener should be improved in order to include properly the drupal component providing the function while processing @covers.

Remaining tasks

Deciding if this should be a documentation updated (solution 1) or a bug fix (solution 2)

CommentFileSizeAuthor
#19 covers-annotation-not-working-for-global--2783291-18.patch1.11 KBgambry

Comments

gambry created an issue. See original summary.

gambry’s picture

gambry
Primary language Italian
Location Milan
commented
Related issues: +#2325985: No documentation about @group @coversDefaultClass @covers
gambry’s picture

gambry
Primary language Italian
Location Milan
commented
Parent issue: » #2765437: _user_mail_notify() always sends emails even if $notify is FALSE

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented

This works `@covers ::global_function()` https://github.com/sebastianbergmann/phpunit/issues/248#issuecomment-102...

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented
Status: Active » Closed (won't fix)
gambry’s picture

gambry
Primary language Italian
Location Milan
commented
Status: Closed (won't fix) » Active

@subhojit777 your link proves @covers for global functions works from a PHPUnit prospective, have you tried to run that on Drupal?
Does it work now?

Possibly the error on #2765437: _user_mail_notify() always sends emails even if $notify is FALSE was related to the usage of Simpletest, and maybe that is now fixed by moving tests framework completely to PHPUnit, however we need to prove it as if that is still broken IS Proposed Solutions are still valid.

subhojit777’s picture

subhojit777
he/him
Primary language Bengali
commented

Yes it is working for me. I have noticed that in the test class you should not use `@coversDefaultClass` otherwise it breaks, if you mention a `@covers` for a test function.

Yes I think so, it broke for you because earlier it was simpletest. Now since everything is moved to phpunit, we can refer phpunit directly.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave commented
Status: Active » Closed (outdated)
Issue tags: +Bug Smash Initiative

Closing out as outdated. I'm not seeing any issue with the test cases using @covers on Functional tests.

If you still feel this is an issue please reopen with an updated issue summary.

Thanks!

gambry’s picture

gambry
Primary language Italian
Location Milan
commented
Status: Closed (outdated) » Needs review

confirmed, it seems working fine now on local, but let's try again with testbot.

gambry’s picture

gambry
Primary language Italian
Location Milan
commented
StatusFileSize
new covers-annotation-not-working-for-global--2783291-18.patch1.11 KB

rerolling patch from #2765437: _user_mail_notify() always sends emails even if $notify is FALSE #43.

gambry’s picture

gambry
Primary language Italian
Location Milan
commented
Status: Needs review » Closed (outdated)

All good. Back to closed.