Problem/Motivation

Spinoff of #3274474: Fix 'Access to an undefined property' PHPStan L0 errors, for test code only.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3281535

Command icon 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

mondrake created an issue. See original summary.

mondrake’s picture

Assigned: Unassigned » mondrake

On this

mondrake’s picture

Has the execution time of phpstan analyze --generate-baseline more than tripled for me only? Since latest updates of phpstan and phpstan-drupal.

Not sure what to do about typehinting e.g. User vs UserInterface - in runtime code we'd certainly be using the interfaces, but in tests?

mondrake’s picture

Status: Active » Needs review
mondrake’s picture

Status: Needs review » Needs work
mondrake’s picture

Status: Needs work » Needs review

Ready for review.

mondrake’s picture

Assigned: mondrake » Unassigned
mallezie’s picture

Did an initial review.

Changes look correct, and in scope of only test classes. (Did not check if we have all of them here, but we're removing 150+ instances, so I would not block of missed some in test classes here).

Only question from my side (but could be wrong here) is that we're a bit inconsistent in using classes vs interfaces here. (In some tests we add User vs UserInterface, same for Node vs NodeInterface). Would not block on that, just wondering if there is a 'standard' we should use.

Needs rebase however.

mondrake’s picture

Re #4, #9, asked for guidance in Slack

andypost’s picture

mondrake’s picture

Consistently use concrete classes for entities

daffie’s picture

Status: Needs review » Needs work

Just 1 nitpick. After that it is RTBC for me.

mondrake’s picture

Status: Needs work » Needs review
daffie’s picture

Status: Needs review » Reviewed & tested by the community

All code changes look good to me.
All change are for tests only.
For me it is RTBC.

mondrake’s picture

rebased

alexpott’s picture

Version: 10.0.x-dev » 9.4.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

We should backport the test changes here. For PHP 8.2 compatibility and to make backports easier.

Committed 6dd0db5 and pushed to 10.0.x. Thanks!

  • alexpott committed 6dd0db5 on 10.0.x
    Issue #3281535 by mondrake, daffie: Fix 'Access to an undefined property...
mondrake’s picture

Assigned: Unassigned » mondrake
Status: Patch (to be ported) » Needs work

on that

mondrake’s picture

Status: Needs work » Needs review
StatusFileSize
new58.04 KB

Here's a 9.5.x and 9.4.x patch.

mondrake’s picture

Assigned: mondrake » Unassigned

Status: Needs review » Needs work

The last submitted patch, 21: 3281535-21-95x.patch, failed testing. View results

mondrake’s picture

Status: Needs work » Needs review
daffie’s picture

Status: Needs review » Reviewed & tested by the community

The changes are the same ones as the ones for D10, only without the added parameter typehints.
For me it is RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

We need to account for #3291018: Move \Drupal\Tests\ckeditor5\Functional\CKEditor5QuickEditLibraryTest to the quickedit namespace/directory - error: core/modules/ckeditor5/tests/src/Functional/CKEditor5QuickEditLibraryTest.php: does not exist in index - so the test has moved...

mondrake’s picture

Status: Needs work » Reviewed & tested by the community
StatusFileSize
new58.07 KB

Rerolled and adjusted for #26

mondrake’s picture

Version: 9.4.x-dev » 9.5.x-dev

A 9.4.x port will require another patch again.

spokje’s picture

Version: 9.5.x-dev » 9.4.x-dev
StatusFileSize
new2.05 KB
new58.05 KB

Added patch for 9.4.x.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 76ca2c3 and pushed to 9.5.x. Thanks!
Committed e71bebf and pushed to 9.4.x. Thanks!

Keeping tests aligned.

  • alexpott committed 76ca2c3 on 9.5.x
    Issue #3281535 by mondrake, Spokje, daffie, alexpott: Fix 'Access to an...

  • alexpott committed e71bebf on 9.4.x
    Issue #3281535 by mondrake, Spokje, daffie, alexpott: Fix 'Access to an...

Status: Fixed » Closed (fixed)

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