- Issue created by @mparker17
- 🇨🇦Canada mparker17 UTC-4
Note that I've started on some tests in my local environment, but due to other factors limiting my time this week, I haven't committed them yet. I do plan to get back to them soon, however!
- Status changed to Needs review
8 months ago 11:30pm 1 April 2024 - 🇨🇦Canada mparker17 UTC-4
The tests in this merge request work on Drupal 9 and 10, testing CKEditor5.
More specifically, it covers the CKEditor Abbreviation JS library, and Drupal's integration with it through the text format, toolbar, and custom integration CSS/JS.
I consider this ready for review! Feedback is welcome.
Given that Drupal 9 has reached end of life on November 1st, 2023 → , I haven't bothered to write tests for Drupal 9 testing CKEditor4, i.e.: the file at
web/modules/contrib/ckeditor_abbreviation/src/Plugin/CKEditorPlugin/AbbreviationCKEditorButton.php
remains untested. Note that we can't drop support for D9, delete the CKEditor4 plugin without breaking backwards compatibility, so deleting this will have to wait for major version 5.Also, regarding the
ckeditor_abbreviation.abbreviation_dialog
route, and the form it points to (\Drupal\ckeditor_abbreviation\Form\CkeditorAbbreviationDialog
)... I can't figure out where these are used, so I'm not sure how to test them, and they remain untested. I suspect this is "dead code", i.e.: no longer used... but if that's wrong, please let me know how I could test this code! - Status changed to Needs work
8 months ago 12:51pm 2 April 2024 - Status changed to Needs review
8 months ago 5:45pm 2 April 2024 - 🇨🇦Canada mparker17 UTC-4
Awesome, new tests pass with no linter notices, so this is ready for review again.
- 🇨🇦Canada mparker17 UTC-4
Was trying to add a test for editing an abbreviation, but that turned out to be more difficult than it appeared, because the API for selecting text in JavaScript acts differently depending on whether you happen to be looking at a Document Node containing only text, or a Document Node containing text mixed with HTML elements. CKEditor appears to have its own way to select text, but there isn't any good documentation on how to use it.
Anyway, I think tests for editing an abbreviation can come later: they shouldn't block merging this.
- Status changed to Fixed
8 months ago 12:51pm 3 April 2024 - 🇨🇦Canada mparker17 UTC-4
After turning off the computer yesterday, I thought of an easier way to write a test to edit an abbreviation, and happily, it works.
All the tests and lints are passing, so I'm going to merge it!
-
mparker17 →
committed d343e149 on 4.0.x
Issue #3424968 by mparker17: Add automated regression tests
-
mparker17 →
committed d343e149 on 4.0.x
Automatically closed - issue fixed for 2 weeks with no activity.