- Issue created by @nicxvan
- π¬π§United Kingdom catch
One possible idea, it's quite horrible.
Instead of the exception, can we reverse engineer what the expected function name would be from the OOP hook implementation, check function_exists(), and if it's not there, trigger_error() something?
- πΊπΈUnited States nicxvan
I'm not sure why this is failling:
core/tests/Drupal/KernelTests/Core/Asset/ResolvedLibraryDefinitionsFilesMatchTest.php fails cause it's looking for modules as themes and they don't exist.
This test doesn't run locally so I can't easily debug.
- πΊπΈUnited States nicxvan
For some reason this change is triggering module handler to not have the modules when LibraryDiscoveryParser::buildByExtension is checking so it checks for modules as themes.
This doesn't make sense, this shouldn't change anything related to this.
I even tried an alternate version where we pass the module in through collect by attribute to procedural check and throw the error only there if the function doesn't exist.
I reverted my changes and it still fails. Is this an 11.1.x only failure?
- πΊπΈUnited States nicxvan
That test is busted on 11.1.x: https://git.drupalcode.org/project/drupal/-/pipelines/499192
- πΊπΈUnited States nicxvan
I added a test for this specifically, but even with the expect exception for the one we expect, I'm getting a symfony exception.
I could use some assistance getting past this:
1) Drupal\KernelTests\Core\Hook\HookCollectorPassTest::testHookPreprocessNoFunction Symfony\Component\HttpFoundation\Exception\SessionNotFoundException: There is currently no session available.
- πΊπΈUnited States nicxvan
I converted to trigger_error as suggested in 2. There doesn't seem to be a way to assert user warnings anymore. I tried converting to an exception in the test as suggested by the phpunit issue, but I run into the same symfony exception.
- πΊπΈUnited States nicxvan
This is truly ready for review now!
Please credit @fathershawn for his help on slack getting me past the test issues!
The failure is unrelate I think and exists on 11.1.x HEAD.
- πΊπΈUnited States nicxvan
Created π [11.1] ResolvedLibraryDefinitionsFilesMatchTest is failing Active for the failing test on 11.1.x HEAD
The Needs Review Queue Bot β tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide β to find step-by-step guides for working with issues.
- πΊπΈUnited States nicxvan
I think the bot is wrong here cause it's a different branch.
- π¬π§United Kingdom catch
This looks great.
Currently, if you add an OOP preprocess hook, you get an exception on 11.1 because it doesn't support preprocess has OOP hooks, and it's trying to stop you prematurely converting things that won't work.
With the MR, the next 11.1 patch release will allow the OOP hook to exist and ignore it, as long as there's a procedural version too - e.g. still using
LegacyHook
, but warn you if the legacy hook is missing.The result is that modules will be able to require >= 10.4.0 | >= 11.1.8 (or whatever exact version it ends up in) if they want to convert. Otherwise their only option would be >= 10.4.0 | > 11.2.0
-
alexpott β
committed 59f1bee0 on 11.1.x
Issue #3524716 by nicxvan, catch: [11.1] update gather rules to manage...
-
alexpott β
committed 59f1bee0 on 11.1.x
- πΊπΈUnited States nicxvan
I updated the CR in https://www.drupal.org/node/3496491 β