Skip to content
Snippets Groups Projects

Fix "call to undefined' PHPStan-0 issues in migration system

Closed quietone requested to merge issue/drupal-3469836:3469836--fix-call into 11.x
3 unresolved threads

Closes #3469836

Merge request reports

Code Quality is loading
Test summary results are being parsed

Closed by catchcatch 4 months ago (Feb 14, 2025 12:18pm UTC)

Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • 32 32 ];
    33 33 }
    34 34
    35 /**
    36 * Install required entity schemas.
    37 */
    38 protected function installEntitySchemas() {
    • Duplicating this in two places feels a step backwards... more to maintain - feels the wrong fix. Not sure yet what the correct fix is.

    • Author Maintainer

      And yet, migration tests are full of duplication in the d6/d7 versions of some test. The effort to make a base class or other solution for the same code in the Kernel migration tests out weighed the benefit. And I would argue the same applied here, but sort of in reverse. When this was added there was other 'duplications' in the D6/D7 versions of the Audit tests but for whatever reason they were not pulled out into a shared method.

      I think it is OK to duplicate.

    • Please register or sign in to reply
  • 342 342 return NULL;
    343 343 }
    344 344
    345 /**
    346 * Checks public and private files are copied but not temporary files.
    347 */
    348 protected function assertFileMigrations() {
    • If anything in contrib or custom has used this TestBase class then this is a potential breaking change - not sure we should be making changes like this due to PHPStan. Not sure what the correct fix is yet though.

    • quietone changed this line in version 3 of the diff

      changed this line in version 3 of the diff

    • Author Maintainer

      Hmm, I forgot to leave a note about this. In general, contrib does not write functional tests for migration. And at the time I checked and the only used was in commerce_migrate. Today, this search shows that still be true. And I will make any changes in commerce_migrate for this.

      However, this now has a difference solution which is to add a method getManagedFiles to the base class. That seems a bit simpler.

    • Please register or sign in to reply
  • quietone added 40 commits

    added 40 commits

    Compare with previous version

  • added 42 commits

    • ec08defb...2fddf325 - 41 commits from branch project:11.x
    • b1d755a1 - Merge remote-tracking branch 'origin/11.x' into drupal-3469836-3469836--fix-call

    Compare with previous version

  • quietone added 5 commits

    added 5 commits

    Compare with previous version

  • added 1 commit

    • 66f58b79 - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • added 1 commit

    • e8f74639 - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • Michael Strelan
  • Binoli Lalani added 1 commit

    added 1 commit

    Compare with previous version

  • quietone added 405 commits

    added 405 commits

    Compare with previous version

  • Lucas Hedding added 1 commit

    added 1 commit

    Compare with previous version

  • Lucas Hedding added 6 commits

    added 6 commits

    Compare with previous version

  • 175 175 $this->migration = $migration;
    176 176 }
    177 177 if ($this instanceof MigrateDumpAlterInterface) {
    178 static::migrateDumpAlter($this);
    178 $this::migrateDumpAlter($this);
  • quietone added 17 commits

    added 17 commits

    Compare with previous version

  • closed

  • Please register or sign in to reply
    Loading