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(): fix build caching #9404

Merged
merged 4 commits into from
Oct 7, 2023
Merged

CI(): fix build caching #9404

merged 4 commits into from
Oct 7, 2023

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Oct 7, 2023

Motivation

closes #9403

Description

The build caching was producing a cache for the push to master event.
That is not desired because each time the build changes.
The cache key was wrong (npm-). It was generic so the stale cache was used for a long time.

Changes

Cache the build only for PRs

Gist

In Action

@ShaMan123 ShaMan123 added the CI/CD label Oct 7, 2023
@ShaMan123 ShaMan123 requested a review from asturur October 7, 2023 00:51
@ShaMan123 ShaMan123 closed this Oct 7, 2023
@ShaMan123 ShaMan123 reopened this Oct 7, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2023

Build Stats

file / KB (diff) bundled minified
fabric 913.552 (0) 305.724 (0)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2023

Coverage after merging ci-fix-build-caching into master will be

82.92%

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%119, 130, 139, 32, 95
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.ts78.99%77.35%83.05%79.61%1003–1004, 1004, 1004–1006, 1009–1010, 1010, 1010, 1012, 1020, 1020, 1020–1022, 1022, 1022, 1029–1030, 1038–1039, 1039, 1039–1040, 1046, 1048, 1079–1081, 1084–1085, 1089–1090, 1209–1211, 1214–1215, 1288, 1407, 1529, 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, 893, 905, 912, 912, 912, 933, 965, 986–987
   CanvasOptions.ts100%100%100%100%
   SelectableCanvas.ts94.35%92.06%94.23%96.20%1080, 1082, 1084–1085, 301, 471–472, 474–475, 475, 475, 524–525, 586–587, 600, 640–642, 674, 679–680, 707–708, 880, 880–881, 884, 904, 904, 953, 961
   StaticCanvas.ts96.78%93.09%100%98.53%1031, 1041, 1093–1094, 1097, 1132–1133, 1209, 1218, 1218, 1222, 1222, 1269–1270, 187–188, 204, 571, 583–584, 914–915, 915, 915–916
   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.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.ts7.25%0%0%13.51%103, 108, 120, 120, 120, 120, 120, 122–125, 125, 128, 135, 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%
   

@asturur
Copy link
Member

asturur commented Oct 7, 2023

This was supposed to rebuild every commit, so i wonder why with master wasn't good.
The sha of head of pr changes every commit and either changes or not changes when merging to master.
In each case the last build should be correct, no?

@asturur
Copy link
Member

asturur commented Oct 7, 2023

oh right the merge event probably doesn't have a github.event.pull_request.head.sha but should have something similar we could have used, to reuse the latest one.

@asturur asturur merged commit 9178224 into master Oct 7, 2023
22 checks passed
@asturur asturur deleted the ci-fix-build-caching branch October 7, 2023 10:18
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.

[Bug CI]: tests are failing when merging to master
2 participants