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

feat: added coverage annotations filters #410

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

xShteff
Copy link

@xShteff xShteff commented Feb 27, 2024

Hi there @ArtiomTr, hope it's alright I open a PR here.

We tried using your action and found it a bit loud. For five lines of code it managed to produce an equal amount of annotations. I've tried to pull out a way to filter out some of these, also updated some of your tests to match the new implementation.

image

Let me know if I missed anything :)

Copy link
Contributor

github-actions bot commented Feb 27, 2024

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements
95.39% (+0.89% 🔼)
848/889
🟢 Branches
88.84% (+1.77% 🔼)
430/484
🟢 Functions
93.14% (+1.01% 🔼)
163/175
🟢 Lines
95.01% (+1.01% 🔼)
762/802
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / getFailedTestsAnnotationsBody.ts
100% 100% 100% 100%
🟢
... / getTestRunSummary.ts
100% 100% 100% 100%
🟢
... / CoverageAnnotationType.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢 typings/Options.ts
97.83% (-2.17% 🔻)
88.24% (+1.57% 🔼)
83.33% (-16.67% 🔻)
97.67% (-2.33% 🔻)
🟢
... / switchBranch.ts
94.87% (+1.12% 🔼)
87.5% (-12.5% 🔻)
100%
94.44% (+1.34% 🔼)
🟢 run.ts
90.82% (-0.21% 🔻)
83.84% (-2.13% 🔻)
100%
90.72% (-0.19% 🔻)
🟡
... / checkThreshold.ts
75.51% (-0.05% 🔻)
50% (-50% 🔻)
46.15% 75%
🟢
... / getNormalThreshold.ts
87.5% (+1.79% 🔼)
50% (-50% 🔻)
100%
85.71% (+2.38% 🔼)
🟢
... / parseJestConfig.ts
100%
80% (-7.5% 🔻)
100% 100%

Test suite run success

168 tests passing in 57 suites.

Report generated by 🧪jest coverage report action from 7fba574

@xShteff
Copy link
Author

xShteff commented Feb 27, 2024

Actually I see the coverage for Options.ts went down - I'll look into it :)

@ArtiomTr
Copy link
Owner

ArtiomTr commented Mar 3, 2024

Hello @xShteff,

Thank you for the contribution. Coverage in Options.ts isn't a big deal.

About your problem - I think it is better to have something like #147, than filtering coverage annotations - you can accidentally filter out important annotations.

@xShteff xShteff marked this pull request as ready for review March 4, 2024 13:49
@xShteff
Copy link
Author

xShteff commented Mar 4, 2024

Thank you for the contribution. Coverage in Options.ts isn't a big deal.

Fair enough, I'll mark it as ready then :)

About your problem - I think it is better to have something like #147, than filtering coverage annotations - you can accidentally filter out important annotations.

This issue comes with any service that allows for customisation. I've allowed by default all the annotations so that existing users will not have to worry about mentioning anything - I still believe that a filter option would be nice to have, as long as the user understands its implications.

Perhaps at some point it could be worth looking into comments as well, but for now this would bring us a lot of value (:

@ArtiomTr
Copy link
Owner

ArtiomTr commented Mar 6, 2024

Perhaps at some point it could be worth looking into comments as well, but for now this would bring us a lot of value (:

Yeah, I understand that it is a good fix to reduce loudness. But my point is - maybe uncovered line annotations will suit your needs? I just don't want to add a new feature, that is likely to be deprecated once uncovered line annotations will land. I believe, that user expects to see which lines are not covered, not statements/branches/functions.

@xShteff
Copy link
Author

xShteff commented Mar 8, 2024

Perhaps at some point it could be worth looking into comments as well, but for now this would bring us a lot of value (:

Yeah, I understand that it is a good fix to reduce loudness. But my point is - maybe uncovered line annotations will suit your needs? I just don't want to add a new feature, that is likely to be deprecated once uncovered line annotations will land. I believe, that user expects to see which lines are not covered, not statements/branches/functions.

It is fair - in the end it's your repo and it's up to you if you want to introduce the feature or not :). In my mind this is not a enforced change but a possibility to make it more customisable to fit our specific use case.

Let me know if I should close the PR :)

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.

None yet

2 participants