Integration tests and CKEditor

Development
#1

In the context of the modernization of our integration tests by updating some legacy tests to Docker, I’m in the process of moving the integration tests of the commments, now supporting CKEditor, from xwiki-platform-distribution-flavor-test-ui to xwiki-platform-flamingo-skin-test-docker.
But more generally, the proposal below places itself in the context of enforcing an efficient and strong testing strategy for XWiki.

Doing the migration to docker integration tests is essential and brings a lot of benefits, but I identified one drawback that I find important to discuss. The docker tests are now executed without CKEditor.

This means that many UI issues purely related to CKEditor will not be automatically tested.
I think that’s a big miss for our automated test suites, because while we test XWiki on one side, and CKEditor in another side, we are no longer going to test their integration, which was the case until now.

Some examples of problems encountered specifically with CKEditor that cannot be detected with other editors, and that can be more easily catch with automated integration tests:

  • some shortcuts where not working due to the integration
  • cursor focus “stolen” by the CKEditor inputs
  • unwanted “This page is asking you to confirm that you want to leave - data you have entered may not be saved.” messages displayed by the browser

Hence, I’d like to open a discussion on which strategies we can deploy to offer integration tests of XWiki with CKEditor.

Proposal 1

Let each test module choose to have a dependency to CKEditor if they need it.

Pros:

  • Fine tuning of the integration test

Cons:

  • More dependencies means longer builds

Proposal 2

Introducing a new option, in addition to the existing browser or database and others, allowing to choose externally which editor should be loaded.

Pros:

  • More tuned integration tests

Cons:

  • A new kind of testing variability to implement
  • Even longer execution time on the CI
  • Is it really a variation point? CKEditor is the only editor in XS currently.

Proposal 3

Keeping xwiki-platform-distribution-flavor-test-ui but migrating it to docker to keep having some tests running on a fully build distribution.

Pros:

  • Full integration testing for XWiki Standard with all features

Cons:

  • Need to chose which test to run on minimal distribution / full distribution, with the risk of duplicated tests

WDYT?

Moving CKEditor Integration to XWiki Platform
#2

I don’t understand this part: If a test is about testing the WYSIWYG editor it must test it with CK for sure! Changing that would break the test. And in this case the test module must install CK asa a dependency.

I guess your question is more: when should we test with the WYSIWYG editor rather than test with the wiki editor? I think that all our func tests should test with the wiki editor, except for modules contributing extensions to CK. For example the Mentions module is probably contributing support for@ and thus it needs to draw the WYSIWYG editor as a dep in its tests to test that feature.

What we need is a test suite for our WYSIWYG editor. @mflorea: the other day we looked for CK tests with Simon and we couldn’t find any. How do we ensure that the WYSIWYG editor is working fine today for all the features it proposes? I know that CK is itself tested but we have substantial changes for it with lots of plugins/etc. Where is this tested? Also, it might still be good to have some tests to verify WYSIWYG behaviors for bugs that were raised in the past (like copy pasting images, etc), even if they are supposed to be tested by CK so that we can control our quality (I’m not saying to duplicate CK’s test suite but to have some well-chosen tests for features that can be more easily broken, possibly across browsers too).

We need to keep some smoke tests to test the full flavor but only smoke tests (i.e. only a few designed to be smoke tests).If only to verify that the XS starts correctly.

#3

Rationale:

  • Testing with the WYSIWYG editor is slower and our test suite is already slow (without counting the tests with the various environments which make it worse)
  • It wouldn’t be consistent to do that in the tests of one module and not the other. That’s why it’s better IMO to have the WYSIWYG editor tests in its own module, see below.
  • Testing with the WYSIWYG editor is also more fragile (flickering)
  • Tests of the WYSIWYG editor should be done inside the WYSIWYG editor module
  • We only need tests with the WYSIWYG in a given module if that module is doing something special with the WYSIWYG editor (like mentions).

