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

ci(e2e): node canvas visual tests #9134

Merged
merged 30 commits into from
Sep 10, 2023
Merged

ci(e2e): node canvas visual tests #9134

merged 30 commits into from
Sep 10, 2023

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Jul 30, 2023

Motivation

Use playwright to do visual diffs with browser and node.

This PR add a test example that is structured as follow:

  • index.ts does not do anything if not running a function that setup the canvas, returns an empty map because no interaction is planned for those tests during the test code.
  • the setup function code is shared in a file, since it needs to be imported also somewhere else
  • the test code has 2 steps: browser and node and they strictly just match the snapshot of the canvas with the golden

to make this possible an util has been added createNodeSnapshot that will take as argument the setup function from the test, running the same function in node, and returning the buffer of the canvas that is what the matchSnapshot function would expect

Description

Added a template that demonstrates how to do use e2e with node canvas.
Fairly simple.

TODO:

Changes

Gist

In Action

@github-actions
Copy link
Contributor

github-actions bot commented Jul 30, 2023

Build Stats

file / KB (diff) bundled minified
fabric 915.345 (0) 305.446 (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.

not bad...
could be better.
for now I think we can move forward
Thoughts?

@ShaMan123
Copy link
Contributor Author

Ahh coverage

@ShaMan123 ShaMan123 requested a review from asturur July 30, 2023 08:18
@asturur
Copy link
Member

asturur commented Aug 2, 2023

i think this would be great, but you have to give me time to digest it. I lost track of all the playwrigth changes. I need to write a test to recover some comfort with it

@asturur
Copy link
Member

asturur commented Aug 31, 2023

tests are becoming flaky already we need to be careful of how we run them.
i think 1-2 at time is maximum if we have many tests.
on my machine they already get stuck easily because macos is eventually unhappy of opening 5 chrome all together

@asturur
Copy link
Member

asturur commented Aug 31, 2023

There is something that doesn't make this test pass and i don't understand what it is.

@ShaMan123
Copy link
Contributor Author

tests are becoming flaky already we need to be careful of how we run them.
i think 1-2 at time is maximum if we have many tests.
on my machine they already get stuck easily because macos is eventually unhappy of opening 5 chrome all together

Did you try to change the parallel config?

@ShaMan123
Copy link
Contributor Author

I also saw jest has a plugin for image diffs...

@asturur
Copy link
Member

asturur commented Sep 3, 2023

the reporting features of playwright are great imho, but anyway, why is this failing? did it ever pass to you in the CI?

@ShaMan123
Copy link
Contributor Author

I will look into it when I am back
Seems the setup is running twice for some reason

@github-actions
Copy link
Contributor

github-actions bot commented Sep 10, 2023

Coverage after merging test-visaul-node-canvas into master will be

82.97%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
index.node.ts7.69%100%0%14.29%17, 20, 23, 35, 38, 41
src
   ClassRegistry.ts100%100%100%100%
   Collection.ts94.71%94.64%86.67%97.09%101, 104, 207–208, 233–234
   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.ts75%66.67%66.67%82.76%130, 138, 138, 138, 138, 138–140, 151–153
   constants.ts100%100%100%100%
src/Pattern
   Pattern.ts92.31%91.89%90%93.55%118, 129, 138, 31, 94
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, 149, 149, 149–152, 154–157, 161–162, 164, 166–169, 17, 172, 179, 18, 180, 182, 184–185, 187, 194–195, 197–198, 20, 201, 201, 208, 208, 21, 212, 22, 22, 22–24, 28, 32, 39, 46, 53, 60, 67, 84–86, 94–96, 98–99
src/canvas
   Canvas.ts79.12%77.68%83.05%79.61%1000, 1003–1004, 1004, 1004, 1006, 1014, 1014, 1014–1016, 1016, 1016, 1023–1024, 1032–1033, 1033, 1033–1034, 1040, 1042, 1073–1075, 1078–1079, 1083–1084, 1197–1199, 1202–1203, 1276, 1395, 1517, 157, 182, 292–293, 296–300, 305, 328–329, 334–339, 359, 359, 359–360, 360, 360–361, 369, 37, 374–375, 375, 375–376, 378, 387, 393–394, 394, 394, 41, 437, 445, 449, 449, 449–450, 452, 535–536, 536, 536–537, 543, 543, 543–545, 565, 567, 567, 567–568, 568, 568, 571, 571, 571–572, 575, 584–585, 587–588, 590, 590–591, 593–594, 606–607, 607, 607–608, 610–615, 621, 628, 665, 665, 665, 667, 669–674, 680, 686, 686, 686–687, 689, 692, 697, 710, 738, 738, 796–797, 797, 797–798, 800, 803–804, 804, 804–805, 807–808, 811, 811–813, 816–817, 887, 899, 906, 927, 959, 980–981, 997–998, 998, 998–999
   CanvasOptions.ts100%100%100%100%
   SelectableCanvas.ts94.44%92.28%94.23%96.18%1099, 1101, 1103–1104, 301, 478–479, 481–482, 482, 482, 531–532, 593–594, 647–649, 681, 686–687, 714–715, 899, 899–900, 903, 923, 923, 972, 980
   StaticCanvas.ts96.78%93.09%100%98.53%1030, 1040, 1092–1093, 1096, 1131–1132, 1208, 1217, 1217, 1221, 1221, 1268–1269, 185–186, 202, 570, 582–583, 913–914, 914, 914–915
   StaticCanvasOptions.ts100%100%100%100%
   TextEditingManager.ts84.31%71.43%91.67%88%17–18, 18, 18–19, 19, 19
src/canvas/DOMManagers
   CanvasDOMManager.ts95.52%70%100%100%21–22, 29
   StaticCanvasDOMManager.ts100%100%100%100%
   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.ts93.33%87.88%91.67%97.78%175, 240, 327, 327, 362
   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.ts6.06%0%0%11.43%103, 117, 117, 117, 117, 117, 119–122, 122, 125, 132, 17, 25–29, 29, 29, 29, 29, 29, 29, 29, 50–56, 56, 56, 56, 56, 58, 63–64, 66, 76, 82–84, 84, 86, 89–90, 90, 90, 90, 90, 92, 97
   rotate.ts19.57%12.50%50%21.43%41, 45, 51, 51, 51–52, 55–57, 59, 59, 59, 59, 59–61, 61, 61–63, 65, 65, 65–67, 67, 67–68, 73, 73, 73–74, 76, 78, 80–81
   scale.ts93.57%92.94%100%93.67%129–130, 132–134, 148–149, 181–183, 42
   scaleSkew.ts78.79%64.29%100%85.71%27, 29, 29, 29, 31, 33, 35
   skew.ts91.03%79.31%100%97.67%131–132, 163–164, 171, 177, 179
   util.ts100%100%100%100%
   wrapWithFireEvent.ts100%100%100%100%
   wrapWithFixedAnchor.ts100%100%100%100%
src/env
   browser.ts

commit bdbb33d8e75fa7cb3e927440a3c45a6192f5f510
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sun Sep 10 13:10:52 2023 +0530

    fix test flakiness

commit dba358c55e68358fc590c47ba92fc430afcc1c55
Author: ShaMan123 <shacharnen@gmail.com>
Date:   Sun Sep 10 13:07:21 2023 +0530

    Update setupSelectors.ts
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.

FIX FLAKINESS

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems that importing these files is what made tests flaky.
Calling the setup as a method seems to fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is also flaky, fixed by trycatch

@ShaMan123
Copy link
Contributor Author

@asturur please merge this asap

@asturur
Copy link
Member

asturur commented Sep 10, 2023

i wonder if this also solves the issue i was having on text tests.
One issue with copy paste test is that it SEEMS to me that 2 text tests running in parallel are sharing the same instance of fabricjs and so one consumes the clipboard data ( styles ) of the other.

@asturur asturur merged commit 8c0cbef into master Sep 10, 2023
24 checks passed
@asturur asturur deleted the test-visaul-node-canvas branch September 10, 2023 12:54
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.

None yet

2 participants