Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.UpgradePathTaxonomyTestCase::testTaxonomyUpgrade():
// Ensure instance variables are getting through.
foreach ($instances as $instance) {
$this->assertTrue(isset($instance['required']), 'The required setting was preserved during the upgrade path.');
$this->assertTrue($instance['description'], 'The description was preserved during the upgrade path');
}
$instance is a string, not the field array. This causes failures on PHP 5.4+. There is a secondary problem, the upgrade test doesn't actually set an $instance['description'], and taxonomy_update_7004() doesn't set the required field in all cases.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | interdiff.txt | 1.2 KB | David_Rothstein |
| #4 | 2660766-4.patch | 10.08 KB | David_Rothstein |
| #2 | 2660766-2.patch | 9.5 KB | twistor |
Comments
Comment #2
twistor commentedComment #3
David_Rothstein commentedWow, nice find - that $instances is a very poorly-named variable! (But that's an existing problem which we don't have to fix here.)
Ah, this is confusing, but yes - $instance['description'] deliberately comes from the 'help' field on Drupal 6 vocabularies (not the 'description' field) so that's why the patch needs to set 'help'. However, it should do it in generate-d6-content.sh too (since that's where the database dump comes from).
That will also fix one case in the above patch where the help text was incorrect (not that it matters much).
The rest of the patch looks good to me.
Comment #4
David_Rothstein commentedLooks like generate-d6-content.sh hasn't been run in years (probably not a surprise!) so running it produces lots of other cruft... but here's the patch with that update and just the changes to the database dump that we need for this issue.
Comment #5
David_Rothstein commentedComment #6
David_Rothstein commentedI'm actually going to mark this RTBC myself since my changes were so small and didn't affect the actual functionality at all.
Will leave this for someone else to commit though.
Comment #7
fabianx commentedI was confused for a moment, because of the required => FALSE change, but that is just the default and that makes a lot of sense.
Marked for commit, will leave for a moment in that state and likely commit tomorrow.
Comment #9
fabianx commentedCommitted and pushed to 7.x! Thanks!