Widget / image resize tests refactoring #115

mlewand · 2020-01-15T09:55:36Z

Suggested merge commit message (convention)

Tests: Extracted relevant parts of widget resize plugin from the image back to widget plugin. Closes ckeditor/ckeditor5#4614.

Additional information

Not all the tests from imageresize are ported yet, I still want to move the size verification tests to new lower level unit tests in tests/widgetresize/resizer.js. The goal here was to push the PR sooner to get the feedback earlier.

There's a sub pr in ckeditor/ckeditor5-image#340

The editor was attempted to be removed twice.

…I'll rewrite these in a moment.

It had wrong assertions previously. Also corrected test suite name.

…zer that it maintains.

… the object is resized as if it would be center-aligned.

…perly recognized. Without this test added in b675585 is failing.

Also renamed a variable.

Dirty so far, will be cleaned up in a next commit.

Only partially so far, next part in next commit.

…moved. Probably thanks to adjusting the size of the editor / editable.

Looks like it is no longer needed.

…er positions.

coveralls · 2020-02-10T14:13:55Z

Coverage decreased (-2.3%) to 97.733% when pulling 34f4d3b on i/4614 into 886df83 on master.

mlewand · 2020-02-11T09:55:29Z

I'm not sure about the above code coverage message, the CC has been maxed afterwards but I see no further messages.

