Problem/Motivation

\Drupal\migrate\Entity\Migration::getProcessPlugins() will return NULL if an empty array is passed.

I hit this with the d6_user_picture_file migration, which does not specify a process plugin for the fid field. The resulting error is

Invalid argument supplied for foreach() MigrateExecutable.php:359

I don't know why this does not show up in the test suite.

Proposed resolution

Initialize $this->processPlugins[$index] to an empty array.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#4 2335345-3-migrate-process-plugins-array.patch1.54 KBtstoeckler
#4 2335345-1-3-interdiff.txt627 byteststoeckler
#1 2335345-1-migrate-process-plugins-array.patch1.54 KBtstoeckler
#1 2335345-1-tests-only.patch910 byteststoeckler

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Active » Needs review
StatusFileSize
new 2335345-1-tests-only.patch910 bytes
new 2335345-1-migrate-process-plugins-array.patch1.54 KB

Here we go.

The last submitted patch, 1: 2335345-1-tests-only.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
commented 
  1. +++ b/core/modules/migrate/tests/src/Unit/Entity/MigrationTest.php
@@ -0,0 +1,30 @@
+   * @covers ::getProcessPlugins()

    Didn't we switched to use no ()? 

  2. +++ b/core/modules/migrate/tests/src/Unit/Entity/MigrationTest.php
@@ -0,0 +1,30 @@
+    $migration = new Migration(array(), 'migration');
+    $this->assertEquals([], $migration->getProcessPlugins([]));

    let's just use the short array syntax everywhere.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
StatusFileSize
new 2335345-1-3-interdiff.txt627 bytes
new 2335345-3-migrate-process-plugins-array.patch1.54 KB

Thanks!

Re 1. We should still wait for some more feedback on #2328919: Remove () from a bunch of @covers definitions in PHPUnit IMO, and as stated there, I find the () to be more consistent with @see. I won't lose sleep over this, though, if you insist.

Fixed 2. (It's gonna be a while until [] becomes second nature...)

benjy’s picture

benjy commented
Status: Needs review » Reviewed & tested by the community

Not sure we should hold this functional issue + a test up while we decide on some doc standards. That issue can update this file if they change the standards.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 2ef89a6 and pushed to 8.0.x. Thanks!

  • alexpott committed 2ef89a6 on 8.0.x 
    Issue #2335345 by tstoeckler: Fixed Entity\Migration::getProcessPlugins...

Status: Fixed » Closed (fixed)

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