During kernel tests, ExtensionDiscovery is set up with the site directory, ORIGIN_SITE, as a virtual filesystem. This means the $searchDir item for this is of the form 'vfs://root/sites/simpletest/28733299'.
However, scanDirectory() doesn't manage to find anything in this, because it treats all the directories as being relative, and prefixes them all with the root:
$dir_prefix = ($dir == '' ? '' : "$dir/");
$absolute_dir = ($dir == '' ? $this->root : $this->root . "/$dir");
That means we end up with the directory '/Users/me/whatever/etc/vfs://root/sites/simpletest/28733299' which is broken.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 2961541-2.core_.ExtensionDiscovery-fails-to-scan-the-vfsStream-filesystem-for-modules-or-profiles.patch | 953 bytes | joachim |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
joachim commentedComment #3
dawehnerThank you for the patch!
I'm currently looking at
\Drupal\Tests\Core\Extension\ExtensionDiscoveryTestas it is using VFS already. Do you think it would be possible to extend the test coverage?Comment #4
dawehnerComment #5
joachim commented> Do you think it would be possible to extend the test coverage?
One thing to bear in mind is that ExtensionDiscovery keeps a static cache, and you can't clear it during a request, or for that matter, during a Kernel test.
Over at Drupal Code Builder, I wrote Kernel tests that write a module to the vfsStream filesystem, and then try to enable it. I had to resort to using PHP Reflection to get ExtensionDiscovery to notice the new module -- see https://github.com/drupal-code-builder/drupal-code-builder/blob/3.2.x/Te...
Comment #6
dawehnerYeah maybe we should indeed add a mechanism to reset it. Its output also depends on the current installation profile, so its result might change.
Comment #9
andypostComment #14
wim leersRan into this with #3228505: Plugin definition DX: automatically check for plugin definitions whether their ::getDefaultSettings() matches the config schema too, but worked around it. Added a
@todothere to remove it when this issue gets fixed.Comment #18
smustgrave commentedThis issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.
For the tests requested in #4
Also should follow the default template for an issue summary please.
Comment #20
joachim commentedRebase on 10.1.x and made a MR.
Still needs test.
Comment #22
wim leersMaybe deleting the work-around in
\Drupal\Tests\ckeditor5\Kernel\CKEditor5PluginManagerTestwhich was necessary before this fix is enough to prove it now works — maybe we don't really need explicit test coverage for such a detail? 😇Unless it fails, of course.
😱 Which it does!
Comment #23
wim leersLooks like the static file cache in
\Drupal\Core\Extension\ExtensionDiscoveryis still a problem. There's no way to wipe that.Comment #24
wim leersComment #25
wim leersWorked on this again for #3401837: Add basic validation to config schema definitions. I'm pretty sure I have a solution now. 🤓
Comment #26
wim leersComment #27
wim leersHm, looks like the CKEditor 5 plugin manager test is extra complicated due to
\Drupal\ckeditor5\Plugin\CKEditor5PluginDefinition::getTypedConfig()using the (global) container, which contains some static caches that get in the way. 😬Comment #28
borisson_Back to needs work to correctly reflect the current state.
I see that you also implemented this fix (or at least the
\Drupal::rootcall) in #3401837: Add basic validation to config schema definitions, does that mean we close this as a duplicate or do we want to keep this open?Comment #29
wim leersWe should keep this open: this issue is specifically scoped to fix this problem, in #3401837 I had no choice but to figure out a solution for this. Ideally we'd be able to remove this change from #3401837 to allow a tighter scope 😇