Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
EntityResourceTestBase::getUrl() returns a URL, but BrowserTestBase::getUrl() (which the former overrides) returns a string.
Proposed resolution
Rename EntityResourceTestBase::getUrl() to EntityResourceTestBase::getResourceUrl(). To keep them in sync also rename EntityResourceTestBase::getPostUrl() to EntityResourceTestBase::getResourcePostUrl()
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 2869415-4.patch | 7.39 KB | wim leers |
Comments
Comment #2
wim leersDarn — I/we totally missed that in #2737719: EntityResource: Provide comprehensive test coverage: for every entity type, every format, every method! Great find!
EntityResourceTestBasehas:getUrl()(Gets an entity resource's GET/PATCH/DELETE URL.)getPostUrl()(Gets an entity resource's POST URL.)I propose we change this to:
getResourceUrl()getResourcePostUrl()Comment #3
tstoecklerYes, that would make a lot of sense, I think.
Comment #4
wim leersHrm, the comment says
Gets an entity resource's GET/PATCH/DELETE URL.. It doesn't say . It says .So, I think this is better:
getEntityResourceUrl()getEntityResourcePostUrl()Comment #5
dawehner+1 for naming things different if its doing different things.
Comment #6
gábor hojtsyI agree the renames make sense. Do contrib test depend on the existing functionality? After all the renames effect protected methods.
Comment #7
dawehner@Gábor Hojtsy raised a fair point. Let's keep the old functions around ... at least for BC reasons with some
@trigger_error()code.Comment #8
dawehnerComment #9
wim leersQuoting https://www.drupal.org/core/d8-bc-policy:
We're continuously updating
(Entity)ResourceTestBaseas we improve its test coverage.Furthermore, 99% of the calls to the affected methods happen in
test(Get|Post|Patch|Delete), which are provided byEntityResourceTestBase, which means that 99% of contrib modules' test coverage will not actually be affected.99% of contrib module tests will look like e.g.
\Drupal\Tests\rest\Functional\EntityResource\Node\NodeResourceTestBaseand\Drupal\Tests\rest\Functional\EntityResource\Node\NodeJsonBasicAuthTest: with zero calls to the affected methods!Comment #10
gábor hojtsyComment #13
gábor hojtsyAll right. I asked for second opinions from other committers and the conclusion between @alexpott, @catch and @xjm was that this is a tricky place because it is a method override in a test. So it cannot be marked deprecated because the parent is not deprecated. However it works differently from the parent, which should not have been done in the first place. Also not backporting the change would be tricky because contributed modules would need to somehow be both compatible with the different testing API in 8.3.x and 8.4.x.
So the conclusion was to make this jump and if there are contributed modules who end up being affected, that is expected to be minimal disruption.
Thanks all!
Comment #14
wim leersThanks for your due diligence. I'm glad we arrived at this sane solution :)
And thanks again to @tstoeckler for noticing this in the first place!