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

fix(test): Textbox fromObject test #8686

Merged
merged 5 commits into from
Feb 11, 2023
Merged

fix(test): Textbox fromObject test #8686

merged 5 commits into from
Feb 11, 2023

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Feb 10, 2023

Motivation

This test is blocking me while working on features and makes me think my fixes are buggy

You asked for scoped PRs so here it is.
Blocking control set fix.

Description

this test was causing a lot of problems
it took me a lot of time to understand what was wrong
because we spread the input arg in Text.fromObject passing instance to fromObject pollutes the created instance with class props that are enumerable strangly. This might be a node/QUnit thing.

I don't think we should care about it, though it is an edge case.
No sense in passing a live instance to fromObject but if you think differently then we can safeguard it by running cloneDeep before spreading it

Changes

Gist

In Action

this test was causing a lot of problems
it took me a lot of time to understand what is wrong
So because we spread the input arg in `Text.fromObject` passing instance to `fromObject` pollutes the created instance with class props

I don't think we should care about it, though it is an edge case. No sense in passing a live instance to `fromObject` but if you think differently then we can safeguard it by running `cloneDeep` before spreading
@github-actions
Copy link
Contributor

Build Stats

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

@melchiar can you shed some light on this test?

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2023

Coverage after merging fix-textbox-test into master will be

83.03%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
fabric.ts100%100%100%100%
index.node.ts7.69%100%0%14.29%18, 21, 24, 30, 33, 36
src
   Collection.ts95.65%95%89.66%97.83%205–206, 231–232
   CommonMethods.ts100%100%100%100%
   Intersection.ts100%100%100%100%
   Observable.ts87.23%85.29%84.62%89.36%138–139, 164–165, 39–40, 48, 57, 91, 99
   Pattern.ts95.77%94.29%100%96.43%124, 135, 144
   Point.ts100%100%100%100%
   Shadow.ts98.48%96%100%100%156
   cache.ts97.06%90%100%100%56
   config.ts75%64.29%66.67%82.76%138, 138, 138, 138, 138–140, 151–153
   constants.ts100%100%100%100%
   typedefs.ts100%100%100%100%
src/brushes
   BaseBrush.ts100%100%100%100%
   CircleBrush.ts0%0%0%0%100, 102–104, 113, 113, 113, 115, 117, 119–121, 123–126, 133–134, 141, 143, 28–29, 37–41, 45–49, 56–59, 67–71, 73, 81, 81, 81, 81, 81–82, 84, 84, 84–87, 89, 97–98
   PatternBrush.ts97.06%87.50%100%100%21
   PencilBrush.ts91.01%82.35%100%93.75%122–123, 152, 152–154, 176, 176, 276, 280, 285–286, 68–69, 84–85
   SprayBrush.ts0%0%0%0%100–101, 103–104, 112, 112, 112, 112, 112–113, 115–116, 123–124, 126, 128–132, 141, 145–146, 146, 154, 154, 154–157, 159–162, 166–167, 169, 171–174, 177, 184–185, 187, 189–190, 192, 199–200, 202–203, 206, 206, 213, 213, 217, 22–23, 25–27, 27, 27–29, 33, 42, 49, 56, 63, 70, 89–91, 99
src/canvas
   Canvas.ts78.91%77.01%83.08%79.65%1009–1010, 1010, 1010–1012, 1014–1015, 1015, 1015, 1017, 1025, 1025, 1025–1027, 1027, 1027, 1033–1034, 1042–1043, 1043, 1043–1044, 1049, 1051, 1082–1084, 1087–1088, 1092–1093, 1210–1212, 1215–1216, 1289, 1409, 1503–1504, 1510, 1514–1515, 1531, 1553, 1600, 1605, 1653, 171, 196, 305–306, 309–313, 318, 341–342, 347–352, 372, 372, 372–373, 373, 373–374, 38, 382, 387–388, 388, 388–389, 391, 400, 406–407, 407, 407, 42, 450, 458, 462, 462, 462–463, 465, 547–548, 548, 548–549, 555, 555, 555–557, 577, 579, 579, 579–580, 580, 580, 583, 583, 583–584, 587, 596–597, 599–600, 602, 602–603, 605–606, 618–619, 619, 619–620, 622–626, 632, 639, 679, 679, 679, 681, 683–687, 693, 699, 699, 699–700, 702, 705, 710, 723, 750, 750, 808–809, 809, 809–810, 812, 815–816, 816, 816–817, 819–820, 823, 823–825, 828–829, 899, 911, 918, 939, 971, 992–993
   SelectableCanvas.ts93.82%90.71%94.12%96.11%1133, 1133–1134, 1137, 1157, 1157, 1216, 1266–1267, 1288, 1296, 1409–1410, 1412–1413, 1433–1434, 552–553, 558, 568, 701–702, 704–705, 705, 705, 751–752, 813–814, 867–869, 899, 904–905, 934–935
   StaticCanvas.ts95.85%91.04%100%97.92%1083–1084, 1084, 1084–1085, 1205, 1215, 1269–1270, 1273, 1308–1309, 1386, 1395, 1395, 1399, 1399, 1446–1447, 1661, 1661–1662, 282–283, 300, 380–381, 383–384, 745, 757–758, 843
   TextEditingManager.ts100%100%100%100%
src/color
   Color.ts92.16%86.49%100%94.29%330–331, 335–336, 339–340, 58, 88–89, 89, 91, 91, 91–92, 94–95
   color_map.ts100%100%100%100%
   constants.ts100%100%100%100%
   util.ts100%100%100%100%
src/controls
   Control.ts93.90%88.89%90.91%97.73%235, 319, 319, 354
   changeWidth.ts100%100%100%100%
   controls.render.ts81.63%78%100%84.78%106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
   default_controls.ts86.67%66.67%100%100%122, 129
   drag.ts100%100%100%100%
   polyControl.ts6.15%0%0%11.11%100, 105, 119, 119, 119, 121–124, 124, 127, 134, 17, 25–28, 30, 30, 30, 30, 30, 30, 30, 30, 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.49%92.77%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%130–131, 162–163, 170, 176, 178
   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.ts93.75%50%100%100%20
   node.ts76.92%50%66.67%88.24%27, 31–32, 32, 32
src/filters
   BaseFilter.ts19.77%20%30%17.65%100–101, 101, 101–102, 109–112, 112, 112–113, 119, 119,

@asturur asturur merged commit f6e9282 into master Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants