search

MishaKav / pytest-coverage-comment

Comments a pull request with the pytest code coverage badge and full report
MIT License
181 stars 59 forks source link

Support branch coverage #169

Open eltoder opened 4 months ago

eltoder commented 4 months ago

Branch coverage parsing is only implemented for the xml coverage format. If there is interest, I can look into parsing the text format as well.

eltoder commented 4 months ago

@MishaKav Looks like the workflow doesn't work in forks

MishaKav commented 4 months ago

Thanks for the PR, it's a great feature, it has some problems with forks #68 , I will try to find some time, to test your changes and to merge them.

eltoder commented 4 months ago

@MishaKav thank you. Let me know if you have any comments in the PR, happy to fix or improve it.

eltoder commented 4 months ago

@MishaKav It looks like to fix the issues in the pipeline, the two common approaches are to either

  1. Use the pull_request_target trigger instead of pull_request. Set the minimal permissions for the job, probably {repo: read, issues: write}. There is a small chance that someone will abuse this, but since you review the changes before triggering the workflow, it should be pretty safe. Some background on this is given here.
  2. Add a workflow_run workflow in addition to the pull_request one you have. For this, you'll need to add an option to your action to save the output as a, say, json file instead of posting the comment directly. In the pull_request workflow you will produce that json file and upload it as an artifact. Then in the workflow_run you will download that artifact and post it as a comment. See here for an example. This is clearly more work than option 1, but the advantage is that no untrusted code ever runs with access to your project. Other people using your action to can use this approach as well.

Let me know if you are interested in implementing either of the options.

MishaKav commented 4 months ago

I reviewed your PR in GitHub, and it looks good. Since I don't have UnitTests, I will run manual tests, to check if other things still work and there are no regressions. I believe I will release a new version with your code in the next few days. It's an amazing improvement to the current action!

eltoder commented 4 months ago

@MishaKav thank you! FWIW I've been using this change for two months without issues. The caveat is that I use the xml format, so I did not update the text format parser.