Closed (fixed)
Project:
Migrate Plus
Version:
8.x-5.x-dev
Component:
Tests
Priority:
Major
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
1 Apr 2022 at 17:14 UTC
Updated:
22 Apr 2022 at 18:19 UTC
Jump to comment: Most recent, Most recent file
Show commands
compare
Comments
Comment #2
matroskeenComment #3
matroskeenLet's see if this is enough at least for 9.3.x
Comment #4
matroskeenHere is another patch that aims to fix also PHP 8.1 deprecations.
Unfortunately, it won't help because the "Build successful" message is present also when there are no deprecation messages at all!
See: https://www.drupal.org/pift-ci-job/2352908
The issue is somewhere else...
Comment #5
matroskeenComment #7
matroskeenI've changed return types in Iterator classes in 6.0.x branch to follow the same pattern as Drupal core.
Comment #9
matroskeenThis is just an attempt to run tests without a custom
drupalci.ymlfile.Comment #10
matroskeen#9 was just a guess that custom
drupalci.ymlmight cause "Build Successful" message, but it's not the case.Deprecation messages for 9.3.x and PHP 8.1 are fixed in both branches. There is a deprecation message for 9.2.10, but that's not our problem: #3259744: PHPUnit 9.5.12 (released 2022-01-21) throws unhandled deprecation notice on "Drupal\Tests\Listeners\DrupalListener".
Comment #11
heddnAre the return type changes really needed? They were passing at one point on 8.1. But then drush was falling over so tests couldn't run for a few weeks.
Comment #12
matroskeenIt wasn't needed for tests to pass, but I noticed that in core they added
#[ReturnTypeWillChange]to existing classes for backward compatibility. I thought it was a more safe way to avoid the deprecations.See: #3224523: [PHP 8.1] Add ReturnTypeWillChange attribute where necessary.
And speaking of "Build Successful" message (which triggered me to look into deprecations), it was fixed here: #3274227: Test results show "Build Successful" instead of fails/passes.
Comment #13
heddnthat is needed for BC if you support php 7.3 since return type isn't supported. See https://www.php.net/manual/en/language.types.declarations.php. Since we implicitly state php 7.4+ (see https://git.drupalcode.org/project/migrate_plus/-/blob/6.0.x/migrate_plu...), we don't really need to do that.
Comment #14
matroskeenOh, I see... Should we revert 6.0.x commit?
Comment #15
heddnWe could try and see if it still passes green on 7.4 and 8.1.
Comment #16
matroskeenit's all green here: https://www.drupal.org/node/2202391/qa