There are ~~three~~ two tests failing (but it's an upstream issue, not caused by this PR).

Unfold this summary to see failing tests

FAILED TESTS:
Widget - integration
	× should do nothing if clicked inside a nested editable
	Chrome 80.0.3987 (Windows 10.0.0)
	AssertionError: expected '[<div class="ck-widget ck-widget_selected" contenteditable="false"><figcaption contenteditable="true">foo bar</figcaption></div>]' to equal '[]<div class="ck-widget" c
ontenteditable="false"><figcaption contenteditable="true">foo bar</figcaption></div>'

	+ expected - actual

	-[<div class="ck-widget ck-widget_selected" contenteditable="false"><figcaption contenteditable="true">foo bar</figcaption></div>]
	+[]<div class="ck-widget" contenteditable="false"><figcaption contenteditable="true">foo bar</figcaption></div>

	at Context.eval (webpack:///./packages/ckeditor5-widget/tests/widget-integration.js?:128:118)


	× should does nothing for non-Safari browser
	Chrome 80.0.3987 (Windows 10.0.0)
	AssertionError: expected '[<div class="ck-widget ck-widget_selected" contenteditable="false"><figcaption contenteditable="true">foo bar</figcaption></div>]' to equal '[]<div class="ck-widget" c
ontenteditable="false"><figcaption contenteditable="true">foo bar</figcaption></div>'

	+ expected - actual

	-[<div class="ck-widget ck-widget_selected" contenteditable="false"><figcaption contenteditable="true">foo bar</figcaption></div>]
	+[]<div class="ck-widget" contenteditable="false"><figcaption contenteditable="true">foo bar</figcaption></div>

	at Context.eval (webpack:///./packages/ckeditor5-widget/tests/widget-integration.js?:247:118)

That's caused by ckeditor/ckeditor5#6198 and not related to this issue.

~~There's one followup: ckeditor/ckeditor5#6214~~

Reinmar · 2020-02-12T10:15:03Z

tests/widgetresize.js

+		} );
+	} );
+
+	describe( '_getResizerByHandle', () => {


Suggested change

describe( '_getResizerByHandle', () => {

describe( '_getResizerByHandle()', () => {

Same for all other describes.

Pushed a fix for that and few other occurrences.

Reinmar · 2020-02-12T10:17:33Z

The existing tests look quite fine. What I miss... is this PR to be complete ;). It's hard to review this if I don't know what's missing.

Please remember – the goal is to have 100% CC in this package by having reasonable tests of this feature alone. You do test every single bit of this functionality differently, so I can only tell that so far I don't see bigger issues.

Reinmar

As commented.

mlewand · 2020-02-12T11:00:44Z

The existing tests look quite fine. What I miss... is this PR to be complete ;)

@Reinmar Can you turn this into an actionable item? What is missing for the PR to be complete?

As for CC: the entire widgetresize and imageresize features have 100% CC. Image upload observer doesn't have 100% CC but this is outside of the scope of this PR.

Reinmar · 2020-02-12T11:25:03Z

As for CC: the entire widgetresize and imageresize features have 100% CC. Image upload observer doesn't have 100% CC but this is outside of the scope of this PR.

ckeditor5-image and ckeditor5-widget are separate packages and each of them separately should have 100% CC. Not together. Separately.

So, the items would be:

Make sure that ckeditor5-image has 100% CC
Make sure that ckeditor5-widget has 100% CC
Port all remaining tests that you're mentioning that you want to port

Reinmar · 2020-02-25T13:21:23Z

Still no 100% CC:

SUMMARY:
✔ 253 tests completed

=============================== Coverage summary ===============================
Statements   : 98.52% ( 534/542 )
Branches     : 96.12% ( 248/258 )
Functions    : 99.23% ( 129/130 )
Lines        : 98.51% ( 528/536 )
================================================================================
Coverage report saved in '/Users/p/Workspace/ckeditor5/coverage'.
✨  Done in 12.68s.

Reinmar · 2020-02-25T13:26:23Z

I extracted the CC to ckeditor/ckeditor5#6326.

mlewand added 30 commits January 7, 2020 15:44

Tests: initial tests commit.

52a7780

Tests: More tests for produced HTML markup.

3b3024c

Internal: Removed unused private method.

8494c45

Internal: Extracted mouse listeners used in widget resizer.

fa09f04

Tests: Introduced test suite for widget resize class.

a9bb77f

Tests: Dirty checkup commit.

248f208

Tests: Adjusted breaking test.

c9ca31b

The editor was attempted to be removed twice.

Tests: Attempt to add integration tests, but it doesn't look good so …

f26cbe1

…I'll rewrite these in a moment.

Tests: Minor adjustments.

1a9f249

Tests: Removed extra editor and reused existing one.

fcb3d3f

Tests: Further tests simplification.

4b53b76

Tests: Fixed unit test case.

497c1c3

It had wrong assertions previously. Also corrected test suite name.

Tests: Simplified editor creation method.

5f2bc49

Internal: Widget resize upon its destroy should also destroy any resi…

4af8970

…zer that it maintains.

Tests: This test has been moved.

b6f0e14

Tests: Added unit test ensuring that if no isCenter option is passed,…

fb548de

… the object is resized as if it would be center-aligned.

Tests: Added integration test for resize in percents.

b675585

Internal: Fixed an issue where default resizing unit might not be pro…

bee7838

…perly recognized. Without this test added in b675585 is failing.

Tests: Changed default widget width for increased code coverage.

a83a4fc

Tests: Removed extra suite wrapping.

1529135

Also renamed a variable.

Tests: Code coverage in case there's no _activeResizer set.

5cc0578

Dirty so far, will be cleaned up in a next commit.

Tests: Extracted common code.

6eb73ba

Tests: Extracted common onCommit stub.

5b93518

Only partially so far, next part in next commit.

Tests: Extracted more common code.

27b158e

Tests: Since editor is focused for each test move it to beforeEach.

7865154

Tests: Forced resizer redraw is no longer required, so the code is re…

9dd91ba

…moved. Probably thanks to adjusting the size of the editor / editable.

Tests: Removed unused utilities.

af63138

Tests: Removed waiting in-between mouse operations.

203feaa

Looks like it is no longer needed.

Tests: Bumped year in the license header.

65d803a

Tests: Added resizer state test suite.

80cd267

mlewand added 11 commits February 10, 2020 09:41

Tests: Ported percentage widget resize tests.

65a10d2

Tests: getWidgetDomParts is now a shared util.

29817b2

Tests: Make getHandleCenterPoint a shared util.

4cdfd89

Tests: Resizer mouse mock also accepts point object.

3d940fc

Tests: Improved dragTo - it now can accept explicit from and to point…

2e2cc7e

…er positions.

Tests: Expose focusEditor function.

e825829

Tests: Move initialization into a single function.

67688bd

Docs: Adjusted API docs.

01fdd05

Tests: Moved test cases to proper group.

52e1945

Docs: Temporarily use generic event type in API docs.

e2a71d8

Merge branch 'master' into i/4614

7832d9b

Tests: Added a test case to improve CC.

2354d26

mlewand added 2 commits February 11, 2020 10:58

Tests: Renamed mouseMock to resizerMouseSimulator.

17c4532

Tests: Turns out that behavior described in #6221 was invalid.

6ef7ac0

mlewand mentioned this pull request Feb 11, 2020

Fixed tests failing due to invalid widget selection #117

Merged

Merge branch 'master' into i/4614

df4517f

Reinmar reviewed Feb 12, 2020

View reviewed changes

Reinmar suggested changes Feb 12, 2020

View reviewed changes

Tests: Method names in suite description should feature parenthesis.

706414c

mlewand and others added 2 commits February 18, 2020 08:29

Merge remote-tracking branch 'origin/master' into i/4614

3835f24

Merge branch 'master' into i/4614

34f4d3b

Reinmar merged commit d339a64 into master Feb 25, 2020

Reinmar deleted the i/4614 branch February 25, 2020 13:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Widget / image resize tests refactoring #115

Widget / image resize tests refactoring #115

mlewand commented Jan 15, 2020 •

edited

Loading

coveralls commented Feb 10, 2020 •

edited

Loading

mlewand commented Feb 11, 2020 •

edited

Loading

Reinmar Feb 12, 2020

mlewand Feb 12, 2020

Reinmar commented Feb 12, 2020

Reinmar left a comment

mlewand commented Feb 12, 2020

Reinmar commented Feb 12, 2020

Reinmar commented Feb 25, 2020

Reinmar commented Feb 25, 2020

	describe( '_getResizerByHandle', () => {
	describe( '_getResizerByHandle()', () => {

Widget / image resize tests refactoring #115

Widget / image resize tests refactoring #115

Conversation

mlewand commented Jan 15, 2020 • edited Loading

Suggested merge commit message (convention)

Additional information

coveralls commented Feb 10, 2020 • edited Loading

mlewand commented Feb 11, 2020 • edited Loading

Reinmar Feb 12, 2020

Choose a reason for hiding this comment

mlewand Feb 12, 2020

Choose a reason for hiding this comment

Reinmar commented Feb 12, 2020

Reinmar left a comment

Choose a reason for hiding this comment

mlewand commented Feb 12, 2020

Reinmar commented Feb 12, 2020

Reinmar commented Feb 25, 2020

Reinmar commented Feb 25, 2020

mlewand commented Jan 15, 2020 •

edited

Loading

coveralls commented Feb 10, 2020 •

edited

Loading

mlewand commented Feb 11, 2020 •

edited

Loading