WDYT?

#4

I agree that it is slower and more fragile but my point is that in addition to modules that contribute to the WYSIWYG editor, modules that uses WYSIWYG editors in specific ways should also be tested with it. For instance, the comments are now instantiating and manipulating WYSIWYG editors and we already observed that this can lead to issues that IMO are useful to test automatically.

Of course we can discuss if these tests should be in the smoke test suites, and in this case shouldn’t be moved outside of xwiki-platform-distribution-flavor-test-ui.

#5

I don’t see what special things the comments module is doing with the WYSIWYG editor, could you elaborate?

EDIT: ok you mentioned " instantiating and manipulating WYSIWYG editors" but all pages instantiate the WYSIWYG editor in edit mode too. Is there something specific done? And WDYM by manipulating? Is there some special JS code related to WYSIWYG in the comments module?

Thanks

#6

+1. That’s how I see it also.

These tests should be in the CKEditor module. The CKEditor module is supposed to test all the places where the editor is integrated.

#7

If not then it’s a test that should be written in the WYSIWYG editor module itself (to be able to instantiate it and “manipulate it”, whatever this means).

#8

hehe nice to see we’re exactly on the same page :slight_smile:

#9

Where did you look? There are two places:

#10

I get your point. By “manipulate” I mean that the Javascript of the comments is interacting with CKEditor by sending events. The event are generic but we need to ensure that they are working as expected when CKEditor is the comments editors.

In this case I guess we could move the tests of the comments tests to the CKEditor module as proposed by @mflorea.

#11

Hi,

reacting there following this comments https://github.com/xwiki/xwiki-platform/commit/cc180a73320333575687841b69d8d9eecbc1e7c7#r42617162

I don’t think I agree with that assumption.
Here’s what we experienced: we changed comments to allow editing them with CKEditor. It didn’t change much the behaviour of comments with standard Wiki editor, so testing only with Wiki editor did not helped to find much regression.
Now we experienced some problems with CKEditor, because of some javascript interactions. Those interactions are not related of comments code doing something specific with CKEditor, it’s more related with CKEditor doing specific JS calls when you integrate it in a textarea.

So my opinion here is that CKEditor is an important feature of XWiki and one of the most used one: if we perform integration tests of editing things without it, we are actually not testing “in real condition”. I agree that it comes with technical difficulties to properly test with it, but I do think we need it in some cases like Comments where users will actually use it.

#12

Yes and it’s so important that it needs its own tests! :slight_smile:

If the CKeditor integration provides some API to call it/use it, then it needs some func tests about that.

AFAICS this is what is missing and what Marius pointed out too at https://forum.xwiki.org/t/integration-tests-and-ckeditor/7453/6?u=vmassol

#13

It definitely need its own tests, but those tests will be performed with a minimal setup since it’s our best practice and it makes sense.
IMO we also need real integration tests of other features that uses editor, to integrate CKEditor and that’s what I was talking about: to have Comment tests (which doesn’t depend directly on CKEditor) to also be tested with CKEditor being integrated.

I was not talking about APIs just before, but about JS interactions. To give you a very simple example: CKEditor when loaded is performing some checks by loading an editor and destroying it afterwards. Those checks are triggering some JS focus events on the page. Those focus events were directly responsible for some bugs in annotation reply, because the page was not properly scrolled when replying to an annotation.
You cannot find those kind of interactions without testing really the integration.

#14

yes ofc but the goal is not to test everything, that’s just impossible. The goal is to be able to have the most tests we can run in a given timeframe.

So if I have to choose between:

  • Write a func test for a feature that is not currently tested
  • Write variations of an existing test to make sure it works well with the wiki editor, the in-place wysiwyg editor and the standalone wysiwyg editor.

Then my choice is to write the func fest for the new feature.

It’s a probability game.

Also there are other dimensions to consider:

  • We usually use the wiki editor in most of our tests, not because we don’t want to test with the wysiwyg editor but because it’s faster and leads to less flickers.

