docs: Describe PR Code Coverage #18814
Conversation
|
Thanks, addressed your suggestions. |
doc/developer/code-coverage.md
Outdated
|
|
||
| ## Motivation | ||
|
|
||
| A full [code coverage report](http://65.109.125.29/coverage_01879007-43c4-49db-a81d-a44e93dfb55a/) allows you to check for each part of the code if there is a test that currently covers it by executing the relevant lines of code. Unfortunately the motivation to increase code coverage using full reports is low, and the effort to understand why no existing mzcompose-based test covers the code in question is high. |
There was a problem hiding this comment.
I think you should avoid linking directly to this machine. I would have suggested that you link to a buildkite artifact, but those can expire , so I do not have a suitable link to suggest as a replacement at this time.
There was a problem hiding this comment.
I also thought about that and don't have a good place to put it yet, but wanted to have a demo report somewhere. Removed the link for now until we have a better solution.
|
|
||
|  | ||
|
|
||
| After some time your [Coverage run](https://buildkite.com/materialize/coverage/builds/26) will be finished and contain a summary of any uncovered lines in the form of a `git diff` where each removed line is not covered: |
There was a problem hiding this comment.
I was thinking about this the other day. It may be easier to explain this if we used git diff --output-indicator-old=! , to use "!", a neutral yet alarming symbol, so that people are not confused by the "-". This will slightly reduce the cognitive load required to process the diff. (no need to do this in this PR)
There was a problem hiding this comment.
Great suggestion 👍, I was also thinking about replacing the - with something else (but was not aware that this can already be specified in the git command).
There was a problem hiding this comment.
Good idea, that confused people already, will do so in follow-up.
For reviewing: https://github.com/MaterializeInc/materialize/blob/3878dccf196ea103267e7b34e8864e075d768335/doc/developer/code-coverage.md
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way) and therefore is tagged with aT-protolabel.