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.
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?
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).
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.
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.
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.
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.
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 IMO we should have more often tests using CKEditor directly for anything involving edition.
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.]
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).
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:
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
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.
it’s more fragile (more flickers) to use the WYSIWYG editor
the minimum thing that must work in xwiki platform is the wiki editor. The wysiwyg editor is a bonus. Said differently the minimal flavor should not include the WYSIWYG editor, i.e. XWiki can be operated without a WYSIWYG editor. So from a POV of platform it makes sense to test with the wiki editor. From the POV of the XS flavor, it makes less sense but we don’t want to rerun all tests either with the WYSIWYG editor because we don’t have the agent power to do that. Func tests already take too long.
In any case all I’m looking after is some unequivocal rules so that a dev will know for sure if he has to write tests with the WYSIWYG editor not. My preference still goes to not use the WYSIWYG by default and exceptionally use it (it needs to be justified in comment when it’s used), and the dev should think hard first about how that test could be put in the CKEditor module instead.
I agree that we have a problem with our CI and agent power, now I’m really afraid that we loose on usability by taking this perspective. I don’t agree saying that the wysiwyg editor is a bonus, I’m not even sure to agree that it shouldn’t be included in the minimal flavor now. Maybe it’s actually where the debate should took place.
Basically my reasoning is the following:
we need to improve globally perceived usability in XWiki
WYSIWYG editor is a key feature of usability
we push towards more and more automated tests
automated tests are not using WISYWIG editor
there is more and more risk to miss regression linked to side-effect on WISYWIG editor
there is more and more risk to decrease the perceived usability on that side
So here I’m more looking for a long-term perspective for it, do we want to keep considering it as an optional feature of XWiki, or is it on the contrary a key feature that cannot be left aside and that should be considered as a core feature?
The rationale is that it’s a very important feature of XS and thats why it must be well tested in the CKEditor module itself.
There’s very little reasons it would fail outside of it. And the cost of testing with all the other modules is too high for the small risk of failure (which will happen for sure).
So you’re right ofc that testing everything with the WYSIWYG editor will reveal more problems, even if 99% of the cases are useless. My issue is that we’d pay a too high price for the 1%.
A good example could be the “inline macro editing” feature we added a while back. Where should tests for it be? First, in the CKEditor project for sure since we provide a contract for macros (they need to use the proper HTML) and if the macros follow it then the WYSIWYG should work fine. Now we probably also need to add tests in the macros themselves to make sure they follow the contract. But we don’t need to add integration tests for all editable macros because that’d take a lot of time to execute and testing that they follow the contract (testing the generated HTML) is enough to guarantee a good level of confidence (even if there can be problems).
Basically my POV we should have the max number of unit tests and the min number of functional tests because we won’t scale otherwise (we already don’t). It also makes it simpler and faster to debug.
WDYT?
@mflorea, @tmortagne you haven’t express your POV recently and it’s discussion between Simon and myself lately I’d be curious to hear your POV.
I’d be curious to hear your POV.