The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.There are two bugs intertwined and fixed in this issue: we call drupal_theme_initialize in TestBase::prepareEnvironment where this makes no sense as the database prefix is not switched yet so this initializes the parent theme. Removing this, however, breaks some Views tests. Some stout and steady detective work revealed that state() is not working during DrupalUnitTestBase. How the two are related is not well understood but certainly fixing state() is a) necessary b) enough to get the Views tests pass again.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | 1874694-9.patch | 3.59 KB | damiankloip |
| #6 | 1874694_6.patch | 3.15 KB | chx |
| #3 | 1874694-3.patch | 1.25 KB | damiankloip |
| no_theme_in.patch | 784 bytes | chx |
Comments
Comment #1
chx commentedComment #3
damiankloip commentedThere is something going on with ViewExecutable::render calling drupal_theme_initialize() in unit tests. Do we need to call this?
Comment #4
damiankloip commentedComment #6
chx commentedOpsie! Every call to state() during DrupalUnitTestBase creates a new memory storage. Great! Erm. Maybe not.
Our code base is so very friendly with error messages. Erm. Maybe not.
Comment #7
chx commentedRe #3: yes, we need the theme initialize call there :P
Comment #8
damiankloip commentedYeah, we do :)
Comment #9
damiankloip commentedYeah, all in all that is some serious digging :)
Docs etc..
Comment #10
Anonymous (not verified) commentedgreat detective work!
can we fix the drupal_theme_initialize() in another issue? reading the patch, and the comments in the OP make me want to separate what is a straight forward fix (the stupid state() stuff) from what is an unknown, if this is possible.
Comment #11
chx commentedlet's not do that; we do not have test coverage otherwise. and removing that drupal_theme_initialize is necessary for the clean fix of a critical so i'd rather get it in asap. In #1874562: Upgrade path broken and yet tests pass we need to catch an exception for when drupal_theme_initialize wanders off and tries to find themes on disk. Removing that call makes it so that doesn't happen. This issue is about removing that drupal_theme_initialize really... how long an issue chain you want to fix an upgrade critical?
Comment #12
berdirI think fixing this together makes sense. The KeyValueMemory changes look good and make sense to me, the memory implementation of course only works as expected if you get the same instance back every time you call it.
We have a similar problem with cache system currently, some cache backends are defined as a separate service, like cache.config, which directly calls into CacheFactory, but the static cache is in cache(). So if you configure cache.config (or any of the other 2-3 defined cache backend services) to use the memory backend ( or another backend that stores some information in the implementation) and then sometimes use cache('config') and sometimes drupal_container()->get('cache.config'), unexpected things will happen.
Which is why I need to get back to the cache in DIC issue :)
Comment #13
catchCommitted/pushed to 8.x, thanks!
Comment #14
chx commentedI need to roll this back and understand what went wrong with plugins cos it's not clear to anyone what went wrong here and that can't stand.
Comment #15
sunAlso, the removal of
drupal_theme_initialize()was not correct. There's a dedicated issue + patch for that for quite some time already:#1770902: Theme of parent site executing test leaks into all tests
Comment #16
chx commentedThe removal was correct from TestBase, I see the other issue adding to other test bases, let's discuss that there. If you want, you can bump the other to major, right now this is kept open to track me trying to debug what is going on with plugins -- I very much dislike bugs I can't understand.
Comment #16.0
chx commentedUpdated issue summary.
Comment #31
quietone commentedThis was committed to 8.0.x in December 2012 and re-opened for further investigation. In the intervening 9 years there is no evidence that any investigation has happened.
I asked in #bugsmash about this, longwave replied (paraphrased) that the two base classes are in simpletest, which is now a contrib project. The keyvalue fix looks valid as per Berdir in #12 it has been in core for years now.
Based on that I am restoring the fixed status of this and the version to when it was fixed.
Thanks!