The unit tests of Block_content's Migrate source plugins should be changed to the base class introduced in #2791119: Write meaningful Migrate source tests.

CommentFileSizeAuthor
#12 interdiff.txt1.69 KBquietone
#12 2807863-12.patch5.29 KBquietone
#4 interdiff.txt608 bytesquietone
#4 2807863-4.patch5.21 KBquietone
#2 2807863-2.patch5.21 KBphenaproxima

Comments

quietone created an issue. See original summary.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Needs review
Issue tags: +Dublin2016
StatusFileSize
new 2807863-2.patch5.21 KB

Status: Needs review » Needs work

The last submitted patch, 2: 2807863-2.patch, failed testing.

quietone’s picture

quietone commented
Status: Needs work » Needs review
StatusFileSize
new 2807863-4.patch5.21 KB
new interdiff.txt608 bytes

Fixed namespace.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Those PHP 7 fails are a total WTF, but I think they're going to be preventing RTBC for fear of breaking HEAD. Not sure how to verify (or deny) them, though...we probably need input from someone more knowledgeable than myself.

quietone’s picture

quietone commented

For what its worth, the two failing tests, UrlConversionTest and.phpPathAliasTest.php pass locally on Php7 and Maria10.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Not only that, but #2807879: Convert Contact's Migrate source tests to new base class is also having the exact same PHP 7 failures...even though it was passing on PHP 7 before, right here on Drupal CI. I saw it with my very own peepers.

I call shenanigans. Unfortunately I cannot RTBC since I wrote the original patch, but if I could, I would.

chipway’s picture

chipway commented
Status: Needs review » Needs work

These test failures on PHP 7 seems independent of your patch.
On my local PHP 7 + MySQL 5.7, no failure for UrlConversionTest.php and LinkGenerationTest.php, and couldn't run PathAliasTest.php. Same failures on #2807879: Convert Contact's Migrate source tests to new base class

quietone’s picture

quietone commented
Status: Needs work » Needs review

Tests are passing again.

chipway’s picture

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

I have read 2807863-4.patch.
It applies well on 8.3.x, remove the 2 old files and add the 2 new ones as intended.
So it fixes this issue, stays in scope and I RTBC it.

alexpott’s picture

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

See #2807879-14: Convert Contact's Migrate source tests to new base class - let's use better array keys to make it easier to understand what is going on.

quietone’s picture

quietone commented
Status: Needs work » Needs review
StatusFileSize
new 2807863-12.patch5.29 KB
new interdiff.txt1.69 KB

OK.

chipway’s picture

chipway commented

Thanks @alexpott for your hint and @quietone for the new 2807863-12.patch.
It applies well on 8.3.x, fixes this issue, includes #11 comment hint, stays in scope. Tests passes locally on my PHP7.
So I just wait for the confirmation thru bot tests to RTBC it.

chipway’s picture

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

Bot tests OK.

alexpott’s picture

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

Committed and pushed 7d70af5 to 8.3.x and b840cbc to 8.2.x. Thanks!

  • alexpott committed 7d70af5 on 8.3.x 
    Issue #2807863 by quietone, phenaproxima: Convert Block_content's...

  • alexpott committed b840cbc on 8.2.x 
    Issue #2807863 by quietone, phenaproxima: Convert Block_content's...

Status: Fixed » Closed (fixed)

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