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.
Once #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method lands, (Read|Create|Update|Delete)Test
will be obsolete. We cannot delete their base class (RESTTestBase
), because that might break contrib module tests, so we can only deprecate it.
Comment | File | Size | Author |
---|---|---|---|
#10 | rest_delete_old_tests-2824576-10-8.3.x.patch | 84.54 KB | Wim Leers |
#10 | rest_delete_old_tests-2824576-10-8.2.x.patch | 84.32 KB | Wim Leers |
Comments
Comment #2
Wim LeersHavin rolled this patch, I couldn't help but notice that:
\Drupal\rest\Tests\AuthTest
is also completely incorporated by #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, so deleted that too\Drupal\rest\Tests\CsrfTest
is also completely incorporated by #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, so deleted that too\Drupal\rest\Tests\NodeTest
is mostly incorporated by #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, so deleted the parts that have been incorporated\Drupal\rest\Tests\PageCacheTest
is mostly incorporated by #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, if we just add some cache tag test coverage to #2737719, we can delete that tooWorking on points 3 and 4 over in that other issue.
Comment #3
Wim LeersAddressed point 4 at #2737719-113: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, so rerolling this patch to also remove
PageCacheTest
.Comment #4
Wim LeersAddressed point 3 at #2737719-114: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, so rerolling this patch to also remove
NodeTest
. In doing so, I discovered #2824827: \Drupal\hal\Normalizer\ContentEntityNormalizer::denormalize() fails with fatal PHP error when bundles are missing from link relation types, which again shows why we need to test all edge cases against all formats.Comment #5
catchWhat about hal module's tests? Can any of those be consolidated?
Comment #6
Wim Leers+1! Will do that in this patch too.
Comment #7
Wim Leers\Drupal\hal\Tests\FileDenormalizeTest
+\Drupal\Tests\hal\Kernel\FileNormalizeTest
because addingFile
entity test coverage is not done by #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, that's for #2824572: Write EntityResourceTestBase subclasses for every other entity type. to add.\Drupal\Tests\hal\Kernel\DenormalizeTest
is testing edge cases for "type" link handling. All other test methods (the majority) can be removed. Although I think it's better to keep itstestMarkFieldForDeletion()
method too, that's explicit test coverage for an edge case, where #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method only added implicit test coverage for that. We should eventually move that into\Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase
too, but that'd make #2737719 even biggger, so I think it's fine to defer that to a follow-up.\Drupal\Tests\hal\Kernel\EntityNormalizeTest
tests a subset of #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method, removed completely\Drupal\Tests\hal\Kernel\EntityTranslationNormalizeTest
+\Drupal\Tests\hal\Kernel\NormalizeTest
are testing translation support, which actually is not properly supported at all yet, for that we have #2135829: [PP-1] EntityResource: translations supportComment #8
Wim Leers#2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method landed!
Let's see if this patch still applies.
Comment #9
Wim LeersWill roll a separate 8.3.x patch first thing tomorrow.
Comment #10
Wim LeersThere were two changes in 8.3.x that prevented the patch from applying to 8.3.x too:
Reuploading the same 8.2.x patch as in #7, and a separate patch for 8.3.x.
Comment #11
tedbowLooking at both branch patches and they both look good! RTBC!
@Wim Leers thanks for cleaning this out.
Comment #13
catchCommitted/pushed to 8.3.x and 8.2.x, thanks!
Comment #15
Wim LeersYay!