feat: implement coverage details for process tree #416
Conversation
Show line coverage for each subtree of the tree of spawned processes.
|
Changes Unknown when pulling ce440a9 on addaleax:process-tree-per-subtree-coverage into * on istanbuljs:master*. |
There was a problem hiding this comment.
This is awesome @addaleax.
I think that starting with just the overall coverage per branch is sufficient; it's nice to know how much coverage your seeing in your integration vs. true unit-tests, as an example.
I don't think we need an extra configuration option for this; seems like a lovely thing to always have enabled as soon as you start printing the process tree.
| @@ -460,7 +460,24 @@ NYC.prototype.checkCoverage = function (thresholds) { | |||
| } | |||
|
|
|||
| NYC.prototype._loadProcessInfoTree = function () { | |||
| return ProcessInfo.buildProcessTree(this._loadProcessInfos()) | |||
| var _this = this | |||
| var processTree = ProcessInfo.buildProcessTree(this._loadProcessInfos()) | |||
There was a problem hiding this comment.
I'd be tempted to encapsulate this logic inside process.js; and all that index.js would need to do would be:
console.log(processInfo.render()).
| processTree.getCoverageMap(function (filenames, maps) { | ||
| var map = libCoverage.createCoverageMap({}) | ||
|
|
||
| _this._loadReports(filenames).forEach(function (report) { |
There was a problem hiding this comment.
we can rename _loadReports to loadReports, and process.js can call this method on index.js.
There was a problem hiding this comment.
@bcoe I just updated this PR which what I think you had in mind – maybe you meant something totally different, this feels a bit icky now (as in, this doesn’t seem quite right from a separation of concerns pov?).
| @@ -57,6 +63,20 @@ ProcessInfo.buildProcessTree = function (infos) { | |||
| return treeRoot | |||
| } | |||
|
|
|||
| ProcessInfo.prototype.getCoverageMap = function (merger) { | |||
There was a problem hiding this comment.
This is cool! it works because we already output the coverage for each subprocess to a separate file named after the pid?
There was a problem hiding this comment.
Yep – the coverage filename is included in the file containing the process info. I thought about magically inferring the filename from the PID, but I wanted to keep the flexibility – mostly because I’ve been hearing Bad Things™ about assuming uniqueness of PIDs on Windows. 😄
|
Changes Unknown when pulling 6a3b4b9 on addaleax:process-tree-per-subtree-coverage into * on istanbuljs:master*. |
|
@addaleax love this refactor 👍 |
|
@addaleax give this a spin when you have a sec:
|
Show line coverage for each subtree of the tree of spawned processes.
This is just a start, but while I definitely wanted to begin working in that direction as planned, I’m not too certain about how this is best implemented when it comes to visual display… In theory, a full per-file coverage table could be shown for each subtree, but that seems like overkill, so I’ve just implemented something small. Also, maybe printing this extra information should be optional/configurable, too?
What do you think?