Skip to content

Commit 9648cca

Browse files
hetunanduhetunandu
authored
Fix for new and removed coverage diffs getting marked incorrectly (anuraag016#10)
* Fix for new/removed file diff marked incorrectly * default to 0 * Update report object creation
1 parent 44273b1 commit 9648cca

File tree

3 files changed

+61
-88
lines changed

3 files changed

+61
-88
lines changed

‎__tests__/DiffChecker.test.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,22 +41,28 @@ describe("DiffChecker", () => {
4141
const codeCoverageOld = {
4242
file1: mock99CoverageFile,
4343
file2: mock100CoverageFile,
44-
file3: mockEmptyCoverageFile,
4544
file4: mock100CoverageFile,
45+
file5: mock99CoverageFile,
4646
};
4747
const codeCoverageNew = {
4848
file1: mock100CoverageFile,
4949
file2: mock99CoverageFile,
5050
file3: mock100CoverageFile,
51-
file4: mockEmptyCoverageFile,
51+
file5: {
52+
statements: mock99Coverage,
53+
branches: mockEmptyCoverage,
54+
functions: mock99Coverage,
55+
lines: mock99Coverage,
56+
},
5257
};
5358
const diffChecker = new DiffChecker(codeCoverageNew, codeCoverageOld);
5459
const details = diffChecker.getCoverageDetails(false, "")
5560
expect(details).toStrictEqual([
5661
" :green_circle: | file1 | 100 **(1)** | 100 **(1)** | 100 **(1)** | 100 **(1)**",
5762
" :red_circle: | file2 | 99 **(-1)** | 99 **(-1)** | 99 **(-1)** | 99 **(-1)**",
58-
" :new: | **file3** | **100** | **100** | **100** | **100**",
59-
" :red_circle: | ~~file4~~ | ~~100~~ | ~~100~~ | ~~100~~ | ~~100~~",
63+
" :sparkles: :new: | **file3** | **100** | **100** | **100** | **100**",
64+
" :red_circle: | file5 | 99 **(0)** | 0 **(-99)** | 99 **(0)** | 99 **(0)**",
65+
" :x: | ~~file4~~ | ~~100~~ | ~~100~~ | ~~100~~ | ~~100~~",
6066
])
6167
});
6268
});

‎dist/index.js‎

