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(playwright): add snapshots, refactor utils, coverage #9078

Merged
merged 77 commits into from
Jul 17, 2023

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Jul 8, 2023

Motivation

playing around with playwright cause I wanted to write another test

Description

Made the exisitng test create visual snapshots
Refactored util to be more useful
Added coverage reporting
Added an app concept to each test

Changes

Each test is a folder containing:

  • *.spec.ts - playwright test
  • index.ts - fabric test setup:
    • beforeAll
      • a simple hook that creates the canvas, attaches and runs index.ts
      • return a map of objects that playwright can access via eval
      • it is awaited by playwright to avoid race conditions
    • before: same as beforeAll only that is accepts a selector to select the canvas element to operate on
    • afterAll
      • executed by playwright at the end of a test
      • also awaited
    • after: same as afterAll only that is accepts a selector to select the canvas element to operate on
  • index.html
    • used to override the default body (e.g. a test with multiple canvases, offset etc.)

Gist

In Action

@ShaMan123 ShaMan123 requested a review from asturur July 8, 2023 16:31
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

Build Stats

file / KB (diff) bundled minified
fabric 910.800 (0) 303.116 (0)

@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2023

Coverage after merging test-playwright into master will be

83.07%

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%10
   Intersection.ts100%100%100%100%
   Observable.ts87.23%85.29%84.62%89.36%144–145, 170–171, 39–40, 48, 57, 91, 99
   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.05%77.54%83.05%79.57%1000–1001, 1001, 1001–1003, 1005–1006, 1006, 1006, 1008, 1016, 1016, 1016–1018, 1018, 1018, 1024–1025, 1033–1034, 1034, 1034–1035, 1040, 1042, 1073–1075, 1078–1079, 1083–1084, 1197–1199, 1202–1203, 1276, 1395, 1515, 161, 186, 296–297, 300–304, 309, 332–333, 338–343, 36, 363, 363, 363–364, 364, 364–365, 373, 378–379, 379, 379–380, 382, 391, 397–398, 398, 398, 40, 441, 449, 453, 453, 453–454, 456, 538–539, 539, 539–540, 546, 546, 546–548, 568, 570, 570, 570–571, 571, 571, 574, 574, 574–575, 578, 587–588, 590–591, 593, 593–594, 596–597, 609–610, 610, 610–611, 613–618, 624, 631, 668, 668, 668, 670, 672–677, 683, 689, 689, 689–690, 692, 695, 700, 713, 741, 741, 799–800, 800, 800–801, 803, 806–807, 807, 807–808, 810–811, 814, 814–816, 819–820, 890, 902, 909, 930, 962, 983–984
   SelectableCanvas.ts94.43%92.24%94.23%96.17%1115, 1115–1116, 1119, 1139, 1139, 1188, 1196, 1315, 1317, 1319–1320, 519, 694–695, 697–698, 698, 698, 747–748, 809–810, 863–865, 897, 902–903, 930–931
   StaticCanvas.ts96.78%93.09%100%98.53%1036–1037, 1037, 1037–1038, 1158, 1168, 1224–1225, 1228, 1263–1264, 1340, 1349, 1349, 1353, 1353, 1400–1401, 308–309, 325, 693, 705–706
   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.ts5.97%0%0%11.11%100, 105, 119, 119, 119, 119, 119, 121–124, 124, 127, 134, 17, 25–29, 29, 29, 29, 29, 29, 29, 29, 50–56, 56, 56, 56, 56, 58, 63–64, 66, 76, 82–83, 83, 83–84, 88–90, 90, 90, 90, 90, 92
   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.ts84.21%77.78%50%100%14, 17
   index.ts100%100%100%100%

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.

extracted logic from util/objects to ObjectUtil

@ShaMan123 ShaMan123 added the CI/CD label Jul 8, 2023
@ShaMan123 ShaMan123 changed the title test(playwright): tweak initial test test(playwright): add snapshots, refactor utils Jul 8, 2023
@ShaMan123
Copy link
Contributor Author

I really want to use jest inside playwright for object snapshots, is there a way to do that?

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.

Concept

Each test is a folder containing:

  • *.spec.ts - playwright test
  • index.ts - fabric test setup:
    • beforeAll
      • a simple hook that creates the canvas, attaches and runs index.ts
      • return a map of objects that playwright can access via eval
      • it is awaited by playwright to avoid race conditions
    • afterAll
      • executed by playwright at the end of a test
      • also awaited

@@ -1,25 +1,19 @@
<html>
<head>
<script type="text/javascript" src="/dist/index.js"></script>
<title>Fabric e2e Testing Suite</title>
<link rel="icon" type="image/x-icon" href="./favicon.ico" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to silence playwright raising an error

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.

I am DONE

@ShaMan123 ShaMan123 requested a review from luciemars July 17, 2023 05:19
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.

For the last time
DONE
Not touching, I sware

Playwright is a powerful tool indeed, and this PR makes it easy for us to use it and to migrate tests to it.
If it had data snapshots that would have been amazing, testing state during interaction

- name: Install Playwright Browsers
run: npx playwright install --with-deps chromium
- name: Run Playwright tests
run: xvfb-run npm run test:e2e
Copy link
Contributor Author

Choose a reason for hiding this comment

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

do we want/need to run CI in headed mode?
@asturur ?
@mxschmitt would love your input

Copy link
Member

Choose a reason for hiding this comment

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

For browsers test as unit tests in browsers is not needed.
For playwright i don't know the framework so well yet.
i assume that to take screenshots you need xvfb ready unless you are doing them through the browser with an internal browser buffer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you look before this commit you will see I didn't use the headed option thus nkt needing xvfb
But I don't know what the diff is, tests pass in both cases

Copy link
Member

Choose a reason for hiding this comment

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

Let's keep what works, at some point we will optimize instance usage and strain, for now starting the X server on linux who cares

@asturur
Copy link
Member

asturur commented Jul 17, 2023

Let's try this setup.
There is a lot ot absorb tho.

@asturur asturur merged commit 8b8d309 into master Jul 17, 2023
@asturur asturur deleted the test-playwright branch July 17, 2023 15:49
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.

4 participants