Problem/Motivation
I'm testing PhpStorm IDE to see how code coverage test works and found some out-of-date @covers statements in:
- core/tests/Drupal/Tests/Core/Cache/CacheTest.php
- core/tests/Drupal/Tests/Core/DrupalTest.php
- core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php
- core/tests/Drupal/Tests/Core/Plugin/ContextHandlerTest.php
- core/tests/Drupal/Tests/Core/Plugin/DefaultPluginManagerTest.php
- core/tests/Drupal/Tests/Core/Routing/AccessAwareRouterTest.php
Proposed resolution
Update the @covers statement to relate to the correct method under test.
Remaining tasks
User interface changes
API changes
Suggested commit message: Issue #2358657 by zaporylie, geerlingguy: Fixed Wrong @covers definitions in Drupal project.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | interdiff.txt | 1.7 KB | geerlingguy |
| #10 | wrong_covers-2358657-10.patch | 7.76 KB | geerlingguy |
| #5 | interdiff-2358657-1-5.txt | 1.24 KB | zaporylie |
| #5 | wrong_covers-2358657-5.patch | 6.06 KB | zaporylie |
| #1 | wrong_covers-2358657-1.patch | 6.1 KB | zaporylie |
Comments
Comment #1
zaporylieThat's initial patch to solve it.
Comment #2
dawehner@zaporylie
Can you find those instances automatically?
Comment #3
zaporylieBy running PHPUnit test with Coverage option.
Comment #4
alexpottIt'd be better to add the
@coversDefaultClass \Drupalhere.What have these become? Just we be added stuff from #2330121: Replace ContentEntityDatabaseStorage::getSchema() with a new EntityTypeListenerInterface implemented by SqlContentEntityStorageSchema
This method never seems to have existed - related #2359005: LocalTaskManagerTest only works because we mock a non existing method
This should be
Drupal\Core\Routing\AccessAwareRouter::__callComment #5
zaporylieI fixed points 1 and 4 from #4. 2 and 3 still needs to be fixed.
Comment #6
alexpottPoint 3 from #4 does not need fixing - what you've done is correct - I was just pointing out why what you've done is correct and that there is no replacement.
Comment #7
alexpottThis is a dupe of #2359007: PHPUnit CLI test runs fail when used with --coverage-text or vica versa. This one was rtbc first and has a smaller nid so I'm going to leave this open.
Suggested commit message:
Issue #2358657 by zaporylie, geerlingguy: Fixed Wrong @covers definitions in Drupal project.I think there might be 1 or 2 changes on that issue that are not in this issue.
Comment #8
geerlingguy commentedAh, didn't notice this issue (thanks to @alexpott for pointing me here); I was trying to get coverage reporting through Travis CI when I ran into this. See, as an example, https://travis-ci.org/geerlingguy/drupal-travis-ci/builds/38821121 (that's with head as of today).
The patch over in #2359007: PHPUnit CLI test runs fail when used with --coverage-text seems to have cleared up all the errors I was getting, but I'll run this patch as well and see if it solves all the issues.
Also, to reproduce locally:
coredirectory, run$ ./vendor/phpunit/phpunit/phpunit --coverage-textComment #9
geerlingguy commentedLet's at least let the testbot have a go. I'm running tests locally to confirm.
Comment #10
geerlingguy commentedUpdated patch; was missing a few corrections.
Comment #11
geerlingguy commented@alexpott had mentioned in IRC:
From my quick look-through, it seems there isn't any replacement that needs to be covered. I'm guessing the
getSchema()function was either removed, or never existed, insideSqlContentEntityStorageSchema... but I didn't spend more than a few minutes trying to figure this out.Comment #12
zaporylieBased on patch committed to core (#2330121-35: Replace ContentEntityDatabaseStorage::getSchema() with a new EntityTypeListenerInterface implemented by SqlContentEntityStorageSchema) getSchema has been removed.
Comment #13
mile23Hey, let's join forces: #2364555: Add @covers annotation, fix some --strict for PHPUnit :-)
Comment #14
mile23As of Nov 9, the patch in #10 applies.
RTBC because it makes the coverage report work.
Marking my issue as postponed because I found other stuff that isn't here. I'll re-roll after this has dropped. #2364555: Add @covers annotation, fix some --strict for PHPUnit
FYI: At the command line you can type:
Or if you want a full HTML report:
No need to do the phpunit/phpunit/phpunit/phpunit/phpunit thing. :-)
Comment #15
alexpottThis issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 433b435 and pushed to 8.0.x. Thanks!