Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(): migrate tests to playwright/jest #9333

Merged
merged 32 commits into from
Jan 20, 2024
Merged

test(): migrate tests to playwright/jest #9333

merged 32 commits into from
Jan 20, 2024

Conversation

ShaMan123
Copy link
Contributor

Motivation

#9329

Description

Working on #9329 unconvered many ubgs.
Trying to fix them made tests fail.
These tests are not human friendly, they dump endless output onto the console so they are useless.
To be percise draggable_text was written by me while grinding my teeth. It was hard and I knew it was incorrect to write it like that.
So I have moved draggable_text to e2e as it should be, removing redundancies.
And I have moved all the findTarget related tests to jest.

Changes

Gist

In Action

@github-actions
Copy link
Contributor

github-actions bot commented Sep 13, 2023

Build Stats

file / KB (diff) bundled minified
fabric 907.643 (0) 304.633 (0)

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted, now only tests are here and #9329

"canvas"
],
[
"dragleave",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird, a bug.
after dragstart nothing has entered yet

Comment on lines 23 to 55
"dragenter",
{
"e": {
"isTrusted": true
},
"target": "a",
"subTargets": [],
"dragSource": "b"
},
"canvas"
],
[
"dragleave",
{
"subTargets": [],
"dragSource": "b",
"e": {
"isTrusted": true
},
"target": "b",
"nextTarget": "a",
"isClick": false,
"pointer": {
"x": 120,
"y": 55
},
"absolutePointer": {
"x": 120,
"y": 55
}
},
"b"
],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here canvas says b is entering while b is saying it is leaving

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@asturur please merge this
The tests in this PR are much more freindly and reliable - I have PRs with tests that are failing, tests that are migrated here.

Copy link
Contributor Author

@ShaMan123 ShaMan123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that tests are failing becuase of font differences.
We need to register the font - the code you removed from the other CI PR

