Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Adding support for posting test coverage #4

Open
theScud opened this issue Aug 22, 2022 · 8 comments
Open

Adding support for posting test coverage #4

theScud opened this issue Aug 22, 2022 · 8 comments
Labels
enhancement New feature or request

Comments

@theScud
Copy link

theScud commented Aug 22, 2022 •
edited
Loading

Hey @el-hoshino are you already woking on this or can i take a crack at this ?

Just to make sure we are on the same page , these are the Feature Requirements based on what i have seen in https://github.com/f-meloni/danger-swift-coverage

  1. use xccov via XCResult to read code coverage data from xcresult bundle
  2. give option for user to warn or fail build if coverage isn't found
  3. add option filter out (ignore) code coverage for specific targets ie: test or pods coverage
  4. add option to filter and post coverage for changed files. - TBD
@el-hoshino el-hoshino added the enhancement New feature or request label Aug 22, 2022
@el-hoshino
Copy link
Member

@theScud Thanks for your investigation!
I just released 0.1.1 and basic test coverage feature should be included (actually I implemented it in March and I forgot to release it till now...), so it should be able to parse coverage from xcresult bundle now.

The missing features are

    1. give option for user to warn or fail build if coverage isn't found (currently only warning)
    1. add option filter out (ignore) code coverage for specific targets ie: test or pods coverage, and
    1. add option to filter and post coverage for changed files (there is a TODO placeholder though)

I'm busy with my company's work now so if you can implement these for me it would be very appreciated!

Basically all options should be able to found in XCResultParsingConfiguration

@theScud
Copy link
Author

theScud commented Aug 22, 2022

cool i'll push something in a week or so

@theScud
Copy link
Author

theScud commented Aug 22, 2022

are you sure we want to put everything inside XCResultParsingConfiguration , will get kinda bloated no ?

@el-hoshino
Copy link
Member

@theScud As long as all properties can be set in the public initializer, and there is a default value for each property in that public initializer, it should be fine.

@theScud
Copy link
Author

theScud commented Aug 22, 2022

cool

theScud pushed a commit to theScud/danger-swift-kantoku that referenced this issue Aug 23, 2022
Add code to PostCodeCoverage
dec6e57 
1. adds 'failIfCoverageUnavailable' to allow user to fail the build if coverage is'nt avaiable
2. adds 'excludeCoverageTarget' to allow the user to filter out specific targets like pods or tests
3. adds 'showCoverageForChangedFiles' to allow user to show coverage for code changes and 'excludeCoverageForFiles' to exclude specific files
4. add code to support the above config vairables

we first filter out the exculded targets, then exludedFiles. After which we proceed to post the
Overall coverage following the coverage for diff files grouped by target.

yumemi-inc#4
@theScud
Copy link
Author

theScud commented Aug 23, 2022 •
edited
Loading

@el-hoshino i have raised a draft PR
#5

i'll add a few more details in the description tomorrow or day after.

but i am a little confused about the commit log. i pull your merged main into mine and cut a new feature branch. Not sure why i had merge conflicts.

@el-hoshino
Copy link
Member

@theScud If you mean "Merging is blocked" text, don't worry it's not conflict, I just changed the merging settings that it now requires at least 1 approval in code review before merging.

@theScud
Copy link
Author

theScud commented Aug 25, 2022

@el-hoshino not exactly when i raised the PR , initially there were merge conflicts , which was strange. My default setting for Git pull is rebase. so could be just that

# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@theScud @el-hoshino