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\ContentEntityBaseUnitTest to be ContentEntityBaseTest to be in line with standard naming conventions. This will likely be in another issue.

User interface changes

API changes

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Unfrozen changes Unfrozen because it improves automated tests and maintainability of code.
CommentFileSizeAuthor
#4 interdiff.txt4.09 KBmile23
#4 2388537_4.patch5.16 KBmile23
#1 2388537_1.patch2.52 KBmile23

Comments

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Active » Needs review
StatusFileSize
new 2388537_1.patch2.52 KB

Patch adds a test and data provider for testing ContentEntityBase::get().

daffie’s picture

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

The 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?

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Reviewed & tested by the community » Needs work

Yes, planning to do more. Thanks for the RTBC, though.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Status: Needs work » Needs review
StatusFileSize
new 2388537_4.patch5.16 KB
new interdiff.txt4.09 KB

Added test for getFields().

Very minor annotation changes for testGet().

daffie’s picture

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

The 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.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Not 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!

  • webchick committed 7650e6d on 8.0.x 
    Issue #2388537 by Mile23: Expand unit testing for Drupal\Core\Entity\...

Status: Fixed » Closed (fixed)

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