Closed (fixed)
Project:
Drupal core
Version:
8.6.x-dev
Component:
base system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
26 Jan 2018 at 21:49 UTC
Updated:
11 Feb 2018 at 23:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
alexpottComment #3
mile23Thanks, @alexpott.
Should be
assertSame().Comment #4
alexpottChanged to use assertSame() and expanded the test to cover more of system_get_info()'s functionality.
Comment #6
dawehnerThis looks like a proper bugfix! Thank you for the detailed test coverage.
Just a general comment: Wouldn't it be nice to have a dedicated exception type for this?
Comment #7
mile23Running the test without the changes to system.module I got this, which seems appropriate:
Also it's nice to get coverage for themes.
In #3, I meant that
assertSame()was needed for checking against an expected empty array, but changing them all toassertSame()is fine, too. :-)And.... I can install ajax_example, which was my original issue: #2208429-340: Extension System, Part III: ExtensionList, ModuleExtensionList and ProfileExtensionList
So this LGTM.
Re: #6, we should really deprecate
system_get_info()for something else rather than changing its return values.Comment #8
dawehnerMost failure in #2659940: Extension System, Part III: ThemeExtensionList and ThemeEngineExtensionList right now are actually triggered and prevented by this patch.
Comment #9
almaudoh commentedI believe this to be the case also. So just waiting for this patch to be committed.
Comment #10
larowlanI agree with #6 - can we get a follow up for that?
Adding review credit for @Mile23
Comment #12
larowlanCommitted 38c2ac7 and pushed to 8.6.x.
Thanks!
Comment #13
mile23Someone just added a pile of deprecations around this: #2940190: [meta] Deprecations of old functions in Extension system and #2940189: Deprecate system_get_info()
Comment #14
almaudoh commentedCreated #2940203: Use dedicated Exception classes for extension system for that.