Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Trying to generate a coverage report, but I found @covers
errors throughout Drupal\Tests\Core\Controller\AjaxControllerTest
.
This file was last touched by #2364127: Merge AjaxResponseRenderer into AjaxController so I'm linking it here.
Proposed resolution
Fix the errors.
Remaining tasks
User interface changes
No.
API changes
No.
Comment | File | Size | Author |
---|---|---|---|
#4 | AjaxController.php_.html_.txt | 60.17 KB | YesCT |
#4 | ajaxcoverage.png | 209.13 KB | YesCT |
#1 | 2366645_1.patch | 1.73 KB | Mile23 |
Comments
Comment #1
Mile23Le patch.
Just changes @coversDefaultClass and @covers, plus function summary docs where appropriate.
Comment #2
Mile23Comment #3
YesCT CreditAttribution: YesCT commentedI read the patch, read the tests, and the changes saying they are covering ::content seem reasonable.
I also checked out the commit hash from after #2364127: Merge AjaxResponseRenderer into AjaxController when those lines were added, and there is no methods renderContentIntoResponse or renderMainContent then (or now). so clearly the comments were wrong.
in general, to run test coverage, make sure xdebug is on, (maybe edit a php.ini and restart apache)
then
cd core
./vendor/phpunit/phpunit/phpunit --filter=AjaxControllerTest --coverage-html /tmp/report
before the patch, on head:
Trying to @cover or @use not existing method "\Drupal\Core\Controller\AjaxController::renderContentIntoResponse".
after the patch:
it gets further:
(but after 9 minutes did not get to "done" and the report generated. but that might not be a problem with this patch and due to something else)
[edit: it took 10 minutes and did finish. :)]
I cant think of anything to improve this patch. so rtbc.
Comment #4
YesCT CreditAttribution: YesCT commentedthe coverage report:
Comment #5
Mile23Thanks for the review.
The proof is in the pudding... The tests only call
content()
, so therefore they can only @cover that method. eg:Also: Report generation can take a while for HTML, but you can also verify that it works correctly with
--coverage-text
. It's quicker, will fail on the same errors, and will show you abbreviated results in the console.Comment #6
Wim LeersMy bad.
Patch looks good!
Comment #7
YesCT CreditAttribution: YesCT commented@Mile23 oh, thanks for the hint about leaving off the html to speed that up.
Comment #8
alexpottThis issue is an improvement to automated testing and there is unfrozen as per #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?. Committed 211fd40 and pushed to 8.0.x. Thanks!