Closed (outdated)
Project:
Drupal core
Version:
7.x-dev
Component:
base system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
6 Oct 2010 at 12:40 UTC
Updated:
18 Jul 2013 at 15:16 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
sun.core commentedAt this point, test-only issues are no longer considered major.
Comment #2
Crell commentedIf I understand how the registration works properly, the attached test should verify that contrib can, in fact, override the class that gets used for updating modules. (Themes are identically handled by the same code so do not need their own test.)
dww, please verify and RTBC.
Comment #3
dwwThanks for getting this rolling! I fixed some code style problems (whitespace) and comment formatting.
Although this is a good start, I'm concerned about a few things:
A)
The ModuleUpdater classes can't currently deal with core at all. I know all we're testing is if we get an instance of the right class back. But, this seems a bit fragile given that we might want to put some logic in ModuleUpdater that prevents itself from working with a core module at all. We certainly wouldn't want to give anyone the impression that the above code was something they could do. So, either we need a comment here explaining why in this case it's okay, or we should do something else here entirely. However, I'm not exactly sure what that should be, given that simpletest has to be self-contained and not rely on true contribs. So, I guess a comment is the best we can do (unless someone else has a bright idea).
B) This is basically a unit test to see if the class instantiation works inside a full bootstrap. However, what we really need to do is see if the class instantiation works from inside authorize.php. As we saw with #609772: Impossible to extend the FileTransfer class system in contrib even if the class itself "works" and there's a system that's supposed to let you register your own classes, the way that core is actually using that system doesn't work at all. So, while this test is helpful to make sure the info hook basically works at all, we really need to verify that it actually works in practice. Hence, we need a functional test here, not just a unit test. Ideally, we need a fake FileTransfer class and a fake Updater class and a test that actually runs through authorize.php and ensures that our fake Updater class was successfully instantiated even from authorize.php.
Comment #4
cosmicdreams commentedbumping to needs review so that dww's patch can go through the test-bot.
Comment #5
dwwOkay, great, the bot is happy (for now). But this issue still needs work for the reasons I listed in #3...
Comment #6
cosmicdreams commentedHolding out hope that there will be a patch to test tonight.
Comment #7
Crell commented