Note: This only happens in tests. It was introduced here:
#521838-78: Clean up drupal_get_schema_versions() and commited in #521838-91: Clean up drupal_get_schema_versions() with the awesome comment "The patch looks fairly harmless (which probably means it will break everything ;))."
- drupal_get_schema_versions() caches the schema for all enabled modules.
- A parent test site has usually modules enabled like 'block' or 'node', but as the .install file for those is not loaded, drupal_get_schema_versions() returns array() for those.
- Therefore in system_install() when the module_list() is reset, but drupal_get_schema_versions() is not, the next time drupal_get_schema_versions() is called the wrong schema is returned. And therefore the wrong schema is recorded in the database.
This however happens just for the first test method in a test class, because as the .install files have been loaded the next time drupal_get_schema_versions() magically works (as once loaded functions don't disappear)!
- Reset the drupal_get_schema_versions() static in system_install(), which is fine as it is _just_ called afterwards again anyway (in drupal_install_system).
- Create patch
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 2759899-4.patch | 1.48 KB | fabianx |
| #6 | 2759899-4--tests-only.patch | 1018 bytes | fabianx |
| #6 | interdiff.txt | 1.71 KB | fabianx |
Comments
Comment #2
fabianx commentedHere is the patch and test. I decided to add this to the existing test method to make the test explicit.
Comment #4
fabianx commentedI double checked that this is not a problem in D8 both by caching the setUp(), which revealed the bug in D7 and by porting the --tests-only patch over to D8.
Comment #5
David_Rothstein commentedThat looks very good to me.
I'm going to mark this RTBC since I know it's blocking another issue. I'm not sure I understand your reasoning for putting it in an existing test method though... Since the test relies on being the first test method in the class, I would think a dedicated test method would work a little better for that? Because then we could be very explicit (via a code comment or the method name) that it always has to be first... right now that info is sort of there in the code comment but it gets a little overshadowed by all the other things around it, and I can imagine someone not noticing that later and adding a method that happens to run before it.
Comment #6
fabianx commentedHmm, I wanted to save on performance, but that is a very good point.
Changed it like that, but leaving at RTBC as I only moved around code.
Comment #8
stefan.r commentedNice! This only affects tests (when module_list()/drupal_get_schema_versions() are already populated), and fixes a regression introduced 6 years ago :)
Comment #10
stefan.r commentedCommitted and pushed to 7.x, thanks!