#1890502: WYSIWYG: Add CKEditor module to core was committed with pretty solid test coverage; one of the things it tests are the available CKEditor plugins out-of-the-box. This uses PluginManagerBase::getDefinitions(), which does not provide any guarantees on the order in which the definitions are returned. For some unknown reason, it used to pass just fine at #1890502: WYSIWYG: Add CKEditor module to core, but now, that's no longer the case. (I don't know what the root cause is, but clearly plugin discovery is either stateful or non-deterministic.)

Patch test failures at:

CommentFileSizeAuthor
#2 1912028-1-fail.patch1.15 KBwim leers
#2 1912028-1-pass.patch1.27 KBwim leers

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Closed (duplicate)

Dupe of #1912026: CKEditorPluginManagerTest fails because plugins are discovered in slightly different order. Double click, I think :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Closed (duplicate) » Needs review
StatusFileSize
new 1912028-1-pass.patch1.27 KB
new 1912028-1-fail.patch1.15 KB

The "pass" patch should pass and provides the fix.
The "fail" patch should fail, yet only contains a new comment.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#1: ugh! That explains why I got an "issue created" status message upon attaching a file, LOL!

Now, since the patches are here, I'm going to mark the other one as a dupe.

amateescu’s picture

amateescu commented
Status: Needs review » Reviewed & tested by the community

Yes please!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

The results in #2 for the "fail" patch indicate that getDefinitions() is non-deterministic. (Or that's at least what it looks like, I don't know anything about the internals of the plugin system, so…).

In any case, please commit 1912028-1-pass.patch ASAP to unblock lots of inappropriately failed patch reviews in the VDC issue queue and elsewhere :)

fabianx’s picture

fabianx commented

#2: 1912028-1-fail.patch queued for re-testing.

ParisLiakos’s picture

ParisLiakos commented

#1911178: Remove hook_exit() is a victim as well

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Component: editor.module » ckeditor.module

Automatically closed -- issue fixed for 2 weeks with no activity.