The migrate module is still considered an experimental module. As a result, its code and tests have been reviewed less than other more mature code. Attach are changes noted during a review of the files in core/modules/migrate/src/Tests/*.php. These changes include adding docblocks, missing @param/@var/@return descriptions and a few coding style changes.

CommentFileSizeAuthor
#9 2624914-9-migrate-module-src-test-d8.patch17.57 KBlars toomre
#2 2624914-2-migrate-module-src-tests-d8.patch17.56 KBlars toomre

Comments

Lars Toomre created an issue. See original summary.

lars toomre’s picture

lars toomre commented
Status: Active » Needs review
Related issues: +#2624660: Some fixes to migrate/src/Plugin/*.php files
StatusFileSize
new 2624914-2-migrate-module-src-tests-d8.patch17.56 KB

Attached is a patch that includes corrections for eleven files in migrate/src/Tests directory.

lars toomre’s picture

lars toomre commented
Related issues: +#2624934: False sniff for Drupal.Classes.UnusedUseStatement.UnusedUse

For file migrate/src/Tests/MigrateEmbeddedDataTest.php, the sniff Drupal.Classes.UnusedUseStatement.UnusedUse reports a false positive error for an unused use statement. Created Coder issue #2624934: False sniff for Drupal.Classes.UnusedUseStatement.UnusedUse to resolve this for automatic Coder review.

lars toomre’s picture

lars toomre commented
Related issues: +#2624936: False sniffs with short in-line anonymous functions

The file migrate/src/Tests/MigrateTestBase.php reports four false positive sniffs for a code line with a short, in-line anonymous function. Created Coder issue #2624936: False sniffs with short in-line anonymous functions to address this issue in the future.

lars toomre’s picture

lars toomre commented
Related issues: +#2624578: Correct blank line spacing in DocComment.TagGroupSpacing

Filed a third Coder issue #2624578: Correct blank line spacing in DocComment.TagGroupSpacing to resolve the successive @group and @coversDefaultClass directives without a blank line in between. That results in a false positive sniff Drupal.Commenting.DocComment.TagGroupSpacing for the file migrate/src/Tests/MigrationTest.php.

With the application of the patch in issue and resolution of the three Coder issues, all of the files in migrate/src/Tests/*.php should pass the Coder review.

benjy’s picture

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

Migrate updates look good.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2624914-2-migrate-module-src-tests-d8.patch, failed testing.

The last submitted patch, 2: 2624914-2-migrate-module-src-tests-d8.patch, failed testing.

lars toomre’s picture

lars toomre commented
Status: Needs work » Needs review
StatusFileSize
new 2624914-9-migrate-module-src-test-d8.patch17.57 KB

I am a bit puzzled about why the patch from #2 fails to apply. Here is a re-roll against current core to see if I am missing something.

quietone’s picture

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

A diff of the two previous two patches shows a probably copy/paste error in /core/modules/migrate/src/Tests/TemplateTest.php

c84
< index 729ca70..87eaa69 100644
---
> index 7345c53..dcf2a8b 100644
185c185
< index 139bc2f..bb7dc30 100644
---
> index d8e4bbd..e9012e3 100644
363c363
< index 631bab7..99f8045 100644
---
> index f98d807..44400e9 100644
429c429
<      /** @var \Drupal\migrate\MigrateTemplateStorage $template_storage */
---
>      /** @var \Drupal\migrate\MigrateTemplateStorageInterface $template_storage */

So, since benjy RTBC'ed before this wee mistake, setting back to RTBC.

alexpott’s picture

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

Committed f6b32fd and pushed to 8.0.x and 8.1.x. Thanks!

  • alexpott committed 8f7a0d6 on 
    Issue #2624914 by Lars Toomre: Fixes to migrate/src/Tests/*.php files

Status: Fixed » Closed (fixed)

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