Comment on lines +95 to +97
...(pointer ? { pointer: roundPoint(pointer) } : {}),
...(absolutePointer
? { absolutePointer: roundPoint(absolutePointer) }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scenePoint/viewportPoint

Copy link
Contributor

Coverage after merging test-etc into master will be

82.71%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
index.node.ts14.29%100%0%25%23, 26, 29, 41, 44, 47
src
   ClassRegistry.ts100%100%100%100%
   Collection.ts94.85%94.83%87.10%97.14%109, 112, 215–216, 241–242
   CommonMethods.ts96.55%87.50%100%100%9
   Intersection.ts100%100%100%100%
   Observable.ts87.76%85.29%87.50%89.58%134–135, 160–161, 32–33, 41, 50, 78, 89
   Point.ts100%100%100%100%
   Shadow.ts98.36%95.65%100%100%178
   cache.ts97.06%90%100%100%57
   config.ts74.14%65%66.67%82.76%131, 139, 139, 139, 139, 139–141, 152–154, 30
   constants.ts100%100%100%100%
src/LayoutManager
   LayoutManager.ts87.50%89.29%76.92%90%133–134, 136–137, 137, 137, 137, 137, 231, 289–290, 301, 46
   constants.ts100%100%100%100%
src/LayoutManager/LayoutStrategies
   ClipPathLayout.ts82.05%72.22%100%88.89%30–31, 42, 54, 57–58, 65
   FitContentLayout.ts100%100%100%100%
   FixedLayout.ts20%0%0%40%20–21, 23, 23, 23, 23, 23
   LayoutStrategy.ts86%64.71%100%96.15%36, 43, 43, 43, 51, 71–72
   utils.ts91.67%85.71%100%100%28, 34
src/Pattern
   Pattern.ts90.54%93.94%80%90.32%119, 130, 139, 32, 36
src/brushes
   BaseBrush.ts100%100%100%100%
   CircleBrush.ts0%0%0%0%108, 108, 108, 110, 112, 114–116, 118–121, 128–129, 13, 136, 138, 23–24, 32–36, 40–44, 51–54, 62–66, 68, 76, 76, 76, 76, 76–77, 79, 79, 79–82, 84, 92–93, 95, 97–99
   PatternBrush.ts97.06%87.50%100%100%21
   PencilBrush.ts91.06%82.35%100%93.81%122–123, 152, 152–154, 176, 176, 276, 280, 285–286, 68–69, 84–85
   SprayBrush.ts0%0%0%0%107, 107, 107, 107, 107–108, 110–111, 118–119, 121, 123–127, 136, 140–141, 141, 148, 148, 148–151, 153–156, 160–161, 163, 165–168, 17, 171, 178–179, 18, 181, 183–184, 186, 193–194, 196–197, 20, 200, 200, 207, 207, 21, 211, 22, 22, 22–24, 28, 32, 39, 46, 53, 60, 67, 84–86, 94–96, 98–99
src/canvas
   Canvas.ts78.66%77.04%82.14%79.33%1001–1003, 1006–1007, 1011–1012, 1123–1125, 1128–1129, 1202, 1314, 1435, 158, 183, 289, 289–294, 299, 322–323, 328–333, 353, 353, 353–354, 354, 354–355, 363, 368–369, 369, 369–370, 372, 381, 387–388, 388, 388, 42, 431, 439, 443, 443, 443–444, 446, 46, 528–529, 529, 529–530, 536, 536, 536–538, 558, 560, 560, 560–561, 561, 561, 564, 564, 564–565, 568, 577–578, 580–581, 583, 583–584, 586–587, 599–600, 600, 600–601, 603–608, 614, 621, 658, 658, 658, 660, 662–667, 673, 679, 679, 679–680, 682, 685, 690, 703, 731, 783–784, 784, 784, 784, 784, 784, 787–788, 791, 791–793, 796–797, 879, 886, 886, 886, 899, 928–929, 929, 929–931, 934–935, 935, 935, 937, 945, 945, 945–947, 947, 947, 954–955, 963–964, 964, 964–965, 971, 973
   CanvasOptions.ts100%100%100%100%
   SelectableCanvas.ts90.22%87.30%94.55%91.74%1004, 1012, 1123, 1125, 1127–1128, 1199, 1220–1221, 1239–1240, 460–461, 463–464, 464, 464, 513–514, 591, 596, 666, 671–672, 699–700, 724, 727–730, 730, 730–731, 731, 731, 737–738, 740, 760–761, 766–770, 772, 807–808, 811, 811, 811–812, 931, 931–932, 935, 955, 955
   StaticCanvas.ts96.51%93.04%98.91%98.28%1014, 1024, 1075–1077, 1080, 1115–1116, 1192, 1201, 1201, 1205, 1205, 1252–1253, 179–180, 196, 554, 566–567, 897–898, 898, 898–899
   StaticCanvasOptions.ts100%100%100%100%
   TextEditingManager.ts86%71.43%91.67%91.67%17, 17, 17–18, 18, 18
src/canvas/DOMManagers
   CanvasDOMManager.ts95.52%70%100%100%21–22, 29
   StaticCanvasDOMManager.ts97.50%88.89%100%100%33
   util.ts86.67%80.56%83.33%93.94%14, 26, 63–64, 67, 67, 74, 93–94
src/color
   Color.ts94.96%91.67%96.30%96.05%233, 258–259, 267–268, 48
   color_map.ts100%100%100%100%
   constants.ts100%100%100%100%
   util.ts85.71%76.92%100%89.74%55–56, 56, 58, 58, 58–59, 61–62, 89
src/controls
   Control.ts94.44%93.10%91.67%96.77%183, 249, 354
   changeWidth.ts100%100%100%100%
   commonControls.ts100%100%100%100%
   controlRendering.ts81.63%78%100%84.78%106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
   drag.ts100%100%100%100%
   fireEvent.ts88.89%75%100%100%13
   polyControl.ts10.87%0%0%16.13%114, 114, 114, 114, 114, 116–119, 119, 122, 129, 24–26, 50–52, 58–59, 61, 71, 77–79, 79, 81, 84, 86, 90–92, 94, 99
   rotate.ts19.57%12.50%50%21.43%41, 45, 51, 51, 51–52, 55–57, 59, 59,

@asturur
Copy link
Member

asturur commented Jan 20, 2024

I ll make an issue with a followup for this PR since the the position of the mouse should be refactored with the proper text utils where possible.
Also one test for me doesn't pass locally

@asturur asturur merged commit 432e1ad into master Jan 20, 2024
22 checks passed
@asturur asturur deleted the test-etc branch January 20, 2024 19:19
@ShaMan123
Copy link
Contributor Author

I ll make an issue with a followup for this PR since the the position of the mouse should be refactored with the proper text utils where possible. Also one test for me doesn't pass locally

My point of view on this is that we should use hardcoded position because it makes the test strict.
If we use a function that depends on what we are testing then the test becomes weaker, don't you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants