Cilefen discovered in (https://www.drupal.org/node/2633422#comment-10661646) that we should be able to document the new test function with @covers::getMigration, but we can't properly do that because the class itself has the wrong annotation:

@coversDefaultClass \Drupal\Tests\migrate\Unit\MigrateExecutableTest

CommentFileSizeAuthor
#7 2633664-7.patch565 bytesjordanpagewhite
#4 2633664-4.patch2.02 KBjordanpagewhite
#2 2633644-2.patch2.06 KBjordanpagewhite

Comments

jordanpagewhite created an issue. See original summary.

jordanpagewhite’s picture

jordanpagewhite commented
Status: Active » Needs review
StatusFileSize
new 2633644-2.patch2.06 KB

I changed the @coversDefaultClass of MigrateExecutableTest to \Drupal\migrate\MigrateExecutable and I added a @coversDefaultClass ::getMigration to the testGetMigration method added in (https://www.drupal.org/node/2633422).

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work
Related issues: +#2633422: Add getter for Migration to MigrateExecutableInterface

This should only cover a single change i.e. coversDefaultClass. The stuff in #2633422: Add getter for Migration to MigrateExecutableInterface shouldn't get embedded here.

jordanpagewhite’s picture

jordanpagewhite commented
StatusFileSize
new 2633664-4.patch2.02 KB

I apologize. I was completely misunderstanding the correct git workflow that was necessary. I am still kind of new to contributing. I believe that the rebase fixed the unnecessary changes that you noted in my previous patch.

jordanpagewhite’s picture

jordanpagewhite commented
Status: Needs work » Needs review

I forgot to update the status.

heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Needs work 
+++ b/core/modules/migrate/tests/src/Unit/MigrateExecutableTest.php
@@ -14,7 +14,7 @@
+ * @coversDefaultClass \Drupal\migrate\MigrateExecutable

This is all that should be in the patch.

jordanpagewhite’s picture

jordanpagewhite commented
StatusFileSize
new 2633664-7.patch565 bytes

Yeah, sorry. I think I finally have it. I guess I'm struggling with git branches. Thanks again.

jordanpagewhite’s picture

jordanpagewhite commented
Status: Needs work » Needs review
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Assigned: jordanpagewhite » Unassigned
Status: Needs review » Reviewed & tested by the community

Looks good now.

alexpott’s picture

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

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

  • alexpott committed a2f2069 on 8.1.x 
    Issue #2633664 by jordanpagewhite, heddn: Wrong Annotation in...

  • alexpott committed 4558472 on 
    Issue #2633664 by jordanpagewhite, heddn: Wrong Annotation in...

Status: Fixed » Closed (fixed)

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