Skip to content

Commit 446d4dd

Browse files
abdulraqeeb33AR Abdul Azeez
authored and
jinliu9508
committed
chore: Restrict test coverage just for the lines of code changed (#2527)
Co-authored-by: AR Abdul Azeez <abdul@onesignal.com>
1 parent f1603cf commit 446d4dd

1 file changed

Lines changed: 149 additions & 19 deletions

File tree

‎OneSignalSDK/coverage/checkCoverage.sh‎

Lines changed: 149 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
# Diff Coverage Check Script
44
# This script generates coverage reports and checks diff coverage against the base branch
5+
# Only checks coverage for newly added/modified lines (not entire files)
56
# Uses a manual coverage check that reliably matches JaCoCo paths to git diff paths
67
#
78
# Usage:
@@ -25,7 +26,10 @@ SKIP_COVERAGE_CHECK=${SKIP_COVERAGE_CHECK:-false} # Set to 'true' to bypass cov
2526

2627
# Get script directory and project root
2728
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
29+
# PROJECT_ROOT is the OneSignalSDK directory (where build reports are)
2830
PROJECT_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
31+
# REPO_ROOT is the git repository root (parent of OneSignalSDK)
32+
REPO_ROOT="$(cd "$PROJECT_ROOT/.." && pwd)"
2933

3034
# Paths relative to project root
3135
COVERAGE_REPORT="$PROJECT_ROOT/build/reports/jacoco/merged/jacocoMergedReport.xml"
@@ -42,7 +46,7 @@ if [ "$SKIP_COVERAGE_CHECK" = "true" ]; then
4246
BYPASS_REASON="SKIP_COVERAGE_CHECK environment variable set"
4347
elif [ -n "$GITHUB_EVENT_NAME" ] && [ "$GITHUB_EVENT_NAME" = "pull_request" ]; then
4448
# Check commit messages for bypass keyword
45-
cd "$PROJECT_ROOT"
49+
cd "$REPO_ROOT"
4650
COMMIT_MESSAGES=$(git log --format=%B origin/main..HEAD 2>/dev/null || git log --format=%B "$BASE_BRANCH"..HEAD 2>/dev/null || echo "")
4751
if echo "$COMMIT_MESSAGES" | grep -qiE "\[skip coverage\]|\[bypass coverage\]|\[no coverage\]"; then
4852
BYPASS_REASON="Commit message contains [skip coverage] keyword"
@@ -71,8 +75,13 @@ echo -e "${YELLOW}[2/3] Checking diff coverage against $BASE_BRANCH...${NC}"
7175
echo -e "${YELLOW}Threshold: ${COVERAGE_THRESHOLD}%${NC}\n"
7276

7377
# Get changed files (run from project root)
74-
cd "$PROJECT_ROOT"
75-
CHANGED_FILES=$(git diff --name-only "$BASE_BRANCH"...HEAD 2>/dev/null | grep -E '\.(kt|java)$' || true)
78+
# Include committed changes, staged changes, and unstaged changes
79+
cd "$REPO_ROOT"
80+
COMMITTED_FILES=$(git diff --name-only "$BASE_BRANCH"...HEAD 2>/dev/null | grep -E '\.(kt|java)$' || true)
81+
STAGED_FILES=$(git diff --cached --name-only 2>/dev/null | grep -E '\.(kt|java)$' || true)
82+
UNSTAGED_FILES=$(git diff --name-only 2>/dev/null | grep -E '\.(kt|java)$' || true)
83+
# Combine all, remove duplicates, and filter to OneSignalSDK files
84+
CHANGED_FILES=$(echo -e "$COMMITTED_FILES\n$STAGED_FILES\n$UNSTAGED_FILES" | grep -E '^OneSignalSDK/' | sort -u || true)
7685

7786
if [ -z "$CHANGED_FILES" ]; then
7887
echo -e "${BLUE}No Kotlin/Java files changed${NC}\n"
@@ -91,17 +100,93 @@ else
91100
export COVERAGE_REPORT
92101
export GENERATE_MARKDOWN
93102
export MARKDOWN_REPORT
103+
export BASE_BRANCH
104+
export REPO_ROOT
94105
python3 << PYEOF
95106
import xml.etree.ElementTree as ET
96107
import re
97108
import sys
98109
import os
110+
import subprocess
99111
100112
coverage_report = os.environ.get('COVERAGE_REPORT')
101113
threshold = int(os.environ.get('COVERAGE_THRESHOLD', '80'))
102114
changed_files_str = """$CHANGED_FILES"""
103115
generate_markdown = os.environ.get('GENERATE_MARKDOWN', 'false').lower() == 'true'
104116
markdown_report = os.environ.get('MARKDOWN_REPORT', 'diff_coverage.md')
117+
base_branch = os.environ.get('BASE_BRANCH', 'origin/main')
118+
repo_root_env = os.environ.get('REPO_ROOT')
119+
120+
def get_changed_lines(file_path, project_root):
121+
"""Get line numbers of added/modified lines from git diff"""
122+
try:
123+
# First try to get diff from committed changes
124+
result = subprocess.run(
125+
['git', 'diff', '--unified=0', base_branch + '...HEAD', '--', file_path],
126+
capture_output=True,
127+
text=True,
128+
cwd=project_root
129+
)
130+
131+
# If no committed changes, check staged changes
132+
if result.returncode != 0 or not result.stdout.strip():
133+
result = subprocess.run(
134+
['git', 'diff', '--cached', '--unified=0', '--', file_path],
135+
capture_output=True,
136+
text=True,
137+
cwd=project_root
138+
)
139+
140+
# If no staged changes, check unstaged changes
141+
if result.returncode != 0 or not result.stdout.strip():
142+
result = subprocess.run(
143+
['git', 'diff', '--unified=0', '--', file_path],
144+
capture_output=True,
145+
text=True,
146+
cwd=project_root
147+
)
148+
149+
# If still nothing, try alternative base branch format
150+
if result.returncode != 0 or not result.stdout.strip():
151+
result = subprocess.run(
152+
['git', 'diff', '--unified=0', base_branch, 'HEAD', '--', file_path],
153+
capture_output=True,
154+
text=True,
155+
cwd=project_root
156+
)
157+
158+
if result.returncode != 0 or not result.stdout.strip():
159+
return None
160+
161+
changed_lines = set()
162+
current_new_line = None
163+
164+
for line in result.stdout.split('\n'):
165+
# Parse unified diff format
166+
# @@ -old_start,old_count +new_start,new_count @@
167+
match = re.match(r'@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@', line)
168+
if match:
169+
current_new_line = int(match.group(1))
170+
count = int(match.group(2)) if match.group(2) else 1
171+
# The count tells us how many lines are in this hunk
172+
# We'll track them as we see + lines
173+
elif line.startswith('+') and not line.startswith('+++'):
174+
# Added/modified line (starts with +)
175+
if current_new_line is not None:
176+
changed_lines.add(current_new_line)
177+
current_new_line += 1
178+
elif line.startswith('-') and not line.startswith('---'):
179+
# Deleted line - don't add to changed_lines, don't increment current_new_line
180+
pass
181+
elif line.startswith(' '):
182+
# Context line (unchanged, starts with space) - increment current_new_line
183+
if current_new_line is not None:
184+
current_new_line += 1
185+
186+
return changed_lines if changed_lines else None
187+
except Exception as e:
188+
# Silently fail and return None - we'll fall back to checking all lines
189+
return None
105190
106191
try:
107192
tree = ET.parse(coverage_report)
@@ -110,6 +195,22 @@ except Exception as e:
110195
print(f"Error parsing coverage report: {e}")
111196
sys.exit(1)
112197
198+
# Get repository root - prefer environment variable, then try to detect from coverage report path
199+
if repo_root_env:
200+
project_root = repo_root_env
201+
else:
202+
# Fallback: try to detect from coverage report path
203+
# Coverage report is in OneSignalSDK/build/..., so go up two levels to get repo root
204+
detected_root = os.path.dirname(os.path.dirname(coverage_report)) if '/build/' in coverage_report else os.path.dirname(coverage_report)
205+
# Look for OneSignalSDK in the path and go one level up
206+
parts = coverage_report.split('/')
207+
if 'OneSignalSDK' in parts:
208+
idx = parts.index('OneSignalSDK')
209+
project_root = '/'.join(parts[:idx])
210+
else:
211+
# Fallback: assume we're in repo root
212+
project_root = os.getcwd()
213+
113214
changed_files = [f.strip() for f in changed_files_str.split('\n') if f.strip()]
114215
115216
total_uncovered = 0
@@ -119,7 +220,7 @@ files_checked = []
119220
markdown_output = []
120221
121222
if generate_markdown:
122-
markdown_output.append("## Diff Coverage Report\n")
223+
markdown_output.append("## Diff Coverage Report (Changed Lines Only)\n")
123224
markdown_output.append(f"**Threshold:** {threshold}%\n\n")
124225
markdown_output.append("### Changed Files Coverage\n\n")
125226
@@ -141,6 +242,9 @@ for changed_file in changed_files:
141242
filename = match.group(3)
142243
package_name = package_path.replace('/', '/')
143244
245+
# Get changed line numbers for this file
246+
changed_lines = get_changed_lines(changed_file, project_root)
247+
144248
# Find in coverage report
145249
found = False
146250
for package in root.findall(f'.//package[@name="{package_name}"]'):
@@ -149,26 +253,38 @@ for changed_file in changed_files:
149253
files_checked.append(filename)
150254
151255
lines = sourcefile.findall('line')
152-
file_total = len([l for l in lines if int(l.get('mi', 0)) > 0 or int(l.get('ci', 0)) > 0])
153-
file_covered = len([l for l in lines if int(l.get('ci', 0)) > 0])
154-
file_uncovered = len([l for l in lines if l.get('ci') == '0' and int(l.get('mi', 0)) > 0])
256+
257+
# Filter to only changed lines if we have that info
258+
if changed_lines is not None and len(changed_lines) > 0:
259+
# Only check lines that were added/modified
260+
relevant_lines = [l for l in lines if int(l.get('nr', 0)) in changed_lines]
261+
else:
262+
# Fallback: check all lines if we can't get changed lines
263+
relevant_lines = lines
264+
265+
# Count only executable lines (mi > 0 means instructions exist)
266+
file_total = len([l for l in relevant_lines if int(l.get('mi', 0)) > 0 or int(l.get('ci', 0)) > 0])
267+
file_covered = len([l for l in relevant_lines if int(l.get('ci', 0)) > 0])
268+
file_uncovered = len([l for l in relevant_lines if l.get('ci') == '0' and int(l.get('mi', 0)) > 0])
155269
156270
if file_total > 0:
157271
total_lines += file_total
158272
total_uncovered += file_uncovered
159-
coverage_pct = (file_covered / file_total * 100)
273+
coverage_pct = (file_covered / file_total * 100) if file_total > 0 else 100
160274
161275
if generate_markdown:
162276
status = "✅" if coverage_pct >= threshold else "❌"
163-
markdown_output.append(f"- {status} **{filename}**: {file_covered}/{file_total} lines ({coverage_pct:.1f}%)")
277+
changed_info = f" ({len(changed_lines)} changed lines)" if changed_lines else " (all lines - could not determine changed lines)"
278+
markdown_output.append(f"- {status} **{filename}**: {file_covered}/{file_total} changed lines ({coverage_pct:.1f}%){changed_info}")
164279
if coverage_pct < threshold:
165280
files_below_threshold.append((filename, coverage_pct, file_uncovered))
166-
markdown_output.append(f" - ⚠️ Below threshold: {file_uncovered} uncovered lines")
281+
markdown_output.append(f" - ⚠️ Below threshold: {file_uncovered} uncovered changed lines")
167282
else:
168283
status = "✓" if coverage_pct >= threshold else "✗"
169284
color = "" if coverage_pct >= threshold else "\033[0;31m"
170285
reset = "\033[0m" if color else ""
171-
print(f" {color}{status}{reset} {filename}: {file_covered}/{file_total} lines ({coverage_pct:.1f}%)")
286+
changed_info = f" ({len(changed_lines)} changed lines)" if changed_lines else " (all lines - could not determine changed lines)"
287+
print(f" {color}{status}{reset} {filename}: {file_covered}/{file_total} changed lines ({coverage_pct:.1f}%){changed_info}")
172288
if coverage_pct < threshold:
173289
files_below_threshold.append((filename, coverage_pct, file_uncovered))
174290
break
@@ -185,14 +301,14 @@ if total_lines > 0:
185301
overall_coverage = ((total_lines - total_uncovered) / total_lines * 100)
186302
187303
if generate_markdown:
188-
markdown_output.append(f"\n### Overall Coverage\n")
189-
markdown_output.append(f"**{total_lines - total_uncovered}/{total_lines}** lines covered ({overall_coverage:.1f}%)\n")
304+
markdown_output.append(f"\n### Overall Coverage (Changed Lines Only)\n")
305+
markdown_output.append(f"**{total_lines - total_uncovered}/{total_lines}** changed lines covered ({overall_coverage:.1f}%)\n")
190306
191307
if files_below_threshold:
192308
markdown_output.append(f"\n### ❌ Coverage Check Failed\n")
193309
markdown_output.append(f"Files below {threshold}% threshold:\n")
194310
for filename, pct, uncovered in files_below_threshold:
195-
markdown_output.append(f"- **{filename}**: {pct:.1f}% ({uncovered} uncovered lines)\n")
311+
markdown_output.append(f"- **{filename}**: {pct:.1f}% ({uncovered} uncovered changed lines)\n")
196312
197313
# Write markdown file
198314
with open(markdown_report, 'w') as f:
@@ -206,15 +322,15 @@ if total_lines > 0:
206322
else:
207323
sys.exit(0)
208324
else:
209-
print(f"\n Overall: {(total_lines - total_uncovered)}/{total_lines} lines covered ({overall_coverage:.1f}%)")
325+
print(f"\n Overall: {(total_lines - total_uncovered)}/{total_lines} changed lines covered ({overall_coverage:.1f}%)")
210326
211327
if files_below_threshold:
212328
print(f"\n Files below {threshold}% threshold:")
213329
for filename, pct, uncovered in files_below_threshold:
214-
print(f" • {filename}: {pct:.1f}% ({uncovered} uncovered lines)")
330+
print(f" • {filename}: {pct:.1f}% ({uncovered} uncovered changed lines)")
215331
sys.exit(1)
216332
else:
217-
print(f"\n ✓ All files meet {threshold}% threshold")
333+
print(f"\n ✓ All files meet {threshold}% threshold for changed lines")
218334
sys.exit(0)
219335
elif files_checked:
220336
# Files were found but had no executable lines
@@ -279,13 +395,27 @@ echo -e "${YELLOW}[3/3] Generating HTML coverage report...${NC}"
279395
# Try to generate HTML report using diff-cover if available, otherwise skip
280396
if python3 -m diff_cover.diff_cover_tool --version &>/dev/null 2>&1; then
281397
# Try diff-cover for HTML report (may not work due to path issues, but worth trying)
282-
cd "$PROJECT_ROOT"
283-
python3 -m diff_cover.diff_cover_tool "build/reports/jacoco/merged/jacocoMergedReport.xml" \
398+
cd "$REPO_ROOT"
399+
# Check if there are uncommitted changes - if so, we need to handle them differently
400+
STAGED_COUNT=$(git diff --cached --name-only 2>/dev/null | grep -E '\.(kt|java)$' | wc -l | tr -d ' ')
401+
UNSTAGED_COUNT=$(git diff --name-only 2>/dev/null | grep -E '\.(kt|java)$' | wc -l | tr -d ' ')
402+
403+
if [ "$STAGED_COUNT" -gt 0 ] || [ "$UNSTAGED_COUNT" -gt 0 ]; then
404+
# There are uncommitted changes - diff-cover won't see them with --compare-branch
405+
# So we'll note this in the output
406+
echo -e "${YELLOW} Note: HTML report shows committed changes only${NC}"
407+
echo -e "${YELLOW} Uncommitted changes are checked in the console output above${NC}"
408+
fi
409+
410+
python3 -m diff_cover.diff_cover_tool "$PROJECT_ROOT/build/reports/jacoco/merged/jacocoMergedReport.xml" \
284411
--compare-branch="$BASE_BRANCH" \
285412
--format html:"$HTML_REPORT" 2>&1 | grep -v "No lines with coverage" || true
286413

287414
if [ -f "$HTML_REPORT" ]; then
288415
echo -e "${GREEN}✓ HTML report generated: $HTML_REPORT${NC}"
416+
if [ "$STAGED_COUNT" -gt 0 ] || [ "$UNSTAGED_COUNT" -gt 0 ]; then
417+
echo -e "${YELLOW} Note: Report shows committed changes only (uncommitted changes shown in console)${NC}"
418+
fi
289419
echo -e "${BLUE} Open it in your browser to see detailed coverage${NC}\n"
290420
else
291421
echo -e "${YELLOW} HTML report generation had issues (non-fatal)${NC}\n"

0 commit comments

Comments
 (0)