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
Drupal\Core\Entity\ContentEntityBase has exceedingly poor unit test coverage, and an alarmingly high CRAP value (19805.58).
I decided that I'd work on ::get() and ::set() in order to improve coverage for some code that's used all the way through Drupal 8.
I might have found a bug in ::set(), so I didn't write any tests: #2388501: $notify in FieldableEntityInterface::set() is useless
Otherwise, I made a test for ::get()
Proposed resolution
- Evaluate the patch.
- Commit the patch.
- Rejoice.
Remaining tasks
- Add more coverage in subsequent patches to this issue.
- Eventually rename
Drupal\Tests\Core\Entity\ContentEntityBaseUnitTestto beContentEntityBaseTestto be in line with standard naming conventions. This will likely be in another issue.
User interface changes
API changes
Beta phase evaluation
| Unfrozen changes | Unfrozen because it improves automated tests and maintainability of code. |
|---|
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | interdiff.txt | 4.09 KB | Mile23 |
| #4 | 2388537_4.patch | 5.16 KB | Mile23 |
| #1 | 2388537_1.patch | 2.52 KB | Mile23 |
Comments
Comment #1
Mile23Patch adds a test and data provider for testing
ContentEntityBase::get().Comment #2
daffie CreditAttribution: daffie commentedThe ContentEntityBaseUnitTest has been extended with testing for the method get().
The comments are all in order.
It looks good to me so I give it a RTBC.
@Mile23: There are a lot of methods that not have test coverage. Do you want to add more?
Comment #3
Mile23Yes, planning to do more. Thanks for the RTBC, though.
Comment #4
Mile23Added test for
getFields().Very minor annotation changes for
testGet().Comment #5
daffie CreditAttribution: daffie commentedThe ContentEntityBaseUnitTest has been extended with testing for the method get() and getFields().
The comments are all in order.
It looks good to me so I give it a RTBC.
Comment #6
webchickNot really the most qualified to give these sign-off, but they've been sitting here long enough for someone to raise objections, and moar test coverage is never a bad thing.
Committed and pushed to 8.0.x. Thanks!