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(): invoke tests after changelog action #8974

Merged
merged 6 commits into from
Jun 4, 2023
Merged

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Jun 2, 2023

Motivation

Since I have disabled tests when a change is made to the changelog they don't show up in the PR UI after the changelog finishes and it is vital they do.
It is used for one commit PRs because if you continue pushing then tests will start and everything is fine. There are other options I have been using (closing and re-opening a PR and I think I can invoke manually but it is annoying)
We could simply remove the ignore-paths directive instead of what I did in this PR.
Whatever you decide is good for me.

It didn't work so I removed paths-ignore

Description

Changes

Gist

In Action

@ShaMan123 ShaMan123 added the CI/CD label Jun 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023

Build Stats

file / KB (diff) bundled minified
fabric 920.051 (0) 302.319 (0)

@ShaMan123
Copy link
Contributor Author

Well it didn't work so I am going for the second option

@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 2023

Coverage after merging ci-post-changelog into master will be

83.70%

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.21%91.89%90%93.33%116, 127, 136, 29, 92
src/brushes
   BaseBrush.ts100%100%100%100%
   CircleBrush.ts0%0%0%0%108, 108, 108, 110, 112, 114–116, 118–121, 128–129, 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.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%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, 37, 44, 51, 58, 65, 84–86, 94–96, 98–99
src/canvas
   Canvas.ts79.05%77.54%83.05%79.57%1001–1002, 1002, 1002–1004, 1006–1007, 1007, 1007, 1009, 1017, 1017, 1017–1019, 1019, 1019, 1025–1026, 1034–1035, 1035, 1035–1036, 1041, 1043, 1074–1076, 1079–1080, 1084–1085, 1198–1200, 1203–1204, 1277, 1396, 1519, 162, 187, 297–298, 301–305, 310, 333–334, 339–344, 364, 364, 364–365, 365, 365–366, 37, 374, 379–380, 380, 380–381, 383, 392, 398–399, 399, 399, 41, 442, 450, 454, 454, 454–455, 457, 539–540, 540, 540–541, 547, 547, 547–549, 569, 571, 571, 571–572, 572, 572, 575, 575, 575–576, 579, 588–589, 591–592, 594, 594–595, 597–598, 610–611, 611, 611–612, 614–619, 625, 632, 669, 669, 669, 671, 673–678, 684, 690, 690, 690–691, 693, 696, 701, 714, 742, 742, 800–801, 801, 801–802, 804, 807–808, 808, 808–809, 811–812, 815, 815–817, 820–821, 891, 903, 910, 931, 963, 984–985
   SelectableCanvas.ts94.42%92.24%94.23%96.15%1121, 1121–1122, 1125, 1145, 1145, 1194, 1202, 1327, 1329, 1331–1332, 525, 700–701, 703–704, 704, 704, 753–754, 815–816, 869–871, 903, 908–909, 936–937
   StaticCanvas.ts96.73%92.88%100%98.52%1037–1038, 1038, 1038–1039, 1159, 1169, 1223–1224, 1227, 1262–1263, 1339, 1348, 1348, 1352, 1352, 1399–1400, 309–310, 326, 694, 706–707
   TextEditingManager.ts82.69%55.56%91.67%100%15, 15, 15, 15, 15, 15, 15, 15
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
   

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.

DONE

@asturur
Copy link
Member

asturur commented Jun 4, 2023

I think in general if we reduce the amount of tests/checks we run is better, especially the CI infrastructure is free until people abuse it.
Something should be possible and i didn't find a way to do, is that it should be possible to specify the merge strategy for changelog.MD to specify that we always keep both changes and so autoresolve the confilct

@asturur asturur merged commit d5a9553 into master Jun 4, 2023
16 of 17 checks passed
@asturur asturur deleted the ci-post-changelog branch June 4, 2023 10:29
@ShaMan123
Copy link
Contributor Author

ShaMan123 commented Jun 4, 2023

I think in general if we reduce the amount of tests/checks we run is better, especially the CI infrastructure is free until people abuse it.

I agree, that is why I added the ignore paths but the UI is so annoying. Once the changelog action commits, the tests are skipped and the UI doesn't show the previous run.

Something should be possible and i didn't find a way to do, is that it should be possible to specify the merge strategy for changelog.MD to specify that we always keep both changes and so autoresolve the confilct

We have this already in .gitattributes

@ShaMan123
Copy link
Contributor Author

I don't mind reverting this
All it means is that if the changelog action commits I close and re-open the PR so the UI shows the test status

ShaMan123 added a commit that referenced this pull request Jun 12, 2023
ShaMan123 added a commit that referenced this pull request Jun 16, 2023
* Revert "ci(): invoke tests after changelog action (#8974)"

This reverts commit d5a9553.

* Update CHANGELOG.md

---------

Co-authored-by: Andrea Bogazzi <andreabogazzi79@gmail.com>
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