Skip to content

Commit 44273b1

Browse files
hetunanduhetunandu
authored
Improve diff comment ui (anuraag016#9)
* Improve diff comment ui * Missed out extra column in table * Update build file * Round percentage diff to two decimal points * Add better heading * Update dist/index.js * Update src/DiffChecker.ts
1 parent 3971cd5 commit 44273b1

File tree

5 files changed

+141
-18
lines changed

5 files changed

+141
-18
lines changed

‎.gitignore

+4-1
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,7 @@ Thumbs.db
9696

9797
# Ignore built ts files
9898
__tests__/runner/*
99-
lib/**/*
99+
lib/**/*
100+
101+
# Ignore editor config
102+
/.idea

‎__tests__/DiffChecker.test.ts

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { DiffChecker } from "../src/DiffChecker";
2+
3+
describe("DiffChecker", () => {
4+
const mock100Coverage = {
5+
total: 100,
6+
covered: 100,
7+
skipped: 0,
8+
pct: 100,
9+
}
10+
const mock99Coverage = {
11+
total: 100,
12+
covered: 99,
13+
skipped: 1,
14+
pct: 99,
15+
}
16+
const mockEmptyCoverage = {
17+
total: 100,
18+
covered: 0,
19+
skipped: 100,
20+
pct: 0,
21+
}
22+
const mock99CoverageFile = {
23+
statements: mock99Coverage,
24+
branches: mock99Coverage,
25+
functions: mock99Coverage,
26+
lines: mock99Coverage,
27+
};
28+
const mock100CoverageFile = {
29+
statements: mock100Coverage,
30+
branches: mock100Coverage,
31+
functions: mock100Coverage,
32+
lines: mock100Coverage,
33+
}
34+
const mockEmptyCoverageFile = {
35+
statements: mockEmptyCoverage,
36+
branches: mockEmptyCoverage,
37+
functions: mockEmptyCoverage,
38+
lines: mockEmptyCoverage,
39+
}
40+
it("generates the correct diff", () => {
41+
const codeCoverageOld = {
42+
file1: mock99CoverageFile,
43+
file2: mock100CoverageFile,
44+
file3: mockEmptyCoverageFile,
45+
file4: mock100CoverageFile,
46+
};
47+
const codeCoverageNew = {
48+
file1: mock100CoverageFile,
49+
file2: mock99CoverageFile,
50+
file3: mock100CoverageFile,
51+
file4: mockEmptyCoverageFile,
52+
};
53+
const diffChecker = new DiffChecker(codeCoverageNew, codeCoverageOld);
54+
const details = diffChecker.getCoverageDetails(false, "")
55+
expect(details).toStrictEqual([
56+
" :green_circle: | file1 | 100 **(1)** | 100 **(1)** | 100 **(1)** | 100 **(1)**",
57+
" :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~~",
60+
])
61+
});
62+
});

‎dist/index.js

+31-9
Original file line numberDiff line numberDiff line change
@@ -2046,15 +2046,16 @@ function run() {
20462046
.toString()
20472047
.trim();
20482048
const diffChecker = new DiffChecker_1.DiffChecker(codeCoverageNew, codeCoverageOld);
2049-
let messageToPost = `Code coverage diff between base branch:${branchNameBase} and head branch: ${branchNameHead} \n`;
2049+
let messageToPost = `## Test coverage results :test_tube: \n
2050+
Code coverage diff between base branch:${branchNameBase} and head branch: ${branchNameHead} \n`;
20502051
const coverageDetails = diffChecker.getCoverageDetails(!fullCoverage, `${currentDirectory}/`);
20512052
if (coverageDetails.length === 0) {
20522053
messageToPost =
20532054
'No changes to code coverage between the base branch and the head branch';
20542055
}
20552056
else {
20562057
messageToPost +=
2057-
'File | % Stmts | % Branch | % Funcs | % Lines \n -----|---------|----------|---------|------ \n';
2058+
'Status | File | % Stmts | % Branch | % Funcs | % Lines \n -----|-----|---------|----------|---------|------ \n';
20582059
messageToPost += coverageDetails.join('\n');
20592060
}
20602061
yield githubClient.issues.createComment({
@@ -6676,6 +6677,9 @@ module.exports = isPlainObject;
66766677

66776678
Object.defineProperty(exports, "__esModule", { value: true });
66786679
exports.DiffChecker = void 0;
6680+
const increasedCoverageIcon = ':green_circle:';
6681+
const decreasedCoverageIcon = ':red_circle:';
6682+
const newCoverageIcon = ':new:';
66796683
class DiffChecker {
66806684
constructor(coverageReportNew, coverageReportOld) {
66816685
this.diffCoverageReport = {};
@@ -6744,9 +6748,7 @@ class DiffChecker {
67446748
const keys = Object.keys(diffCoverageData);
67456749
for (const key of keys) {
67466750
if (diffCoverageData[key].oldPct !== diffCoverageData[key].newPct) {
6747-
const oldValue = Number(diffCoverageData[key].oldPct);
6748-
const newValue = Number(diffCoverageData[key].newPct);
6749-
if (oldValue - newValue > delta) {
6751+
if (-this.getPercentageDiff(diffCoverageData[key]) > delta) {
67506752
return true;
67516753
}
67526754
}
@@ -6756,12 +6758,16 @@ class DiffChecker {
67566758
}
67576759
createDiffLine(name, diffFileCoverageData) {
67586760
if (!diffFileCoverageData.branches.oldPct) {
6759-
return `**${name}** | **${diffFileCoverageData.statements.newPct}** | **${diffFileCoverageData.branches.newPct}** | **${diffFileCoverageData.functions.newPct}** | **${diffFileCoverageData.lines.newPct}**`;
6761+
// No old coverage found so that means we added a new file coverage
6762+
return ` ${newCoverageIcon} | **${name}** | **${diffFileCoverageData.statements.newPct}** | **${diffFileCoverageData.branches.newPct}** | **${diffFileCoverageData.functions.newPct}** | **${diffFileCoverageData.lines.newPct}**`;
67606763
}
67616764
else if (!diffFileCoverageData.branches.newPct) {
6762-
return `~~${name}~~ | ~~${diffFileCoverageData.statements.oldPct}~~ | ~~${diffFileCoverageData.branches.oldPct}~~ | ~~${diffFileCoverageData.functions.oldPct}~~ | ~~${diffFileCoverageData.lines.oldPct}~~`;
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}~~`;
67636767
}
6764-
return `${name} | ~~${diffFileCoverageData.statements.oldPct}~~ **${diffFileCoverageData.statements.newPct}** | ~~${diffFileCoverageData.branches.oldPct}~~ **${diffFileCoverageData.branches.newPct}** | ~~${diffFileCoverageData.functions.oldPct}~~ **${diffFileCoverageData.functions.newPct}** | ~~${diffFileCoverageData.lines.oldPct}~~ **${diffFileCoverageData.lines.newPct}**`;
6768+
// Coverage existed before so calculate the diff status
6769+
const statusIcon = this.getStatusIcon(diffFileCoverageData);
6770+
return ` ${statusIcon} | ${name} | ${diffFileCoverageData.statements.newPct} **(${this.getPercentageDiff(diffFileCoverageData.statements)})** | ${diffFileCoverageData.branches.newPct} **(${this.getPercentageDiff(diffFileCoverageData.branches)})** | ${diffFileCoverageData.functions.newPct} **(${this.getPercentageDiff(diffFileCoverageData.functions)})** | ${diffFileCoverageData.lines.newPct} **(${this.getPercentageDiff(diffFileCoverageData.lines)})**`;
67656771
}
67666772
compareCoverageValues(diffCoverageData) {
67676773
const keys = Object.keys(diffCoverageData);
@@ -6775,6 +6781,22 @@ class DiffChecker {
67756781
getPercentage(coverageData) {
67766782
return coverageData.pct;
67776783
}
6784+
getStatusIcon(diffFileCoverageData) {
6785+
let overallDiff = 0;
6786+
Object.values(diffFileCoverageData).forEach(coverageData => {
6787+
overallDiff = overallDiff + this.getPercentageDiff(coverageData);
6788+
});
6789+
if (overallDiff < 0) {
6790+
return decreasedCoverageIcon;
6791+
}
6792+
return increasedCoverageIcon;
6793+
}
6794+
getPercentageDiff(diffData) {
6795+
// get diff
6796+
const diff = Number(diffData.newPct) - Number(diffData.oldPct);
6797+
// round off the diff to 2 decimal places
6798+
return Math.round((diff + Number.EPSILON) * 100) / 100;
6799+
}
67786800
}
67796801
exports.DiffChecker = DiffChecker;
67806802

@@ -9710,4 +9732,4 @@ function onceStrict (fn) {
97109732

97119733
/***/ })
97129734

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

‎src/DiffChecker.ts

+41-6
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@ import {CoverageReport} from './Model/CoverageReport'
22
import {DiffCoverageReport} from './Model/DiffCoverageReport'
33
import {CoverageData} from './Model/CoverageData'
44
import {DiffFileCoverageData} from './Model/DiffFileCoverageData'
5+
import {DiffCoverageData} from './Model/DiffCoverageData'
6+
7+
const increasedCoverageIcon = ':green_circle:'
8+
const decreasedCoverageIcon = ':red_circle:'
9+
const newCoverageIcon = ':new:'
510

611
export class DiffChecker {
712
private diffCoverageReport: DiffCoverageReport = {}
@@ -95,9 +100,7 @@ export class DiffChecker {
95100
>Object.keys(diffCoverageData)
96101
for (const key of keys) {
97102
if (diffCoverageData[key].oldPct !== diffCoverageData[key].newPct) {
98-
const oldValue: number = Number(diffCoverageData[key].oldPct)
99-
const newValue: number = Number(diffCoverageData[key].newPct)
100-
if (oldValue - newValue > delta) {
103+
if (-this.getPercentageDiff(diffCoverageData[key]) > delta) {
101104
return true
102105
}
103106
}
@@ -112,11 +115,23 @@ export class DiffChecker {
112115
diffFileCoverageData: DiffFileCoverageData
113116
): string {
114117
if (!diffFileCoverageData.branches.oldPct) {
115-
return `**${name}** | **${diffFileCoverageData.statements.newPct}** | **${diffFileCoverageData.branches.newPct}** | **${diffFileCoverageData.functions.newPct}** | **${diffFileCoverageData.lines.newPct}**`
118+
// No old coverage found so that means we added a new file coverage
119+
return ` ${newCoverageIcon} | **${name}** | **${diffFileCoverageData.statements.newPct}** | **${diffFileCoverageData.branches.newPct}** | **${diffFileCoverageData.functions.newPct}** | **${diffFileCoverageData.lines.newPct}**`
116120
} else if (!diffFileCoverageData.branches.newPct) {
117-
return `~~${name}~~ | ~~${diffFileCoverageData.statements.oldPct}~~ | ~~${diffFileCoverageData.branches.oldPct}~~ | ~~${diffFileCoverageData.functions.oldPct}~~ | ~~${diffFileCoverageData.lines.oldPct}~~`
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}~~`
118123
}
119-
return `${name} | ~~${diffFileCoverageData.statements.oldPct}~~ **${diffFileCoverageData.statements.newPct}** | ~~${diffFileCoverageData.branches.oldPct}~~ **${diffFileCoverageData.branches.newPct}** | ~~${diffFileCoverageData.functions.oldPct}~~ **${diffFileCoverageData.functions.newPct}** | ~~${diffFileCoverageData.lines.oldPct}~~ **${diffFileCoverageData.lines.newPct}**`
124+
// Coverage existed before so calculate the diff status
125+
const statusIcon = this.getStatusIcon(diffFileCoverageData)
126+
return ` ${statusIcon} | ${name} | ${
127+
diffFileCoverageData.statements.newPct
128+
} **(${this.getPercentageDiff(diffFileCoverageData.statements)})** | ${
129+
diffFileCoverageData.branches.newPct
130+
} **(${this.getPercentageDiff(diffFileCoverageData.branches)})** | ${
131+
diffFileCoverageData.functions.newPct
132+
} **(${this.getPercentageDiff(diffFileCoverageData.functions)})** | ${
133+
diffFileCoverageData.lines.newPct
134+
} **(${this.getPercentageDiff(diffFileCoverageData.lines)})**`
120135
}
121136

122137
private compareCoverageValues(
@@ -136,4 +151,24 @@ export class DiffChecker {
136151
private getPercentage(coverageData: CoverageData): number {
137152
return coverageData.pct
138153
}
154+
155+
private getStatusIcon(
156+
diffFileCoverageData: DiffFileCoverageData
157+
): ':green_circle:' | ':red_circle:' {
158+
let overallDiff = 0
159+
Object.values(diffFileCoverageData).forEach(coverageData => {
160+
overallDiff = overallDiff + this.getPercentageDiff(coverageData)
161+
})
162+
if (overallDiff < 0) {
163+
return decreasedCoverageIcon
164+
}
165+
return increasedCoverageIcon
166+
}
167+
168+
private getPercentageDiff(diffData: DiffCoverageData): number {
169+
// get diff
170+
const diff = Number(diffData.newPct) - Number(diffData.oldPct)
171+
// round off the diff to 2 decimal places
172+
return Math.round((diff + Number.EPSILON) * 100) / 100
173+
}
139174
}

‎src/main.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ async function run(): Promise<void> {
3535
codeCoverageNew,
3636
codeCoverageOld
3737
)
38-
let messageToPost = `Code coverage diff between base branch:${branchNameBase} and head branch: ${branchNameHead} \n`
38+
let messageToPost = `## Test coverage results :test_tube: \n
39+
Code coverage diff between base branch:${branchNameBase} and head branch: ${branchNameHead} \n`
3940
const coverageDetails = diffChecker.getCoverageDetails(
4041
!fullCoverage,
4142
`${currentDirectory}/`
@@ -45,7 +46,7 @@ async function run(): Promise<void> {
4546
'No changes to code coverage between the base branch and the head branch'
4647
} else {
4748
messageToPost +=
48-
'File | % Stmts | % Branch | % Funcs | % Lines \n -----|---------|----------|---------|------ \n'
49+
'Status | File | % Stmts | % Branch | % Funcs | % Lines \n -----|-----|---------|----------|---------|------ \n'
4950
messageToPost += coverageDetails.join('\n')
5051
}
5152
await githubClient.issues.createComment({

0 commit comments

Comments
 (0)