Refactored filterLineCoverage() and filterBranchCoverage() in renderer.ts #406
Conversation
|
Interesting that the Windows build of this failed. It seems to pass all tests when I run 'npm run test' on my local machine after installing all dependencies so I think the logic with my change is correct. Any thoughts as to why that is? |
Sometimes pulling the vscode image from microsoft's website can fail, and it causes 1/3 build pipelines to implode. |
| @@ -116,22 +116,19 @@ export class Renderer { | |||
| if (!section || !section.lines) { | |||
| return; | |||
| } | |||
| // TODO cleanup this area by using maps, filters, etc | |||
There was a problem hiding this comment.
Thanks for tackling these to-dos!
src/coverage-system/renderer.ts
Outdated
| section.branches.details.forEach((detail) => { | ||
| if (detail.taken === 0) { | ||
| if (detail.line < 0) { return; } | ||
| section.branches.details.filter((detail) => detail.taken === 0 && detail.line >= 0).forEach((detail) => { |
There was a problem hiding this comment.
I like this filter, but we might want to put it on multiple lines to aid in readability (if possible).
section.branches.details
.filter((detail) => detail.taken === 0 && detail.line >= 0)
.forEach((detail) => {
const partialRange = new Range(detail.line - 1, 0, detail.line - 1, 0);
// Evaluates to true if at least one element in range is equal to partialRange
if (coverageLines.full.some((range) => range.isEqual(partialRange))){
coverageLines.full = coverageLines.full.filter((range) => !range.isEqual(partialRange));
coverageLines.partial.push(partialRange);
}
});
src/coverage-system/renderer.ts
Outdated
| // only add a none coverage if no full ones exist | ||
| coverageLines.none.push(lineRange); | ||
| } | ||
| section.lines.details.filter((detail) => detail.line >= 0).forEach((detail) => { |
There was a problem hiding this comment.
Similar here as below (around re-ability and a small tweak to the indent)
section.lines.details
.filter((detail) => detail.line >= 0)
.forEach((detail) => {
const lineRange = new Range(detail.line - 1, 0, detail.line - 1, 0);
if (detail.hit > 0) {
// Evaluates to true if at least one element in range is equal to LineRange
if (coverageLines.none.some((range) => range.isEqual(lineRange))) {
coverageLines.none = coverageLines.none.filter((range) => !range.isEqual(lineRange))
}
coverageLines.full.push(lineRange);
} else {
if (!coverageLines.full.some((range) => range.isEqual(lineRange))) {
// only add a none coverage if no full ones exist
coverageLines.none.push(lineRange);
}
}
});There was a problem hiding this comment.
Makes sense, thanks for the feedback Ryan. I'll make those changes right away.
There was a problem hiding this comment.
Sweet, thanks! I will try to take a look within the week.
|
Looking great, thanks for the contribution @MylesJP! |
Description
Cleanup some to-dos around using more modern typescript array helpers.
Work
.filter()to reduce number of if-statements and consolidated if-statements where possible..some()rather than.find()to determine if match exists, since we only need boolean return..some()returns after first match.Documentation
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some