search

dartclub / turf_dart

A turf.js-like geospatial analysis library working with GeoJSON, written in pure Dart.
https://pub.dev/packages/turf
MIT License
67 stars 30 forks source link

Update PR coverage comment step to continue on error #41

Closed baparham closed 2 years ago

baparham commented 2 years ago

ok @lukas-h this finally seems to work, needed the step in the job to continue on fail, and the job itself to continue on fail, so to break out a coverage comment failure vs an actual test failure I broke these up into two jobs in the workflow

baparham commented 2 years ago

I have an idea: Let's move all of the steps after the step "Run tests with coverage enabled" to the new job comment-on-pr, so we have a clearer separation what the two jobs should do:

  • Job 1 (test): Run the dart tests
  • Job 2 (comment-on-pr): Convert to LCOV, generate HTML, and finally comment

By doing that, we would make sure that the unit tests finish. Independently if anything of the coverage fails or not.

lmk what you think

I agree in separating the jobs more cleanly. I'll push some attempts at getting the artifact paths correct to have the tests give the the raw coverage .json files to the coverage job.

baparham commented 2 years ago

Also, I am learning more about the different common workflow triggers, and there appears to be some best practices (of course there are) to handle things like commenting coverage results on PRs.

From https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

..., a new trigger workflow_run was introduced to enable scenarios that require building the untrusted code and also need write permissions to update the PR with e.g. code coverage results or other test results. To do this in a secure manner, the untrusted code must be handled via the pull_request trigger so that it is isolated in an unprivileged environment. The workflow processing the PR should then store any results like code coverage or failed/passed tests in artifacts and exit. The following workflow then starts on workflow_run where it is granted write permission to the target repository and access to repository secrets, so that it can download the artifacts and make any necessary modifications to the repository or interact with third party services that require repository secrets (e.g. API tokens).

While I'm at it, I'll test this flow out ^^^

baparham commented 2 years ago

should work in theory, but probably can't run the new workflow until this is merged in, so it's kind of a catch-22 to debug this more :-\

lukas-h commented 2 years ago

until this is merged in

Let‘s test it in main. It won‘t mess with our releases so it‘s okay.