Skip to content

Add coverage reporting per OS type#3717

Merged
jsumners merged 2 commits intomainfrom
all-coverage
Feb 20, 2022
Merged

Add coverage reporting per OS type#3717
jsumners merged 2 commits intomainfrom
all-coverage

Conversation

@jsumners
Copy link
Member

@jsumners jsumners commented Feb 20, 2022

Currently, our CI code coverage reporting depends upon the unit tests succeeding. Which means if a unit run fails, no coverage report is uploaded to Coveralls for that suite. When the suite fails due to missing coverage, we have no easy way of seeing what coverage loss generated the error. In particular, non-Windows people cannot see the coverage report in https://github.com/fastify/fastify/runs/5259373178?check_suite_focus=true

This pull request adds a new step to our CI that generates coverage reports subsequent to linting and prior to unit testing. The result of this coverage report will be attached to the Actions panel for the CI run as a zip file. This zip file can be downloaded and explored with a web browser.

I think this step could also be used to generate an lcov file to upload to Coveralls and thus skip the final Coveralls step. But I'm not sure how to accomplish that with the various OS and Node.js matrices we use along with Coveralls's "parallel" feature. Probably not worth the effort to figure it out, so I haven't.

The coverage reports generated by this new CI flow target the current Node.js LTS only. This is done to simply the setup, and reduce the CI run time.

This PR also tweaks a couple items in the ci.yml to speed up multiple runs within a single PR lifecycle, and to use the LTS version of Node in the jobs where we were using a single Node version.

Checklist

@jsumners jsumners force-pushed the all-coverage branch 4 times, most recently from fc81d6f to ec300d4 Compare February 20, 2022 14:28
@jsumners jsumners marked this pull request as draft February 20, 2022 14:32
@jsumners jsumners force-pushed the all-coverage branch 6 times, most recently from 1ee626a to df90892 Compare February 20, 2022 14:47
@jsumners jsumners marked this pull request as ready for review February 20, 2022 15:09
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Co-authored-by: Manuel Spigolon <behemoth89@gmail.com>
@jsumners jsumners merged commit 77680ff into main Feb 20, 2022
@jsumners jsumners deleted the all-coverage branch February 20, 2022 19:19
This was referenced Feb 20, 2022
@jsumners jsumners mentioned this pull request Jul 8, 2023
2 tasks
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants