fix(ci): PR diff coverage uses base branch SHAs and aggregate 80% gate

[abdulraqeeb33]

Summary

Aligns diff coverage with the PR’s chosen base branch and clarifies what the check enforces.

Changes

• PR diff source: On pull_request, use github.event.pull_request.base.sha and head.sha so the measured patch matches GitHub’s PR (any base branch, not main only and not the merge ref alone).
• Gate: Require aggregate coverage on touched executable lines ≥ DIFF_COVERAGE_THRESHOLD (80%). Per-file numbers remain in the report as context.
• Safety: If src/main Kotlin/Java files change in the diff but none map to the merged JaCoCo report, the job fails with a clear message.
• UX: Renamed the workflow step so it reads as coverage on changed lines, not a vague “diff tests” label.

Testing

• bash -n OneSignalSDK/coverage/checkCoverage.sh
• CI will exercise the script on this PR.

Made with Cursor

[Copilot]

Pull request overview

Updates the SDK CI diff-coverage check so PR coverage is computed against the PR’s selected base branch (by SHA), and changes the gate to enforce an aggregate threshold across all touched executable lines (while still reporting per-file stats for context).

Changes:

• Add support for computing diffs via DIFF_BASE_SHA...DIFF_HEAD_SHA to match GitHub’s PR patch regardless of base branch.
• Change the enforcement to an aggregate diff-coverage gate (default 80%) across touched executable lines.
• Fail the job when src/main Kotlin/Java files change but can’t be mapped to the merged JaCoCo XML (with a clearer message), and rename the CI step accordingly.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File	Description
OneSignalSDK/coverage/checkCoverage.sh	Adds SHA-based PR diffing, switches the gate to aggregate touched-line coverage, and adds a safety failure mode for unmapped main-source changes.
.github/workflows/ci.yml	Passes PR base/head SHAs into the coverage script and renames the workflow step label to reflect “changed lines” coverage.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In PR mode, the script now relies on git diff $DIFF_BASE_SHA...$DIFF_HEAD_SHA to discover changed files. With the default actions/checkout settings (shallow clone), those SHAs may not exist locally; git diff will fail and COMMITTED_FILES becomes empty, causing the job to incorrectly report “No Kotlin/Java files changed” and skip the coverage gate. To avoid silent false-passes, either fetch sufficient history/objects in the workflow checkout (e.g., fetch-depth: 0 or otherwise fetch both SHAs), and/or make the script explicitly fail when git diff $DIFF_RANGE errors in the SHA-based mode.

[claude]

🔴 The staged/unstaged fallback in get_changed_lines is dead code for local runs because DIFF_RANGE is always exported as a non-empty string before the Python heredoc. For local development where CHANGED_FILES correctly includes staged/unstaged files, the function runs git diff origin/main...HEAD -- file, gets empty output for uncommitted files, and immediately returns None—silently falling back to measuring the entire JaCoCo file rather than just changed lines. This is a regression from the old code that explicitly tried git diff --cached and git diff as fallbacks.

Extended reasoning...

Root cause: In the bash script, DIFF_RANGE is unconditionally set to a non-empty string before it is exported: origin/main...HEAD (local/workflow_dispatch) or BASE_SHA...HEAD_SHA (PR). Python reads it via os.environ.get('DIFF_RANGE', '').strip(), which is therefore always truthy. The if diff_range: branch is always taken, and the else block with staged/cached fallbacks is unreachable dead code.

Code path that triggers it: In the local/workflow_dispatch case, the bash script correctly collects staged and unstaged Kotlin/Java files via git diff --cached --name-only and git diff --name-only, adds them to CHANGED_FILES, and exports DIFF_RANGE=origin/main...HEAD. Python then calls git diff --unified=0 origin/main...HEAD -- for each file. For a file with only staged or unstaged changes, this command returns empty output because the changes are not yet in git commit history. The function hits if result.returncode != 0 or not result.stdout.strip(): return None and exits immediately with no fallback.

Why existing code does not prevent it: The intent was that the else branch would handle the staged/unstaged case. But because DIFF_RANGE is exported unconditionally, Python never reaches the else branch. The regression was introduced when the PR changed the bash script to always export DIFF_RANGE while the Python logic still expects diff_range to be falsy for the local path.

Impact: When get_changed_lines returns None, the loop falls back to relevant_lines = lines (all JaCoCo lines for the file). Coverage is then measured against the entire files executable lines rather than just the changed lines. This silently inflates or deflates reported diff coverage for locally-modified or staged files. No error is emitted. Impact is limited to local development; GitHub PR runs are unaffected (clean checkout, SHA-range diff) and workflow_dispatch CI is also unaffected (clean checkout, no staged/unstaged files).

Step-by-step proof:

1. Developer modifies OneSignalSDK/.../Foo.kt but only stages it (git add), no commit.
2. Bash collects STAGED_FILES via git diff --cached --name-only, picks up Foo.kt.
3. Bash sets DIFF_RANGE=origin/main...HEAD (non-empty) and exports it.
4. Python reads diff_range = 'origin/main...HEAD' (truthy).
5. Python calls git diff --unified=0 origin/main...HEAD -- .../Foo.kt — returns empty because Foo.kt has only staged changes, not committed ones.
6. result.stdout.strip() is empty -> return None with no further fallback.
7. Coverage check uses all JaCoCo lines for Foo.kt instead of just the staged-line numbers, producing wrong coverage numbers.

Fix: Either (a) do not export DIFF_RANGE for the local/workflow_dispatch path (leave it unset) so Python falls through to the else branch with its staged/cached fallbacks, or (b) within the if diff_range: block, add a second-chance fallback to git diff --cached and git diff when the three-dot range returns empty and the range is a branch name rather than a SHA pair.