#821290: Unexpected error after disabling - unistalling - re-enabling forum module added testEnableForumTaxonomyFieldDependency() to help test the odd code at the beginning of forum_enable():
// If we enable forum at the same time as taxonomy we need to call
// field_associate_fields() as otherwise the field won't be enabled until
// hook modules_enabled is called which takes place after hook_enable events.
field_associate_fields('taxonomy');
This is only one of two places field_associate_fields() is used: http://api.drupal.org/api/drupal/core!modules!field!field.module/functio...
Then, in #943772: field_delete_field() and others fail for inactive fields, the test was moved and stripped down to the point where it doesn't test this anymore.
#821290-31: Unexpected error after disabling - unistalling - re-enabling forum module:
There seems to be 4 options
* Move when modules_enabled is invoked however this seems to be a finalise process so doesn't seem to be option
* Add a call to associate fields as part of the individual install process and just clear the cache modules_enabled()
* Add a hook_enable to the taxonomy module
* Call field_associate_fields in forum_enable and add comments to explain why on earth it is happening.
Option 4 was the one chosen, but there is another possibility:
Changing forum_enable() to forum_modules_enabled().
Unfortunately, because of other changes to the module enabling process (and I think the switching from standard profile to minimal profile), re-adding the old test as-is currently fails.
Also, this blocks #1735118: Convert Field API to CMI because it attempts to change the parameters for field_associate_fields().
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | forum-1740432-3-fail.patch | 723 bytes | tim.plunkett |
| #3 | forum-1740432-3-pass.patch | 961 bytes | tim.plunkett |
| #1 | forum-1740432-1.patch | 852 bytes | tim.plunkett |
Comments
Comment #1
tim.plunkettI now understand why the test was changed, since #943772: field_delete_field() and others fail for inactive fields now prevents taxonomy from being disabled until forum is uninstalled.
This is just a quick attempt at using hook_modules_enabled() instead.
Comment #2
tim.plunkettThis still has code coverage in ForumTest::testEnableForumField().
Comment #3
tim.plunkettI added a comment about why we're using this hook instead of the old one, and a second patch removing the function call without switching the hook.
Comment #4
tim.plunkettWell, crap. That disproves my theory.
In this case we're either still missing test coverage, or that's legacy code that I don't understand why it's still around.
Comment #5
moshe weitzman commentedI'm seeing all green here. I guess you expected a fail?
Comment #6
tim.plunkettYes. Which means either that there is no test coverage, or the code is useless. Likely the former.
Comment #7
tim.plunkett#3: forum-1740432-3-fail.patch queued for re-testing.
Comment #8
tim.plunkett#3: forum-1740432-3-pass.patch queued for re-testing.
Comment #9
tim.plunkettNow that #3 is failing as expected, this isn't major.
Comment #10
tim.plunkettObsolete as of #1735118: Convert Field API to CMI
Comment #11
andypostIn #1907960-205: Helper issue for "Comment field" we have test failure when field-tables are exists but no field definition found. Not sure how this could be possible