So it’s a global equation about finding the right balance. My point was that by adding a test with the wysiwyg editor in the comment func test is raising the following points:

  • It means that whoever wrote it, didn’t write the test in the ckeditor module (the test about manipulating a ckeditor instance)
  • The choice of using the wysiwyg editor is “random” and not explained at all in the test. Why use it here and not, in the 99% other func tests? What is the strategy? There needs to be some consistency and a best practice defined so that next time someone write a UI test, he/she can decide whether he/she shold use the wiki editor or the wysiwyg editor.
#15

Remember that the whole topic here is not necessarily about writing new tests. But about refactoring the existing flavor-test-ui tests that are by definition already running with CKEditor being integrated.
And for me that’s the whole point: we are already testing with CKEditor integration in that module (even if we don’t test directly CKEditor features), but if we move all our tests to Docker tests with a minimal setup (as we are supposed to do) we’ll lose that.

I don’t understand this. It wouldn’t make sense at all to perform a Comment test on CKEditor module: it’s not a feature of CKEditor to write comments so it wouldn’t make sense to bring in CKEditor tests the needed dependency to test comments.
Now it is one feature of XWiki Standard to write comments with CKEditor so it makes sense to tests this in XWiki Standard.

Well that’s basically the purpose of this discussion to be able to write next a proposal on the subject :slight_smile: IMO we should have more often tests using CKEditor directly for anything involving edition.

#16

It wouldn’t make sense, I agree. But comments is using CKeditor in some way. That’s what needs to be tested. Again see Marius’s comment at https://forum.xwiki.org/t/integration-tests-and-ckeditor/7453/6?u=vmassol and my comment at https://forum.xwiki.org/t/integration-tests-and-ckeditor/7453/7?u=vmassol

#17

Marius was talking about Mentions. Not comments. [Edit: ok second quote was about comments but I don’t see really what should be tested on CKEditor for it. Now Manuel should know better than me.]

#18

ok didn’t notice there were two topics but I don’t think it changes anything: either the feature (comments, mentions, etc) is doing something special with CKEditor in the way it interacts with it (it has some JS code, etc) and then it makes sense to test with CK to exercise that code, either it doesn’t and this case I don’t think it’s necessary or good to test with CK (extra test time, no strategy/consistency with other tests, etc).

These focus events are inside the comments or annotation module?

In any case, I agree that you need to test all the infinite combination of items to be really sure. But you cannot. You’re mentioning this after the fact (i.e. you know the problem). But what if there’s an issue with the wiki editor now? or when annotation/comments is combined with mentions (and that’s not tested since we test feature by feature in isolation), etc? So it’s about probability and what we can test in timespan we have (ie with the number of commits we do every day and the number of agents we have).

#19

Note that the reason we do this is:

  • Faster tests
  • Ability to test more easily XWiki, feature by feature
  • Verification that dependencies are correctly defined

This is at the detriment of not noticing issues that would arrive when features are combined together.

#20

They are inside CKEditor. And they impact the way Annotation Javascript code is working.

That’s not what I’m saying.
Basically I see two different things:

  1. test features in the same environment that users: it means testing Annotation for example, with CKEditor being used (loaded in XWiki). You don’t need to test directly CKEditor’s features, but the simple fact to load it impacts the javascript and this is why we have some bugs
  2. test features related to CKEditor: for using Mentions in comments for example you actually want to test some features in interactions with CKEditor, and here it’s also debatable if we should perform the integration tests on CKEditor side or on XWiki Platform side

My feeling is that the debate was more on point 2 than on point 1. And as I said, right now we are supporting point 1 with flavor-test-ui tests. But once we’ll finish to refactor them, we’ll lose that.

Like you said, we cannot test everything and it’s a matter of choice. My opinion is that WYSIWYG editor is more used than Wiki editor and it should be more tested.