Problem/Motivation
PHPStan baseline is currently skipping multiple Call to an undefined method errors, #3291519: Fix remaining method.notFound PHPStan L0 errors.
Proposed resolution
Fix the error about getExpectedUnauthorizedEntityAccessCacheability.
'message' => '#^Call to an undefined method Drupal\\\\Tests\\\\file\\\\Functional\\\\FileUploadJsonBasicAuthTest\\:\\:getExpectedUnauthorizedEntityAccessCacheability\\(\\)\\.$#',
'count' => 1,
'path' => __DIR__ . '/modules/file/tests/src/Functional/FileUploadJsonBasicAuthTest.php',
--
'message' => '#^Call to an undefined method Drupal\\\\Tests\\\\file\\\\Functional\\\\FileUploadJsonCookieTest\\:\\:getExpectedUnauthorizedEntityAccessCacheability\\(\\)\\.$#',
'count' => 1,
'path' => __DIR__ . '/modules/file/tests/src/Functional/FileUploadJsonCookieTest.php',
'message' => '#^Call to an undefined method Drupal\\\\Tests\\\\user\\\\Functional\\\\UserRegistrationRestTest\\:\\:getExpectedUnauthorizedEntityAccessCacheability\\(\\)\\.$#',
'count' => 1,
'path' => __DIR__ . '/modules/user/tests/src/Functional/UserRegistrationRestTest.php',
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | 3469843-nr-bot.txt | 90 bytes | needs-review-queue-bot |
Issue fork drupal-3469843
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
quietone commentedNot sure the solution for the last one.
Comment #4
smustgrave commentedSeems straight forward and failure appears to be random block one.
Comment #5
mondrakeComment #6
catchTwo questions on the MR.
Comment #7
quietone commentedComment #8
quietone commentedIf we can continue to use this solution then the remaining instance, in UserRegistrationRestTest, can be fixed.
Comment #9
quietone commentedComment #10
smustgrave commentedActually since these aren't doing anything do we need the parameter to be based? Surprised now a phpstan rule that picks up unused parameter
Comment #11
quietone commentedWhat is the work to do here? If there is work to be done from #10, can you be more specific? I do not understand the question in #10.
Comment #12
smustgrave commentedI meant why add bool $is_authenticated?
NW for the rebase though.
Comment #14
quietone commentedrebased and rebuilt the phpstan baseline.
Comment #15
smustgrave commentedSo seems now we are trading 1 phpstan error for another. Does it need to return something (even empty string)
Comment #16
quietone commentedSo, void no longer work here. How about adding a return of \Drupal\Core\Cache\CacheableMetadata which is what is expected by the parent method, \Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBase::getExpectedUnauthorizedEntityAccessCacheability/
Comment #17
smustgrave commentedWorks for me!
Comment #18
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #19
quietone commentedRebase with conflicts in the baseline, which I then rebuilt.
Comment #20
mstrelan commentedI think this fix is just covering up a code smell. The method
getExpectedUnauthorizedEntityAccessCacheabilityexists in\Drupal\Tests\rest\Functional\EntityResource\EntityResourceTestBasebut not in\Drupal\Tests\rest\Functional\ResourceTestBase, yet the test trait that calls this method is added to classes that don't extend fromEntityResourceTestBase. If we're saying this method needs to exist in all child classes then we might as well just add it to the base, but that makes it clear that it doesn't belong there. Ideally it should be refactored out of the shared trait, or failing that we should check if$this instanceof EntityResourceTestBasebefore calling that function.Comment #23
longwaveAgree with #20, either the trait should declare the method itself or check that it exists - the latter seems cleaner, so let's do that.
Comment #24
dcam commentedTests are passing. The change was OKed and written by a framework manager. There are no more "undefined method" entries in the baseline for
getExpectedUnauthorizedEntityAccessCacheability(). So I think we're good here.Comment #25
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #27
dcam commentedThe bot was complaining about the old MR.
Comment #28
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #29
dcam commentedComment #31
catchCommitted/pushed to main and 11.x, thanks!