Lines changed: 24 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -6679,52 +6679,35 @@ Object.defineProperty(exports, "__esModule", { value: true });
66796679
exports.DiffChecker = void 0;
66806680
const increasedCoverageIcon = ':green_circle:';
66816681
const decreasedCoverageIcon = ':red_circle:';
6682-
const newCoverageIcon = ':new:';
6682+
const newCoverageIcon = ':sparkles: :new:';
6683+
const removedCoverageIcon = ':x:';
66836684
class DiffChecker {
66846685
constructor(coverageReportNew, coverageReportOld) {
6686+
var _a, _b, _c, _d, _e, _f, _g, _h;
66856687
this.diffCoverageReport = {};
66866688
const reportNewKeys = Object.keys(coverageReportNew);
6687-
for (const key of reportNewKeys) {
6688-
this.diffCoverageReport[key] = {
6689+
const reportOldKeys = Object.keys(coverageReportOld);
6690+
const reportKeys = new Set([...reportNewKeys, ...reportOldKeys]);
6691+
for (const filePath of reportKeys) {
6692+
this.diffCoverageReport[filePath] = {
66896693
branches: {
6690-
newPct: this.getPercentage(coverageReportNew[key].branches)
6694+
newPct: this.getPercentage((_a = coverageReportNew[filePath]) === null || _a === void 0 ? void 0 : _a.branches),
6695+
oldPct: this.getPercentage((_b = coverageReportOld[filePath]) === null || _b === void 0 ? void 0 : _b.branches)
66916696
},
66926697
statements: {
6693-
newPct: this.getPercentage(coverageReportNew[key].statements)
6698+
newPct: this.getPercentage((_c = coverageReportNew[filePath]) === null || _c === void 0 ? void 0 : _c.statements),
6699+
oldPct: this.getPercentage((_d = coverageReportOld[filePath]) === null || _d === void 0 ? void 0 : _d.statements)
66946700
},
66956701
lines: {
6696-
newPct: this.getPercentage(coverageReportNew[key].lines)
6702+
newPct: this.getPercentage((_e = coverageReportNew[filePath]) === null || _e === void 0 ? void 0 : _e.lines),
6703+
oldPct: this.getPercentage((_f = coverageReportOld[filePath]) === null || _f === void 0 ? void 0 : _f.lines)
66976704
},
66986705
functions: {
6699-
newPct: this.getPercentage(coverageReportNew[key].functions)
6706+
newPct: this.getPercentage((_g = coverageReportNew[filePath]) === null || _g === void 0 ? void 0 : _g.functions),
6707+
oldPct: this.getPercentage((_h = coverageReportOld[filePath]) === null || _h === void 0 ? void 0 : _h.functions)
67006708
}
67016709
};
67026710
}
6703-
const reportOldKeys = Object.keys(coverageReportOld);
6704-
for (const key of reportOldKeys) {
6705-
if (this.diffCoverageReport[key]) {
6706-
this.diffCoverageReport[key].statements.oldPct = this.getPercentage(coverageReportOld[key].statements);
6707-
this.diffCoverageReport[key].branches.oldPct = this.getPercentage(coverageReportOld[key].branches);
6708-
this.diffCoverageReport[key].functions.oldPct = this.getPercentage(coverageReportOld[key].functions);
6709-
this.diffCoverageReport[key].lines.oldPct = this.getPercentage(coverageReportOld[key].lines);
6710-
}
6711-
else {
6712-
this.diffCoverageReport[key] = {
6713-
branches: {
6714-
oldPct: this.getPercentage(coverageReportOld[key].branches)
6715-
},
6716-
statements: {
6717-
oldPct: this.getPercentage(coverageReportOld[key].statements)
6718-
},
6719-
lines: {
6720-
oldPct: this.getPercentage(coverageReportOld[key].lines)
6721-
},
6722-
functions: {
6723-
oldPct: this.getPercentage(coverageReportOld[key].functions)
6724-
}
6725-
};
6726-
}
6727-
}
67286711
}
67296712
getCoverageDetails(diffOnly, currentDirectory) {
67306713
const keys = Object.keys(this.diffCoverageReport);
@@ -6757,13 +6740,15 @@ class DiffChecker {
67576740
return false;
67586741
}
67596742
createDiffLine(name, diffFileCoverageData) {
6760-
if (!diffFileCoverageData.branches.oldPct) {
6761-
// No old coverage found so that means we added a new file coverage
6743+
// No old coverage found so that means we added a new file coverage
6744+
const fileNewCoverage = Object.values(diffFileCoverageData).every(coverageData => coverageData.oldPct === 0);
6745+
// No new coverage found so that means we deleted a file coverage
6746+
const fileRemovedCoverage = Object.values(diffFileCoverageData).every(coverageData => coverageData.newPct === 0);
6747+
if (fileNewCoverage) {
67626748
return ` ${newCoverageIcon} | **${name}** | **${diffFileCoverageData.statements.newPct}** | **${diffFileCoverageData.branches.newPct}** | **${diffFileCoverageData.functions.newPct}** | **${diffFileCoverageData.lines.newPct}**`;
67636749
}
6764-
else if (!diffFileCoverageData.branches.newPct) {
6765-
// No new coverage found so that means we added a new deleted coverage
6766-
return ` ${decreasedCoverageIcon} | ~~${name}~~ | ~~${diffFileCoverageData.statements.oldPct}~~ | ~~${diffFileCoverageData.branches.oldPct}~~ | ~~${diffFileCoverageData.functions.oldPct}~~ | ~~${diffFileCoverageData.lines.oldPct}~~`;
6750+
else if (fileRemovedCoverage) {
6751+
return ` ${removedCoverageIcon} | ~~${name}~~ | ~~${diffFileCoverageData.statements.oldPct}~~ | ~~${diffFileCoverageData.branches.oldPct}~~ | ~~${diffFileCoverageData.functions.oldPct}~~ | ~~${diffFileCoverageData.lines.oldPct}~~`;
67676752
}
67686753
// Coverage existed before so calculate the diff status
67696754
const statusIcon = this.getStatusIcon(diffFileCoverageData);
@@ -6779,7 +6764,7 @@ class DiffChecker {
67796764
return 0;
67806765
}
67816766
getPercentage(coverageData) {
6782-
return coverageData.pct;
6767+
return (coverageData === null || coverageData === void 0 ? void 0 : coverageData.pct) || 0;
67836768
}
67846769
getStatusIcon(diffFileCoverageData) {
67856770
let overallDiff = 0;
@@ -9732,4 +9717,4 @@ function onceStrict (fn) {
97329717

97339718
/***/ })
97349719

9735-
/******/ });
9720+
/******/ });

‎src/DiffChecker.ts‎

Lines changed: 27 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ import {DiffCoverageData} from './Model/DiffCoverageData'
66

77
const increasedCoverageIcon = ':green_circle:'
88
const decreasedCoverageIcon = ':red_circle:'
9-
const newCoverageIcon = ':new:'
9+
const newCoverageIcon = ':sparkles: :new:'
10+
const removedCoverageIcon = ':x:'
1011

1112
export class DiffChecker {
1213
private diffCoverageReport: DiffCoverageReport = {}
@@ -15,51 +16,26 @@ export class DiffChecker {
1516
coverageReportOld: CoverageReport
1617
) {
1718
const reportNewKeys = Object.keys(coverageReportNew)
18-
for (const key of reportNewKeys) {
19-
this.diffCoverageReport[key] = {
19+
const reportOldKeys = Object.keys(coverageReportOld)
20+
const reportKeys = new Set([...reportNewKeys, ...reportOldKeys])
21+
22+
for (const filePath of reportKeys) {
23+
this.diffCoverageReport[filePath] = {
2024
branches: {
21-
newPct: this.getPercentage(coverageReportNew[key].branches)
25+
newPct: this.getPercentage(coverageReportNew[filePath]?.branches),
26+
oldPct: this.getPercentage(coverageReportOld[filePath]?.branches)
2227
},
2328
statements: {
24-
newPct: this.getPercentage(coverageReportNew[key].statements)
29+
newPct: this.getPercentage(coverageReportNew[filePath]?.statements),
30+
oldPct: this.getPercentage(coverageReportOld[filePath]?.statements)
2531
},
2632
lines: {
27-
newPct: this.getPercentage(coverageReportNew[key].lines)
33+
newPct: this.getPercentage(coverageReportNew[filePath]?.lines),
34+
oldPct: this.getPercentage(coverageReportOld[filePath]?.lines)
2835
},
2936
functions: {
30-
newPct: this.getPercentage(coverageReportNew[key].functions)
31-
}
32-
}
33-
}
34-
const reportOldKeys = Object.keys(coverageReportOld)
35-
for (const key of reportOldKeys) {
36-
if (this.diffCoverageReport[key]) {
37-
this.diffCoverageReport[key].statements.oldPct = this.getPercentage(
38-
coverageReportOld[key].statements
39-
)
40-
this.diffCoverageReport[key].branches.oldPct = this.getPercentage(
41-
coverageReportOld[key].branches
42-
)
43-
this.diffCoverageReport[key].functions.oldPct = this.getPercentage(
44-
coverageReportOld[key].functions
45-
)
46-
this.diffCoverageReport[key].lines.oldPct = this.getPercentage(
47-
coverageReportOld[key].lines
48-
)
49-
} else {
50-
this.diffCoverageReport[key] = {
51-
branches: {
52-
oldPct: this.getPercentage(coverageReportOld[key].branches)
53-
},
54-
statements: {
55-
oldPct: this.getPercentage(coverageReportOld[key].statements)
56-
},
57-
lines: {
58-
oldPct: this.getPercentage(coverageReportOld[key].lines)
59-
},
60-
functions: {
61-
oldPct: this.getPercentage(coverageReportOld[key].functions)
62-
}
37+
newPct: this.getPercentage(coverageReportNew[filePath]?.functions),
38+
oldPct: this.getPercentage(coverageReportOld[filePath]?.functions)
6339
}
6440
}
6541
}
@@ -114,12 +90,18 @@ export class DiffChecker {
11490
name: string,
11591
diffFileCoverageData: DiffFileCoverageData
11692
): string {
117-
if (!diffFileCoverageData.branches.oldPct) {
118-
// No old coverage found so that means we added a new file coverage
93+
// No old coverage found so that means we added a new file coverage
94+
const fileNewCoverage = Object.values(diffFileCoverageData).every(
95+
coverageData => coverageData.oldPct === 0
96+
)
97+
// No new coverage found so that means we deleted a file coverage
98+
const fileRemovedCoverage = Object.values(diffFileCoverageData).every(
99+
coverageData => coverageData.newPct === 0
100+
)
101+
if (fileNewCoverage) {
119102
return ` ${newCoverageIcon} | **${name}** | **${diffFileCoverageData.statements.newPct}** | **${diffFileCoverageData.branches.newPct}** | **${diffFileCoverageData.functions.newPct}** | **${diffFileCoverageData.lines.newPct}**`
120-
} else if (!diffFileCoverageData.branches.newPct) {
121-
// No new coverage found so that means we added a new deleted coverage
122-
return ` ${decreasedCoverageIcon} | ~~${name}~~ | ~~${diffFileCoverageData.statements.oldPct}~~ | ~~${diffFileCoverageData.branches.oldPct}~~ | ~~${diffFileCoverageData.functions.oldPct}~~ | ~~${diffFileCoverageData.lines.oldPct}~~`
103+
} else if (fileRemovedCoverage) {
104+
return ` ${removedCoverageIcon} | ~~${name}~~ | ~~${diffFileCoverageData.statements.oldPct}~~ | ~~${diffFileCoverageData.branches.oldPct}~~ | ~~${diffFileCoverageData.functions.oldPct}~~ | ~~${diffFileCoverageData.lines.oldPct}~~`
123105
}
124106
// Coverage existed before so calculate the diff status
125107
const statusIcon = this.getStatusIcon(diffFileCoverageData)
@@ -149,7 +131,7 @@ export class DiffChecker {
149131
}
150132

151133
private getPercentage(coverageData: CoverageData): number {
152-
return coverageData.pct
134+
return coverageData?.pct || 0
153135
}
154136

155137
private getStatusIcon(

0 commit comments

Comments
 (0)